Retain TDZ violations in common scenarios and fix similar var bugs

src/ast/nodes/Identifier.ts

@@ -9,6 +9,7 @@ import { HasEffectsContext, InclusionContext } from '../ExecutionContext';
 import { NodeEvent } from '../NodeEvents';
 import FunctionScope from '../scopes/FunctionScope';
 import { EMPTY_PATH, ObjectPath, PathTracker } from '../utils/PathTracker';
+import { UNDEFINED_EXPRESSION } from '../values';
 import GlobalVariable from '../variables/GlobalVariable';
 import LocalVariable from '../variables/LocalVariable';
 import Variable from '../variables/Variable';
@@ -20,6 +21,14 @@ import { PatternNode } from './shared/Pattern';
 
 export type IdentifierWithVariable = Identifier & { variable: Variable };
 
+const tdzInitTypesToIgnore = {
+	__proto__: null,
+	ArrowFunctionExpression: true,
+	ClassExpression: true,
+	FunctionExpression: true,
+	ObjectExpression: true
+};
+
 export default class Identifier extends NodeBase implements PatternNode {
 	name!: string;
 	type!: NodeType.tIdentifier;
@@ -109,6 +118,7 @@ export default class Identifier extends NodeBase implements PatternNode {
 
 	hasEffects(): boolean {
 		if (!this.deoptimized) this.applyDeoptimizations();
+		if (this.isPossibleTDZ()) return true;
 		return (
 			(this.context.options.treeshake as NormalizedTreeshakingOptions).unknownGlobalSideEffects &&
 			this.variable instanceof GlobalVariable &&
@@ -182,6 +192,68 @@ export default class Identifier extends NodeBase implements PatternNode {
 		}
 	}
 
+	protected isPossibleTDZ(): boolean {
+		// TDZ-style violations cannot generally be known without executing the code -
+		// just check the most common statically analyzable scenarios.
+		// Note: the boolean return value is a best guess effort, and may be inaccurate.
+
+		if (!(this.variable instanceof LocalVariable)) return false;
+
+		let init, init_parent;
+		if (
+			(init = (this.variable as any).init) &&
+			init !== UNDEFINED_EXPRESSION &&
+			(init_parent = (init as any).parent) &&
+			init_parent.type == 'VariableDeclarator' &&
+			init_parent.id.variable === this.variable
+		) {
+			if (
+				this !== init_parent.id &&
+				this.scope &&
+				init_parent.id.scope === this.scope &&
+				this.start < init.start
+			) {
+				// same scope variable access before its declaration:
+				//   c ? 1 : 2; const c = true;
+				//   console.log(v ? 3 : 4); var v = true;
+				return true;
+			}
+
+			if (
+				this.start >= init.start &&
+				this.start < init.end &&
+				!(init_parent.init.type in tdzInitTypesToIgnore)
+			) {
+				// any scope variable access within its own declaration init:
+				//   let x = x + 1;
+				return true;
+			}
+		}
+
+		let decl_id, decl, decl_parent;
+		if (
+			(!init || init === UNDEFINED_EXPRESSION) &&
+			this.parent.type !== 'VariableDeclarator' &&
+			this.variable.declarations.length === 1 &&
+			(decl_id = this.variable.declarations[0] as any) &&
+			decl_id.type === 'Identifier' &&
+			(decl = decl_id.parent as any) &&
+			decl.type === 'VariableDeclarator' &&
+			(decl_parent = decl.parent as any) &&
+			decl_parent.type === 'VariableDeclaration' &&
+			decl_parent.kind === 'let' &&
+			this.scope &&
+			decl_id.scope === this.scope &&
+			this.start < decl.start
+		) {
+			// a `let` variable without an init accessed before declaration:
+			//   x; let x;
+			return true;
+		}
+
+		return false;
+	}
+
 	private disallowImportReassignment() {
 		return this.context.error(
 			{

test/form/samples/tdz-common/_config.js

@@ -0,0 +1,3 @@
+module.exports = {
+	description: 'preserve common TDZ violations'
+};

test/form/samples/tdz-common/_expected.js

@@ -0,0 +1,40 @@
+console.log(function() {
+	if (x) return "HELLO"; // TDZ
+	const x = 1;           // keep
+}());
+
+const C = 1 + C + 2; // TDZ
+let L = L;           // TDZ
+var V = V;           // TODO: uncommon scenario, but should be dropped
+console.log("X+" ); // optimize
+
+console.log(Y ? "Y+" : "Y-"); // TDZ
+const Y = 2;                  // keep
+
+console.log(Z ? "Z+" : "Z-"); // TDZ
+const Z = 3;                  // keep
+console.log("Z+" ); // keep
+
+console.log(obj.x.y ? 1 : 2); // TDZ
+const obj = {                 // keep
+	x: {
+		y: true
+	}
+};
+console.log(3 ); // keep
+
+L2;              // TDZ for L2
+L3 = 20;    // TDZ for L3
+let L2, L3;  // keep L2, L3
+L3 = 30;             // keep
+
+// Note that typical var/const/let use is still optimized
+(function() {
+	console.log(A ? "A" : "!A");
+	var A = 1;
+	console.log("A" );
+	console.log("B");
+	console.log("B" );
+	console.log("C" );
+	console.log("D" );
+})();

test/form/samples/tdz-common/main.js

@@ -0,0 +1,53 @@
+console.log(function() {
+	if (x) return "HELLO"; // TDZ
+	const x = 1;           // keep
+	return "WORLD";        // not reached
+}());
+
+const unused1 = 1;   // drop
+let unused2 = 2;     // drop
+var unused3 = 3;     // drop
+
+const C = 1 + C + 2; // TDZ
+let L = L;           // TDZ
+var V = V;           // TODO: uncommon scenario, but should be dropped
+
+const X = 1;                  // drop
+console.log(X ? "X+" : "X-"); // optimize
+
+console.log(Y ? "Y+" : "Y-"); // TDZ
+const Y = 2;                  // keep
+
+console.log(Z ? "Z+" : "Z-"); // TDZ
+const Z = 3;                  // keep
+console.log(Z ? "Z+" : "Z-"); // keep
+
+console.log(obj.x.y ? 1 : 2); // TDZ
+const obj = {                 // keep
+	x: {
+		y: true
+	}
+};
+console.log(obj.x.y ? 3 : 4); // keep
+
+V2, L2;              // TDZ for L2
+var V2;              // drop
+V3 = 10, L3 = 20;    // TDZ for L3
+let L1, L2, L3, L4;  // keep L2, L3
+var V3;              // drop
+L3 = 30;             // keep
+L4 = 40;             // drop
+
+// Note that typical var/const/let use is still optimized
+(function() {
+	console.log(A ? "A" : "!A");
+	var A = 1, B = 2;
+	const C = 3;
+	let D = 4;
+	console.log(A ? "A" : "!A");
+	if (B) console.log("B");
+	else console.log("!B");
+	console.log(B ? "B" : "!B");
+	console.log(C ? "C" : "!C");
+	console.log(D ? "D" : "!D");
+})();

test/function/samples/use-var-before-decl/_config.js

@@ -0,0 +1,3 @@
+module.exports = {
+	description: 'exercise `var` variables before their declarations'
+};

test/function/samples/use-var-before-decl/main.js

@@ -0,0 +1,47 @@
+var results = [], log = x => results.push(x);
+
+(function () {
+	var a = "PASS1";
+	for (var b = 2; --b >= 0; ) {
+		(function() {
+			var c = function() {
+				return 1;
+			}(c && (a = "FAIL1"));
+		})();
+	}
+	log(a);
+})();
+
+log(a ? "FAIL2" : "PASS2");
+var a = 1;
+
+var b = 2;
+log(b ? "PASS3" : "FAIL3");
+
+log(c ? "FAIL4" : "PASS4");
+var c = 3;
+log(c ? "PASS5" : "FAIL5");
+
+/* TODO: multi-scope var-use-before-defined bug to be addressed in a future PR
+(function () {
+	var first = state();
+	var on = true;
+	var obj = {
+		state: state
+	};
+	log(first, obj.state());
+	function state() {
+		return on ? "ON" : "OFF";
+	}
+})();
+*/
+
+/* TODO: previously existing bug still fails, but in a different way
+	log(function() {
+		if (x) return "HELLO";
+		var x = 1;
+		return "WORLD";
+	}());
+*/
+
+assert.strictEqual(results.join(" "), "PASS1 PASS2 PASS3 PASS4 PASS5");