From 681ac8ed2df7afa44f48790a91f6d57030814505 Mon Sep 17 00:00:00 2001 From: Hasegawa-Yukihiro Date: Sun, 2 Mar 2025 21:45:04 +0900 Subject: [PATCH 01/10] fix: parenthesize type in suggestion fixer if necessary --- .../src/rules/no-unnecessary-type-parameters.ts | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts index f66f75482289..2ad56ac1ca4e 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts @@ -108,7 +108,20 @@ export default createRule({ for (const reference of smTypeParameterVariable.references) { if (reference.isTypeReference) { const referenceNode = reference.identifier; - yield fixer.replaceText(referenceNode, constraintText); + const isUnionTypeArray = + (constraint?.type === AST_NODE_TYPES.TSUnionType || + constraint?.type === + AST_NODE_TYPES.TSIntersectionType) && + referenceNode.parent.parent?.type === + AST_NODE_TYPES.TSArrayType; + if (isUnionTypeArray) { + yield fixer.replaceText( + referenceNode, + `(${constraintText})`, + ); + } else { + yield fixer.replaceText(referenceNode, constraintText); + } } } From e5cbe8e73c9428d693ab6d6efb33f9311bb2fd38 Mon Sep 17 00:00:00 2001 From: Hasegawa-Yukihiro Date: Sun, 2 Mar 2025 21:45:41 +0900 Subject: [PATCH 02/10] test: add test --- .../no-unnecessary-type-parameters.test.ts | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-type-parameters.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-type-parameters.test.ts index cba7279691f3..6cfce3cdd52b 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-type-parameters.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-type-parameters.test.ts @@ -1737,6 +1737,52 @@ class Joiner { join(els: number[]) { return els.map(el => '' + el).join(','); } +} + `, + }, + ], + }, + ], + }, + { + code: ` +function join(els: T[]) { + return els.map(el => '' + el).join(','); +} + `, + errors: [ + { + data: { descriptor: 'function', name: 'T', uses: 'used only once' }, + messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` +function join(els: (string | number)[]) { + return els.map(el => '' + el).join(','); +} + `, + }, + ], + }, + ], + }, + { + code: ` +function join(els: T[]) { + return els.map(el => '' + el).join(','); +} + `, + errors: [ + { + data: { descriptor: 'function', name: 'T', uses: 'used only once' }, + messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` +function join(els: (string & number)[]) { + return els.map(el => '' + el).join(','); } `, }, From cc8af2e763a46b323ec55009bd609af6628b5997 Mon Sep 17 00:00:00 2001 From: Hasegawa-Yukihiro Date: Sat, 8 Mar 2025 12:52:18 +0900 Subject: [PATCH 03/10] feat: support TSIndexedAccessType --- .../src/rules/no-unnecessary-type-parameters.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts index 2ad56ac1ca4e..9c05113e1621 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts @@ -108,13 +108,14 @@ export default createRule({ for (const reference of smTypeParameterVariable.references) { if (reference.isTypeReference) { const referenceNode = reference.identifier; - const isUnionTypeArray = - (constraint?.type === AST_NODE_TYPES.TSUnionType || - constraint?.type === - AST_NODE_TYPES.TSIntersectionType) && - referenceNode.parent.parent?.type === - AST_NODE_TYPES.TSArrayType; - if (isUnionTypeArray) { + const isCompositeType = + constraint?.type === AST_NODE_TYPES.TSUnionType || + constraint?.type === AST_NODE_TYPES.TSIntersectionType; + const hasMatchingAncestorType = [ + AST_NODE_TYPES.TSArrayType, + AST_NODE_TYPES.TSIndexedAccessType, + ].some(type => referenceNode.parent.parent?.type === type); + if (isCompositeType && hasMatchingAncestorType) { yield fixer.replaceText( referenceNode, `(${constraintText})`, From 1a60c5beb4aaa59cf74005ace38953b5d10ff089 Mon Sep 17 00:00:00 2001 From: Hasegawa-Yukihiro Date: Sat, 8 Mar 2025 12:52:30 +0900 Subject: [PATCH 04/10] test: add test --- .../no-unnecessary-type-parameters.test.ts | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-type-parameters.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-type-parameters.test.ts index 6cfce3cdd52b..1322bc514d2d 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-type-parameters.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-type-parameters.test.ts @@ -1783,6 +1783,29 @@ function join(els: T[]) { output: ` function join(els: (string & number)[]) { return els.map(el => '' + el).join(','); +} + `, + }, + ], + }, + ], + }, + { + code: ` +function join(els: T['hoge'][]) { + return els.map(el => '' + el).join(','); +} + `, + errors: [ + { + data: { descriptor: 'function', name: 'T', uses: 'used only once' }, + messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` +function join(els: ({ hoge: string } | { hoge: number })['hoge'][]) { + return els.map(el => '' + el).join(','); } `, }, From 91dd50d9df0d4758deba94405cba78a6e2b4459e Mon Sep 17 00:00:00 2001 From: Hasegawa-Yukihiro Date: Mon, 10 Mar 2025 22:25:30 +0900 Subject: [PATCH 05/10] fix: add "!" type assertion --- .../eslint-plugin/src/rules/no-unnecessary-type-parameters.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts index 9c05113e1621..581088dbbe1c 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts @@ -114,7 +114,8 @@ export default createRule({ const hasMatchingAncestorType = [ AST_NODE_TYPES.TSArrayType, AST_NODE_TYPES.TSIndexedAccessType, - ].some(type => referenceNode.parent.parent?.type === type); + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + ].some(type => referenceNode.parent.parent!.type === type); if (isCompositeType && hasMatchingAncestorType) { yield fixer.replaceText( referenceNode, From 4cb399a58302f85fd78a67688b17822e4640e219 Mon Sep 17 00:00:00 2001 From: Hasegawa-Yukihiro Date: Mon, 10 Mar 2025 23:08:13 +0900 Subject: [PATCH 06/10] test: add test --- .../no-unnecessary-type-parameters.test.ts | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-type-parameters.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-type-parameters.test.ts index 1322bc514d2d..aefaab0dcc14 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-type-parameters.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-type-parameters.test.ts @@ -1792,6 +1792,52 @@ function join(els: (string & number)[]) { }, { code: ` +function join(els: T[]) { + return els.map(el => '' + el).join(','); +} + `, + errors: [ + { + data: { descriptor: 'function', name: 'T', uses: 'used only once' }, + messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` +function join(els: ((string & number) | boolean)[]) { + return els.map(el => '' + el).join(','); +} + `, + }, + ], + }, + ], + }, + { + code: noFormat` +function join(els: T[]) { + return els.map(el => '' + el).join(','); +} + `, + errors: [ + { + data: { descriptor: 'function', name: 'T', uses: 'used only once' }, + messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` +function join(els: (string | number)[]) { + return els.map(el => '' + el).join(','); +} + `, + }, + ], + }, + ], + }, + { + code: ` function join(els: T['hoge'][]) { return els.map(el => '' + el).join(','); } From dd180515830952863dcc16aa360cd311522e138a Mon Sep 17 00:00:00 2001 From: Hasegawa-Yukihiro Date: Thu, 20 Mar 2025 14:28:49 +0900 Subject: [PATCH 07/10] fix: correct type precedence logic to consider broader context --- .../rules/no-unnecessary-type-parameters.ts | 9 ++-- .../no-unnecessary-type-parameters.test.ts | 52 +++++++++++++++++++ 2 files changed, 58 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts index 581088dbbe1c..257c541575b4 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts @@ -108,15 +108,18 @@ export default createRule({ for (const reference of smTypeParameterVariable.references) { if (reference.isTypeReference) { const referenceNode = reference.identifier; - const isCompositeType = + const isComplexType = constraint?.type === AST_NODE_TYPES.TSUnionType || - constraint?.type === AST_NODE_TYPES.TSIntersectionType; + constraint?.type === AST_NODE_TYPES.TSIntersectionType || + constraint?.type === AST_NODE_TYPES.TSConditionalType; const hasMatchingAncestorType = [ AST_NODE_TYPES.TSArrayType, AST_NODE_TYPES.TSIndexedAccessType, + AST_NODE_TYPES.TSIntersectionType, + AST_NODE_TYPES.TSUnionType, // eslint-disable-next-line @typescript-eslint/no-non-null-assertion ].some(type => referenceNode.parent.parent!.type === type); - if (isCompositeType && hasMatchingAncestorType) { + if (isComplexType && hasMatchingAncestorType) { yield fixer.replaceText( referenceNode, `(${constraintText})`, diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-type-parameters.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-type-parameters.test.ts index aefaab0dcc14..9f8f05479c9c 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-type-parameters.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-type-parameters.test.ts @@ -1859,5 +1859,57 @@ function join(els: ({ hoge: string } | { hoge: number })['hoge'][]) { }, ], }, + { + code: ` +type A = string; +type B = string; +type C = string; +declare function f(): T & C; + `, + errors: [ + { + data: { descriptor: 'function', name: 'T', uses: 'used only once' }, + messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` +type A = string; +type B = string; +type C = string; +declare function f(): (A | B) & C; + `, + }, + ], + }, + ], + }, + { + code: ` +type A = string; +type B = string; +type C = string; +type D = string; +declare function f(): T | null; + `, + errors: [ + { + data: { descriptor: 'function', name: 'T', uses: 'used only once' }, + messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` +type A = string; +type B = string; +type C = string; +type D = string; +declare function f(): (A extends B ? C : D) | null; + `, + }, + ], + }, + ], + }, ], }); From 8505b5464a529d81238b64c8f5b1a8877b01bc91 Mon Sep 17 00:00:00 2001 From: Hasegawa-Yukihiro Date: Thu, 20 Mar 2025 16:32:38 +0900 Subject: [PATCH 08/10] fix: use getWrappingFixer --- .../rules/no-unnecessary-type-parameters.ts | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts index 257c541575b4..7296a0fa0a97 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts @@ -10,6 +10,7 @@ import type { MakeRequired } from '../util'; import { createRule, getParserServices, + getWrappingFixer, nullThrows, NullThrowsReasons, } from '../util'; @@ -120,10 +121,18 @@ export default createRule({ // eslint-disable-next-line @typescript-eslint/no-non-null-assertion ].some(type => referenceNode.parent.parent!.type === type); if (isComplexType && hasMatchingAncestorType) { - yield fixer.replaceText( - referenceNode, - `(${constraintText})`, - ); + const fixResult = getWrappingFixer({ + node: referenceNode, + innerNode: constraint, + sourceCode: context.sourceCode, + wrap: constraintNode => constraintNode, + })(fixer); + if (fixResult == null) { + return null; + } + if ('text' in fixResult) { + yield fixResult; + } } else { yield fixer.replaceText(referenceNode, constraintText); } @@ -140,7 +149,7 @@ export default createRule({ // We are assuming at this point that the reported type parameter // is present in the inspected node's type parameters. if (typeParamsNode.params.length === 1) { - // Remove the whole generic syntax if we're removing the only type parameter in the list. + // Remove the whole generic syntax if we're le Date: Thu, 20 Mar 2025 16:34:36 +0900 Subject: [PATCH 09/10] chore: revert comment --- .../eslint-plugin/src/rules/no-unnecessary-type-parameters.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts index 7296a0fa0a97..f02c92dfd006 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts @@ -149,7 +149,7 @@ export default createRule({ // We are assuming at this point that the reported type parameter // is present in the inspected node's type parameters. if (typeParamsNode.params.length === 1) { - // Remove the whole generic syntax if we're le generic syntax if we're removing the only type parameter in the list. yield fixer.remove(typeParamsNode); } else { const index = typeParamsNode.params.indexOf(esTypeParameter); From 66a498861630f4bfffcef8d9986f75154b83ac37 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger <53019676+kirkwaiblinger@users.noreply.github.com> Date: Mon, 28 Apr 2025 14:18:23 -0600 Subject: [PATCH 10/10] address cov --- .../src/rules/no-unnecessary-type-parameters.ts | 7 +------ packages/eslint-plugin/src/util/getWrappingFixer.ts | 2 +- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts index f02c92dfd006..3a486639dff7 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts @@ -127,12 +127,7 @@ export default createRule({ sourceCode: context.sourceCode, wrap: constraintNode => constraintNode, })(fixer); - if (fixResult == null) { - return null; - } - if ('text' in fixResult) { - yield fixResult; - } + yield fixResult; } else { yield fixer.replaceText(referenceNode, constraintText); } diff --git a/packages/eslint-plugin/src/util/getWrappingFixer.ts b/packages/eslint-plugin/src/util/getWrappingFixer.ts index 26afcaa6405b..2ad20e1d4893 100644 --- a/packages/eslint-plugin/src/util/getWrappingFixer.ts +++ b/packages/eslint-plugin/src/util/getWrappingFixer.ts @@ -32,7 +32,7 @@ interface WrappingFixerParams { */ export function getWrappingFixer( params: WrappingFixerParams, -): TSESLint.ReportFixFunction { +): (fixer: TSESLint.RuleFixer) => TSESLint.RuleFix { const { node, innerNode = node, sourceCode, wrap } = params; const innerNodes = Array.isArray(innerNode) ? innerNode : [innerNode];