Skip to content

Commit c443303

Browse files
lukastaegertlukastaegert
authored
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
1 parent 35e7657 commit c443303

59 files changed

Lines changed: 1147 additions & 1058 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎src/ast/CallOptions.ts‎

Lines changed: 0 additions & 10 deletions
This file was deleted.

‎src/ast/Entity.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { HasEffectsContext } from './ExecutionContext';
2+
import { NodeInteractionAssigned } from './NodeInteractions';
23
import type { ObjectPath } from './utils/PathTracker';
34

45
// eslint-disable-next-line @typescript-eslint/no-empty-interface
@@ -13,5 +14,9 @@ export interface WritableEntity extends Entity {
1314
*/
1415
deoptimizePath(path: ObjectPath): void;
1516

16-
hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean;
17+
hasEffectsOnInteractionAtPath(
18+
path: ObjectPath,
19+
interaction: NodeInteractionAssigned,
20+
context: HasEffectsContext
21+
): boolean;
1722
}

‎src/ast/NodeEvents.ts‎

Lines changed: 0 additions & 5 deletions
This file was deleted.

‎src/ast/NodeInteractions.ts‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
import SpreadElement from './nodes/SpreadElement';
2+
import { ExpressionEntity, UNKNOWN_EXPRESSION } from './nodes/shared/Expression';
3+
4+
export const INTERACTION_ACCESSED = 0;
5+
export const INTERACTION_ASSIGNED = 1;
6+
export const INTERACTION_CALLED = 2;
7+
8+
export interface NodeInteractionAccessed {
9+
thisArg: ExpressionEntity | null;
10+
type: typeof INTERACTION_ACCESSED;
11+
}
12+
13+
export const NODE_INTERACTION_UNKNOWN_ACCESS: NodeInteractionAccessed = {
14+
thisArg: null,
15+
type: INTERACTION_ACCESSED
16+
};
17+
18+
export interface NodeInteractionAssigned {
19+
args: readonly [ExpressionEntity];
20+
thisArg: ExpressionEntity | null;
21+
type: typeof INTERACTION_ASSIGNED;
22+
}
23+
24+
export const UNKNOWN_ARG = [UNKNOWN_EXPRESSION] as const;
25+
26+
export const NODE_INTERACTION_UNKNOWN_ASSIGNMENT: NodeInteractionAssigned = {
27+
args: UNKNOWN_ARG,
28+
thisArg: null,
29+
type: INTERACTION_ASSIGNED
30+
};
31+
32+
export interface NodeInteractionCalled {
33+
args: readonly (ExpressionEntity | SpreadElement)[];
34+
thisArg: ExpressionEntity | null;
35+
type: typeof INTERACTION_CALLED;
36+
withNew: boolean;
37+
}
38+
39+
export const NO_ARGS = [];
40+
41+
// While this is technically a call without arguments, we can compare against
42+
// this reference in places where precise values or thisArg would make a
43+
// difference
44+
export const NODE_INTERACTION_UNKNOWN_CALL: NodeInteractionCalled = {
45+
args: NO_ARGS,
46+
thisArg: null,
47+
type: INTERACTION_CALLED,
48+
withNew: false
49+
};
50+
51+
export type NodeInteraction =
52+
| NodeInteractionAccessed
53+
| NodeInteractionAssigned
54+
| NodeInteractionCalled;
55+
56+
export type NodeInteractionWithThisArg = NodeInteraction & { thisArg: ExpressionEntity };

‎src/ast/nodes/ArrayExpression.ts‎

Lines changed: 10 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
import type { CallOptions } from '../CallOptions';
21
import type { DeoptimizableEntity } from '../DeoptimizableEntity';
32
import type { HasEffectsContext } from '../ExecutionContext';
4-
import type { NodeEvent } from '../NodeEvents';
3+
import type { NodeInteractionCalled, NodeInteractionWithThisArg } from '../NodeInteractions';
4+
import { NodeInteraction } from '../NodeInteractions';
55
import {
66
type ObjectPath,
77
type PathTracker,
@@ -25,18 +25,12 @@ export default class ArrayExpression extends NodeBase {
2525
this.getObjectEntity().deoptimizePath(path);
2626
}
2727

28-
deoptimizeThisOnEventAtPath(
29-
event: NodeEvent,
28+
deoptimizeThisOnInteractionAtPath(
29+
interaction: NodeInteractionWithThisArg,
3030
path: ObjectPath,
31-
thisParameter: ExpressionEntity,
3231
recursionTracker: PathTracker
3332
): void {
34-
this.getObjectEntity().deoptimizeThisOnEventAtPath(
35-
event,
36-
path,
37-
thisParameter,
38-
recursionTracker
39-
);
33+
this.getObjectEntity().deoptimizeThisOnInteractionAtPath(interaction, path, recursionTracker);
4034
}
4135

4236
getLiteralValueAtPath(
@@ -49,32 +43,24 @@ export default class ArrayExpression extends NodeBase {
4943

5044
getReturnExpressionWhenCalledAtPath(
5145
path: ObjectPath,
52-
callOptions: CallOptions,
46+
interaction: NodeInteractionCalled,
5347
recursionTracker: PathTracker,
5448
origin: DeoptimizableEntity
5549
): ExpressionEntity {
5650
return this.getObjectEntity().getReturnExpressionWhenCalledAtPath(
5751
path,
58-
callOptions,
52+
interaction,
5953
recursionTracker,
6054
origin
6155
);
6256
}
6357

64-
hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
65-
return this.getObjectEntity().hasEffectsWhenAccessedAtPath(path, context);
66-
}
67-
68-
hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
69-
return this.getObjectEntity().hasEffectsWhenAssignedAtPath(path, context);
70-
}
71-
72-
hasEffectsWhenCalledAtPath(
58+
hasEffectsOnInteractionAtPath(
7359
path: ObjectPath,
74-
callOptions: CallOptions,
60+
interaction: NodeInteraction,
7561
context: HasEffectsContext
7662
): boolean {
77-
return this.getObjectEntity().hasEffectsWhenCalledAtPath(path, callOptions, context);
63+
return this.getObjectEntity().hasEffectsOnInteractionAtPath(path, interaction, context);
7864
}
7965

8066
protected applyDeoptimizations(): void {

‎src/ast/nodes/ArrayPattern.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { HasEffectsContext } from '../ExecutionContext';
2+
import { NodeInteractionAssigned } from '../NodeInteractions';
23
import { EMPTY_PATH, type ObjectPath } from '../utils/PathTracker';
34
import type LocalVariable from '../variables/LocalVariable';
45
import type Variable from '../variables/Variable';
@@ -38,9 +39,13 @@ export default class ArrayPattern extends NodeBase implements PatternNode {
3839
}
3940

4041
// Patterns are only checked at the emtpy path at the moment
41-
hasEffectsWhenAssignedAtPath(_path: ObjectPath, context: HasEffectsContext): boolean {
42+
hasEffectsOnInteractionAtPath(
43+
_path: ObjectPath,
44+
interaction: NodeInteractionAssigned,
45+
context: HasEffectsContext
46+
): boolean {
4247
for (const element of this.elements) {
43-
if (element?.hasEffectsWhenAssignedAtPath(EMPTY_PATH, context)) return true;
48+
if (element?.hasEffectsOnInteractionAtPath(EMPTY_PATH, interaction, context)) return true;
4449
}
4550
return false;
4651
}

‎src/ast/nodes/ArrowFunctionExpression.ts‎

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import { type CallOptions } from '../CallOptions';
21
import { type HasEffectsContext, InclusionContext } from '../ExecutionContext';
2+
import { INTERACTION_CALLED, NodeInteraction } from '../NodeInteractions';
33
import ReturnValueScope from '../scopes/ReturnValueScope';
44
import type Scope from '../scopes/Scope';
55
import { type ObjectPath } from '../utils/PathTracker';
@@ -30,22 +30,24 @@ export default class ArrowFunctionExpression extends FunctionBase {
3030
return false;
3131
}
3232

33-
hasEffectsWhenCalledAtPath(
33+
hasEffectsOnInteractionAtPath(
3434
path: ObjectPath,
35-
callOptions: CallOptions,
35+
interaction: NodeInteraction,
3636
context: HasEffectsContext
3737
): boolean {
38-
if (super.hasEffectsWhenCalledAtPath(path, callOptions, context)) return true;
39-
const { ignore, brokenFlow } = context;
40-
context.ignore = {
41-
breaks: false,
42-
continues: false,
43-
labels: new Set(),
44-
returnYield: true
45-
};
46-
if (this.body.hasEffects(context)) return true;
47-
context.ignore = ignore;
48-
context.brokenFlow = brokenFlow;
38+
if (super.hasEffectsOnInteractionAtPath(path, interaction, context)) return true;
39+
if (interaction.type === INTERACTION_CALLED) {
40+
const { ignore, brokenFlow } = context;
41+
context.ignore = {
42+
breaks: false,
43+
continues: false,
44+
labels: new Set(),
45+
returnYield: true
46+
};
47+
if (this.body.hasEffects(context)) return true;
48+
context.ignore = ignore;
49+
context.brokenFlow = brokenFlow;
50+
}
4951
return false;
5052
}
5153

‎src/ast/nodes/AssignmentExpression.ts‎

Lines changed: 46 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
type HasEffectsContext,
1818
type InclusionContext
1919
} from '../ExecutionContext';
20+
import { NodeInteraction } from '../NodeInteractions';
2021
import { EMPTY_PATH, type ObjectPath, UNKNOWN_PATH } from '../utils/PathTracker';
2122
import type Variable from '../variables/Variable';
2223
import Identifier from './Identifier';
@@ -45,70 +46,78 @@ export default class AssignmentExpression extends NodeBase {
4546
declare type: NodeType.tAssignmentExpression;
4647

4748
hasEffects(context: HasEffectsContext): boolean {
48-
if (!this.deoptimized) this.applyDeoptimizations();
49+
const { deoptimized, left, right } = this;
50+
if (!deoptimized) this.applyDeoptimizations();
51+
// MemberExpressions do not access the property before assignments if the
52+
// operator is '='.
4953
return (
50-
this.right.hasEffects(context) ||
51-
this.left.hasEffects(context) ||
52-
this.left.hasEffectsWhenAssignedAtPath(EMPTY_PATH, context)
54+
right.hasEffects(context) || left.hasEffectsAsAssignmentTarget(context, this.operator !== '=')
5355
);
5456
}
5557

56-
hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
57-
return path.length > 0 && this.right.hasEffectsWhenAccessedAtPath(path, context);
58+
hasEffectsOnInteractionAtPath(
59+
path: ObjectPath,
60+
interaction: NodeInteraction,
61+
context: HasEffectsContext
62+
): boolean {
63+
return this.right.hasEffectsOnInteractionAtPath(path, interaction, context);
5864
}
5965

6066
include(context: InclusionContext, includeChildrenRecursively: IncludeChildren): void {
61-
if (!this.deoptimized) this.applyDeoptimizations();
67+
const { deoptimized, left, right, operator } = this;
68+
if (!deoptimized) this.applyDeoptimizations();
6269
this.included = true;
63-
let hasEffectsContext;
6470
if (
6571
includeChildrenRecursively ||
66-
this.operator !== '=' ||
67-
this.left.included ||
68-
((hasEffectsContext = createHasEffectsContext()),
69-
this.left.hasEffects(hasEffectsContext) ||
70-
this.left.hasEffectsWhenAssignedAtPath(EMPTY_PATH, hasEffectsContext))
72+
operator !== '=' ||
73+
left.included ||
74+
left.hasEffectsAsAssignmentTarget(createHasEffectsContext(), false)
7175
) {
72-
this.left.include(context, includeChildrenRecursively);
76+
left.includeAsAssignmentTarget(context, includeChildrenRecursively, operator !== '=');
7377
}
74-
this.right.include(context, includeChildrenRecursively);
78+
right.include(context, includeChildrenRecursively);
79+
}
80+
81+
initialise(): void {
82+
this.left.setAssignedValue(this.right);
7583
}
7684

7785
render(
7886
code: MagicString,
7987
options: RenderOptions,
8088
{ preventASI, renderedParentType, renderedSurroundingElement }: NodeRenderOptions = BLANK
8189
): void {
82-
if (this.left.included) {
83-
this.left.render(code, options);
84-
this.right.render(code, options);
90+
const { left, right, start, end, parent } = this;
91+
if (left.included) {
92+
left.render(code, options);
93+
right.render(code, options);
8594
} else {
8695
const inclusionStart = findNonWhiteSpace(
8796
code.original,
88-
findFirstOccurrenceOutsideComment(code.original, '=', this.left.end) + 1
97+
findFirstOccurrenceOutsideComment(code.original, '=', left.end) + 1
8998
);
90-
code.remove(this.start, inclusionStart);
99+
code.remove(start, inclusionStart);
91100
if (preventASI) {
92-
removeLineBreaks(code, inclusionStart, this.right.start);
101+
removeLineBreaks(code, inclusionStart, right.start);
93102
}
94-
this.right.render(code, options, {
95-
renderedParentType: renderedParentType || this.parent.type,
96-
renderedSurroundingElement: renderedSurroundingElement || this.parent.type
103+
right.render(code, options, {
104+
renderedParentType: renderedParentType || parent.type,
105+
renderedSurroundingElement: renderedSurroundingElement || parent.type
97106
});
98107
}
99108
if (options.format === 'system') {
100-
if (this.left instanceof Identifier) {
101-
const variable = this.left.variable!;
109+
if (left instanceof Identifier) {
110+
const variable = left.variable!;
102111
const exportNames = options.exportNamesByVariable.get(variable);
103112
if (exportNames) {
104113
if (exportNames.length === 1) {
105-
renderSystemExportExpression(variable, this.start, this.end, code, options);
114+
renderSystemExportExpression(variable, start, end, code, options);
106115
} else {
107116
renderSystemExportSequenceAfterExpression(
108117
variable,
109-
this.start,
110-
this.end,
111-
this.parent.type !== NodeType.ExpressionStatement,
118+
start,
119+
end,
120+
parent.type !== NodeType.ExpressionStatement,
112121
code,
113122
options
114123
);
@@ -117,12 +126,12 @@ export default class AssignmentExpression extends NodeBase {
117126
}
118127
} else {
119128
const systemPatternExports: Variable[] = [];
120-
this.left.addExportedVariables(systemPatternExports, options.exportNamesByVariable);
129+
left.addExportedVariables(systemPatternExports, options.exportNamesByVariable);
121130
if (systemPatternExports.length > 0) {
122131
renderSystemExportFunction(
123132
systemPatternExports,
124-
this.start,
125-
this.end,
133+
start,
134+
end,
126135
renderedSurroundingElement === NodeType.ExpressionStatement,
127136
code,
128137
options
@@ -132,13 +141,13 @@ export default class AssignmentExpression extends NodeBase {
132141
}
133142
}
134143
if (
135-
this.left.included &&
136-
this.left instanceof ObjectPattern &&
144+
left.included &&
145+
left instanceof ObjectPattern &&
137146
(renderedSurroundingElement === NodeType.ExpressionStatement ||
138147
renderedSurroundingElement === NodeType.ArrowFunctionExpression)
139148
) {
140-
code.appendRight(this.start, '(');
141-
code.prependLeft(this.end, ')');
149+
code.appendRight(start, '(');
150+
code.prependLeft(end, ')');
142151
}
143152
}
144153

0 commit comments

Comments
 (0)