From 4507ac84c76da0ced43b6679305afe1891e7afca Mon Sep 17 00:00:00 2001 From: Joshua Chen Date: Fri, 6 May 2022 15:06:57 +0800 Subject: [PATCH] fix(eslint-plugin): [no-unnecessary-type-constraint] change to suggestion fix, fix multiple trailing comma failures (#4901) --- packages/eslint-plugin/README.md | 2 +- packages/eslint-plugin/docs/rules/README.md | 2 +- .../rules/no-unnecessary-type-constraint.md | 2 +- .../rules/no-unnecessary-type-constraint.ts | 36 ++- .../no-unnecessary-type-constraint.test.ts | 241 ++++++++++++++++-- 5 files changed, 250 insertions(+), 33 deletions(-) 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: | 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 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..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 @@ -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,142 @@ 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: 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: [ + { + 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 +254,14 @@ function data() {} endColumn: 32, column: 15, line: 1, + suggestions: [ + { + messageId: 'removeUnnecessaryConstraint', + output: 'function data() {}', + }, + ], }, ], - output: 'function data() {}', }, { code: 'const data = () => {};', @@ -115,9 +272,14 @@ function data() {} endColumn: 28, column: 15, line: 1, + suggestions: [ + { + messageId: 'removeUnnecessaryConstraint', + output: 'const data = () => {};', + }, + ], }, ], - output: 'const data = () => {};', }, { code: 'const data = () => {};', @@ -128,9 +290,14 @@ function data() {} endColumn: 32, column: 15, line: 1, + suggestions: [ + { + messageId: 'removeUnnecessaryConstraint', + output: 'const data = () => {};', + }, + ], }, ], - output: 'const data = () => {};', }, { code: 'class Data {}', @@ -141,9 +308,14 @@ function data() {} endColumn: 29, column: 12, line: 1, + suggestions: [ + { + messageId: 'removeUnnecessaryConstraint', + output: 'class Data {}', + }, + ], }, ], - output: 'class Data {}', }, { code: 'const Data = class {};', @@ -154,16 +326,21 @@ function data() {} endColumn: 37, column: 20, line: 1, + suggestions: [ + { + messageId: 'removeUnnecessaryConstraint', + output: 'const Data = class {};', + }, + ], }, ], - output: 'const Data = class {};', }, { code: ` class Data { member() {} } - `, + `.trimEnd(), errors: [ { data: { constraint: 'unknown', name: 'T' }, @@ -171,20 +348,25 @@ class Data { endColumn: 27, column: 10, line: 3, - }, - ], - output: ` + suggestions: [ + { + messageId: 'removeUnnecessaryConstraint', + output: ` class Data { member() {} } - `, + `.trimEnd(), + }, + ], + }, + ], }, { code: ` const Data = class { member() {} }; - `, + `.trimEnd(), errors: [ { data: { constraint: 'unknown', name: 'T' }, @@ -192,13 +374,18 @@ const Data = class { endColumn: 27, column: 10, line: 3, - }, - ], - output: ` + suggestions: [ + { + messageId: 'removeUnnecessaryConstraint', + output: ` const Data = class { member() {} }; - `, + `.trimEnd(), + }, + ], + }, + ], }, { code: 'interface Data {}', @@ -209,9 +396,14 @@ const Data = class { endColumn: 33, column: 16, line: 1, + suggestions: [ + { + messageId: 'removeUnnecessaryConstraint', + output: 'interface Data {}', + }, + ], }, ], - output: 'interface Data {}', }, { code: 'type Data = {};', @@ -222,9 +414,14 @@ const Data = class { endColumn: 28, column: 11, line: 1, + suggestions: [ + { + messageId: 'removeUnnecessaryConstraint', + output: 'type Data = {};', + }, + ], }, ], - output: 'type Data = {};', }, ], });