From ca7854f1953d8f1734ddbd0f843c6b60404df2a1 Mon Sep 17 00:00:00 2001 From: Vignesh Shanmugam Date: Thu, 3 May 2018 15:41:47 +0200 Subject: [PATCH 1/5] fix: account for different scopes during path evaluation --- packages/babel-helper-evaluate-path/src/index.js | 14 +++++++++----- .../src/index.js | 8 ++++++-- .../src/index.js | 14 ++++++++++---- .../babel-plugin-minify-simplify/src/helpers.js | 3 ++- .../src/logical-expression.js | 9 +++++---- 5 files changed, 32 insertions(+), 16 deletions(-) diff --git a/packages/babel-helper-evaluate-path/src/index.js b/packages/babel-helper-evaluate-path/src/index.js index 3ed6d322a..baed0e119 100644 --- a/packages/babel-helper-evaluate-path/src/index.js +++ b/packages/babel-helper-evaluate-path/src/index.js @@ -1,14 +1,14 @@ "use strict"; module.exports = function evaluate(path, { tdz = false } = {}) { - if (!tdz) { - return baseEvaluate(path); - } - if (path.isReferencedIdentifier()) { return evaluateIdentifier(path); } + if (!tdz) { + return baseEvaluate(path); + } + const state = { confident: true }; @@ -113,8 +113,12 @@ function evaluateBasedOnControlFlow(binding, refPath) { return { shouldDeopt: true }; } - let blockParent = binding.path.scope.getBlockParent().path; const fnParent = binding.path.getFunctionParent(); + if (!fnParent) { + return { shouldDeopt: true }; + } + + let blockParent = binding.path.scope.getBlockParent().path; if (blockParent === fnParent) { if (!fnParent.isProgram()) blockParent = blockParent.get("body"); 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 448fa7d18..92ee8f130 100644 --- a/packages/babel-plugin-minify-dead-code-elimination/src/index.js +++ b/packages/babel-plugin-minify-dead-code-elimination/src/index.js @@ -976,10 +976,14 @@ module.exports = ({ types: t, traverse }) => { } else { path.traverse({ VariableDeclaration(varPath) { - if (!varPath.isVariableDeclaration({ kind: "var" })) return; + const { node } = varPath; + + if (node.kind !== "var") { + return; + } if (!isSameFunctionScope(varPath, path)) return; - for (const decl of varPath.node.declarations) { + for (const decl of node.declarations) { const bindingIds = Object.keys(t.getBindingIdentifiers(decl.id)); declarators.push( ...bindingIds.map(name => diff --git a/packages/babel-plugin-minify-guarded-expressions/src/index.js b/packages/babel-plugin-minify-guarded-expressions/src/index.js index c144fd5f2..3c0f392a3 100644 --- a/packages/babel-plugin-minify-guarded-expressions/src/index.js +++ b/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 ) { diff --git a/packages/babel-plugin-minify-simplify/src/helpers.js b/packages/babel-plugin-minify-simplify/src/helpers.js index 1fa8e6269..0dd6c698e 100644 --- a/packages/babel-plugin-minify-simplify/src/helpers.js +++ b/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; }; diff --git a/packages/babel-plugin-minify-simplify/src/logical-expression.js b/packages/babel-plugin-minify-simplify/src/logical-expression.js index 448d08e87..191c477fe 100644 --- a/packages/babel-plugin-minify-simplify/src/logical-expression.js +++ b/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)); } From a81bea12516fe832cc18757490e0dca05d9ca205 Mon Sep 17 00:00:00 2001 From: Vignesh Shanmugam Date: Thu, 3 May 2018 15:43:37 +0200 Subject: [PATCH 2/5] update dead code tests --- .../__tests__/fixtures/issue-574/expected.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/babel-plugin-minify-dead-code-elimination/__tests__/fixtures/issue-574/expected.js b/packages/babel-plugin-minify-dead-code-elimination/__tests__/fixtures/issue-574/expected.js index a0c11b759..9390b33f2 100644 --- a/packages/babel-plugin-minify-dead-code-elimination/__tests__/fixtures/issue-574/expected.js +++ b/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); } \ No newline at end of file From feee1ecc9ade26083513c537e642a6dfc816abdf Mon Sep 17 00:00:00 2001 From: Vignesh Shanmugam Date: Thu, 3 May 2018 22:10:59 +0200 Subject: [PATCH 3/5] handle when binding is not present --- .../babel-helper-evaluate-path/src/index.js | 19 ++++++++++++++++++- .../__tests__/preset-tests.js | 16 ++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/packages/babel-helper-evaluate-path/src/index.js b/packages/babel-helper-evaluate-path/src/index.js index baed0e119..43c49844c 100644 --- a/packages/babel-helper-evaluate-path/src/index.js +++ b/packages/babel-helper-evaluate-path/src/index.js @@ -63,7 +63,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) { @@ -119,6 +133,9 @@ function evaluateBasedOnControlFlow(binding, refPath) { } let blockParent = binding.path.scope.getBlockParent().path; + if (!blockParent) { + return { shouldDeopt: true }; + } if (blockParent === fnParent) { if (!fnParent.isProgram()) blockParent = blockParent.get("body"); diff --git a/packages/babel-preset-minify/__tests__/preset-tests.js b/packages/babel-preset-minify/__tests__/preset-tests.js index 72cb3b447..dd3a6060d 100644 --- a/packages/babel-preset-minify/__tests__/preset-tests.js +++ b/packages/babel-preset-minify/__tests__/preset-tests.js @@ -177,4 +177,20 @@ describe("preset", () => { } ` ); + + thePlugin( + "should fix issue#810 declaration inside different scope", + ` + if (false) { + var bar = true; + } + if (bar) { + alert('bug!'); + } + `, + ` + var bar; + bar && alert('bug!'); + ` + ); }); From b6d9675d7644c0634222ac6d122b4ed97b1711ce Mon Sep 17 00:00:00 2001 From: Vignesh Shanmugam Date: Fri, 11 May 2018 16:14:50 +0200 Subject: [PATCH 4/5] handle removed nodes in evaluation phase --- .../babel-helper-evaluate-path/src/index.js | 37 ++++++++++--------- .../src/index.js | 14 ++++--- .../__tests__/preset-tests.js | 1 - 3 files changed, 27 insertions(+), 25 deletions(-) diff --git a/packages/babel-helper-evaluate-path/src/index.js b/packages/babel-helper-evaluate-path/src/index.js index 43c49844c..abb866a70 100644 --- a/packages/babel-helper-evaluate-path/src/index.js +++ b/packages/babel-helper-evaluate-path/src/index.js @@ -1,12 +1,14 @@ "use strict"; +const t = require("@babel/types"); + module.exports = function evaluate(path, { tdz = false } = {}) { - if (path.isReferencedIdentifier()) { - return evaluateIdentifier(path); + if (!tdz && !path.isReferencedIdentifier()) { + return baseEvaluate(path); } - if (!tdz) { - return baseEvaluate(path); + if (path.isReferencedIdentifier()) { + return evaluateIdentifier(path); } const state = { @@ -119,26 +121,27 @@ 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.getFunctionParent(); - if (!fnParent) { - return { shouldDeopt: true }; - } + const fnParent = ( + binding.path.scope.getFunctionParent() || + binding.path.scope.getProgramParent() + ).path; let blockParent = binding.path.scope.getBlockParent().path; - if (!blockParent) { - return { shouldDeopt: true }; - } - if (blockParent === fnParent) { - if (!fnParent.isProgram()) blockParent = blockParent.get("body"); + if (blockParent === fnParent && !fnParent.isProgram()) { + blockParent = blockParent.get("body"); } // detect Usage Outside Init Scope @@ -164,8 +167,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 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 92ee8f130..a15b14e3f 100644 --- a/packages/babel-plugin-minify-dead-code-elimination/src/index.js +++ b/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) { @@ -976,14 +981,11 @@ module.exports = ({ types: t, traverse }) => { } else { path.traverse({ VariableDeclaration(varPath) { - const { node } = varPath; + if (!varPath.isVariableDeclaration({ kind: "var" })) return; - if (node.kind !== "var") { - return; - } if (!isSameFunctionScope(varPath, path)) return; - for (const decl of node.declarations) { + for (const decl of varPath.node.declarations) { const bindingIds = Object.keys(t.getBindingIdentifiers(decl.id)); declarators.push( ...bindingIds.map(name => diff --git a/packages/babel-preset-minify/__tests__/preset-tests.js b/packages/babel-preset-minify/__tests__/preset-tests.js index dd3a6060d..11f016130 100644 --- a/packages/babel-preset-minify/__tests__/preset-tests.js +++ b/packages/babel-preset-minify/__tests__/preset-tests.js @@ -190,7 +190,6 @@ describe("preset", () => { `, ` var bar; - bar && alert('bug!'); ` ); }); From a353f91a476e317c22bfd28a5dc63d00c2e69229 Mon Sep 17 00:00:00 2001 From: Vignesh Shanmugam Date: Fri, 11 May 2018 17:14:04 +0200 Subject: [PATCH 5/5] array check for block parent --- packages/babel-helper-evaluate-path/src/index.js | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/babel-helper-evaluate-path/src/index.js b/packages/babel-helper-evaluate-path/src/index.js index abb866a70..3e887edc9 100644 --- a/packages/babel-helper-evaluate-path/src/index.js +++ b/packages/babel-helper-evaluate-path/src/index.js @@ -145,8 +145,15 @@ function evaluateBasedOnControlFlow(binding, refPath) { } // 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