From c32a72bec4232d1df46856b4c1a6798b9b6635e0 Mon Sep 17 00:00:00 2001 From: Benjamin Lichtman Date: Mon, 28 Jan 2019 13:46:38 -0800 Subject: [PATCH 1/7] feat(eslint-plugin): add no-unnecessary-type-assertion rule --- packages/eslint-plugin/README.md | 1 + packages/eslint-plugin/ROADMAP.md | 26 +-- .../rules/no-unnecessary-type-assertion.md | 57 ++++++ .../rules/no-unnecessary-type-assertion.js | 179 ++++++++++++++++++ .../rules/no-unnecessary-type-assertion.js | 113 +++++++++++ 5 files changed, 363 insertions(+), 13 deletions(-) create mode 100644 packages/eslint-plugin/docs/rules/no-unnecessary-type-assertion.md create mode 100644 packages/eslint-plugin/lib/rules/no-unnecessary-type-assertion.js create mode 100644 packages/eslint-plugin/tests/lib/rules/no-unnecessary-type-assertion.js 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..fcf91b041fe8 100644 --- a/packages/eslint-plugin/ROADMAP.md +++ b/packages/eslint-plugin/ROADMAP.md @@ -1,9 +1,9 @@ # Roadmap -✅ (27) = done -🌟 (79) = in ESLint core -🔌 (33) = in another plugin -🌓 (16) = implementations differ or ESLint version is missing functionality +✅ (27) = done +🌟 (79) = in ESLint core +🔌 (33) = in another plugin +🌓 (16) = implementations differ or ESLint version is missing functionality 🛑 (71) = 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/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 | @@ -96,7 +96,7 @@ | [`use-default-type-parameter`] | 🛑 | N/A | | [`use-isnan`] | 🌟 | [`use-isnan`][use-isnan] | -[1] The ESLint rule also supports silencing with an extra set of parens (`if ((foo = bar)) {}`) +[1] The ESLint rule also supports silencing with an extra set of parens (`if ((foo = bar)) {}`) [2] Missing private class member support. [`@typescript-eslint/no-unused-vars`] adds support for some TS-specific features. ### Maintainability @@ -120,7 +120,7 @@ | [`prefer-readonly`] | 🛑 | N/A | | [`trailing-comma`] | 🌓 | [`comma-dangle`][comma-dangle] or [Prettier] | -[1] Only warns when importing deprecated symbols +[1] Only warns when importing deprecated symbols [2] Missing support for blank-line-delimited sections ### Style @@ -179,7 +179,7 @@ | [`variable-name`] | 🌟 | [2] | | [`whitespace`] | 🔌 | Use [Prettier] | -[1] Recommended config: `["error", { blankLine: "always", prev: "*", next: "return" }]` +[1] Recommended config: `["error", { blankLine: "always", prev: "*", next: "return" }]` [2] [`camelcase`][camelcase], [`no-underscore-dangle`][no-underscore-dangle], [`id-blacklist`][id-blacklist], and/or [`id-match`][id-match] ## tslint-microsoft-contrib rules @@ -245,10 +245,10 @@ Relevant plugins: [`chai-expect-keywords`](https://github.com/gavinaiken/eslint- | `use-named-parameter` | 🛑 | N/A | | `use-simple-attributes` | 🛑 | N/A | -[1] Enforces blank lines both at the beginning and end of a block -[2] Recommended config: `["error", "ForInStatement"]` -[3] Recommended config: `["error", "declaration", { "allowArrowFunctions": true }]` -[4] Recommended config: `["error", { "terms": ["BUG", "HACK", "FIXME", "LATER", "LATER2", "TODO"], "location": "anywhere" }]` +[1] Enforces blank lines both at the beginning and end of a block +[2] Recommended config: `["error", "ForInStatement"]` +[3] Recommended config: `["error", "declaration", { "allowArrowFunctions": true }]` +[4] Recommended config: `["error", { "terms": ["BUG", "HACK", "FIXME", "LATER", "LATER2", "TODO"], "location": "anywhere" }]` [5] Does not check class fields. [insecure-random]: https://github.com/desktop/desktop/blob/master/eslint-rules/insecure-random.js @@ -310,7 +310,7 @@ Relevant plugins: [`chai-expect-keywords`](https://github.com/gavinaiken/eslint- | `react-a11y-titles` | 🛑 | N/A | | `react-anchor-blank-noopener` | 🛑 | N/A | -[1] TSLint rule is more strict +[1] TSLint rule is more strict [2] ESLint rule only reports for click handlers [prettier]: https://prettier.io 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..2675c48fad01 --- /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 a list of type names to ignore. + +For example, with `typescript/no-unnecessary-type-assertion: ["error", ["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..6014e97d3b04 --- /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 +//------------------------------------------------------------------------------ + +const FAILURE_STRING = + 'This assertion is unnecessary since it does not change the type of the expression.'; + +/** + * 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 iff 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) { + /** + * @type {ts.NonNullExpression} + */ + const originalNode = context.parserServices.esTreeNodeToTSNodeMap.get(node); + + if (!originalNode) { + return; + } + const type = checker.getTypeAtLocation(originalNode.expression); + + if (type === checker.getNonNullableType(type)) { + context.report({ + node, + message: FAILURE_STRING, + 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) { + /** + * @type {ts.AssertionExpression} + */ + const originalNode = context.parserServices.esTreeNodeToTSNodeMap.get(node); + + if (!originalNode) { + return; + } + + const typesToIgnore = context.options[0]; + + if ( + typesToIgnore && + 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, + message: FAILURE_STRING, + 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', + schema: [ + { + 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..1f36fce433ec --- /dev/null +++ b/packages/eslint-plugin/tests/lib/rules/no-unnecessary-type-assertion.js @@ -0,0 +1,113 @@ +/** + * @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' +}); + +const message = + 'This assertion is unnecessary since it does not change the type of the expression.'; + +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;', + { + code: ` +type Foo = number; +const foo = (3 + 5) as Foo;`, + options: [['Foo']] + }, + { + code: ` +type Foo = number; +const foo = (3 + 5);`, + options: [['Foo']] + } + ], + + invalid: [ + { + code: ` +const foo = 3; +const bar = foo!;`, + errors: [ + { + message, + line: 3, + column: 13 + } + ] + }, + { + code: ` +const foo = (3 + 5) as number;`, + errors: [ + { + message, + line: 2, + column: 13 + } + ] + }, + { + code: ` +const foo = (3 + 5);`, + errors: [ + { + message, + line: 2, + column: 13 + } + ] + }, + { + code: ` +type Foo = number; +const foo = (3 + 5) as Foo;`, + errors: [ + { + message, + line: 3, + column: 13 + } + ] + }, + { + code: ` +type Foo = number; +const foo= (3 + 5);`, + errors: [ + { + message, + line: 3, + column: 12 + } + ] + } + ] +}); From 5d540b26621a9c53b250212202bed8cd2234f971 Mon Sep 17 00:00:00 2001 From: Benjamin Lichtman Date: Mon, 28 Jan 2019 17:22:51 -0800 Subject: [PATCH 2/7] test: test couldBeTupleType --- .../tests/lib/rules/no-unnecessary-type-assertion.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) 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 index 1f36fce433ec..e3e7ef74c99d 100644 --- a/packages/eslint-plugin/tests/lib/rules/no-unnecessary-type-assertion.js +++ b/packages/eslint-plugin/tests/lib/rules/no-unnecessary-type-assertion.js @@ -36,6 +36,9 @@ ruleTester.run('no-unnecessary-type-assertion', rule, { 'const foo = 3;', 'const foo = <3>3;', 'const foo = 3 as 3;', + ` +type Tuple = [3, "hi", "bye"]; +const foo = ([3, "hi", "bye"]) as Tuple;`, { code: ` type Foo = number; @@ -100,12 +103,12 @@ const foo = (3 + 5) as Foo;`, { code: ` type Foo = number; -const foo= (3 + 5);`, +const foo = (3 + 5);`, errors: [ { message, line: 3, - column: 12 + column: 13 } ] } From 1fd19b6c19ad1093c0371f605a83169e95ed5d4d Mon Sep 17 00:00:00 2001 From: Benjamin Lichtman Date: Mon, 28 Jan 2019 17:31:31 -0800 Subject: [PATCH 3/7] test: convert to messageId --- .../lib/rules/no-unnecessary-type-assertion.js | 11 ++++++----- .../lib/rules/no-unnecessary-type-assertion.js | 13 +++++-------- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/packages/eslint-plugin/lib/rules/no-unnecessary-type-assertion.js b/packages/eslint-plugin/lib/rules/no-unnecessary-type-assertion.js index 6014e97d3b04..493b6bda98dd 100644 --- a/packages/eslint-plugin/lib/rules/no-unnecessary-type-assertion.js +++ b/packages/eslint-plugin/lib/rules/no-unnecessary-type-assertion.js @@ -15,9 +15,6 @@ const util = require('../util'); // Rule Definition //------------------------------------------------------------------------------ -const FAILURE_STRING = - 'This assertion is unnecessary since it does not change the type of the expression.'; - /** * 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 @@ -72,7 +69,7 @@ function checkNonNullAssertion(node, context, checker) { if (type === checker.getNonNullableType(type)) { context.report({ node, - message: FAILURE_STRING, + messageId: 'unnecessaryAssertion', fix(fixer) { return fixer.removeRange([ originalNode.expression.end, @@ -125,7 +122,7 @@ function verifyCast(node, context, checker) { if (uncastType === castType) { context.report({ node, - message: FAILURE_STRING, + messageId: 'unnecessaryAssertion', fix(fixer) { return originalNode.kind === ts.SyntaxKind.TypeAssertionExpression ? fixer.removeRange([ @@ -150,6 +147,10 @@ module.exports = { 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: 'array', 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 index e3e7ef74c99d..67ecb3059b30 100644 --- a/packages/eslint-plugin/tests/lib/rules/no-unnecessary-type-assertion.js +++ b/packages/eslint-plugin/tests/lib/rules/no-unnecessary-type-assertion.js @@ -27,9 +27,6 @@ const ruleTester = new RuleTester({ parser: '@typescript-eslint/parser' }); -const message = - 'This assertion is unnecessary since it does not change the type of the expression.'; - ruleTester.run('no-unnecessary-type-assertion', rule, { valid: [ 'const foo = 3 as number;', @@ -60,7 +57,7 @@ const foo = 3; const bar = foo!;`, errors: [ { - message, + messageId: 'unnecessaryAssertion', line: 3, column: 13 } @@ -71,7 +68,7 @@ const bar = foo!;`, const foo = (3 + 5) as number;`, errors: [ { - message, + messageId: 'unnecessaryAssertion', line: 2, column: 13 } @@ -82,7 +79,7 @@ const foo = (3 + 5) as number;`, const foo = (3 + 5);`, errors: [ { - message, + messageId: 'unnecessaryAssertion', line: 2, column: 13 } @@ -94,7 +91,7 @@ type Foo = number; const foo = (3 + 5) as Foo;`, errors: [ { - message, + messageId: 'unnecessaryAssertion', line: 3, column: 13 } @@ -106,7 +103,7 @@ type Foo = number; const foo = (3 + 5);`, errors: [ { - message, + messageId: 'unnecessaryAssertion', line: 3, column: 13 } From f4e471b4e72e19e6b6a418d1e0ddb8347637ea04 Mon Sep 17 00:00:00 2001 From: Benjamin Lichtman Date: Mon, 28 Jan 2019 17:43:03 -0800 Subject: [PATCH 4/7] fix: use object for options instead of array --- .../rules/no-unnecessary-type-assertion.md | 4 ++-- .../lib/rules/no-unnecessary-type-assertion.js | 18 ++++++++++++------ .../lib/rules/no-unnecessary-type-assertion.js | 4 ++-- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/no-unnecessary-type-assertion.md b/packages/eslint-plugin/docs/rules/no-unnecessary-type-assertion.md index 2675c48fad01..19f7dc005e96 100644 --- a/packages/eslint-plugin/docs/rules/no-unnecessary-type-assertion.md +++ b/packages/eslint-plugin/docs/rules/no-unnecessary-type-assertion.md @@ -39,9 +39,9 @@ const foo = 3 as number; ### Options -This rule optionally takes a list of type names to ignore. +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/no-unnecessary-type-assertion: ["error", ["Foo"]]`, the following is **correct** code": +For example, with `typescript/no-unnecessary-type-assertion: ["error", { typesToIgnore: ['Foo'] }]`, the following is **correct** code": ```ts type Foo = 3; diff --git a/packages/eslint-plugin/lib/rules/no-unnecessary-type-assertion.js b/packages/eslint-plugin/lib/rules/no-unnecessary-type-assertion.js index 493b6bda98dd..ab1e947d7e4c 100644 --- a/packages/eslint-plugin/lib/rules/no-unnecessary-type-assertion.js +++ b/packages/eslint-plugin/lib/rules/no-unnecessary-type-assertion.js @@ -96,11 +96,12 @@ function verifyCast(node, context, checker) { return; } - const typesToIgnore = context.options[0]; + const options = context.options[0]; if ( - typesToIgnore && - typesToIgnore.indexOf(originalNode.type.getText()) !== -1 + options && + options.typesToIgnore && + options.typesToIgnore.indexOf(originalNode.type.getText()) !== -1 ) { return; } @@ -153,9 +154,14 @@ module.exports = { }, schema: [ { - type: 'array', - items: { - type: 'string' + type: 'object', + properties: { + typesToIgnore: { + type: 'array', + items: { + type: 'string' + } + } } } ], 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 index 67ecb3059b30..c0b8901e8f5c 100644 --- a/packages/eslint-plugin/tests/lib/rules/no-unnecessary-type-assertion.js +++ b/packages/eslint-plugin/tests/lib/rules/no-unnecessary-type-assertion.js @@ -40,13 +40,13 @@ const foo = ([3, "hi", "bye"]) as Tuple;`, code: ` type Foo = number; const foo = (3 + 5) as Foo;`, - options: [['Foo']] + options: [{ typesToIgnore: ['Foo'] }] }, { code: ` type Foo = number; const foo = (3 + 5);`, - options: [['Foo']] + options: [{ typesToIgnore: ['Foo'] }] } ], From f65330a099f1ceeb53023beea159adf8ea0e9080 Mon Sep 17 00:00:00 2001 From: Benjamin Lichtman Date: Mon, 28 Jan 2019 17:52:24 -0800 Subject: [PATCH 5/7] chore: fix trimmed spaces --- packages/eslint-plugin/ROADMAP.md | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/packages/eslint-plugin/ROADMAP.md b/packages/eslint-plugin/ROADMAP.md index fcf91b041fe8..e5fad5a357f5 100644 --- a/packages/eslint-plugin/ROADMAP.md +++ b/packages/eslint-plugin/ROADMAP.md @@ -1,9 +1,9 @@ -# Roadmap +# Roadmap -✅ (27) = done -🌟 (79) = in ESLint core -🔌 (33) = in another plugin -🌓 (16) = implementations differ or ESLint version is missing functionality +✅ (27) = done +🌟 (79) = in ESLint core +🔌 (33) = in another plugin +🌓 (16) = implementations differ or ESLint version is missing functionality 🛑 (71) = unimplemented ## TSLint rules @@ -96,7 +96,7 @@ | [`use-default-type-parameter`] | 🛑 | N/A | | [`use-isnan`] | 🌟 | [`use-isnan`][use-isnan] | -[1] The ESLint rule also supports silencing with an extra set of parens (`if ((foo = bar)) {}`) +[1] The ESLint rule also supports silencing with an extra set of parens (`if ((foo = bar)) {}`) [2] Missing private class member support. [`@typescript-eslint/no-unused-vars`] adds support for some TS-specific features. ### Maintainability @@ -120,7 +120,7 @@ | [`prefer-readonly`] | 🛑 | N/A | | [`trailing-comma`] | 🌓 | [`comma-dangle`][comma-dangle] or [Prettier] | -[1] Only warns when importing deprecated symbols +[1] Only warns when importing deprecated symbols [2] Missing support for blank-line-delimited sections ### Style @@ -179,7 +179,7 @@ | [`variable-name`] | 🌟 | [2] | | [`whitespace`] | 🔌 | Use [Prettier] | -[1] Recommended config: `["error", { blankLine: "always", prev: "*", next: "return" }]` +[1] Recommended config: `["error", { blankLine: "always", prev: "*", next: "return" }]` [2] [`camelcase`][camelcase], [`no-underscore-dangle`][no-underscore-dangle], [`id-blacklist`][id-blacklist], and/or [`id-match`][id-match] ## tslint-microsoft-contrib rules @@ -245,10 +245,10 @@ Relevant plugins: [`chai-expect-keywords`](https://github.com/gavinaiken/eslint- | `use-named-parameter` | 🛑 | N/A | | `use-simple-attributes` | 🛑 | N/A | -[1] Enforces blank lines both at the beginning and end of a block -[2] Recommended config: `["error", "ForInStatement"]` -[3] Recommended config: `["error", "declaration", { "allowArrowFunctions": true }]` -[4] Recommended config: `["error", { "terms": ["BUG", "HACK", "FIXME", "LATER", "LATER2", "TODO"], "location": "anywhere" }]` +[1] Enforces blank lines both at the beginning and end of a block +[2] Recommended config: `["error", "ForInStatement"]` +[3] Recommended config: `["error", "declaration", { "allowArrowFunctions": true }]` +[4] Recommended config: `["error", { "terms": ["BUG", "HACK", "FIXME", "LATER", "LATER2", "TODO"], "location": "anywhere" }]` [5] Does not check class fields. [insecure-random]: https://github.com/desktop/desktop/blob/master/eslint-rules/insecure-random.js @@ -310,7 +310,7 @@ Relevant plugins: [`chai-expect-keywords`](https://github.com/gavinaiken/eslint- | `react-a11y-titles` | 🛑 | N/A | | `react-anchor-blank-noopener` | 🛑 | N/A | -[1] TSLint rule is more strict +[1] TSLint rule is more strict [2] ESLint rule only reports for click handlers [prettier]: https://prettier.io From d3bdc232db70feecc22d972d0c90edc25bc427a7 Mon Sep 17 00:00:00 2001 From: Benjamin Lichtman Date: Tue, 29 Jan 2019 10:04:20 -0800 Subject: [PATCH 6/7] fix: respond to CR --- packages/eslint-plugin/ROADMAP.md | 6 +++--- .../docs/rules/no-unnecessary-type-assertion.md | 2 +- .../lib/rules/no-unnecessary-type-assertion.js | 13 +++---------- 3 files changed, 7 insertions(+), 14 deletions(-) diff --git a/packages/eslint-plugin/ROADMAP.md b/packages/eslint-plugin/ROADMAP.md index e5fad5a357f5..378a3140ffb8 100644 --- a/packages/eslint-plugin/ROADMAP.md +++ b/packages/eslint-plugin/ROADMAP.md @@ -1,10 +1,10 @@ # 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`] | ✅ | [`typescript/no-unnecessary-type-assertion`] | +| [`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 index 19f7dc005e96..c41d0f2a4300 100644 --- a/packages/eslint-plugin/docs/rules/no-unnecessary-type-assertion.md +++ b/packages/eslint-plugin/docs/rules/no-unnecessary-type-assertion.md @@ -41,7 +41,7 @@ const foo = 3 as number; 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/no-unnecessary-type-assertion: ["error", { typesToIgnore: ['Foo'] }]`, the following is **correct** code": +For example, with `@typescript-eslint/no-unnecessary-type-assertion: ["error", { typesToIgnore: ['Foo'] }]`, the following is **correct** code": ```ts type Foo = 3; diff --git a/packages/eslint-plugin/lib/rules/no-unnecessary-type-assertion.js b/packages/eslint-plugin/lib/rules/no-unnecessary-type-assertion.js index ab1e947d7e4c..ed1747c0d95e 100644 --- a/packages/eslint-plugin/lib/rules/no-unnecessary-type-assertion.js +++ b/packages/eslint-plugin/lib/rules/no-unnecessary-type-assertion.js @@ -19,7 +19,7 @@ const util = require('../util'); * 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 iff type could be a tuple type + * @returns {boolean} true if type could be a tuple type */ function couldBeTupleType(type) { const properties = type.getProperties(); @@ -57,13 +57,10 @@ function couldBeTupleType(type) { */ function checkNonNullAssertion(node, context, checker) { /** + * Corresponding TSNode is guaranteed to be in map * @type {ts.NonNullExpression} */ const originalNode = context.parserServices.esTreeNodeToTSNodeMap.get(node); - - if (!originalNode) { - return; - } const type = checker.getTypeAtLocation(originalNode.expression); if (type === checker.getNonNullableType(type)) { @@ -88,14 +85,10 @@ function checkNonNullAssertion(node, context, checker) { */ function verifyCast(node, context, checker) { /** + * * Corresponding TSNode is guaranteed to be in map * @type {ts.AssertionExpression} */ const originalNode = context.parserServices.esTreeNodeToTSNodeMap.get(node); - - if (!originalNode) { - return; - } - const options = context.options[0]; if ( From c3b8f34ca6ea004e7712f4bce72aae657b259deb Mon Sep 17 00:00:00 2001 From: Benjamin Lichtman Date: Tue, 29 Jan 2019 10:05:38 -0800 Subject: [PATCH 7/7] test: improve code coverage --- .../tests/lib/rules/no-unnecessary-type-assertion.js | 9 +++++++++ 1 file changed, 9 insertions(+) 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 index c0b8901e8f5c..633f1a334330 100644 --- a/packages/eslint-plugin/tests/lib/rules/no-unnecessary-type-assertion.js +++ b/packages/eslint-plugin/tests/lib/rules/no-unnecessary-type-assertion.js @@ -36,6 +36,15 @@ ruleTester.run('no-unnecessary-type-assertion', rule, { ` 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;