Refactor side effect handling for property interactions (#4522)

* Rename event -> interaction

* Refactor MemberExpression assignment logic to prepare for new interactions

* Use objects for interactions

* Add additional interaction parameters

* Pick "this"-argument from interaction

* Use interaction for getReturnExpression

* Replace dedicated methods with hasEffectsOnInteractionAtPath

* Fix accessor handling for loops and updated expressions

* Simplify assignment handling

* Change assignment to use args property

* Simplify ObjectEntity effect handling

* Cache remaining interactions that may be cached

* Improve coverage

src/ast/Entity.ts

@@ -1,4 +1,5 @@
 import type { HasEffectsContext } from './ExecutionContext';
+import { NodeInteractionAssigned } from './NodeInteractions';
 import type { ObjectPath } from './utils/PathTracker';
 
 // eslint-disable-next-line @typescript-eslint/no-empty-interface
@@ -13,5 +14,9 @@ export interface WritableEntity extends Entity {
 	 */
 	deoptimizePath(path: ObjectPath): void;
 
-	hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean;
+	hasEffectsOnInteractionAtPath(
+		path: ObjectPath,
+		interaction: NodeInteractionAssigned,
+		context: HasEffectsContext
+	): boolean;
 }

src/ast/NodeInteractions.ts

@@ -0,0 +1,56 @@
+import SpreadElement from './nodes/SpreadElement';
+import { ExpressionEntity, UNKNOWN_EXPRESSION } from './nodes/shared/Expression';
+
+export const INTERACTION_ACCESSED = 0;
+export const INTERACTION_ASSIGNED = 1;
+export const INTERACTION_CALLED = 2;
+
+export interface NodeInteractionAccessed {
+	thisArg: ExpressionEntity | null;
+	type: typeof INTERACTION_ACCESSED;
+}
+
+export const NODE_INTERACTION_UNKNOWN_ACCESS: NodeInteractionAccessed = {
+	thisArg: null,
+	type: INTERACTION_ACCESSED
+};
+
+export interface NodeInteractionAssigned {
+	args: readonly [ExpressionEntity];
+	thisArg: ExpressionEntity | null;
+	type: typeof INTERACTION_ASSIGNED;
+}
+
+export const UNKNOWN_ARG = [UNKNOWN_EXPRESSION] as const;
+
+export const NODE_INTERACTION_UNKNOWN_ASSIGNMENT: NodeInteractionAssigned = {
+	args: UNKNOWN_ARG,
+	thisArg: null,
+	type: INTERACTION_ASSIGNED
+};
+
+export interface NodeInteractionCalled {
+	args: readonly (ExpressionEntity | SpreadElement)[];
+	thisArg: ExpressionEntity | null;
+	type: typeof INTERACTION_CALLED;
+	withNew: boolean;
+}
+
+export const NO_ARGS = [];
+
+// While this is technically a call without arguments, we can compare against
+// this reference in places where precise values or thisArg would make a
+// difference
+export const NODE_INTERACTION_UNKNOWN_CALL: NodeInteractionCalled = {
+	args: NO_ARGS,
+	thisArg: null,
+	type: INTERACTION_CALLED,
+	withNew: false
+};
+
+export type NodeInteraction =
+	| NodeInteractionAccessed
+	| NodeInteractionAssigned
+	| NodeInteractionCalled;
+
+export type NodeInteractionWithThisArg = NodeInteraction & { thisArg: ExpressionEntity };

src/ast/nodes/ArrayExpression.ts

@@ -1,7 +1,7 @@
-import type { CallOptions } from '../CallOptions';
 import type { DeoptimizableEntity } from '../DeoptimizableEntity';
 import type { HasEffectsContext } from '../ExecutionContext';
-import type { NodeEvent } from '../NodeEvents';
+import type { NodeInteractionCalled, NodeInteractionWithThisArg } from '../NodeInteractions';
+import { NodeInteraction } from '../NodeInteractions';
 import {
 	type ObjectPath,
 	type PathTracker,
@@ -25,18 +25,12 @@ export default class ArrayExpression extends NodeBase {
 		this.getObjectEntity().deoptimizePath(path);
 	}
 
-	deoptimizeThisOnEventAtPath(
-		event: NodeEvent,
+	deoptimizeThisOnInteractionAtPath(
+		interaction: NodeInteractionWithThisArg,
 		path: ObjectPath,
-		thisParameter: ExpressionEntity,
 		recursionTracker: PathTracker
 	): void {
-		this.getObjectEntity().deoptimizeThisOnEventAtPath(
-			event,
-			path,
-			thisParameter,
-			recursionTracker
-		);
+		this.getObjectEntity().deoptimizeThisOnInteractionAtPath(interaction, path, recursionTracker);
 	}
 
 	getLiteralValueAtPath(
@@ -49,32 +43,24 @@ export default class ArrayExpression extends NodeBase {
 
 	getReturnExpressionWhenCalledAtPath(
 		path: ObjectPath,
-		callOptions: CallOptions,
+		interaction: NodeInteractionCalled,
 		recursionTracker: PathTracker,
 		origin: DeoptimizableEntity
 	): ExpressionEntity {
 		return this.getObjectEntity().getReturnExpressionWhenCalledAtPath(
 			path,
-			callOptions,
+			interaction,
 			recursionTracker,
 			origin
 		);
 	}
 
-	hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
-		return this.getObjectEntity().hasEffectsWhenAccessedAtPath(path, context);
-	}
-
-	hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
-		return this.getObjectEntity().hasEffectsWhenAssignedAtPath(path, context);
-	}
-
-	hasEffectsWhenCalledAtPath(
+	hasEffectsOnInteractionAtPath(
 		path: ObjectPath,
-		callOptions: CallOptions,
+		interaction: NodeInteraction,
 		context: HasEffectsContext
 	): boolean {
-		return this.getObjectEntity().hasEffectsWhenCalledAtPath(path, callOptions, context);
+		return this.getObjectEntity().hasEffectsOnInteractionAtPath(path, interaction, context);
 	}
 
 	protected applyDeoptimizations(): void {

src/ast/nodes/ArrayPattern.ts

@@ -1,4 +1,5 @@
 import type { HasEffectsContext } from '../ExecutionContext';
+import { NodeInteractionAssigned } from '../NodeInteractions';
 import { EMPTY_PATH, type ObjectPath } from '../utils/PathTracker';
 import type LocalVariable from '../variables/LocalVariable';
 import type Variable from '../variables/Variable';
@@ -38,9 +39,13 @@ export default class ArrayPattern extends NodeBase implements PatternNode {
 	}
 
 	// Patterns are only checked at the emtpy path at the moment
-	hasEffectsWhenAssignedAtPath(_path: ObjectPath, context: HasEffectsContext): boolean {
+	hasEffectsOnInteractionAtPath(
+		_path: ObjectPath,
+		interaction: NodeInteractionAssigned,
+		context: HasEffectsContext
+	): boolean {
 		for (const element of this.elements) {
-			if (element?.hasEffectsWhenAssignedAtPath(EMPTY_PATH, context)) return true;
+			if (element?.hasEffectsOnInteractionAtPath(EMPTY_PATH, interaction, context)) return true;
 		}
 		return false;
 	}

src/ast/nodes/ArrowFunctionExpression.ts

@@ -1,5 +1,5 @@
-import { type CallOptions } from '../CallOptions';
 import { type HasEffectsContext, InclusionContext } from '../ExecutionContext';
+import { INTERACTION_CALLED, NodeInteraction } from '../NodeInteractions';
 import ReturnValueScope from '../scopes/ReturnValueScope';
 import type Scope from '../scopes/Scope';
 import { type ObjectPath } from '../utils/PathTracker';
@@ -30,22 +30,24 @@ export default class ArrowFunctionExpression extends FunctionBase {
 		return false;
 	}
 
-	hasEffectsWhenCalledAtPath(
+	hasEffectsOnInteractionAtPath(
 		path: ObjectPath,
-		callOptions: CallOptions,
+		interaction: NodeInteraction,
 		context: HasEffectsContext
 	): boolean {
-		if (super.hasEffectsWhenCalledAtPath(path, callOptions, context)) return true;
-		const { ignore, brokenFlow } = context;
-		context.ignore = {
-			breaks: false,
-			continues: false,
-			labels: new Set(),
-			returnYield: true
-		};
-		if (this.body.hasEffects(context)) return true;
-		context.ignore = ignore;
-		context.brokenFlow = brokenFlow;
+		if (super.hasEffectsOnInteractionAtPath(path, interaction, context)) return true;
+		if (interaction.type === INTERACTION_CALLED) {
+			const { ignore, brokenFlow } = context;
+			context.ignore = {
+				breaks: false,
+				continues: false,
+				labels: new Set(),
+				returnYield: true
+			};
+			if (this.body.hasEffects(context)) return true;
+			context.ignore = ignore;
+			context.brokenFlow = brokenFlow;
+		}
 		return false;
 	}
 

src/ast/nodes/AssignmentExpression.ts

@@ -17,6 +17,7 @@ import {
 	type HasEffectsContext,
 	type InclusionContext
 } from '../ExecutionContext';
+import { NodeInteraction } from '../NodeInteractions';
 import { EMPTY_PATH, type ObjectPath, UNKNOWN_PATH } from '../utils/PathTracker';
 import type Variable from '../variables/Variable';
 import Identifier from './Identifier';
@@ -45,70 +46,78 @@ export default class AssignmentExpression extends NodeBase {
 	declare type: NodeType.tAssignmentExpression;
 
 	hasEffects(context: HasEffectsContext): boolean {
-		if (!this.deoptimized) this.applyDeoptimizations();
+		const { deoptimized, left, right } = this;
+		if (!deoptimized) this.applyDeoptimizations();
+		// MemberExpressions do not access the property before assignments if the
+		// operator is '='.
 		return (
-			this.right.hasEffects(context) ||
-			this.left.hasEffects(context) ||
-			this.left.hasEffectsWhenAssignedAtPath(EMPTY_PATH, context)
+			right.hasEffects(context) || left.hasEffectsAsAssignmentTarget(context, this.operator !== '=')
 		);
 	}
 
-	hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
-		return path.length > 0 && this.right.hasEffectsWhenAccessedAtPath(path, context);
+	hasEffectsOnInteractionAtPath(
+		path: ObjectPath,
+		interaction: NodeInteraction,
+		context: HasEffectsContext
+	): boolean {
+		return this.right.hasEffectsOnInteractionAtPath(path, interaction, context);
 	}
 
 	include(context: InclusionContext, includeChildrenRecursively: IncludeChildren): void {
-		if (!this.deoptimized) this.applyDeoptimizations();
+		const { deoptimized, left, right, operator } = this;
+		if (!deoptimized) this.applyDeoptimizations();
 		this.included = true;
-		let hasEffectsContext;
 		if (
 			includeChildrenRecursively ||
-			this.operator !== '=' ||
-			this.left.included ||
-			((hasEffectsContext = createHasEffectsContext()),
-			this.left.hasEffects(hasEffectsContext) ||
-				this.left.hasEffectsWhenAssignedAtPath(EMPTY_PATH, hasEffectsContext))
+			operator !== '=' ||
+			left.included ||
+			left.hasEffectsAsAssignmentTarget(createHasEffectsContext(), false)
 		) {
-			this.left.include(context, includeChildrenRecursively);
+			left.includeAsAssignmentTarget(context, includeChildrenRecursively, operator !== '=');
 		}
-		this.right.include(context, includeChildrenRecursively);
+		right.include(context, includeChildrenRecursively);
+	}
+
+	initialise(): void {
+		this.left.setAssignedValue(this.right);
 	}
 
 	render(
 		code: MagicString,
 		options: RenderOptions,
 		{ preventASI, renderedParentType, renderedSurroundingElement }: NodeRenderOptions = BLANK
 	): void {
-		if (this.left.included) {
-			this.left.render(code, options);
-			this.right.render(code, options);
+		const { left, right, start, end, parent } = this;
+		if (left.included) {
+			left.render(code, options);
+			right.render(code, options);
 		} else {
 			const inclusionStart = findNonWhiteSpace(
 				code.original,
-				findFirstOccurrenceOutsideComment(code.original, '=', this.left.end) + 1
+				findFirstOccurrenceOutsideComment(code.original, '=', left.end) + 1
 			);
-			code.remove(this.start, inclusionStart);
+			code.remove(start, inclusionStart);
 			if (preventASI) {
-				removeLineBreaks(code, inclusionStart, this.right.start);
+				removeLineBreaks(code, inclusionStart, right.start);
 			}
-			this.right.render(code, options, {
-				renderedParentType: renderedParentType || this.parent.type,
-				renderedSurroundingElement: renderedSurroundingElement || this.parent.type
+			right.render(code, options, {
+				renderedParentType: renderedParentType || parent.type,
+				renderedSurroundingElement: renderedSurroundingElement || parent.type
 			});
 		}
 		if (options.format === 'system') {
-			if (this.left instanceof Identifier) {
-				const variable = this.left.variable!;
+			if (left instanceof Identifier) {
+				const variable = left.variable!;
 				const exportNames = options.exportNamesByVariable.get(variable);
 				if (exportNames) {
 					if (exportNames.length === 1) {
-						renderSystemExportExpression(variable, this.start, this.end, code, options);
+						renderSystemExportExpression(variable, start, end, code, options);
 					} else {
 						renderSystemExportSequenceAfterExpression(
 							variable,
-							this.start,
-							this.end,
-							this.parent.type !== NodeType.ExpressionStatement,
+							start,
+							end,
+							parent.type !== NodeType.ExpressionStatement,
 							code,
 							options
 						);
@@ -117,12 +126,12 @@ export default class AssignmentExpression extends NodeBase {
 				}
 			} else {
 				const systemPatternExports: Variable[] = [];
-				this.left.addExportedVariables(systemPatternExports, options.exportNamesByVariable);
+				left.addExportedVariables(systemPatternExports, options.exportNamesByVariable);
 				if (systemPatternExports.length > 0) {
 					renderSystemExportFunction(
 						systemPatternExports,
-						this.start,
-						this.end,
+						start,
+						end,
 						renderedSurroundingElement === NodeType.ExpressionStatement,
 						code,
 						options
@@ -132,13 +141,13 @@ export default class AssignmentExpression extends NodeBase {
 			}
 		}
 		if (
-			this.left.included &&
-			this.left instanceof ObjectPattern &&
+			left.included &&
+			left instanceof ObjectPattern &&
 			(renderedSurroundingElement === NodeType.ExpressionStatement ||
 				renderedSurroundingElement === NodeType.ArrowFunctionExpression)
 		) {
-			code.appendRight(this.start, '(');
-			code.prependLeft(this.end, ')');
+			code.appendRight(start, '(');
+			code.prependLeft(end, ')');
 		}
 	}