mirror of
https://github.com/facebook/react.git
synced 2026-02-21 19:31:52 +00:00
[compiler] Add fault tolerance test fixtures
This commit is contained in:
@@ -327,4 +327,5 @@ Walk through `runWithEnvironment` and wrap each pass call site. This is the inte
|
||||
* **Phase 3 (BuildHIR) revealed that most error sites already used `builder.errors.push()` for accumulation.** The existing lowering code was designed to accumulate errors rather than throw. The main changes were: (1) changing `lower()` return type from `Result` to `HIRFunction`, (2) recording builder errors on env, (3) adding a try/catch around body lowering to catch thrown CompilerErrors from sub-calls like `resolveBinding()`, (4) treating `var` as `let` instead of skipping declarations, and (5) fixing ForStatement init/test handling to produce valid CFG structure.
|
||||
* **Partial HIR can trigger downstream invariants.** When lowering skips or partially handles constructs (e.g., unreachable hoisted functions, `var` declarations before the fix), downstream passes like `InferMutationAliasingEffects` may encounter uninitialized identifiers and throw invariants. This is acceptable since the function still correctly bails out of compilation, but error messages may be less specific. The fix for `var` (treating as `let`) demonstrates how to avoid this: continue lowering with a best-effort representation rather than skipping entirely.
|
||||
* **Errors accumulated on `env` are lost when an invariant propagates out of the pipeline.** Since invariant CompilerErrors always re-throw through `tryRecord()`, they exit the pipeline as exceptions. The caller only sees the invariant error, not any errors previously recorded on `env`. This is a design limitation that could be addressed by aggregating env errors with caught exceptions in `tryCompileFunction()`.
|
||||
* **Dedicated fault tolerance test fixtures** were added in `__tests__/fixtures/compiler/fault-tolerance/`. Each fixture combines two or more errors from different passes to verify the compiler reports all of them rather than short-circuiting on the first. Coverage includes: `var`+props mutation (BuildHIR→InferMutationAliasingEffects), `var`+ref access (BuildHIR→ValidateNoRefAccessInRender), `try/finally`+props mutation (BuildHIR→InferMutationAliasingEffects), `try/finally`+ref access (BuildHIR→ValidateNoRefAccessInRender), and a 3-error test combining try/finally+ref access+props mutation.
|
||||
|
||||
|
||||
@@ -0,0 +1,66 @@
|
||||
|
||||
## Input
|
||||
|
||||
```javascript
|
||||
/**
|
||||
* Fault tolerance test: two independent errors should both be reported.
|
||||
*
|
||||
* Error 1 (BuildHIR): `try/finally` is not supported
|
||||
* Error 2 (InferMutationAliasingEffects): Mutation of frozen props
|
||||
*/
|
||||
function Component(props) {
|
||||
// Error: try/finally (Todo from BuildHIR)
|
||||
try {
|
||||
doWork();
|
||||
} finally {
|
||||
doCleanup();
|
||||
}
|
||||
|
||||
// Error: mutating frozen props
|
||||
props.value = 1;
|
||||
|
||||
return <div>{props.value}</div>;
|
||||
}
|
||||
|
||||
```
|
||||
|
||||
|
||||
## Error
|
||||
|
||||
```
|
||||
Found 2 errors:
|
||||
|
||||
Todo: (BuildHIR::lowerStatement) Handle TryStatement without a catch clause
|
||||
|
||||
error.try-finally-and-mutation-of-props.ts:9:2
|
||||
7 | function Component(props) {
|
||||
8 | // Error: try/finally (Todo from BuildHIR)
|
||||
> 9 | try {
|
||||
| ^^^^^
|
||||
> 10 | doWork();
|
||||
| ^^^^^^^^^^^^^
|
||||
> 11 | } finally {
|
||||
| ^^^^^^^^^^^^^
|
||||
> 12 | doCleanup();
|
||||
| ^^^^^^^^^^^^^
|
||||
> 13 | }
|
||||
| ^^^^ (BuildHIR::lowerStatement) Handle TryStatement without a catch clause
|
||||
14 |
|
||||
15 | // Error: mutating frozen props
|
||||
16 | props.value = 1;
|
||||
|
||||
Error: This value cannot be modified
|
||||
|
||||
Modifying component props or hook arguments is not allowed. Consider using a local variable instead.
|
||||
|
||||
error.try-finally-and-mutation-of-props.ts:16:2
|
||||
14 |
|
||||
15 | // Error: mutating frozen props
|
||||
> 16 | props.value = 1;
|
||||
| ^^^^^ value cannot be modified
|
||||
17 |
|
||||
18 | return <div>{props.value}</div>;
|
||||
19 | }
|
||||
```
|
||||
|
||||
|
||||
@@ -0,0 +1,19 @@
|
||||
/**
|
||||
* Fault tolerance test: two independent errors should both be reported.
|
||||
*
|
||||
* Error 1 (BuildHIR): `try/finally` is not supported
|
||||
* Error 2 (InferMutationAliasingEffects): Mutation of frozen props
|
||||
*/
|
||||
function Component(props) {
|
||||
// Error: try/finally (Todo from BuildHIR)
|
||||
try {
|
||||
doWork();
|
||||
} finally {
|
||||
doCleanup();
|
||||
}
|
||||
|
||||
// Error: mutating frozen props
|
||||
props.value = 1;
|
||||
|
||||
return <div>{props.value}</div>;
|
||||
}
|
||||
@@ -0,0 +1,69 @@
|
||||
|
||||
## Input
|
||||
|
||||
```javascript
|
||||
// @validateRefAccessDuringRender
|
||||
/**
|
||||
* Fault tolerance test: two independent errors should both be reported.
|
||||
*
|
||||
* Error 1 (BuildHIR): `try/finally` is not supported
|
||||
* Error 2 (ValidateNoRefAccessInRender): reading ref.current during render
|
||||
*/
|
||||
function Component() {
|
||||
const ref = useRef(null);
|
||||
|
||||
// Error: try/finally (Todo from BuildHIR)
|
||||
try {
|
||||
doSomething();
|
||||
} finally {
|
||||
cleanup();
|
||||
}
|
||||
|
||||
// Error: reading ref during render
|
||||
const value = ref.current;
|
||||
|
||||
return <div>{value}</div>;
|
||||
}
|
||||
|
||||
```
|
||||
|
||||
|
||||
## Error
|
||||
|
||||
```
|
||||
Found 2 errors:
|
||||
|
||||
Todo: (BuildHIR::lowerStatement) Handle TryStatement without a catch clause
|
||||
|
||||
error.try-finally-and-ref-access.ts:12:2
|
||||
10 |
|
||||
11 | // Error: try/finally (Todo from BuildHIR)
|
||||
> 12 | try {
|
||||
| ^^^^^
|
||||
> 13 | doSomething();
|
||||
| ^^^^^^^^^^^^^^^^^^
|
||||
> 14 | } finally {
|
||||
| ^^^^^^^^^^^^^^^^^^
|
||||
> 15 | cleanup();
|
||||
| ^^^^^^^^^^^^^^^^^^
|
||||
> 16 | }
|
||||
| ^^^^ (BuildHIR::lowerStatement) Handle TryStatement without a catch clause
|
||||
17 |
|
||||
18 | // Error: reading ref during render
|
||||
19 | const value = ref.current;
|
||||
|
||||
Error: Cannot access refs during render
|
||||
|
||||
React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the `current` property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef).
|
||||
|
||||
error.try-finally-and-ref-access.ts:19:16
|
||||
17 |
|
||||
18 | // Error: reading ref during render
|
||||
> 19 | const value = ref.current;
|
||||
| ^^^^^^^^^^^ Cannot access ref value during render
|
||||
20 |
|
||||
21 | return <div>{value}</div>;
|
||||
22 | }
|
||||
```
|
||||
|
||||
|
||||
@@ -0,0 +1,22 @@
|
||||
// @validateRefAccessDuringRender
|
||||
/**
|
||||
* Fault tolerance test: two independent errors should both be reported.
|
||||
*
|
||||
* Error 1 (BuildHIR): `try/finally` is not supported
|
||||
* Error 2 (ValidateNoRefAccessInRender): reading ref.current during render
|
||||
*/
|
||||
function Component() {
|
||||
const ref = useRef(null);
|
||||
|
||||
// Error: try/finally (Todo from BuildHIR)
|
||||
try {
|
||||
doSomething();
|
||||
} finally {
|
||||
cleanup();
|
||||
}
|
||||
|
||||
// Error: reading ref during render
|
||||
const value = ref.current;
|
||||
|
||||
return <div>{value}</div>;
|
||||
}
|
||||
@@ -0,0 +1,86 @@
|
||||
|
||||
## Input
|
||||
|
||||
```javascript
|
||||
// @validateRefAccessDuringRender
|
||||
/**
|
||||
* Fault tolerance test: three independent errors should all be reported.
|
||||
*
|
||||
* Error 1 (BuildHIR): `try/finally` is not supported
|
||||
* Error 2 (ValidateNoRefAccessInRender): reading ref.current during render
|
||||
* Error 3 (InferMutationAliasingEffects): Mutation of frozen props
|
||||
*/
|
||||
function Component(props) {
|
||||
const ref = useRef(null);
|
||||
|
||||
// Error: try/finally (Todo from BuildHIR)
|
||||
try {
|
||||
doWork();
|
||||
} finally {
|
||||
cleanup();
|
||||
}
|
||||
|
||||
// Error: reading ref during render
|
||||
const value = ref.current;
|
||||
|
||||
// Error: mutating frozen props
|
||||
props.items = [];
|
||||
|
||||
return <div>{value}</div>;
|
||||
}
|
||||
|
||||
```
|
||||
|
||||
|
||||
## Error
|
||||
|
||||
```
|
||||
Found 3 errors:
|
||||
|
||||
Todo: (BuildHIR::lowerStatement) Handle TryStatement without a catch clause
|
||||
|
||||
error.try-finally-ref-access-and-mutation.ts:13:2
|
||||
11 |
|
||||
12 | // Error: try/finally (Todo from BuildHIR)
|
||||
> 13 | try {
|
||||
| ^^^^^
|
||||
> 14 | doWork();
|
||||
| ^^^^^^^^^^^^^
|
||||
> 15 | } finally {
|
||||
| ^^^^^^^^^^^^^
|
||||
> 16 | cleanup();
|
||||
| ^^^^^^^^^^^^^
|
||||
> 17 | }
|
||||
| ^^^^ (BuildHIR::lowerStatement) Handle TryStatement without a catch clause
|
||||
18 |
|
||||
19 | // Error: reading ref during render
|
||||
20 | const value = ref.current;
|
||||
|
||||
Error: This value cannot be modified
|
||||
|
||||
Modifying component props or hook arguments is not allowed. Consider using a local variable instead.
|
||||
|
||||
error.try-finally-ref-access-and-mutation.ts:23:2
|
||||
21 |
|
||||
22 | // Error: mutating frozen props
|
||||
> 23 | props.items = [];
|
||||
| ^^^^^ value cannot be modified
|
||||
24 |
|
||||
25 | return <div>{value}</div>;
|
||||
26 | }
|
||||
|
||||
Error: Cannot access refs during render
|
||||
|
||||
React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the `current` property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef).
|
||||
|
||||
error.try-finally-ref-access-and-mutation.ts:20:16
|
||||
18 |
|
||||
19 | // Error: reading ref during render
|
||||
> 20 | const value = ref.current;
|
||||
| ^^^^^^^^^^^ Cannot access ref value during render
|
||||
21 |
|
||||
22 | // Error: mutating frozen props
|
||||
23 | props.items = [];
|
||||
```
|
||||
|
||||
|
||||
@@ -0,0 +1,26 @@
|
||||
// @validateRefAccessDuringRender
|
||||
/**
|
||||
* Fault tolerance test: three independent errors should all be reported.
|
||||
*
|
||||
* Error 1 (BuildHIR): `try/finally` is not supported
|
||||
* Error 2 (ValidateNoRefAccessInRender): reading ref.current during render
|
||||
* Error 3 (InferMutationAliasingEffects): Mutation of frozen props
|
||||
*/
|
||||
function Component(props) {
|
||||
const ref = useRef(null);
|
||||
|
||||
// Error: try/finally (Todo from BuildHIR)
|
||||
try {
|
||||
doWork();
|
||||
} finally {
|
||||
cleanup();
|
||||
}
|
||||
|
||||
// Error: reading ref during render
|
||||
const value = ref.current;
|
||||
|
||||
// Error: mutating frozen props
|
||||
props.items = [];
|
||||
|
||||
return <div>{value}</div>;
|
||||
}
|
||||
@@ -0,0 +1,54 @@
|
||||
|
||||
## Input
|
||||
|
||||
```javascript
|
||||
/**
|
||||
* Fault tolerance test: two independent errors should both be reported.
|
||||
*
|
||||
* Error 1 (BuildHIR): `var` declarations are not supported (treated as `let`)
|
||||
* Error 2 (InferMutationAliasingEffects): Mutation of frozen props
|
||||
*/
|
||||
function Component(props) {
|
||||
// Error: var declaration (Todo from BuildHIR)
|
||||
var items = props.items;
|
||||
|
||||
// Error: mutating frozen props (detected during inference)
|
||||
props.items = [];
|
||||
|
||||
return <div>{items.length}</div>;
|
||||
}
|
||||
|
||||
```
|
||||
|
||||
|
||||
## Error
|
||||
|
||||
```
|
||||
Found 2 errors:
|
||||
|
||||
Todo: (BuildHIR::lowerStatement) Handle var kinds in VariableDeclaration
|
||||
|
||||
error.var-declaration-and-mutation-of-props.ts:9:2
|
||||
7 | function Component(props) {
|
||||
8 | // Error: var declaration (Todo from BuildHIR)
|
||||
> 9 | var items = props.items;
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^ (BuildHIR::lowerStatement) Handle var kinds in VariableDeclaration
|
||||
10 |
|
||||
11 | // Error: mutating frozen props (detected during inference)
|
||||
12 | props.items = [];
|
||||
|
||||
Error: This value cannot be modified
|
||||
|
||||
Modifying component props or hook arguments is not allowed. Consider using a local variable instead.
|
||||
|
||||
error.var-declaration-and-mutation-of-props.ts:12:2
|
||||
10 |
|
||||
11 | // Error: mutating frozen props (detected during inference)
|
||||
> 12 | props.items = [];
|
||||
| ^^^^^ value cannot be modified
|
||||
13 |
|
||||
14 | return <div>{items.length}</div>;
|
||||
15 | }
|
||||
```
|
||||
|
||||
|
||||
@@ -0,0 +1,15 @@
|
||||
/**
|
||||
* Fault tolerance test: two independent errors should both be reported.
|
||||
*
|
||||
* Error 1 (BuildHIR): `var` declarations are not supported (treated as `let`)
|
||||
* Error 2 (InferMutationAliasingEffects): Mutation of frozen props
|
||||
*/
|
||||
function Component(props) {
|
||||
// Error: var declaration (Todo from BuildHIR)
|
||||
var items = props.items;
|
||||
|
||||
// Error: mutating frozen props (detected during inference)
|
||||
props.items = [];
|
||||
|
||||
return <div>{items.length}</div>;
|
||||
}
|
||||
@@ -0,0 +1,57 @@
|
||||
|
||||
## Input
|
||||
|
||||
```javascript
|
||||
// @validateRefAccessDuringRender
|
||||
/**
|
||||
* Fault tolerance test: two independent errors should both be reported.
|
||||
*
|
||||
* Error 1 (BuildHIR): `var` declarations are not supported (treated as `let`)
|
||||
* Error 2 (ValidateNoRefAccessInRender): reading ref.current during render
|
||||
*/
|
||||
function Component() {
|
||||
const ref = useRef(null);
|
||||
|
||||
// Error: var declaration (Todo from BuildHIR)
|
||||
var items = [1, 2, 3];
|
||||
|
||||
// Error: reading ref during render
|
||||
const value = ref.current;
|
||||
|
||||
return <div>{value}{items.length}</div>;
|
||||
}
|
||||
|
||||
```
|
||||
|
||||
|
||||
## Error
|
||||
|
||||
```
|
||||
Found 2 errors:
|
||||
|
||||
Todo: (BuildHIR::lowerStatement) Handle var kinds in VariableDeclaration
|
||||
|
||||
error.var-declaration-and-ref-access.ts:12:2
|
||||
10 |
|
||||
11 | // Error: var declaration (Todo from BuildHIR)
|
||||
> 12 | var items = [1, 2, 3];
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^ (BuildHIR::lowerStatement) Handle var kinds in VariableDeclaration
|
||||
13 |
|
||||
14 | // Error: reading ref during render
|
||||
15 | const value = ref.current;
|
||||
|
||||
Error: Cannot access refs during render
|
||||
|
||||
React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the `current` property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef).
|
||||
|
||||
error.var-declaration-and-ref-access.ts:15:16
|
||||
13 |
|
||||
14 | // Error: reading ref during render
|
||||
> 15 | const value = ref.current;
|
||||
| ^^^^^^^^^^^ Cannot access ref value during render
|
||||
16 |
|
||||
17 | return <div>{value}{items.length}</div>;
|
||||
18 | }
|
||||
```
|
||||
|
||||
|
||||
@@ -0,0 +1,23 @@
|
||||
// @validateRefAccessDuringRender
|
||||
/**
|
||||
* Fault tolerance test: two independent errors should both be reported.
|
||||
*
|
||||
* Error 1 (BuildHIR): `var` declarations are not supported (treated as `let`)
|
||||
* Error 2 (ValidateNoRefAccessInRender): reading ref.current during render
|
||||
*/
|
||||
function Component() {
|
||||
const ref = useRef(null);
|
||||
|
||||
// Error: var declaration (Todo from BuildHIR)
|
||||
var items = [1, 2, 3];
|
||||
|
||||
// Error: reading ref during render
|
||||
const value = ref.current;
|
||||
|
||||
return (
|
||||
<div>
|
||||
{value}
|
||||
{items.length}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
Reference in New Issue
Block a user