From 2a83d138a966cd5ce787d1eecf595b59b78232d4 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Tue, 21 Jan 2020 13:56:00 +1300 Subject: [PATCH] feat(eslint-plugin): create `ban-ts-comment` rule (#1361) --- .cspell.json | 1 + packages/eslint-plugin/README.md | 2 +- .../docs/rules/ban-ts-comment.md | 65 +++++ .../eslint-plugin/docs/rules/ban-ts-ignore.md | 2 + packages/eslint-plugin/src/configs/all.json | 2 +- .../eslint-plugin/src/rules/ban-ts-comment.ts | 82 ++++++ .../eslint-plugin/src/rules/ban-ts-ignore.ts | 2 + packages/eslint-plugin/src/rules/index.ts | 2 + .../tests/rules/ban-ts-comment.test.ts | 233 ++++++++++++++++++ 9 files changed, 389 insertions(+), 2 deletions(-) create mode 100644 packages/eslint-plugin/docs/rules/ban-ts-comment.md create mode 100644 packages/eslint-plugin/src/rules/ban-ts-comment.ts create mode 100644 packages/eslint-plugin/tests/rules/ban-ts-comment.test.ts diff --git a/.cspell.json b/.cspell.json index 0c045aaa4bc..d2061775d83 100644 --- a/.cspell.json +++ b/.cspell.json @@ -61,6 +61,7 @@ "estree", "linebreaks", "necroing", + "nocheck", "nullish", "parameterised", "performant", diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index f2418485d52..77709f77832 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -98,7 +98,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int | [`@typescript-eslint/adjacent-overload-signatures`](./docs/rules/adjacent-overload-signatures.md) | Require that member overloads be consecutive | :heavy_check_mark: | | | | [`@typescript-eslint/array-type`](./docs/rules/array-type.md) | Requires using either `T[]` or `Array` for arrays | | :wrench: | | | [`@typescript-eslint/await-thenable`](./docs/rules/await-thenable.md) | Disallows awaiting a value that is not a Thenable | :heavy_check_mark: | | :thought_balloon: | -| [`@typescript-eslint/ban-ts-ignore`](./docs/rules/ban-ts-ignore.md) | Bans “// @ts-ignore” comments from being used | :heavy_check_mark: | | | +| [`@typescript-eslint/ban-ts-comment`](./docs/rules/ban-ts-comment.md) | Bans `// @ts-` comments from being used | | | | | [`@typescript-eslint/ban-types`](./docs/rules/ban-types.md) | Bans specific types from being used | :heavy_check_mark: | :wrench: | | | [`@typescript-eslint/consistent-type-assertions`](./docs/rules/consistent-type-assertions.md) | Enforces consistent usage of type assertions | :heavy_check_mark: | | | | [`@typescript-eslint/consistent-type-definitions`](./docs/rules/consistent-type-definitions.md) | Consistent with type definition either `interface` or `type` | | :wrench: | | diff --git a/packages/eslint-plugin/docs/rules/ban-ts-comment.md b/packages/eslint-plugin/docs/rules/ban-ts-comment.md new file mode 100644 index 00000000000..58f99dc048a --- /dev/null +++ b/packages/eslint-plugin/docs/rules/ban-ts-comment.md @@ -0,0 +1,65 @@ +# Bans `// @ts-` comments from being used (`ban-ts-comment`) + +TypeScript provides several directive comments that can be used to alter how it processes files. +Using these to suppress TypeScript Compiler Errors reduces the effectiveness of TypeScript overall. + +The directive comments supported by TypeScript are: + +``` +// @ts-ignore +// @ts-nocheck +// @ts-check +``` + +## 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. + +The configuration looks like this: + +``` +interface Options { + 'ts-ignore'?: boolean; + 'ts-nocheck'?: boolean; + 'ts-check'?: boolean; +} + +const defaultOptions: Options = { + 'ts-ignore': true, + 'ts-nocheck': true, + 'ts-check': false +} +``` + +A value of `true` for a particular directive means that this rule will report if it finds any usage of said directive. + +For example, with the defaults above the following patterns are considered warnings: + +```ts +if (false) { + // @ts-ignore: Unreachable code error + console.log('hello'); +} +``` + +The following patterns are not warnings: + +```ts +if (false) { + // Compiler warns about unreachable code error + console.log('hello'); +} +``` + +## When Not To Use It + +If you want to use all of the TypeScript directives. + +## Further Reading + +- TypeScript [Type Checking JavaScript Files](https://www.typescriptlang.org/docs/handbook/type-checking-javascript-files.html) + +## Compatibility + +- TSLint: [ban-ts-ignore](https://palantir.github.io/tslint/rules/ban-ts-ignore/) diff --git a/packages/eslint-plugin/docs/rules/ban-ts-ignore.md b/packages/eslint-plugin/docs/rules/ban-ts-ignore.md index d8d987a95ed..8f69c864669 100644 --- a/packages/eslint-plugin/docs/rules/ban-ts-ignore.md +++ b/packages/eslint-plugin/docs/rules/ban-ts-ignore.md @@ -1,5 +1,7 @@ # Bans “// @ts-ignore” comments from being used (`ban-ts-ignore`) +This rule has been deprecated in favor of [`ban-ts-comment`](./ban-ts-comment.md) + Suppressing TypeScript Compiler Errors can be hard to discover. ## Rule Details diff --git a/packages/eslint-plugin/src/configs/all.json b/packages/eslint-plugin/src/configs/all.json index 91380db1932..1ee81fe91ca 100644 --- a/packages/eslint-plugin/src/configs/all.json +++ b/packages/eslint-plugin/src/configs/all.json @@ -4,7 +4,7 @@ "@typescript-eslint/adjacent-overload-signatures": "error", "@typescript-eslint/array-type": "error", "@typescript-eslint/await-thenable": "error", - "@typescript-eslint/ban-ts-ignore": "error", + "@typescript-eslint/ban-ts-comment": "error", "@typescript-eslint/ban-types": "error", "brace-style": "off", "@typescript-eslint/brace-style": "error", diff --git a/packages/eslint-plugin/src/rules/ban-ts-comment.ts b/packages/eslint-plugin/src/rules/ban-ts-comment.ts new file mode 100644 index 00000000000..6370ef029f5 --- /dev/null +++ b/packages/eslint-plugin/src/rules/ban-ts-comment.ts @@ -0,0 +1,82 @@ +import * as util from '../util'; + +interface Options { + 'ts-ignore'?: boolean; + 'ts-nocheck'?: boolean; + 'ts-check'?: boolean; +} + +const defaultOptions: [Options] = [ + { + 'ts-ignore': true, + 'ts-nocheck': true, + 'ts-check': false, + }, +]; + +type MessageIds = 'tsDirectiveComment'; + +export default util.createRule<[Options], MessageIds>({ + name: 'ban-ts-comment', + meta: { + type: 'problem', + docs: { + description: 'Bans `// @ts-` comments from being used', + category: 'Best Practices', + recommended: false, + }, + messages: { + tsDirectiveComment: + 'Do not use "// @ts-{directive}" because it alters compilation errors.', + }, + schema: [ + { + type: 'object', + properties: { + 'ts-ignore': { + type: 'boolean', + default: true, + }, + 'ts-nocheck': { + type: 'boolean', + default: true, + }, + 'ts-check': { + type: 'boolean', + default: false, + }, + }, + additionalProperties: false, + }, + ], + }, + defaultOptions, + create(context, [options]) { + const tsCommentRegExp = /^\/*\s*@ts-(ignore|check|nocheck)/; + const sourceCode = context.getSourceCode(); + + return { + Program(): void { + const comments = sourceCode.getAllComments(); + + comments.forEach(comment => { + if (comment.type !== 'Line') { + return; + } + + const [, directive] = tsCommentRegExp.exec(comment.value) ?? []; + + const fullDirective = `ts-${directive}` as keyof Options; + + if (options[fullDirective]) { + context.report({ + data: { directive }, + node: comment, + messageId: 'tsDirectiveComment', + }); + } + }); + }, + }; + }, +}); diff --git a/packages/eslint-plugin/src/rules/ban-ts-ignore.ts b/packages/eslint-plugin/src/rules/ban-ts-ignore.ts index 7d7a32e6cfc..a00b4a4f67d 100644 --- a/packages/eslint-plugin/src/rules/ban-ts-ignore.ts +++ b/packages/eslint-plugin/src/rules/ban-ts-ignore.ts @@ -14,6 +14,8 @@ export default util.createRule({ tsIgnoreComment: 'Do not use "// @ts-ignore" comments because they suppress compilation errors.', }, + deprecated: true, + replacedBy: ['@typescript-eslint/ban-ts-comment'], }, defaultOptions: [], create(context) { diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index ac12f27e8ba..7f9f47e538a 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -2,6 +2,7 @@ import adjacentOverloadSignatures from './adjacent-overload-signatures'; import arrayType from './array-type'; import awaitThenable from './await-thenable'; import banTsIgnore from './ban-ts-ignore'; +import banTsComment from './ban-ts-comment'; import banTypes from './ban-types'; import braceStyle from './brace-style'; import camelcase from './camelcase'; @@ -85,6 +86,7 @@ export default { 'array-type': arrayType, 'await-thenable': awaitThenable, 'ban-ts-ignore': banTsIgnore, + 'ban-ts-comment': banTsComment, 'ban-types': banTypes, 'brace-style': braceStyle, camelcase: camelcase, diff --git a/packages/eslint-plugin/tests/rules/ban-ts-comment.test.ts b/packages/eslint-plugin/tests/rules/ban-ts-comment.test.ts new file mode 100644 index 00000000000..3a59be879f9 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/ban-ts-comment.test.ts @@ -0,0 +1,233 @@ +import rule from '../../src/rules/ban-ts-comment'; +import { RuleTester } from '../RuleTester'; + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('ts-ignore', rule, { + valid: [ + `// just a comment containing @ts-ignore somewhere`, + `/* @ts-ignore */`, + `/** @ts-ignore */`, + `/* +// @ts-ignore in a block +*/`, + { + code: '// @ts-ignore', + options: [{ 'ts-ignore': false }], + }, + ], + invalid: [ + { + code: '// @ts-ignore', + options: [{ 'ts-ignore': true }], + errors: [ + { + data: { directive: 'ignore' }, + messageId: 'tsDirectiveComment', + line: 1, + column: 1, + }, + ], + }, + { + code: '// @ts-ignore', + errors: [ + { + data: { directive: 'ignore' }, + messageId: 'tsDirectiveComment', + line: 1, + column: 1, + }, + ], + }, + { + code: '// @ts-ignore: Suppress next line', + errors: [ + { + data: { directive: 'ignore' }, + messageId: 'tsDirectiveComment', + line: 1, + column: 1, + }, + ], + }, + { + code: '/////@ts-ignore: Suppress next line', + errors: [ + { + data: { directive: 'ignore' }, + messageId: 'tsDirectiveComment', + line: 1, + column: 1, + }, + ], + }, + { + code: ` +if (false) { + // @ts-ignore: Unreachable code error + console.log("hello"); +} + `, + errors: [ + { + data: { directive: 'ignore' }, + messageId: 'tsDirectiveComment', + line: 3, + column: 3, + }, + ], + }, + ], +}); + +ruleTester.run('ts-nocheck', rule, { + valid: [ + `// just a comment containing @ts-nocheck somewhere`, + `/* @ts-nocheck */`, + `/** @ts-nocheck */`, + `/* +// @ts-nocheck in a block +*/`, + { + code: '// @ts-nocheck', + options: [{ 'ts-nocheck': false }], + }, + ], + invalid: [ + { + code: '// @ts-nocheck', + options: [{ 'ts-nocheck': true }], + errors: [ + { + data: { directive: 'nocheck' }, + messageId: 'tsDirectiveComment', + line: 1, + column: 1, + }, + ], + }, + { + code: '// @ts-nocheck', + errors: [ + { + data: { directive: 'check' }, + messageId: 'tsDirectiveComment', + line: 1, + column: 1, + }, + ], + }, + { + code: '// @ts-nocheck: Suppress next line', + errors: [ + { + data: { directive: 'check' }, + messageId: 'tsDirectiveComment', + line: 1, + column: 1, + }, + ], + }, + { + code: '/////@ts-nocheck: Suppress next line', + errors: [ + { + data: { directive: 'check' }, + messageId: 'tsDirectiveComment', + line: 1, + column: 1, + }, + ], + }, + { + code: ` +if (false) { + // @ts-nocheck: Unreachable code error + console.log("hello"); +} + `, + errors: [ + { + data: { directive: 'check' }, + messageId: 'tsDirectiveComment', + line: 3, + column: 3, + }, + ], + }, + ], +}); + +ruleTester.run('ts-check', rule, { + valid: [ + `// just a comment containing @ts-check somewhere`, + `/* @ts-check */`, + `/** @ts-check */`, + `/* +// @ts-check in a block +*/`, + { + code: '// @ts-check', + options: [{ 'ts-check': false }], + }, + ], + invalid: [ + { + code: '// @ts-check', + options: [{ 'ts-check': true }], + errors: [ + { + data: { directive: 'check' }, + messageId: 'tsDirectiveComment', + line: 1, + column: 1, + }, + ], + }, + { + code: '// @ts-check: Suppress next line', + options: [{ 'ts-check': true }], + errors: [ + { + data: { directive: 'check' }, + messageId: 'tsDirectiveComment', + line: 1, + column: 1, + }, + ], + }, + { + code: '/////@ts-check: Suppress next line', + options: [{ 'ts-check': true }], + + errors: [ + { + data: { directive: 'check' }, + messageId: 'tsDirectiveComment', + line: 1, + column: 1, + }, + ], + }, + { + code: ` +if (false) { + // @ts-check: Unreachable code error + console.log("hello"); +} + `, + options: [{ 'ts-check': true }], + errors: [ + { + data: { directive: 'check' }, + messageId: 'tsDirectiveComment', + line: 3, + column: 3, + }, + ], + }, + ], +});