Skip to content
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
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/eslint-plugin/README.md
Expand Up @@ -180,5 +180,6 @@ Then you should add `airbnb` (or `airbnb-base`) to your `extends` section of `.e
| [`@typescript-eslint/type-annotation-spacing`](./docs/rules/type-annotation-spacing.md) | Require consistent spacing around type annotations | :heavy_check_mark: | :wrench: | |
| [`@typescript-eslint/unbound-method`](./docs/rules/unbound-method.md) | Enforces unbound methods are called with their expected scope | | | :thought_balloon: |
| [`@typescript-eslint/unified-signatures`](./docs/rules/unified-signatures.md) | Warns for any two overloads that could be unified into one by using a union or an optional/rest parameter | | | |
| [`@typescript-eslint/use-default-type-parameter`](./docs/rules/use-default-type-parameter.md) | Warns if an explicitly specified type argument is the default for that type parameter | | :wrench: | :thought_balloon: |

<!-- end rule list -->
3 changes: 2 additions & 1 deletion packages/eslint-plugin/ROADMAP.md
Expand Up @@ -98,7 +98,7 @@
| [`triple-equals`] | 🌟 | [`eqeqeq`][eqeqeq] |
| [`typeof-compare`] | 🌟 | [`valid-typeof`][valid-typeof] |
| [`unnecessary-constructor`] | 🌟 | [`no-useless-constructor`][no-useless-constructor] |
| [`use-default-type-parameter`] | 🛑 | N/A |
| [`use-default-type-parameter`] | | [`@typescript-eslint/use-default-type-parameter`] |
| [`use-isnan`] | 🌟 | [`use-isnan`][use-isnan] |

<sup>[1]</sup> The ESLint rule also supports silencing with an extra set of parens (`if ((foo = bar)) {}`)<br>
Expand Down Expand Up @@ -615,6 +615,7 @@ Relevant plugins: [`chai-expect-keywords`](https://github.com/gavinaiken/eslint-
[`@typescript-eslint/no-unnecessary-qualifier`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-unnecessary-qualifier.md
[`@typescript-eslint/semi`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/semi.md
[`@typescript-eslint/no-floating-promises`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-floating-promises.md
[`@typescript-eslint/use-default-type-parameter`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/use-default-type-parameter.md

<!-- eslint-plugin-import -->

Expand Down
53 changes: 53 additions & 0 deletions packages/eslint-plugin/docs/rules/use-default-type-parameter.md
@@ -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)
3 changes: 2 additions & 1 deletion packages/eslint-plugin/src/configs/all.json
Expand Up @@ -66,6 +66,7 @@
"@typescript-eslint/semi": "error",
"@typescript-eslint/type-annotation-spacing": "error",
"@typescript-eslint/unbound-method": "error",
"@typescript-eslint/unified-signatures": "error"
"@typescript-eslint/unified-signatures": "error",
"@typescript-eslint/use-default-type-parameter": "error"
}
}
Expand Up @@ -17,7 +17,7 @@ export interface TreeValue {
* can easily be swapped out.
*/
export class BinarySearchTree {
private rbTree = createTree<TreeValue, number>();
private rbTree = createTree<TreeValue>();

/**
* Inserts an entry into the tree.
Expand Down
2 changes: 2 additions & 0 deletions packages/eslint-plugin/src/rules/index.ts
Expand Up @@ -55,6 +55,7 @@ import semi from './semi';
import typeAnnotationSpacing from './type-annotation-spacing';
import unboundMethod from './unbound-method';
import unifiedSignatures from './unified-signatures';
import useDefaultTypeParameter from './use-default-type-parameter';

export default {
'adjacent-overload-signatures': adjacentOverloadSignatures,
Expand Down Expand Up @@ -114,4 +115,5 @@ export default {
'type-annotation-spacing': typeAnnotationSpacing,
'unbound-method': unboundMethod,
'unified-signatures': unifiedSignatures,
'use-default-type-parameter': useDefaultTypeParameter,
};
159 changes: 159 additions & 0 deletions packages/eslint-plugin/src/rules/use-default-type-parameter.ts
@@ -0,0 +1,159 @@
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

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?

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;
}

function getAliasedSymbol(
symbol: ts.Symbol,
checker: ts.TypeChecker,
): ts.Symbol | undefined {
return tsutils.isSymbolFlagSet(symbol, ts.SymbolFlags.Alias)
? checker.getAliasedSymbol(symbol)
: symbol;
}
14 changes: 14 additions & 0 deletions packages/eslint-plugin/src/util/misc.ts
Expand Up @@ -84,6 +84,20 @@ export function arraysAreEqual<T>(
);
}

/** Returns the first non-`undefined` result. */
export function findFirstResult<T, U>(
inputs: T[],
getResult: (t: T) => U | undefined,
): U | undefined {
for (const element of inputs) {
const result = getResult(element);
if (result !== undefined) {
return result;
}
}
return undefined;
}

/**
* Gets a string name representation of the name of the given MethodDefinition
* or ClassProperty node, with handling for computed property names.
Expand Down