From 2b98b98073dd4087dc03302662c69e7c13d43d80 Mon Sep 17 00:00:00 2001 From: Boopathi Rajaa Date: Thu, 19 Jan 2017 22:02:28 +0100 Subject: [PATCH] Add mark eval scopes helper and deopt DCE fn unused params (#371) * Add mark eval scopes * Fix readme bug as a result of copy-pasting * Add more tests * Use Symbol for EVAL_SCOPE_MARKER * Fix lint --- .../babel-helper-mark-eval-scopes/README.md | 9 ++ .../__tests__/helper-mark-eval-scopes-test.js | 48 ++++++++ .../package.json | 17 +++ .../src/index.js | 52 ++++++++ .../__tests__/dead-code-elimination-test.js | 115 ++++++++++++++++++ .../package.json | 1 + .../src/index.js | 9 ++ .../package.json | 4 +- .../src/index.js | 53 ++++---- 9 files changed, 279 insertions(+), 29 deletions(-) create mode 100644 packages/babel-helper-mark-eval-scopes/README.md create mode 100644 packages/babel-helper-mark-eval-scopes/__tests__/helper-mark-eval-scopes-test.js create mode 100644 packages/babel-helper-mark-eval-scopes/package.json create mode 100644 packages/babel-helper-mark-eval-scopes/src/index.js diff --git a/packages/babel-helper-mark-eval-scopes/README.md b/packages/babel-helper-mark-eval-scopes/README.md new file mode 100644 index 000000000..1c30b0ffc --- /dev/null +++ b/packages/babel-helper-mark-eval-scopes/README.md @@ -0,0 +1,9 @@ +# babel-helper-mark-eval-scopes + +Traverse through input path and mark all scopes that contain Direct eval (`eval("")`) calls. + +## Installation + +```sh +npm install babel-helper-mark-eval-scopes +``` diff --git a/packages/babel-helper-mark-eval-scopes/__tests__/helper-mark-eval-scopes-test.js b/packages/babel-helper-mark-eval-scopes/__tests__/helper-mark-eval-scopes-test.js new file mode 100644 index 000000000..26c09861f --- /dev/null +++ b/packages/babel-helper-mark-eval-scopes/__tests__/helper-mark-eval-scopes-test.js @@ -0,0 +1,48 @@ +jest.autoMockOff(); + +const babel = require("babel-core"); +const helper = require("../src"); + +function getPath(source) { + let path; + + babel.transform(source, { + babelrc: false, + plugins: [ + function ({traverse}) { + traverse.clearCache(); + return { + visitor: { + Program(programPath) { + path = programPath; + } + } + }; + } + ] + }); + + return path; +} + +describe("babel-helper-mark-eval-scopes", () => { + it("getEvalScopes - should give a set of scopes which contains eval", () => { + const source = ` + function foo() { + function bar() { + eval(";"); + } + function baz() { + noeval(); + } + } + `; + + const program = getPath(source); + const evalScopes = [...helper.getEvalScopes(program)]; + + expect(evalScopes).toContain(program.scope); + expect(evalScopes).toContain(program.get("body.0.body.body.0").scope); + expect(evalScopes).not.toContain(program.get("body.0.body.body.1").scope); + }); +}); diff --git a/packages/babel-helper-mark-eval-scopes/package.json b/packages/babel-helper-mark-eval-scopes/package.json new file mode 100644 index 000000000..7f8f8d68f --- /dev/null +++ b/packages/babel-helper-mark-eval-scopes/package.json @@ -0,0 +1,17 @@ +{ + "name": "babel-helper-mark-eval-scopes", + "version": "0.0.1", + "description": "Mark scopes for deopt which contain a direct eval call", + "homepage": "https://github.com/babel/babili#readme", + "repository": "https://github.com/babel/babili/tree/master/packages/babel-helper-mark-eval-scopes", + "bugs": "https://github.com/babel/babili/issues", + "author": "boopathi", + "license": "MIT", + "main": "lib/index.js", + "keywords": [ + "babel-plugin", + "babili" + ], + "dependencies": {}, + "devDependencies": {} +} diff --git a/packages/babel-helper-mark-eval-scopes/src/index.js b/packages/babel-helper-mark-eval-scopes/src/index.js new file mode 100644 index 000000000..466a9fa69 --- /dev/null +++ b/packages/babel-helper-mark-eval-scopes/src/index.js @@ -0,0 +1,52 @@ +"use strict"; + +const EVAL_SCOPE_MARKER = Symbol("evalInScope"); + +module.exports = { + EVAL_SCOPE_MARKER, + getEvalScopes, + markEvalScopes, + isMarked, + hasEval, +}; + +function getEvalScopes(path) { + const evalScopes = new Set; + + function add(scope) { + let evalScope = scope; + do { + evalScopes.add(evalScope); + } while (evalScope = evalScope.parent); + } + + path.traverse({ + CallExpression(evalPath) { + const callee = evalPath.get("callee"); + + if (callee.isIdentifier() && callee.node.name === "eval" && !callee.scope.getBinding("eval")) { + add(callee.scope); + } + } + }); + + return evalScopes; +} + +function markEvalScopes(path, key = EVAL_SCOPE_MARKER) { + const evalScopes = getEvalScopes(path); + [...evalScopes].forEach((scope) => { + scope[key] = true; + }); +} + +function isMarked(scope, key = EVAL_SCOPE_MARKER) { + return Object.prototype.hasOwnProperty.call(scope, key); +} + +function hasEval(scope, key = EVAL_SCOPE_MARKER) { + if (!isMarked(scope, key)) { + markEvalScopes(scope, key); + } + return scope[key]; +} diff --git a/packages/babel-plugin-minify-dead-code-elimination/__tests__/dead-code-elimination-test.js b/packages/babel-plugin-minify-dead-code-elimination/__tests__/dead-code-elimination-test.js index cd3922821..372da8dcf 100644 --- a/packages/babel-plugin-minify-dead-code-elimination/__tests__/dead-code-elimination-test.js +++ b/packages/babel-plugin-minify-dead-code-elimination/__tests__/dead-code-elimination-test.js @@ -2306,4 +2306,119 @@ describe("dce-plugin", () => { `); expect(transformWithSimplify(source)).toBe(expected); }); + + it("should not remove params from functions containing direct eval", () => { + const source = unpad(` + function a(b, c, d) { + eval(";"); + return b; + } + function b(c, d, e) { + (1, eval)(";"); + return c; + } + `); + + const expected = unpad(` + function a(b, c, d) { + eval(";"); + return b; + } + function b(c) { + (1, eval)(";"); + return c; + } + `); + + expect(transform(source)).toBe(expected); + }); + + it("should not remove params/vars from functions containing direct eval", () => { + const source = unpad(` + function foo(bar, baz) { + function foox(a, b, c) { + x.then((data, unused) => { + let unused1; + eval(data); + foox1(); + { + var unused2; + } + }); + + function foox1(unused) { + console.log("foox1"); + } + } + function fooy(unused1, unused2) { + console.log("fooy"); + } + } + `); + + const expected = unpad(` + function foo(bar, baz) { + function foox(a, b, c) { + x.then((data, unused) => { + let unused1; + eval(data); + foox1(); + { + var unused2; + } + }); + + function foox1() { + console.log("foox1"); + } + } + function fooy() { + console.log("fooy"); + } + } + `); + + expect(transform(source)).toBe(expected); + }); + + it("should not optimize/remove vars from functions containing direct eval", () => { + const source = unpad(` + function foo() { + bar(); + + var x = 5; + return x; + + function bar() { + eval(";"); + return 5; + } + + function baz() { + let x = 10; + return x; + } + } + `); + + const expected = unpad(` + function foo() { + bar(); + + var x = 5; + return x; + + function bar() { + eval(";"); + return 5; + } + + function baz() { + return 10; + } + } + `); + + expect(transform(source)).toBe(expected); + }); }); diff --git a/packages/babel-plugin-minify-dead-code-elimination/package.json b/packages/babel-plugin-minify-dead-code-elimination/package.json index 01efde2ea..efca8b50d 100644 --- a/packages/babel-plugin-minify-dead-code-elimination/package.json +++ b/packages/babel-plugin-minify-dead-code-elimination/package.json @@ -12,6 +12,7 @@ "babel-plugin" ], "dependencies": { + "babel-helper-mark-eval-scopes": "^0.0.1", "babel-helper-remove-or-void": "^0.0.1", "lodash.some": "^4.6.0" }, diff --git a/packages/babel-plugin-minify-dead-code-elimination/src/index.js b/packages/babel-plugin-minify-dead-code-elimination/src/index.js index 47bebe420..8fcc304ab 100644 --- a/packages/babel-plugin-minify-dead-code-elimination/src/index.js +++ b/packages/babel-plugin-minify-dead-code-elimination/src/index.js @@ -1,6 +1,7 @@ "use strict"; const some = require("lodash.some"); +const { markEvalScopes, isMarked: isEvalScopesMarked, hasEval } = require("babel-helper-mark-eval-scopes"); module.exports = ({ types: t, traverse }) => { const removeOrVoid = require("babel-helper-remove-or-void")(t); @@ -104,6 +105,10 @@ module.exports = ({ types: t, traverse }) => { return; } + if (hasEval(path.scope)) { + return; + } + const { scope } = path; // if the scope is created by a function, we obtain its @@ -712,6 +717,10 @@ module.exports = ({ types: t, traverse }) => { traverse.clearCache(); path.scope.crawl(); + if (!isEvalScopesMarked(path.scope)) { + markEvalScopes(path); + } + // We need to run this plugin in isolation. path.traverse(main, { functionToBindings: new Map(), diff --git a/packages/babel-plugin-minify-mangle-names/package.json b/packages/babel-plugin-minify-mangle-names/package.json index bd82d9f45..2f4a5343d 100644 --- a/packages/babel-plugin-minify-mangle-names/package.json +++ b/packages/babel-plugin-minify-mangle-names/package.json @@ -11,6 +11,8 @@ "keywords": [ "babel-plugin" ], - "dependencies": {}, + "dependencies": { + "babel-helper-mark-eval-scopes": "^0.0.1" + }, "devDependencies": {} } diff --git a/packages/babel-plugin-minify-mangle-names/src/index.js b/packages/babel-plugin-minify-mangle-names/src/index.js index a2d1cc999..e55bcd474 100644 --- a/packages/babel-plugin-minify-mangle-names/src/index.js +++ b/packages/babel-plugin-minify-mangle-names/src/index.js @@ -1,3 +1,11 @@ +"use strict"; + +const { + markEvalScopes, + isMarked: isEvalScopesMarked, + hasEval, +} = require("babel-helper-mark-eval-scopes"); + module.exports = ({ types: t, traverse }) => { const hop = Object.prototype.hasOwnProperty; @@ -49,39 +57,28 @@ module.exports = ({ types: t, traverse }) => { collect() { const mangler = this; - const collectVisitor = { - // capture direct evals - CallExpression(path) { - const callee = path.get("callee"); - - if (callee.isIdentifier() - && callee.node.name === "eval" - && !callee.scope.getBinding("eval") - ) { - mangler.markUnsafeScopes(path.scope); - } - } - }; + if (!isEvalScopesMarked(mangler.program.scope)) { + markEvalScopes(mangler.program); + } if (this.charset.shouldConsider) { - // charset considerations - collectVisitor.Identifier = function Identifier(path) { - const { node } = path; - - if ((path.parentPath.isMemberExpression({ property: node })) || - (path.parentPath.isObjectProperty({ key: node })) - ) { - mangler.charset.consider(node.name); + const collectVisitor = { + Identifier(path) { + const { node } = path; + + if ((path.parentPath.isMemberExpression({ property: node })) || + (path.parentPath.isObjectProperty({ key: node })) + ) { + mangler.charset.consider(node.name); + } + }, + Literal({ node }) { + mangler.charset.consider(String(node.value)); } }; - // charset considerations - collectVisitor.Literal = function Literal({ node }) { - mangler.charset.consider(String(node.value)); - }; + mangler.program.traverse(collectVisitor); } - - this.program.traverse(collectVisitor); } mangle() { @@ -91,7 +88,7 @@ module.exports = ({ types: t, traverse }) => { Scopable(path) { const {scope} = path; - if (!mangler.eval && mangler.unsafeScopes.has(scope)) return; + if (!mangler.eval && hasEval(scope)) return; if (mangler.visitedScopes.has(scope)) return; mangler.visitedScopes.add(scope);