feat(eslint-plugin): [ban-ts-comments] add "allow-with-description" o…

…ption (#2099)

packages/eslint-plugin/README.md

@@ -92,7 +92,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                                                                      | :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-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                                                                            |                    |          |                   |

packages/eslint-plugin/docs/rules/ban-ts-comment.md

@@ -1,4 +1,4 @@
-# Bans `// @ts-<directive>` comments from being used (`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.
@@ -21,20 +21,24 @@ The configuration looks like this:
 
 ```ts
 interface Options {
-  'ts-expect-error'?: boolean;
-  'ts-ignore'?: boolean;
-  'ts-nocheck'?: boolean;
-  'ts-check'?: boolean;
+  'ts-expect-error'?: boolean | 'allow-with-description';
+  'ts-ignore'?: boolean | 'allow-with-description';
+  'ts-nocheck'?: boolean | 'allow-with-description';
+  'ts-check'?: boolean | 'allow-with-description';
+  minimumDescriptionLength?: number;
 }
 
 const defaultOptions: Options = {
   'ts-expect-error': true,
   'ts-ignore': true,
   'ts-nocheck': true,
   'ts-check': false,
+  minimumDescriptionLength: 3,
 };
 ```
 
+### `ts-expect-error`, `ts-ignore`, `ts-nocheck`, `ts-check` directives
+
 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:
@@ -55,6 +59,50 @@ if (false) {
 }
 ```
 
+### `allow-with-description`
+
+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:
+
+```ts
+if (false) {
+  // @ts-expect-error
+  console.log('hello');
+}
+```
+
+The following pattern is not a warning:
+
+```ts
+if (false) {
+  // @ts-expect-error: Unreachable code error
+  console.log('hello');
+}
+```
+
+### `minimumDescriptionLength`
+
+Use `minimumDescriptionLength` to set a minimum length for descriptions when using the `allow-with-description` option for a directive.
+
+For example, with `{ 'ts-expect-error': 'allow-with-description', minimumDescriptionLength: 10 }` the following pattern is considered a warning:
+
+```ts
+if (false) {
+  // @ts-expect-error: TODO
+  console.log('hello');
+}
+```
+
+The following pattern is not a warning:
+
+```ts
+if (false) {
+  // @ts-expect-error The rationale for this override is described in issue #1337 on GitLab
+  console.log('hello');
+}
+```
+
 ## When Not To Use It
 
 If you want to use all of the TypeScript directives.

packages/eslint-plugin/src/rules/ban-ts-comment.ts

@@ -2,55 +2,96 @@ 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;
+  'ts-expect-error'?: boolean | 'allow-with-description';
+  'ts-ignore'?: boolean | 'allow-with-description';
+  'ts-nocheck'?: boolean | 'allow-with-description';
+  'ts-check'?: boolean | 'allow-with-description';
+  minimumDescriptionLength?: number;
 }
 
+export const defaultMinimumDescriptionLength = 3;
+
 const defaultOptions: [Options] = [
   {
     'ts-expect-error': true,
     'ts-ignore': true,
     'ts-nocheck': true,
     'ts-check': false,
+    minimumDescriptionLength: defaultMinimumDescriptionLength,
   },
 ];
 
-type MessageIds = 'tsDirectiveComment';
+type MessageIds =
+  | 'tsDirectiveComment'
+  | 'tsDirectiveCommentRequiresDescription';
 
 export default util.createRule<[Options], MessageIds>({
   name: 'ban-ts-comment',
   meta: {
     type: 'problem',
     docs: {
-      description: 'Bans `// @ts-<directive>` comments from being used',
+      description:
+        '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.',
+      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.',
     },
     schema: [
       {
         type: 'object',
         properties: {
           'ts-expect-error': {
-            type: 'boolean',
-            default: true,
+            oneOf: [
+              {
+                type: 'boolean',
+                default: true,
+              },
+              {
+                enum: ['allow-with-description'],
+              },
+            ],
           },
           'ts-ignore': {
-            type: 'boolean',
-            default: true,
+            oneOf: [
+              {
+                type: 'boolean',
+                default: true,
+              },
+              {
+                enum: ['allow-with-description'],
+              },
+            ],
           },
           'ts-nocheck': {
-            type: 'boolean',
-            default: true,
+            oneOf: [
+              {
+                type: 'boolean',
+                default: true,
+              },
+              {
+                enum: ['allow-with-description'],
+              },
+            ],
           },
           'ts-check': {
-            type: 'boolean',
-            default: false,
+            oneOf: [
+              {
+                type: 'boolean',
+                default: true,
+              },
+              {
+                enum: ['allow-with-description'],
+              },
+            ],
+          },
+          minimumDescriptionLength: {
+            type: 'number',
+            default: defaultMinimumDescriptionLength,
           },
         },
         additionalProperties: false,
@@ -59,7 +100,7 @@ export default util.createRule<[Options], MessageIds>({
   },
   defaultOptions,
   create(context, [options]) {
-    const tsCommentRegExp = /^\/*\s*@ts-(expect-error|ignore|check|nocheck)/;
+    const tsCommentRegExp = /^\/*\s*@ts-(expect-error|ignore|check|nocheck)(.*)/;
     const sourceCode = context.getSourceCode();
 
     return {
@@ -71,17 +112,32 @@ export default util.createRule<[Options], MessageIds>({
             return;
           }
 
-          const [, directive] = tsCommentRegExp.exec(comment.value) ?? [];
+          const [, directive, description] =
+            tsCommentRegExp.exec(comment.value) ?? [];
 
           const fullDirective = `ts-${directive}` as keyof Options;
 
-          if (options[fullDirective]) {
+          const option = options[fullDirective];
+          if (option === true) {
             context.report({
               data: { directive },
               node: comment,
               messageId: 'tsDirectiveComment',
             });
           }
+
+          if (option === 'allow-with-description') {
+            const {
+              minimumDescriptionLength = defaultMinimumDescriptionLength,
+            } = options;
+            if (description.trim().length < minimumDescriptionLength) {
+              context.report({
+                data: { directive, minimumDescriptionLength },
+                node: comment,
+                messageId: 'tsDirectiveCommentRequiresDescription',
+              });
+            }
+          }
         });
       },
     };