Merge pull request #6273 from ooflorent/fix-4857/unreachable_branches

Eliminate unreachable branches

lib/ConstPlugin.js

@@ -12,6 +12,90 @@ const getQuery = (request) => {
 	return request.includes("?") ? request.substr(i) : "";
 };
 
+const collectDeclaration = (declarations, pattern) => {
+	const stack = [pattern];
+	while(stack.length > 0) {
+		const node = stack.pop();
+		switch(node.type) {
+			case "Identifier":
+				declarations.add(node.name);
+				break;
+			case "ArrayPattern":
+				for(const element of node.elements)
+					if(element) stack.push(element);
+				break;
+			case "AssignmentPattern":
+				stack.push(node.left);
+				break;
+			case "ObjectPattern":
+				for(const property of node.properties)
+					stack.push(property.value);
+				break;
+			case "RestElement":
+				stack.push(node.argument);
+				break;
+		}
+	}
+};
+
+const getHoistedDeclarations = (branch, includeFunctionDeclarations) => {
+	const declarations = new Set();
+	const stack = [branch];
+	while(stack.length > 0) {
+		const node = stack.pop();
+		// Some node could be `null` or `undefined`.
+		if(!node)
+			continue;
+		switch(node.type) {
+			// Walk through control statements to look for hoisted declarations.
+			// Some branches are skipped since they do not allow declarations.
+			case "BlockStatement":
+				for(const stmt of node.body)
+					stack.push(stmt);
+				break;
+			case "IfStatement":
+				stack.push(node.consequent);
+				stack.push(node.alternate);
+				break;
+			case "ForStatement":
+				stack.push(node.init);
+				stack.push(node.body);
+				break;
+			case "ForInStatement":
+			case "ForOfStatement":
+				stack.push(node.left);
+				stack.push(node.body);
+				break;
+			case "DoWhileStatement":
+			case "WhileStatement":
+			case "LabeledStatement":
+				stack.push(node.body);
+				break;
+			case "SwitchStatement":
+				for(const cs of node.cases)
+					for(const consequent of cs.consequent)
+						stack.push(consequent);
+				break;
+			case "TryStatement":
+				stack.push(node.block);
+				if(node.handler)
+					stack.push(node.handler.body);
+				stack.push(node.finalizer);
+				break;
+			case "FunctionDeclaration":
+				if(includeFunctionDeclarations)
+					collectDeclaration(declarations, node.id);
+				break;
+			case "VariableDeclaration":
+				if(node.kind === "var")
+					for(const decl of node.declarations)
+						collectDeclaration(declarations, decl.id);
+				break;
+		}
+	}
+	return Array.from(declarations);
+};
+
 class ConstPlugin {
 	apply(compiler) {
 		compiler.hooks.compilation.tap("ConstPlugin", (compilation, {
@@ -30,6 +114,57 @@ class ConstPlugin {
 							dep.loc = statement.loc;
 							parser.state.current.addDependency(dep);
 						}
+						const branchToRemove = bool ? statement.alternate : statement.consequent;
+						if(branchToRemove) {
+							// Before removing the dead branch, the hoisted declarations
+							// must be collected.
+							//
+							// Given the following code:
+							//
+							//     if (true) f() else g()
+							//     if (false) {
+							//       function f() {}
+							//       const g = function g() {}
+							//       if (someTest) {
+							//         let a = 1
+							//         var x, {y, z} = obj
+							//       }
+							//     } else {
+							//       …
+							//     }
+							//
+							// the generated code is:
+							//
+							//     if (true) f() else {}
+							//     if (false) {
+							//       var f, x, y, z;   (in loose mode)
+							//       var x, y, z;      (in strict mode)
+							//     } else {
+							//       …
+							//     }
+							//
+							// NOTE: When code runs in strict mode, `var` declarations
+							// are hoisted but `function` declarations don't.
+							//
+							let declarations;
+							if(parser.scope.isStrict) {
+								// If the code runs in strict mode, variable declarations
+								// using `var` must be hoisted.
+								declarations = getHoistedDeclarations(branchToRemove, false);
+							} else {
+								// Otherwise, collect all hoisted declaration.
+								declarations = getHoistedDeclarations(branchToRemove, true);
+							}
+							let replacement;
+							if(declarations.length > 0) {
+								replacement = `{ var ${declarations.join(", ")}; }`;
+							} else {
+								replacement = "{}";
+							}
+							const dep = new ConstDependency(replacement, branchToRemove.range);
+							dep.loc = branchToRemove.loc;
+							parser.state.current.addDependency(dep);
+						}
 						return bool;
 					}
 				});
@@ -42,6 +177,21 @@ class ConstPlugin {
 							dep.loc = expression.loc;
 							parser.state.current.addDependency(dep);
 						}
+						// Expressions do not hoist.
+						// It is safe to remove the dead branch.
+						//
+						// Given the following code:
+						//
+						//   false ? someExpression() : otherExpression();
+						//
+						// the generated code is:
+						//
+						//   false ? undefined : otherExpression();
+						//
+						const branchToRemove = bool ? expression.alternate : expression.consequent;
+						const dep = new ConstDependency("undefined", branchToRemove.range);
+						dep.loc = branchToRemove.loc;
+						parser.state.current.addDependency(dep);
 						return bool;
 					}
 				});

lib/Parser.js

@@ -948,6 +948,7 @@ class Parser extends Tapable {
 			this.walkPattern(param);
 		this.inScope(statement.params, () => {
 			if(statement.body.type === "BlockStatement") {
+				this.detectStrictMode(statement.body.body);
 				this.prewalkStatement(statement.body);
 				this.walkStatement(statement.body);
 			} else {
@@ -1316,6 +1317,7 @@ class Parser extends Tapable {
 			this.walkPattern(param);
 		this.inScope(expression.params, () => {
 			if(expression.body.type === "BlockStatement") {
+				this.detectStrictMode(expression.body.body);
 				this.prewalkStatement(expression.body);
 				this.walkStatement(expression.body);
 			} else {
@@ -1329,6 +1331,7 @@ class Parser extends Tapable {
 			this.walkPattern(param);
 		this.inScope(expression.params, () => {
 			if(expression.body.type === "BlockStatement") {
+				this.detectStrictMode(expression.body.body);
 				this.prewalkStatement(expression.body);
 				this.walkStatement(expression.body);
 			} else {
@@ -1580,6 +1583,7 @@ class Parser extends Tapable {
 		this.scope = {
 			inTry: false,
 			inShorthand: false,
+			isStrict: oldScope.isStrict,
 			definitions: oldScope.definitions.createChild(),
 			renames: oldScope.renames.createChild()
 		};
@@ -1604,6 +1608,16 @@ class Parser extends Tapable {
 		this.scope = oldScope;
 	}
 
+	detectStrictMode(statements) {
+		const isStrict = statements.length >= 1 &&
+			statements[0].type === "ExpressionStatement" &&
+			statements[0].expression.type === "Literal" &&
+			statements[0].expression.value === "use strict";
+		if(isStrict) {
+			this.scope.isStrict = true;
+		}
+	}
+
 	enterPattern(pattern, onIdent) {
 		if(!pattern) return;
 		switch(pattern.type) {
@@ -1775,12 +1789,15 @@ class Parser extends Tapable {
 		const oldComments = this.comments;
 		this.scope = {
 			inTry: false,
+			inShorthand: false,
+			isStrict: false,
 			definitions: new StackedSetMap(),
 			renames: new StackedSetMap()
 		};
 		const state = this.state = initialState || {};
 		this.comments = comments;
 		if(this.hooks.program.call(ast, comments) === undefined) {
+			this.detectStrictMode(ast.body);
 			this.prewalkStatements(ast.body);
 			this.walkStatements(ast.body);
 		}

lib/UseStrictPlugin.js

@@ -12,7 +12,6 @@ class UseStrictPlugin {
 			normalModuleFactory
 		}) => {
 			const handler = (parser) => {
-				const parserInstance = parser;
 				parser.hooks.program.tap("UseStrictPlugin", (ast) => {
 					const firstNode = ast.body[0];
 					if(firstNode &&
@@ -24,8 +23,8 @@ class UseStrictPlugin {
 						// @see https://github.com/webpack/webpack/issues/1970
 						const dep = new ConstDependency("", firstNode.range);
 						dep.loc = firstNode.loc;
-						parserInstance.state.current.addDependency(dep);
-						parserInstance.state.module.buildInfo.strict = true;
+						parser.state.current.addDependency(dep);
+						parser.state.module.buildInfo.strict = true;
 					}
 				});
 			};

lib/dependencies/HarmonyDetectionParserPlugin.js

@@ -43,6 +43,7 @@ module.exports = class HarmonyDetectionParserPlugin {
 				};
 				module.addDependency(initDep);
 				parser.state.harmonyParserScope = parser.state.harmonyParserScope || {};
+				parser.scope.isStrict = true;
 				module.buildMeta.exportsType = "namespace";
 				module.buildInfo.strict = true;
 				module.buildInfo.exportsArgument = "__webpack_exports__";

test/configCases/parsing/issue-4857/index.js

@@ -0,0 +1,97 @@
+it("should transpile unreachable branches", () => {
+	let count = 0;
+
+	// BlockStatement
+	if(true) {
+		count++;
+	} else {
+		import("NOT_REACHABLE");
+	}
+	if(false) {
+		import("NOT_REACHABLE");
+	} else {
+		count++;
+	}
+
+	// ExpressionStatement
+	if(true) count++;
+	else import("NOT_REACHABLE");
+	if(false) import("NOT_REACHABLE");
+	else count++;
+
+	// ConditionalExpression
+	true ? count++ : import("NOT_REACHABLE");
+	false ? import("NOT_REACHABLE") : count++;
+
+	count.should.be.eql(6);
+});
+
+it("should not remove hoisted variable declarations", () => {
+	if(false) {
+		var a, [,,b,] = [], {c, D: d, ["E"]: e = 2} = {};
+		var [{["_"]: f}, ...g] = [];
+		do {
+			switch(g) {
+				default:
+					var h;
+					break;
+			}
+			loop: for(var i;;)
+				for(var j in {})
+					for(var k of {})
+						break;
+			try {
+				var l;
+			} catch(e) {
+				var m;
+			} finally {
+				var n;
+			}
+			{
+				var o;
+			}
+		} while(true);
+		with (o) {
+			var withVar;
+		}
+	}
+	(() => {
+		a;
+		b;
+		c;
+		d;
+		e;
+		f;
+		g;
+		h;
+		i;
+		j;
+		k;
+		l;
+		m;
+		n;
+		o;
+	}).should.not.throw();
+	(() => {
+		withVar;
+	}).should.throw();
+});
+
+it("should not remove hoisted function declarations in loose mode", () => {
+	if(false) {
+		function funcDecl() {}
+	}
+	(() => {
+		funcDecl;
+	}).should.not.throw();
+});
+
+it("should remove hoisted function declarations in strict mode", () => {
+	"use strict";
+	if(false) {
+		function funcDecl() {}
+	}
+	(() => {
+		funcDecl;
+	}).should.throw();
+});

test/configCases/parsing/issue-4857/webpack.config.js

@@ -0,0 +1,5 @@
+module.exports = {
+	optimization: {
+		minimize: false
+	}
+};