Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(eslint-plugin): add rule ban-tslint-comment #2140

Merged
merged 14 commits into from Jun 7, 2020
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/eslint-plugin/README.md
Expand Up @@ -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<T>` 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-<directive>` 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:<rule-flag>` 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 | | | |
Expand Down
29 changes: 29 additions & 0 deletions packages/eslint-plugin/docs/rules/ban-tslint-comment.md
@@ -0,0 +1,29 @@
# Bans `// tslint:<rule-flag>` 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.
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/configs/all.ts
Expand Up @@ -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',
Expand Down
60 changes: 60 additions & 0 deletions 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:<rule-flag>` 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]);
bradzacher marked this conversation as resolved.
Show resolved Hide resolved
},
});
}
});
},
};
},
});
2 changes: 2 additions & 0 deletions packages/eslint-plugin/src/rules/index.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -100,6 +101,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,
Expand Down
84 changes: 84 additions & 0 deletions 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,
},
],
}),
),
});