* No longer reassign variable initializers during initialization phase

* Reassign all initializers of variables with multiple declarations
rollup · Jun 20, 2018 · 6bedd8c · 6bedd8c
1 parent 41d6fe8
commit 6bedd8c
Show file tree

Hide file tree

Showing 17 changed files with 75 additions and 17 deletions.
diff --git a/src/ast/nodes/ArrayPattern.ts b/src/ast/nodes/ArrayPattern.ts
@@ -9,7 +9,7 @@ export default class ArrayPattern extends NodeBase implements PatternNode {
 	type: NodeType.tArrayPattern;
 	elements: (PatternNode | null)[];
 
-	declare(kind: string, _init: ExpressionEntity | null) {
+	declare(kind: string, _init: ExpressionEntity) {
 		for (const element of this.elements) {
 			if (element !== null) {
 				element.declare(kind, UNKNOWN_EXPRESSION);

diff --git a/src/ast/nodes/ArrowFunctionExpression.ts b/src/ast/nodes/ArrowFunctionExpression.ts
@@ -53,7 +53,7 @@ export default class ArrowFunctionExpression extends NodeBase {
 	initialise() {
 		this.included = false;
 		for (const param of this.params) {
-			param.declare('parameter', null);
+			param.declare('parameter', UNKNOWN_EXPRESSION);
 		}
 		if (this.body instanceof BlockStatement) {
 			this.body.addImplicitReturnExpressionToScope();

diff --git a/src/ast/nodes/AssignmentPattern.ts b/src/ast/nodes/AssignmentPattern.ts
@@ -15,7 +15,7 @@ export default class AssignmentPattern extends NodeBase implements PatternNode {
 		this.left.reassignPath(EMPTY_PATH);
 	}
 
-	declare(kind: string, init: ExpressionEntity | null) {
+	declare(kind: string, init: ExpressionEntity) {
 		this.left.declare(kind, init);
 	}
 

diff --git a/src/ast/nodes/CatchClause.ts b/src/ast/nodes/CatchClause.ts
@@ -1,5 +1,6 @@
 import CatchScope from '../scopes/CatchScope';
 import Scope from '../scopes/Scope';
+import { UNKNOWN_EXPRESSION } from '../values';
 import BlockStatement from './BlockStatement';
 import * as NodeType from './NodeType';
 import { GenericEsTreeNode, NodeBase } from './shared/Node';
@@ -19,7 +20,7 @@ export default class CatchClause extends NodeBase {
 
 	initialise() {
 		this.included = false;
-		this.param.declare('parameter', null);
+		this.param.declare('parameter', UNKNOWN_EXPRESSION);
 	}
 
 	parseNode(esTreeNode: GenericEsTreeNode) {

diff --git a/src/ast/nodes/Identifier.ts b/src/ast/nodes/Identifier.ts
@@ -7,6 +7,7 @@ import { ExecutionPathOptions } from '../ExecutionPathOptions';
 import FunctionScope from '../scopes/FunctionScope';
 import { ImmutableEntityPathTracker } from '../utils/ImmutableEntityPathTracker';
 import { LiteralValueOrUnknown, ObjectPath, UNKNOWN_EXPRESSION, UNKNOWN_VALUE } from '../values';
+import LocalVariable from '../variables/LocalVariable';
 import Variable from '../variables/Variable';
 import AssignmentExpression from './AssignmentExpression';
 import * as NodeType from './NodeType';
@@ -33,9 +34,16 @@ export default class Identifier extends NodeBase {
 			this.variable = this.scope.findVariable(this.name);
 			this.variable.addReference(this);
 		}
+		if (
+			this.variable !== null &&
+			(<LocalVariable>this.variable).isLocal &&
+			!(<LocalVariable>this.variable).bound
+		) {
+			(<LocalVariable>this.variable).bind();
+		}
 	}
 
-	declare(kind: string, init: ExpressionEntity | null) {
+	declare(kind: string, init: ExpressionEntity) {
 		switch (kind) {
 			case 'var':
 			case 'function':

diff --git a/src/ast/nodes/ObjectPattern.ts b/src/ast/nodes/ObjectPattern.ts
@@ -10,7 +10,7 @@ export default class ObjectPattern extends NodeBase implements PatternNode {
 	type: NodeType.tObjectPattern;
 	properties: AssignmentProperty[];
 
-	declare(kind: string, init: ExpressionEntity | null) {
+	declare(kind: string, init: ExpressionEntity) {
 		for (const property of this.properties) {
 			property.declare(kind, init);
 		}

diff --git a/src/ast/nodes/Property.ts b/src/ast/nodes/Property.ts
@@ -33,7 +33,7 @@ export default class Property extends NodeBase {
 		}
 	}
 
-	declare(kind: string, _init: ExpressionEntity | null) {
+	declare(kind: string, _init: ExpressionEntity) {
 		this.value.declare(kind, UNKNOWN_EXPRESSION);
 	}
 

diff --git a/src/ast/nodes/RestElement.ts b/src/ast/nodes/RestElement.ts
@@ -9,7 +9,7 @@ export default class RestElement extends NodeBase implements PatternNode {
 	type: NodeType.tRestElement;
 	argument: PatternNode;
 
-	declare(kind: string, _init: ExpressionEntity | null) {
+	declare(kind: string, _init: ExpressionEntity) {
 		this.argument.declare(kind, UNKNOWN_EXPRESSION);
 	}
 

diff --git a/src/ast/nodes/VariableDeclarator.ts b/src/ast/nodes/VariableDeclarator.ts
@@ -1,4 +1,4 @@
-import { ObjectPath } from '../values';
+import { ObjectPath, UNDEFINED_EXPRESSION } from '../values';
 import * as NodeType from './NodeType';
 import { ExpressionNode, NodeBase } from './shared/Node';
 import { PatternNode } from './shared/Pattern';
@@ -9,7 +9,7 @@ export default class VariableDeclarator extends NodeBase {
 	init: ExpressionNode | null;
 
 	declareDeclarator(kind: string) {
-		this.id.declare(kind, this.init);
+		this.id.declare(kind, this.init || UNDEFINED_EXPRESSION);
 	}
 
 	reassignPath(path: ObjectPath) {

diff --git a/src/ast/nodes/shared/FunctionNode.ts b/src/ast/nodes/shared/FunctionNode.ts
@@ -72,7 +72,7 @@ export default class FunctionNode extends NodeBase {
 			this.id.declare('function', this);
 		}
 		for (const param of this.params) {
-			param.declare('parameter', null);
+			param.declare('parameter', UNKNOWN_EXPRESSION);
 		}
 		this.body.addImplicitReturnExpressionToScope();
 	}

diff --git a/src/ast/scopes/Scope.ts b/src/ast/scopes/Scope.ts
@@ -3,7 +3,7 @@ import ExportDefaultDeclaration from '../nodes/ExportDefaultDeclaration';
 import Identifier from '../nodes/Identifier';
 import { ExpressionEntity } from '../nodes/shared/Expression';
 import { EntityPathTracker } from '../utils/EntityPathTracker';
-import { EMPTY_PATH, UNDEFINED_EXPRESSION } from '../values';
+import { UNDEFINED_EXPRESSION } from '../values';
 import ArgumentsVariable from '../variables/ArgumentsVariable';
 import ExportDefaultVariable from '../variables/ExportDefaultVariable';
 import ExternalVariable from '../variables/ExternalVariable';
@@ -38,9 +38,7 @@ export default class Scope {
 	) {
 		const name = identifier.name;
 		if (this.variables[name]) {
-			const variable = <LocalVariable>this.variables[name];
-			variable.addDeclaration(identifier);
-			variable.reassignPath(EMPTY_PATH);
+			(<LocalVariable>this.variables[name]).addDeclaration(identifier, init);
 		} else {
 			this.variables[name] = new LocalVariable(
 				identifier.name,

diff --git a/src/ast/variables/LocalVariable.ts b/src/ast/variables/LocalVariable.ts
@@ -22,7 +22,11 @@ const MAX_PATH_DEPTH = 7;
 export default class LocalVariable extends Variable {
 	declarations: (Identifier | ExportDefaultDeclaration)[];
 	init: ExpressionEntity;
-	reassignmentTracker: EntityPathTracker;
+	isLocal: true;
+	bound: boolean = false;
+
+	private initialisers: ExpressionEntity[];
+	private reassignmentTracker: EntityPathTracker;
 
 	constructor(
 		name: string,
@@ -33,17 +37,32 @@ export default class LocalVariable extends Variable {
 		super(name);
 		this.declarations = declarator ? [declarator] : [];
 		this.init = init;
+		this.initialisers = [init];
 		this.reassignmentTracker = reassignmentTracker;
 	}
 
-	addDeclaration(identifier: Identifier) {
+	addDeclaration(identifier: Identifier, init: ExpressionEntity) {
 		this.declarations.push(identifier);
+		this.initialisers.push(init);
+	}
+
+	bind() {
+		if (this.bound) return;
+		this.bound = true;
+		if (this.initialisers.length > 1) {
+			this.isReassigned = true;
+			for (const initialiser of this.initialisers) {
+				initialiser.reassignPath(UNKNOWN_PATH);
+			}
+			this.init = UNKNOWN_EXPRESSION;
+		}
 	}
 
 	getLiteralValueAtPath(
 		path: ObjectPath,
 		recursionTracker: ImmutableEntityPathTracker
 	): LiteralValueOrUnknown {
+		if (!this.bound) this.bind();
 		if (
 			this.isReassigned ||
 			!this.init ||
@@ -59,6 +78,7 @@ export default class LocalVariable extends Variable {
 		path: ObjectPath,
 		recursionTracker: ImmutableEntityPathTracker
 	): ExpressionEntity {
+		if (!this.bound) this.bind();
 		if (
 			this.isReassigned ||
 			!this.init ||
@@ -138,6 +158,7 @@ export default class LocalVariable extends Variable {
 	}
 
 	reassignPath(path: ObjectPath) {
+		if (!this.bound) this.bind();
 		if (path.length > MAX_PATH_DEPTH) return;
 		if (!(this.isReassigned || this.reassignmentTracker.track(this, path))) {
 			if (path.length === 0) {
@@ -151,3 +172,5 @@ export default class LocalVariable extends Variable {
 		}
 	}
 }
+
+LocalVariable.prototype.isLocal = true;
diff --git a/test/function/samples/handles-double-declarations/_config.js b/test/function/samples/handles-double-declarations/_config.js
@@ -0,0 +1,3 @@
+module.exports = {
+	description: 'does not fail for double declarations with initializers from other modules'
+};
diff --git a/test/function/samples/handles-double-declarations/foobar.js b/test/function/samples/handles-double-declarations/foobar.js
@@ -0,0 +1,2 @@
+export var foo = { foo: true };
+export var bar = { bar: true };
diff --git a/test/function/samples/handles-double-declarations/main.js b/test/function/samples/handles-double-declarations/main.js
@@ -0,0 +1,6 @@
+import { foo, bar } from './foobar.js';
+
+var baz = foo;
+var baz = bar;
+
+assert.ok(baz.bar);
diff --git a/test/function/samples/reassign-double-declarations/_config.js b/test/function/samples/reassign-double-declarations/_config.js
@@ -0,0 +1,3 @@
+module.exports = {
+	description: 'makes sure reassignments of double declared variables and their initializers are tracked'
+};
diff --git a/test/function/samples/reassign-double-declarations/main.js b/test/function/samples/reassign-double-declarations/main.js
@@ -0,0 +1,14 @@
+var bar = {x: true};
+var foo = bar;
+reassignFooX();
+
+var baz = {x: true};
+var foo = baz;
+reassignFooX();
+
+function reassignFooX() {
+	foo.x = false;
+}
+
+if (bar.x) assert.fail('bar was not reassigned');
+if (baz.x) assert.fail('baz was not reassigned');