Skip to content

fix(eslint-plugin): [prefer-ts-expect-error] support block comments #2541

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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Sep 21, 2020
Merged

fix(eslint-plugin): [prefer-ts-expect-error] support block comments #2541

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
28afda4
fix(eslint-plugin): add support for @ts-ignore in block comments
Sep 11, 2020
b0a88be
fix(eslint-plugin): add support for @ts-ignore in block comments
Sep 11, 2020
7047b57
fix(eslint-plugin): minor refactor
Sep 11, 2020
fec7d36
fix(eslint-plugin): improve multipline support and correct example code
Sep 13, 2020
63159fe
fix(eslint-plugin): adopt ts regex
Sep 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/eslint-plugin/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: |
Expand Down
24 changes: 21 additions & 3 deletions packages/eslint-plugin/docs/rules/prefer-ts-expect-error.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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];
Expand All @@ -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];
Expand All @@ -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

Expand Down
69 changes: 51 additions & 18 deletions packages/eslint-plugin/src/rules/prefer-ts-expect-error.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -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,
});
}
});
Expand Down
83 changes: 76 additions & 7 deletions packages/eslint-plugin/tests/rules/prefer-ts-expect-error.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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',
`
Expand All @@ -24,6 +24,15 @@ if (false) {
console.log('hello');
}
`,
`
/**
* Explaining comment
*
* @ts-expect-error
*
* Not last line
* */
`,
],
invalid: [
{
Expand All @@ -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',
Expand Down Expand Up @@ -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,
},
],
},
],
});