Why include this, if it's not something that can even be unit tested?

My paranoia is: the rule fixerassumes the type parameter to be removed/replaced isin the method/function/class's declared list of type parameters of the node being inspected. But the way we get the type parameter to be removed is indirect.

1. ESTree selector inspects function/method:

   typescript-eslint/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts

   Lines 206 to 226 in9a42740

   	return{
   	[[
   	'ArrowFunctionExpression[typeParameters]',
   	'FunctionDeclaration[typeParameters]',
   	'FunctionExpression[typeParameters]',
   	'TSCallSignatureDeclaration[typeParameters]',
   	'TSConstructorType[typeParameters]',
   	'TSDeclareFunction[typeParameters]',
   	'TSEmptyBodyFunctionExpression[typeParameters]',
   	'TSFunctionType[typeParameters]',
   	'TSMethodSignature[typeParameters]',
   	].join(', ')](node:TSESTree.FunctionLike):void{
   	checkNode(node,'function');
   	},
   	[[
   	'ClassDeclaration[typeParameters]',
   	'ClassExpression[typeParameters]',
   	].join(', ')](node:TSESTree.FunctionLike):void{
   	checkNode(node,'class');
   	},
   	};

2. We switch into the ts AST node world with a type assertion:

   typescript-eslint/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts

   Lines 43 to 46 in9a42740

   	functioncheckNode(node:TSESTree.FunctionLike,descriptor:string):void{
   	consttsNode=parserServices.esTreeNodeToTSNodeMap.get(
   	node,
   	)asNodeWithTypeParameters;

3. We traverse to its type parameters and back out to the ESTree type parameter node

   typescript-eslint/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts

   Lines 54 to 58 in9a42740

   	for(consttypeParameteroftsNode.typeParameters){
   	constesTypeParameter=
   	parserServices.tsNodeToESTreeNodeMap.get<TSESTree.TSTypeParameter>(
   	typeParameter,
   	);

4. Weassume that the ESTree node from step 1. is thegrandparent of the type parameter from 3, in order to get its type parameter declaration (which we have to assert even exists):

   typescript-eslint/packages/eslint-plugin/src/rules/no-unnecessary-type-parameters.ts

   Lines 117 to 120 in9a42740

   	consttypeParams=nullThrows(
   	node.typeParameters,
   	'node should have type parameters',
   	).params;

5. We need to figure out what position the reported ESTree type parameter is within that asserted-to-exist type parameter declaration's list of type parameters. My goal was todouble-check here that the reported type parameter reallyis in that list, since, we have come here by two different paths completely, and it doesn't seem that far-fetched that a change to the rule logic could break some of our assumptions.

For reference, the ASTs look like

ESTREE: node that we inspect ---(maybe has) ---> TSTypeParameterDeclaration -----> Array<TypeParameter>TS AST: node that we inspect -----> Array<TypeParameterDeclaration>

Anyways, I can go ahead and remove it if you prefer. Just one of those things that I prefer to do so that we fail faster and more understandably when a nonlocal code change breaks some of our assumptions.

Yeah this kind of strategy used to be a lot more common in the project. But it practically never paid off 😞. Our test coverage is so high & users are so active in reporting that it just ended up being a lot of code bloat. I'm still a -1 on it, but if you want to advocate for it I wouldn't be opposed to hearing more inputs from the team.

i chop 🔪