Skip to content

refactor: TypeParameter parsing and no-unnecessary-type-arguments rule #1381

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 26, 2019
Merged

refactor: TypeParameter parsing and no-unnecessary-type-arguments rule #1381

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9ebd705
fix(eslint-plugin): improve no-unnecessary-type-arguments for classes
armano2 Dec 26, 2019
5ba9c62
refactor(eslint-plugin): cleanup code
armano2 Dec 26, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 20 additions & 45 deletions packages/eslint-plugin/src/rules/no-unnecessary-type-arguments.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,6 @@ import * as ts from 'typescript';
import * as util from '../util';
import { findFirstResult } from '../util';

interface ArgsAndParams {
typeArguments: ts.NodeArray<ts.TypeNode>;
typeParameters: readonly ts.TypeParameterDeclaration[];
}

type ExtendingClassLikeDeclaration = ts.ClassLikeDeclaration & {
heritageClauses: ts.NodeArray<ts.HeritageClause>;
};

type ParameterCapableTSNode =
| ts.CallExpression
| ts.NewExpression
Expand Down Expand Up @@ -43,65 +34,52 @@ export default util.createRule<[], MessageIds>({
create(context) {
const parserServices = util.getParserServices(context);
const checker = parserServices.program.getTypeChecker();
const sourceCode = context.getSourceCode();

function checkTSArgsAndParameters(
esParameters: TSESTree.TSTypeParameterInstantiation,
{ typeArguments, typeParameters }: ArgsAndParams,
typeParameters: readonly ts.TypeParameterDeclaration[],
): void {
// Just check the last one. Must specify previous type parameters if the last one is specified.
const i = typeArguments.length - 1;
const arg = typeArguments[i];
const i = esParameters.params.length - 1;
const arg = esParameters.params[i];
const param = typeParameters[i];

// TODO: would like checker.areTypesEquivalent. https://github.com/Microsoft/TypeScript/issues/13502
if (
param.default === undefined ||
param.default.getText() !== arg.getText()
!param.default ||
param.default.getText() !== sourceCode.getText(arg)
) {
return;
}

context.report({
node: arg,
messageId: 'unnecessaryTypeParameter',
fix: fixer =>
fixer.removeRange(
i === 0
? [typeArguments.pos - 1, typeArguments.end + 1]
: [typeArguments[i - 1].end, arg.end],
? esParameters.range
: [esParameters.params[i - 1].range[1], arg.range[1]],
),
messageId: 'unnecessaryTypeParameter',
node: esParameters.params[i],
});
}

return {
TSTypeParameterInstantiation(node): void {
const parentDeclaration = parserServices.esTreeNodeToTSNodeMap.get<
ExtendingClassLikeDeclaration | ParameterCapableTSNode
>(node.parent!);

const expression = tsutils.isClassLikeDeclaration(parentDeclaration)
? parentDeclaration.heritageClauses[0].types[0]
: parentDeclaration;
const expression = parserServices.esTreeNodeToTSNodeMap.get<
ParameterCapableTSNode
>(node);

const argsAndParams = getArgsAndParameters(expression, checker);
if (argsAndParams !== undefined) {
checkTSArgsAndParameters(node, argsAndParams);
const typeParameters = getTypeParametersFromNode(expression, checker);
if (typeParameters) {
checkTSArgsAndParameters(node, typeParameters);
}
},
};
},
});

function getArgsAndParameters(
node: ParameterCapableTSNode,
checker: ts.TypeChecker,
): ArgsAndParams | undefined {
const typeParameters = getTypeParametersFromNode(node, checker);
return typeParameters === undefined
? undefined
: { typeArguments: node.typeArguments!, typeParameters };
}

function getTypeParametersFromNode(
node: ParameterCapableTSNode,
checker: ts.TypeChecker,
Expand All @@ -126,14 +104,11 @@ function getTypeParametersFromType(
checker: ts.TypeChecker,
): readonly ts.TypeParameterDeclaration[] | undefined {
const symAtLocation = checker.getSymbolAtLocation(type);
if (symAtLocation === undefined) {
if (!symAtLocation) {
return undefined;
}

const sym = getAliasedSymbol(symAtLocation, checker);
if (sym === undefined || sym.declarations === undefined) {
return undefined;
}

return findFirstResult(sym.declarations, decl =>
tsutils.isClassLikeDeclaration(decl) ||
Expand All @@ -149,8 +124,8 @@ function getTypeParametersFromCall(
checker: ts.TypeChecker,
): readonly ts.TypeParameterDeclaration[] | undefined {
const sig = checker.getResolvedSignature(node);
const sigDecl = sig === undefined ? undefined : sig.getDeclaration();
if (sigDecl === undefined) {
const sigDecl = sig?.getDeclaration();
if (!sigDecl) {
return ts.isNewExpression(node)
? getTypeParametersFromType(node.expression, checker)
: undefined;
Expand All @@ -162,7 +137,7 @@ function getTypeParametersFromCall(
function getAliasedSymbol(
symbol: ts.Symbol,
checker: ts.TypeChecker,
): ts.Symbol | undefined {
): ts.Symbol {
return tsutils.isSymbolFlagSet(symbol, ts.SymbolFlags.Alias)
? checker.getAliasedSymbol(symbol)
: symbol;
Expand Down
36 changes: 36 additions & 0 deletions packages/eslint-plugin/tests/rules/no-unnecessary-type-arguments.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@ const ruleTester = new RuleTester({

ruleTester.run('no-unnecessary-type-arguments', rule, {
valid: [
`f<>();`,
`f<string>();`,
`class Foo extends Bar<> {}`,
`class Foo extends Bar<string> {}`,
`class Foo implements Bar<> {}`,
`class Foo implements Bar<string> {}`,
`function f<T = number>() { }
f();`,
`function f<T = number>() { }
Expand Down Expand Up @@ -60,6 +66,14 @@ ruleTester.run('no-unnecessary-type-arguments', rule, {
`class Foo<T> {}
const foo = new Foo<number>();`,
`type Foo<T> = import('foo').Foo<T>;`,
`class Bar<T = number> {}
class Foo<T = number> extends Bar<T> {}`,
`interface Bar<T = number> {}
class Foo<T = number> implements Bar<T> {}`,
`class Bar<T = number> {}
class Foo<T = number> extends Bar<string> {}`,
`interface Bar<T = number> {}
class Foo<T = number> implements Bar<string> {}`,
],
invalid: [
{
Expand Down Expand Up @@ -141,5 +155,27 @@ ruleTester.run('no-unnecessary-type-arguments', rule, {
output: `class Foo<T = number> {}
const foo = new Foo();`,
},
{
code: `interface Bar<T = string> {}
class Foo<T = number> implements Bar<string> {}`,
errors: [
{
messageId: 'unnecessaryTypeParameter',
},
],
output: `interface Bar<T = string> {}
class Foo<T = number> implements Bar {}`,
},
{
code: `class Bar<T = string> {}
class Foo<T = number> extends Bar<string> {}`,
errors: [
{
messageId: 'unnecessaryTypeParameter',
},
],
output: `class Bar<T = string> {}
class Foo<T = number> extends Bar {}`,
},
],
});
40 changes: 30 additions & 10 deletions packages/typescript-estree/src/convert.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,20 +300,21 @@ export class Converter {

/**
* Converts a ts.Node's typeArguments to TSTypeParameterInstantiation node
* @param typeArguments ts.Node typeArguments
* @param typeArguments ts.NodeArray typeArguments
* @param node parent used to create this node
* @returns TypeParameterInstantiation node
*/
private convertTypeArgumentsToTypeParameters(
typeArguments: ts.NodeArray<ts.TypeNode>,
node: ts.Node,
): TSESTree.TSTypeParameterInstantiation {
const greaterThanToken = findNextToken(typeArguments, this.ast, this.ast)!;

return {
return this.createNode<TSESTree.TSTypeParameterInstantiation>(node, {
type: AST_NODE_TYPES.TSTypeParameterInstantiation,
range: [typeArguments.pos - 1, greaterThanToken.end],
loc: getLocFor(typeArguments.pos - 1, greaterThanToken.end, this.ast),
params: typeArguments.map(typeArgument => this.convertType(typeArgument)),
};
});
}

/**
Expand Down Expand Up @@ -386,7 +387,7 @@ export class Converter {
if ('typeArguments' in node) {
result.typeParameters =
node.typeArguments && 'pos' in node.typeArguments
? this.convertTypeArgumentsToTypeParameters(node.typeArguments)
? this.convertTypeArgumentsToTypeParameters(node.typeArguments, node)
: null;
}
if ('typeParameters' in node) {
Expand Down Expand Up @@ -1296,7 +1297,10 @@ export class Converter {
return this.createNode<TSESTree.TaggedTemplateExpression>(node, {
type: AST_NODE_TYPES.TaggedTemplateExpression,
typeParameters: node.typeArguments
? this.convertTypeArgumentsToTypeParameters(node.typeArguments)
? this.convertTypeArgumentsToTypeParameters(
node.typeArguments,
node,
)
: undefined,
tag: this.convertChild(node.tag),
quasi: this.convertChild(node.template),
Expand Down Expand Up @@ -1440,6 +1444,7 @@ export class Converter {
if (superClass.types[0] && superClass.types[0].typeArguments) {
result.superTypeParameters = this.convertTypeArgumentsToTypeParameters(
superClass.types[0].typeArguments,
superClass.types[0],
);
}
}
Expand Down Expand Up @@ -1781,6 +1786,7 @@ export class Converter {
if (node.typeArguments) {
result.typeParameters = this.convertTypeArgumentsToTypeParameters(
node.typeArguments,
node,
);
}
return result;
Expand All @@ -1798,6 +1804,7 @@ export class Converter {
if (node.typeArguments) {
result.typeParameters = this.convertTypeArgumentsToTypeParameters(
node.typeArguments,
node,
);
}
return result;
Expand Down Expand Up @@ -1958,7 +1965,10 @@ export class Converter {
openingElement: this.createNode<TSESTree.JSXOpeningElement>(node, {
type: AST_NODE_TYPES.JSXOpeningElement,
typeParameters: node.typeArguments
? this.convertTypeArgumentsToTypeParameters(node.typeArguments)
? this.convertTypeArgumentsToTypeParameters(
node.typeArguments,
node,
)
: undefined,
selfClosing: true,
name: this.convertJSXTagName(node.tagName, node),
Expand All @@ -1976,7 +1986,10 @@ export class Converter {
return this.createNode<TSESTree.JSXOpeningElement>(node, {
type: AST_NODE_TYPES.JSXOpeningElement,
typeParameters: node.typeArguments
? this.convertTypeArgumentsToTypeParameters(node.typeArguments)
? this.convertTypeArgumentsToTypeParameters(
node.typeArguments,
node,
)
: undefined,
selfClosing: false,
name: this.convertJSXTagName(node.tagName, node),
Expand Down Expand Up @@ -2081,7 +2094,10 @@ export class Converter {
type: AST_NODE_TYPES.TSTypeReference,
typeName: this.convertType(node.typeName),
typeParameters: node.typeArguments
? this.convertTypeArgumentsToTypeParameters(node.typeArguments)
? this.convertTypeArgumentsToTypeParameters(
node.typeArguments,
node,
)
: undefined,
});
}
Expand Down Expand Up @@ -2362,6 +2378,7 @@ export class Converter {
if (node.typeArguments) {
result.typeParameters = this.convertTypeArgumentsToTypeParameters(
node.typeArguments,
node,
);
}
return result;
Expand Down Expand Up @@ -2454,7 +2471,10 @@ export class Converter {
parameter: this.convertChild(node.argument),
qualifier: this.convertChild(node.qualifier),
typeParameters: node.typeArguments
? this.convertTypeArgumentsToTypeParameters(node.typeArguments)
? this.convertTypeArgumentsToTypeParameters(
node.typeArguments,
node,
)
: null,
});

Expand Down