From ccb0d0152ad1a41f02906bdb666e51db176e6d4f Mon Sep 17 00:00:00 2001 From: Joshua Chen Date: Wed, 4 May 2022 12:50:31 +0800 Subject: [PATCH 1/4] fix(eslint-plugin): [no-unnecessary-type-contraint] change to suggestion fix, fix multiple trailing comma failures --- .../rules/no-unnecessary-type-constraint.ts | 36 +++- .../no-unnecessary-type-constraint.test.ts | 180 ++++++++++++++++-- 2 files changed, 188 insertions(+), 28 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-constraint.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-constraint.ts index c3d8e6fbb39..3987d957dd1 100644 --- 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,12 +60,25 @@ 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 () => {} would need trailing comma + return ( + (node.parent as TSESTree.TSTypeParameterDeclaration).params.length === + 1 && + source.getTokensAfter(node)[0].value !== ',' && + !node.default + ); + } if (constraint) { context.report({ @@ -71,12 +86,17 @@ export default util.createRule({ 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, }); diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-type-constraint.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-type-constraint.test.ts index c71821ffeb4..6b38f6b311e 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-type-constraint.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-type-constraint.test.ts @@ -36,9 +36,14 @@ function data() {} endColumn: 28, column: 15, line: 1, + suggestions: [ + { + messageId: 'removeUnnecessaryConstraint', + output: 'function data() {}', + }, + ], }, ], - output: 'function data() {}', }, { code: 'function data() {}', @@ -49,9 +54,14 @@ function data() {} endColumn: 28, column: 15, line: 1, + suggestions: [ + { + messageId: 'removeUnnecessaryConstraint', + output: 'function data() {}', + }, + ], }, ], - output: 'function data() {}', }, { code: 'function data() {}', @@ -62,9 +72,14 @@ function data() {} endColumn: 31, column: 18, line: 1, + suggestions: [ + { + messageId: 'removeUnnecessaryConstraint', + output: 'function data() {}', + }, + ], }, ], - output: 'function data() {}', }, { code: 'function data() {}', @@ -75,9 +90,14 @@ function data() {} endColumn: 28, column: 15, line: 1, + suggestions: [ + { + messageId: 'removeUnnecessaryConstraint', + output: 'function data() {}', + }, + ], }, ], - output: 'function data() {}', }, { code: 'const data = () => {};', @@ -88,10 +108,85 @@ function data() {} endColumn: 28, column: 15, line: 1, + suggestions: [ + { + messageId: 'removeUnnecessaryConstraint', + output: noFormat`const data = () => {};`, + }, + ], + }, + ], + filename: 'react.tsx', + }, + { + code: noFormat`const data = () => {};`, + errors: [ + { + data: { constraint: 'any', name: 'T' }, + messageId: 'unnecessaryConstraint', + endColumn: 28, + column: 15, + line: 1, + suggestions: [ + { + messageId: 'removeUnnecessaryConstraint', + output: noFormat`const data = () => {};`, + }, + ], + }, + ], + filename: 'react.tsx', + }, + { + code: 'const data = () => {};', + errors: [ + { + data: { constraint: 'any', name: 'T' }, + messageId: 'unnecessaryConstraint', + endColumn: 38, + column: 15, + line: 1, + suggestions: [ + { + messageId: 'removeUnnecessaryConstraint', + output: 'const data = () => {};', + }, + ], + }, + ], + filename: 'react.tsx', + }, + { + code: 'const data = () => {};', + errors: [ + { + data: { constraint: 'any', name: 'T' }, + messageId: 'unnecessaryConstraint', + endColumn: 28, + column: 15, + line: 1, + suggestions: [ + { + messageId: 'removeUnnecessaryConstraint', + output: noFormat`const data = () => {};`, + }, + ], + }, + { + data: { constraint: 'any', name: 'U' }, + messageId: 'unnecessaryConstraint', + endColumn: 43, + column: 30, + line: 1, + suggestions: [ + { + messageId: 'removeUnnecessaryConstraint', + output: noFormat`const data = () => {};`, + }, + ], }, ], filename: 'react.tsx', - output: noFormat`const data = () => {};`, }, { code: 'function data() {}', @@ -102,9 +197,14 @@ function data() {} endColumn: 32, column: 15, line: 1, + suggestions: [ + { + messageId: 'removeUnnecessaryConstraint', + output: 'function data() {}', + }, + ], }, ], - output: 'function data() {}', }, { code: 'const data = () => {};', @@ -115,9 +215,14 @@ function data() {} endColumn: 28, column: 15, line: 1, + suggestions: [ + { + messageId: 'removeUnnecessaryConstraint', + output: 'const data = () => {};', + }, + ], }, ], - output: 'const data = () => {};', }, { code: 'const data = () => {};', @@ -128,9 +233,14 @@ function data() {} endColumn: 32, column: 15, line: 1, + suggestions: [ + { + messageId: 'removeUnnecessaryConstraint', + output: 'const data = () => {};', + }, + ], }, ], - output: 'const data = () => {};', }, { code: 'class Data {}', @@ -141,9 +251,14 @@ function data() {} endColumn: 29, column: 12, line: 1, + suggestions: [ + { + messageId: 'removeUnnecessaryConstraint', + output: 'class Data {}', + }, + ], }, ], - output: 'class Data {}', }, { code: 'const Data = class {};', @@ -154,9 +269,14 @@ function data() {} endColumn: 37, column: 20, line: 1, + suggestions: [ + { + messageId: 'removeUnnecessaryConstraint', + output: 'const Data = class {};', + }, + ], }, ], - output: 'const Data = class {};', }, { code: ` @@ -171,13 +291,18 @@ class Data { endColumn: 27, column: 10, line: 3, - }, - ], - output: ` + suggestions: [ + { + messageId: 'removeUnnecessaryConstraint', + output: ` class Data { member() {} } - `, + `, + }, + ], + }, + ], }, { code: ` @@ -192,13 +317,18 @@ const Data = class { endColumn: 27, column: 10, line: 3, - }, - ], - output: ` + suggestions: [ + { + messageId: 'removeUnnecessaryConstraint', + output: ` const Data = class { member() {} }; - `, + `, + }, + ], + }, + ], }, { code: 'interface Data {}', @@ -209,9 +339,14 @@ const Data = class { endColumn: 33, column: 16, line: 1, + suggestions: [ + { + messageId: 'removeUnnecessaryConstraint', + output: 'interface Data {}', + }, + ], }, ], - output: 'interface Data {}', }, { code: 'type Data = {};', @@ -222,9 +357,14 @@ const Data = class { endColumn: 28, column: 11, line: 1, + suggestions: [ + { + messageId: 'removeUnnecessaryConstraint', + output: 'type Data = {};', + }, + ], }, ], - output: 'type Data = {};', }, ], }); From 35f3d1e045da99997d6f826360bff7f8bc02220f Mon Sep 17 00:00:00 2001 From: Joshua Chen Date: Thu, 5 May 2022 23:24:07 +0800 Subject: [PATCH 2/4] test: fix --- .../no-unnecessary-type-constraint.test.ts | 65 +++++++++++++++++-- 1 file changed, 61 insertions(+), 4 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-type-constraint.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-type-constraint.test.ts index 6b38f6b311e..91798c8b059 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-type-constraint.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-type-constraint.test.ts @@ -137,6 +137,63 @@ function data() {} ], filename: 'react.tsx', }, + { + code: noFormat`const data = () => {};`, + errors: [ + { + data: { constraint: 'any', name: 'T' }, + messageId: 'unnecessaryConstraint', + endColumn: 28, + column: 15, + line: 1, + suggestions: [ + { + messageId: 'removeUnnecessaryConstraint', + output: noFormat`const data = () => {};`, + }, + ], + }, + ], + filename: 'react.tsx', + }, + { + code: noFormat`const data = () => {};`, + errors: [ + { + data: { constraint: 'any', name: 'T' }, + messageId: 'unnecessaryConstraint', + endColumn: 28, + column: 15, + line: 1, + suggestions: [ + { + messageId: 'removeUnnecessaryConstraint', + output: noFormat`const data = () => {};`, + }, + ], + }, + ], + filename: 'react.tsx', + }, + { + code: noFormat`const data = () => {};`, + errors: [ + { + data: { constraint: 'any', name: 'T' }, + messageId: 'unnecessaryConstraint', + endColumn: 28, + column: 15, + line: 1, + suggestions: [ + { + messageId: 'removeUnnecessaryConstraint', + output: noFormat`const data = () => {};`, + }, + ], + }, + ], + filename: 'react.tsx', + }, { code: 'const data = () => {};', errors: [ @@ -283,7 +340,7 @@ function data() {} class Data { member() {} } - `, + `.trimEnd(), errors: [ { data: { constraint: 'unknown', name: 'T' }, @@ -298,7 +355,7 @@ class Data { class Data { member() {} } - `, + `.trimEnd(), }, ], }, @@ -309,7 +366,7 @@ class Data { const Data = class { member() {} }; - `, + `.trimEnd(), errors: [ { data: { constraint: 'unknown', name: 'T' }, @@ -324,7 +381,7 @@ const Data = class { const Data = class { member() {} }; - `, + `.trimEnd(), }, ], }, From aa34afa5144689d4ba3f3d5e2946301cfbe79f8a Mon Sep 17 00:00:00 2001 From: Joshua Chen Date: Thu, 5 May 2022 23:37:19 +0800 Subject: [PATCH 3/4] docs: update --- packages/eslint-plugin/docs/rules/README.md | 2 +- .../eslint-plugin/docs/rules/no-unnecessary-type-constraint.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/README.md b/packages/eslint-plugin/docs/rules/README.md index 02a16d4452b..944a7452a35 100644 --- 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 index 5bbb2c6cb65..122b6428566 100644 --- 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 From 0b697489c2799feb940c5739eb041c12aea1d918 Mon Sep 17 00:00:00 2001 From: Joshua Chen Date: Fri, 6 May 2022 14:46:16 +0800 Subject: [PATCH 4/4] docs: fix --- packages/eslint-plugin/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 7b1d5d03cc2..511f9679f11 100644 --- 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: |