diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 63b5b6e7f32..8fac3dcde07 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/docs/rules/prefer-ts-expect-error.md b/packages/eslint-plugin/docs/rules/prefer-ts-expect-error.md index 7ec0cac771b..60fdf79c806 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,15 @@ Examples of **incorrect** code for this rule: // @ts-ignore const str: string = 1; +/** + * Explaining comment + * + * @ts-ignore */ +const multiLine: 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 +41,15 @@ Examples of **correct** code for this rule: // @ts-expect-error const str: string = 1; +/** + * Explaining comment + * + * @ts-expect-error */ +const multiLine: 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 +58,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 1b87b132786..d696fc04573 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,12 @@ -import { AST_TOKEN_TYPES } from '@typescript-eslint/experimental-utils'; import * as util from '../util'; +import { + AST_TOKEN_TYPES, + TSESTree, +} from '@typescript-eslint/experimental-utils'; +import { + RuleFixer, + RuleFix, +} from '@typescript-eslint/experimental-utils/dist/ts-eslint'; type MessageIds = 'preferExpectErrorComment'; @@ -8,44 +15,70 @@ 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: [], }, defaultOptions: [], create(context) { - const tsIgnoreRegExp = /^\/*\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 (isLineComment(comment)) { + return comment.value; + } + + // For multiline comments - we look at only the last line. + 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 (comment.type !== AST_TOKEN_TYPES.Line) { - return; - } + if (isValidTsIgnorePresent(comment)) { + 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', + )}*/`, + ); - if (tsIgnoreRegExp.test(comment.value)) { context.report({ node: comment, messageId: 'preferExpectErrorComment', - fix: fixer => - fixer.replaceText( - comment, - `//${comment.value.replace( - '@ts-ignore', - '@ts-expect-error', - )}`, - ), + 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 8e9c04f09ed..ccc510187d9 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', ` @@ -24,6 +24,15 @@ if (false) { console.log('hello'); } `, + ` +/** + * Explaining comment + * + * @ts-expect-error + * + * Not last line + * */ + `, ], invalid: [ { @@ -50,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', @@ -81,5 +90,65 @@ if (false) { }, ], }, + { + code: '/* @ts-ignore */', + output: '/* @ts-expect-error */', + errors: [ + { + messageId: 'preferExpectErrorComment', + line: 1, + column: 1, + }, + ], + }, + { + 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 */', + errors: [ + { + messageId: 'preferExpectErrorComment', + line: 1, + column: 1, + }, + ], + }, + { + code: ` +/* +// @ts-ignore in a block with single line comments */ + `, + output: ` +/* +// @ts-expect-error in a block with single line comments */ + `, + errors: [ + { + messageId: 'preferExpectErrorComment', + line: 2, + column: 1, + }, + ], + }, ], });