summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDmitrii Filippov <dmfilippov@google.com>2020-10-21 17:03:52 +0200
committerLuca Milanesio <luca.milanesio@gmail.com>2020-11-10 16:15:32 +0000
commitf5d728ede54efad09fe509db61e217b084c2189a (patch)
tree872c0efe52e5383f620cc4dfd53b2c80572ecb1c
parent68aac7fb2e84a7f19b442443c827fb9bf2644c9c (diff)
Cleanup eslint rules and fix some eslint warnings
-rw-r--r--polygerrit-ui/app/.eslintrc.js2
-rw-r--r--polygerrit-ui/app/services/gr-reporting/gr-reporting_impl.ts4
-rw-r--r--polygerrit-ui/app/test/common-test-setup-karma.ts3
-rw-r--r--polygerrit-ui/app/test/common-test-setup.ts4
-rw-r--r--polygerrit-ui/app/types/globals.ts93
-rw-r--r--polygerrit-ui/app/utils/common-util.ts3
-rw-r--r--tools/js/eslint-rules/goog-module-id.js160
-rw-r--r--tools/js/eslint-rules/report-ts-error.js101
-rw-r--r--tools/js/eslint-rules/ts-imports-js.js109
9 files changed, 57 insertions, 422 deletions
diff --git a/polygerrit-ui/app/.eslintrc.js b/polygerrit-ui/app/.eslintrc.js
index fe6bd36224..9834ddc664 100644
--- a/polygerrit-ui/app/.eslintrc.js
+++ b/polygerrit-ui/app/.eslintrc.js
@@ -300,8 +300,6 @@ module.exports = {
{
"files": [
"*.html",
- "common-test-setup.js",
- "common-test-setup-karma.js",
"*_test.js",
"a11y-test-utils.js",
],
diff --git a/polygerrit-ui/app/services/gr-reporting/gr-reporting_impl.ts b/polygerrit-ui/app/services/gr-reporting/gr-reporting_impl.ts
index 29cdd1eca5..f3aacdf44b 100644
--- a/polygerrit-ui/app/services/gr-reporting/gr-reporting_impl.ts
+++ b/polygerrit-ui/app/services/gr-reporting/gr-reporting_impl.ts
@@ -127,8 +127,8 @@ export function initErrorReporter(appContext: AppContext) {
oldOnError(msg, url, line, column, error);
}
if (error) {
- line = line || (error as any).lineNumber;
- column = column || (error as any).columnNumber;
+ line = line || error.lineNumber;
+ column = column || error.columnNumber;
let shortenedErrorStack = msg;
if (error.stack) {
const errorStackLines = error.stack.split('\n');
diff --git a/polygerrit-ui/app/test/common-test-setup-karma.ts b/polygerrit-ui/app/test/common-test-setup-karma.ts
index f553eadd04..3d07d8a380 100644
--- a/polygerrit-ui/app/test/common-test-setup-karma.ts
+++ b/polygerrit-ui/app/test/common-test-setup-karma.ts
@@ -86,6 +86,9 @@ function flushImpl(callback?: () => void): Promise<void> | void {
// Ideally, this function would be a call to Polymer.dom.flush, but that
// doesn't support a callback yet
// (https://github.com/Polymer/polymer-dev/issues/851)
+ // The type is used only in one place, disable eslint warning instead of
+ // creating an interface
+ // eslint-disable-next-line @typescript-eslint/no-explicit-any
(window as any).Polymer.dom.flush();
if (callback) {
nativeSetTimeout(callback, 0);
diff --git a/polygerrit-ui/app/test/common-test-setup.ts b/polygerrit-ui/app/test/common-test-setup.ts
index e8a35dcf36..71c45f751f 100644
--- a/polygerrit-ui/app/test/common-test-setup.ts
+++ b/polygerrit-ui/app/test/common-test-setup.ts
@@ -182,9 +182,7 @@ function checkChildAllowed(element: Element) {
`The following node remains in document after the test:
${element.tagName}
Outer HTML:
- ${element.outerHTML},
- Stack trace:
- ${(element as any).stackTrace}`
+ ${element.outerHTML}`
);
}
function checkGlobalSpace() {
diff --git a/polygerrit-ui/app/types/globals.ts b/polygerrit-ui/app/types/globals.ts
index 9ddd5216dd..29621589ea 100644
--- a/polygerrit-ui/app/types/globals.ts
+++ b/polygerrit-ui/app/types/globals.ts
@@ -75,50 +75,50 @@ declare global {
// TODO(TS): should clean up those and removing them may break certain plugin behaviors
// TODO(TS): as @brohlfs suggested, to avoid importing anything from elements/ to types/
// use any for them for now
- GrDisplayNameUtils: any;
- GrAnnotation: any;
- GrAttributeHelper: any;
- GrDiffLine: any;
- GrDiffLineType: any;
- GrDiffGroup: any;
- GrDiffGroupType: any;
- GrDiffBuilder: any;
- GrDiffBuilderSideBySide: any;
- GrDiffBuilderImage: any;
- GrDiffBuilderUnified: any;
- GrDiffBuilderBinary: any;
- GrChangeActionsInterface: any;
- GrChangeReplyInterface: any;
- GrEditConstants: any;
- GrDomHooksManager: any;
- GrDomHook: any;
- GrEtagDecorator: any;
- GrThemeApi: any;
- SiteBasedCache: any;
- FetchPromisesCache: any;
- GrRestApiHelper: any;
- GrLinkTextParser: any;
- GrPluginEndpoints: any;
- GrReviewerUpdatesParser: any;
- GrPopupInterface: any;
- GrCountStringFormatter: any;
- GrReviewerSuggestionsProvider: any;
- util: any;
- Auth: any;
- EventEmitter: any;
- GrAdminApi: any;
- GrAnnotationActionsContext: any;
- GrAnnotationActionsInterface: any;
- GrChangeMetadataApi: any;
- GrEmailSuggestionsProvider: any;
- GrGroupSuggestionsProvider: any;
- GrEventHelper: any;
- GrPluginRestApi: any;
- GrRepoApi: any;
- GrSettingsApi: any;
- GrStylesApi: any;
- PluginLoader: any;
- GrPluginActionContext: any;
+ GrDisplayNameUtils: unknown;
+ GrAnnotation: unknown;
+ GrAttributeHelper: unknown;
+ GrDiffLine: unknown;
+ GrDiffLineType: unknown;
+ GrDiffGroup: unknown;
+ GrDiffGroupType: unknown;
+ GrDiffBuilder: unknown;
+ GrDiffBuilderSideBySide: unknown;
+ GrDiffBuilderImage: unknown;
+ GrDiffBuilderUnified: unknown;
+ GrDiffBuilderBinary: unknown;
+ GrChangeActionsInterface: unknown;
+ GrChangeReplyInterface: unknown;
+ GrEditConstants: unknown;
+ GrDomHooksManager: unknown;
+ GrDomHook: unknown;
+ GrEtagDecorator: unknown;
+ GrThemeApi: unknown;
+ SiteBasedCache: unknown;
+ FetchPromisesCache: unknown;
+ GrRestApiHelper: unknown;
+ GrLinkTextParser: unknown;
+ GrPluginEndpoints: unknown;
+ GrReviewerUpdatesParser: unknown;
+ GrPopupInterface: unknown;
+ GrCountStringFormatter: unknown;
+ GrReviewerSuggestionsProvider: unknown;
+ util: unknown;
+ Auth: unknown;
+ EventEmitter: unknown;
+ GrAdminApi: unknown;
+ GrAnnotationActionsContext: unknown;
+ GrAnnotationActionsInterface: unknown;
+ GrChangeMetadataApi: unknown;
+ GrEmailSuggestionsProvider: unknown;
+ GrGroupSuggestionsProvider: unknown;
+ GrEventHelper: unknown;
+ GrPluginRestApi: unknown;
+ GrRepoApi: unknown;
+ GrSettingsApi: unknown;
+ GrStylesApi: unknown;
+ PluginLoader: unknown;
+ GrPluginActionContext: unknown;
_apiUtils: {};
}
@@ -138,4 +138,9 @@ declare global {
// TODO(TS): replace with composedPath if possible
readonly path: EventTarget[];
}
+
+ interface Error {
+ lineNumber?: number; // non-standard property
+ columnNumber?: number; // non-standard property
+ }
}
diff --git a/polygerrit-ui/app/utils/common-util.ts b/polygerrit-ui/app/utils/common-util.ts
index 183e56cb52..5b332eac49 100644
--- a/polygerrit-ui/app/utils/common-util.ts
+++ b/polygerrit-ui/app/utils/common-util.ts
@@ -33,7 +33,8 @@ export function hasOwnProperty(obj: any, prop: PropertyKey) {
}
// TODO(TS): move to common types once we have type utils
-// tslint:disable-next-line:no-any Required for constructor signature.
+// Required for constructor signature.
+// eslint-disable-next-line @typescript-eslint/no-explicit-any
export type Constructor<T> = new (...args: any[]) => T;
/**
diff --git a/tools/js/eslint-rules/goog-module-id.js b/tools/js/eslint-rules/goog-module-id.js
deleted file mode 100644
index 56cd645e6a..0000000000
--- a/tools/js/eslint-rules/goog-module-id.js
+++ /dev/null
@@ -1,160 +0,0 @@
-/**
- * @license
- * Copyright (C) 2020 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-const fs = require('fs');
-const path = require('path');
-const jsExt = '.js';
-
-class NonJsValidator {
- onProgramEnd(context, node) {
- }
- onGoogDeclareModuleId(context, node) {
- context.report({
- message: 'goog.declareModuleId is allowed only in .js files',
- node: node,
- });
- }
-}
-
-class JsOnlyValidator {
- onProgramEnd(context, node) {
- }
- onGoogDeclareModuleId(context, node) {
- context.report({
- message: 'goog.declareModuleId present, but .d.ts file doesn\'t exist. '
- + 'Either remove goog.declareModuleId or add the .d.ts file.',
- node: node,
- });
- }
-}
-
-class JsWithDtsValidator {
- constructor() {
- this._googDeclareModuleIdExists = false;
- }
- onProgramEnd(context, node) {
- if(!this._googDeclareModuleIdExists) {
- context.report({
- message: 'goog.declareModuleId(...) is missed. ' +
- 'Either add it or remove the associated .d.ts file.',
- node: node,
- })
- }
- }
- onGoogDeclareModuleId(context, node) {
- if(this._googDeclareModuleIdExists) {
- context.report({
- message: 'Duplicated goog.declareModuleId.',
- node: node,
- });
- return;
- }
-
- const filename = context.getFilename();
- this._googDeclareModuleIdExists = true;
-
- const scope = context.getScope();
- if(scope.type !== 'global' && scope.type !== 'module') {
- context.report({
- message: 'goog.declareModuleId is allowed only at the root level.',
- node: node,
- });
- // no return - other problems are possible
- }
- if(node.arguments.length !== 1) {
- context.report({
- message: 'goog.declareModuleId must have exactly one parameter.',
- node: node,
- });
- if(node.arguments.length === 0) {
- return;
- }
- }
-
- const argument = node.arguments[0];
- if(argument.type !== 'Literal') {
- context.report({
- message: 'The argument for the declareModuleId method '
- + 'must be a string literal.',
- node: argument,
- });
- return;
- }
- const pathStart = '/polygerrit-ui/app/';
- const index = filename.lastIndexOf(pathStart);
- if(index < 0) {
- context.report({
- message: 'The file located outside of polygerrit-ui/app directory. ' +
- 'Please check eslint config.',
- node: argument,
- });
- return;
- }
- const expectedName = 'polygerrit.' +
- filename.slice(index + pathStart.length, -jsExt.length)
- .replace(/\//g, '.') // Replace all occurrences of '/' with '.'
- .replace(/-/g, '$2d'); // Replace all occurrences of '-' with '$2d'
- if(argument.value !== expectedName) {
- context.report({
- message: `Invalid module id. It must be '${expectedName}'.`,
- node: argument,
- fix: function(fixer) {
- return fixer.replaceText(argument, `'${expectedName}'`);
- },
- });
- }
- }
-}
-
-module.exports = {
- meta: {
- type: 'problem',
- docs: {
- description: 'Check that goog.declareModuleId is valid',
- category: 'TS imports JS errors',
- recommended: false,
- },
- fixable: "code",
- schema: [],
- },
- create: function (context) {
- let fileValidator;
- return {
- Program: function(node) {
- const filename = context.getFilename();
- if(filename.endsWith(jsExt)) {
- const dtsFilename = filename.slice(0, -jsExt.length) + ".d.ts";
- if(fs.existsSync(dtsFilename)) {
- fileValidator = new JsWithDtsValidator();
- } else {
- fileValidator = new JsOnlyValidator();
- }
- }
- else {
- fileValidator = new NonJsValidator();
- }
- },
- "Program:exit": function(node) {
- fileValidator.onProgramEnd(context, node);
- fileValidator = null;
- },
- 'ExpressionStatement > CallExpression[callee.property.name="declareModuleId"][callee.object.name="goog"]': function(node) {
- fileValidator.onGoogDeclareModuleId(context, node);
- }
- };
- },
-};
diff --git a/tools/js/eslint-rules/report-ts-error.js b/tools/js/eslint-rules/report-ts-error.js
deleted file mode 100644
index 48dddf4039..0000000000
--- a/tools/js/eslint-rules/report-ts-error.js
+++ /dev/null
@@ -1,101 +0,0 @@
-/**
- * @license
- * Copyright (C) 2020 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-// While we are migrating to typescript, gerrit can have .d.ts files.
-// The option "skipLibCheck" is set to true In the tsconfig.json.
-// This is required, because we want to skip type checking in node_modules
-// directory - some .d.ts files in 3rd-party modules are incorrect.
-// Unfortunately, this options also excludes our own .d.ts files from type
-// checking. This rule reports all .ts errors in a file as tslint errors.
-
-function getMassageTextFromChain(chainNode, prefix) {
- let nestedMessages = prefix + chainNode.messageText;
- if (chainNode.next && chainNode.next.length > 0) {
- nestedMessages += "\n";
- for (const node of chainNode.next) {
- nestedMessages +=
- getMassageTextFromChain(node, prefix + " ");
- if(!nestedMessages.endsWith('\n')) {
- nestedMessages += "\n";
- }
- }
- }
- return nestedMessages;
-}
-
-function getMessageText(diagnostic) {
- if (typeof diagnostic.messageText === 'string') {
- return diagnostic.messageText;
- }
- return getMassageTextFromChain(diagnostic.messageText, "");
-}
-
-function getDiagnosticStartAndEnd(diagnostic) {
- if(diagnostic.start) {
- const file = diagnostic.file;
- const start = file.getLineAndCharacterOfPosition(diagnostic.start);
- const length = diagnostic.length ? diagnostic.length : 0;
- return {
- start,
- end: file.getLineAndCharacterOfPosition(diagnostic.start + length),
- };
- }
- return {
- start: {line:0, character: 0},
- end: {line:0, character: 0},
- }
-}
-
-module.exports = {
- meta: {
- type: "problem",
- docs: {
- description: "Reports all typescript problems as linter problems",
- category: ".d.ts",
- recommended: false
- },
- schema: [],
- },
- create: function (context) {
- const program = context.parserServices.program;
- return {
- Program: function(node) {
- const sourceFile =
- context.parserServices.esTreeNodeToTSNodeMap.get(node);
- const allDiagnostics = [
- ...program.getDeclarationDiagnostics(sourceFile),
- ...program.getSemanticDiagnostics(sourceFile)];
- for(const diagnostic of allDiagnostics) {
- const {start, end } = getDiagnosticStartAndEnd(diagnostic);
- context.report({
- message: getMessageText(diagnostic),
- loc: {
- start: {
- line: start.line + 1,
- column: start.character,
- },
- end: {
- line: end.line + 1,
- column: end.character,
- }
- }
- });
- }
- },
- };
- }
-};
diff --git a/tools/js/eslint-rules/ts-imports-js.js b/tools/js/eslint-rules/ts-imports-js.js
deleted file mode 100644
index c7f4278245..0000000000
--- a/tools/js/eslint-rules/ts-imports-js.js
+++ /dev/null
@@ -1,109 +0,0 @@
-/**
- * @license
- * Copyright (C) 2020 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-const path = require('path');
-const fs = require('fs');
-
-function checkImportValid(context, node) {
- const file = context.getFilename();
- const importSource = node.source.value;
-
- if(importSource.startsWith('/')) {
- return {
- message: 'Do not use absolute path for import.',
- };
- }
-
- const targetFile = path.resolve(path.dirname(file), importSource);
- const extName = path.extname(targetFile);
- // There is a polymer.dom.js file, so .dom is not an extension
- if(extName !== '' && !targetFile.endsWith('polymer.dom')) {
- return {
- message: 'Do not specify extensions for import path.',
- fix: function(fixer) {
- return fixer.replaceText(node.source, `'${importSource.slice(0, -extName.length)}'`);
- },
- };
- }
-
- if(!importSource.startsWith('./') && !importSource.startsWith('../')) {
- // Import from node_modules - nothing else to check
- return null;
- }
-
-
- if(fs.existsSync(targetFile + ".ts")) {
- // .ts file exists - nothing to check
- return null;
- }
-
- const jsFileExists = fs.existsSync(targetFile + '.js');
- const dtsFileExists = fs.existsSync(targetFile + '.d.ts');
-
- if(jsFileExists && !dtsFileExists) {
- return {
- message: `The '${importSource}.d.ts' file doesn't exist.`
- };
- }
-
- if(!jsFileExists && dtsFileExists) {
- return {
- message: `The '${importSource}.js' file doesn't exist.`
- };
- }
- // If both files (.js and .d.ts) don't exist, the error is reported by
- // the typescript compiler. Do not report anything from the rule.
- return null;
-}
-
-module.exports = {
- meta: {
- type: "problem",
- docs: {
- description: "Check that TS file can import specific JS file",
- category: "TS imports JS errors",
- recommended: false
- },
- schema: [],
- fixable: "code",
- },
- create: function (context) {
- return {
- Program: function(node) {
- const filename = context.getFilename();
- if(filename.endsWith('.ts') && !filename.endsWith('.d.ts')) {
- return;
- }
- context.report({
- message: 'The rule must be used only with .ts files. ' +
- 'Check eslint settings.',
- node: node,
- });
- },
- ImportDeclaration: function (node) {
- const importProblem = checkImportValid(context, node);
- if(importProblem) {
- context.report({
- message: importProblem.message,
- node: node.source,
- fix: importProblem.fix,
- });
- }
- }
- };
- }
-};