diff --git a/packages/eslint-plugin/docs/rules/no-unnecessary-type-parameters.mdx b/packages/eslint-plugin/docs/rules/no-unnecessary-type-parameters.mdx index c27da7616a0f..06a0a9e07163 100644 --- a/packages/eslint-plugin/docs/rules/no-unnecessary-type-parameters.mdx +++ b/packages/eslint-plugin/docs/rules/no-unnecessary-type-parameters.mdx @@ -82,10 +82,30 @@ For example, the following `T` is used multiple times by virtue of being in an ` declare function createStateHistory(): T[]; ``` -This is because the type parameter `T` relates multiple methods in the `T[]` together, making it used more than once. +This is because the type parameter `T` relates multiple methods in `T[]` (`Array`) together, making it used more than once. Therefore, this rule won't report on type parameters used as a type argument. -That includes type arguments given to global types such as `Array` (including the `T[]` shorthand and in tuples), `Map`, and `Set`. +This includes type arguments provided to global types such as `Array`, `Map`, and `Set` that have multiple methods and properties that can change values based on the type parameter. + +On the other hand, readonly and fixed array-likes such as `readonly T[]`, `ReadonlyArray`, and tuples such as `[T]` are special cases that are specifically reported on when used as input types, or as `readonly` output types. +The following example will be reported because `T` is used only once as type argument for the `ReadonlyArray` global type: + + + + +```ts +declare function length(array: ReadonlyArray): number; +``` + + + + +```ts +declare function length(array: ReadonlyArray): number; +``` + + + ## FAQ 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 3e9930128cab..42305e0a1d9b 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts @@ -239,9 +239,17 @@ function isTypeParameterRepeatedInAST( const grandparent = skipConstituentsUpward( reference.identifier.parent.parent, ); + if ( grandparent.type === AST_NODE_TYPES.TSTypeParameterInstantiation && - grandparent.params.includes(reference.identifier.parent) + grandparent.params.includes(reference.identifier.parent) && + // Array and ReadonlyArray must be handled carefully + // let's defer the check to the type-aware phase + !( + grandparent.parent.type === AST_NODE_TYPES.TSTypeReference && + grandparent.parent.typeName.type === AST_NODE_TYPES.Identifier && + ['Array', 'ReadonlyArray'].includes(grandparent.parent.typeName.name) + ) ) { return true; } @@ -281,13 +289,13 @@ function countTypeParameterUsage( if (ts.isClassLike(node)) { for (const typeParameter of node.typeParameters) { - collectTypeParameterUsageCounts(checker, typeParameter, counts); + collectTypeParameterUsageCounts(checker, typeParameter, counts, true); } for (const member of node.members) { - collectTypeParameterUsageCounts(checker, member, counts); + collectTypeParameterUsageCounts(checker, member, counts, true); } } else { - collectTypeParameterUsageCounts(checker, node, counts); + collectTypeParameterUsageCounts(checker, node, counts, false); } return counts; @@ -302,6 +310,7 @@ function collectTypeParameterUsageCounts( checker: ts.TypeChecker, node: ts.Node, foundIdentifierUsages: Map, + fromClass: boolean, // We are talking about the type parameters of a class or one of its methods ): void { const visitedSymbolLists = new Set(); const type = checker.getTypeAtLocation(node); @@ -325,6 +334,7 @@ function collectTypeParameterUsageCounts( function visitType( type: ts.Type | undefined, assumeMultipleUses: boolean, + isReturnType = false, ): void { // Seeing the same type > (threshold=3 ** 2) times indicates a likely // recursive type, like `type T = { [P in keyof T]: T }`. @@ -380,9 +390,23 @@ function collectTypeParameterUsageCounts( // Tuple types like `[K, V]` // Generic type references like `Map` - else if (tsutils.isTupleType(type) || tsutils.isTypeReference(type)) { + else if (tsutils.isTypeReference(type)) { for (const typeArgument of type.typeArguments ?? []) { - visitType(typeArgument, true); + // currently, if we are in a "class context", everything is accepted + let thisAssumeMultipleUses = fromClass || assumeMultipleUses; + + // special cases - readonly arrays/tuples are considered only to use the + // type parameter once. Mutable arrays/tuples are considered to use the + // type parameter multiple times if and only if they are returned. + // other kind of type references always count as multiple uses + thisAssumeMultipleUses ||= tsutils.isTupleType(type.target) + ? isReturnType && !type.target.readonly + : checker.isArrayType(type.target) + ? isReturnType && + (type.symbol as ts.Symbol | undefined)?.getName() === 'Array' + : true; + + visitType(typeArgument, thisAssumeMultipleUses, isReturnType); } } @@ -472,6 +496,7 @@ function collectTypeParameterUsageCounts( checker.getTypePredicateOfSignature(signature)?.type ?? signature.getReturnType(), false, + true, ); } diff --git a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unnecessary-type-parameters.shot b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unnecessary-type-parameters.shot index 7e52aaf3324d..53dc464211ba 100644 --- a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unnecessary-type-parameters.shot +++ b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unnecessary-type-parameters.shot @@ -47,3 +47,18 @@ function getProperty(obj: T, key: K) { } " `; + +exports[`Validating rule docs no-unnecessary-type-parameters.mdx code examples ESLint output 3`] = ` +"Incorrect + +declare function length(array: ReadonlyArray): number; + ~ Type parameter T is used only once in the function signature. +" +`; + +exports[`Validating rule docs no-unnecessary-type-parameters.mdx code examples ESLint output 4`] = ` +"Correct + +declare function length(array: ReadonlyArray): number; +" +`; 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 81fcdc9b43a8..9a636116cc37 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 @@ -64,13 +64,6 @@ ruleTester.run('no-unnecessary-type-parameters', rule, { } } `, - ` - class Joiner { - join(els: T[]) { - return els.map(el => '' + el).join(','); - } - } - `, ` declare class Foo { getProp(this: Record<'prop', T>): T; @@ -243,8 +236,14 @@ ruleTester.run('no-unnecessary-type-parameters', rule, { 'declare function makeSets(): [Set][];', 'declare function makeMap(): Map;', 'declare function makeMap(): [Map];', + 'declare function makeArray(): T[];', + 'declare function makeArrayNullish(): (T | null)[];', + 'declare function makeTupleMulti(): [T | null, T | null];', + 'declare function takeTupleMulti(input: [T, T]): void;', + 'declare function takeTupleMultiNullish(input: [T | null, T | null]): void;', 'declare function arrayOfPairs(): [T, T][];', 'declare function fetchJson(url: string): Promise;', + 'declare function fetchJsonTuple(url: string): Promise<[T]>;', 'declare function fn(input: T): 0 extends 0 ? T : never;', 'declare function useFocus(): [React.RefObject];', ` @@ -447,6 +446,36 @@ const f = ( }, ], }, + { + code: 'const func = (param: [T]) => null;', + errors: [ + { + data: { descriptor: 'function', name: 'T', uses: 'used only once' }, + messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: 'const func = (param: [unknown]) => null;', + }, + ], + }, + ], + }, + { + code: 'const func = (param: T[]) => null;', + errors: [ + { + data: { descriptor: 'function', name: 'T', uses: 'used only once' }, + messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: 'const func = (param: unknown[]) => null;', + }, + ], + }, + ], + }, { code: 'const f1 = (): T => {};', errors: [ @@ -1249,6 +1278,139 @@ function foo(_: unknown): (input: T) => T { }, ], }, + { + code: 'declare function makeReadonlyArray(): readonly T[];', + errors: [ + { + data: { descriptor: 'function', name: 'T', uses: 'used only once' }, + messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: + 'declare function makeReadonlyArray(): readonly unknown[];', + }, + ], + }, + ], + }, + { + code: 'declare function makeReadonlyTuple(): readonly [T];', + errors: [ + { + data: { descriptor: 'function', name: 'T', uses: 'used only once' }, + messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: + 'declare function makeReadonlyTuple(): readonly [unknown];', + }, + ], + }, + ], + }, + { + code: 'declare function makeReadonlyTupleNullish(): readonly [T | null];', + errors: [ + { + data: { descriptor: 'function', name: 'T', uses: 'used only once' }, + messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: + 'declare function makeReadonlyTupleNullish(): readonly [unknown | null];', + }, + ], + }, + ], + }, + { + code: 'declare function takeArray(input: T[]): void;', + errors: [ + { + data: { descriptor: 'function', name: 'T', uses: 'used only once' }, + messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: 'declare function takeArray(input: unknown[]): void;', + }, + ], + }, + ], + }, + { + code: 'declare function takeArrayNullish(input: (T | null)[]): void;', + errors: [ + { + data: { descriptor: 'function', name: 'T', uses: 'used only once' }, + messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: + 'declare function takeArrayNullish(input: (unknown | null)[]): void;', + }, + ], + }, + ], + }, + { + code: 'declare function takeTuple(input: [T]): void;', + errors: [ + { + data: { descriptor: 'function', name: 'T', uses: 'used only once' }, + messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: 'declare function takeTuple(input: [unknown]): void;', + }, + ], + }, + ], + }, + { + code: 'declare function takeTupleMultiUnrelated(input: [T, number]): void;', + errors: [ + { + data: { descriptor: 'function', name: 'T', uses: 'used only once' }, + messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: + 'declare function takeTupleMultiUnrelated(input: [unknown, number]): void;', + }, + ], + }, + ], + }, + { + code: ` + declare function takeTupleMultiUnrelatedNullish( + input: [T | null, null], + ): void; + `, + errors: [ + { + data: { descriptor: 'function', name: 'T', uses: 'used only once' }, + messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` + declare function takeTupleMultiUnrelatedNullish( + input: [unknown | null, null], + ): void; + `, + }, + ], + }, + ], + }, { code: 'type Fn = () => T;', errors: [ @@ -1555,5 +1717,32 @@ function f(x: unknown): void { }, ], }, + { + code: ` +class Joiner { + 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: ` +class Joiner { + join(els: number[]) { + return els.map(el => '' + el).join(','); + } +} + `, + }, + ], + }, + ], + }, ], });