diff --git a/eslint.config.mjs b/eslint.config.mjs index b4d50a9435af..aa0565d5e459 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -128,6 +128,7 @@ export default tseslint.config( 'error', { allowConstantLoopConditions: true }, ], + '@typescript-eslint/no-unnecessary-type-parameters': 'error', '@typescript-eslint/no-var-requires': 'off', '@typescript-eslint/prefer-literal-enum-member': [ 'error', diff --git a/packages/eslint-plugin/docs/rules/no-unnecessary-type-parameters.mdx b/packages/eslint-plugin/docs/rules/no-unnecessary-type-parameters.mdx new file mode 100644 index 000000000000..130c40b1ce04 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-unnecessary-type-parameters.mdx @@ -0,0 +1,115 @@ +--- +description: 'Disallow type parameters that only appear once.' +--- + +import Tabs from '@theme/Tabs'; +import TabItem from '@theme/TabItem'; + +> 🛑 This file is source code, not the primary documentation location! 🛑 +> +> See **https://typescript-eslint.io/rules/no-unnecessary-type-parameters** for documentation. + +This rule forbids type parameters that only appear once in a function, method, or class definition. + +Type parameters relate two types. +If a type parameter only appears once, then it is not relating anything. +It can usually be replaced with explicit types such as `unknown`. + +At best unnecessary type parameters make code harder to read. +At worst they can be used to disguise unsafe type assertions. + +:::warning Early Stage +This rule was recently added to typescript-eslint and still considered experimental. +It might change significantly between minor versions. +Please try it out and give us feedback! +::: + +## Examples + + + + +```ts +function second(a: A, b: B): B { + return b; +} + +function parseJSON(input: string): T { + return JSON.parse(input); +} + +function printProperty(obj: T, key: K) { + console.log(obj[key]); +} +``` + + + + +```ts +function second(a: unknown, b: B): B { + return b; +} + +function parseJSON(input: string): unknown { + return JSON.parse(input); +} + +function printProperty(obj: T, key: keyof T) { + console.log(obj[key]); +} + +// T appears twice: in the type of arg and as the return type +function identity(arg: T): T { + return arg; +} + +// T appears twice: "keyof T" and in the inferred return type (T[K]). +// K appears twice: "key: K" and in the inferred return type (T[K]). +function getProperty(obj: T, key: K) { + return obj[key]; +} +``` + + + + +## Limitations + +Note that this rule allows any type parameter that is used multiple times, even if those uses are via a type argument. +For example, the following `T` is used multiple times by virtue of being in an `Array`, even though its name only appears once after declaration: + +```ts +declare function createStateHistory(): T[]; +``` + +This is because the type parameter `T` relates multiple methods in the `T[]` 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`. + +## When Not To Use It + +This rule will report on functions that use type parameters solely to test types, for example: + +```ts +function assertType(arg: T) {} + +assertType(123); +assertType('abc'); +// ~~~~~ +// Argument of type 'string' is not assignable to parameter of type 'number'. +``` + +If you're using this pattern then you'll want to disable this rule on files that test types. + +## Further Reading + +- TypeScript handbook: [Type Parameters Should Appear Twice](https://microsoft.github.io/TypeScript-New-Handbook/everything/#type-parameters-should-appear-twice) +- Effective TypeScript: [The Golden Rule of Generics](https://effectivetypescript.com/2020/08/12/generics-golden-rule/) + +## Related To + +- eslint-plugin-etc's [`no-misused-generics`](https://github.com/cartant/eslint-plugin-etc/blob/main/docs/rules/no-misused-generics.md) +- wotan's [`no-misused-generics`](https://github.com/fimbullinter/wotan/blob/master/packages/mimir/docs/no-misused-generics.md) +- DefinitelyTyped-tools' [`no-unnecessary-generics`](https://github.com/microsoft/DefinitelyTyped-tools/blob/main/packages/eslint-plugin/docs/rules/no-unnecessary-generics.md) diff --git a/packages/eslint-plugin/src/configs/all.ts b/packages/eslint-plugin/src/configs/all.ts index ab24d6943de0..02fff1af4da7 100644 --- a/packages/eslint-plugin/src/configs/all.ts +++ b/packages/eslint-plugin/src/configs/all.ts @@ -95,6 +95,7 @@ export = { '@typescript-eslint/no-unnecessary-type-arguments': 'error', '@typescript-eslint/no-unnecessary-type-assertion': 'error', '@typescript-eslint/no-unnecessary-type-constraint': 'error', + '@typescript-eslint/no-unnecessary-type-parameters': 'error', '@typescript-eslint/no-unsafe-argument': 'error', '@typescript-eslint/no-unsafe-assignment': 'error', '@typescript-eslint/no-unsafe-call': 'error', diff --git a/packages/eslint-plugin/src/configs/disable-type-checked.ts b/packages/eslint-plugin/src/configs/disable-type-checked.ts index 9a45d83452cf..9ea7a2f3f296 100644 --- a/packages/eslint-plugin/src/configs/disable-type-checked.ts +++ b/packages/eslint-plugin/src/configs/disable-type-checked.ts @@ -32,6 +32,7 @@ export = { '@typescript-eslint/no-unnecessary-template-expression': 'off', '@typescript-eslint/no-unnecessary-type-arguments': 'off', '@typescript-eslint/no-unnecessary-type-assertion': 'off', + '@typescript-eslint/no-unnecessary-type-parameters': 'off', '@typescript-eslint/no-unsafe-argument': 'off', '@typescript-eslint/no-unsafe-assignment': 'off', '@typescript-eslint/no-unsafe-call': 'off', diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index d11366a54341..de2fb52ca811 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -72,6 +72,7 @@ import noUnnecessaryTemplateExpression from './no-unnecessary-template-expressio import noUnnecessaryTypeArguments from './no-unnecessary-type-arguments'; import noUnnecessaryTypeAssertion from './no-unnecessary-type-assertion'; import noUnnecessaryTypeConstraint from './no-unnecessary-type-constraint'; +import noUnnecessaryTypeParameters from './no-unnecessary-type-parameters'; import noUnsafeArgument from './no-unsafe-argument'; import noUnsafeAssignment from './no-unsafe-assignment'; import noUnsafeCall from './no-unsafe-call'; @@ -198,6 +199,7 @@ export default { 'no-unnecessary-type-arguments': noUnnecessaryTypeArguments, 'no-unnecessary-type-assertion': noUnnecessaryTypeAssertion, 'no-unnecessary-type-constraint': noUnnecessaryTypeConstraint, + 'no-unnecessary-type-parameters': noUnnecessaryTypeParameters, 'no-unsafe-argument': noUnsafeArgument, 'no-unsafe-assignment': noUnsafeAssignment, 'no-unsafe-call': noUnsafeCall, diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts new file mode 100644 index 000000000000..0f78a6d14ea6 --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts @@ -0,0 +1,387 @@ +import type { Reference } from '@typescript-eslint/scope-manager'; +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'; + +import type { MakeRequired } from '../util'; +import { createRule, getParserServices } from '../util'; + +type NodeWithTypeParameters = MakeRequired< + ts.SignatureDeclaration | ts.ClassLikeDeclaration, + 'typeParameters' +>; + +export default createRule({ + defaultOptions: [], + meta: { + docs: { + description: 'Disallow type parameters that only appear once', + requiresTypeChecking: true, + }, + messages: { + sole: 'Type parameter {{name}} is used only once.', + }, + schema: [], + type: 'problem', + }, + name: 'no-unnecessary-type-parameters', + create(context) { + const parserServices = getParserServices(context); + return { + [[ + 'ArrowFunctionExpression[typeParameters]', + 'ClassDeclaration[typeParameters]', + 'ClassExpression[typeParameters]', + 'FunctionDeclaration[typeParameters]', + 'FunctionExpression[typeParameters]', + 'TSCallSignatureDeclaration[typeParameters]', + 'TSConstructorType[typeParameters]', + 'TSDeclareFunction[typeParameters]', + 'TSEmptyBodyFunctionExpression[typeParameters]', + 'TSFunctionType[typeParameters]', + 'TSMethodSignature[typeParameters]', + ].join(', ')](node: TSESTree.FunctionLike): void { + const tsNode = parserServices.esTreeNodeToTSNodeMap.get( + node, + ) as NodeWithTypeParameters; + + const checker = parserServices.program.getTypeChecker(); + let counts: Map | undefined; + + for (const typeParameter of tsNode.typeParameters) { + const esTypeParameter = + parserServices.tsNodeToESTreeNodeMap.get( + typeParameter, + ); + const scope = context.sourceCode.getScope(esTypeParameter); + + // Quick path: if the type parameter is used multiple times in the AST, + // we don't need to dip into types to know it's repeated. + if (isTypeParameterRepeatedInAST(esTypeParameter, scope.references)) { + continue; + } + + // For any inferred types, we have to dip into type checking. + counts ??= countTypeParameterUsage(checker, tsNode); + const identifierCounts = counts.get(typeParameter.name); + if (!identifierCounts || identifierCounts > 2) { + continue; + } + + context.report({ + data: { + name: typeParameter.name.text, + }, + node: esTypeParameter, + messageId: 'sole', + }); + } + }, + }; + }, +}); + +function isTypeParameterRepeatedInAST( + node: TSESTree.TSTypeParameter, + references: Reference[], +): boolean { + let total = 0; + + for (const reference of references) { + // References inside the type parameter's definition don't count. + if ( + reference.identifier.range[0] < node.range[1] && + reference.identifier.range[1] > node.range[0] + ) { + continue; + } + + // Neither do references that aren't to the same type parameter, + // namely value-land (non-type) identifiers of the type parameter's type, + // and references to different type parameters or values. + if ( + !reference.isTypeReference || + reference.identifier.name !== node.name.name + ) { + continue; + } + + // If the type parameter is being used as a type argument, then we + // know the type parameter is being reused and can't be reported. + if (reference.identifier.parent.type === AST_NODE_TYPES.TSTypeReference) { + const grandparent = skipConstituentsUpward( + reference.identifier.parent.parent, + ); + if ( + grandparent.type === AST_NODE_TYPES.TSTypeParameterInstantiation && + grandparent.params.includes(reference.identifier.parent) + ) { + return true; + } + } + + total += 1; + + if (total > 2) { + return true; + } + } + + return false; +} + +function skipConstituentsUpward(node: TSESTree.Node): TSESTree.Node { + switch (node.type) { + case AST_NODE_TYPES.TSIntersectionType: + case AST_NODE_TYPES.TSUnionType: + return skipConstituentsUpward(node.parent); + default: + return node; + } +} + +/** + * Count uses of type parameters in inferred return types. + * We need to resolve and analyze the inferred return type of a function + * to see whether it contains additional references to the type parameters. + * For classes, we need to do this for all their methods. + */ +function countTypeParameterUsage( + checker: ts.TypeChecker, + node: NodeWithTypeParameters, +): Map { + const counts = new Map(); + + if (ts.isClassLike(node)) { + for (const typeParameter of node.typeParameters) { + collectTypeParameterUsageCounts(checker, typeParameter, counts); + } + for (const member of node.members) { + collectTypeParameterUsageCounts(checker, member, counts); + } + } else { + collectTypeParameterUsageCounts(checker, node, counts); + } + + return counts; +} + +/** + * Populates {@link foundIdentifierUsages} by the number of times each type parameter + * appears in the given type by checking its uses through its type references. + * This is essentially a limited subset of the scope manager, but for types. + */ +function collectTypeParameterUsageCounts( + checker: ts.TypeChecker, + node: ts.Node, + foundIdentifierUsages: Map, +): void { + const visitedSymbolLists = new Set(); + const type = checker.getTypeAtLocation(node); + const typeUsages = new Map(); + const visitedConstraints = new Set(); + let functionLikeType = false; + let visitedDefault = false; + + if ( + ts.isCallSignatureDeclaration(node) || + ts.isConstructorDeclaration(node) + ) { + functionLikeType = true; + visitSignature(checker.getSignatureFromDeclaration(node)); + } + + if (!functionLikeType) { + visitType(type, false); + } + + function visitType( + type: ts.Type | undefined, + assumeMultipleUses: boolean, + ): void { + // Seeing the same type > (threshold=3 ** 2) times indicates a likely + // recursive type, like `type T = { [P in keyof T]: T }`. + // If it's not recursive, then heck, we've seen it enough times that any + // referenced types have been counted enough to qualify as used. + if (!type || incrementTypeUsages(type) > 9) { + return; + } + + // https://github.com/JoshuaKGoldberg/ts-api-utils/issues/382 + if ((tsutils.isTypeParameter as (type: ts.Type) => boolean)(type)) { + const declaration = type.getSymbol()?.getDeclarations()?.[0] as + | ts.TypeParameterDeclaration + | undefined; + + if (declaration) { + incrementIdentifierCount(declaration.name, assumeMultipleUses); + + // Visiting the type of a constrained type parameter will recurse into + // the constraint. We avoid infinite loops by visiting each only once. + if ( + declaration.constraint && + !visitedConstraints.has(declaration.constraint) + ) { + visitedConstraints.add(declaration.constraint); + visitType(checker.getTypeAtLocation(declaration.constraint), false); + } + + if (declaration.default && !visitedDefault) { + visitedDefault = true; + visitType(checker.getTypeAtLocation(declaration.default), false); + } + } + } + + // Intersections and unions like `0 | 1` + else if (tsutils.isUnionOrIntersectionType(type)) { + visitTypesList(type.types, assumeMultipleUses); + } + + // Index access types like `T[K]` + else if (tsutils.isIndexedAccessType(type)) { + visitType(type.objectType, assumeMultipleUses); + visitType(type.indexType, assumeMultipleUses); + } + + // Tuple types like `[K, V]` + // Generic type references like `Map` + else if (tsutils.isTupleType(type) || tsutils.isTypeReference(type)) { + for (const typeArgument of type.typeArguments ?? []) { + visitType(typeArgument, true); + } + } + + // Template literals like `a${T}b` + else if (tsutils.isTemplateLiteralType(type)) { + for (const subType of type.types) { + visitType(subType, assumeMultipleUses); + } + } + + // Conditional types like `T extends string ? T : never` + else if (tsutils.isConditionalType(type)) { + visitType(type.checkType, assumeMultipleUses); + visitType(type.extendsType, assumeMultipleUses); + } + + // Catch-all: inferred object types like `{ K: V }`. + // These catch-alls should be _after_ more specific checks like + // `isTypeReference` to avoid descending into all the properties of a + // generic interface/class, e.g. `Map`. + else if (tsutils.isObjectType(type)) { + visitSymbolsListOnce(type.getProperties(), false); + + if (isMappedType(type)) { + visitType(type.typeParameter, false); + } + + for (const typeArgument of type.aliasTypeArguments ?? []) { + visitType(typeArgument, true); + } + + visitType(type.getNumberIndexType(), true); + visitType(type.getStringIndexType(), true); + + type.getCallSignatures().forEach(signature => { + functionLikeType = true; + visitSignature(signature); + }); + + type.getConstructSignatures().forEach(signature => { + functionLikeType = true; + visitSignature(signature); + }); + } + + // Catch-all: operator types like `keyof T` + else if (isOperatorType(type)) { + visitType(type.type, assumeMultipleUses); + } + + // Catch-all: generic type references like `Exclude` + else if (type.aliasTypeArguments) { + visitTypesList(type.aliasTypeArguments, true); + } + } + + function incrementIdentifierCount( + id: ts.Identifier, + assumeMultipleUses: boolean, + ): void { + const identifierCount = foundIdentifierUsages.get(id) ?? 0; + const value = assumeMultipleUses ? 2 : 1; + foundIdentifierUsages.set(id, identifierCount + value); + } + + function incrementTypeUsages(type: ts.Type): number { + const count = (typeUsages.get(type) ?? 0) + 1; + typeUsages.set(type, count); + return count; + } + + function visitSignature(signature: ts.Signature | undefined): void { + if (!signature) { + return; + } + + if (signature.thisParameter) { + visitType(checker.getTypeOfSymbol(signature.thisParameter), false); + } + + for (const parameter of signature.parameters) { + visitType(checker.getTypeOfSymbol(parameter), false); + } + + for (const typeParameter of signature.getTypeParameters() ?? []) { + visitType(typeParameter, false); + } + + visitType( + checker.getTypePredicateOfSignature(signature)?.type ?? + signature.getReturnType(), + false, + ); + } + + function visitSymbolsListOnce( + symbols: ts.Symbol[], + assumeMultipleUses: boolean, + ): void { + if (visitedSymbolLists.has(symbols)) { + return; + } + + visitedSymbolLists.add(symbols); + + for (const symbol of symbols) { + visitType(checker.getTypeOfSymbol(symbol), assumeMultipleUses); + } + } + + function visitTypesList( + types: readonly ts.Type[], + assumeMultipleUses: boolean, + ): void { + for (const type of types) { + visitType(type, assumeMultipleUses); + } + } +} + +interface MappedType extends ts.ObjectType { + typeParameter?: ts.Type; +} + +function isMappedType(type: ts.Type): type is MappedType { + return 'typeParameter' in type; +} + +interface OperatorType extends ts.Type { + type: ts.Type; +} + +function isOperatorType(type: ts.Type): type is OperatorType { + return 'type' in type && !!type.type; +} diff --git a/packages/eslint-plugin/src/util/collectUnusedVariables.ts b/packages/eslint-plugin/src/util/collectUnusedVariables.ts index b00b8ff048c9..0993a44cec96 100644 --- a/packages/eslint-plugin/src/util/collectUnusedVariables.ts +++ b/packages/eslint-plugin/src/util/collectUnusedVariables.ts @@ -115,9 +115,7 @@ class UnusedVarsVisitor extends Visitor { //#region HELPERS - private getScope( - currentNode: TSESTree.Node, - ): T { + private getScope(currentNode: TSESTree.Node): TSESLint.Scope.Scope { // On Program node, get the outermost scope to avoid return Node.js special function scope or ES modules scope. const inner = currentNode.type !== AST_NODE_TYPES.Program; @@ -127,15 +125,15 @@ class UnusedVarsVisitor extends Visitor { if (scope) { if (scope.type === ScopeType.functionExpressionName) { - return scope.childScopes[0] as T; + return scope.childScopes[0]; } - return scope as T; + return scope; } node = node.parent; } - return this.#scopeManager.scopes[0] as T; + return this.#scopeManager.scopes[0]; } private markVariableAsUsed( @@ -184,7 +182,7 @@ class UnusedVarsVisitor extends Visitor { node: TSESTree.ClassDeclaration | TSESTree.ClassExpression, ): void { // skip a variable of class itself name in the class scope - const scope = this.getScope(node); + const scope = this.getScope(node) as TSESLint.Scope.Scopes.ClassScope; for (const variable of scope.variables) { if (variable.identifiers[0] === scope.block.id) { this.markVariableAsUsed(variable); 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 new file mode 100644 index 000000000000..ff3a00a42b6e --- /dev/null +++ b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unnecessary-type-parameters.shot @@ -0,0 +1,49 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Validating rule docs no-unnecessary-type-parameters.mdx code examples ESLint output 1`] = ` +"Incorrect + +function second(a: A, b: B): B { + ~ Type parameter A is used only once. + return b; +} + +function parseJSON(input: string): T { + ~ Type parameter T is used only once. + return JSON.parse(input); +} + +function printProperty(obj: T, key: K) { + ~~~~~~~~~~~~~~~~~ Type parameter K is used only once. + console.log(obj[key]); +} +" +`; + +exports[`Validating rule docs no-unnecessary-type-parameters.mdx code examples ESLint output 2`] = ` +"Correct + +function second(a: unknown, b: B): B { + return b; +} + +function parseJSON(input: string): unknown { + return JSON.parse(input); +} + +function printProperty(obj: T, key: keyof T) { + console.log(obj[key]); +} + +// T appears twice: in the type of arg and as the return type +function identity(arg: T): T { + return arg; +} + +// T appears twice: "keyof T" and in the inferred return type (T[K]). +// K appears twice: "key: K" and in the inferred return type (T[K]). +function getProperty(obj: T, key: K) { + return obj[key]; +} +" +`; 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 new file mode 100644 index 000000000000..f1d98ea5734e --- /dev/null +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-type-parameters.test.ts @@ -0,0 +1,612 @@ +import { RuleTester } from '@typescript-eslint/rule-tester'; + +import rule from '../../src/rules/no-unnecessary-type-parameters'; +import { getFixturesRootDir } from '../RuleTester'; + +const rootPath = getFixturesRootDir(); + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', + parserOptions: { + sourceType: 'module', + tsconfigRootDir: rootPath, + project: './tsconfig.json', + }, +}); + +ruleTester.run('no-unnecessary-type-parameters', rule, { + valid: [ + ` + class ClassyArray { + arr: T[]; + } + `, + ` + class ClassyArray { + value1: T; + value2: T; + } + `, + ` + class ClassyArray { + arr: T[]; + constructor(arr: T[]) { + this.arr = arr; + } + } + `, + ` + class ClassyArray { + arr: T[]; + workWith(value: T) { + this.arr.indexOf(value); + } + } + `, + ` + abstract class ClassyArray { + arr: T[]; + abstract workWith(value: T): void; + } + `, + ` + class Box { + val: T | null = null; + get() { + return this.val; + } + } + `, + ` + class Joiner { + join(els: T[]) { + return els.map(el => '' + el).join(','); + } + } + `, + ` + class Joiner { + join(els: T[]) { + return els.map(el => '' + el).join(','); + } + } + `, + ` + declare class Foo { + getProp(this: Record<'prop', T>): T; + } + `, + 'type Fn = (input: T) => T;', + 'type Fn = (input: T) => T;', + 'type Fn = (input: T) => `a${T}b`;', + 'type Fn = new (input: T) => T;', + 'type Fn = (input: T) => typeof input;', + 'type Fn = (input: T) => keyof typeof input;', + 'type Fn = (input: Partial) => typeof input;', + 'type Fn = (input: Partial) => input is T;', + 'type Fn = (input: T) => { [K in keyof T]: K };', + 'type Fn = (input: T) => { [K in keyof T as K]: string };', + 'type Fn = (input: T) => { [K in keyof T as `${K & string}`]: string };', + 'type Fn = (input: T) => Partial;', + 'type Fn = (input: { [i: number]: T }) => T;', + 'type Fn = (input: { [i: number]: T }) => Partial;', + 'type Fn = (input: { [i: string]: T }) => Partial;', + 'type Fn = (input: T) => { [i: number]: T };', + 'type Fn = (input: T) => { [i: string]: T };', + "type Fn = (input: T) => Omit;", + ` + interface I { + (value: T): T; + } + `, + ` + interface I { + new (value: T): T; + } + `, + ` + function identity(arg: T): T { + return arg; + } + `, + ` + function printProperty(obj: T, key: keyof T) { + console.log(obj[key]); + } + `, + ` + function getProperty(obj: T, key: K) { + return obj[key]; + } + `, + ` + function box(val: T) { + return { val }; + } + `, + ` + function doStuff(map: Map, key: K) { + let v = map.get(key); + v = 1; + map.set(key, v); + return v; + } + `, + ` + function makeMap() { + return new Map(); + } + `, + ` + function makeMap(ks: K[], vs: V[]) { + const r = new Map(); + ks.forEach((k, i) => { + r.set(k, vs[i]); + }); + return r; + } + `, + ` + function arrayOfPairs() { + return [] as [T, T][]; + } + `, + ` + function isNonNull(v: T): v is Exclude { + return v !== null; + } + `, + ` + function both( + fn1: (...args: Args) => void, + fn2: (...args: Args) => void, + ): (...args: Args) => void { + return function (...args: Args) { + fn1(...args); + fn2(...args); + }; + } + `, + ` + function lengthyIdentity(x: T) { + return x; + } + `, + ` + interface Lengthy { + length: number; + } + function lengthyIdentity(x: T) { + return x; + } + `, + ` + function ItemComponent(props: { item: T; onSelect: (item: T) => void }) {} + `, + ` + interface ItemProps { + item: readonly T; + onSelect: (item: T) => void; + } + function ItemComponent(props: ItemProps) {} + `, + ` + function useFocus(): [ + React.RefObject, + () => void, + ]; + `, + ` + function findFirstResult( + inputs: unknown[], + getResult: (t: unknown) => U | undefined, + ): U | undefined; + `, + ` + function findFirstResult( + inputs: T[], + getResult: (t: T) => () => [U | undefined], + ): () => [U | undefined]; + `, + ` + function getData(url: string): Promise { + return Promise.resolve(null); + } + `, + ` + function getData(url: string): Promise { + return Promise.resolve(null); + } + `, + ` + function getData(url: string): Promise<\`a\${T}b\`> { + return Promise.resolve(null); + } + `, + ` + async function getData(url: string): Promise { + return null; + } + `, + 'declare function get(): void;', + 'declare function get(param: T[]): T;', + 'declare function box(val: T): { val: T };', + 'declare function identity(param: T): T;', + 'declare function compare(param1: T, param2: T): boolean;', + 'declare function example(a: Set): T;', + 'declare function example(a: Set, b: T[]): void;', + 'declare function example(a: Map): void;', + 'declare function example(t: T, u: U): U;', + 'declare function makeSet(): Set;', + 'declare function makeSet(): [Set];', + 'declare function makeSets(): Set[];', + 'declare function makeSets(): [Set][];', + 'declare function makeMap(): Map;', + 'declare function makeMap(): [Map];', + 'declare function arrayOfPairs(): [T, T][];', + 'declare function fetchJson(url: string): Promise;', + 'declare function fn(input: T): 0 extends 0 ? T : never;', + 'declare function useFocus(): [React.RefObject];', + ` + declare function useFocus(): { + ref: React.RefObject; + }; + `, + ` + interface TwoMethods { + a(x: T): void; + b(x: T): void; + } + + declare function two(props: TwoMethods): void; + `, + ` + type Obj = { a: string }; + + declare function hasOwnProperty( + obj: Obj, + key: K, + ): obj is Obj & { [key in K]-?: Obj[key] }; + `, + ` + type AsMutable = { + -readonly [Key in keyof T]: T[Key]; + }; + + declare function makeMutable(input: T): MakeMutable; + `, + ` + type AsMutable = { + -readonly [Key in keyof T]: T[Key]; + }; + + declare function makeMutable(input: T): MakeMutable; + `, + ` + type ValueNulls = {} & { + [P in U]: null; + }; + + declare function invert(obj: T): ValueNulls; + `, + ` + interface Middle { + inner: boolean; + } + + type Conditional = {} & (T['inner'] extends true ? {} : {}); + + function withMiddle(options: T): Conditional { + return options; + } + `, + ` + import * as ts from 'typescript'; + + declare function forEachReturnStatement( + body: ts.Block, + visitor: (stmt: ts.ReturnStatement) => T, + ): T | undefined; + `, + ` + import type { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/types'; + + declare const isNodeOfType: ( + nodeType: NodeType, + ) => node is Extract; + `, + ` + import type { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/types'; + + const isNodeOfType = + (nodeType: NodeType) => + ( + node: TSESTree.Node | null, + ): node is Extract => + node?.type === nodeType; + `, + ` + import type { AST_TOKEN_TYPES, TSESTree } from '@typescript-eslint/types'; + + export const isNotTokenOfTypeWithConditions = + < + TokenType extends AST_TOKEN_TYPES, + ExtractedToken extends Extract, + Conditions extends Partial, + >( + tokenType: TokenType, + conditions: Conditions, + ): (( + token: TSESTree.Token | null | undefined, + ) => token is Exclude) => + (token): token is Exclude => + tokenType in conditions; + `, + ], + + invalid: [ + { + code: 'const func = (param: T) => null;', + errors: [{ messageId: 'sole', data: { name: 'T' } }], + }, + { + code: 'const f1 = (): T => {};', + errors: [{ messageId: 'sole', data: { name: 'T' } }], + }, + { + code: ` + interface I { + (value: T): void; + } + `, + errors: [{ messageId: 'sole', data: { name: 'T' } }], + }, + { + code: ` + interface I { + m(x: T): void; + } + `, + errors: [{ messageId: 'sole' }], + }, + { + code: ` + class Joiner { + join(el: T, other: string) { + return [el, other].join(','); + } + } + `, + errors: [{ messageId: 'sole', data: { name: 'T' } }], + }, + { + code: ` + declare class C {} + `, + errors: [{ messageId: 'sole', data: { name: 'V' } }], + }, + { + code: ` + declare class C { + method(param: T): U; + } + `, + errors: [ + { messageId: 'sole', data: { name: 'T' } }, + { messageId: 'sole', data: { name: 'U' } }, + ], + }, + { + code: ` + declare class C { + method(param: T): U; + } + `, + errors: [ + { messageId: 'sole', data: { name: 'T' } }, + { messageId: 'sole', data: { name: 'U' } }, + ], + }, + { + code: ` + declare class C { + prop:

() => P; + } + `, + errors: [{ messageId: 'sole', data: { name: 'P' } }], + }, + { + code: ` + declare class Foo { + foo(this: T): void; + } + `, + errors: [ + { + messageId: 'sole', + data: { name: 'T' }, + }, + ], + }, + { + code: ` + function third(a: A, b: B, c: C): C { + return c; + } + `, + errors: [ + { messageId: 'sole', data: { name: 'A' } }, + { messageId: 'sole', data: { name: 'B' } }, + ], + }, + { + code: ` + function parseYAML(input: string): T { + return input as any as T; + } + `, + errors: [{ messageId: 'sole', data: { name: 'T' } }], + }, + { + code: ` + function printProperty(obj: T, key: K) { + console.log(obj[key]); + } + `, + errors: [{ messageId: 'sole', data: { name: 'K' } }], + }, + { + code: ` + function fn(param: string) { + let v: T = null!; + return v; + } + `, + errors: [ + { + data: { name: 'T' }, + messageId: 'sole', + }, + ], + }, + { + code: ` + function both< + Args extends unknown[], + CB1 extends (...args: Args) => void, + CB2 extends (...args: Args) => void, + >(fn1: CB1, fn2: CB2): (...args: Args) => void { + return function (...args: Args) { + fn1(...args); + fn2(...args); + }; + } + `, + errors: [ + { messageId: 'sole', data: { name: 'CB1' } }, + { messageId: 'sole', data: { name: 'CB2' } }, + ], + }, + { + code: ` + function getLength(x: T) { + return x.length; + } + `, + errors: [{ messageId: 'sole' }], + }, + { + code: ` + interface Lengthy { + length: number; + } + function getLength(x: T) { + return x.length; + } + `, + errors: [{ messageId: 'sole' }], + }, + { + code: 'declare function get(): T;', + errors: [{ messageId: 'sole', data: { name: 'T' } }], + }, + { + code: 'declare function get(): T;', + errors: [{ messageId: 'sole', data: { name: 'T' } }], + }, + { + code: 'declare function take(param: T): void;', + errors: [{ messageId: 'sole', data: { name: 'T' } }], + }, + { + code: 'declare function take(param: T): void;', + errors: [{ messageId: 'sole', data: { name: 'T' } }], + }, + { + code: 'declare function take(param1: T, param2: U): void;', + errors: [{ messageId: 'sole', data: { name: 'U' } }], + }, + { + code: 'declare function take(param: T): U;', + errors: [{ messageId: 'sole', data: { name: 'U' } }], + }, + { + code: 'declare function take(param: U): U;', + errors: [{ messageId: 'sole', data: { name: 'T' } }], + }, + { + code: 'declare function get(param: U): U;', + errors: [{ messageId: 'sole', data: { name: 'T' } }], + }, + { + code: 'declare function get(param: T): U;', + errors: [{ messageId: 'sole', data: { name: 'U' } }], + }, + { + code: 'declare function compare(param1: T, param2: U): boolean;', + errors: [{ messageId: 'sole', data: { name: 'U' } }], + }, + { + code: 'declare function get(param: (param: U) => V): T;', + errors: [ + { messageId: 'sole', data: { name: 'T' } }, + { messageId: 'sole', data: { name: 'U' } }, + { messageId: 'sole', data: { name: 'V' } }, + ], + }, + { + code: 'declare function get(param: (param: T) => U): T;', + errors: [ + { messageId: 'sole', data: { name: 'T' } }, + { messageId: 'sole', data: { name: 'T' } }, + { messageId: 'sole', data: { name: 'U' } }, + ], + }, + { + code: 'type Fn = () => T;', + errors: [{ messageId: 'sole', data: { name: 'T' } }], + }, + { + code: 'type Fn = () => [];', + errors: [{ messageId: 'sole', data: { name: 'T' } }], + }, + { + code: ` + type Other = 0; + type Fn = () => Other; + `, + errors: [{ messageId: 'sole', data: { name: 'T' } }], + }, + { + code: ` + type Other = 0 | 1; + type Fn = () => Other; + `, + errors: [{ messageId: 'sole', data: { name: 'T' } }], + }, + { + code: 'type Fn = (param: U) => void;', + errors: [{ messageId: 'sole', data: { name: 'U' } }], + }, + { + code: 'type Ctr = new () => T;', + errors: [{ messageId: 'sole', data: { name: 'T' } }], + }, + { + code: 'type Fn = () => { [K in keyof T]: K };', + errors: [{ messageId: 'sole', data: { name: 'T' } }], + }, + { + code: "type Fn = () => { [K in 'a']: T };", + errors: [{ messageId: 'sole', data: { name: 'T' } }], + }, + { + code: 'type Fn = (value: unknown) => value is T;', + errors: [{ messageId: 'sole', data: { name: 'T' } }], + }, + { + code: 'type Fn = () => `a${T}b`;', + errors: [{ messageId: 'sole', data: { name: 'T' } }], + }, + ], +}); diff --git a/packages/eslint-plugin/tests/schema-snapshots/no-unnecessary-type-parameters.shot b/packages/eslint-plugin/tests/schema-snapshots/no-unnecessary-type-parameters.shot new file mode 100644 index 000000000000..a1e7d6b72117 --- /dev/null +++ b/packages/eslint-plugin/tests/schema-snapshots/no-unnecessary-type-parameters.shot @@ -0,0 +1,14 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Rule schemas should be convertible to TS types for documentation purposes no-unnecessary-type-parameters 1`] = ` +" +# SCHEMA: + +[] + + +# TYPES: + +/** No options declared */ +type Options = [];" +`; diff --git a/packages/repo-tools/src/generate-contributors.mts b/packages/repo-tools/src/generate-contributors.mts index 220750350dd5..7e27ce3345fc 100644 --- a/packages/repo-tools/src/generate-contributors.mts +++ b/packages/repo-tools/src/generate-contributors.mts @@ -35,6 +35,8 @@ interface User { html_url: string; } +// This is an internal script, we're ok with the unsafe assertion. 🤫 +// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-parameters async function getData(url: string | undefined): Promise { if (url == null) { return null; diff --git a/packages/scope-manager/tests/test-utils/getSpecificNode.ts b/packages/scope-manager/tests/test-utils/getSpecificNode.ts index d8189a9cdfed..aa075fb059fe 100644 --- a/packages/scope-manager/tests/test-utils/getSpecificNode.ts +++ b/packages/scope-manager/tests/test-utils/getSpecificNode.ts @@ -11,12 +11,13 @@ function getSpecificNode< ): Node; function getSpecificNode< Selector extends AST_NODE_TYPES, - Node extends Extract, ReturnType extends TSESTree.Node, >( ast: TSESTree.Node, selector: Selector, - cb: (node: Node) => ReturnType | null | undefined, + cb: ( + node: Extract, + ) => ReturnType | null | undefined, ): ReturnType; function getSpecificNode( diff --git a/packages/typescript-eslint/src/configs/all.ts b/packages/typescript-eslint/src/configs/all.ts index dc899379b164..ba050418fbc9 100644 --- a/packages/typescript-eslint/src/configs/all.ts +++ b/packages/typescript-eslint/src/configs/all.ts @@ -104,6 +104,7 @@ export default ( '@typescript-eslint/no-unnecessary-type-arguments': 'error', '@typescript-eslint/no-unnecessary-type-assertion': 'error', '@typescript-eslint/no-unnecessary-type-constraint': 'error', + '@typescript-eslint/no-unnecessary-type-parameters': 'error', '@typescript-eslint/no-unsafe-argument': 'error', '@typescript-eslint/no-unsafe-assignment': 'error', '@typescript-eslint/no-unsafe-call': 'error', diff --git a/packages/typescript-eslint/src/configs/disable-type-checked.ts b/packages/typescript-eslint/src/configs/disable-type-checked.ts index 9df504415e37..e173917faaff 100644 --- a/packages/typescript-eslint/src/configs/disable-type-checked.ts +++ b/packages/typescript-eslint/src/configs/disable-type-checked.ts @@ -35,6 +35,7 @@ export default ( '@typescript-eslint/no-unnecessary-template-expression': 'off', '@typescript-eslint/no-unnecessary-type-arguments': 'off', '@typescript-eslint/no-unnecessary-type-assertion': 'off', + '@typescript-eslint/no-unnecessary-type-parameters': 'off', '@typescript-eslint/no-unsafe-argument': 'off', '@typescript-eslint/no-unsafe-assignment': 'off', '@typescript-eslint/no-unsafe-call': 'off', diff --git a/packages/typescript-estree/src/parser-options.ts b/packages/typescript-estree/src/parser-options.ts index 8b4496526d5c..8aeeaa8ff049 100644 --- a/packages/typescript-estree/src/parser-options.ts +++ b/packages/typescript-estree/src/parser-options.ts @@ -217,6 +217,8 @@ export type TSESTreeOptions = ParseAndGenerateServicesOptions; // This lets us use generics to type the return value, and removes the need to // handle the undefined type in the get method export interface ParserWeakMap { + // This is unsafe internally, so it should only be exposed via safe wrappers. + // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-parameters get(key: Key): Value; has(key: unknown): boolean; } diff --git a/packages/typescript-estree/tests/lib/convert.test.ts b/packages/typescript-estree/tests/lib/convert.test.ts index 306f25227409..59930729adbc 100644 --- a/packages/typescript-estree/tests/lib/convert.test.ts +++ b/packages/typescript-estree/tests/lib/convert.test.ts @@ -280,11 +280,17 @@ describe('convert', () => { describe('suppressDeprecatedPropertyWarnings', () => { const makeNodeGetter = - ( + < + // Small convenience for testing the nodes: + // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-parameters + S extends ts.Statement, + // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-parameters + TNode extends TSESTree.Node, + >( code: string, tsToEsNode: (statement: S) => TSNode, ) => - (converterOptions?: ConverterOptions): T => { + (converterOptions?: ConverterOptions): TNode => { const ast = convertCode(code); const instance = new Converter(ast, { shouldPreserveNodeMaps: true, diff --git a/packages/typescript-estree/tests/test-utils/test-utils.ts b/packages/typescript-estree/tests/test-utils/test-utils.ts index 71a84f1691cb..8b08393ee5e7 100644 --- a/packages/typescript-estree/tests/test-utils/test-utils.ts +++ b/packages/typescript-estree/tests/test-utils/test-utils.ts @@ -77,7 +77,7 @@ export function isJSXFileType(fileType: string): boolean { * @param ast the AST object * @returns copy of the AST object */ -export function deeplyCopy(ast: T): T { +export function deeplyCopy>(ast: T): T { return omitDeep(ast) as T; } @@ -96,8 +96,8 @@ function isObjectLike(value: unknown): value is UnknownObject { * @param selectors advance ast modifications * @returns formatted object */ -export function omitDeep( - root: T, +export function omitDeep( + root: UnknownObject, keysToOmit: { key: string; predicate: (value: unknown) => boolean }[] = [], selectors: Record< string, @@ -152,5 +152,5 @@ export function omitDeep( return node; } - return visit(root as UnknownObject, null); + return visit(root, null); } diff --git a/packages/utils/src/ast-utils/helpers.ts b/packages/utils/src/ast-utils/helpers.ts index afe8a6fede89..c76d24f88246 100644 --- a/packages/utils/src/ast-utils/helpers.ts +++ b/packages/utils/src/ast-utils/helpers.ts @@ -40,6 +40,8 @@ export const isNodeOfTypeWithConditions = < export const isTokenOfTypeWithConditions = < TokenType extends AST_TOKEN_TYPES, + // This is technically unsafe, but we find it useful to extract out the type + // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-parameters ExtractedToken extends Extract, Conditions extends Partial, >( diff --git a/packages/utils/src/eslint-utils/deepMerge.ts b/packages/utils/src/eslint-utils/deepMerge.ts index f6513944c1bd..a4ace614df5d 100644 --- a/packages/utils/src/eslint-utils/deepMerge.ts +++ b/packages/utils/src/eslint-utils/deepMerge.ts @@ -4,7 +4,7 @@ type ObjectLike = Record; * Check if the variable contains an object strictly rejecting arrays * @returns `true` if obj is an object */ -function isObjectNotArray(obj: unknown): obj is T { +function isObjectNotArray(obj: unknown): obj is ObjectLike { return typeof obj === 'object' && obj != null && !Array.isArray(obj); } diff --git a/packages/website/src/globals.d.ts b/packages/website/src/globals.d.ts index c7bf5f2aa0f6..966cee026906 100644 --- a/packages/website/src/globals.d.ts +++ b/packages/website/src/globals.d.ts @@ -3,6 +3,9 @@ import type * as ts from 'typescript'; declare global { interface WindowRequire { + // We know it's an unsafe assertion. It's for window.require usage, so we + // don't have to use verbose type assertions on every call. + // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-parameters ( files: string[], success?: (...arg: T) => void,