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.
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
feat(eslint-plugin): added new rule use-default-type-parameter #562
Merged
JamesHenry
merged 17 commits into
typescript-eslint:master
from
JoshuaKGoldberg:typescript-eslint-use-default-type-parameter
Jul 25, 2019
Merged
Changes from 12 commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
942c324
feat(eslint-plugin): added new rule use-default-type-parameter
ec6c29c
Added docs and fix output tests
4c87679
Ha, fixed discovered lint failure
dca4fb0
Added assumption of existing heritageClauses
e41a0f6
Whoops, left tests commented out...
faf4214
Added any and unknown as test cases
b6a3677
Changed recommended to false; added to all
159ba9b
No more recommendation in README.md table
1beaa93
Merge branch 'master' into typescript-eslint-use-default-type-parameter
bradzacher 546abd6
A teeny bit more coverage and a bang
e557f1e
Added assetion for getAliasedSymbol too
cb5a6e8
Merge branch 'master' into typescript-eslint-use-default-type-parameter
bradzacher 5cc3715
Removed useless ternary
0c381fb
Merge branch 'typescript-eslint-use-default-type-parameter' of https:…
ba7c76f
Renamed to no-unnecessary-type-arguments
a8f202a
Merge branch 'master'
1c9d040
Merge branch 'master' into typescript-eslint-use-default-type-parameter
JamesHenry File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
53 changes: 53 additions & 0 deletions
53
packages/eslint-plugin/docs/rules/use-default-type-parameter.md
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
# Enforces that types will not to be used (use-default-type-parameter) | ||
|
||
Warns if an explicitly specified type argument is the default for that type parameter. | ||
|
||
## Rule Details | ||
|
||
Type parameters in TypeScript may specify a default value. | ||
For example: | ||
|
||
```ts | ||
function f<T = number>() {} | ||
``` | ||
|
||
It is redundant to provide an explicit type parameter equal to that default. | ||
|
||
Examples of **incorrect** code for this rule: | ||
|
||
```ts | ||
function f<T = number>() {} | ||
f<number>(); | ||
|
||
function g<T = number, U = string>() {} | ||
g<string, string>(); | ||
|
||
class C<T = number> {} | ||
function h(c: C<number>) {} | ||
new C<number>(); | ||
class D extends C<number> {} | ||
|
||
interface I<T = number> {} | ||
class Impl implements I<number> {} | ||
``` | ||
|
||
Examples of **correct** code for this rule: | ||
|
||
```ts | ||
function f<T = number>() {} | ||
f<string>(); | ||
|
||
function g<T = number, U = string>() {} | ||
g<number, number>(); | ||
|
||
class C<T = number> {} | ||
new C<string>(); | ||
class D extends C<string> {} | ||
|
||
interface I<T = number> {} | ||
class Impl implements I<string> {} | ||
``` | ||
|
||
## Related to | ||
|
||
- TSLint: [use-default-type-parameter](https://palantir.github.io/tslint/rules/use-default-type-parameter) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
161 changes: 161 additions & 0 deletions
161
packages/eslint-plugin/src/rules/use-default-type-parameter.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,161 @@ | ||
import { TSESTree } from '@typescript-eslint/experimental-utils'; | ||
import * as tsutils from 'tsutils'; | ||
import 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 | ||
| ts.TypeReferenceNode | ||
| ts.ExpressionWithTypeArguments; | ||
|
||
type MessageIds = 'unnecessaryTypeParameter'; | ||
|
||
export default util.createRule<[], MessageIds>({ | ||
name: 'use-default-type-parameter', | ||
meta: { | ||
docs: { | ||
description: | ||
'Warns if an explicitly specified type argument is the default for that type parameter', | ||
category: 'Best Practices', | ||
recommended: false, | ||
}, | ||
fixable: 'code', | ||
messages: { | ||
unnecessaryTypeParameter: | ||
'This is the default value for this type parameter, so it can be omitted.', | ||
}, | ||
schema: [], | ||
type: 'suggestion', | ||
}, | ||
defaultOptions: [], | ||
create(context) { | ||
const parserServices = util.getParserServices(context); | ||
const checker = parserServices.program.getTypeChecker(); | ||
|
||
function checkTSArgsAndParameters( | ||
esParameters: TSESTree.TSTypeParameterInstantiation, | ||
{ typeArguments, typeParameters }: ArgsAndParams, | ||
): 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 param = typeParameters[i]; | ||
|
||
// TODO: would like checker.areTypesEquivalent. https://github.com/Microsoft/TypeScript/issues/13502 | ||
if ( | ||
param.default === undefined || | ||
param.default.getText() !== arg.getText() | ||
) { | ||
return; | ||
} | ||
|
||
context.report({ | ||
fix: fixer => | ||
fixer.removeRange( | ||
i === 0 | ||
? [typeArguments.pos - 1, typeArguments.end + 1] | ||
: [typeArguments[i - 1].end, arg.end], | ||
), | ||
messageId: 'unnecessaryTypeParameter', | ||
node: esParameters!.params[i], | ||
}); | ||
} | ||
|
||
return { | ||
TSTypeParameterInstantiation(node) { | ||
const parentDeclaration = parserServices.esTreeNodeToTSNodeMap.get( | ||
node.parent!, | ||
) as ExtendingClassLikeDeclaration | ParameterCapableTSNode; | ||
|
||
const expression = tsutils.isClassLikeDeclaration(parentDeclaration) | ||
? parentDeclaration.heritageClauses[0].types[0] | ||
: parentDeclaration; | ||
|
||
const argsAndParams = getArgsAndParameters(expression, checker); | ||
if (argsAndParams !== undefined) { | ||
checkTSArgsAndParameters(node, argsAndParams); | ||
} | ||
}, | ||
}; | ||
}, | ||
}); | ||
|
||
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, | ||
) { | ||
if (ts.isExpressionWithTypeArguments(node)) { | ||
return getTypeParametersFromType(node.expression, checker); | ||
} | ||
|
||
if (ts.isTypeReferenceNode(node)) { | ||
return getTypeParametersFromType(node.typeName, checker); | ||
} | ||
|
||
return getTypeParametersFromCall(node, checker); | ||
} | ||
|
||
function getTypeParametersFromType( | ||
type: ts.EntityName | ts.Expression | ts.ClassDeclaration, | ||
checker: ts.TypeChecker, | ||
): readonly ts.TypeParameterDeclaration[] | undefined { | ||
const sym = getAliasedSymbol(checker.getSymbolAtLocation(type)!, checker); | ||
if (sym === undefined || sym.declarations === undefined) { | ||
return undefined; | ||
} | ||
|
||
return findFirstResult(sym.declarations, decl => | ||
tsutils.isClassLikeDeclaration(decl) || | ||
ts.isTypeAliasDeclaration(decl) || | ||
ts.isInterfaceDeclaration(decl) | ||
? decl.typeParameters | ||
: undefined, | ||
); | ||
} | ||
|
||
function getTypeParametersFromCall( | ||
node: ts.CallExpression | ts.NewExpression, | ||
checker: ts.TypeChecker, | ||
): readonly ts.TypeParameterDeclaration[] | undefined { | ||
const sig = checker.getResolvedSignature(node); | ||
const sigDecl = sig === undefined ? undefined : sig.getDeclaration(); | ||
if (sigDecl === undefined) { | ||
return ts.isNewExpression(node) | ||
? getTypeParametersFromType(node.expression, checker) | ||
: undefined; | ||
} | ||
|
||
return sigDecl.typeParameters === undefined | ||
? undefined | ||
: sigDecl.typeParameters; | ||
JoshuaKGoldberg marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
function getAliasedSymbol( | ||
symbol: ts.Symbol, | ||
checker: ts.TypeChecker, | ||
): ts.Symbol | undefined { | ||
return tsutils.isSymbolFlagSet(symbol, ts.SymbolFlags.Alias) | ||
? checker.getAliasedSymbol(symbol) | ||
: symbol; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we just introduce https://github.com/runem/ts-simple-type#readme ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In theory, sure. Would you like me to do this here or in a separate PR?