From 2c1f6acb6fe96839b2d0841ca8c8a134e630d0cd Mon Sep 17 00:00:00 2001 From: Benjamin Lichtman Date: Thu, 7 Feb 2019 10:01:08 -0800 Subject: [PATCH 1/5] fix(typescript-estree, eslint-plugin): stop adding PEs to node maps --- .../rules/no-unnecessary-type-assertion.js | 240 +++++++++--------- .../rules/no-unnecessary-type-assertion.js | 12 + packages/typescript-estree/src/convert.ts | 7 +- .../semanticInfo/parenthesized-expression.ts | 1 + .../tests/lib/semanticInfo.ts | 17 ++ 5 files changed, 162 insertions(+), 115 deletions(-) create mode 100644 packages/typescript-estree/tests/fixtures/semanticInfo/parenthesized-expression.ts 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 ed1747c0d95e..42afb06dcdc8 100644 --- a/packages/eslint-plugin/lib/rules/no-unnecessary-type-assertion.js +++ b/packages/eslint-plugin/lib/rules/no-unnecessary-type-assertion.js @@ -15,120 +15,6 @@ const util = require('../util'); // 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: { @@ -162,6 +48,132 @@ module.exports = { }, create(context) { + const sourceCode = context.getSourceCode(); + + /** + * 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) { + const options = context.options[0]; + + if ( + options && + options.typesToIgnore && + options.typesToIgnore.indexOf( + sourceCode.getText(node.typeAnnotation) + ) !== -1 + ) { + return; + } + + /** + * Corresponding TSNode is guaranteed to be in map + * @type {ts.AssertionExpression} + */ + const originalNode = context.parserServices.esTreeNodeToTSNodeMap.get( + node + ); + 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 + ]); + } + }); + } + } + const checker = util.getParserServices(context).program.getTypeChecker(); return { 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 633f1a334330..8795cff89c51 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 @@ -51,6 +51,18 @@ type Foo = number; const foo = (3 + 5) as Foo;`, options: [{ typesToIgnore: ['Foo'] }] }, + { + code: `const foo = (3 + 5) as any;`, + options: [{ typesToIgnore: ['any'] }] + }, + { + code: `((Syntax as any).ArrayExpression = 'foo')`, + options: [{ typesToIgnore: ['any'] }] + }, + { + code: `const foo = (3 + 5) as string;`, + options: [{ typesToIgnore: ['string'] }] + }, { code: ` type Foo = number; diff --git a/packages/typescript-estree/src/convert.ts b/packages/typescript-estree/src/convert.ts index bf121fa01bb0..bb74ad99e8c0 100644 --- a/packages/typescript-estree/src/convert.ts +++ b/packages/typescript-estree/src/convert.ts @@ -119,7 +119,12 @@ export class Converter { if (result && this.options.shouldProvideParserServices) { this.tsNodeToESTreeNodeMap.set(node, result); - this.esTreeNodeToTSNodeMap.set(result, node); + if (node.kind !== SyntaxKind.ParenthesizedExpression) { + // Parenthesized expressions do not have individual nodes in ESTree. + // Therefore, result.type will never "match" node.kind if it is a ParenthesizedExpression + // and furthermore, will overwrite the "matching" node + this.esTreeNodeToTSNodeMap.set(result, node); + } } this.inTypeMode = typeMode; diff --git a/packages/typescript-estree/tests/fixtures/semanticInfo/parenthesized-expression.ts b/packages/typescript-estree/tests/fixtures/semanticInfo/parenthesized-expression.ts new file mode 100644 index 000000000000..700db96574b8 --- /dev/null +++ b/packages/typescript-estree/tests/fixtures/semanticInfo/parenthesized-expression.ts @@ -0,0 +1 @@ +const y = (3 + 5); diff --git a/packages/typescript-estree/tests/lib/semanticInfo.ts b/packages/typescript-estree/tests/lib/semanticInfo.ts index 531ab1a34d54..d46f18fd648a 100644 --- a/packages/typescript-estree/tests/lib/semanticInfo.ts +++ b/packages/typescript-estree/tests/lib/semanticInfo.ts @@ -18,6 +18,7 @@ import { parseCodeAndGenerateServices } from '../../tools/test-utils'; import { parseAndGenerateServices } from '../../src/parser'; +import { VariableDeclaration } from '../../src/typedefs'; //------------------------------------------------------------------------------ // Setup @@ -101,6 +102,22 @@ describe('semanticInfo', () => { testIsolatedFile(parseResult); }); + it('parenthesized-expression tests', () => { + const fileName = resolve(FIXTURES_DIR, 'parenthesized-expression.ts'); + const parseResult = parseCodeAndGenerateServices( + readFileSync(fileName, 'utf8'), + createOptions(fileName) + ); + + expect(parseResult).toHaveProperty('services.esTreeNodeToTSNodeMap'); + const binaryExpression = (parseResult.ast.body[0] as VariableDeclaration) + .declarations[0].init!; + const tsBinaryExpression = parseResult.services.esTreeNodeToTSNodeMap!.get( + binaryExpression + ); + expect(tsBinaryExpression.kind).toEqual(ts.SyntaxKind.BinaryExpression); + }); + it('imported-file tests', () => { const fileName = resolve(FIXTURES_DIR, 'import-file.src.ts'); const parseResult = parseCodeAndGenerateServices( From b807916af60e7dfd16f90fe438b63522e6e7f688 Mon Sep 17 00:00:00 2001 From: Benjamin Lichtman Date: Thu, 7 Feb 2019 10:11:43 -0800 Subject: [PATCH 2/5] refactor(eslint-plugin): move only verifyCast into create --- .../rules/no-unnecessary-type-assertion.js | 126 +++++++++--------- 1 file changed, 62 insertions(+), 64 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 42afb06dcdc8..a5a4066a916a 100644 --- a/packages/eslint-plugin/lib/rules/no-unnecessary-type-assertion.js +++ b/packages/eslint-plugin/lib/rules/no-unnecessary-type-assertion.js @@ -15,6 +15,68 @@ const util = require('../util'); // 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 + ]); + } + }); + } +} + /** @type {import("eslint").Rule.RuleModule} */ module.exports = { meta: { @@ -50,70 +112,6 @@ module.exports = { create(context) { const sourceCode = context.getSourceCode(); - /** - * 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 From 8f0b44e706164ec5a4cebc6e66c2b52173fb4db8 Mon Sep 17 00:00:00 2001 From: Benjamin Lichtman Date: Thu, 7 Feb 2019 15:47:17 -0800 Subject: [PATCH 3/5] fix(typescript-estree): don't add computedpropertyname to map --- packages/typescript-estree/src/convert.ts | 9 +- .../non-existent-estree-nodes.src.ts | 5 + .../semanticInfo/parenthesized-expression.ts | 1 - .../lib/__snapshots__/semanticInfo.ts.snap | 607 ++++++++++++++++++ .../tests/lib/semanticInfo.ts | 17 +- 5 files changed, 632 insertions(+), 7 deletions(-) create mode 100644 packages/typescript-estree/tests/fixtures/semanticInfo/non-existent-estree-nodes.src.ts delete mode 100644 packages/typescript-estree/tests/fixtures/semanticInfo/parenthesized-expression.ts diff --git a/packages/typescript-estree/src/convert.ts b/packages/typescript-estree/src/convert.ts index bb74ad99e8c0..9f14bbcc2489 100644 --- a/packages/typescript-estree/src/convert.ts +++ b/packages/typescript-estree/src/convert.ts @@ -119,10 +119,13 @@ export class Converter { if (result && this.options.shouldProvideParserServices) { this.tsNodeToESTreeNodeMap.set(node, result); - if (node.kind !== SyntaxKind.ParenthesizedExpression) { - // Parenthesized expressions do not have individual nodes in ESTree. + if ( + node.kind !== SyntaxKind.ParenthesizedExpression && + node.kind !== SyntaxKind.ComputedPropertyName + ) { + // Parenthesized expressions and computed property names do not have individual nodes in ESTree. // Therefore, result.type will never "match" node.kind if it is a ParenthesizedExpression - // and furthermore, will overwrite the "matching" node + // or a ComputedPropertyName and, furthermore, will overwrite the "matching" node this.esTreeNodeToTSNodeMap.set(result, node); } } diff --git a/packages/typescript-estree/tests/fixtures/semanticInfo/non-existent-estree-nodes.src.ts b/packages/typescript-estree/tests/fixtures/semanticInfo/non-existent-estree-nodes.src.ts new file mode 100644 index 000000000000..885ee45c7b60 --- /dev/null +++ b/packages/typescript-estree/tests/fixtures/semanticInfo/non-existent-estree-nodes.src.ts @@ -0,0 +1,5 @@ +const y = (3 + 5); + +class Bar { + ['test']: string; +} diff --git a/packages/typescript-estree/tests/fixtures/semanticInfo/parenthesized-expression.ts b/packages/typescript-estree/tests/fixtures/semanticInfo/parenthesized-expression.ts deleted file mode 100644 index 700db96574b8..000000000000 --- a/packages/typescript-estree/tests/fixtures/semanticInfo/parenthesized-expression.ts +++ /dev/null @@ -1 +0,0 @@ -const y = (3 + 5); diff --git a/packages/typescript-estree/tests/lib/__snapshots__/semanticInfo.ts.snap b/packages/typescript-estree/tests/lib/__snapshots__/semanticInfo.ts.snap index 5af1b41ba261..82849fec5363 100644 --- a/packages/typescript-estree/tests/lib/__snapshots__/semanticInfo.ts.snap +++ b/packages/typescript-estree/tests/lib/__snapshots__/semanticInfo.ts.snap @@ -1133,4 +1133,611 @@ Object { } `; +exports[`semanticInfo fixtures/non-existent-estree-nodes.src 1`] = ` +Object { + "body": Array [ + Object { + "declarations": Array [ + Object { + "id": Object { + "loc": Object { + "end": Object { + "column": 7, + "line": 1, + }, + "start": Object { + "column": 6, + "line": 1, + }, + }, + "name": "y", + "range": Array [ + 6, + 7, + ], + "type": "Identifier", + }, + "init": Object { + "left": Object { + "loc": Object { + "end": Object { + "column": 12, + "line": 1, + }, + "start": Object { + "column": 11, + "line": 1, + }, + }, + "range": Array [ + 11, + 12, + ], + "raw": "3", + "type": "Literal", + "value": 3, + }, + "loc": Object { + "end": Object { + "column": 16, + "line": 1, + }, + "start": Object { + "column": 11, + "line": 1, + }, + }, + "operator": "+", + "range": Array [ + 11, + 16, + ], + "right": Object { + "loc": Object { + "end": Object { + "column": 16, + "line": 1, + }, + "start": Object { + "column": 15, + "line": 1, + }, + }, + "range": Array [ + 15, + 16, + ], + "raw": "5", + "type": "Literal", + "value": 5, + }, + "type": "BinaryExpression", + }, + "loc": Object { + "end": Object { + "column": 17, + "line": 1, + }, + "start": Object { + "column": 6, + "line": 1, + }, + }, + "range": Array [ + 6, + 17, + ], + "type": "VariableDeclarator", + }, + ], + "kind": "const", + "loc": Object { + "end": Object { + "column": 18, + "line": 1, + }, + "start": Object { + "column": 0, + "line": 1, + }, + }, + "range": Array [ + 0, + 18, + ], + "type": "VariableDeclaration", + }, + Object { + "body": Object { + "body": Array [ + Object { + "computed": true, + "key": Object { + "loc": Object { + "end": Object { + "column": 9, + "line": 4, + }, + "start": Object { + "column": 3, + "line": 4, + }, + }, + "range": Array [ + 35, + 41, + ], + "raw": "'test'", + "type": "Literal", + "value": "test", + }, + "loc": Object { + "end": Object { + "column": 19, + "line": 4, + }, + "start": Object { + "column": 2, + "line": 4, + }, + }, + "range": Array [ + 34, + 51, + ], + "static": false, + "type": "ClassProperty", + "typeAnnotation": Object { + "loc": Object { + "end": Object { + "column": 18, + "line": 4, + }, + "start": Object { + "column": 10, + "line": 4, + }, + }, + "range": Array [ + 42, + 50, + ], + "type": "TSTypeAnnotation", + "typeAnnotation": Object { + "loc": Object { + "end": Object { + "column": 18, + "line": 4, + }, + "start": Object { + "column": 12, + "line": 4, + }, + }, + "range": Array [ + 44, + 50, + ], + "type": "TSStringKeyword", + }, + }, + "value": null, + }, + ], + "loc": Object { + "end": Object { + "column": 1, + "line": 5, + }, + "start": Object { + "column": 10, + "line": 3, + }, + }, + "range": Array [ + 30, + 54, + ], + "type": "ClassBody", + }, + "id": Object { + "loc": Object { + "end": Object { + "column": 9, + "line": 3, + }, + "start": Object { + "column": 6, + "line": 3, + }, + }, + "name": "Bar", + "range": Array [ + 26, + 29, + ], + "type": "Identifier", + }, + "loc": Object { + "end": Object { + "column": 1, + "line": 5, + }, + "start": Object { + "column": 0, + "line": 3, + }, + }, + "range": Array [ + 20, + 54, + ], + "superClass": null, + "type": "ClassDeclaration", + }, + ], + "comments": Array [], + "loc": Object { + "end": Object { + "column": 0, + "line": 6, + }, + "start": Object { + "column": 0, + "line": 1, + }, + }, + "range": Array [ + 0, + 55, + ], + "sourceType": "script", + "tokens": Array [ + Object { + "loc": Object { + "end": Object { + "column": 5, + "line": 1, + }, + "start": Object { + "column": 0, + "line": 1, + }, + }, + "range": Array [ + 0, + 5, + ], + "type": "Keyword", + "value": "const", + }, + Object { + "loc": Object { + "end": Object { + "column": 7, + "line": 1, + }, + "start": Object { + "column": 6, + "line": 1, + }, + }, + "range": Array [ + 6, + 7, + ], + "type": "Identifier", + "value": "y", + }, + Object { + "loc": Object { + "end": Object { + "column": 9, + "line": 1, + }, + "start": Object { + "column": 8, + "line": 1, + }, + }, + "range": Array [ + 8, + 9, + ], + "type": "Punctuator", + "value": "=", + }, + Object { + "loc": Object { + "end": Object { + "column": 11, + "line": 1, + }, + "start": Object { + "column": 10, + "line": 1, + }, + }, + "range": Array [ + 10, + 11, + ], + "type": "Punctuator", + "value": "(", + }, + Object { + "loc": Object { + "end": Object { + "column": 12, + "line": 1, + }, + "start": Object { + "column": 11, + "line": 1, + }, + }, + "range": Array [ + 11, + 12, + ], + "type": "Numeric", + "value": "3", + }, + Object { + "loc": Object { + "end": Object { + "column": 14, + "line": 1, + }, + "start": Object { + "column": 13, + "line": 1, + }, + }, + "range": Array [ + 13, + 14, + ], + "type": "Punctuator", + "value": "+", + }, + Object { + "loc": Object { + "end": Object { + "column": 16, + "line": 1, + }, + "start": Object { + "column": 15, + "line": 1, + }, + }, + "range": Array [ + 15, + 16, + ], + "type": "Numeric", + "value": "5", + }, + Object { + "loc": Object { + "end": Object { + "column": 17, + "line": 1, + }, + "start": Object { + "column": 16, + "line": 1, + }, + }, + "range": Array [ + 16, + 17, + ], + "type": "Punctuator", + "value": ")", + }, + Object { + "loc": Object { + "end": Object { + "column": 18, + "line": 1, + }, + "start": Object { + "column": 17, + "line": 1, + }, + }, + "range": Array [ + 17, + 18, + ], + "type": "Punctuator", + "value": ";", + }, + Object { + "loc": Object { + "end": Object { + "column": 5, + "line": 3, + }, + "start": Object { + "column": 0, + "line": 3, + }, + }, + "range": Array [ + 20, + 25, + ], + "type": "Keyword", + "value": "class", + }, + Object { + "loc": Object { + "end": Object { + "column": 9, + "line": 3, + }, + "start": Object { + "column": 6, + "line": 3, + }, + }, + "range": Array [ + 26, + 29, + ], + "type": "Identifier", + "value": "Bar", + }, + Object { + "loc": Object { + "end": Object { + "column": 11, + "line": 3, + }, + "start": Object { + "column": 10, + "line": 3, + }, + }, + "range": Array [ + 30, + 31, + ], + "type": "Punctuator", + "value": "{", + }, + Object { + "loc": Object { + "end": Object { + "column": 3, + "line": 4, + }, + "start": Object { + "column": 2, + "line": 4, + }, + }, + "range": Array [ + 34, + 35, + ], + "type": "Punctuator", + "value": "[", + }, + Object { + "loc": Object { + "end": Object { + "column": 9, + "line": 4, + }, + "start": Object { + "column": 3, + "line": 4, + }, + }, + "range": Array [ + 35, + 41, + ], + "type": "String", + "value": "'test'", + }, + Object { + "loc": Object { + "end": Object { + "column": 10, + "line": 4, + }, + "start": Object { + "column": 9, + "line": 4, + }, + }, + "range": Array [ + 41, + 42, + ], + "type": "Punctuator", + "value": "]", + }, + Object { + "loc": Object { + "end": Object { + "column": 11, + "line": 4, + }, + "start": Object { + "column": 10, + "line": 4, + }, + }, + "range": Array [ + 42, + 43, + ], + "type": "Punctuator", + "value": ":", + }, + Object { + "loc": Object { + "end": Object { + "column": 18, + "line": 4, + }, + "start": Object { + "column": 12, + "line": 4, + }, + }, + "range": Array [ + 44, + 50, + ], + "type": "Identifier", + "value": "string", + }, + Object { + "loc": Object { + "end": Object { + "column": 19, + "line": 4, + }, + "start": Object { + "column": 18, + "line": 4, + }, + }, + "range": Array [ + 50, + 51, + ], + "type": "Punctuator", + "value": ";", + }, + Object { + "loc": Object { + "end": Object { + "column": 1, + "line": 5, + }, + "start": Object { + "column": 0, + "line": 5, + }, + }, + "range": Array [ + 53, + 54, + ], + "type": "Punctuator", + "value": "}", + }, + ], + "type": "Program", +} +`; + exports[`semanticInfo malformed project file 1`] = `"Compiler option 'compileOnSave' requires a value of type boolean."`; diff --git a/packages/typescript-estree/tests/lib/semanticInfo.ts b/packages/typescript-estree/tests/lib/semanticInfo.ts index d46f18fd648a..126ad35879b6 100644 --- a/packages/typescript-estree/tests/lib/semanticInfo.ts +++ b/packages/typescript-estree/tests/lib/semanticInfo.ts @@ -18,7 +18,11 @@ import { parseCodeAndGenerateServices } from '../../tools/test-utils'; import { parseAndGenerateServices } from '../../src/parser'; -import { VariableDeclaration } from '../../src/typedefs'; +import { + VariableDeclaration, + ClassDeclaration, + ClassProperty +} from '../../src/typedefs'; //------------------------------------------------------------------------------ // Setup @@ -102,8 +106,8 @@ describe('semanticInfo', () => { testIsolatedFile(parseResult); }); - it('parenthesized-expression tests', () => { - const fileName = resolve(FIXTURES_DIR, 'parenthesized-expression.ts'); + it('non-existent-estree-nodes tests', () => { + const fileName = resolve(FIXTURES_DIR, 'non-existent-estree-nodes.src.ts'); const parseResult = parseCodeAndGenerateServices( readFileSync(fileName, 'utf8'), createOptions(fileName) @@ -116,6 +120,13 @@ describe('semanticInfo', () => { binaryExpression ); expect(tsBinaryExpression.kind).toEqual(ts.SyntaxKind.BinaryExpression); + + const computedPropertyString = ((parseResult.ast + .body[1] as ClassDeclaration).body.body[0] as ClassProperty).key; + const tsComputedPropertyString = parseResult.services.esTreeNodeToTSNodeMap!.get( + computedPropertyString + ); + expect(tsComputedPropertyString.kind).toEqual(ts.SyntaxKind.StringLiteral); }); it('imported-file tests', () => { From 5ddf20c50bc60996a234b946ff55f70b48eb6c2b Mon Sep 17 00:00:00 2001 From: Benjamin Lichtman Date: Thu, 7 Feb 2019 15:56:14 -0800 Subject: [PATCH 4/5] fix: fix line endings in new fixture --- .../non-existent-estree-nodes.src.ts | 2 +- .../lib/__snapshots__/semanticInfo.ts.snap | 166 +++++++++--------- 2 files changed, 84 insertions(+), 84 deletions(-) diff --git a/packages/typescript-estree/tests/fixtures/semanticInfo/non-existent-estree-nodes.src.ts b/packages/typescript-estree/tests/fixtures/semanticInfo/non-existent-estree-nodes.src.ts index 885ee45c7b60..4eb9dba432b4 100644 --- a/packages/typescript-estree/tests/fixtures/semanticInfo/non-existent-estree-nodes.src.ts +++ b/packages/typescript-estree/tests/fixtures/semanticInfo/non-existent-estree-nodes.src.ts @@ -1,4 +1,4 @@ -const y = (3 + 5); +const binExp = (3 + 5); class Bar { ['test']: string; diff --git a/packages/typescript-estree/tests/lib/__snapshots__/semanticInfo.ts.snap b/packages/typescript-estree/tests/lib/__snapshots__/semanticInfo.ts.snap index 82849fec5363..f5722861dd24 100644 --- a/packages/typescript-estree/tests/lib/__snapshots__/semanticInfo.ts.snap +++ b/packages/typescript-estree/tests/lib/__snapshots__/semanticInfo.ts.snap @@ -1142,7 +1142,7 @@ Object { "id": Object { "loc": Object { "end": Object { - "column": 7, + "column": 12, "line": 1, }, "start": Object { @@ -1150,10 +1150,10 @@ Object { "line": 1, }, }, - "name": "y", + "name": "binExp", "range": Array [ 6, - 7, + 12, ], "type": "Identifier", }, @@ -1161,17 +1161,17 @@ Object { "left": Object { "loc": Object { "end": Object { - "column": 12, + "column": 17, "line": 1, }, "start": Object { - "column": 11, + "column": 16, "line": 1, }, }, "range": Array [ - 11, - 12, + 16, + 17, ], "raw": "3", "type": "Literal", @@ -1179,33 +1179,33 @@ Object { }, "loc": Object { "end": Object { - "column": 16, + "column": 21, "line": 1, }, "start": Object { - "column": 11, + "column": 16, "line": 1, }, }, "operator": "+", "range": Array [ - 11, 16, + 21, ], "right": Object { "loc": Object { "end": Object { - "column": 16, + "column": 21, "line": 1, }, "start": Object { - "column": 15, + "column": 20, "line": 1, }, }, "range": Array [ - 15, - 16, + 20, + 21, ], "raw": "5", "type": "Literal", @@ -1215,7 +1215,7 @@ Object { }, "loc": Object { "end": Object { - "column": 17, + "column": 22, "line": 1, }, "start": Object { @@ -1225,7 +1225,7 @@ Object { }, "range": Array [ 6, - 17, + 22, ], "type": "VariableDeclarator", }, @@ -1233,7 +1233,7 @@ Object { "kind": "const", "loc": Object { "end": Object { - "column": 18, + "column": 23, "line": 1, }, "start": Object { @@ -1243,7 +1243,7 @@ Object { }, "range": Array [ 0, - 18, + 23, ], "type": "VariableDeclaration", }, @@ -1264,8 +1264,8 @@ Object { }, }, "range": Array [ - 35, - 41, + 40, + 46, ], "raw": "'test'", "type": "Literal", @@ -1282,8 +1282,8 @@ Object { }, }, "range": Array [ - 34, - 51, + 39, + 56, ], "static": false, "type": "ClassProperty", @@ -1299,8 +1299,8 @@ Object { }, }, "range": Array [ - 42, - 50, + 47, + 55, ], "type": "TSTypeAnnotation", "typeAnnotation": Object { @@ -1315,8 +1315,8 @@ Object { }, }, "range": Array [ - 44, - 50, + 49, + 55, ], "type": "TSStringKeyword", }, @@ -1335,8 +1335,8 @@ Object { }, }, "range": Array [ - 30, - 54, + 35, + 58, ], "type": "ClassBody", }, @@ -1353,8 +1353,8 @@ Object { }, "name": "Bar", "range": Array [ - 26, - 29, + 31, + 34, ], "type": "Identifier", }, @@ -1369,8 +1369,8 @@ Object { }, }, "range": Array [ - 20, - 54, + 25, + 58, ], "superClass": null, "type": "ClassDeclaration", @@ -1389,7 +1389,7 @@ Object { }, "range": Array [ 0, - 55, + 59, ], "sourceType": "script", "tokens": Array [ @@ -1414,7 +1414,7 @@ Object { Object { "loc": Object { "end": Object { - "column": 7, + "column": 12, "line": 1, }, "start": Object { @@ -1424,25 +1424,25 @@ Object { }, "range": Array [ 6, - 7, + 12, ], "type": "Identifier", - "value": "y", + "value": "binExp", }, Object { "loc": Object { "end": Object { - "column": 9, + "column": 14, "line": 1, }, "start": Object { - "column": 8, + "column": 13, "line": 1, }, }, "range": Array [ - 8, - 9, + 13, + 14, ], "type": "Punctuator", "value": "=", @@ -1450,17 +1450,17 @@ Object { Object { "loc": Object { "end": Object { - "column": 11, + "column": 16, "line": 1, }, "start": Object { - "column": 10, + "column": 15, "line": 1, }, }, "range": Array [ - 10, - 11, + 15, + 16, ], "type": "Punctuator", "value": "(", @@ -1468,17 +1468,17 @@ Object { Object { "loc": Object { "end": Object { - "column": 12, + "column": 17, "line": 1, }, "start": Object { - "column": 11, + "column": 16, "line": 1, }, }, "range": Array [ - 11, - 12, + 16, + 17, ], "type": "Numeric", "value": "3", @@ -1486,17 +1486,17 @@ Object { Object { "loc": Object { "end": Object { - "column": 14, + "column": 19, "line": 1, }, "start": Object { - "column": 13, + "column": 18, "line": 1, }, }, "range": Array [ - 13, - 14, + 18, + 19, ], "type": "Punctuator", "value": "+", @@ -1504,17 +1504,17 @@ Object { Object { "loc": Object { "end": Object { - "column": 16, + "column": 21, "line": 1, }, "start": Object { - "column": 15, + "column": 20, "line": 1, }, }, "range": Array [ - 15, - 16, + 20, + 21, ], "type": "Numeric", "value": "5", @@ -1522,17 +1522,17 @@ Object { Object { "loc": Object { "end": Object { - "column": 17, + "column": 22, "line": 1, }, "start": Object { - "column": 16, + "column": 21, "line": 1, }, }, "range": Array [ - 16, - 17, + 21, + 22, ], "type": "Punctuator", "value": ")", @@ -1540,17 +1540,17 @@ Object { Object { "loc": Object { "end": Object { - "column": 18, + "column": 23, "line": 1, }, "start": Object { - "column": 17, + "column": 22, "line": 1, }, }, "range": Array [ - 17, - 18, + 22, + 23, ], "type": "Punctuator", "value": ";", @@ -1567,8 +1567,8 @@ Object { }, }, "range": Array [ - 20, 25, + 30, ], "type": "Keyword", "value": "class", @@ -1585,8 +1585,8 @@ Object { }, }, "range": Array [ - 26, - 29, + 31, + 34, ], "type": "Identifier", "value": "Bar", @@ -1603,8 +1603,8 @@ Object { }, }, "range": Array [ - 30, - 31, + 35, + 36, ], "type": "Punctuator", "value": "{", @@ -1621,8 +1621,8 @@ Object { }, }, "range": Array [ - 34, - 35, + 39, + 40, ], "type": "Punctuator", "value": "[", @@ -1639,8 +1639,8 @@ Object { }, }, "range": Array [ - 35, - 41, + 40, + 46, ], "type": "String", "value": "'test'", @@ -1657,8 +1657,8 @@ Object { }, }, "range": Array [ - 41, - 42, + 46, + 47, ], "type": "Punctuator", "value": "]", @@ -1675,8 +1675,8 @@ Object { }, }, "range": Array [ - 42, - 43, + 47, + 48, ], "type": "Punctuator", "value": ":", @@ -1693,8 +1693,8 @@ Object { }, }, "range": Array [ - 44, - 50, + 49, + 55, ], "type": "Identifier", "value": "string", @@ -1711,8 +1711,8 @@ Object { }, }, "range": Array [ - 50, - 51, + 55, + 56, ], "type": "Punctuator", "value": ";", @@ -1729,8 +1729,8 @@ Object { }, }, "range": Array [ - 53, - 54, + 57, + 58, ], "type": "Punctuator", "value": "}", From fd05cb95978ae9a4d4f9a65d56e7c467d75b6539 Mon Sep 17 00:00:00 2001 From: Benjamin Lichtman Date: Thu, 7 Feb 2019 17:07:19 -0800 Subject: [PATCH 5/5] fix: move all functions into closure --- .../rules/no-unnecessary-type-assertion.js | 136 +++++++++--------- 1 file changed, 66 insertions(+), 70 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 a5a4066a916a..bcbe300a01b0 100644 --- a/packages/eslint-plugin/lib/rules/no-unnecessary-type-assertion.js +++ b/packages/eslint-plugin/lib/rules/no-unnecessary-type-assertion.js @@ -15,68 +15,6 @@ const util = require('../util'); // 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 - ]); - } - }); - } -} - /** @type {import("eslint").Rule.RuleModule} */ module.exports = { meta: { @@ -111,14 +49,74 @@ module.exports = { create(context) { const sourceCode = context.getSourceCode(); + const checker = util.getParserServices(context).program.getTypeChecker(); + + /** + * 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 verifyCast(node, context, checker) { + function checkNonNullAssertion(node) { + /** + * 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 + * @returns {void} + */ + function verifyCast(node) { const options = context.options[0]; if ( @@ -172,17 +170,15 @@ module.exports = { } } - const checker = util.getParserServices(context).program.getTypeChecker(); - return { TSNonNullExpression(node) { - checkNonNullAssertion(node, context, checker); + checkNonNullAssertion(node); }, TSTypeAssertion(node) { - verifyCast(node, context, checker); + verifyCast(node); }, TSAsExpression(node) { - verifyCast(node, context, checker); + verifyCast(node); } }; }