Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(eslint-plugin): add rule prefer-ts-expect-error #1705

Merged
merged 3 commits into from Apr 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/eslint-plugin/README.md
Expand Up @@ -151,6 +151,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int
| [`@typescript-eslint/prefer-readonly-parameter-types`](./docs/rules/prefer-readonly-parameter-types.md) | Requires that function parameters are typed as readonly to prevent accidental mutation of inputs | | | :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: |
Expand Down
47 changes: 47 additions & 0 deletions 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)
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/configs/all.json
Expand Up @@ -88,6 +88,7 @@
"@typescript-eslint/prefer-readonly-parameter-types": "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",
Expand Down
2 changes: 2 additions & 0 deletions packages/eslint-plugin/src/rules/index.ts
Expand Up @@ -77,6 +77,7 @@ import preferReadonly from './prefer-readonly';
import preferReadonlyParameterTypes from './prefer-readonly-parameter-types';
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';
Expand Down Expand Up @@ -174,6 +175,7 @@ export default {
'prefer-readonly': preferReadonly,
'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,
Expand Down
55 changes: 55 additions & 0 deletions 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',
)}`,
),
});
}
});
},
};
},
});
85 changes: 85 additions & 0 deletions 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,
},
],
},
],
});
1 change: 1 addition & 0 deletions packages/typescript-estree/src/ts-estree/ts-estree.ts
Expand Up @@ -115,6 +115,7 @@ export interface LineComment extends BaseToken {
export type Comment = BlockComment | LineComment;
export type Token =
| BooleanToken
| Comment
bradzacher marked this conversation as resolved.
Show resolved Hide resolved
| IdentifierToken
| JSXIdentifierToken
| JSXTextToken
Expand Down