Handle hoisted variables in dead branches of nested if statements (#3782

)

* Handle hoisted variables in dead branches of nested if statements

* Update dependencies

package.json

@@ -71,13 +71,13 @@
     "@rollup/plugin-node-resolve": "^9.0.0",
     "@rollup/plugin-replace": "^2.3.3",
     "@types/micromatch": "^4.0.1",
-    "@types/node": "^14.0.27",
+    "@types/node": "^14.10.2",
     "@types/require-relative": "^0.8.0",
     "@types/signal-exit": "^3.0.0",
     "@types/yargs-parser": "^15.0.0",
     "acorn": "^8.0.1",
     "acorn-class-fields": "^0.3.7",
-    "acorn-jsx": "^5.2.0",
+    "acorn-jsx": "^5.3.1",
     "acorn-numeric-separator": "^0.3.6",
     "acorn-static-class-features": "^0.2.4",
     "acorn-walk": "^8.0.0",
@@ -90,30 +90,30 @@
     "date-time": "^3.1.0",
     "es5-shim": "^4.5.14",
     "es6-shim": "^0.35.5",
-    "eslint": "^7.6.0",
+    "eslint": "^7.9.0",
     "eslint-plugin-import": "^2.22.0",
     "execa": "^4.0.3",
     "fixturify": "^2.1.0",
     "hash.js": "^1.1.7",
-    "husky": "^4.2.5",
+    "husky": "^4.3.0",
     "is-reference": "^1.2.1",
-    "lint-staged": "^10.2.11",
+    "lint-staged": "^10.3.0",
     "locate-character": "^2.0.5",
     "magic-string": "^0.25.7",
     "markdownlint-cli": "^0.23.2",
     "micromatch": "^4.0.2",
-    "mocha": "^8.1.1",
-    "node-fetch": "^2.6.0",
+    "mocha": "^8.1.3",
+    "node-fetch": "^2.6.1",
     "nyc": "^15.1.0",
-    "prettier": "^2.0.5",
-    "pretty-bytes": "^5.3.0",
+    "prettier": "^2.1.2",
+    "pretty-bytes": "^5.4.1",
     "pretty-ms": "^7.0.0",
     "require-relative": "^0.8.7",
     "requirejs": "^2.3.6",
-    "rollup": "^2.23.1",
+    "rollup": "^2.26.11",
     "rollup-plugin-license": "^2.2.0",
     "rollup-plugin-string": "^3.0.0",
-    "rollup-plugin-terser": "^7.0.0",
+    "rollup-plugin-terser": "^7.0.2",
     "rollup-plugin-thatworks": "^1.0.4",
     "rollup-plugin-typescript": "^1.0.1",
     "rollup-pluginutils": "^2.8.2",
@@ -123,13 +123,13 @@
     "source-map": "^0.7.3",
     "source-map-support": "^0.5.19",
     "sourcemap-codec": "^1.4.8",
-    "systemjs": "^6.5.0",
-    "terser": "^5.0.0",
+    "systemjs": "^6.6.1",
+    "terser": "^5.3.1",
     "tslib": "^2.0.1",
     "tslint": "^6.1.3",
-    "typescript": "^3.9.7",
+    "typescript": "^4.0.2",
     "url-parse": "^1.4.7",
-    "yargs-parser": "^19.0.1"
+    "yargs-parser": "^20.0.0"
   },
   "files": [
     "dist/**/*.js",

src/ast/nodes/IfStatement.ts

@@ -117,7 +117,7 @@ export default class IfStatement extends StatementBase implements DeoptimizableE
 				hoistedDeclarations.push(...this.alternateScope!.hoistedDeclarations);
 			}
 		}
-		renderHoistedDeclarations(hoistedDeclarations, this.start, code);
+		this.renderHoistedDeclarations(hoistedDeclarations, code);
 	}
 
 	private getTestValue(): LiteralValueOrUnknown {
@@ -170,6 +170,27 @@ export default class IfStatement extends StatementBase implements DeoptimizableE
 		}
 	}
 
+	private renderHoistedDeclarations(hoistedDeclarations: Identifier[], code: MagicString) {
+		const hoistedVars = [
+			...new Set(
+				hoistedDeclarations.map(identifier => {
+					const variable = identifier.variable!;
+					return variable.included ? variable.getName() : '';
+				})
+			)
+		]
+			.filter(Boolean)
+			.join(', ');
+		if (hoistedVars) {
+			const parentType = this.parent.type;
+			const needsBraces = parentType !== NodeType.Program && parentType !== NodeType.BlockStatement;
+			code.prependRight(this.start, `${needsBraces ? '{ ' : ''}var ${hoistedVars}; `);
+			if (needsBraces) {
+				code.appendLeft(this.end, ` }`);
+			}
+		}
+	}
+
 	private shouldKeepAlternateBranch() {
 		let currentParent = this.parent;
 		do {
@@ -184,16 +205,3 @@ export default class IfStatement extends StatementBase implements DeoptimizableE
 		return false;
 	}
 }
-
-function renderHoistedDeclarations(
-	hoistedDeclarations: Identifier[],
-	prependPosition: number,
-	code: MagicString
-) {
-	const hoistedVars = [
-		...new Set(hoistedDeclarations.map(identifier => identifier.variable!.getName()))
-	].join(', ');
-	if (hoistedVars) {
-		code.prependRight(prependPosition, `var ${hoistedVars}; `);
-	}
-}

src/utils/options/normalizeInputOptions.ts

@@ -26,6 +26,7 @@ import { defaultOnWarn, GenericConfigObject, warnUnknownOptions } from './option
 export interface CommandConfigObject {
 	external: (string | RegExp)[];
 	globals: { [id: string]: string } | undefined;
+
 	[key: string]: unknown;
 }
 
@@ -133,10 +134,7 @@ const getIdMatcher = <T extends Array<any>>(
 				ids.add(value);
 			}
 		}
-		return (id => ids.has(id) || matchers.some(matcher => matcher.test(id))) as (
-			id: string,
-			...args: T
-		) => boolean;
+		return (id: string, ..._args: T) => ids.has(id) || matchers.some(matcher => matcher.test(id));
 	}
 	return () => false;
 };

test/form/samples/hoisted-variable-if-stmt/_expected.js

@@ -5,7 +5,9 @@ if (foo) {
 }
 
 {
-    var bar = true;
+    var bar; {
+        bar = true;
+    }
 }
 
 if (bar) {

test/form/samples/hoisted-variable-if-stmt/main.js

@@ -1,13 +1,18 @@
 if (false) {
     var foo = true;
+    var unused;
 }
 
 if (foo) {
     console.log("nope");
 }
 
 if (true) {
-    var bar = true;
+    if (false) {
+        var bar = false;
+    } else {
+        bar = true;
+    }
 }
 
 if (bar) {

test/function/samples/hoisted-variable-if-else/_config.js

@@ -0,0 +1,11 @@
+const assert = require('assert');
+
+module.exports = {
+	description: 'handles hoisted variables in chained if statements',
+	exports(exports) {
+		exports.test(true);
+		assert.strictEqual(exports.result, 'first');
+		exports.test(false);
+		assert.strictEqual(exports.result, 'third');
+	}
+};

test/function/samples/hoisted-variable-if-else/main.js

@@ -0,0 +1,19 @@
+export let result;
+
+export function test(value) {
+	if (value) {
+		v = 'first';
+		result = v;
+	} else if (false) {
+		var v, w;
+		result = 'second';
+	} else {
+		v = 'third';
+		result = v;
+	}
+
+	for (var foo of []) if (false) var x; else {
+		x = 'fourth';
+		result = x;
+	}
+}