[compiler] Phase 4 (batch 1): Update validation passes to record errors on env (#35875)

Update 9 validation passes to record errors directly on fn.env instead
of
returning Result<void, CompilerError>:
- validateHooksUsage
- validateNoCapitalizedCalls (also changed throwInvalidReact to
recordError)
- validateUseMemo
- dropManualMemoization
- validateNoRefAccessInRender
- validateNoSetStateInRender
- validateNoImpureFunctionsInRender
- validateNoFreezingKnownMutableFunctions
- validateExhaustiveDependencies

Each pass now calls fn.env.recordErrors() instead of returning
errors.asResult().
Pipeline.ts call sites updated to remove tryRecord() wrappers and
.unwrap().

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35875).
* #35888
* #35884
* #35883
* #35882
* #35881
* #35880
* #35879
* #35878
* #35877
* #35876
* __->__ #35875
This commit is contained in:
Joseph Savona
2026-02-23 15:35:52 -08:00
committed by GitHub
parent 426a394845
commit e3e5d95cc4
12 changed files with 86 additions and 95 deletions

View File

@@ -127,49 +127,49 @@ All validation passes need to record errors on the environment instead of return
These passes already accumulate errors internally and return `Result<void, CompilerError>`. The change is: instead of returning the Result, record errors on `env` and return void. Remove the `.unwrap()` call in Pipeline.ts.
- [ ] **4.1 `validateHooksUsage`** (`src/Validation/ValidateHooksUsage.ts`)
- [x] **4.1 `validateHooksUsage`** (`src/Validation/ValidateHooksUsage.ts`)
- Change signature from `(fn: HIRFunction): Result<void, CompilerError>` to `(fn: HIRFunction): void`
- Record errors on `fn.env` instead of returning `errors.asResult()`
- Update Pipeline.ts call site (line 211): remove `.unwrap()`
- [ ] **4.2 `validateNoCapitalizedCalls`** (`src/Validation/ValidateNoCapitalizedCalls.ts`)
- [x] **4.2 `validateNoCapitalizedCalls`** (`src/Validation/ValidateNoCapitalizedCalls.ts`)
- Change signature to return void
- Fix the hybrid pattern: the direct `CallExpression` path currently throws via `CompilerError.throwInvalidReact()` — change to record on env
- The `MethodCall` path already accumulates — change to record on env
- Update Pipeline.ts call site (line 214): remove `.unwrap()`
- [ ] **4.3 `validateUseMemo`** (`src/Validation/ValidateUseMemo.ts`)
- [x] **4.3 `validateUseMemo`** (`src/Validation/ValidateUseMemo.ts`)
- Change signature to return void
- Record hard errors on env instead of returning `errors.asResult()`
- The soft `voidMemoErrors` path already uses `env.logErrors()` — keep as-is or also record
- Update Pipeline.ts call site (line 170): remove `.unwrap()`
- [ ] **4.4 `dropManualMemoization`** (`src/Inference/DropManualMemoization.ts`)
- [x] **4.4 `dropManualMemoization`** (`src/Inference/DropManualMemoization.ts`)
- Change signature to return void
- Record errors on env instead of returning `errors.asResult()`
- Update Pipeline.ts call site (line 178): remove `.unwrap()`
- [ ] **4.5 `validateNoRefAccessInRender`** (`src/Validation/ValidateNoRefAccessInRender.ts`)
- [x] **4.5 `validateNoRefAccessInRender`** (`src/Validation/ValidateNoRefAccessInRender.ts`)
- Change signature to return void
- Record errors on env instead of returning Result
- Update Pipeline.ts call site (line 275): remove `.unwrap()`
- [ ] **4.6 `validateNoSetStateInRender`** (`src/Validation/ValidateNoSetStateInRender.ts`)
- [x] **4.6 `validateNoSetStateInRender`** (`src/Validation/ValidateNoSetStateInRender.ts`)
- Change signature to return void
- Record errors on env
- Update Pipeline.ts call site (line 279): remove `.unwrap()`
- [ ] **4.7 `validateNoImpureFunctionsInRender`** (`src/Validation/ValidateNoImpureFunctionsInRender.ts`)
- [x] **4.7 `validateNoImpureFunctionsInRender`** (`src/Validation/ValidateNoImpureFunctionsInRender.ts`)
- Change signature to return void
- Record errors on env
- Update Pipeline.ts call site (line 300): remove `.unwrap()`
- [ ] **4.8 `validateNoFreezingKnownMutableFunctions`** (`src/Validation/ValidateNoFreezingKnownMutableFunctions.ts`)
- [x] **4.8 `validateNoFreezingKnownMutableFunctions`** (`src/Validation/ValidateNoFreezingKnownMutableFunctions.ts`)
- Change signature to return void
- Record errors on env
- Update Pipeline.ts call site (line 303): remove `.unwrap()`
- [ ] **4.9 `validateExhaustiveDependencies`** (`src/Validation/ValidateExhaustiveDependencies.ts`)
- [x] **4.9 `validateExhaustiveDependencies`** (`src/Validation/ValidateExhaustiveDependencies.ts`)
- Change signature to return void
- Record errors on env
- Update Pipeline.ts call site (line 315): remove `.unwrap()`

View File

@@ -164,14 +164,10 @@ function runWithEnvironment(
env.tryRecord(() => {
validateContextVariableLValues(hir);
});
env.tryRecord(() => {
validateUseMemo(hir).unwrap();
});
validateUseMemo(hir);
if (env.enableDropManualMemoization) {
env.tryRecord(() => {
dropManualMemoization(hir).unwrap();
});
dropManualMemoization(hir);
log({kind: 'hir', name: 'DropManualMemoization', value: hir});
}
@@ -204,14 +200,10 @@ function runWithEnvironment(
if (env.enableValidations) {
if (env.config.validateHooksUsage) {
env.tryRecord(() => {
validateHooksUsage(hir).unwrap();
});
validateHooksUsage(hir);
}
if (env.config.validateNoCapitalizedCalls) {
env.tryRecord(() => {
validateNoCapitalizedCalls(hir).unwrap();
});
validateNoCapitalizedCalls(hir);
}
}
@@ -259,15 +251,11 @@ function runWithEnvironment(
}
if (env.config.validateRefAccessDuringRender) {
env.tryRecord(() => {
validateNoRefAccessInRender(hir).unwrap();
});
validateNoRefAccessInRender(hir);
}
if (env.config.validateNoSetStateInRender) {
env.tryRecord(() => {
validateNoSetStateInRender(hir).unwrap();
});
validateNoSetStateInRender(hir);
}
if (
@@ -290,7 +278,7 @@ function runWithEnvironment(
}
env.tryRecord(() => {
validateNoFreezingKnownMutableFunctions(hir).unwrap();
validateNoFreezingKnownMutableFunctions(hir);
});
}
@@ -303,9 +291,7 @@ function runWithEnvironment(
env.config.validateExhaustiveEffectDependencies
) {
// NOTE: this relies on reactivity inference running first
env.tryRecord(() => {
validateExhaustiveDependencies(hir).unwrap();
});
validateExhaustiveDependencies(hir);
}
}

View File

@@ -31,7 +31,6 @@ import {
makeInstructionId,
} from '../HIR';
import {createTemporaryPlace, markInstructionIds} from '../HIR/HIRBuilder';
import {Result} from '../Utils/Result';
type ManualMemoCallee = {
kind: 'useMemo' | 'useCallback';
@@ -389,9 +388,7 @@ function extractManualMemoizationArgs(
* This pass also validates that useMemo callbacks return a value (not void), ensuring that useMemo
* is only used for memoizing values and not for running arbitrary side effects.
*/
export function dropManualMemoization(
func: HIRFunction,
): Result<void, CompilerError> {
export function dropManualMemoization(func: HIRFunction): void {
const errors = new CompilerError();
const isValidationEnabled =
func.env.config.validatePreserveExistingMemoizationGuarantees ||
@@ -553,7 +550,9 @@ export function dropManualMemoization(
}
}
return errors.asResult();
if (errors.hasAnyErrors()) {
func.env.recordErrors(errors);
}
}
function findOptionalPlaces(fn: HIRFunction): Set<IdentifierId> {

View File

@@ -44,7 +44,6 @@ import {
eachInstructionValueOperand,
eachTerminalOperand,
} from '../HIR/visitors';
import {Result} from '../Utils/Result';
import {retainWhere} from '../Utils/utils';
const DEBUG = false;
@@ -88,9 +87,7 @@ const DEBUG = false;
* When we go to compute the dependencies, we then think that the user's manual dep
* logic is part of what the memo computation logic.
*/
export function validateExhaustiveDependencies(
fn: HIRFunction,
): Result<void, CompilerError> {
export function validateExhaustiveDependencies(fn: HIRFunction): void {
const env = fn.env;
const reactive = collectReactiveIdentifiersHIR(fn);
@@ -217,7 +214,9 @@ export function validateExhaustiveDependencies(
},
false, // isFunctionExpression
);
return error.asResult();
if (error.hasAnyErrors()) {
fn.env.recordErrors(error);
}
}
function validateDependencies(

View File

@@ -26,7 +26,6 @@ import {
eachTerminalOperand,
} from '../HIR/visitors';
import {assertExhaustive} from '../Utils/utils';
import {Result} from '../Utils/Result';
/**
* Represents the possible kinds of value which may be stored at a given Place during
@@ -88,9 +87,7 @@ function joinKinds(a: Kind, b: Kind): Kind {
* may not appear as the callee of a conditional call.
* See the note for Kind.PotentialHook for sources of potential hooks
*/
export function validateHooksUsage(
fn: HIRFunction,
): Result<void, CompilerError> {
export function validateHooksUsage(fn: HIRFunction): void {
const unconditionalBlocks = computeUnconditionalBlocks(fn);
const errors = new CompilerError();
@@ -426,7 +423,9 @@ export function validateHooksUsage(
for (const [, error] of errorsByPlace) {
errors.pushErrorDetail(error);
}
return errors.asResult();
if (errors.hasAnyErrors()) {
fn.env.recordErrors(errors);
}
}
function visitFunctionExpression(errors: CompilerError, fn: HIRFunction): void {

View File

@@ -5,15 +5,12 @@
* LICENSE file in the root directory of this source tree.
*/
import {CompilerError, EnvironmentConfig} from '..';
import {CompilerError, CompilerErrorDetail, EnvironmentConfig} from '..';
import {ErrorCategory} from '../CompilerError';
import {HIRFunction, IdentifierId} from '../HIR';
import {DEFAULT_GLOBALS} from '../HIR/Globals';
import {Result} from '../Utils/Result';
export function validateNoCapitalizedCalls(
fn: HIRFunction,
): Result<void, CompilerError> {
export function validateNoCapitalizedCalls(fn: HIRFunction): void {
const envConfig: EnvironmentConfig = fn.env.config;
const ALLOW_LIST = new Set([
...DEFAULT_GLOBALS.keys(),
@@ -48,13 +45,16 @@ export function validateNoCapitalizedCalls(
const calleeIdentifier = value.callee.identifier.id;
const calleeName = capitalLoadGlobals.get(calleeIdentifier);
if (calleeName != null) {
CompilerError.throwInvalidReact({
category: ErrorCategory.CapitalizedCalls,
reason,
description: `${calleeName} may be a component`,
loc: value.loc,
suggestions: null,
});
fn.env.recordError(
new CompilerErrorDetail({
category: ErrorCategory.CapitalizedCalls,
reason,
description: `${calleeName} may be a component`,
loc: value.loc,
suggestions: null,
}),
);
continue;
}
break;
}
@@ -85,5 +85,7 @@ export function validateNoCapitalizedCalls(
}
}
}
return errors.asResult();
if (errors.hasAnyErrors()) {
fn.env.recordErrors(errors);
}
}

View File

@@ -18,7 +18,6 @@ import {
eachTerminalOperand,
} from '../HIR/visitors';
import {AliasingEffect} from '../Inference/AliasingEffects';
import {Result} from '../Utils/Result';
/**
* Validates that functions with known mutations (ie due to types) cannot be passed
@@ -43,9 +42,7 @@ import {Result} from '../Utils/Result';
* This pass detects functions with *known* mutations (Store or Mutate, not ConditionallyMutate)
* that are passed where a frozen value is expected and rejects them.
*/
export function validateNoFreezingKnownMutableFunctions(
fn: HIRFunction,
): Result<void, CompilerError> {
export function validateNoFreezingKnownMutableFunctions(fn: HIRFunction): void {
const errors = new CompilerError();
const contextMutationEffects: Map<
IdentifierId,
@@ -162,5 +159,7 @@ export function validateNoFreezingKnownMutableFunctions(
visitOperand(operand);
}
}
return errors.asResult();
if (errors.hasAnyErrors()) {
fn.env.recordErrors(errors);
}
}

View File

@@ -9,7 +9,6 @@ import {CompilerDiagnostic, CompilerError} from '..';
import {ErrorCategory} from '../CompilerError';
import {HIRFunction} from '../HIR';
import {getFunctionCallSignature} from '../Inference/InferMutationAliasingEffects';
import {Result} from '../Utils/Result';
/**
* Checks that known-impure functions are not called during render. Examples of invalid functions to
@@ -20,9 +19,7 @@ import {Result} from '../Utils/Result';
* this in several of our validation passes and should unify those analyses into a reusable helper
* and use it here.
*/
export function validateNoImpureFunctionsInRender(
fn: HIRFunction,
): Result<void, CompilerError> {
export function validateNoImpureFunctionsInRender(fn: HIRFunction): void {
const errors = new CompilerError();
for (const [, block] of fn.body.blocks) {
for (const instr of block.instructions) {
@@ -55,5 +52,7 @@ export function validateNoImpureFunctionsInRender(
}
}
}
return errors.asResult();
if (errors.hasAnyErrors()) {
fn.env.recordErrors(errors);
}
}

View File

@@ -27,7 +27,6 @@ import {
eachPatternOperand,
eachTerminalOperand,
} from '../HIR/visitors';
import {Err, Ok, Result} from '../Utils/Result';
import {retainWhere} from '../Utils/utils';
/**
@@ -120,12 +119,14 @@ class Env {
}
}
export function validateNoRefAccessInRender(
fn: HIRFunction,
): Result<void, CompilerError> {
export function validateNoRefAccessInRender(fn: HIRFunction): void {
const env = new Env();
collectTemporariesSidemap(fn, env);
return validateNoRefAccessInRenderImpl(fn, env).map(_ => undefined);
const errors = new CompilerError();
validateNoRefAccessInRenderImpl(fn, env, errors);
if (errors.hasAnyErrors()) {
fn.env.recordErrors(errors);
}
}
function collectTemporariesSidemap(fn: HIRFunction, env: Env): void {
@@ -305,7 +306,8 @@ function joinRefAccessTypes(...types: Array<RefAccessType>): RefAccessType {
function validateNoRefAccessInRenderImpl(
fn: HIRFunction,
env: Env,
): Result<RefAccessType, CompilerError> {
errors: CompilerError,
): RefAccessType {
let returnValues: Array<undefined | RefAccessType> = [];
let place;
for (const param of fn.params) {
@@ -336,7 +338,6 @@ function validateNoRefAccessInRenderImpl(
env.resetChanged();
returnValues = [];
const safeBlocks: Array<{block: BlockId; ref: RefId}> = [];
const errors = new CompilerError();
for (const [, block] of fn.body.blocks) {
retainWhere(safeBlocks, entry => entry.block !== block.id);
for (const phi of block.phis) {
@@ -432,13 +433,15 @@ function validateNoRefAccessInRenderImpl(
case 'FunctionExpression': {
let returnType: RefAccessType = {kind: 'None'};
let readRefEffect = false;
const innerErrors = new CompilerError();
const result = validateNoRefAccessInRenderImpl(
instr.value.loweredFunc.func,
env,
innerErrors,
);
if (result.isOk()) {
returnType = result.unwrap();
} else if (result.isErr()) {
if (!innerErrors.hasAnyErrors()) {
returnType = result;
} else {
readRefEffect = true;
}
env.set(instr.lvalue.identifier.id, {
@@ -729,7 +732,7 @@ function validateNoRefAccessInRenderImpl(
}
if (errors.hasAnyErrors()) {
return Err(errors);
return {kind: 'None'};
}
}
@@ -738,10 +741,8 @@ function validateNoRefAccessInRenderImpl(
loc: GeneratedSource,
});
return Ok(
joinRefAccessTypes(
...returnValues.filter((env): env is RefAccessType => env !== undefined),
),
return joinRefAccessTypes(
...returnValues.filter((env): env is RefAccessType => env !== undefined),
);
}

View File

@@ -13,7 +13,6 @@ import {
import {HIRFunction, IdentifierId, isSetStateType} from '../HIR';
import {computeUnconditionalBlocks} from '../HIR/ComputeUnconditionalBlocks';
import {eachInstructionValueOperand} from '../HIR/visitors';
import {Result} from '../Utils/Result';
/**
* Validates that the given function does not have an infinite update loop
@@ -43,17 +42,21 @@ import {Result} from '../Utils/Result';
* y();
* ```
*/
export function validateNoSetStateInRender(
fn: HIRFunction,
): Result<void, CompilerError> {
export function validateNoSetStateInRender(fn: HIRFunction): void {
const unconditionalSetStateFunctions: Set<IdentifierId> = new Set();
return validateNoSetStateInRenderImpl(fn, unconditionalSetStateFunctions);
const errors = validateNoSetStateInRenderImpl(
fn,
unconditionalSetStateFunctions,
);
if (errors.hasAnyErrors()) {
fn.env.recordErrors(errors);
}
}
function validateNoSetStateInRenderImpl(
fn: HIRFunction,
unconditionalSetStateFunctions: Set<IdentifierId>,
): Result<void, CompilerError> {
): CompilerError {
const unconditionalBlocks = computeUnconditionalBlocks(fn);
let activeManualMemoId: number | null = null;
const errors = new CompilerError();
@@ -92,7 +95,7 @@ function validateNoSetStateInRenderImpl(
validateNoSetStateInRenderImpl(
instr.value.loweredFunc.func,
unconditionalSetStateFunctions,
).isErr()
).hasAnyErrors()
) {
// This function expression unconditionally calls a setState
unconditionalSetStateFunctions.add(instr.lvalue.identifier.id);
@@ -183,5 +186,5 @@ function validateNoSetStateInRenderImpl(
}
}
return errors.asResult();
return errors;
}

View File

@@ -20,9 +20,8 @@ import {
eachInstructionValueOperand,
eachTerminalOperand,
} from '../HIR/visitors';
import {Result} from '../Utils/Result';
export function validateUseMemo(fn: HIRFunction): Result<void, CompilerError> {
export function validateUseMemo(fn: HIRFunction): void {
const errors = new CompilerError();
const voidMemoErrors = new CompilerError();
const useMemos = new Set<IdentifierId>();
@@ -177,7 +176,9 @@ export function validateUseMemo(fn: HIRFunction): Result<void, CompilerError> {
}
}
fn.env.logErrors(voidMemoErrors.asResult());
return errors.asResult();
if (errors.hasAnyErrors()) {
fn.env.recordErrors(errors);
}
}
function validateNoContextVariableAssignment(

View File

@@ -64,6 +64,9 @@ testRule(
makeTestCaseError(
'Capitalized functions are reserved for components',
),
makeTestCaseError(
'Capitalized functions are reserved for components',
),
],
},
],