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

fix(eslint-plugin): [ban-ts-comment] support block comments #2644

Merged
merged 12 commits into from Oct 12, 2020
2 changes: 1 addition & 1 deletion packages/eslint-plugin/README.md
Expand Up @@ -100,7 +100,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<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-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: | |
Expand Down
24 changes: 20 additions & 4 deletions packages/eslint-plugin/docs/rules/ban-ts-comment.md
@@ -1,4 +1,4 @@
# Bans `// @ts-<directive>` comments from being used or requires descriptions after directive (`ban-ts-comment`)
# Bans `@ts-<directive>` comments from being used or requires descriptions after directive (`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.
Expand Down Expand Up @@ -41,13 +41,19 @@ const defaultOptions: Options = {

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:
For example, with the defaults above the following patterns are considered warnings for single line or comment block lines:

```ts
if (false) {
// @ts-ignore: Unreachable code error
console.log('hello');
}
if (false) {
/*
@ts-ignore: Unreachable code error
*/
console.log('hello');
}
```

The following patterns are not warnings:
Expand All @@ -63,22 +69,32 @@ if (false) {

A value of `'allow-with-description'` for a particular directive means that this rule will report if it finds a directive that does not have a description following the directive (on the same line).

For example, with `{ 'ts-expect-error': 'allow-with-description' }` the following pattern is considered a warning:
For example, with `{ 'ts-expect-error': 'allow-with-description' }` the following patterns are considered a warning:

```ts
if (false) {
// @ts-expect-error
console.log('hello');
}
if (false) {
/* @ts-expect-error */
console.log('hello');
}
```

The following pattern is not a warning:
The following patterns are not a warning:

```ts
if (false) {
// @ts-expect-error: Unreachable code error
console.log('hello');
}
if (false) {
/*
@ts-expect-error: Unreachable code error
*/
console.log('hello');
}
```

### `minimumDescriptionLength`
Expand Down
21 changes: 13 additions & 8 deletions packages/eslint-plugin/src/rules/ban-ts-comment.ts
Expand Up @@ -21,15 +21,15 @@ export default util.createRule<[Options], MessageIds>({
type: 'problem',
docs: {
description:
'Bans `// @ts-<directive>` comments from being used or requires descriptions after directive',
'Bans `@ts-<directive>` comments from being used or requires descriptions after directive',
category: 'Best Practices',
recommended: 'error',
},
messages: {
tsDirectiveComment:
'Do not use "// @ts-{{directive}}" because it alters compilation errors.',
'Do not use "@ts-{{directive}}" because it alters compilation errors.',
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.',
'Include a description after the "@ts-{{directive}}" directive to explain why the @ts-{{directive}} is necessary. The description must be {{minimumDescriptionLength}} characters or longer.',
},
schema: [
{
Expand Down Expand Up @@ -98,20 +98,25 @@ export default util.createRule<[Options], MessageIds>({
},
],
create(context, [options]) {
const tsCommentRegExp = /^\/*\s*@ts-(expect-error|ignore|check|nocheck)(.*)/;
/*
The regex used are taken from the ones used in the official TypeScript repo -
https://github.com/microsoft/TypeScript/blob/master/src/compiler/scanner.ts#L281-L289
*/
const commentDirectiveRegExSingleLine = /^\/*\s*@ts-(expect-error|ignore|check|nocheck)(.*)/;
const commentDirectiveRegExMultiLine = /^\s*(?:\/|\*)*\s*@ts-(expect-error|ignore|check|nocheck)(.*)/;
dopecodez marked this conversation as resolved.
Show resolved Hide resolved
const sourceCode = context.getSourceCode();

return {
Program(): void {
const comments = sourceCode.getAllComments();

comments.forEach(comment => {
let regExp = commentDirectiveRegExSingleLine;

if (comment.type !== AST_TOKEN_TYPES.Line) {
return;
regExp = commentDirectiveRegExMultiLine;
}

const [, directive, description] =
tsCommentRegExp.exec(comment.value) ?? [];
const [, directive, description] = regExp.exec(comment.value) ?? [];

const fullDirective = `ts-${directive}` as keyof Options;

Expand Down