* Add ExecutionPathOptions to all bind calls

* Make sure bindxyz and hasEffectsWhenxyz are as symmetric as possible so that in the future, we could make this the same function and possibly remove assignment/call binding from the bind phase * Add the calling node to CallOptions to have a break-off condition that works for well return value assertions This all resolves a maximum call stack error
rollup · Nov 8, 2017 · 52f4684 · 52f4684
1 parent e2daaec
commit 52f4684
Show file tree

Hide file tree

Showing 35 changed files with 230 additions and 223 deletions.
diff --git a/src/ast/CallOptions.js b/src/ast/CallOptions.js
@@ -1,29 +1,15 @@
-import Immutable from 'immutable';
-
-const RESULT_KEY = {};
-
 export default class CallOptions {
 	static create ( callOptions ) {
-		return new this( callOptions, Immutable.Map() );
+		return new this( callOptions );
 	}
 
-	constructor ( { withNew = false, args = [] } = {}, nodesCalledAtPath ) {
+	constructor ( { withNew = false, args = [], caller } = {} ) {
 		this.withNew = withNew;
 		this.args = args;
-		this._nodesCalledAtPath = nodesCalledAtPath;
-	}
-
-	addCalledNodeAtPath ( path, node ) {
-		return new this.constructor( this, this._nodesCalledAtPath.setIn( [ node, ...path, RESULT_KEY ], true ) );
+		this.caller = caller;
 	}
 
 	equals ( callOptions ) {
-		return this.withNew === callOptions.withNew
-			&& this.args.length === callOptions.args.length
-			&& this.args.every( ( parameter, index ) => parameter === callOptions.args[ index ] );
-	}
-
-	hasNodeBeenCalledAtPath ( path, node ) {
-		return this._nodesCalledAtPath.getIn( [ node, ...path, RESULT_KEY ] );
+		return callOptions && this.caller === callOptions.caller;
 	}
 }
diff --git a/src/ast/Node.js b/src/ast/Node.js
@@ -19,19 +19,6 @@ export default class Node {
 		this.bindNode();
 	}
 
-	/**
-	 * Override this to bind assignments to variables and do any initialisations that
-	 * require the scopes to be populated with variables.
-	 */
-	bindNode () {}
-
-	/**
-	 * Override to control on which children "bind" is called.
-	 */
-	bindChildren () {
-		this.eachChild( child => child.bind() );
-	}
-
 	/**
 	 * Bind an expression as an assignment to a node given a path.
 	 * E.g., node.bindAssignmentAtPath(['x', 'y'], otherNode) is called when otherNode
@@ -40,16 +27,31 @@ export default class Node {
 	 * always returns true for this node. Otherwise it should be overridden.
 	 * @param {String[]} path
 	 * @param {Node} expression
+	 * @param {ExecutionPathOptions} options
 	 */
-	bindAssignmentAtPath ( path, expression ) {}
+	bindAssignmentAtPath ( path, expression, options ) {}
 
 	/**
 	 * Binds the arguments a node is called with to this node and possibly its parameters.
 	 * Should usually be overridden together with hasEffectsWhenCalled.
 	 * @param {String[]} path
 	 * @param {CallOptions} callOptions
+	 * @param {ExecutionPathOptions} options
 	 */
-	bindCallAtPath ( path, callOptions ) {}
+	bindCallAtPath ( path, callOptions, options ) {}
+
+	/**
+	 * Override to control on which children "bind" is called.
+	 */
+	bindChildren () {
+		this.eachChild( child => child.bind() );
+	}
+
+	/**
+	 * Override this to bind assignments to variables and do any initialisations that
+	 * require the scopes to be populated with variables.
+	 */
+	bindNode () {}
 
 	eachChild ( callback ) {
 		this.keys.forEach( key => {
@@ -69,8 +71,9 @@ export default class Node {
 	 * @param {String[]} path
 	 * @param {CallOptions} callOptions
 	 * @param {Function} callback
+	 * @param {ExecutionPathOptions} options
 	 */
-	forEachReturnExpressionWhenCalledAtPath ( path, callOptions, callback ) {}
+	forEachReturnExpressionWhenCalledAtPath ( path, callOptions, callback, options ) {}
 
 	getValue () {
 		return UNKNOWN_VALUE;

diff --git a/src/ast/nodes/ArrayPattern.js b/src/ast/nodes/ArrayPattern.js
@@ -2,15 +2,14 @@ import Node from '../Node.js';
 import { UNKNOWN_ASSIGNMENT } from '../values';
 
 export default class ArrayPattern extends Node {
-	bindAssignmentAtPath () {
-		this.eachChild( child => child.bindAssignmentAtPath( [], UNKNOWN_ASSIGNMENT ) );
+	bindAssignmentAtPath ( path, expression, options ) {
+		path.length === 0
+		&& this.eachChild( child => child.bindAssignmentAtPath( [], UNKNOWN_ASSIGNMENT, options ) );
 	}
 
 	hasEffectsWhenAssignedAtPath ( path, options ) {
-		if ( path.length > 0 ) {
-			return true;
-		}
-		return this.someChild( child => child.hasEffectsWhenAssignedAtPath( [], options ) );
+		return path.length > 0
+			|| this.someChild( child => child.hasEffectsWhenAssignedAtPath( [], options ) );
 	}
 
 	initialiseAndDeclare ( parentScope, kind ) {

diff --git a/src/ast/nodes/ArrowFunctionExpression.js b/src/ast/nodes/ArrowFunctionExpression.js
@@ -4,9 +4,8 @@ import ReturnValueScope from '../scopes/ReturnValueScope';
 
 export default class ArrowFunctionExpression extends Node {
 	bindCallAtPath ( path, { args } ) {
-		if ( path.length === 0 ) {
-			this.scope.bindCallArguments( args );
-		}
+		path.length === 0
+		&& this.scope.bindCallArguments( args );
 	}
 
 	bindNode () {
@@ -15,10 +14,9 @@ export default class ArrowFunctionExpression extends Node {
 			: this.scope.addReturnExpression( this.body );
 	}
 
-	forEachReturnExpressionWhenCalledAtPath ( path, callOptions, callback ) {
-		if ( path.length === 0 ) {
-			this.scope.forEachReturnExpressionWhenCalled( callback );
-		}
+	forEachReturnExpressionWhenCalledAtPath ( path, callOptions, callback, options ) {
+		path.length === 0
+		&& this.scope.forEachReturnExpressionWhenCalled( callOptions, callback, options );
 	}
 
 	hasEffects () {

diff --git a/src/ast/nodes/AssignmentExpression.js b/src/ast/nodes/AssignmentExpression.js
@@ -1,10 +1,11 @@
 import Node from '../Node.js';
 import disallowIllegalReassignment from './shared/disallowIllegalReassignment.js';
+import ExecutionPathOptions from '../ExecutionPathOptions';
 
 export default class AssignmentExpression extends Node {
 	bindNode () {
 		disallowIllegalReassignment( this.scope, this.left );
-		this.left.bindAssignmentAtPath( [], this.right );
+		this.left.bindAssignmentAtPath( [], this.right, ExecutionPathOptions.create() );
 	}
 
 	hasEffects ( options ) {

diff --git a/src/ast/nodes/AssignmentPattern.js b/src/ast/nodes/AssignmentPattern.js
@@ -1,19 +1,19 @@
 import Node from '../Node.js';
+import ExecutionPathOptions from '../ExecutionPathOptions';
 
 export default class AssignmentPattern extends Node {
 	bindNode () {
-		this.left.bindAssignmentAtPath( [], this.right );
+		this.left.bindAssignmentAtPath( [], this.right, ExecutionPathOptions.create() );
 	}
 
-	bindAssignmentAtPath ( path, expression ) {
-		this.left.bindAssignmentAtPath( path, expression );
+	bindAssignmentAtPath ( path, expression, options ) {
+		path.length === 0
+		&& this.left.bindAssignmentAtPath( path, expression, options );
 	}
 
 	hasEffectsWhenAssignedAtPath ( path, options ) {
-		if ( path.length > 0 ) {
-			return true;
-		}
-		return this.left.hasEffectsWhenAssignedAtPath( [], options );
+		return path.length > 0
+			|| this.left.hasEffectsWhenAssignedAtPath( [], options );
 	}
 
 	initialiseAndDeclare ( parentScope, kind, init ) {

diff --git a/src/ast/nodes/CallExpression.js b/src/ast/nodes/CallExpression.js
@@ -1,25 +1,18 @@
 import Node from '../Node.js';
 import CallOptions from '../CallOptions';
-import StructuredAssignmentTracker from '../variables/StructuredAssignmentTracker';
+import ExecutionPathOptions from '../ExecutionPathOptions';
 
 export default class CallExpression extends Node {
-	bindAssignmentAtPath ( path, expression ) {
-		if ( this._boundExpressions.hasAtPath( path, expression ) ) return;
-		this._boundExpressions.addAtPath( path, expression );
-		this.callee.forEachReturnExpressionWhenCalledAtPath( [], this._callOptions,
-			node => node.bindAssignmentAtPath( path, expression ) );
+	bindAssignmentAtPath ( path, expression, options ) {
+		!options.hasReturnExpressionBeenAssignedAtPath( path, this )
+		&& this.callee.forEachReturnExpressionWhenCalledAtPath( [], this._callOptions, innerOptions => node =>
+			node.bindAssignmentAtPath( path, expression, innerOptions.addAssignedReturnExpressionAtPath( path, this ) ), options );
 	}
 
-	bindCallAtPath ( path, callOptions ) {
-		if ( this._boundCalls.hasAtPath( path, callOptions ) ) return;
-		this._boundCalls.addAtPath( path, callOptions );
-		this.callee.forEachReturnExpressionWhenCalledAtPath( [], this._callOptions,
-			node => node.bindCallAtPath( path, callOptions ) );
-	}
-
-	forEachReturnExpressionWhenCalledAtPath ( path, callOptions, callback ) {
-		this.callee.forEachReturnExpressionWhenCalledAtPath( [], this._callOptions,
-			node => node.forEachReturnExpressionWhenCalledAtPath( path, callOptions, callback ) );
+	bindCallAtPath ( path, callOptions, options ) {
+		!options.hasReturnExpressionBeenCalledAtPath( path, this )
+		&& this.callee.forEachReturnExpressionWhenCalledAtPath( [], this._callOptions, innerOptions => node =>
+			node.bindCallAtPath( path, callOptions, innerOptions.addCalledReturnExpressionAtPath( path, this ) ), options );
 	}
 
 	bindNode () {
@@ -41,7 +34,12 @@ export default class CallExpression extends Node {
 				}, this.start );
 			}
 		}
-		this.callee.bindCallAtPath( [], this._callOptions );
+		this.callee.bindCallAtPath( [], this._callOptions, ExecutionPathOptions.create() );
+	}
+
+	forEachReturnExpressionWhenCalledAtPath ( path, callOptions, callback, options ) {
+		this.callee.forEachReturnExpressionWhenCalledAtPath( [], this._callOptions, innerOptions => node =>
+			node.forEachReturnExpressionWhenCalledAtPath( path, callOptions, callback, innerOptions ), options );
 	}
 
 	hasEffects ( options ) {
@@ -69,10 +67,7 @@ export default class CallExpression extends Node {
 	}
 
 	initialiseNode () {
-		this._callOptions = CallOptions.create( { withNew: false, args: this.arguments } );
-		// To avoid infinite recursions when dealing with recursive functions
-		this._boundExpressions = new StructuredAssignmentTracker();
-		this._boundCalls = new StructuredAssignmentTracker();
+		this._callOptions = CallOptions.create( { withNew: false, args: this.arguments, caller: this } );
 	}
 
 	someReturnExpressionWhenCalledAtPath ( path, callOptions, predicateFunction, options ) {

diff --git a/src/ast/nodes/ClassBody.js b/src/ast/nodes/ClassBody.js
@@ -1,20 +1,18 @@
 import Node from '../Node';
 
 export default class ClassBody extends Node {
-	bindCallAtPath ( path, callOptions ) {
-		if ( path.length === 0 && this.classConstructor ) {
-			this.classConstructor.bindCallAtPath( path, callOptions );
-		}
+	bindCallAtPath ( path, callOptions, options ) {
+		path.length === 0
+		&& this.classConstructor
+		&& this.classConstructor.bindCallAtPath( path, callOptions, options );
 	}
 
 	hasEffectsWhenCalledAtPath ( path, callOptions, options ) {
 		if ( path.length > 0 ) {
 			return true;
 		}
-		if ( this.classConstructor ) {
-			return this.classConstructor.hasEffectsWhenCalledAtPath( [], callOptions, options );
-		}
-		return false;
+		return this.classConstructor
+			&& this.classConstructor.hasEffectsWhenCalledAtPath( [], callOptions, options );
 	}
 
 	initialiseNode () {

diff --git a/src/ast/nodes/ConditionalExpression.js b/src/ast/nodes/ConditionalExpression.js
@@ -2,18 +2,17 @@ import Node from '../Node.js';
 import { UNKNOWN_VALUE } from '../values.js';
 
 export default class ConditionalExpression extends Node {
-	bindAssignmentAtPath ( path, expression ) {
-		if ( path.length > 0 ) {
-			this._forEachRelevantBranch( node => node.bindAssignmentAtPath( path, expression ) );
-		}
+	bindAssignmentAtPath ( path, expression, options ) {
+		path.length > 0
+		&& this._forEachRelevantBranch( node => node.bindAssignmentAtPath( path, expression, options ) );
 	}
 
-	bindCallAtPath ( path, callOptions ) {
-		this._forEachRelevantBranch( node => node.bindCallAtPath( path, callOptions ) );
+	bindCallAtPath ( path, callOptions, options ) {
+		this._forEachRelevantBranch( node => node.bindCallAtPath( path, callOptions, options ) );
 	}
 
-	forEachReturnExpressionWhenCalledAtPath ( path, callOptions, callback ) {
-		this._forEachRelevantBranch( node => node.forEachReturnExpressionWhenCalledAtPath( path, callOptions, callback ) );
+	forEachReturnExpressionWhenCalledAtPath ( path, callOptions, callback, options ) {
+		this._forEachRelevantBranch( node => node.forEachReturnExpressionWhenCalledAtPath( path, callOptions, callback, options ) );
 	}
 
 	getValue () {
@@ -32,18 +31,16 @@ export default class ConditionalExpression extends Node {
 
 	hasEffectsWhenAccessedAtPath ( path, options ) {
 		return path.length > 0
-			&& this._someRelevantBranch( node =>
-				node.hasEffectsWhenAccessedAtPath( path, options ) );
+			&& this._someRelevantBranch( node => node.hasEffectsWhenAccessedAtPath( path, options ) );
 	}
 
 	hasEffectsWhenAssignedAtPath ( path, options ) {
-		return this._someRelevantBranch( node =>
-			node.hasEffectsWhenAssignedAtPath( path, options ) );
+		return path.length === 0
+			|| this._someRelevantBranch( node => node.hasEffectsWhenAssignedAtPath( path, options ) );
 	}
 
 	hasEffectsWhenCalledAtPath ( path, callOptions, options ) {
-		return this._someRelevantBranch( node =>
-			node.hasEffectsWhenCalledAtPath( path, callOptions, options ) );
+		return this._someRelevantBranch( node => node.hasEffectsWhenCalledAtPath( path, callOptions, options ) );
 	}
 
 	initialiseChildren ( parentScope ) {

diff --git a/src/ast/nodes/ForOfStatement.js b/src/ast/nodes/ForOfStatement.js
@@ -1,10 +1,11 @@
 import Statement from './shared/Statement.js';
 import BlockScope from '../scopes/BlockScope';
 import { UNKNOWN_ASSIGNMENT } from '../values';
+import ExecutionPathOptions from '../ExecutionPathOptions';
 
 export default class ForOfStatement extends Statement {
 	bindNode () {
-		this.left.bindAssignmentAtPath( [], UNKNOWN_ASSIGNMENT );
+		this.left.bindAssignmentAtPath( [], UNKNOWN_ASSIGNMENT, ExecutionPathOptions.create() );
 	}
 
 	hasEffects ( options ) {

diff --git a/src/ast/nodes/Identifier.js b/src/ast/nodes/Identifier.js
@@ -3,18 +3,16 @@ import isReference from 'is-reference';
 import { UNKNOWN_ASSIGNMENT } from '../values';
 
 export default class Identifier extends Node {
-	bindAssignmentAtPath ( path, expression ) {
+	bindAssignmentAtPath ( path, expression, options ) {
 		this._bindVariableIfMissing();
-		if ( this.variable ) {
-			this.variable.bindAssignmentAtPath( path, expression );
-		}
+		this.variable
+		&& this.variable.bindAssignmentAtPath( path, expression, options );
 	}
 
-	bindCallAtPath ( path, callOptions ) {
+	bindCallAtPath ( path, callOptions, options ) {
 		this._bindVariableIfMissing();
-		if ( this.variable ) {
-			this.variable.bindCallAtPath( path, callOptions );
-		}
+		this.variable
+		&& this.variable.bindCallAtPath( path, callOptions, options );
 	}
 
 	bindNode () {
@@ -28,11 +26,10 @@ export default class Identifier extends Node {
 		}
 	}
 
-	forEachReturnExpressionWhenCalledAtPath ( path, callOptions, callback ) {
+	forEachReturnExpressionWhenCalledAtPath ( path, callOptions, callback, options ) {
 		this._bindVariableIfMissing();
-		if ( this.variable ) {
-			this.variable.forEachReturnExpressionWhenCalledAtPath( path, callOptions, callback );
-		}
+		this.variable
+		&& this.variable.forEachReturnExpressionWhenCalledAtPath( path, callOptions, callback, options );
 	}
 
 	hasEffectsWhenAccessedAtPath ( path, options ) {