diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 642efc19068..9213a8d5273 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -143,6 +143,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int | [`@typescript-eslint/no-unnecessary-qualifier`](./docs/rules/no-unnecessary-qualifier.md) | Warns when a namespace qualifier is unnecessary | | :wrench: | :thought_balloon: | | [`@typescript-eslint/no-unnecessary-type-arguments`](./docs/rules/no-unnecessary-type-arguments.md) | Enforces that type arguments will not be used if not required | | :wrench: | :thought_balloon: | | [`@typescript-eslint/no-unnecessary-type-assertion`](./docs/rules/no-unnecessary-type-assertion.md) | Warns if a type assertion does not change the type of an expression | :heavy_check_mark: | :wrench: | :thought_balloon: | +| [`@typescript-eslint/no-unnecessary-type-constraint`](./docs/rules/no-unnecessary-type-constraint.md) | Disallows unnecessary constraints on generic types | | :wrench: | | | [`@typescript-eslint/no-unsafe-assignment`](./docs/rules/no-unsafe-assignment.md) | Disallows assigning any to variables and properties | :heavy_check_mark: | | :thought_balloon: | | [`@typescript-eslint/no-unsafe-call`](./docs/rules/no-unsafe-call.md) | Disallows calling an any type value | :heavy_check_mark: | | :thought_balloon: | | [`@typescript-eslint/no-unsafe-member-access`](./docs/rules/no-unsafe-member-access.md) | Disallows member access on any typed variables | :heavy_check_mark: | | :thought_balloon: | diff --git a/packages/eslint-plugin/docs/rules/no-unnecessary-type-constraint.md b/packages/eslint-plugin/docs/rules/no-unnecessary-type-constraint.md new file mode 100644 index 00000000000..8656ace566e --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-unnecessary-type-constraint.md @@ -0,0 +1,55 @@ +# Disallows unnecessary constraints on generic types (`no-unnecessary-type-constraint`) + +## Rule Details + +Type parameters (``) may be "constrained" with an `extends` keyword ([docs](https://www.typescriptlang.org/docs/handbook/generics.html#generic-constraints)). +When not provided, type parameters happen to default to: + +- As of TypeScript 3.9: `unknown` ([docs](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-9.html#type-parameters-that-extend-any-no-longer-act-as-any)) +- Before that, as of 3.5: `any` ([docs](https://devblogs.microsoft.com/typescript/announcing-typescript-3-5/#breaking-changes)) + +It is therefore redundant to `extend` from these types in later versions of TypeScript. + +Examples of **incorrect** code for this rule: + +```ts +interface FooAny {} +interface FooUnknown {} + +type BarAny = {}; +type BarUnknown = {}; + +class BazAny { + quxUnknown() {} +} + +class BazUnknown { + quxUnknown() {} +} + +const QuuxAny = () => {}; +const QuuxUnknown = () => {}; + +function QuuzAny() {} +function QuuzUnknown() {} +``` + +Examples of **correct** code for this rule: + +```ts +interface Foo {} + +type Bar = {}; + +class Baz { + qux { } +} + +const Quux = () => {}; + +function Quuz() {} +``` + +## When Not To Use It + +If you don't care about the specific styles of your type constraints, or never use them in the first place, then you will not need this rule. diff --git a/packages/eslint-plugin/src/configs/all.ts b/packages/eslint-plugin/src/configs/all.ts index 7806a034eb4..d847322d986 100644 --- a/packages/eslint-plugin/src/configs/all.ts +++ b/packages/eslint-plugin/src/configs/all.ts @@ -92,6 +92,7 @@ export = { '@typescript-eslint/no-type-alias': 'error', '@typescript-eslint/no-unnecessary-boolean-literal-compare': 'error', '@typescript-eslint/no-unnecessary-condition': 'error', + '@typescript-eslint/no-unnecessary-type-constraint': 'error', '@typescript-eslint/no-unnecessary-qualifier': 'error', '@typescript-eslint/no-unnecessary-type-arguments': 'error', '@typescript-eslint/no-unnecessary-type-assertion': 'error', diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 059596e4d86..a4540aa5788 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -67,6 +67,7 @@ import noUnnecessaryCondition from './no-unnecessary-condition'; import noUnnecessaryQualifier from './no-unnecessary-qualifier'; import noUnnecessaryTypeArguments from './no-unnecessary-type-arguments'; import noUnnecessaryTypeAssertion from './no-unnecessary-type-assertion'; +import noUnnecessaryTypeConstraint from './no-unnecessary-type-constraint'; import noUnsafeAssignment from './no-unsafe-assignment'; import noUnsafeCall from './no-unsafe-call'; import noUnsafeMemberAccess from './no-unsafe-member-access'; @@ -177,6 +178,7 @@ export default { 'no-unnecessary-qualifier': noUnnecessaryQualifier, 'no-unnecessary-type-arguments': noUnnecessaryTypeArguments, 'no-unnecessary-type-assertion': noUnnecessaryTypeAssertion, + 'no-unnecessary-type-constraint': noUnnecessaryTypeConstraint, 'no-unsafe-assignment': noUnsafeAssignment, 'no-unsafe-call': noUnsafeCall, 'no-unsafe-member-access': noUnsafeMemberAccess, diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-constraint.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-constraint.ts new file mode 100644 index 00000000000..6d5036c2d75 --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-constraint.ts @@ -0,0 +1,103 @@ +import { + AST_NODE_TYPES, + TSESTree, +} from '@typescript-eslint/experimental-utils'; +import * as semver from 'semver'; +import * as ts from 'typescript'; +import * as util from '../util'; + +type MakeRequired = Omit & + Required>; + +type TypeParameterWithConstraint = MakeRequired< + TSESTree.TSTypeParameter, + 'constraint' +>; + +const is3dot5 = semver.satisfies( + ts.version, + `>= 3.5.0 || >= 3.5.1-rc || >= 3.5.0-beta`, + { + includePrerelease: true, + }, +); + +const is3dot9 = + is3dot5 && + semver.satisfies(ts.version, `>= 3.9.0 || >= 3.9.1-rc || >= 3.9.0-beta`, { + includePrerelease: true, + }); + +export default util.createRule({ + name: 'no-unnecessary-type-constraint', + meta: { + docs: { + category: 'Best Practices', + description: 'Disallows unnecessary constraints on generic types', + recommended: false, + suggestion: true, + }, + fixable: 'code', + messages: { + unnecessaryConstraint: + 'Constraining the generic type `{{name}}` to `{{constraint}}` does nothing and is unnecessary.', + }, + schema: [], + type: 'suggestion', + }, + defaultOptions: [], + create(context) { + if (!is3dot5) { + return {}; + } + + // In theory, we could use the type checker for more advanced constraint types... + // ...but in practice, these types are rare, and likely not worth requiring type info. + // https://github.com/typescript-eslint/typescript-eslint/pull/2516#discussion_r495731858 + const unnecessaryConstraints = is3dot9 + ? new Map([ + [AST_NODE_TYPES.TSAnyKeyword, 'any'], + [AST_NODE_TYPES.TSUnknownKeyword, 'unknown'], + ]) + : new Map([[AST_NODE_TYPES.TSUnknownKeyword, 'unknown']]); + + const inJsx = context.getFilename().toLowerCase().endsWith('tsx'); + + const checkNode = ( + node: TypeParameterWithConstraint, + inArrowFunction: boolean, + ): void => { + const constraint = unnecessaryConstraints.get(node.constraint.type); + + if (constraint) { + context.report({ + data: { + constraint, + name: node.name.name, + }, + fix(fixer) { + return fixer.replaceTextRange( + [node.name.range[1], node.constraint.range[1]], + inArrowFunction && inJsx ? ',' : '', + ); + }, + messageId: 'unnecessaryConstraint', + node, + }); + } + }; + + return { + ':not(ArrowFunctionExpression) > TSTypeParameterDeclaration > TSTypeParameter[constraint]'( + node: TypeParameterWithConstraint, + ): void { + checkNode(node, false); + }, + 'ArrowFunctionExpression > TSTypeParameterDeclaration > TSTypeParameter[constraint]'( + node: TypeParameterWithConstraint, + ): void { + checkNode(node, true); + }, + }; + }, +}); diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-type-constraint.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-type-constraint.test.ts new file mode 100644 index 00000000000..c71821ffeb4 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-type-constraint.test.ts @@ -0,0 +1,230 @@ +import rule from '../../src/rules/no-unnecessary-type-constraint'; +import { RuleTester, noFormat } from '../RuleTester'; + +const ruleTester = new RuleTester({ + parserOptions: { + ecmaVersion: 2018, + }, + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('no-unnecessary-type-constraint', rule, { + valid: [ + 'function data() {}', + 'function data() {}', + 'function data() {}', + 'function data() {}', + 'function data() {}', + 'function data() {}', + ` +type TODO = any; +function data() {} + `, + 'const data = () => {};', + 'const data = () => {};', + 'const data = () => {};', + 'const data = () => {};', + 'const data = () => {};', + ], + invalid: [ + { + code: 'function data() {}', + errors: [ + { + data: { constraint: 'any', name: 'T' }, + messageId: 'unnecessaryConstraint', + endColumn: 28, + column: 15, + line: 1, + }, + ], + output: 'function data() {}', + }, + { + code: 'function data() {}', + errors: [ + { + data: { constraint: 'any', name: 'T' }, + messageId: 'unnecessaryConstraint', + endColumn: 28, + column: 15, + line: 1, + }, + ], + output: 'function data() {}', + }, + { + code: 'function data() {}', + errors: [ + { + data: { constraint: 'any', name: 'U' }, + messageId: 'unnecessaryConstraint', + endColumn: 31, + column: 18, + line: 1, + }, + ], + output: 'function data() {}', + }, + { + code: 'function data() {}', + errors: [ + { + data: { constraint: 'any', name: 'T' }, + messageId: 'unnecessaryConstraint', + endColumn: 28, + column: 15, + line: 1, + }, + ], + output: 'function data() {}', + }, + { + code: 'const data = () => {};', + errors: [ + { + data: { constraint: 'any', name: 'T' }, + messageId: 'unnecessaryConstraint', + endColumn: 28, + column: 15, + line: 1, + }, + ], + filename: 'react.tsx', + output: noFormat`const data = () => {};`, + }, + { + code: 'function data() {}', + errors: [ + { + data: { constraint: 'unknown', name: 'T' }, + messageId: 'unnecessaryConstraint', + endColumn: 32, + column: 15, + line: 1, + }, + ], + output: 'function data() {}', + }, + { + code: 'const data = () => {};', + errors: [ + { + data: { constraint: 'any', name: 'T' }, + messageId: 'unnecessaryConstraint', + endColumn: 28, + column: 15, + line: 1, + }, + ], + output: 'const data = () => {};', + }, + { + code: 'const data = () => {};', + errors: [ + { + data: { constraint: 'unknown', name: 'T' }, + messageId: 'unnecessaryConstraint', + endColumn: 32, + column: 15, + line: 1, + }, + ], + output: 'const data = () => {};', + }, + { + code: 'class Data {}', + errors: [ + { + data: { constraint: 'unknown', name: 'T' }, + messageId: 'unnecessaryConstraint', + endColumn: 29, + column: 12, + line: 1, + }, + ], + output: 'class Data {}', + }, + { + code: 'const Data = class {};', + errors: [ + { + data: { constraint: 'unknown', name: 'T' }, + messageId: 'unnecessaryConstraint', + endColumn: 37, + column: 20, + line: 1, + }, + ], + output: 'const Data = class {};', + }, + { + code: ` +class Data { + member() {} +} + `, + errors: [ + { + data: { constraint: 'unknown', name: 'T' }, + messageId: 'unnecessaryConstraint', + endColumn: 27, + column: 10, + line: 3, + }, + ], + output: ` +class Data { + member() {} +} + `, + }, + { + code: ` +const Data = class { + member() {} +}; + `, + errors: [ + { + data: { constraint: 'unknown', name: 'T' }, + messageId: 'unnecessaryConstraint', + endColumn: 27, + column: 10, + line: 3, + }, + ], + output: ` +const Data = class { + member() {} +}; + `, + }, + { + code: 'interface Data {}', + errors: [ + { + data: { constraint: 'unknown', name: 'T' }, + messageId: 'unnecessaryConstraint', + endColumn: 33, + column: 16, + line: 1, + }, + ], + output: 'interface Data {}', + }, + { + code: 'type Data = {};', + errors: [ + { + data: { constraint: 'unknown', name: 'T' }, + messageId: 'unnecessaryConstraint', + endColumn: 28, + column: 11, + line: 1, + }, + ], + output: 'type Data = {};', + }, + ], +});