diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 4f6f771d47c4..09b00a8e69a6 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -105,6 +105,7 @@ See [@typescript-eslint/parser's README.md](../parser/README.md) for more inform | [`@typescript-eslint/no-this-alias`](./docs/rules/no-this-alias.md) | Disallow aliasing `this` (`no-this-assignment` from TSLint) | | | | [`@typescript-eslint/no-triple-slash-reference`](./docs/rules/no-triple-slash-reference.md) | Disallow `/// ` comments (`no-reference` from TSLint) | :heavy_check_mark: | | | [`@typescript-eslint/no-type-alias`](./docs/rules/no-type-alias.md) | Disallow the use of type aliases (`interface-over-type-literal` from TSLint) | | | +| [`typescript/no-unnecessary-type-assertion`](./docs/rules/no-unnecessary-type-assertion.md) | Warns if a type assertion does not change the type of an expression (`no-unnecessary-type-assertion` from TSLint) | | :wrench: | | [`@typescript-eslint/no-unused-vars`](./docs/rules/no-unused-vars.md) | Disallow unused variables (`no-unused-variable` from TSLint) | :heavy_check_mark: | | | [`@typescript-eslint/no-use-before-define`](./docs/rules/no-use-before-define.md) | Disallow the use of variables before they are defined | :heavy_check_mark: | | | [`@typescript-eslint/no-var-requires`](./docs/rules/no-var-requires.md) | Disallows the use of require statements except in import statements (`no-var-requires` from TSLint) | :heavy_check_mark: | | diff --git a/packages/eslint-plugin/ROADMAP.md b/packages/eslint-plugin/ROADMAP.md index 1fce16992a9c..378a3140ffb8 100644 --- a/packages/eslint-plugin/ROADMAP.md +++ b/packages/eslint-plugin/ROADMAP.md @@ -1,10 +1,10 @@ -# Roadmap +# Roadmap -✅ (27) = done +✅ (28) = done 🌟 (79) = in ESLint core 🔌 (33) = in another plugin 🌓 (16) = implementations differ or ESLint version is missing functionality -🛑 (71) = unimplemented +🛑 (70) = unimplemented ## TSLint rules @@ -26,7 +26,7 @@ | [`no-non-null-assertion`] | ✅ | [`@typescript-eslint/no-non-null-assertion`] | | [`no-parameter-reassignment`] | ✅ | [`no-param-reassign`][no-param-reassign] | | [`no-reference`] | ✅ | [`@typescript-eslint/no-triple-slash-reference`] | -| [`no-unnecessary-type-assertion`] | 🛑 | N/A | +| [`no-unnecessary-type-assertion`] | ✅ | [`@typescript-eslint/no-unnecessary-type-assertion`] | | [`no-var-requires`] | ✅ | [`@typescript-eslint/no-var-requires`] | | [`only-arrow-functions`] | 🔌 | [`prefer-arrow/prefer-arrow-functions`] | | [`prefer-for-of`] | 🛑 | N/A | diff --git a/packages/eslint-plugin/docs/rules/no-unnecessary-type-assertion.md b/packages/eslint-plugin/docs/rules/no-unnecessary-type-assertion.md new file mode 100644 index 000000000000..c41d0f2a4300 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-unnecessary-type-assertion.md @@ -0,0 +1,57 @@ +# Warns if a type assertion does not change the type of an expression (no-unnecessary-type-assertion) + +This rule prohibits using a type assertion that does not change the type of an expression. + +## Rule Details + +This rule aims to prevent unnecessary type assertions. + +Examples of **incorrect** code for this rule: + +```ts +const foo = 3; +const bar = foo!; +``` + +```ts +const foo = <3>3; +``` + +```ts +type Foo = 3; +const foo = 3; +``` + +```ts +type Foo = 3; +const foo = 3 as Foo; +``` + +Examples of **correct** code for this rule: + +```ts +const foo = 3; +``` + +```ts +const foo = 3 as number; +``` + +### Options + +This rule optionally takes an object with a single property `typesToIgnore`, which can be set to a list of type names to ignore. + +For example, with `@typescript-eslint/no-unnecessary-type-assertion: ["error", { typesToIgnore: ['Foo'] }]`, the following is **correct** code": + +```ts +type Foo = 3; +const foo: Foo = 3; +``` + +## When Not To Use It + +If you don't care about having no-op type assertions in your code, then you can turn off this rule. + +## Related to + +- TSLint: ['no-unnecessary-type-assertion`](https://palantir.github.io/tslint/rules/no-unnecessary-type-assertion/) diff --git a/packages/eslint-plugin/lib/rules/no-unnecessary-type-assertion.js b/packages/eslint-plugin/lib/rules/no-unnecessary-type-assertion.js new file mode 100644 index 000000000000..ed1747c0d95e --- /dev/null +++ b/packages/eslint-plugin/lib/rules/no-unnecessary-type-assertion.js @@ -0,0 +1,179 @@ +/** + * @fileoverview Rule to warn if a type assertion does not change the type of an expression + * @author Benjamin Lichtman + */ + +'use strict'; +const tsutils = require('tsutils'); +const ts = require('typescript'); +const util = require('../util'); + +/** @typedef {import("estree").Node} Node */ +/** @typedef {import("eslint").Rule.RuleContext} Context */ + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ + +/** + * Sometimes tuple types don't have ObjectFlags.Tuple set, like when they're being matched against an inferred type. + * So, in addition, check if there are integer properties 0..n and no other numeric keys + * @param {ts.ObjectType} type type + * @returns {boolean} true if type could be a tuple type + */ +function couldBeTupleType(type) { + const properties = type.getProperties(); + + if (properties.length === 0) { + return false; + } + let i = 0; + + for (; i < properties.length; ++i) { + const name = properties[i].name; + + if (String(i) !== name) { + if (i === 0) { + // if there are no integer properties, this is not a tuple + return false; + } + break; + } + } + for (; i < properties.length; ++i) { + if (String(+properties[i].name) === properties[i].name) { + return false; // if there are any other numeric properties, this is not a tuple + } + } + return true; +} + +/** + * + * @param {Node} node node being linted + * @param {Context} context linting context + * @param {ts.TypeChecker} checker TypeScript typechecker + * @returns {void} + */ +function checkNonNullAssertion(node, context, checker) { + /** + * Corresponding TSNode is guaranteed to be in map + * @type {ts.NonNullExpression} + */ + const originalNode = context.parserServices.esTreeNodeToTSNodeMap.get(node); + const type = checker.getTypeAtLocation(originalNode.expression); + + if (type === checker.getNonNullableType(type)) { + context.report({ + node, + messageId: 'unnecessaryAssertion', + fix(fixer) { + return fixer.removeRange([ + originalNode.expression.end, + originalNode.end + ]); + } + }); + } +} + +/** + * @param {Node} node node being linted + * @param {Context} context linting context + * @param {ts.TypeChecker} checker TypeScript typechecker + * @returns {void} + */ +function verifyCast(node, context, checker) { + /** + * * Corresponding TSNode is guaranteed to be in map + * @type {ts.AssertionExpression} + */ + const originalNode = context.parserServices.esTreeNodeToTSNodeMap.get(node); + const options = context.options[0]; + + if ( + options && + options.typesToIgnore && + options.typesToIgnore.indexOf(originalNode.type.getText()) !== -1 + ) { + return; + } + const castType = checker.getTypeAtLocation(originalNode); + + if ( + tsutils.isTypeFlagSet(castType, ts.TypeFlags.Literal) || + (tsutils.isObjectType(castType) && + (tsutils.isObjectFlagSet(castType, ts.ObjectFlags.Tuple) || + couldBeTupleType(castType))) + ) { + // It's not always safe to remove a cast to a literal type or tuple + // type, as those types are sometimes widened without the cast. + return; + } + + const uncastType = checker.getTypeAtLocation(originalNode.expression); + + if (uncastType === castType) { + context.report({ + node, + messageId: 'unnecessaryAssertion', + fix(fixer) { + return originalNode.kind === ts.SyntaxKind.TypeAssertionExpression + ? fixer.removeRange([ + originalNode.getStart(), + originalNode.expression.getStart() + ]) + : fixer.removeRange([originalNode.expression.end, originalNode.end]); + } + }); + } +} + +/** @type {import("eslint").Rule.RuleModule} */ +module.exports = { + meta: { + docs: { + description: + 'Warns if a type assertion does not change the type of an expression', + category: 'TypeScript-specific', + recommended: false, + extraDescription: [util.tslintRule('no-unnecessary-type-assertion')], + url: util.metaDocsUrl('no-unnecessary-type-assertion') + }, + fixable: 'code', + messages: { + unnecessaryAssertion: + 'This assertion is unnecessary since it does not change the type of the expression.' + }, + schema: [ + { + type: 'object', + properties: { + typesToIgnore: { + type: 'array', + items: { + type: 'string' + } + } + } + } + ], + type: 'suggestion' + }, + + create(context) { + const checker = util.getParserServices(context).program.getTypeChecker(); + + return { + TSNonNullExpression(node) { + checkNonNullAssertion(node, context, checker); + }, + TSTypeAssertion(node) { + verifyCast(node, context, checker); + }, + TSAsExpression(node) { + verifyCast(node, context, checker); + } + }; + } +}; diff --git a/packages/eslint-plugin/tests/lib/rules/no-unnecessary-type-assertion.js b/packages/eslint-plugin/tests/lib/rules/no-unnecessary-type-assertion.js new file mode 100644 index 000000000000..633f1a334330 --- /dev/null +++ b/packages/eslint-plugin/tests/lib/rules/no-unnecessary-type-assertion.js @@ -0,0 +1,122 @@ +/** + * @fileoverview Warns if a type assertion does not change the type of an expression. + * @author Benjamin Lichtman + */ +'use strict'; + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +const rule = require('../../../lib/rules/no-unnecessary-type-assertion'), + RuleTester = require('eslint').RuleTester, + path = require('path'); + +//------------------------------------------------------------------------------ +// Tests +//------------------------------------------------------------------------------ + +const rootDir = path.join(process.cwd(), 'tests/fixtures'); +const parserOptions = { + ecmaVersion: 2015, + tsconfigRootDir: rootDir, + project: './tsconfig.json' +}; +const ruleTester = new RuleTester({ + parserOptions, + parser: '@typescript-eslint/parser' +}); + +ruleTester.run('no-unnecessary-type-assertion', rule, { + valid: [ + 'const foo = 3 as number;', + 'const foo = 3;', + 'const foo = <3>3;', + 'const foo = 3 as 3;', + ` +type Tuple = [3, "hi", "bye"]; +const foo = ([3, "hi", "bye"]) as Tuple;`, + ` +type PossibleTuple = {}; +const foo = ({}) as PossibleTuple;`, + ` +type PossibleTuple = { hello: "hello" }; +const foo = ({ hello: "hello" }) as PossibleTuple;`, + ` +type PossibleTuple = { 0: "hello", 5: "hello" }; +const foo = ({ 0: "hello", 5: "hello" }) as PossibleTuple;`, + { + code: ` +type Foo = number; +const foo = (3 + 5) as Foo;`, + options: [{ typesToIgnore: ['Foo'] }] + }, + { + code: ` +type Foo = number; +const foo = (3 + 5);`, + options: [{ typesToIgnore: ['Foo'] }] + } + ], + + invalid: [ + { + code: ` +const foo = 3; +const bar = foo!;`, + errors: [ + { + messageId: 'unnecessaryAssertion', + line: 3, + column: 13 + } + ] + }, + { + code: ` +const foo = (3 + 5) as number;`, + errors: [ + { + messageId: 'unnecessaryAssertion', + line: 2, + column: 13 + } + ] + }, + { + code: ` +const foo = (3 + 5);`, + errors: [ + { + messageId: 'unnecessaryAssertion', + line: 2, + column: 13 + } + ] + }, + { + code: ` +type Foo = number; +const foo = (3 + 5) as Foo;`, + errors: [ + { + messageId: 'unnecessaryAssertion', + line: 3, + column: 13 + } + ] + }, + { + code: ` +type Foo = number; +const foo = (3 + 5);`, + errors: [ + { + messageId: 'unnecessaryAssertion', + line: 3, + column: 13 + } + ] + } + ] +});