Skip to content

Commit 4c475e2

Browse files
committed
[compiler] Bail out and log calls that likely have side effects
ghstack-source-id: cb4397a Pull Request resolved: #30572
1 parent 06d0b89 commit 4c475e2

File tree

2 files changed

+181
-0
lines changed

2 files changed

+181
-0
lines changed

compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ import {
9898
} from '../Validation';
9999
import {validateLocalsNotReassignedAfterRender} from '../Validation/ValidateLocalsNotReassignedAfterRender';
100100
import {outlineFunctions} from '../Optimization/OutlineFunctions';
101+
import {validateNoMutationCalls} from '../ReactiveScopes/ValidateNoMutationCalls';
101102

102103
export type CompilerPipelineValue =
103104
| {kind: 'ast'; name: string; value: CodegenFunction}
@@ -484,6 +485,8 @@ function* runWithEnvironment(
484485
validatePreservedManualMemoization(reactiveFunction);
485486
}
486487

488+
validateNoMutationCalls(reactiveFunction);
489+
487490
const ast = codegenFunction(reactiveFunction, {
488491
uniqueIdentifiers,
489492
fbtOperands,
Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
/**
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
import {CompilerError, ErrorSeverity} from '../CompilerError';
9+
import {Environment} from '../HIR';
10+
import {
11+
Effect,
12+
Identifier,
13+
IdentifierId,
14+
InstructionId,
15+
Place,
16+
ReactiveFunction,
17+
ReactiveInstruction,
18+
getHookKind,
19+
isSetStateType,
20+
} from '../HIR/HIR';
21+
import { eachInstructionValueLValue } from '../HIR/visitors';
22+
import {
23+
ReactiveFunctionVisitor,
24+
eachReactiveValueOperand,
25+
visitReactiveFunction,
26+
} from './visitors';
27+
28+
const allowedNames = new Set(['invariant', 'recoverableViolation']);
29+
30+
class Visitor extends ReactiveFunctionVisitor<CompilerError> {
31+
#env: Environment;
32+
#names = new Map<IdentifierId, string>();
33+
#functions = new Map<IdentifierId, CompilerError>()
34+
35+
constructor(env: Environment) {
36+
super();
37+
this.#env = env;
38+
}
39+
40+
getName(id: Identifier): string | undefined {
41+
if (id.name?.kind === 'named') {
42+
return id.name.value;
43+
} else {
44+
return this.#names.get(id.id);
45+
}
46+
}
47+
48+
setName(id: Identifier, name: string | undefined): void {
49+
if (name != null) {
50+
this.#names.set(id.id, name);
51+
}
52+
}
53+
54+
override visitReactiveFunctionValue(): void {
55+
CompilerError.invariant(false, { reason: 'visitReactiveFunctionValue should not be called', loc: null })
56+
}
57+
58+
override visitInstruction(
59+
instr: ReactiveInstruction,
60+
state: CompilerError,
61+
): void {
62+
if (instr.lvalue != null) {
63+
switch (instr.value.kind) {
64+
case 'LoadGlobal':
65+
this.setName(instr.lvalue.identifier, instr.value.binding.name);
66+
super.visitInstruction(instr, state);
67+
break;
68+
case 'LoadLocal':
69+
case 'LoadContext':
70+
this.setName(
71+
instr.lvalue.identifier,
72+
this.getName(instr.value.place.identifier),
73+
);
74+
super.visitInstruction(instr, state);
75+
break;
76+
case 'PropertyLoad': {
77+
const name = this.getName(instr.value.object.identifier);
78+
if (name != null) {
79+
this.setName(
80+
instr.lvalue.identifier,
81+
name + '.' + instr.value.property,
82+
);
83+
}
84+
super.visitInstruction(instr, state);
85+
break;
86+
}
87+
case 'ComputedLoad': {
88+
const name = this.getName(instr.value.object.identifier);
89+
if (name != null) {
90+
this.setName(instr.lvalue.identifier, name + '[...]');
91+
}
92+
super.visitInstruction(instr, state);
93+
break;
94+
}
95+
case 'FunctionExpression': {
96+
this.setName(instr.lvalue.identifier, instr.value.name ?? undefined);
97+
const state = new CompilerError();
98+
this.visitHirFunction(instr.value.loweredFunc.func, state);
99+
if (state.hasErrors()) {
100+
this.#functions.set(instr.lvalue.identifier.id, state);
101+
}
102+
break;
103+
}
104+
case 'ReactiveFunctionValue': {
105+
this.setName(instr.lvalue.identifier, instr.value.fn.id ?? undefined);
106+
const state = new CompilerError();
107+
this.visitBlock(instr.value.fn.body, state);
108+
if (state.hasErrors()) {
109+
this.#functions.set(instr.lvalue.identifier.id, state);
110+
}
111+
break;
112+
}
113+
default: {
114+
super.visitInstruction(instr, state);
115+
break;
116+
}
117+
}
118+
}
119+
120+
let hookKind = null;
121+
let callee = null;
122+
if (
123+
instr.value.kind === 'CallExpression' ||
124+
instr.value.kind === 'MethodCall'
125+
) {
126+
if (instr.value.kind === 'CallExpression') {
127+
callee = instr.value.callee;
128+
} else {
129+
callee = instr.value.property;
130+
}
131+
hookKind = getHookKind(this.#env, callee.identifier);
132+
}
133+
134+
if (hookKind !== 'useEffect' && hookKind !== 'useLayoutEffect' && hookKind !== 'useInsertionEffect' && instr.value.kind !== "JsxExpression") {
135+
for (const operand of eachReactiveValueOperand(instr.value)) {
136+
const errors = this.#functions.get(operand.identifier.id);
137+
if (errors != null) {
138+
for (const lval of eachInstructionValueLValue(instr.value)) {
139+
const existing = this.#functions.get(lval.identifier.id) ?? new CompilerError();
140+
errors.details.forEach(detail => existing.pushErrorDetail(detail));
141+
this.#functions.set(lval.identifier.id, existing);
142+
}
143+
}
144+
}
145+
}
146+
147+
if (callee != null) {
148+
const isException =
149+
hookKind != null ||
150+
isSetStateType(callee.identifier);
151+
const name = this.getName(callee.identifier) ?? "(unknown)";
152+
153+
this.#functions.get(callee.identifier.id)?.details?.forEach(detail => state.pushErrorDetail(detail));
154+
155+
if (instr.lvalue === null && !isException && !allowedNames.has(name)) {
156+
let allReads = true;
157+
for (const operand of eachReactiveValueOperand(instr.value)) {
158+
allReads &&= operand.effect === Effect.Read;
159+
}
160+
if (allReads) {
161+
state.push({
162+
reason: `Likely illegal mutation call \`${name}\``,
163+
loc: instr.loc,
164+
severity: ErrorSeverity.Todo,
165+
});
166+
}
167+
}
168+
}
169+
}
170+
}
171+
172+
export function validateNoMutationCalls(fn: ReactiveFunction): void {
173+
const error = new CompilerError();
174+
visitReactiveFunction(fn, new Visitor(fn.env), error);
175+
if (error.hasErrors()) {
176+
throw error;
177+
}
178+
}

0 commit comments

Comments
 (0)