fix: account for different scopes during path evaluation (#831)

* fix: account for different scopes during path evaluation

* update dead code tests

* handle when binding is not present

* handle removed nodes in evaluation phase

* array check for block parent

packages/babel-helper-evaluate-path/src/index.js

@@ -1,7 +1,9 @@
 "use strict";
 
+const t = require("@babel/types");
+
 module.exports = function evaluate(path, { tdz = false } = {}) {
-  if (!tdz) {
+  if (!tdz && !path.isReferencedIdentifier()) {
     return baseEvaluate(path);
   }
 
@@ -63,7 +65,21 @@ function evaluateIdentifier(path) {
   const binding = path.scope.getBinding(node.name);
 
   if (!binding) {
-    return deopt(path);
+    const { name } = node;
+    if (!name) {
+      return deopt(path);
+    }
+
+    switch (name) {
+      case "undefined":
+        return { confident: true, value: undefined };
+      case "NaN":
+        return { confident: true, value: NaN };
+      case "Infinity":
+        return { confident: true, value: Infinity };
+      default:
+        return deopt(path);
+    }
   }
 
   if (binding.constantViolations.length > 0) {
@@ -105,24 +121,39 @@ function evaluateBasedOnControlFlow(binding, refPath) {
   if (binding.kind === "var") {
     // early-exit
     const declaration = binding.path.parentPath;
+
     if (
-      declaration.parentPath.isIfStatement() ||
-      declaration.parentPath.isLoop() ||
-      declaration.parentPath.isSwitchCase()
+      t.isIfStatement(declaration.parentPath) ||
+      t.isLoop(declaration.parentPath) ||
+      t.isSwitchCase(declaration.parentPath)
     ) {
+      if (declaration.parentPath.removed) {
+        return { confident: true, value: void 0 };
+      }
       return { shouldDeopt: true };
     }
 
+    const fnParent = (
+      binding.path.scope.getFunctionParent() ||
+      binding.path.scope.getProgramParent()
+    ).path;
+
     let blockParent = binding.path.scope.getBlockParent().path;
-    const fnParent = binding.path.getFunctionParent();
 
-    if (blockParent === fnParent) {
-      if (!fnParent.isProgram()) blockParent = blockParent.get("body");
+    if (blockParent === fnParent && !fnParent.isProgram()) {
+      blockParent = blockParent.get("body");
     }
 
     // detect Usage Outside Init Scope
-    if (!blockParent.get("body").some(stmt => stmt.isAncestor(refPath))) {
-      return { shouldDeopt: true };
+    const blockBody = blockParent.get("body");
+
+    if (
+      Array.isArray(blockBody) &&
+      !blockBody.some(stmt => stmt.isAncestor(refPath))
+    ) {
+      return {
+        shouldDeopt: true
+      };
     }
 
     // Detect usage before init
@@ -143,8 +174,6 @@ function evaluateBasedOnControlFlow(binding, refPath) {
       ) {
         return { confident: true, value: void 0 };
       }
-
-      return { shouldDeopt: true };
     }
   } else if (binding.kind === "let" || binding.kind === "const") {
     // binding.path is the declarator

packages/babel-plugin-minify-dead-code-elimination/__tests__/fixtures/issue-574/expected.js

@@ -1,4 +1,4 @@
 function foo(v) {
-  if (v) ;
-  console.log("hello", v);
+  if (v) var w = 10;
+  if (w) console.log("hello", v);
 }

packages/babel-plugin-minify-dead-code-elimination/src/index.js

@@ -5,6 +5,11 @@ const { markEvalScopes, hasEval } = require("babel-helper-mark-eval-scopes");
 const removeUseStrict = require("./remove-use-strict");
 const evaluate = require("babel-helper-evaluate-path");
 
+function evaluateTruthy(path) {
+  const res = evaluate(path);
+  if (res.confident) return !!res.value;
+}
+
 function prevSiblings(path) {
   const parentPath = path.parentPath;
   const siblings = [];
@@ -548,7 +553,7 @@ module.exports = ({ types: t, traverse }) => {
 
     ConditionalExpression(path) {
       const { node } = path;
-      const evaluateTest = path.get("test").evaluateTruthy();
+      const evaluateTest = evaluateTruthy(path.get("test"));
       if (evaluateTest === true) {
         path.replaceWith(node.consequent);
       } else if (evaluateTest === false) {
@@ -982,6 +987,7 @@ module.exports = ({ types: t, traverse }) => {
       path.traverse({
         VariableDeclaration(varPath) {
           if (!varPath.isVariableDeclaration({ kind: "var" })) return;
+
           if (!isSameFunctionScope(varPath, path)) return;
 
           for (const decl of varPath.node.declarations) {

packages/babel-plugin-minify-guarded-expressions/src/index.js

@@ -1,4 +1,10 @@
 "use strict";
+const evaluate = require("babel-helper-evaluate-path");
+
+function evaluateTruthy(path) {
+  const res = evaluate(path);
+  if (res.confident) return !!res.value;
+}
 
 module.exports = function({ types: t }) {
   const flipExpressions = require("babel-helper-flip-expressions")(t);
@@ -28,28 +34,28 @@ module.exports = function({ types: t }) {
             const shouldBail = !path.parentPath.isExpressionStatement();
 
             if (node.operator === "&&") {
-              const leftTruthy = left.evaluateTruthy();
+              const leftTruthy = evaluateTruthy(left);
               if (leftTruthy === false) {
                 // Short-circuit
                 path.replaceWith(node.left);
               } else if (leftTruthy === true && left.isPure()) {
                 path.replaceWith(node.right);
               } else if (
-                right.evaluateTruthy() === false &&
+                evaluateTruthy(right) === false &&
                 right.isPure() &&
                 !shouldBail
               ) {
                 path.replaceWith(node.left);
               }
             } else if (node.operator === "||") {
-              const leftTruthy = left.evaluateTruthy();
+              const leftTruthy = evaluateTruthy(left);
               if (leftTruthy === false && left.isPure()) {
                 path.replaceWith(node.right);
               } else if (leftTruthy === true) {
                 // Short-circuit
                 path.replaceWith(node.left);
               } else if (
-                right.evaluateTruthy() === false &&
+                evaluateTruthy(right) === false &&
                 right.isPure() &&
                 !shouldBail
               ) {

packages/babel-plugin-minify-simplify/src/helpers.js

@@ -1,6 +1,7 @@
 "use strict";
 
 const VOID_0 = t => t.unaryExpression("void", t.numericLiteral(0), true);
+const evaluate = require("babel-helper-evaluate-path");
 
 // Types as Symbols - for comparing types
 const types = {};
@@ -39,7 +40,7 @@ const isPatternMatchesPath = t =>
       return patternValue(inputPath);
     }
     if (isNodeOfType(t, inputPath.node, patternValue)) return true;
-    const evalResult = inputPath.evaluate();
+    const evalResult = evaluate(inputPath);
     if (!evalResult.confident || !inputPath.isPure()) return false;
     return evalResult.value === patternValue;
   };

packages/babel-plugin-minify-simplify/src/logical-expression.js

@@ -2,13 +2,14 @@
 
 const h = require("./helpers");
 const PatternMatch = require("./pattern-match");
+const evaluate = require("babel-helper-evaluate-path");
 
 module.exports = t => {
   const OP_AND = input => input === "&&";
   const OP_OR = input => input === "||";
 
   function simplifyPatterns(path) {
-    // cache of path.evaluate()
+    // cache of evaluate(path)
     const evaluateMemo = new Map();
 
     const TRUTHY = input => {
@@ -22,7 +23,7 @@ module.exports = t => {
           return true;
         }
       }
-      const evalResult = input.evaluate();
+      const evalResult = evaluate(input);
       evaluateMemo.set(input, evalResult);
       return evalResult.confident && input.isPure() && evalResult.value;
     };
@@ -35,7 +36,7 @@ module.exports = t => {
           return true;
         }
       }
-      const evalResult = input.evaluate();
+      const evalResult = evaluate(input);
       evaluateMemo.set(input, evalResult);
       return evalResult.confident && input.isPure() && !evalResult.value;
     };
@@ -67,7 +68,7 @@ module.exports = t => {
       if (evaluateMemo.has(left)) {
         value = evaluateMemo.get(left).value;
       } else {
-        value = left.evaluate().value;
+        value = evaluate(left).value;
       }
       path.replaceWith(result.value(t.valueToNode(value), right.node));
     }

packages/babel-preset-minify/__tests__/preset-tests.js

@@ -177,4 +177,19 @@ describe("preset", () => {
       }
     `
   );
+
+  thePlugin(
+    "should fix issue#810 declaration inside different scope",
+    `
+      if (false) {
+        var bar = true;
+      }
+      if (bar) {
+        alert('bug!');
+      }
+    `,
+    `
+      var bar;
+    `
+  );
 });