Skip to content

Commit

Permalink
feat(eslint-plugin): [ban-ts-comments] suggest ts-expect-error over t…
Browse files Browse the repository at this point in the history 
…s-ignore (#7849)

* feat(eslint-plugin): [ban-ts-comments] suggest ts-expect-error instead of ts-ignore

* Always show tweaked message
  • Loading branch information
@NotWoods
NotWoods committed Oct 30, 2023
1 parent 1e47294 commit 5e73a48
Show file tree
Hide file tree
Showing 2 changed files with 125 additions and 25 deletions.
46 changes: 39 additions & 7 deletions packages/eslint-plugin/src/rules/ban-ts-comment.ts
View file
@@ -1,4 +1,4 @@
import { AST_TOKEN_TYPES } from '@typescript-eslint/utils';
import { AST_TOKEN_TYPES, type TSESLint } from '@typescript-eslint/utils';

import { createRule, getStringLength } from '../util';

Expand All @@ -19,8 +19,10 @@ export const defaultMinimumDescriptionLength = 3;

type MessageIds =
| 'tsDirectiveComment'
| 'tsIgnoreInsteadOfExpectError'
| 'tsDirectiveCommentDescriptionNotMatchPattern'
| 'tsDirectiveCommentRequiresDescription';
| 'tsDirectiveCommentRequiresDescription'
| 'replaceTsIgnoreWithTsExpectError';

export default createRule<[Options], MessageIds>({
name: 'ban-ts-comment',
Expand All @@ -34,11 +36,16 @@ export default createRule<[Options], MessageIds>({
messages: {
tsDirectiveComment:
'Do not use "@ts-{{directive}}" because it alters compilation errors.',
tsIgnoreInsteadOfExpectError:
'Use "@ts-expect-error" instead of "@ts-ignore", as "@ts-ignore" will do nothing if the following line is error-free.',
tsDirectiveCommentRequiresDescription:
'Include a description after the "@ts-{{directive}}" directive to explain why the @ts-{{directive}} is necessary. The description must be {{minimumDescriptionLength}} characters or longer.',
tsDirectiveCommentDescriptionNotMatchPattern:
'The description for the "@ts-{{directive}}" directive must match the {{format}} format.',
replaceTsIgnoreWithTsExpectError:
'Replace "@ts-ignore" with "@ts-expect-error".',
},
hasSuggestions: true,
schema: [
{
$defs: {
Expand Down Expand Up @@ -130,11 +137,36 @@ export default createRule<[Options], MessageIds>({

const option = options[fullDirective];
if (option === true) {
context.report({
data: { directive },
node: comment,
messageId: 'tsDirectiveComment',
});
if (directive === 'ignore') {
// Special case to suggest @ts-expect-error instead of @ts-ignore
context.report({
node: comment,
messageId: 'tsIgnoreInsteadOfExpectError',
suggest: [
{
messageId: 'replaceTsIgnoreWithTsExpectError',
fix(fixer): TSESLint.RuleFix {
const commentText = comment.value.replace(
/@ts-ignore/,
'@ts-expect-error',
);
return fixer.replaceText(
comment,
comment.type === AST_TOKEN_TYPES.Line
? `//${commentText}`
: `/*${commentText}*/`,
);
},
},
],
});
} else {
context.report({
data: { directive },
node: comment,
messageId: 'tsDirectiveComment',
});
}
}

if (
Expand Down
104 changes: 86 additions & 18 deletions packages/eslint-plugin/tests/rules/ban-ts-comment.test.ts
View file
Expand Up @@ -303,24 +303,53 @@ ruleTester.run('ts-ignore', rule, {
invalid: [
{
code: '// @ts-ignore',
options: [{ 'ts-ignore': true }],
options: [{ 'ts-ignore': true, 'ts-expect-error': true }],
errors: [
{
data: { directive: 'ignore' },
messageId: 'tsDirectiveComment',
messageId: 'tsIgnoreInsteadOfExpectError',
line: 1,
column: 1,
suggestions: [
{
messageId: 'replaceTsIgnoreWithTsExpectError',
output: '// @ts-expect-error',
},
],
},
],
},
{
code: '// @ts-ignore',
options: [
{ 'ts-ignore': true, 'ts-expect-error': 'allow-with-description' },
],
errors: [
{
data: { directive: 'ignore' },
messageId: 'tsDirectiveComment',
messageId: 'tsIgnoreInsteadOfExpectError',
line: 1,
column: 1,
suggestions: [
{
messageId: 'replaceTsIgnoreWithTsExpectError',
output: '// @ts-expect-error',
},
],
},
],
},
{
code: '// @ts-ignore',
errors: [
{
messageId: 'tsIgnoreInsteadOfExpectError',
line: 1,
column: 1,
suggestions: [
{
messageId: 'replaceTsIgnoreWithTsExpectError',
output: '// @ts-expect-error',
},
],
},
],
},
Expand All @@ -329,10 +358,15 @@ ruleTester.run('ts-ignore', rule, {
options: [{ 'ts-ignore': true }],
errors: [
{
data: { directive: 'ignore' },
messageId: 'tsDirectiveComment',
messageId: 'tsIgnoreInsteadOfExpectError',
line: 1,
column: 1,
suggestions: [
{
messageId: 'replaceTsIgnoreWithTsExpectError',
output: '/* @ts-expect-error */',
},
],
},
],
},
Expand All @@ -345,44 +379,68 @@ ruleTester.run('ts-ignore', rule, {
options: [{ 'ts-ignore': true }],
errors: [
{
data: { directive: 'ignore' },
messageId: 'tsDirectiveComment',
messageId: 'tsIgnoreInsteadOfExpectError',
line: 2,
column: 1,
suggestions: [
{
messageId: 'replaceTsIgnoreWithTsExpectError',
output: `
/*
@ts-expect-error
*/
`,
},
],
},
],
},
{
code: '/** @ts-ignore */',
options: [{ 'ts-ignore': true }],
options: [{ 'ts-ignore': true, 'ts-expect-error': false }],
errors: [
{
data: { directive: 'ignore' },
messageId: 'tsDirectiveComment',
messageId: 'tsIgnoreInsteadOfExpectError',
line: 1,
column: 1,
suggestions: [
{
messageId: 'replaceTsIgnoreWithTsExpectError',
output: '/** @ts-expect-error */',
},
],
},
],
},
{
code: '// @ts-ignore: Suppress next line',
errors: [
{
data: { directive: 'ignore' },
messageId: 'tsDirectiveComment',
messageId: 'tsIgnoreInsteadOfExpectError',
line: 1,
column: 1,
suggestions: [
{
messageId: 'replaceTsIgnoreWithTsExpectError',
output: '// @ts-expect-error: Suppress next line',
},
],
},
],
},
{
code: '/////@ts-ignore: Suppress next line',
errors: [
{
data: { directive: 'ignore' },
messageId: 'tsDirectiveComment',
messageId: 'tsIgnoreInsteadOfExpectError',
line: 1,
column: 1,
suggestions: [
{
messageId: 'replaceTsIgnoreWithTsExpectError',
output: '/////@ts-expect-error: Suppress next line',
},
],
},
],
},
Expand All @@ -395,10 +453,20 @@ if (false) {
`,
errors: [
{
data: { directive: 'ignore' },
messageId: 'tsDirectiveComment',
messageId: 'tsIgnoreInsteadOfExpectError',
line: 3,
column: 3,
suggestions: [
{
messageId: 'replaceTsIgnoreWithTsExpectError',
output: `
if (false) {
// @ts-expect-error: Unreachable code error
console.log('hello');
}
`,
},
],
},
],
},
Expand Down

0 comments on commit 5e73a48

Please sign in to comment.