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

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
+```

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);
+  });
+});

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": {}
+}

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];
+}

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);
+  });
 });

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"
   },

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(),

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": {}
 }

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);