From d21b7061b2f3b6d4e9c83658a3f28fc4dd587ea6 Mon Sep 17 00:00:00 2001 From: Armano Date: Thu, 22 Nov 2018 22:09:30 +0100 Subject: [PATCH 1/2] Fix: missing typeParameters in expressions --- analyze-scope.js | 49 ++ .../expression-type-parameters.ts | 5 + .../lib/__snapshots__/scope-analysis.js.snap | 501 ++++++++++++++++++ visitor-keys.js | 2 + 4 files changed, 557 insertions(+) create mode 100644 tests/fixtures/scope-analysis/expression-type-parameters.ts diff --git a/analyze-scope.js b/analyze-scope.js index 0fb87e9..2971c05 100644 --- a/analyze-scope.js +++ b/analyze-scope.js @@ -218,6 +218,20 @@ class Referencer extends OriginalReferencer { super.visitClass(node); } + /** + * Visit typeParameters. + * @param {*} node The node to visit. + * @returns {void} + */ + visitTypeParameters(node) { + if (node.typeParameters) { + const upperTypeMode = this.typeMode; + this.typeMode = true; + this.visit(node.typeParameters); + this.typeMode = upperTypeMode; + } + } + /** * Override. * Don't create the reference object in the type mode. @@ -286,6 +300,41 @@ class Referencer extends OriginalReferencer { this.typeMode = upperTypeMode; } + /** + * Visit new expression. + * @param {NewExpression} node The NewExpression node to visit. + * @returns {void} + */ + NewExpression(node) { + this.visitTypeParameters(node); + this.visit(node.callee); + if (node.arguments) { + node.arguments.forEach(this.visit, this); + } + } + + /** + * Override. + * Visit call expression. + * @param {CallExpression} node The CallExpression node to visit. + * @returns {void} + */ + CallExpression(node) { + this.visitTypeParameters(node); + + // Check this is direct call to eval + if (!this.scopeManager.__ignoreEval() && node.callee.type === "Identifier" && node.callee.name === "eval") { + + // NOTE: This should be `variableScope`. Since direct eval call always creates Lexical environment and + // let / const should be enclosed into it. Only VariableDeclaration affects on the caller's environment. + this.currentScope().variableScope.__detectEval(); + } + this.visit(node.callee); + if (node.arguments) { + node.arguments.forEach(this.visit, this); + } + } + /** * Define the variable of this function declaration only once. * Because to avoid confusion of `no-redeclare` rule by overloading. diff --git a/tests/fixtures/scope-analysis/expression-type-parameters.ts b/tests/fixtures/scope-analysis/expression-type-parameters.ts new file mode 100644 index 0000000..f6f14f6 --- /dev/null +++ b/tests/fixtures/scope-analysis/expression-type-parameters.ts @@ -0,0 +1,5 @@ +const a, b, c, d, e, f + +new foo(a, b, c); + +baz(d, e, f); diff --git a/tests/lib/__snapshots__/scope-analysis.js.snap b/tests/lib/__snapshots__/scope-analysis.js.snap index 8cf1838..3172c93 100644 --- a/tests/lib/__snapshots__/scope-analysis.js.snap +++ b/tests/lib/__snapshots__/scope-analysis.js.snap @@ -2873,6 +2873,507 @@ Object { } `; +exports[`TypeScript scope analysis tests/fixtures/scope-analysis/expression-type-parameters.ts 1`] = ` +Object { + "$id": 14, + "block": Object { + "range": Array [ + 0, + 67, + ], + "type": "Program", + }, + "childScopes": Array [], + "functionExpressionScope": false, + "isStrict": false, + "references": Array [ + Object { + "$id": 6, + "from": Object { + "$ref": 14, + }, + "identifier": Object { + "name": "foo", + "range": Array [ + 28, + 31, + ], + "type": "Identifier", + }, + "kind": "r", + "resolved": null, + "writeExpr": undefined, + }, + Object { + "$id": 7, + "from": Object { + "$ref": 14, + }, + "identifier": Object { + "name": "a", + "range": Array [ + 37, + 38, + ], + "type": "Identifier", + }, + "kind": "r", + "resolved": Object { + "$ref": 0, + }, + "writeExpr": undefined, + }, + Object { + "$id": 8, + "from": Object { + "$ref": 14, + }, + "identifier": Object { + "name": "b", + "range": Array [ + 40, + 41, + ], + "type": "Identifier", + }, + "kind": "r", + "resolved": Object { + "$ref": 1, + }, + "writeExpr": undefined, + }, + Object { + "$id": 9, + "from": Object { + "$ref": 14, + }, + "identifier": Object { + "name": "c", + "range": Array [ + 43, + 44, + ], + "type": "Identifier", + }, + "kind": "r", + "resolved": Object { + "$ref": 2, + }, + "writeExpr": undefined, + }, + Object { + "$id": 10, + "from": Object { + "$ref": 14, + }, + "identifier": Object { + "name": "baz", + "range": Array [ + 48, + 51, + ], + "type": "Identifier", + }, + "kind": "r", + "resolved": null, + "writeExpr": undefined, + }, + Object { + "$id": 11, + "from": Object { + "$ref": 14, + }, + "identifier": Object { + "name": "d", + "range": Array [ + 57, + 58, + ], + "type": "Identifier", + }, + "kind": "r", + "resolved": Object { + "$ref": 3, + }, + "writeExpr": undefined, + }, + Object { + "$id": 12, + "from": Object { + "$ref": 14, + }, + "identifier": Object { + "name": "e", + "range": Array [ + 60, + 61, + ], + "type": "Identifier", + }, + "kind": "r", + "resolved": Object { + "$ref": 4, + }, + "writeExpr": undefined, + }, + Object { + "$id": 13, + "from": Object { + "$ref": 14, + }, + "identifier": Object { + "name": "f", + "range": Array [ + 63, + 64, + ], + "type": "Identifier", + }, + "kind": "r", + "resolved": Object { + "$ref": 5, + }, + "writeExpr": undefined, + }, + ], + "throughReferences": Array [ + Object { + "$ref": 6, + }, + Object { + "$ref": 10, + }, + ], + "type": "global", + "upperScope": null, + "variableMap": Object { + "a": Object { + "$ref": 0, + }, + "b": Object { + "$ref": 1, + }, + "c": Object { + "$ref": 2, + }, + "d": Object { + "$ref": 3, + }, + "e": Object { + "$ref": 4, + }, + "f": Object { + "$ref": 5, + }, + }, + "variableScope": Object { + "$ref": 14, + }, + "variables": Array [ + Object { + "$id": 0, + "defs": Array [ + Object { + "name": Object { + "name": "a", + "range": Array [ + 6, + 7, + ], + "type": "Identifier", + }, + "node": Object { + "range": Array [ + 6, + 7, + ], + "type": "VariableDeclarator", + }, + "parent": Object { + "range": Array [ + 0, + 22, + ], + "type": "VariableDeclaration", + }, + "type": "Variable", + }, + ], + "eslintUsed": undefined, + "identifiers": Array [ + Object { + "name": "a", + "range": Array [ + 6, + 7, + ], + "type": "Identifier", + }, + ], + "name": "a", + "references": Array [ + Object { + "$ref": 7, + }, + ], + "scope": Object { + "$ref": 14, + }, + }, + Object { + "$id": 1, + "defs": Array [ + Object { + "name": Object { + "name": "b", + "range": Array [ + 9, + 10, + ], + "type": "Identifier", + }, + "node": Object { + "range": Array [ + 9, + 10, + ], + "type": "VariableDeclarator", + }, + "parent": Object { + "range": Array [ + 0, + 22, + ], + "type": "VariableDeclaration", + }, + "type": "Variable", + }, + ], + "eslintUsed": undefined, + "identifiers": Array [ + Object { + "name": "b", + "range": Array [ + 9, + 10, + ], + "type": "Identifier", + }, + ], + "name": "b", + "references": Array [ + Object { + "$ref": 8, + }, + ], + "scope": Object { + "$ref": 14, + }, + }, + Object { + "$id": 2, + "defs": Array [ + Object { + "name": Object { + "name": "c", + "range": Array [ + 12, + 13, + ], + "type": "Identifier", + }, + "node": Object { + "range": Array [ + 12, + 13, + ], + "type": "VariableDeclarator", + }, + "parent": Object { + "range": Array [ + 0, + 22, + ], + "type": "VariableDeclaration", + }, + "type": "Variable", + }, + ], + "eslintUsed": undefined, + "identifiers": Array [ + Object { + "name": "c", + "range": Array [ + 12, + 13, + ], + "type": "Identifier", + }, + ], + "name": "c", + "references": Array [ + Object { + "$ref": 9, + }, + ], + "scope": Object { + "$ref": 14, + }, + }, + Object { + "$id": 3, + "defs": Array [ + Object { + "name": Object { + "name": "d", + "range": Array [ + 15, + 16, + ], + "type": "Identifier", + }, + "node": Object { + "range": Array [ + 15, + 16, + ], + "type": "VariableDeclarator", + }, + "parent": Object { + "range": Array [ + 0, + 22, + ], + "type": "VariableDeclaration", + }, + "type": "Variable", + }, + ], + "eslintUsed": undefined, + "identifiers": Array [ + Object { + "name": "d", + "range": Array [ + 15, + 16, + ], + "type": "Identifier", + }, + ], + "name": "d", + "references": Array [ + Object { + "$ref": 11, + }, + ], + "scope": Object { + "$ref": 14, + }, + }, + Object { + "$id": 4, + "defs": Array [ + Object { + "name": Object { + "name": "e", + "range": Array [ + 18, + 19, + ], + "type": "Identifier", + }, + "node": Object { + "range": Array [ + 18, + 19, + ], + "type": "VariableDeclarator", + }, + "parent": Object { + "range": Array [ + 0, + 22, + ], + "type": "VariableDeclaration", + }, + "type": "Variable", + }, + ], + "eslintUsed": undefined, + "identifiers": Array [ + Object { + "name": "e", + "range": Array [ + 18, + 19, + ], + "type": "Identifier", + }, + ], + "name": "e", + "references": Array [ + Object { + "$ref": 12, + }, + ], + "scope": Object { + "$ref": 14, + }, + }, + Object { + "$id": 5, + "defs": Array [ + Object { + "name": Object { + "name": "f", + "range": Array [ + 21, + 22, + ], + "type": "Identifier", + }, + "node": Object { + "range": Array [ + 21, + 22, + ], + "type": "VariableDeclarator", + }, + "parent": Object { + "range": Array [ + 0, + 22, + ], + "type": "VariableDeclaration", + }, + "type": "Variable", + }, + ], + "eslintUsed": undefined, + "identifiers": Array [ + Object { + "name": "f", + "range": Array [ + 21, + 22, + ], + "type": "Identifier", + }, + ], + "name": "f", + "references": Array [ + Object { + "$ref": 13, + }, + ], + "scope": Object { + "$ref": 14, + }, + }, + ], +} +`; + exports[`TypeScript scope analysis tests/fixtures/scope-analysis/function-overload.ts 1`] = ` Object { "$id": 4, diff --git a/visitor-keys.js b/visitor-keys.js index 1de50cd..0f9ae24 100644 --- a/visitor-keys.js +++ b/visitor-keys.js @@ -19,6 +19,8 @@ module.exports = Evk.unionWith({ Identifier: ["decorators", "typeAnnotation"], MethodDefinition: ["decorators", "key", "value"], ObjectPattern: ["properties", "typeAnnotation"], + NewExpression: ["callee", "typeParameters", "arguments"], + CallExpression: ["callee", "typeParameters", "arguments"], // Additional Nodes. ClassProperty: ["decorators", "key", "typeAnnotation", "value"], From 90156d9ebcf858a00400656417e8d80beaad9181 Mon Sep 17 00:00:00 2001 From: Armano Date: Fri, 23 Nov 2018 01:39:52 +0100 Subject: [PATCH 2/2] ignoreEval is always set to true, than we can ignore this case --- analyze-scope.js | 7 ------- 1 file changed, 7 deletions(-) diff --git a/analyze-scope.js b/analyze-scope.js index 2971c05..a44532f 100644 --- a/analyze-scope.js +++ b/analyze-scope.js @@ -322,13 +322,6 @@ class Referencer extends OriginalReferencer { CallExpression(node) { this.visitTypeParameters(node); - // Check this is direct call to eval - if (!this.scopeManager.__ignoreEval() && node.callee.type === "Identifier" && node.callee.name === "eval") { - - // NOTE: This should be `variableScope`. Since direct eval call always creates Lexical environment and - // let / const should be enclosed into it. Only VariableDeclaration affects on the caller's environment. - this.currentScope().variableScope.__detectEval(); - } this.visit(node.callee); if (node.arguments) { node.arguments.forEach(this.visit, this);