From 59459707fb05bd9dca5ad4bc9513bf75043c04a8 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Mon, 14 Oct 2024 17:59:41 -0600 Subject: [PATCH 1/5] feat(eslint-plugin): [no-unnecessary-type-parameters] add suggestion fixer --- .../rules/no-unnecessary-type-parameters.ts | 118 +++- .../no-unnecessary-type-parameters.test.ts | 545 +++++++++++++++++- 2 files changed, 656 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 e427a17f29e9..8d060dedabfa 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts @@ -1,5 +1,5 @@ import type { Reference } from '@typescript-eslint/scope-manager'; -import type { TSESTree } from '@typescript-eslint/utils'; +import type { TSESLint, TSESTree } from '@typescript-eslint/utils'; import { AST_NODE_TYPES } from '@typescript-eslint/utils'; import * as tsutils from 'ts-api-utils'; @@ -7,7 +7,12 @@ import * as ts from 'typescript'; import type { MakeRequired } from '../util'; -import { createRule, getParserServices, nullThrows } from '../util'; +import { + createRule, + getParserServices, + nullThrows, + NullThrowsReasons, +} from '../util'; type NodeWithTypeParameters = MakeRequired< ts.ClassLikeDeclaration | ts.SignatureDeclaration, @@ -23,7 +28,10 @@ export default createRule({ recommended: 'strict', requiresTypeChecking: true, }, + hasSuggestions: true, messages: { + replaceUsagesWithConstraint: + 'Replace all usages of type parameter with its constraint.', sole: 'Type parameter {{name}} is {{uses}} in the {{descriptor}} signature.', }, schema: [], @@ -84,6 +92,112 @@ export default createRule({ descriptor, uses: identifierCounts === 1 ? 'never used' : 'used only once', }, + suggest: [ + { + messageId: 'replaceUsagesWithConstraint', + *fix(fixer): Generator { + // Replace all the usages of the type parameter with the constraint... + + const constraint = esTypeParameter.constraint; + // special case - a constraint of 'any' actually acts like 'unknown' + const constraintText = + constraint != null && + constraint.type !== AST_NODE_TYPES.TSAnyKeyword + ? context.sourceCode.getText(constraint) + : 'unknown'; + for (const reference of smTypeParameterVariable.references) { + if (reference.isTypeReference) { + const referenceNode = reference.identifier; + yield fixer.replaceText(referenceNode, constraintText); + } + } + + // ...and remove the type parameter itself from the declaration. + + const typeParams = nullThrows( + node.typeParameters, + 'node should have type parameters', + ).params; + + const index = typeParams.indexOf(esTypeParameter); + + if (index === -1) { + throw new Error( + "type parameter should be in node's type parameters", + ); + } else if (typeParams.length === 1) { + const angleBracketBefore = nullThrows( + context.sourceCode.getTokenBefore( + esTypeParameter, + token => token.value === '<', + ), + NullThrowsReasons.MissingToken( + 'angle bracket', + 'type parameter list', + ), + ); + + const angleBracketAfter = nullThrows( + context.sourceCode.getTokenAfter( + esTypeParameter, + token => token.value === '>', + ), + NullThrowsReasons.MissingToken( + 'angle bracket', + 'type parameter list', + ), + ); + + yield fixer.removeRange([ + angleBracketBefore.range[0], + angleBracketAfter.range[1], + ]); + } else if (index === 0) { + const commaAfter = nullThrows( + context.sourceCode.getTokenAfter( + esTypeParameter, + token => token.value === ',', + ), + NullThrowsReasons.MissingToken( + 'comma', + 'type parameter list', + ), + ); + + const tokenAfterComma = nullThrows( + context.sourceCode.getTokenAfter(commaAfter, { + includeComments: true, + }), + NullThrowsReasons.MissingToken( + 'token', + 'type parameter list', + ), + ); + + yield fixer.removeRange([ + esTypeParameter.range[0], + tokenAfterComma.range[0], + ]); + } else { + const commaBefore = nullThrows( + context.sourceCode.getTokenBefore( + esTypeParameter, + token => token.value === ',', + ), + NullThrowsReasons.MissingToken( + 'comma', + 'type parameter list', + ), + ); + + yield fixer.removeRange([ + commaBefore.range[0], + esTypeParameter.range[1], + ]); + } + }, + }, + ], }); } } 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 c55d2b8f2d73..81fcdc9b43a8 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 @@ -438,6 +438,12 @@ const f = ( { data: { descriptor: 'function', name: 'T', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: 'const func = (param: unknown) => null;', + }, + ], }, ], }, @@ -447,6 +453,12 @@ const f = ( { data: { descriptor: 'function', name: 'T', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: 'const f1 = (): unknown => {};', + }, + ], }, ], }, @@ -460,6 +472,16 @@ const f = ( { data: { descriptor: 'function', name: 'T', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` + interface I { + (value: unknown): void; + } + `, + }, + ], }, ], }, @@ -469,7 +491,21 @@ const f = ( m(x: T): void; } `, - errors: [{ messageId: 'sole' }], + errors: [ + { + messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` + interface I { + m(x: unknown): void; + } + `, + }, + ], + }, + ], }, { code: ` @@ -483,6 +519,18 @@ const f = ( { data: { descriptor: 'class', name: 'T', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` + class Joiner { + join(el: string | number, other: string) { + return [el, other].join(','); + } + } + `, + }, + ], }, ], }, @@ -494,6 +542,14 @@ const f = ( { data: { descriptor: 'class', name: 'V', uses: 'never used' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` + declare class C {} + `, + }, + ], }, ], }, @@ -507,10 +563,30 @@ const f = ( { data: { descriptor: 'class', name: 'T', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` + declare class C { + method(param: unknown): U; + } + `, + }, + ], }, { data: { descriptor: 'class', name: 'U', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` + declare class C { + method(param: T): unknown; + } + `, + }, + ], }, ], }, @@ -524,10 +600,30 @@ const f = ( { data: { descriptor: 'function', name: 'T', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` + declare class C { + method(param: unknown): U; + } + `, + }, + ], }, { data: { descriptor: 'function', name: 'U', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` + declare class C { + method(param: T): unknown; + } + `, + }, + ], }, ], }, @@ -541,6 +637,16 @@ const f = ( { data: { descriptor: 'function', name: 'P', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` + declare class C { + prop: () => unknown; + } + `, + }, + ], }, ], }, @@ -554,6 +660,16 @@ const f = ( { data: { descriptor: 'function', name: 'T', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` + declare class Foo { + foo(this: unknown): void; + } + `, + }, + ], }, ], }, @@ -567,10 +683,30 @@ const f = ( { data: { descriptor: 'function', name: 'A', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` + function third(a: unknown, b: B, c: C): C { + return c; + } + `, + }, + ], }, { data: { descriptor: 'function', name: 'B', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` + function third(a: A, b: unknown, c: C): C { + return c; + } + `, + }, + ], }, ], }, @@ -585,6 +721,17 @@ const f = ( { data: { descriptor: 'function', name: 'T', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` + function foo(_: unknown) { + const x: unknown = null!; + const y: unknown = null!; + } + `, + }, + ], }, ], }, @@ -599,20 +746,44 @@ const f = ( { data: { descriptor: 'function', name: 'T', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` + function foo(_: unknown): void { + const x: unknown = null!; + const y: unknown = null!; + } + `, + }, + ], }, ], }, { code: ` - function foo(_: T): (input: T) => T { - const x: T = null!; - const y: T = null!; - } +function foo(_: T): (input: T) => T { + const x: T = null!; + const y: T = null!; + return null!; +} `, errors: [ { data: { descriptor: 'function', name: 'T', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` +function foo(_: unknown): (input: T) => T { + const x: unknown = null!; + const y: unknown = null!; + return null!; +} + `, + }, + ], }, ], }, @@ -631,6 +802,21 @@ const f = ( { data: { descriptor: 'function', name: 'T', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` + function foo(_: unknown) { + function withX(): unknown { + return null!; + } + function withY(): unknown { + return null!; + } + } + `, + }, + ], }, ], }, @@ -644,6 +830,16 @@ const f = ( { data: { descriptor: 'function', name: 'T', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` + function parseYAML(input: string): unknown { + return input as any as unknown; + } + `, + }, + ], }, ], }, @@ -657,6 +853,16 @@ const f = ( { data: { descriptor: 'function', name: 'K', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` + function printProperty(obj: T, key: keyof T) { + console.log(obj[key]); + } + `, + }, + ], }, ], }, @@ -671,6 +877,17 @@ const f = ( { data: { descriptor: 'function', name: 'T', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` + function fn(param: string) { + let v: unknown = null!; + return v; + } + `, + }, + ], }, ], }, @@ -691,10 +908,42 @@ const f = ( { data: { descriptor: 'function', name: 'CB1', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` + function both< + Args extends unknown[], + CB2 extends (...args: Args) => void, + >(fn1: (...args: Args) => void, fn2: CB2): (...args: Args) => void { + return function (...args: Args) { + fn1(...args); + fn2(...args); + }; + } + `, + }, + ], }, { data: { descriptor: 'function', name: 'CB2', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` + function both< + Args extends unknown[], + CB1 extends (...args: Args) => void, + >(fn1: CB1, fn2: (...args: Args) => void): (...args: Args) => void { + return function (...args: Args) { + fn1(...args); + fn2(...args); + }; + } + `, + }, + ], }, ], }, @@ -708,6 +957,16 @@ const f = ( { data: { descriptor: 'function', name: 'T', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` + function getLength(x: { length: number }) { + return x.length; + } + `, + }, + ], }, ], }, @@ -724,6 +983,19 @@ const f = ( { data: { descriptor: 'function', name: 'T', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` + interface Lengthy { + length: number; + } + function getLength(x: Lengthy) { + return x.length; + } + `, + }, + ], }, ], }, @@ -733,6 +1005,12 @@ const f = ( { data: { descriptor: 'function', name: 'T', uses: 'never used' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: 'declare function get(): unknown;', + }, + ], }, ], }, @@ -742,6 +1020,12 @@ const f = ( { data: { descriptor: 'function', name: 'T', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: 'declare function get(): unknown;', + }, + ], }, ], }, @@ -751,6 +1035,12 @@ const f = ( { data: { descriptor: 'function', name: 'T', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: 'declare function get(): object;', + }, + ], }, ], }, @@ -760,6 +1050,12 @@ const f = ( { data: { descriptor: 'function', name: 'T', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: 'declare function take(param: unknown): void;', + }, + ], }, ], }, @@ -769,6 +1065,12 @@ const f = ( { data: { descriptor: 'function', name: 'T', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: 'declare function take(param: object): void;', + }, + ], }, ], }, @@ -778,6 +1080,13 @@ const f = ( { data: { descriptor: 'function', name: 'U', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: + 'declare function take(param1: T, param2: unknown): void;', + }, + ], }, ], }, @@ -787,6 +1096,12 @@ const f = ( { data: { descriptor: 'function', name: 'U', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: 'declare function take(param: T): T;', + }, + ], }, ], }, @@ -796,6 +1111,12 @@ const f = ( { data: { descriptor: 'function', name: 'T', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: 'declare function take(param: U): U;', + }, + ], }, ], }, @@ -805,6 +1126,12 @@ const f = ( { data: { descriptor: 'function', name: 'T', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: 'declare function get(param: U): U;', + }, + ], }, ], }, @@ -814,6 +1141,12 @@ const f = ( { data: { descriptor: 'function', name: 'U', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: 'declare function get(param: T): T;', + }, + ], }, ], }, @@ -823,6 +1156,13 @@ const f = ( { data: { descriptor: 'function', name: 'U', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: + 'declare function compare(param1: T, param2: T): boolean;', + }, + ], }, ], }, @@ -832,14 +1172,35 @@ const f = ( { data: { descriptor: 'function', name: 'T', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: + 'declare function get(param: (param: U) => V): unknown;', + }, + ], }, { data: { descriptor: 'function', name: 'U', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: + 'declare function get(param: (param: unknown) => V): T;', + }, + ], }, { data: { descriptor: 'function', name: 'V', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: + 'declare function get(param: (param: U) => unknown): T;', + }, + ], }, ], }, @@ -847,16 +1208,44 @@ const f = ( code: 'declare function get(param: (param: T) => U): T;', errors: [ { + column: 22, data: { descriptor: 'function', name: 'T', uses: 'used only once' }, + endColumn: 23, + line: 1, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: + 'declare function get(param: (param: T) => U): unknown;', + }, + ], }, { + column: 33, data: { descriptor: 'function', name: 'T', uses: 'used only once' }, + endColumn: 34, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: + 'declare function get(param: (param: unknown) => U): T;', + }, + ], }, { + column: 36, data: { descriptor: 'function', name: 'U', uses: 'used only once' }, + endColumn: 37, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: + 'declare function get(param: (param: T) => unknown): T;', + }, + ], }, ], }, @@ -866,6 +1255,12 @@ const f = ( { data: { descriptor: 'function', name: 'T', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: 'type Fn = () => unknown;', + }, + ], }, ], }, @@ -875,6 +1270,12 @@ const f = ( { data: { descriptor: 'function', name: 'T', uses: 'never used' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: 'type Fn = () => [];', + }, + ], }, ], }, @@ -887,6 +1288,15 @@ const f = ( { data: { descriptor: 'function', name: 'T', uses: 'never used' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` + type Other = 0; + type Fn = () => Other; + `, + }, + ], }, ], }, @@ -899,6 +1309,15 @@ const f = ( { data: { descriptor: 'function', name: 'T', uses: 'never used' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` + type Other = 0 | 1; + type Fn = () => Other; + `, + }, + ], }, ], }, @@ -908,6 +1327,12 @@ const f = ( { data: { descriptor: 'function', name: 'U', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: 'type Fn = (param: unknown) => void;', + }, + ], }, ], }, @@ -917,6 +1342,12 @@ const f = ( { data: { descriptor: 'function', name: 'T', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: 'type Ctr = new () => unknown;', + }, + ], }, ], }, @@ -926,6 +1357,12 @@ const f = ( { data: { descriptor: 'function', name: 'T', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: 'type Fn = () => { [K in keyof unknown]: K };', + }, + ], }, ], }, @@ -935,6 +1372,12 @@ const f = ( { data: { descriptor: 'function', name: 'T', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: "type Fn = () => { [K in 'a']: unknown };", + }, + ], }, ], }, @@ -944,6 +1387,12 @@ const f = ( { data: { descriptor: 'function', name: 'T', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: 'type Fn = (value: unknown) => value is unknown;', + }, + ], }, ], }, @@ -953,6 +1402,12 @@ const f = ( { data: { descriptor: 'function', name: 'T', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: 'type Fn = () => `a${string}b`;', + }, + ], }, ], }, @@ -967,6 +1422,17 @@ const f = ( { data: { descriptor: 'function', name: 'V', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` + declare function mapObj( + obj: { [key in K]?: unknown }, + fn: (key: K) => number, + ): number[]; + `, + }, + ], }, ], }, @@ -978,6 +1444,14 @@ declare function setItem(T): T; { data: { descriptor: 'function', name: 'T', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` +declare function setItem(T): unknown; + `, + }, + ], }, ], }, @@ -991,6 +1465,16 @@ interface StorageService { { data: { descriptor: 'function', name: 'T', uses: 'never used' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` +interface StorageService { + setItem({ key: string, value: T }): Promise; +} + `, + }, + ], }, ], }, @@ -1013,10 +1497,61 @@ type Equal = { data: { descriptor: 'function', name: 'T1', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` +type Compute = A extends Function ? A : { [K in keyof A]: Compute }; +type Equal = + (() => unknown extends Compute ? 1 : 2) extends + (() => T2 extends Compute ? 1 : 2) + ? true + : false; + `, + }, + ], }, { data: { descriptor: 'function', name: 'T2', uses: 'used only once' }, messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` +type Compute = A extends Function ? A : { [K in keyof A]: Compute }; +type Equal = + (() => T1 extends Compute ? 1 : 2) extends + (() => unknown extends Compute ? 1 : 2) + ? true + : false; + `, + }, + ], + }, + ], + }, + { + code: ` +function f(x: T): void { + // @ts-expect-error + x.notAMethod(); +} + `, + errors: [ + { + data: { descriptor: 'function', name: 'T', uses: 'used only once' }, + messageId: 'sole', + suggestions: [ + { + messageId: 'replaceUsagesWithConstraint', + output: ` +function f(x: unknown): void { + // @ts-expect-error + x.notAMethod(); +} + `, + }, + ], }, ], }, From f136a974ea4c9a9aef766bae0cf672422659e618 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Mon, 14 Oct 2024 18:37:26 -0600 Subject: [PATCH 2/5] cov --- .../eslint-plugin/src/rules/no-unnecessary-type-parameters.ts | 1 + 1 file changed, 1 insertion(+) 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 8d060dedabfa..0cd0e52307d5 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts @@ -121,6 +121,7 @@ export default createRule({ const index = typeParams.indexOf(esTypeParameter); + /* istanbul ignore if */ // This should never happen. if (index === -1) { throw new Error( "type parameter should be in node's type parameters", From 9a4274095907d652d2abf07ac4f733a4eea6c121 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Mon, 14 Oct 2024 18:40:17 -0600 Subject: [PATCH 3/5] 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 0cd0e52307d5..885b0f058a50 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts @@ -121,7 +121,7 @@ export default createRule({ const index = typeParams.indexOf(esTypeParameter); - /* istanbul ignore if */ // This should never happen. + /* istanbul ignore if: this is an assertion that should never happen */ if (index === -1) { throw new Error( "type parameter should be in node's type parameters", From 0fe5cb39022cceb6fdcd4a77d094c430c5e15c92 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Tue, 15 Oct 2024 15:47:17 -0600 Subject: [PATCH 4/5] simplify logic for removing last type parameter --- .../rules/no-unnecessary-type-parameters.ts | 36 ++++--------------- 1 file changed, 6 insertions(+), 30 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 885b0f058a50..0f97afbdcc9f 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts @@ -114,45 +114,21 @@ export default createRule({ // ...and remove the type parameter itself from the declaration. - const typeParams = nullThrows( + const typeParamsNode = nullThrows( node.typeParameters, 'node should have type parameters', - ).params; + ); - const index = typeParams.indexOf(esTypeParameter); + const index = typeParamsNode.params.indexOf(esTypeParameter); /* istanbul ignore if: this is an assertion that should never happen */ if (index === -1) { throw new Error( "type parameter should be in node's type parameters", ); - } else if (typeParams.length === 1) { - const angleBracketBefore = nullThrows( - context.sourceCode.getTokenBefore( - esTypeParameter, - token => token.value === '<', - ), - NullThrowsReasons.MissingToken( - 'angle bracket', - 'type parameter list', - ), - ); - - const angleBracketAfter = nullThrows( - context.sourceCode.getTokenAfter( - esTypeParameter, - token => token.value === '>', - ), - NullThrowsReasons.MissingToken( - 'angle bracket', - 'type parameter list', - ), - ); - - yield fixer.removeRange([ - angleBracketBefore.range[0], - angleBracketAfter.range[1], - ]); + } else if (typeParamsNode.params.length === 1) { + // Remove the whole generic syntax if we're removing the only type parameter in the list. + yield fixer.remove(typeParamsNode); } else if (index === 0) { const commaAfter = nullThrows( context.sourceCode.getTokenAfter( From dfa22d01a7fb310561f9abcce74ae0de95c19bfe Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Thu, 17 Oct 2024 11:51:10 -0600 Subject: [PATCH 5/5] remove ostensibly unreachable assertion --- .../rules/no-unnecessary-type-parameters.ts | 97 +++++++++---------- 1 file changed, 48 insertions(+), 49 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 0f97afbdcc9f..3e9930128cab 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts @@ -119,58 +119,57 @@ export default createRule({ 'node should have type parameters', ); - const index = typeParamsNode.params.indexOf(esTypeParameter); - - /* istanbul ignore if: this is an assertion that should never happen */ - if (index === -1) { - throw new Error( - "type parameter should be in node's type parameters", - ); - } else if (typeParamsNode.params.length === 1) { + // 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. yield fixer.remove(typeParamsNode); - } else if (index === 0) { - const commaAfter = nullThrows( - context.sourceCode.getTokenAfter( - esTypeParameter, - token => token.value === ',', - ), - NullThrowsReasons.MissingToken( - 'comma', - 'type parameter list', - ), - ); - - const tokenAfterComma = nullThrows( - context.sourceCode.getTokenAfter(commaAfter, { - includeComments: true, - }), - NullThrowsReasons.MissingToken( - 'token', - 'type parameter list', - ), - ); - - yield fixer.removeRange([ - esTypeParameter.range[0], - tokenAfterComma.range[0], - ]); } else { - const commaBefore = nullThrows( - context.sourceCode.getTokenBefore( - esTypeParameter, - token => token.value === ',', - ), - NullThrowsReasons.MissingToken( - 'comma', - 'type parameter list', - ), - ); - - yield fixer.removeRange([ - commaBefore.range[0], - esTypeParameter.range[1], - ]); + const index = typeParamsNode.params.indexOf(esTypeParameter); + + if (index === 0) { + const commaAfter = nullThrows( + context.sourceCode.getTokenAfter( + esTypeParameter, + token => token.value === ',', + ), + NullThrowsReasons.MissingToken( + 'comma', + 'type parameter list', + ), + ); + + const tokenAfterComma = nullThrows( + context.sourceCode.getTokenAfter(commaAfter, { + includeComments: true, + }), + NullThrowsReasons.MissingToken( + 'token', + 'type parameter list', + ), + ); + + yield fixer.removeRange([ + esTypeParameter.range[0], + tokenAfterComma.range[0], + ]); + } else { + const commaBefore = nullThrows( + context.sourceCode.getTokenBefore( + esTypeParameter, + token => token.value === ',', + ), + NullThrowsReasons.MissingToken( + 'comma', + 'type parameter list', + ), + ); + + yield fixer.removeRange([ + commaBefore.range[0], + esTypeParameter.range[1], + ]); + } } }, },