From 0ed70bf5a958cdcbafa3cbf5afea6be9b9aed5ac Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sun, 8 Mar 2020 12:51:02 +1300 Subject: [PATCH 1/3] feat(eslint-plugin): support `ts-expect-error` comment directive --- .../docs/rules/ban-ts-comment.md | 8 +- .../eslint-plugin/src/rules/ban-ts-comment.ts | 8 +- .../tests/rules/ban-ts-comment.test.ts | 74 ++++++++++++++++++- 3 files changed, 87 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/ban-ts-comment.md b/packages/eslint-plugin/docs/rules/ban-ts-comment.md index 58f99dc048a..a3113a65451 100644 --- a/packages/eslint-plugin/docs/rules/ban-ts-comment.md +++ b/packages/eslint-plugin/docs/rules/ban-ts-comment.md @@ -6,6 +6,7 @@ Using these to suppress TypeScript Compiler Errors reduces the effectiveness of The directive comments supported by TypeScript are: ``` +// @ts-expect-error // @ts-ignore // @ts-nocheck // @ts-check @@ -14,18 +15,23 @@ The directive comments supported by TypeScript are: ## Rule Details This rule lets you set which directive comments you want to allow in your codebase. -By default, only `@ts-check` is allowed, as it enables rather then suppresses errors. +By default two directives are allowed: + +- `@ts-expect-error`, as this errors unless there is an actual error to suppress, making it a lot safer than `@ts-ignore` +- `@ts-check`, since this enables TS checking in a file The configuration looks like this: ``` interface Options { + 'ts-expect-error'?: boolean; 'ts-ignore'?: boolean; 'ts-nocheck'?: boolean; 'ts-check'?: boolean; } const defaultOptions: Options = { + 'ts-expect-error': true, 'ts-ignore': true, 'ts-nocheck': true, 'ts-check': false diff --git a/packages/eslint-plugin/src/rules/ban-ts-comment.ts b/packages/eslint-plugin/src/rules/ban-ts-comment.ts index 53505cfea22..6b4e4343a3d 100644 --- a/packages/eslint-plugin/src/rules/ban-ts-comment.ts +++ b/packages/eslint-plugin/src/rules/ban-ts-comment.ts @@ -2,6 +2,7 @@ import { AST_TOKEN_TYPES } from '@typescript-eslint/experimental-utils'; import * as util from '../util'; interface Options { + 'ts-expect-error'?: boolean; 'ts-ignore'?: boolean; 'ts-nocheck'?: boolean; 'ts-check'?: boolean; @@ -9,6 +10,7 @@ interface Options { const defaultOptions: [Options] = [ { + 'ts-expect-error': false, 'ts-ignore': true, 'ts-nocheck': true, 'ts-check': false, @@ -34,6 +36,10 @@ export default util.createRule<[Options], MessageIds>({ { type: 'object', properties: { + 'ts-expect-error': { + type: 'boolean', + default: false, + }, 'ts-ignore': { type: 'boolean', default: true, @@ -53,7 +59,7 @@ export default util.createRule<[Options], MessageIds>({ }, defaultOptions, create(context, [options]) { - const tsCommentRegExp = /^\/*\s*@ts-(ignore|check|nocheck)/; + const tsCommentRegExp = /^\/*\s*@ts-(expect-error|ignore|check|nocheck)/; const sourceCode = context.getSourceCode(); return { 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 fcbb788a308..89ff1db4a61 100644 --- a/packages/eslint-plugin/tests/rules/ban-ts-comment.test.ts +++ b/packages/eslint-plugin/tests/rules/ban-ts-comment.test.ts @@ -5,7 +5,79 @@ const ruleTester = new RuleTester({ parser: '@typescript-eslint/parser', }); -ruleTester.run('ban-ts-comment', rule, { +ruleTester.run('ts-expect-error', rule, { + valid: [ + '// just a comment containing @ts-expect-error somewhere', + '/* @ts-expect-error */', + '/** @ts-expect-error */', + ` +/* +// @ts-expect-error in a block +*/ + `, + { + code: '// @ts-expect-error', + options: [{ 'ts-expect-error': false }], + }, + ], + invalid: [ + { + code: '// @ts-expect-error', + options: [{ 'ts-expect-error': true }], + errors: [ + { + data: { directive: 'expect-error' }, + messageId: 'tsDirectiveComment', + line: 1, + column: 1, + }, + ], + }, + { + code: '// @ts-expect-error: Suppress next line', + options: [{ 'ts-expect-error': true }], + errors: [ + { + data: { directive: 'expect-error' }, + messageId: 'tsDirectiveComment', + line: 1, + column: 1, + }, + ], + }, + { + code: '/////@ts-expect-error: Suppress next line', + options: [{ 'ts-expect-error': true }], + errors: [ + { + data: { directive: 'expect-error' }, + messageId: 'tsDirectiveComment', + line: 1, + column: 1, + }, + ], + }, + { + code: ` +if (false) { + // @ts-expect-error: Unreachable code error + console.log('hello'); +} + `, + options: [{ 'ts-expect-error': true }], + errors: [ + { + data: { directive: 'expect-error' }, + messageId: 'tsDirectiveComment', + line: 3, + column: 3, + }, + ], + }, + ], +}); + +ruleTester.run('ts-ignore', rule, { valid: [ '// just a comment containing @ts-ignore somewhere', '/* @ts-ignore */', From 9256669359e42ac71a3b43f8c84eb6338226d2bb Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sun, 12 Apr 2020 15:34:34 +1200 Subject: [PATCH 2/3] chore(eslint-plugin): ban `@ts-expect-error` by default --- packages/eslint-plugin/docs/rules/ban-ts-comment.md | 7 ++----- packages/eslint-plugin/src/rules/ban-ts-comment.ts | 4 ++-- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/ban-ts-comment.md b/packages/eslint-plugin/docs/rules/ban-ts-comment.md index a3113a65451..bc9f1b8c892 100644 --- a/packages/eslint-plugin/docs/rules/ban-ts-comment.md +++ b/packages/eslint-plugin/docs/rules/ban-ts-comment.md @@ -15,10 +15,7 @@ The directive comments supported by TypeScript are: ## Rule Details This rule lets you set which directive comments you want to allow in your codebase. -By default two directives are allowed: - -- `@ts-expect-error`, as this errors unless there is an actual error to suppress, making it a lot safer than `@ts-ignore` -- `@ts-check`, since this enables TS checking in a file +By default, only `@ts-check` is allowed, as it enables rather than suppresses errors. The configuration looks like this: @@ -31,7 +28,7 @@ interface Options { } const defaultOptions: Options = { - 'ts-expect-error': true, + 'ts-expect-error': false, 'ts-ignore': true, 'ts-nocheck': true, 'ts-check': false diff --git a/packages/eslint-plugin/src/rules/ban-ts-comment.ts b/packages/eslint-plugin/src/rules/ban-ts-comment.ts index 6b4e4343a3d..a422349eefa 100644 --- a/packages/eslint-plugin/src/rules/ban-ts-comment.ts +++ b/packages/eslint-plugin/src/rules/ban-ts-comment.ts @@ -10,7 +10,7 @@ interface Options { const defaultOptions: [Options] = [ { - 'ts-expect-error': false, + 'ts-expect-error': true, 'ts-ignore': true, 'ts-nocheck': true, 'ts-check': false, @@ -38,7 +38,7 @@ export default util.createRule<[Options], MessageIds>({ properties: { 'ts-expect-error': { type: 'boolean', - default: false, + default: true, }, 'ts-ignore': { type: 'boolean', From bb754e16038243daacf5b9e2041659fe89198ce8 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Sun, 12 Apr 2020 17:52:42 -0700 Subject: [PATCH 3/3] Update ban-ts-comment.md --- packages/eslint-plugin/docs/rules/ban-ts-comment.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/docs/rules/ban-ts-comment.md b/packages/eslint-plugin/docs/rules/ban-ts-comment.md index bc9f1b8c892..b1d93409c28 100644 --- a/packages/eslint-plugin/docs/rules/ban-ts-comment.md +++ b/packages/eslint-plugin/docs/rules/ban-ts-comment.md @@ -28,7 +28,7 @@ interface Options { } const defaultOptions: Options = { - 'ts-expect-error': false, + 'ts-expect-error': true, 'ts-ignore': true, 'ts-nocheck': true, 'ts-check': false