From 0eb5da0de69a53e784b277d8df73876a4b9eb3d8 Mon Sep 17 00:00:00 2001 From: YeonJuan Date: Sun, 15 Dec 2024 03:39:44 +0900 Subject: [PATCH 1/3] fix(eslint-plugin): [no-unnecessary-type-arguments] handle type/value context --- .../rules/no-unnecessary-type-arguments.ts | 97 +++++++++++++++---- .../no-unnecessary-type-arguments.test.ts | 13 +++ 2 files changed, 89 insertions(+), 21 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-arguments.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-arguments.ts index a41afacf62a4..9a16bfbdf759 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-arguments.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-arguments.ts @@ -1,5 +1,6 @@ import type { TSESTree } from '@typescript-eslint/utils'; +import { AST_NODE_TYPES } from '@typescript-eslint/utils'; import * as tsutils from 'ts-api-utils'; import * as ts from 'typescript'; @@ -111,8 +112,12 @@ export default createRule<[], MessageIds>({ return { TSTypeParameterInstantiation(node): void { const expression = services.esTreeNodeToTSNodeMap.get(node); + const typeParameters = getTypeParametersFromNode( + node, + expression, + checker, + ); - const typeParameters = getTypeParametersFromNode(expression, checker); if (typeParameters) { checkTSArgsAndParameters(node, typeParameters); } @@ -122,29 +127,31 @@ export default createRule<[], MessageIds>({ }); function getTypeParametersFromNode( - node: ParameterCapableTSNode, + node: TSESTree.TSTypeParameterInstantiation, + tsNode: ParameterCapableTSNode, checker: ts.TypeChecker, ): readonly ts.TypeParameterDeclaration[] | undefined { - if (ts.isExpressionWithTypeArguments(node)) { - return getTypeParametersFromType(node.expression, checker); + if (ts.isExpressionWithTypeArguments(tsNode)) { + return getTypeParametersFromType(node, tsNode.expression, checker); } - if (ts.isTypeReferenceNode(node)) { - return getTypeParametersFromType(node.typeName, checker); + if (ts.isTypeReferenceNode(tsNode)) { + return getTypeParametersFromType(node, tsNode.typeName, checker); } if ( - ts.isCallExpression(node) || - ts.isNewExpression(node) || - ts.isTaggedTemplateExpression(node) + ts.isCallExpression(tsNode) || + ts.isNewExpression(tsNode) || + ts.isTaggedTemplateExpression(tsNode) ) { - return getTypeParametersFromCall(node, checker); + return getTypeParametersFromCall(node, tsNode, checker); } return undefined; } function getTypeParametersFromType( + node: TSESTree.TSTypeParameterInstantiation, type: ts.ClassDeclaration | ts.EntityName | ts.Expression, checker: ts.TypeChecker, ): readonly ts.TypeParameterDeclaration[] | undefined { @@ -160,24 +167,33 @@ function getTypeParametersFromType( return undefined; } - return findFirstResult(declarations, decl => - ts.isClassLike(decl) || - ts.isTypeAliasDeclaration(decl) || - ts.isInterfaceDeclaration(decl) - ? decl.typeParameters - : undefined, - ); + const sortedDeclaraions = getSortedDeclarations(node, declarations); + return findFirstResult(sortedDeclaraions, decl => { + if ( + ts.isTypeAliasDeclaration(decl) || + ts.isInterfaceDeclaration(decl) || + ts.isClassLike(decl) + ) { + return decl.typeParameters; + } + if (ts.isVariableDeclaration(decl)) { + return getConstructSignatureDeclaration(symAtLocation, checker) + ?.typeParameters; + } + return undefined; + }); } function getTypeParametersFromCall( - node: ts.CallExpression | ts.NewExpression | ts.TaggedTemplateExpression, + node: TSESTree.TSTypeParameterInstantiation, + tsNode: ts.CallExpression | ts.NewExpression | ts.TaggedTemplateExpression, checker: ts.TypeChecker, ): readonly ts.TypeParameterDeclaration[] | undefined { - const sig = checker.getResolvedSignature(node); + const sig = checker.getResolvedSignature(tsNode); const sigDecl = sig?.getDeclaration(); if (!sigDecl) { - return ts.isNewExpression(node) - ? getTypeParametersFromType(node.expression, checker) + return ts.isNewExpression(tsNode) + ? getTypeParametersFromType(node, tsNode.expression, checker) : undefined; } @@ -192,3 +208,42 @@ function getAliasedSymbol( ? checker.getAliasedSymbol(symbol) : symbol; } + +function isInTypeContext(node: TSESTree.TSTypeParameterInstantiation) { + return ( + node.parent.type === AST_NODE_TYPES.TSInterfaceHeritage || + node.parent.type === AST_NODE_TYPES.TSTypeReference || + node.parent.type === AST_NODE_TYPES.TSClassImplements + ); +} + +function isTypeContextDeclaration(decl: ts.Declaration) { + return ts.isTypeAliasDeclaration(decl) || ts.isInterfaceDeclaration(decl); +} + +function typeFirstCompare(declA: ts.Declaration, declB: ts.Declaration) { + const aIsType = isTypeContextDeclaration(declA); + const bIsType = isTypeContextDeclaration(declB); + + return Number(bIsType) - Number(aIsType); +} + +function getSortedDeclarations( + node: TSESTree.TSTypeParameterInstantiation, + declarations: ts.Declaration[], +) { + const sorted = [...declarations].sort(typeFirstCompare); + if (isInTypeContext(node)) { + return sorted; + } + return sorted.reverse(); +} + +function getConstructSignatureDeclaration( + symbol: ts.Symbol, + checker: ts.TypeChecker, +): ts.SignatureDeclaration | undefined { + const type = checker.getTypeOfSymbol(symbol); + const sig = type.getConstructSignatures(); + return sig.at(0)?.getDeclaration(); +} diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-type-arguments.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-type-arguments.test.ts index 28ba6e955dd3..219f49a206f6 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-type-arguments.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-type-arguments.test.ts @@ -155,6 +155,19 @@ type A = Map; type B = T; type C2 = B>; `, + ` +interface Foo {} +declare var Foo: { + new (type: string): any; +}; + +class Bar extends Foo {} + `, + ` +interface Foo {} +class Foo {} +class Bar extends Foo {} + `, ], invalid: [ { From d2f81092c07882d506fc90e68cb1a81d59f3dadf Mon Sep 17 00:00:00 2001 From: YeonJuan Date: Sun, 15 Dec 2024 22:09:55 +0900 Subject: [PATCH 2/3] add tests --- .../no-unnecessary-type-arguments.test.ts | 107 +++++++++++++++++- 1 file changed, 101 insertions(+), 6 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-type-arguments.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-type-arguments.test.ts index 219f49a206f6..ad4133e13ff4 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-type-arguments.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-type-arguments.test.ts @@ -156,17 +156,28 @@ type B = T; type C2 = B>; `, ` -interface Foo {} +interface Foo {} declare var Foo: { - new (type: string): any; + new (type: T): any; }; - -class Bar extends Foo {} +class Bar extends Foo {} + `, + ` +interface Foo {} +class Foo {} +class Bar extends Foo {} + `, + ` +class Foo {} +interface Foo {} +class Bar implements Foo {} `, ` -interface Foo {} class Foo {} -class Bar extends Foo {} +namespace Foo { + export class Bar {} +} +class Bar extends Foo {} `, ], invalid: [ @@ -465,5 +476,89 @@ type E = T; type F = E; `, }, + { + code: ` +interface Foo {} +declare var Foo: { + new (type: T): any; +}; +class Bar extends Foo {} + `, + errors: [ + { + line: 6, + messageId: 'unnecessaryTypeParameter', + }, + ], + output: ` +interface Foo {} +declare var Foo: { + new (type: T): any; +}; +class Bar extends Foo {} + `, + }, + { + code: ` +declare var Foo: { + new (type: T): any; +}; +interface Foo {} +class Bar extends Foo {} + `, + errors: [ + { + line: 6, + messageId: 'unnecessaryTypeParameter', + }, + ], + output: ` +declare var Foo: { + new (type: T): any; +}; +interface Foo {} +class Bar extends Foo {} + `, + }, + { + code: ` +class Foo {} +interface Foo {} +class Bar implements Foo {} + `, + errors: [ + { + line: 4, + messageId: 'unnecessaryTypeParameter', + }, + ], + output: ` +class Foo {} +interface Foo {} +class Bar implements Foo {} + `, + }, + { + code: ` +class Foo {} +namespace Foo { + export class Bar {} +} +class Bar extends Foo {} + `, + errors: [ + { + line: 6, + messageId: 'unnecessaryTypeParameter', + }, + ], + output: ` +class Foo {} +namespace Foo { + export class Bar {} +} +class Bar extends Foo {} + `, + }, ], }); From 5f5e42aeb2bd271f8800367b29d00190fbe3323c Mon Sep 17 00:00:00 2001 From: YeonJuan Date: Sun, 15 Dec 2024 22:23:13 +0900 Subject: [PATCH 3/3] refactor --- .../src/rules/no-unnecessary-type-arguments.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-arguments.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-arguments.ts index 9a16bfbdf759..95f0e66ccecc 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-arguments.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-arguments.ts @@ -167,7 +167,10 @@ function getTypeParametersFromType( return undefined; } - const sortedDeclaraions = getSortedDeclarations(node, declarations); + const sortedDeclaraions = sortDeclarationsByTypeValueContext( + node, + declarations, + ); return findFirstResult(sortedDeclaraions, decl => { if ( ts.isTypeAliasDeclaration(decl) || @@ -228,7 +231,7 @@ function typeFirstCompare(declA: ts.Declaration, declB: ts.Declaration) { return Number(bIsType) - Number(aIsType); } -function getSortedDeclarations( +function sortDeclarationsByTypeValueContext( node: TSESTree.TSTypeParameterInstantiation, declarations: ts.Declaration[], ) {