[compiler] Validate ref reads in render via improved impure value validation

Updates to guard against *reading* refs during render via the improved validateNoImpureValuesInRender() pass. InferMutationAliasingEffects generates `Impure` effects for ref reads, and then this pass validates that those accesses don't flow into `Render` effects. We now call the impure value validation first so that it takes priority over validateNoRefAccessInRender - the latter still has all the existing logic for now, but we can dramatically simplify it in a follow-up PR so that it only has to validate against ref writes.

Author: josephsavona

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

@@ -271,10 +271,6 @@ function runWithEnvironment(
       assertValidMutableRanges(hir);
     }
 
-    if (env.config.validateRefAccessDuringRender) {
-      validateNoRefAccessInRender(hir).unwrap();
-    }
-
     if (env.config.validateNoSetStateInRender) {
       validateNoSetStateInRender(hir).unwrap();
     }
@@ -296,10 +292,17 @@ function runWithEnvironment(
       env.logErrors(validateNoJSXInTryStatement(hir));
     }
 
-    if (env.config.validateNoImpureFunctionsInRender) {
+    if (
+      env.config.validateNoImpureFunctionsInRender ||
+      env.config.validateRefAccessDuringRender
+    ) {
       validateNoImpureValuesInRender(hir).unwrap();
     }
 
+    if (env.config.validateRefAccessDuringRender) {
+      validateNoRefAccessInRender(hir).unwrap();
+    }
+
     validateNoFreezingKnownMutableFunctions(hir).unwrap();
   }
 

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

@@ -1890,6 +1890,13 @@ export function isJsxType(type: Type): boolean {
   return type.kind === 'Object' && type.shapeId === 'BuiltInJsx';
 }
 
+export function isJsxOrJsxUnionType(type: Type): boolean {
+  return (
+    (type.kind === 'Object' && type.shapeId === 'BuiltInJsx') ||
+    (type.kind === 'Phi' && type.operands.some(op => isJsxOrJsxUnionType(op)))
+  );
+}
+
 export function isRefOrRefValue(id: Identifier): boolean {
   return isUseRefType(id) || isRefValueType(id);
 }
@@ -2058,4 +2065,23 @@ export function getHookKindForType(
   return null;
 }
 
+export function areEqualSourceLocations(
+  loc1: SourceLocation,
+  loc2: SourceLocation,
+): boolean {
+  if (typeof loc1 === 'symbol' || typeof loc2 === 'symbol') {
+    return false;
+  }
+  return (
+    loc1.filename === loc2.filename &&
+    loc1.identifierName === loc2.identifierName &&
+    loc1.start.line === loc2.start.line &&
+    loc1.start.column === loc2.start.column &&
+    loc1.start.index === loc2.start.index &&
+    loc1.end.line === loc2.end.line &&
+    loc1.end.column === loc2.end.column &&
+    loc1.end.index === loc2.end.index
+  );
+}
+
 export * from './Types';

compiler/packages/babel-plugin-react-compiler/src/HIR/HIRBuilder.ts

@@ -988,7 +988,7 @@ export function createTemporaryPlace(
     identifier: makeTemporaryIdentifier(env.nextIdentifierId, loc),
     reactive: false,
     effect: Effect.Unknown,
-    loc: GeneratedSource,
+    loc,
   };
 }
 

compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts

@@ -5,7 +5,7 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-import {CompilerError} from '../CompilerError';
+import {CompilerError, ErrorCategory} from '../CompilerError';
 import {AliasingEffect, AliasingSignature} from '../Inference/AliasingEffects';
 import {assertExhaustive} from '../Utils/utils';
 import {
@@ -194,8 +194,11 @@ function parseAliasingSignatureConfig(
           return {
             kind: 'Impure',
             into,
+            category: ErrorCategory.Purity,
             description: effect.description,
             reason: effect.reason,
+            sourceMessage: effect.sourceMessage,
+            usageMessage: effect.usageMessage,
           };
         }
         case 'Render': {

compiler/packages/babel-plugin-react-compiler/src/HIR/TypeSchema.ts

@@ -188,13 +188,17 @@ export type ImpureEffectConfig = {
   into: string;
   reason: string;
   description: string;
+  sourceMessage: string;
+  usageMessage: string;
 };
 
 export const ImpureEffectSchema: z.ZodType<ImpureEffectConfig> = z.object({
   kind: z.literal('Impure'),
   into: LifetimeIdSchema,
   reason: z.string(),
   description: z.string(),
+  sourceMessage: z.string(),
+  usageMessage: z.string(),
 });
 
 export type RenderEffectConfig = {

compiler/packages/babel-plugin-react-compiler/src/Inference/AliasingEffects.ts

@@ -5,7 +5,7 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-import {CompilerDiagnostic} from '../CompilerError';
+import {CompilerDiagnostic, ErrorCategory} from '../CompilerError';
 import {
   FunctionExpression,
   GeneratedSource,
@@ -162,7 +162,15 @@ export type AliasingEffect =
   /**
    * Indicates a side-effect that is not safe during render
    */
-  | {kind: 'Impure'; into: Place; reason: string; description: string}
+  | {
+      kind: 'Impure';
+      into: Place;
+      category: ErrorCategory;
+      reason: string;
+      description: string;
+      usageMessage: string;
+      sourceMessage: string;
+    }
   /**
    * Indicates that a given place is accessed during render. Used to distingush
    * hook arguments that are known to be called immediately vs those used for
@@ -227,6 +235,8 @@ export function hashEffect(effect: AliasingEffect): string {
         effect.into.identifier.id,
         effect.reason,
         effect.description,
+        effect.usageMessage,
+        effect.sourceMessage,
       ].join(':');
     case 'Render': {
       return [effect.kind, effect.place.identifier.id].join(':');

compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts

@@ -27,6 +27,7 @@ import {
   InstructionKind,
   InstructionValue,
   isArrayType,
+  isJsxOrJsxUnionType,
   isMapType,
   isPrimitiveType,
   isRefOrRefValue,
@@ -1988,8 +1989,11 @@ function computeSignatureForInstruction(
         effects.push({
           kind: 'Impure',
           into: lvalue,
+          category: ErrorCategory.Refs,
           reason: `Cannot access ref value during render`,
           description: REF_ERROR_DESCRIPTION,
+          sourceMessage: `Ref is initially accessed`,
+          usageMessage: `Ref value is used during render`,
         });
       }
       break;
@@ -2156,6 +2160,15 @@ function computeSignatureForInstruction(
           into: lvalue,
         });
       }
+      if (value.children != null) {
+        // Children are typically called during render, not used as an event/effect callback
+        for (const child of value.children) {
+          effects.push({
+            kind: 'Render',
+            place: child,
+          });
+        }
+      }
       if (value.kind === 'JsxExpression') {
         if (value.tag.kind === 'Identifier') {
           // Tags are render function, by definition they're called during render
@@ -2164,24 +2177,23 @@ function computeSignatureForInstruction(
             place: value.tag,
           });
         }
-        if (value.children != null) {
-          // Children are typically called during render, not used as an event/effect callback
-          for (const child of value.children) {
+        for (const prop of value.props) {
+          const place =
+            prop.kind === 'JsxAttribute' ? prop.place : prop.argument;
+          if (place.identifier.type.kind === 'Function') {
+            if (isJsxOrJsxUnionType(place.identifier.type.return)) {
+              effects.push({
+                kind: 'Render',
+                place,
+              });
+            }
+          } else {
             effects.push({
               kind: 'Render',
-              place: child,
+              place,
             });
           }
         }
-        for (const prop of value.props) {
-          if (prop.kind === 'JsxAttribute' && /^on[A-Z]/.test(prop.name)) {
-            continue;
-          }
-          effects.push({
-            kind: 'Render',
-            place: prop.kind === 'JsxAttribute' ? prop.place : prop.argument,
-          });
-        }
       }
       break;
     }
@@ -2448,14 +2460,17 @@ function computeEffectsForLegacySignature(
     effects.push({
       kind: 'Impure',
       into: lvalue,
-      reason:
-        signature.canonicalName != null
-          ? `\`${signature.canonicalName}\` is an impure function.`
-          : 'This function is impure',
+      category: ErrorCategory.Purity,
+      reason: 'Cannot access impure value during render',
       description:
         'Calling an impure function can produce unstable results that update ' +
         'unpredictably when the component happens to re-render. ' +
         '(https://react.dev/reference/rules/components-and-hooks-must-be-pure#components-and-hooks-must-be-idempotent)',
+      sourceMessage:
+        signature.canonicalName != null
+          ? `\`${signature.canonicalName}\` is an impure function.`
+          : 'This function is impure',
+      usageMessage: 'Cannot access impure value during render',
     });
   }
   if (signature.knownIncompatible != null && state.env.enableValidations) {
@@ -2755,14 +2770,19 @@ function computeEffectsForSignature(
         break;
       }
       case 'Impure': {
-        const values = substitutions.get(effect.into.identifier.id) ?? [];
-        for (const value of values) {
-          effects.push({
-            kind: effect.kind,
-            into: value,
-            reason: effect.reason,
-            description: effect.description,
-          });
+        if (env.config.validateNoImpureFunctionsInRender) {
+          const values = substitutions.get(effect.into.identifier.id) ?? [];
+          for (const value of values) {
+            effects.push({
+              kind: effect.kind,
+              into: value,
+              category: effect.category,
+              reason: effect.reason,
+              description: effect.description,
+              sourceMessage: effect.sourceMessage,
+              usageMessage: effect.usageMessage,
+            });
+          }
         }
         break;
       }

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

@@ -5,8 +5,13 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-import {CompilerDiagnostic, CompilerError, Effect, ErrorCategory} from '..';
-import {HIRFunction, IdentifierId, isUseRefType} from '../HIR';
+import {CompilerDiagnostic, CompilerError, Effect} from '..';
+import {
+  areEqualSourceLocations,
+  HIRFunction,
+  IdentifierId,
+  isUseRefType,
+} from '../HIR';
 import {
   eachInstructionLValue,
   eachInstructionValueOperand,
@@ -246,23 +251,23 @@ function inferImpureValues(
           continue;
         }
         const impureEffect = impure.get(effect.place.identifier.id)!;
-        error.pushDiagnostic(
-          CompilerDiagnostic.create({
-            category: ErrorCategory.Purity,
-            reason: 'Cannot access impure value during render',
-            description: impureEffect.description,
-          })
-            .withDetails({
-              kind: 'error',
-              loc: effect.place.loc,
-              message: 'Cannot access impure value during render',
-            })
-            .withDetails({
-              kind: 'error',
-              loc: impureEffect.into.loc,
-              message: impureEffect.reason,
-            }),
-        );
+        const diagnostic = CompilerDiagnostic.create({
+          category: impureEffect.category,
+          reason: impureEffect.reason,
+          description: impureEffect.description,
+        }).withDetails({
+          kind: 'error',
+          loc: effect.place.loc,
+          message: impureEffect.usageMessage,
+        });
+        if (!areEqualSourceLocations(effect.place.loc, impureEffect.into.loc)) {
+          diagnostic.withDetails({
+            kind: 'error',
+            loc: impureEffect.into.loc,
+            message: impureEffect.sourceMessage,
+          });
+        }
+        error.pushDiagnostic(diagnostic);
       }
     }
   }
@@ -274,8 +279,11 @@ function inferImpureValues(
       impureEffects.push({
         kind: 'Impure',
         into: {...place},
+        category: impureEffect.category,
         reason: impureEffect.reason,
         description: impureEffect.description,
+        sourceMessage: impureEffect.sourceMessage,
+        usageMessage: impureEffect.usageMessage,
       });
     }
   }