From e06393adaff78eaf5e5c826b306e9fa2eedfaea6 Mon Sep 17 00:00:00 2001 From: Tiger Oakes Date: Fri, 27 Oct 2023 15:42:08 -0700 Subject: [PATCH 1/2] feat(eslint-plugin): [ban-ts-comments] suggest ts-expect-error instead of ts-ignore --- .../eslint-plugin/src/rules/ban-ts-comment.ts | 47 ++++++-- .../tests/rules/ban-ts-comment.test.ts | 112 ++++++++++++++++-- 2 files changed, 142 insertions(+), 17 deletions(-) diff --git a/packages/eslint-plugin/src/rules/ban-ts-comment.ts b/packages/eslint-plugin/src/rules/ban-ts-comment.ts index 4f64e85f5b9c..274e207b40d6 100644 --- a/packages/eslint-plugin/src/rules/ban-ts-comment.ts +++ b/packages/eslint-plugin/src/rules/ban-ts-comment.ts @@ -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'; @@ -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', @@ -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: { @@ -130,11 +137,37 @@ export default createRule<[Options], MessageIds>({ const option = options[fullDirective]; if (option === true) { - context.report({ - data: { directive }, - node: comment, - messageId: 'tsDirectiveComment', - }); + if (directive === 'ignore' && options['ts-expect-error'] !== true) { + // Special case to suggest @ts-expect-error instead of @ts-ignore, + // as long as @ts-expect-error is banned outright. + 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 ( diff --git a/packages/eslint-plugin/tests/rules/ban-ts-comment.test.ts b/packages/eslint-plugin/tests/rules/ban-ts-comment.test.ts index 271b2d27a03c..73e620db62aa 100644 --- a/packages/eslint-plugin/tests/rules/ban-ts-comment.test.ts +++ b/packages/eslint-plugin/tests/rules/ban-ts-comment.test.ts @@ -303,7 +303,7 @@ 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' }, @@ -315,10 +315,28 @@ ruleTester.run('ts-ignore', rule, { }, { 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, }, @@ -326,7 +344,7 @@ ruleTester.run('ts-ignore', rule, { }, { code: '/* @ts-ignore */', - options: [{ 'ts-ignore': true }], + options: [{ 'ts-ignore': true, 'ts-expect-error': true }], errors: [ { data: { directive: 'ignore' }, @@ -336,13 +354,30 @@ ruleTester.run('ts-ignore', rule, { }, ], }, + { + code: '/* @ts-ignore */', + options: [{ 'ts-ignore': true, 'ts-expect-error': false }], + errors: [ + { + messageId: 'tsIgnoreInsteadOfExpectError', + line: 1, + column: 1, + suggestions: [ + { + messageId: 'replaceTsIgnoreWithTsExpectError', + output: '/* @ts-expect-error */', + }, + ], + }, + ], + }, { code: ` /* @ts-ignore */ `, - options: [{ 'ts-ignore': true }], + options: [{ 'ts-ignore': true, 'ts-expect-error': true }], errors: [ { data: { directive: 'ignore' }, @@ -353,8 +388,33 @@ ruleTester.run('ts-ignore', rule, { ], }, { - code: '/** @ts-ignore */', + code: ` +/* + @ts-ignore +*/ + `, options: [{ 'ts-ignore': true }], + errors: [ + { + messageId: 'tsIgnoreInsteadOfExpectError', + line: 2, + column: 1, + suggestions: [ + { + messageId: 'replaceTsIgnoreWithTsExpectError', + output: ` +/* + @ts-expect-error +*/ + `, + }, + ], + }, + ], + }, + { + code: '/** @ts-ignore */', + options: [{ 'ts-ignore': true, 'ts-expect-error': true }], errors: [ { data: { directive: 'ignore' }, @@ -366,6 +426,23 @@ ruleTester.run('ts-ignore', rule, { }, { code: '// @ts-ignore: Suppress next line', + errors: [ + { + messageId: 'tsIgnoreInsteadOfExpectError', + line: 1, + column: 1, + suggestions: [ + { + messageId: 'replaceTsIgnoreWithTsExpectError', + output: '// @ts-expect-error: Suppress next line', + }, + ], + }, + ], + }, + { + code: '// @ts-ignore: Suppress next line', + options: [{ 'ts-ignore': true, 'ts-expect-error': true }], errors: [ { data: { directive: 'ignore' }, @@ -379,10 +456,15 @@ ruleTester.run('ts-ignore', rule, { 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', + }, + ], }, ], }, @@ -395,10 +477,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'); +} + `, + }, + ], }, ], }, From 0a64ad1f87199502562f1e2d872cd308ab770ee7 Mon Sep 17 00:00:00 2001 From: Tiger Oakes Date: Mon, 30 Oct 2023 13:05:22 -0700 Subject: [PATCH 2/2] Always show tweaked message --- .../eslint-plugin/src/rules/ban-ts-comment.ts | 5 +- .../tests/rules/ban-ts-comment.test.ts | 68 ++++++------------- 2 files changed, 24 insertions(+), 49 deletions(-) diff --git a/packages/eslint-plugin/src/rules/ban-ts-comment.ts b/packages/eslint-plugin/src/rules/ban-ts-comment.ts index 274e207b40d6..4413af5bf1a3 100644 --- a/packages/eslint-plugin/src/rules/ban-ts-comment.ts +++ b/packages/eslint-plugin/src/rules/ban-ts-comment.ts @@ -137,9 +137,8 @@ export default createRule<[Options], MessageIds>({ const option = options[fullDirective]; if (option === true) { - if (directive === 'ignore' && options['ts-expect-error'] !== true) { - // Special case to suggest @ts-expect-error instead of @ts-ignore, - // as long as @ts-expect-error is banned outright. + if (directive === 'ignore') { + // Special case to suggest @ts-expect-error instead of @ts-ignore context.report({ node: comment, messageId: 'tsIgnoreInsteadOfExpectError', diff --git a/packages/eslint-plugin/tests/rules/ban-ts-comment.test.ts b/packages/eslint-plugin/tests/rules/ban-ts-comment.test.ts index 73e620db62aa..640f618200f8 100644 --- a/packages/eslint-plugin/tests/rules/ban-ts-comment.test.ts +++ b/packages/eslint-plugin/tests/rules/ban-ts-comment.test.ts @@ -306,10 +306,15 @@ ruleTester.run('ts-ignore', rule, { 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', + }, + ], }, ], }, @@ -339,24 +344,18 @@ ruleTester.run('ts-ignore', rule, { messageId: 'tsIgnoreInsteadOfExpectError', line: 1, column: 1, + suggestions: [ + { + messageId: 'replaceTsIgnoreWithTsExpectError', + output: '// @ts-expect-error', + }, + ], }, ], }, { code: '/* @ts-ignore */', - options: [{ 'ts-ignore': true, 'ts-expect-error': true }], - errors: [ - { - data: { directive: 'ignore' }, - messageId: 'tsDirectiveComment', - line: 1, - column: 1, - }, - ], - }, - { - code: '/* @ts-ignore */', - options: [{ 'ts-ignore': true, 'ts-expect-error': false }], + options: [{ 'ts-ignore': true }], errors: [ { messageId: 'tsIgnoreInsteadOfExpectError', @@ -375,22 +374,6 @@ ruleTester.run('ts-ignore', rule, { code: ` /* @ts-ignore -*/ - `, - options: [{ 'ts-ignore': true, 'ts-expect-error': true }], - errors: [ - { - data: { directive: 'ignore' }, - messageId: 'tsDirectiveComment', - line: 2, - column: 1, - }, - ], - }, - { - code: ` -/* - @ts-ignore */ `, options: [{ 'ts-ignore': true }], @@ -414,13 +397,18 @@ ruleTester.run('ts-ignore', rule, { }, { code: '/** @ts-ignore */', - options: [{ 'ts-ignore': true, 'ts-expect-error': 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 */', + }, + ], }, ], }, @@ -440,18 +428,6 @@ ruleTester.run('ts-ignore', rule, { }, ], }, - { - code: '// @ts-ignore: Suppress next line', - options: [{ 'ts-ignore': true, 'ts-expect-error': true }], - errors: [ - { - data: { directive: 'ignore' }, - messageId: 'tsDirectiveComment', - line: 1, - column: 1, - }, - ], - }, { code: '/////@ts-ignore: Suppress next line', errors: [