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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from Sep 21, 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
2 changes: 1 addition & 1 deletion packages/eslint-plugin/README.md
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
@@ -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
@@ -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(
yasarsid marked this conversation as resolved.
Show resolved Hide resolved
'@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
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,
},
],
},
],
});