diff options
author | Dmitrii Filippov <dmfilippov@google.com> | 2020-10-21 17:03:52 +0200 |
---|---|---|
committer | Luca Milanesio <luca.milanesio@gmail.com> | 2020-11-10 16:15:32 +0000 |
commit | f5d728ede54efad09fe509db61e217b084c2189a (patch) | |
tree | 872c0efe52e5383f620cc4dfd53b2c80572ecb1c | |
parent | 68aac7fb2e84a7f19b442443c827fb9bf2644c9c (diff) |
Cleanup eslint rules and fix some eslint warnings
Change-Id: If4aeaa1bba276ed9c3bfea417af47110f51591f9
-rw-r--r-- | polygerrit-ui/app/.eslintrc.js | 2 | ||||
-rw-r--r-- | polygerrit-ui/app/services/gr-reporting/gr-reporting_impl.ts | 4 | ||||
-rw-r--r-- | polygerrit-ui/app/test/common-test-setup-karma.ts | 3 | ||||
-rw-r--r-- | polygerrit-ui/app/test/common-test-setup.ts | 4 | ||||
-rw-r--r-- | polygerrit-ui/app/types/globals.ts | 93 | ||||
-rw-r--r-- | polygerrit-ui/app/utils/common-util.ts | 3 | ||||
-rw-r--r-- | tools/js/eslint-rules/goog-module-id.js | 160 | ||||
-rw-r--r-- | tools/js/eslint-rules/report-ts-error.js | 101 | ||||
-rw-r--r-- | tools/js/eslint-rules/ts-imports-js.js | 109 |
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, - }); - } - } - }; - } -}; |