fix(eslint-plugin): [no-unnecessary-type-constraint] change to sugges…

…tion fix, fix multiple trailing comma failures (#4901)
typescript-eslint · May 6, 2022 · 4507ac8 · 4507ac8
1 parent da48527
commit 4507ac8
Show file tree

Hide file tree

Showing 5 changed files with 250 additions and 33 deletions.
diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md
@@ -145,7 +145,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int
 | [`@typescript-eslint/no-unnecessary-qualifier`](./docs/rules/no-unnecessary-qualifier.md)                               | Warns when a namespace qualifier is unnecessary                                                                         |                    | :wrench: | :thought_balloon: |
 | [`@typescript-eslint/no-unnecessary-type-arguments`](./docs/rules/no-unnecessary-type-arguments.md)                     | Enforces that type arguments will not be used if not required                                                           |                    | :wrench: | :thought_balloon: |
 | [`@typescript-eslint/no-unnecessary-type-assertion`](./docs/rules/no-unnecessary-type-assertion.md)                     | Warns if a type assertion does not change the type of an expression                                                     | :white_check_mark: | :wrench: | :thought_balloon: |
-| [`@typescript-eslint/no-unnecessary-type-constraint`](./docs/rules/no-unnecessary-type-constraint.md)                   | Disallows unnecessary constraints on generic types                                                                      | :white_check_mark: | :wrench: |                   |
+| [`@typescript-eslint/no-unnecessary-type-constraint`](./docs/rules/no-unnecessary-type-constraint.md)                   | Disallows unnecessary constraints on generic types                                                                      | :white_check_mark: |          |                   |
 | [`@typescript-eslint/no-unsafe-argument`](./docs/rules/no-unsafe-argument.md)                                           | Disallows calling a function with an any type value                                                                     | :white_check_mark: |          | :thought_balloon: |
 | [`@typescript-eslint/no-unsafe-assignment`](./docs/rules/no-unsafe-assignment.md)                                       | Disallows assigning any to variables and properties                                                                     | :white_check_mark: |          | :thought_balloon: |
 | [`@typescript-eslint/no-unsafe-call`](./docs/rules/no-unsafe-call.md)                                                   | Disallows calling an any type value                                                                                     | :white_check_mark: |          | :thought_balloon: |

diff --git a/packages/eslint-plugin/docs/rules/README.md b/packages/eslint-plugin/docs/rules/README.md
@@ -67,7 +67,7 @@ slug: /
 | [`@typescript-eslint/no-unnecessary-qualifier`](./no-unnecessary-qualifier.md)                               | Warns when a namespace qualifier is unnecessary                                                                         |                    | :wrench: | :thought_balloon: |
 | [`@typescript-eslint/no-unnecessary-type-arguments`](./no-unnecessary-type-arguments.md)                     | Enforces that type arguments will not be used if not required                                                           |                    | :wrench: | :thought_balloon: |
 | [`@typescript-eslint/no-unnecessary-type-assertion`](./no-unnecessary-type-assertion.md)                     | Warns if a type assertion does not change the type of an expression                                                     | :white_check_mark: | :wrench: | :thought_balloon: |
-| [`@typescript-eslint/no-unnecessary-type-constraint`](./no-unnecessary-type-constraint.md)                   | Disallows unnecessary constraints on generic types                                                                      | :white_check_mark: | :wrench: |                   |
+| [`@typescript-eslint/no-unnecessary-type-constraint`](./no-unnecessary-type-constraint.md)                   | Disallows unnecessary constraints on generic types                                                                      | :white_check_mark: |          |                   |
 | [`@typescript-eslint/no-unsafe-argument`](./no-unsafe-argument.md)                                           | Disallows calling a function with an any type value                                                                     | :white_check_mark: |          | :thought_balloon: |
 | [`@typescript-eslint/no-unsafe-assignment`](./no-unsafe-assignment.md)                                       | Disallows assigning any to variables and properties                                                                     | :white_check_mark: |          | :thought_balloon: |
 | [`@typescript-eslint/no-unsafe-call`](./no-unsafe-call.md)                                                   | Disallows calling an any type value                                                                                     | :white_check_mark: |          | :thought_balloon: |

diff --git a/packages/eslint-plugin/docs/rules/no-unnecessary-type-constraint.md b/packages/eslint-plugin/docs/rules/no-unnecessary-type-constraint.md
@@ -76,5 +76,5 @@ If you don't care about the specific styles of your type constraints, or never u
 ## Attributes
 
 - [x] ✅ Recommended
-- [x] 🔧 Fixable
+- [ ] 🔧 Fixable
 - [ ] 💭 Requires type information
diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-constraint.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-constraint.ts
@@ -1,4 +1,4 @@
-import { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/utils';
+import { AST_NODE_TYPES, TSESTree, TSESLint } from '@typescript-eslint/utils';
 import * as semver from 'semver';
 import * as ts from 'typescript';
 import * as util from '../util';
@@ -33,10 +33,12 @@ export default util.createRule({
       recommended: 'error',
       suggestion: true,
     },
-    fixable: 'code',
+    hasSuggestions: true,
     messages: {
       unnecessaryConstraint:
         'Constraining the generic type `{{name}}` to `{{constraint}}` does nothing and is unnecessary.',
+      removeUnnecessaryConstraint:
+        'Remove the unnecessary `{{constraint}}` constraint.',
     },
     schema: [],
     type: 'suggestion',
@@ -58,25 +60,43 @@ export default util.createRule({
       : new Map([[AST_NODE_TYPES.TSUnknownKeyword, 'unknown']]);
 
     const inJsx = context.getFilename().toLowerCase().endsWith('tsx');
+    const source = context.getSourceCode();
 
     const checkNode = (
       node: TypeParameterWithConstraint,
       inArrowFunction: boolean,
     ): void => {
       const constraint = unnecessaryConstraints.get(node.constraint.type);
+      function shouldAddTrailingComma(): boolean {
+        if (!inArrowFunction || !inJsx) {
+          return false;
+        }
+        // Only <T>() => {} would need trailing comma
+        return (
+          (node.parent as TSESTree.TSTypeParameterDeclaration).params.length ===
+            1 &&
+          source.getTokensAfter(node)[0].value !== ',' &&
+          !node.default
+        );
+      }
 
       if (constraint) {
         context.report({
           data: {
             constraint,
             name: node.name.name,
           },
-          fix(fixer) {
-            return fixer.replaceTextRange(
-              [node.name.range[1], node.constraint.range[1]],
-              inArrowFunction && inJsx ? ',' : '',
-            );
-          },
+          suggest: [
+            {
+              messageId: 'removeUnnecessaryConstraint',
+              fix(fixer): TSESLint.RuleFix | null {
+                return fixer.replaceTextRange(
+                  [node.name.range[1], node.constraint.range[1]],
+                  shouldAddTrailingComma() ? ',' : '',
+                );
+              },
+            },
+          ],
           messageId: 'unnecessaryConstraint',
           node,
         });