[compiler] Refactor validations to return Result and log where appropriate

Updates ~all of our validations to return a Result, and then updates callers to either unwrap() if they should bailout or else just log.

ghstack-source-id: 418b5f5
Pull Request resolved: facebook#32688

Author: josephsavona

compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts

@@ -6,6 +6,7 @@
  */
 
 import type {SourceLocation} from './HIR';
+import {Err, Ok, Result} from './Utils/Result';
 import {assertExhaustive} from './Utils/utils';
 
 export enum ErrorSeverity {
@@ -224,6 +225,10 @@ export class CompilerError extends Error {
     return this.details.length > 0;
   }
 
+  asResult(): Result<void, CompilerError> {
+    return this.hasErrors() ? Err(this) : Ok(undefined);
+  }
+
   /*
    * An error is critical if it means the compiler has entered into a broken state and cannot
    * continue safely. Other expected errors such as Todos mean that we can skip over that component

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

@@ -100,7 +100,7 @@ import {propagateScopeDependenciesHIR} from '../HIR/PropagateScopeDependenciesHI
 import {outlineJSX} from '../Optimization/OutlineJsx';
 import {optimizePropsMethodCalls} from '../Optimization/OptimizePropsMethodCalls';
 import {transformFire} from '../Transform';
-import {validateNoImpureFunctionsInRender} from '../Validation/ValiateNoImpureFunctionsInRender';
+import {validateNoImpureFunctionsInRender} from '../Validation/ValidateNoImpureFunctionsInRender';
 import {CompilerError} from '..';
 import {validateStaticComponents} from '../Validation/ValidateStaticComponents';
 
@@ -162,7 +162,7 @@ function runWithEnvironment(
   log({kind: 'hir', name: 'PruneMaybeThrows', value: hir});
 
   validateContextVariableLValues(hir);
-  validateUseMemo(hir);
+  validateUseMemo(hir).unwrap();
 
   if (
     env.isInferredMemoEnabled &&
@@ -203,10 +203,10 @@ function runWithEnvironment(
 
   if (env.isInferredMemoEnabled) {
     if (env.config.validateHooksUsage) {
-      validateHooksUsage(hir);
+      validateHooksUsage(hir).unwrap();
     }
     if (env.config.validateNoCapitalizedCalls) {
-      validateNoCapitalizedCalls(hir);
+      validateNoCapitalizedCalls(hir).unwrap();
     }
   }
 
@@ -256,23 +256,23 @@ function runWithEnvironment(
     }
 
     if (env.config.validateRefAccessDuringRender) {
-      validateNoRefAccessInRender(hir);
+      validateNoRefAccessInRender(hir).unwrap();
     }
 
     if (env.config.validateNoSetStateInRender) {
-      validateNoSetStateInRender(hir);
+      validateNoSetStateInRender(hir).unwrap();
     }
 
     if (env.config.validateNoSetStateInPassiveEffects) {
-      validateNoSetStateInPassiveEffects(hir);
+      env.logErrors(validateNoSetStateInPassiveEffects(hir));
     }
 
     if (env.config.validateNoJSXInTryStatements) {
-      validateNoJSXInTryStatement(hir);
+      env.logErrors(validateNoJSXInTryStatement(hir));
     }
 
     if (env.config.validateNoImpureFunctionsInRender) {
-      validateNoImpureFunctionsInRender(hir);
+      validateNoImpureFunctionsInRender(hir).unwrap();
     }
   }
 
@@ -514,14 +514,14 @@ function runWithEnvironment(
   });
 
   if (env.config.validateMemoizedEffectDependencies) {
-    validateMemoizedEffectDependencies(reactiveFunction);
+    validateMemoizedEffectDependencies(reactiveFunction).unwrap();
   }
 
   if (
     env.config.enablePreserveExistingMemoizationGuarantees ||
     env.config.validatePreserveExistingMemoizationGuarantees
   ) {
-    validatePreservedManualMemoization(reactiveFunction);
+    validatePreservedManualMemoization(reactiveFunction).unwrap();
   }
 
   const ast = codegenFunction(reactiveFunction, {

compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateHooksUsage.ts

@@ -26,6 +26,7 @@ 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
@@ -87,7 +88,9 @@ 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): void {
+export function validateHooksUsage(
+  fn: HIRFunction,
+): Result<void, CompilerError> {
   const unconditionalBlocks = computeUnconditionalBlocks(fn);
 
   const errors = new CompilerError();
@@ -423,9 +426,7 @@ export function validateHooksUsage(fn: HIRFunction): void {
   for (const [, error] of errorsByPlace) {
     errors.push(error);
   }
-  if (errors.hasErrors()) {
-    throw errors;
-  }
+  return errors.asResult();
 }
 
 function visitFunctionExpression(errors: CompilerError, fn: HIRFunction): void {

compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateMemoizedEffectDependencies.ts

@@ -22,6 +22,7 @@ import {
   ReactiveFunctionVisitor,
   visitReactiveFunction,
 } from '../ReactiveScopes/visitors';
+import {Result} from '../Utils/Result';
 
 /**
  * Validates that all known effect dependencies are memoized. The algorithm checks two things:
@@ -47,12 +48,12 @@ import {
  * mutate(object); // ... mutable range ends here after this mutation
  * ```
  */
-export function validateMemoizedEffectDependencies(fn: ReactiveFunction): void {
+export function validateMemoizedEffectDependencies(
+  fn: ReactiveFunction,
+): Result<void, CompilerError> {
   const errors = new CompilerError();
   visitReactiveFunction(fn, new Visitor(), errors);
-  if (errors.hasErrors()) {
-    throw errors;
-  }
+  return errors.asResult();
 }
 
 class Visitor extends ReactiveFunctionVisitor<CompilerError> {

compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoCapitalizedCalls.ts

@@ -4,11 +4,14 @@
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
  */
-import {CompilerError, EnvironmentConfig} from '..';
+import {CompilerError, EnvironmentConfig, ErrorSeverity} from '..';
 import {HIRFunction, IdentifierId} from '../HIR';
 import {DEFAULT_GLOBALS} from '../HIR/Globals';
+import {Result} from '../Utils/Result';
 
-export function validateNoCapitalizedCalls(fn: HIRFunction): void {
+export function validateNoCapitalizedCalls(
+  fn: HIRFunction,
+): Result<void, CompilerError> {
   const envConfig: EnvironmentConfig = fn.env.config;
   const ALLOW_LIST = new Set([
     ...DEFAULT_GLOBALS.keys(),
@@ -26,6 +29,7 @@ export function validateNoCapitalizedCalls(fn: HIRFunction): void {
     );
   };
 
+  const errors = new CompilerError();
   const capitalLoadGlobals = new Map<IdentifierId, string>();
   const capitalizedProperties = new Map<IdentifierId, string>();
   const reason =
@@ -73,7 +77,8 @@ export function validateNoCapitalizedCalls(fn: HIRFunction): void {
           const propertyIdentifier = value.property.identifier.id;
           const propertyName = capitalizedProperties.get(propertyIdentifier);
           if (propertyName != null) {
-            CompilerError.throwInvalidReact({
+            errors.push({
+              severity: ErrorSeverity.InvalidReact,
               reason,
               description: `${propertyName} may be a component.`,
               loc: value.loc,
@@ -85,4 +90,5 @@ export function validateNoCapitalizedCalls(fn: HIRFunction): void {
       }
     }
   }
+  return errors.asResult();
 }

compiler/packages/babel-plugin-react-compiler/src/Validation/ValiateNoImpureFunctionsInRender.ts renamed to compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoImpureFunctionsInRender.ts

@@ -8,6 +8,7 @@
 import {CompilerError, ErrorSeverity} from '..';
 import {HIRFunction} from '../HIR';
 import {getFunctionCallSignature} from '../Inference/InferReferenceEffects';
+import {Result} from '../Utils/Result';
 
 /**
  * Checks that known-impure functions are not called during render. Examples of invalid functions to
@@ -18,7 +19,9 @@ import {getFunctionCallSignature} from '../Inference/InferReferenceEffects';
  * 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): void {
+export function validateNoImpureFunctionsInRender(
+  fn: HIRFunction,
+): Result<void, CompilerError> {
   const errors = new CompilerError();
   for (const [, block] of fn.body.blocks) {
     for (const instr of block.instructions) {
@@ -46,7 +49,5 @@ export function validateNoImpureFunctionsInRender(fn: HIRFunction): void {
       }
     }
   }
-  if (errors.hasErrors()) {
-    throw errors;
-  }
+  return errors.asResult();
 }

compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoJSXInTryStatement.ts

@@ -7,6 +7,7 @@
 
 import {CompilerError, ErrorSeverity} from '..';
 import {BlockId, HIRFunction} from '../HIR';
+import {Result} from '../Utils/Result';
 import {retainWhere} from '../Utils/utils';
 
 /**
@@ -19,7 +20,9 @@ import {retainWhere} from '../Utils/utils';
  * created within a try block. JSX is allowed within a catch statement, unless that catch
  * is itself nested inside an outer try.
  */
-export function validateNoJSXInTryStatement(fn: HIRFunction): void {
+export function validateNoJSXInTryStatement(
+  fn: HIRFunction,
+): Result<void, CompilerError> {
   const activeTryBlocks: Array<BlockId> = [];
   const errors = new CompilerError();
   for (const [, block] of fn.body.blocks) {
@@ -46,7 +49,5 @@ export function validateNoJSXInTryStatement(fn: HIRFunction): void {
       activeTryBlocks.push(block.terminal.handler);
     }
   }
-  if (errors.hasErrors()) {
-    throw errors;
-  }
+  return errors.asResult();
 }

compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccesInRender.ts

@@ -99,9 +99,11 @@ class Env extends Map<IdentifierId, RefAccessType> {
   }
 }
 
-export function validateNoRefAccessInRender(fn: HIRFunction): void {
+export function validateNoRefAccessInRender(
+  fn: HIRFunction,
+): Result<void, CompilerError> {
   const env = new Env();
-  validateNoRefAccessInRenderImpl(fn, env).unwrap();
+  return validateNoRefAccessInRenderImpl(fn, env).map(_ => undefined);
 }
 
 function refTypeOfType(place: Place): RefAccessType {

compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoSetStateInPassiveEffects.ts

@@ -14,6 +14,7 @@ import {
   Place,
 } from '../HIR';
 import {eachInstructionValueOperand} from '../HIR/visitors';
+import {Result} from '../Utils/Result';
 
 /**
  * Validates against calling setState in the body of a *passive* effect (useEffect),
@@ -23,7 +24,9 @@ import {eachInstructionValueOperand} from '../HIR/visitors';
  * often bad for performance and frequently has more efficient and straightforward
  * alternatives. See https://react.dev/learn/you-might-not-need-an-effect for examples.
  */
-export function validateNoSetStateInPassiveEffects(fn: HIRFunction): void {
+export function validateNoSetStateInPassiveEffects(
+  fn: HIRFunction,
+): Result<void, CompilerError> {
   const setStateFunctions: Map<IdentifierId, Place> = new Map();
   const errors = new CompilerError();
   for (const [, block] of fn.body.blocks) {
@@ -98,9 +101,7 @@ export function validateNoSetStateInPassiveEffects(fn: HIRFunction): void {
     }
   }
 
-  if (errors.hasErrors()) {
-    throw errors;
-  }
+  return errors.asResult();
 }
 
 function getSetStateCall(

compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoSetStateInRender.ts

@@ -9,7 +9,7 @@ import {CompilerError, ErrorSeverity} from '../CompilerError';
 import {HIRFunction, IdentifierId, isSetStateType} from '../HIR';
 import {computeUnconditionalBlocks} from '../HIR/ComputeUnconditionalBlocks';
 import {eachInstructionValueOperand} from '../HIR/visitors';
-import {Err, Ok, Result} from '../Utils/Result';
+import {Result} from '../Utils/Result';
 
 /**
  * Validates that the given function does not have an infinite update loop
@@ -39,9 +39,11 @@ import {Err, Ok, Result} from '../Utils/Result';
  * y();
  * ```
  */
-export function validateNoSetStateInRender(fn: HIRFunction): void {
+export function validateNoSetStateInRender(
+  fn: HIRFunction,
+): Result<void, CompilerError> {
   const unconditionalSetStateFunctions: Set<IdentifierId> = new Set();
-  validateNoSetStateInRenderImpl(fn, unconditionalSetStateFunctions).unwrap();
+  return validateNoSetStateInRenderImpl(fn, unconditionalSetStateFunctions);
 }
 
 function validateNoSetStateInRenderImpl(
@@ -145,9 +147,5 @@ function validateNoSetStateInRenderImpl(
     }
   }
 
-  if (errors.hasErrors()) {
-    return Err(errors);
-  } else {
-    return Ok(undefined);
-  }
+  return errors.asResult();
 }