From 28afda4c03f5c918b85cbb75e0d7ccd6f4c1ebf2 Mon Sep 17 00:00:00 2001 From: Yasar Siddiqui Date: Fri, 11 Sep 2020 15:52:47 +0530 Subject: [PATCH 1/5] fix(eslint-plugin): add support for @ts-ignore in block comments --- .../docs/rules/prefer-ts-expect-error.md | 26 ++++++- .../src/rules/prefer-ts-expect-error.ts | 31 ++++---- .../rules/prefer-ts-expect-error.test.ts | 74 +++++++++++++++++-- 3 files changed, 107 insertions(+), 24 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/prefer-ts-expect-error.md b/packages/eslint-plugin/docs/rules/prefer-ts-expect-error.md index 7ec0cac771ba..81452451d3b3 100644 --- a/packages/eslint-plugin/docs/rules/prefer-ts-expect-error.md +++ b/packages/eslint-plugin/docs/rules/prefer-ts-expect-error.md @@ -1,6 +1,6 @@ -# Recommends using `// @ts-expect-error` over `// @ts-ignore` (`prefer-ts-expect-error`) +# Recommends using `@ts-expect-error` over `@ts-ignore` (`prefer-ts-expect-error`) -TypeScript allows you to suppress all errors on a line by placing a single-line comment starting with `@ts-ignore` immediately before the erroring line. +TypeScript allows you to suppress all errors on a line by placing a single-line comment or a comment block line starting with `@ts-ignore` immediately before the erroring line. While powerful, there is no way to know if a `@ts-ignore` is actually suppressing an error without manually investigating what happens when the `@ts-ignore` is removed. This means its easy for `@ts-ignore`s to be forgotten about, and remain in code even after the error they were suppressing is fixed. @@ -20,6 +20,16 @@ Examples of **incorrect** code for this rule: // @ts-ignore const str: string = 1; +{ + /* @ts-ignore */ +} +const value: number = 'value'; + +/** + * @ts-ignore + */ +const block: string = 1; + const isOptionEnabled = (key: string): boolean => { // @ts-ignore: if key isn't in globalOptions it'll be undefined which is false return !!globalOptions[key]; @@ -32,6 +42,16 @@ Examples of **correct** code for this rule: // @ts-expect-error const str: string = 1; +{ + /* @ts-expect-error */ +} +const value: number = 'value'; + +/** + * @ts-expect-error + */ +const block: string = 1; + const isOptionEnabled = (key: string): boolean => { // @ts-expect-error: if key isn't in globalOptions it'll be undefined which is false return !!globalOptions[key]; @@ -40,7 +60,7 @@ const isOptionEnabled = (key: string): boolean => { ## When Not To Use It -If you are not using TypeScript 3.9 (or greater), then you will not be able to use this rule, as the directive is not supported +If you are **NOT** using TypeScript 3.9 (or greater), then you will not be able to use this rule, as the directive is not supported ## Further Reading diff --git a/packages/eslint-plugin/src/rules/prefer-ts-expect-error.ts b/packages/eslint-plugin/src/rules/prefer-ts-expect-error.ts index 1b87b1327861..ab8b105d2d12 100644 --- a/packages/eslint-plugin/src/rules/prefer-ts-expect-error.ts +++ b/packages/eslint-plugin/src/rules/prefer-ts-expect-error.ts @@ -1,5 +1,9 @@ -import { AST_TOKEN_TYPES } from '@typescript-eslint/experimental-utils'; import * as util from '../util'; +import { AST_TOKEN_TYPES } from '@typescript-eslint/experimental-utils'; +import { + RuleFixer, + RuleFix, +} from '@typescript-eslint/experimental-utils/dist/ts-eslint'; type MessageIds = 'preferExpectErrorComment'; @@ -22,30 +26,25 @@ export default util.createRule<[], MessageIds>({ }, defaultOptions: [], create(context) { - const tsIgnoreRegExp = /^\/*\s*@ts-ignore/; + const tsIgnoreRegExp = /^((\/)*\s*(\/\*)*\s*)*\s*@ts-ignore/; const sourceCode = context.getSourceCode(); - return { Program(): void { const comments = sourceCode.getAllComments(); - comments.forEach(comment => { - if (comment.type !== AST_TOKEN_TYPES.Line) { - return; - } - if (tsIgnoreRegExp.test(comment.value)) { + const isCommentSingleLine = comment.type == AST_TOKEN_TYPES.Line; + const commentLineRuleFixer = (fixer: RuleFixer): RuleFix => + fixer.replaceText( + comment, + `//${comment.value.replace('@ts-ignore', '@ts-expect-error')}`, + ); + context.report({ node: comment, messageId: 'preferExpectErrorComment', - fix: fixer => - fixer.replaceText( - comment, - `//${comment.value.replace( - '@ts-ignore', - '@ts-expect-error', - )}`, - ), + // We currently do not support fixer for comment blocks. + fix: isCommentSingleLine ? commentLineRuleFixer : null, }); } }); diff --git a/packages/eslint-plugin/tests/rules/prefer-ts-expect-error.test.ts b/packages/eslint-plugin/tests/rules/prefer-ts-expect-error.test.ts index 8e9c04f09ed6..67cf6154483f 100644 --- a/packages/eslint-plugin/tests/rules/prefer-ts-expect-error.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-ts-expect-error.test.ts @@ -10,12 +10,12 @@ ruleTester.run('prefer-ts-expect-error', rule, { '// @ts-nocheck', '// @ts-check', '// just a comment containing @ts-ignore somewhere', - '/* @ts-ignore */', - '/** @ts-ignore */', ` -/* -// @ts-ignore in a block -*/ +{ + /* + just a comment containing @ts-ignore somewhere in a block + */ +} `, '// @ts-expect-error', ` @@ -81,5 +81,69 @@ if (false) { }, ], }, + { + code: '/* @ts-ignore */', + output: '/* @ts-ignore */', + errors: [ + { + messageId: 'preferExpectErrorComment', + line: 1, + column: 1, + }, + ], + }, + { + code: '/* @ts-ignore in a single block */', + output: '/* @ts-ignore in a single block */', + errors: [ + { + messageId: 'preferExpectErrorComment', + line: 1, + column: 1, + }, + ], + }, + { + code: ` +{ + /* +@ts-ignore in a multiline block +*/ +} + `, + output: ` +{ + /* +@ts-ignore in a multiline block +*/ +} + `, + errors: [ + { + messageId: 'preferExpectErrorComment', + line: 3, + column: 3, + }, + ], + }, + { + code: ` +/* +// @ts-ignore in a block with single line comments +*/ + `, + output: ` +/* +// @ts-ignore in a block with single line comments +*/ + `, + errors: [ + { + messageId: 'preferExpectErrorComment', + line: 2, + column: 1, + }, + ], + }, ], }); From b0a88bed556425ad4d56159717add24f478cfedf Mon Sep 17 00:00:00 2001 From: Yasar Siddiqui Date: Fri, 11 Sep 2020 16:42:14 +0530 Subject: [PATCH 2/5] fix(eslint-plugin): add support for @ts-ignore in block comments --- packages/eslint-plugin/README.md | 2 +- .../src/rules/prefer-ts-expect-error.ts | 21 ++++++++++++------- .../rules/prefer-ts-expect-error.test.ts | 8 +++---- 3 files changed, 19 insertions(+), 12 deletions(-) diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 63b5b6e7f321..8fac3dcde078 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -161,7 +161,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int | [`@typescript-eslint/prefer-reduce-type-parameter`](./docs/rules/prefer-reduce-type-parameter.md) | Prefer using type parameter when calling `Array#reduce` instead of casting | | :wrench: | :thought_balloon: | | [`@typescript-eslint/prefer-regexp-exec`](./docs/rules/prefer-regexp-exec.md) | Enforce that `RegExp#exec` is used instead of `String#match` if no global flag is provided | :heavy_check_mark: | | :thought_balloon: | | [`@typescript-eslint/prefer-string-starts-ends-with`](./docs/rules/prefer-string-starts-ends-with.md) | Enforce the use of `String#startsWith` and `String#endsWith` instead of other equivalent methods of checking substrings | | :wrench: | :thought_balloon: | -| [`@typescript-eslint/prefer-ts-expect-error`](./docs/rules/prefer-ts-expect-error.md) | Recommends using `// @ts-expect-error` over `// @ts-ignore` | | :wrench: | | +| [`@typescript-eslint/prefer-ts-expect-error`](./docs/rules/prefer-ts-expect-error.md) | Recommends using `@ts-expect-error` over `@ts-ignore` | | :wrench: | | | [`@typescript-eslint/promise-function-async`](./docs/rules/promise-function-async.md) | Requires any function or method that returns a Promise to be marked async | | | :thought_balloon: | | [`@typescript-eslint/require-array-sort-compare`](./docs/rules/require-array-sort-compare.md) | Requires `Array#sort` calls to always provide a `compareFunction` | | | :thought_balloon: | | [`@typescript-eslint/restrict-plus-operands`](./docs/rules/restrict-plus-operands.md) | When adding two variables, operands must both be of type number or of type string | :heavy_check_mark: | | :thought_balloon: | diff --git a/packages/eslint-plugin/src/rules/prefer-ts-expect-error.ts b/packages/eslint-plugin/src/rules/prefer-ts-expect-error.ts index ab8b105d2d12..4b674b2f154b 100644 --- a/packages/eslint-plugin/src/rules/prefer-ts-expect-error.ts +++ b/packages/eslint-plugin/src/rules/prefer-ts-expect-error.ts @@ -12,15 +12,14 @@ export default util.createRule<[], MessageIds>({ meta: { type: 'problem', docs: { - description: - 'Recommends using `// @ts-expect-error` over `// @ts-ignore`', + description: 'Recommends using `@ts-expect-error` over `@ts-ignore`', category: 'Best Practices', recommended: false, }, fixable: 'code', messages: { preferExpectErrorComment: - 'Use "// @ts-expect-error" to ensure an error is actually being suppressed.', + 'Use "@ts-expect-error" to ensure an error is actually being suppressed.', }, schema: [], }, @@ -33,18 +32,26 @@ export default util.createRule<[], MessageIds>({ const comments = sourceCode.getAllComments(); comments.forEach(comment => { if (tsIgnoreRegExp.test(comment.value)) { - const isCommentSingleLine = comment.type == AST_TOKEN_TYPES.Line; - const commentLineRuleFixer = (fixer: RuleFixer): RuleFix => + const isLineComment = comment.type == AST_TOKEN_TYPES.Line; + const lineCommentRuleFixer = (fixer: RuleFixer): RuleFix => fixer.replaceText( comment, `//${comment.value.replace('@ts-ignore', '@ts-expect-error')}`, ); + const blockCommentRuleFixer = (fixer: RuleFixer): RuleFix => + fixer.replaceText( + comment, + `/*${comment.value.replace( + '@ts-ignore', + '@ts-expect-error', + )}*/`, + ); + context.report({ node: comment, messageId: 'preferExpectErrorComment', - // We currently do not support fixer for comment blocks. - fix: isCommentSingleLine ? commentLineRuleFixer : null, + fix: isLineComment ? lineCommentRuleFixer : blockCommentRuleFixer, }); } }); diff --git a/packages/eslint-plugin/tests/rules/prefer-ts-expect-error.test.ts b/packages/eslint-plugin/tests/rules/prefer-ts-expect-error.test.ts index 67cf6154483f..16017a36ebb0 100644 --- a/packages/eslint-plugin/tests/rules/prefer-ts-expect-error.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-ts-expect-error.test.ts @@ -83,7 +83,7 @@ if (false) { }, { code: '/* @ts-ignore */', - output: '/* @ts-ignore */', + output: '/* @ts-expect-error */', errors: [ { messageId: 'preferExpectErrorComment', @@ -94,7 +94,7 @@ if (false) { }, { code: '/* @ts-ignore in a single block */', - output: '/* @ts-ignore in a single block */', + output: '/* @ts-expect-error in a single block */', errors: [ { messageId: 'preferExpectErrorComment', @@ -114,7 +114,7 @@ if (false) { output: ` { /* -@ts-ignore in a multiline block +@ts-expect-error in a multiline block */ } `, @@ -134,7 +134,7 @@ if (false) { `, output: ` /* -// @ts-ignore in a block with single line comments +// @ts-expect-error in a block with single line comments */ `, errors: [ From 7047b57096dcbd9c018de4fedc8be0eb6cc38a4d Mon Sep 17 00:00:00 2001 From: Yasar Siddiqui Date: Fri, 11 Sep 2020 16:45:29 +0530 Subject: [PATCH 3/5] fix(eslint-plugin): minor refactor --- packages/eslint-plugin/src/rules/prefer-ts-expect-error.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/prefer-ts-expect-error.ts b/packages/eslint-plugin/src/rules/prefer-ts-expect-error.ts index 4b674b2f154b..2ca811971e70 100644 --- a/packages/eslint-plugin/src/rules/prefer-ts-expect-error.ts +++ b/packages/eslint-plugin/src/rules/prefer-ts-expect-error.ts @@ -32,7 +32,6 @@ export default util.createRule<[], MessageIds>({ const comments = sourceCode.getAllComments(); comments.forEach(comment => { if (tsIgnoreRegExp.test(comment.value)) { - const isLineComment = comment.type == AST_TOKEN_TYPES.Line; const lineCommentRuleFixer = (fixer: RuleFixer): RuleFix => fixer.replaceText( comment, @@ -51,7 +50,10 @@ export default util.createRule<[], MessageIds>({ context.report({ node: comment, messageId: 'preferExpectErrorComment', - fix: isLineComment ? lineCommentRuleFixer : blockCommentRuleFixer, + fix: + comment.type === AST_TOKEN_TYPES.Line + ? lineCommentRuleFixer + : blockCommentRuleFixer, }); } }); From fec7d364736a89df2e4045d97b7f277f592f5dea Mon Sep 17 00:00:00 2001 From: Yasar Siddiqui Date: Sun, 13 Sep 2020 09:55:33 +0530 Subject: [PATCH 4/5] fix(eslint-plugin): improve multipline support and correct example code --- .../docs/rules/prefer-ts-expect-error.md | 26 ++++++++-------- .../src/rules/prefer-ts-expect-error.ts | 19 ++++++++++-- .../rules/prefer-ts-expect-error.test.ts | 30 +++++++++++++++++++ 3 files changed, 58 insertions(+), 17 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/prefer-ts-expect-error.md b/packages/eslint-plugin/docs/rules/prefer-ts-expect-error.md index 81452451d3b3..60fdf79c8069 100644 --- a/packages/eslint-plugin/docs/rules/prefer-ts-expect-error.md +++ b/packages/eslint-plugin/docs/rules/prefer-ts-expect-error.md @@ -20,14 +20,13 @@ Examples of **incorrect** code for this rule: // @ts-ignore const str: string = 1; -{ - /* @ts-ignore */ -} -const value: number = 'value'; - /** - * @ts-ignore - */ + * Explaining comment + * + * @ts-ignore */ +const multiLine: number = 'value'; + +/** @ts-ignore */ const block: string = 1; const isOptionEnabled = (key: string): boolean => { @@ -42,14 +41,13 @@ Examples of **correct** code for this rule: // @ts-expect-error const str: string = 1; -{ - /* @ts-expect-error */ -} -const value: number = 'value'; - /** - * @ts-expect-error - */ + * Explaining comment + * + * @ts-expect-error */ +const multiLine: number = 'value'; + +/** @ts-expect-error */ const block: string = 1; const isOptionEnabled = (key: string): boolean => { diff --git a/packages/eslint-plugin/src/rules/prefer-ts-expect-error.ts b/packages/eslint-plugin/src/rules/prefer-ts-expect-error.ts index 2ca811971e70..af5587baa3d5 100644 --- a/packages/eslint-plugin/src/rules/prefer-ts-expect-error.ts +++ b/packages/eslint-plugin/src/rules/prefer-ts-expect-error.ts @@ -1,5 +1,8 @@ import * as util from '../util'; -import { AST_TOKEN_TYPES } from '@typescript-eslint/experimental-utils'; +import { + AST_TOKEN_TYPES, + TSESTree, +} from '@typescript-eslint/experimental-utils'; import { RuleFixer, RuleFix, @@ -25,13 +28,23 @@ export default util.createRule<[], MessageIds>({ }, defaultOptions: [], create(context) { - const tsIgnoreRegExp = /^((\/)*\s*(\/\*)*\s*)*\s*@ts-ignore/; + const tsIgnoreRegExp = /(^|\n)((\*)*(\/)*\s*(\/\*)*\s*)*\s*@ts-ignore/; const sourceCode = context.getSourceCode(); + + function getLastCommentLine(comment: TSESTree.Comment): string { + if (comment.type === AST_TOKEN_TYPES.Line) { + return comment.value; + } + + // For multiline comments - we look at only the last line. + const commentlines = comment.value.split('/\n'); + return commentlines[commentlines.length - 1]; + } return { Program(): void { const comments = sourceCode.getAllComments(); comments.forEach(comment => { - if (tsIgnoreRegExp.test(comment.value)) { + if (tsIgnoreRegExp.test(getLastCommentLine(comment))) { const lineCommentRuleFixer = (fixer: RuleFixer): RuleFix => fixer.replaceText( comment, diff --git a/packages/eslint-plugin/tests/rules/prefer-ts-expect-error.test.ts b/packages/eslint-plugin/tests/rules/prefer-ts-expect-error.test.ts index 16017a36ebb0..0aa6e5e91f05 100644 --- a/packages/eslint-plugin/tests/rules/prefer-ts-expect-error.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-ts-expect-error.test.ts @@ -24,6 +24,15 @@ if (false) { console.log('hello'); } `, + ` +/** + * Explaining comment + * + * @ts-expect-error + * + * Not last line + * */ + `, ], invalid: [ { @@ -92,6 +101,27 @@ if (false) { }, ], }, + { + code: ` +/** + * Explaining comment + * + * @ts-ignore */ + `, + output: ` +/** + * Explaining comment + * + * @ts-expect-error */ + `, + errors: [ + { + messageId: 'preferExpectErrorComment', + line: 2, + column: 1, + }, + ], + }, { code: '/* @ts-ignore in a single block */', output: '/* @ts-expect-error in a single block */', From 63159fe61bdc4bc95c3e05730cf341e5b0f9f57b Mon Sep 17 00:00:00 2001 From: Yasar Siddiqui Date: Tue, 15 Sep 2020 16:30:37 +0530 Subject: [PATCH 5/5] fix(eslint-plugin): adopt ts regex --- .../src/rules/prefer-ts-expect-error.ts | 28 +++++++++++----- .../rules/prefer-ts-expect-error.test.ts | 33 +++---------------- 2 files changed, 24 insertions(+), 37 deletions(-) diff --git a/packages/eslint-plugin/src/rules/prefer-ts-expect-error.ts b/packages/eslint-plugin/src/rules/prefer-ts-expect-error.ts index af5587baa3d5..d696fc045735 100644 --- a/packages/eslint-plugin/src/rules/prefer-ts-expect-error.ts +++ b/packages/eslint-plugin/src/rules/prefer-ts-expect-error.ts @@ -28,23 +28,36 @@ export default util.createRule<[], MessageIds>({ }, defaultOptions: [], create(context) { - const tsIgnoreRegExp = /(^|\n)((\*)*(\/)*\s*(\/\*)*\s*)*\s*@ts-ignore/; + const tsIgnoreRegExpSingleLine = /^\s*\/?\s*@ts-ignore/; + const tsIgnoreRegExpMultiLine = /^\s*(?:\/|\*)*\s*@ts-ignore/; const sourceCode = context.getSourceCode(); + function isLineComment(comment: TSESTree.Comment): boolean { + return comment.type === AST_TOKEN_TYPES.Line; + } + function getLastCommentLine(comment: TSESTree.Comment): string { - if (comment.type === AST_TOKEN_TYPES.Line) { + if (isLineComment(comment)) { return comment.value; } // For multiline comments - we look at only the last line. - const commentlines = comment.value.split('/\n'); + const commentlines = comment.value.split('\n'); return commentlines[commentlines.length - 1]; } + + function isValidTsIgnorePresent(comment: TSESTree.Comment): boolean { + const line = getLastCommentLine(comment); + return isLineComment(comment) + ? tsIgnoreRegExpSingleLine.test(line) + : tsIgnoreRegExpMultiLine.test(line); + } + return { Program(): void { const comments = sourceCode.getAllComments(); comments.forEach(comment => { - if (tsIgnoreRegExp.test(getLastCommentLine(comment))) { + if (isValidTsIgnorePresent(comment)) { const lineCommentRuleFixer = (fixer: RuleFixer): RuleFix => fixer.replaceText( comment, @@ -63,10 +76,9 @@ export default util.createRule<[], MessageIds>({ context.report({ node: comment, messageId: 'preferExpectErrorComment', - fix: - comment.type === AST_TOKEN_TYPES.Line - ? lineCommentRuleFixer - : blockCommentRuleFixer, + fix: isLineComment(comment) + ? lineCommentRuleFixer + : blockCommentRuleFixer, }); } }); diff --git a/packages/eslint-plugin/tests/rules/prefer-ts-expect-error.test.ts b/packages/eslint-plugin/tests/rules/prefer-ts-expect-error.test.ts index 0aa6e5e91f05..ccc510187d97 100644 --- a/packages/eslint-plugin/tests/rules/prefer-ts-expect-error.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-ts-expect-error.test.ts @@ -59,8 +59,8 @@ if (false) { ], }, { - code: '/////@ts-ignore: Suppress next line', - output: '/////@ts-expect-error: Suppress next line', + code: '///@ts-ignore: Suppress next line', + output: '///@ts-expect-error: Suppress next line', errors: [ { messageId: 'preferExpectErrorComment', @@ -135,37 +135,12 @@ if (false) { }, { code: ` -{ - /* -@ts-ignore in a multiline block -*/ -} - `, - output: ` -{ - /* -@ts-expect-error in a multiline block -*/ -} - `, - errors: [ - { - messageId: 'preferExpectErrorComment', - line: 3, - column: 3, - }, - ], - }, - { - code: ` /* -// @ts-ignore in a block with single line comments -*/ +// @ts-ignore in a block with single line comments */ `, output: ` /* -// @ts-expect-error in a block with single line comments -*/ +// @ts-expect-error in a block with single line comments */ `, errors: [ {