From 7021f2151a25db2a8edf17e06cd6f21e90761ec8 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Mon, 13 Apr 2020 13:11:37 +1200 Subject: [PATCH] feat(eslint-plugin): add rule `prefer-ts-expect-error` (#1705) --- packages/eslint-plugin/README.md | 1 + .../docs/rules/prefer-ts-expect-error.md | 47 ++++++++++ packages/eslint-plugin/src/configs/all.json | 1 + packages/eslint-plugin/src/rules/index.ts | 2 + .../src/rules/prefer-ts-expect-error.ts | 55 ++++++++++++ .../rules/prefer-ts-expect-error.test.ts | 85 +++++++++++++++++++ .../src/ts-estree/ts-estree.ts | 1 + 7 files changed, 192 insertions(+) create mode 100644 packages/eslint-plugin/docs/rules/prefer-ts-expect-error.md create mode 100644 packages/eslint-plugin/src/rules/prefer-ts-expect-error.ts create mode 100644 packages/eslint-plugin/tests/rules/prefer-ts-expect-error.test.ts diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index d943ffdcec9..7ed22981926 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -152,6 +152,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 | :heavy_check_mark: | :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/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 | | | :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 new file mode 100644 index 00000000000..7ec0cac771b --- /dev/null +++ b/packages/eslint-plugin/docs/rules/prefer-ts-expect-error.md @@ -0,0 +1,47 @@ +# 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. +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. +This is dangerous, as if a new error arises on that line it'll be suppressed by the forgotten about `@ts-ignore`, and so be missed. + +To address this, TS3.9 ships with a new single-line comment directive: `// @ts-expect-error`. + +This directive operates in the same manner as `@ts-ignore`, but will error if the line it's meant to be suppressing doesn't actually contain an error, making it a lot safer. + +## Rule Details + +This rule looks for usages of `@ts-ignore`, and flags them to be replaced with `@ts-expect-error`. + +Examples of **incorrect** code for this rule: + +```ts +// @ts-ignore +const str: 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]; +}; +``` + +Examples of **correct** code for this rule: + +```ts +// @ts-expect-error +const str: 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]; +}; +``` + +## 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 + +## Further Reading + +- [Original Implementing PR](https://github.com/microsoft/TypeScript/pull/36014) diff --git a/packages/eslint-plugin/src/configs/all.json b/packages/eslint-plugin/src/configs/all.json index dc1ec0490ca..c736840a3ff 100644 --- a/packages/eslint-plugin/src/configs/all.json +++ b/packages/eslint-plugin/src/configs/all.json @@ -89,6 +89,7 @@ "@typescript-eslint/prefer-reduce-type-parameter": "error", "@typescript-eslint/prefer-regexp-exec": "error", "@typescript-eslint/prefer-string-starts-ends-with": "error", + "@typescript-eslint/prefer-ts-expect-error": "error", "@typescript-eslint/promise-function-async": "error", "quotes": "off", "@typescript-eslint/quotes": "error", diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index e941f5148c3..901d31a8b9f 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -78,6 +78,7 @@ import preferReadonlyParameterTypes from './prefer-readonly-parameter-types'; import preferReduceTypeParameter from './prefer-reduce-type-parameter'; import preferRegexpExec from './prefer-regexp-exec'; import preferStringStartsEndsWith from './prefer-string-starts-ends-with'; +import preferTsExpectError from './prefer-ts-expect-error'; import promiseFunctionAsync from './promise-function-async'; import quotes from './quotes'; import requireArraySortCompare from './require-array-sort-compare'; @@ -176,6 +177,7 @@ export default { 'prefer-reduce-type-parameter': preferReduceTypeParameter, 'prefer-regexp-exec': preferRegexpExec, 'prefer-string-starts-ends-with': preferStringStartsEndsWith, + 'prefer-ts-expect-error': preferTsExpectError, 'promise-function-async': promiseFunctionAsync, quotes: quotes, 'require-array-sort-compare': requireArraySortCompare, diff --git a/packages/eslint-plugin/src/rules/prefer-ts-expect-error.ts b/packages/eslint-plugin/src/rules/prefer-ts-expect-error.ts new file mode 100644 index 00000000000..1b87b132786 --- /dev/null +++ b/packages/eslint-plugin/src/rules/prefer-ts-expect-error.ts @@ -0,0 +1,55 @@ +import { AST_TOKEN_TYPES } from '@typescript-eslint/experimental-utils'; +import * as util from '../util'; + +type MessageIds = 'preferExpectErrorComment'; + +export default util.createRule<[], MessageIds>({ + name: 'prefer-ts-expect-error', + meta: { + type: 'problem', + docs: { + 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.', + }, + schema: [], + }, + defaultOptions: [], + create(context) { + const tsIgnoreRegExp = /^\/*\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)) { + context.report({ + node: comment, + messageId: 'preferExpectErrorComment', + fix: fixer => + fixer.replaceText( + comment, + `//${comment.value.replace( + '@ts-ignore', + '@ts-expect-error', + )}`, + ), + }); + } + }); + }, + }; + }, +}); 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 new file mode 100644 index 00000000000..8e9c04f09ed --- /dev/null +++ b/packages/eslint-plugin/tests/rules/prefer-ts-expect-error.test.ts @@ -0,0 +1,85 @@ +import rule from '../../src/rules/prefer-ts-expect-error'; +import { RuleTester } from '../RuleTester'; + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('prefer-ts-expect-error', rule, { + valid: [ + '// @ts-nocheck', + '// @ts-check', + '// just a comment containing @ts-ignore somewhere', + '/* @ts-ignore */', + '/** @ts-ignore */', + ` +/* +// @ts-ignore in a block +*/ + `, + '// @ts-expect-error', + ` +if (false) { + // @ts-expect-error: Unreachable code error + console.log('hello'); +} + `, + ], + invalid: [ + { + code: '// @ts-ignore', + output: '// @ts-expect-error', + errors: [ + { + messageId: 'preferExpectErrorComment', + line: 1, + column: 1, + }, + ], + }, + { + code: '// @ts-ignore: Suppress next line', + output: '// @ts-expect-error: Suppress next line', + + errors: [ + { + messageId: 'preferExpectErrorComment', + line: 1, + column: 1, + }, + ], + }, + { + code: '/////@ts-ignore: Suppress next line', + output: '/////@ts-expect-error: Suppress next line', + errors: [ + { + messageId: 'preferExpectErrorComment', + line: 1, + column: 1, + }, + ], + }, + { + code: ` +if (false) { + // @ts-ignore: Unreachable code error + console.log('hello'); +} + `, + output: ` +if (false) { + // @ts-expect-error: Unreachable code error + console.log('hello'); +} + `, + errors: [ + { + messageId: 'preferExpectErrorComment', + line: 3, + column: 3, + }, + ], + }, + ], +}); diff --git a/packages/typescript-estree/src/ts-estree/ts-estree.ts b/packages/typescript-estree/src/ts-estree/ts-estree.ts index caf979a3012..5792ebc0205 100644 --- a/packages/typescript-estree/src/ts-estree/ts-estree.ts +++ b/packages/typescript-estree/src/ts-estree/ts-estree.ts @@ -115,6 +115,7 @@ export interface LineComment extends BaseToken { export type Comment = BlockComment | LineComment; export type Token = | BooleanToken + | Comment | IdentifierToken | JSXIdentifierToken | JSXTextToken