Skip to content

Commit

Permalink
fix(eslint-plugin): [prefer-ts-expect-error] support block comments (#…
Browse files Browse the repository at this point in the history 
…2541)
  • Loading branch information
@yasarsid
yasarsid committed Sep 21, 2020
1 parent 53a3cbc commit c6f72fb
Show file tree
Hide file tree
Showing 4 changed files with 149 additions and 29 deletions.
2 changes: 1 addition & 1 deletion packages/eslint-plugin/README.md
View file
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
@@ -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
@@ -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
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,
},
],
},
],
});

0 comments on commit c6f72fb

Please sign in to comment.