From 43ee226ffbaaa3e7126081db9476c24b89ec16e9 Mon Sep 17 00:00:00 2001 From: Drew Wyatt Date: Sun, 7 Jun 2020 15:08:42 -0400 Subject: [PATCH] feat(eslint-plugin): add rule `ban-tslint-comment` (#2140) Co-authored-by: Brad Zacher --- packages/eslint-plugin/README.md | 1 + .../docs/rules/ban-tslint-comment.md | 29 +++++++ packages/eslint-plugin/src/configs/all.ts | 1 + .../src/rules/ban-tslint-comment.ts | 60 +++++++++++++ packages/eslint-plugin/src/rules/index.ts | 2 + .../tests/rules/ban-tslint-comment.test.ts | 84 +++++++++++++++++++ 6 files changed, 177 insertions(+) create mode 100644 packages/eslint-plugin/docs/rules/ban-tslint-comment.md create mode 100644 packages/eslint-plugin/src/rules/ban-tslint-comment.ts create mode 100644 packages/eslint-plugin/tests/rules/ban-tslint-comment.test.ts diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 01de689fd78..53b8cbb9412 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -93,6 +93,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int | [`@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-comment`](./docs/rules/ban-ts-comment.md) | Bans `// @ts-` comments from being used or requires descriptions after directive | :heavy_check_mark: | | | +| [`@typescript-eslint/ban-tslint-comment`](./docs/rules/ban-tslint-comment.md) | Bans `// tslint:` comments from being used | | :wrench: | | | [`@typescript-eslint/ban-types`](./docs/rules/ban-types.md) | Bans specific types from being used | :heavy_check_mark: | :wrench: | | | [`@typescript-eslint/class-literal-property-style`](./docs/rules/class-literal-property-style.md) | Ensures that literals on classes are exposed in a consistent style | | :wrench: | | | [`@typescript-eslint/consistent-type-assertions`](./docs/rules/consistent-type-assertions.md) | Enforces consistent usage of type assertions | | | | diff --git a/packages/eslint-plugin/docs/rules/ban-tslint-comment.md b/packages/eslint-plugin/docs/rules/ban-tslint-comment.md new file mode 100644 index 00000000000..6af168f4bc4 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/ban-tslint-comment.md @@ -0,0 +1,29 @@ +# Bans `// tslint:` comments from being used (`ban-tslint-comment`) + +Useful when migrating from TSLint to ESLint. Once TSLint has been removed, this rule helps locate TSLint annotations (e.g. `// tslint:disable`). + +## Rule Details + +Examples of **incorrect** code for this rule: + +All TSLint [rule flags](https://palantir.github.io/tslint/usage/rule-flags/) + +```js +/* tslint:disable */ +/* tslint:enable */ +/* tslint:disable:rule1 rule2 rule3... */ +/* tslint:enable:rule1 rule2 rule3... */ +// tslint:disable-next-line +someCode(); // tslint:disable-line +// tslint:disable-next-line:rule1 rule2 rule3... +``` + +Examples of **correct** code for this rule: + +```js +// This is a comment that just happens to mention tslint +``` + +## When Not To Use It + +If you are still using TSLint. diff --git a/packages/eslint-plugin/src/configs/all.ts b/packages/eslint-plugin/src/configs/all.ts index f70cd1c9a14..25001233883 100644 --- a/packages/eslint-plugin/src/configs/all.ts +++ b/packages/eslint-plugin/src/configs/all.ts @@ -5,6 +5,7 @@ export = { '@typescript-eslint/array-type': 'error', '@typescript-eslint/await-thenable': 'error', '@typescript-eslint/ban-ts-comment': 'error', + '@typescript-eslint/ban-tslint-comment': 'error', '@typescript-eslint/ban-types': 'error', 'brace-style': 'off', '@typescript-eslint/brace-style': 'error', diff --git a/packages/eslint-plugin/src/rules/ban-tslint-comment.ts b/packages/eslint-plugin/src/rules/ban-tslint-comment.ts new file mode 100644 index 00000000000..4521fcb13c3 --- /dev/null +++ b/packages/eslint-plugin/src/rules/ban-tslint-comment.ts @@ -0,0 +1,60 @@ +import { AST_TOKEN_TYPES } from '@typescript-eslint/experimental-utils'; +import * as util from '../util'; + +// tslint regex +// https://github.com/palantir/tslint/blob/95d9d958833fd9dc0002d18cbe34db20d0fbf437/src/enableDisableRules.ts#L32 +const ENABLE_DISABLE_REGEX = /^\s*tslint:(enable|disable)(?:-(line|next-line))?(:|\s|$)/; + +const toText = ( + text: string, + type: AST_TOKEN_TYPES.Line | AST_TOKEN_TYPES.Block, +): string => + type === AST_TOKEN_TYPES.Line + ? ['//', text.trim()].join(' ') + : ['/*', text.trim(), '*/'].join(' '); + +export default util.createRule({ + name: 'ban-tslint-comment', + meta: { + type: 'suggestion', + docs: { + description: 'Bans `// tslint:` comments from being used', + category: 'Stylistic Issues', + recommended: false, + }, + messages: { + commentDetected: 'tslint comment detected: "{{ text }}"', + }, + schema: [], + fixable: 'code', + }, + defaultOptions: [], + create: context => { + const sourceCode = context.getSourceCode(); + return { + Program(): void { + const comments = sourceCode.getAllComments(); + comments.forEach(c => { + if (ENABLE_DISABLE_REGEX.test(c.value)) { + context.report({ + data: { text: toText(c.value, c.type) }, + node: c, + messageId: 'commentDetected', + fix(fixer) { + const rangeStart = sourceCode.getIndexFromLoc({ + column: c.loc.start.column > 0 ? c.loc.start.column - 1 : 0, + line: c.loc.start.line, + }); + const rangeEnd = sourceCode.getIndexFromLoc({ + column: c.loc.end.column, + line: c.loc.end.line, + }); + return fixer.removeRange([rangeStart, rangeEnd + 1]); + }, + }); + } + }); + }, + }; + }, +}); diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 918f4856c59..05016d5705f 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 banTsComment from './ban-ts-comment'; +import banTslintComment from './ban-tslint-comment'; import banTypes from './ban-types'; import braceStyle from './brace-style'; import classLiteralPropertyStyle from './class-literal-property-style'; @@ -101,6 +102,7 @@ export default { 'array-type': arrayType, 'await-thenable': awaitThenable, 'ban-ts-comment': banTsComment, + 'ban-tslint-comment': banTslintComment, 'ban-types': banTypes, 'brace-style': braceStyle, 'class-literal-property-style': classLiteralPropertyStyle, diff --git a/packages/eslint-plugin/tests/rules/ban-tslint-comment.test.ts b/packages/eslint-plugin/tests/rules/ban-tslint-comment.test.ts new file mode 100644 index 00000000000..654bb18c2c6 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/ban-tslint-comment.test.ts @@ -0,0 +1,84 @@ +import rule from '../../src/rules/ban-tslint-comment'; +import { RuleTester } from '../RuleTester'; + +interface Testable { + code: string; + text?: string; + column?: number; + line?: number; + output?: string; +} + +const PALANTIR_EXAMPLES: Testable[] = [ + { code: '/* tslint:disable */' }, // Disable all rules for the rest of the file + { code: '/* tslint:enable */' }, // Enable all rules for the rest of the file + { + code: '/* tslint:disable:rule1 rule2 rule3... */', + }, // Disable the listed rules for the rest of the file + { + code: '/* tslint:enable:rule1 rule2 rule3... */', + }, // Enable the listed rules for the rest of the file + { code: '// tslint:disable-next-line' }, // Disables all rules for the following line + { + code: 'someCode(); // tslint:disable-line', + text: '// tslint:disable-line', + column: 13, + output: 'someCode();', + }, // Disables all rules for the current line + { + code: '// tslint:disable-next-line:rule1 rule2 rule3...', + }, // Disables the listed rules for the next line +]; + +// prettier-ignore +const MORE_EXAMPLES: Testable[] = [ + { + code: `const woah = doSomeStuff(); +// tslint:disable-line +console.log(woah); +`, + output: `const woah = doSomeStuff(); +console.log(woah); +`, + text: '// tslint:disable-line', + line: 2, + }, +] + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('ban-tslint-comment', rule, { + valid: [ + { + code: 'let a: readonly any[] = [];', + }, + { + code: 'let a = new Array();', + }, + { + code: '// some other comment', + }, + { + code: '// TODO: this is a comment that mentions tslint', + }, + { + code: '/* another comment that mentions tslint */', + }, + ], + invalid: [...PALANTIR_EXAMPLES, ...MORE_EXAMPLES].map( + ({ code, column, line, output, text }) => ({ + code, + output: output ?? '', + errors: [ + { + column: column ?? 1, + line: line ?? 1, + data: { text: text ?? code }, + messageId: 'commentDetected' as const, + }, + ], + }), + ), +});