Merge branch 'double-declarations'

CHANGELOG.md

@@ -6,6 +6,7 @@
 * Handle undefined values when evaluating conditionals ([#2264](https://github.com/rollup/rollup/pull/2264))
 * Handle known undefined properties when evaluating conditionals ([#2265](https://github.com/rollup/rollup/pull/2265))
 * Access watch events via the plugin context ([#2261](https://github.com/rollup/rollup/pull/2261))
+* Fix issue when re-declaring variables, track reassignments in more cases ([#2279](https://github.com/rollup/rollup/pull/2279))
 
 ## 0.60.7
 *2018-06-14*

src/ast/nodes/ArrayExpression.ts

@@ -5,7 +5,8 @@ import {
 	getMemberReturnExpressionWhenCalled,
 	hasMemberEffectWhenCalled,
 	ObjectPath,
-	UNKNOWN_EXPRESSION
+	UNKNOWN_EXPRESSION,
+	UNKNOWN_PATH
 } from '../values';
 import * as NodeType from './NodeType';
 import { ExpressionNode, NodeBase } from './shared/Node';
@@ -15,6 +16,13 @@ export default class ArrayExpression extends NodeBase {
 	type: NodeType.tArrayExpression;
 	elements: (ExpressionNode | SpreadElement | null)[];
 
+	bind() {
+		super.bind();
+		for (const element of this.elements) {
+			if (element !== null) element.reassignPath(UNKNOWN_PATH);
+		}
+	}
+
 	getReturnExpressionWhenCalledAtPath(path: ObjectPath) {
 		if (path.length !== 1) return UNKNOWN_EXPRESSION;
 		return getMemberReturnExpressionWhenCalled(arrayMembers, path[0]);

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);

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();

src/ast/nodes/AssignmentPattern.ts

@@ -1,5 +1,5 @@
 import { ExecutionPathOptions } from '../ExecutionPathOptions';
-import { EMPTY_PATH, ObjectPath } from '../values';
+import { EMPTY_PATH, ObjectPath, UNKNOWN_PATH } from '../values';
 import * as NodeType from './NodeType';
 import { ExpressionEntity } from './shared/Expression';
 import { ExpressionNode, NodeBase } from './shared/Node';
@@ -13,9 +13,10 @@ export default class AssignmentPattern extends NodeBase implements PatternNode {
 	bind() {
 		super.bind();
 		this.left.reassignPath(EMPTY_PATH);
+		this.right.reassignPath(UNKNOWN_PATH);
 	}
 
-	declare(kind: string, init: ExpressionEntity | null) {
+	declare(kind: string, init: ExpressionEntity) {
 		this.left.declare(kind, init);
 	}
 

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) {

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).additionalInitializers !== null
+		) {
+			(<LocalVariable>this.variable).consolidateInitializers();
+		}
 	}
 
-	declare(kind: string, init: ExpressionEntity | null) {
+	declare(kind: string, init: ExpressionEntity) {
 		switch (kind) {
 			case 'var':
 			case 'function':

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);
 		}

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);
 	}
 

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);
 	}
 

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) {

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();
 	}

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,

src/ast/variables/LocalVariable.ts

@@ -21,13 +21,16 @@ const MAX_PATH_DEPTH = 7;
 
 export default class LocalVariable extends Variable {
 	declarations: (Identifier | ExportDefaultDeclaration)[];
-	init: ExpressionEntity;
-	reassignmentTracker: EntityPathTracker;
+	init: ExpressionEntity | null;
+	isLocal: true;
+	additionalInitializers: ExpressionEntity[] | null = null;
+
+	private reassignmentTracker: EntityPathTracker;
 
 	constructor(
 		name: string,
 		declarator: Identifier | ExportDefaultDeclaration | null,
-		init: ExpressionEntity,
+		init: ExpressionEntity | null,
 		reassignmentTracker: EntityPathTracker
 	) {
 		super(name);
@@ -36,8 +39,25 @@ export default class LocalVariable extends Variable {
 		this.reassignmentTracker = reassignmentTracker;
 	}
 
-	addDeclaration(identifier: Identifier) {
+	addDeclaration(identifier: Identifier, init: ExpressionEntity | null) {
 		this.declarations.push(identifier);
+		if (this.additionalInitializers === null) {
+			this.additionalInitializers = this.init === null ? [] : [this.init];
+			this.init = UNKNOWN_EXPRESSION;
+			this.isReassigned = true;
+		}
+		if (init !== null) {
+			this.additionalInitializers.push(init);
+		}
+	}
+
+	consolidateInitializers() {
+		if (this.additionalInitializers !== null) {
+			for (const initializer of this.additionalInitializers) {
+				initializer.reassignPath(UNKNOWN_PATH);
+			}
+			this.additionalInitializers = null;
+		}
 	}
 
 	getLiteralValueAtPath(
@@ -151,3 +171,5 @@ export default class LocalVariable extends Variable {
 		}
 	}
 }
+
+LocalVariable.prototype.isLocal = true;

test/function/index.js

@@ -15,8 +15,12 @@ describe('function', () => {
 			if (dir[0] === '.') return; // .DS_Store...
 
 			const config = loadConfig(samples + '/' + dir + '/_config.js');
-			if (!config) return;
-
+			if (
+				!config ||
+				(config.minNodeVersion &&
+					config.minNodeVersion > Number(/^v(\d+)/.exec(process.version)[1]))
+			)
+				return;
 			(config.skip ? it.skip : config.solo ? it.only : it)(dir, () => {
 				process.chdir(samples + '/' + dir);
 

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'
+};

test/function/samples/handles-double-declarations/foobar.js

@@ -0,0 +1,2 @@
+export var foo = { foo: true };
+export var bar = { bar: true };

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);

test/function/samples/reassign-array-literal-elements/_config.js

@@ -0,0 +1,3 @@
+module.exports = {
+	description: 'makes sure reassignments of array elements are tracked'
+};

test/function/samples/reassign-array-literal-elements/main.js

@@ -0,0 +1,8 @@
+var foo = {x: true};
+var bar = {x: true};
+var baz = [foo, bar];
+
+baz[0].x = false;
+baz[1].x = false;
+
+if (foo.x) assert.fail('foo was not reassigned');

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'
+};

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');

test/function/samples/reassign-pattern-defaults/_config.js

@@ -0,0 +1,4 @@
+module.exports = {
+	description: 'makes sure reassignments of pattern defaults are tracked',
+	minNodeVersion: 6
+};

test/function/samples/reassign-pattern-defaults/main.js

@@ -0,0 +1,6 @@
+var foo = {x: true};
+var {bar: bar = foo} = {};
+
+bar.x = false;
+
+if (foo.x) assert.fail('foo was not reassigned');