[compiler] Reposition ref-in-render errors to the read location of .current

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

@@ -15,7 +15,6 @@ import {
   isRefValueType,
   isUseRefType,
 } from '../HIR';
-import {printPlace} from '../HIR/PrintHIR';
 import {
   eachInstructionValueOperand,
   eachTerminalOperand,
@@ -53,6 +52,7 @@ function validateNoRefAccessInRenderImpl(
   refAccessingFunctions: Set<IdentifierId>,
 ): Result<void, CompilerError> {
   const errors = new CompilerError();
+  const lookupLocations: Map<IdentifierId, SourceLocation> = new Map();
   for (const [, block] of fn.body.blocks) {
     for (const instr of block.instructions) {
       switch (instr.value.kind) {
@@ -64,30 +64,51 @@ function validateNoRefAccessInRenderImpl(
                 severity: ErrorSeverity.InvalidReact,
                 reason:
                   'Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)',
-                loc: operand.loc,
-                description: `Cannot access ref value at ${printPlace(
-                  operand,
-                )}`,
+                loc: lookupLocations.get(operand.identifier.id) ?? operand.loc,
+                description:
+                  operand.identifier.name !== null &&
+                  operand.identifier.name.kind === 'named'
+                    ? `Cannot access ref value \`${operand.identifier.name.value}\``
+                    : null,
                 suggestions: null,
               });
             }
           }
           break;
         }
         case 'PropertyLoad': {
+          if (
+            isRefValueType(instr.lvalue.identifier) &&
+            instr.value.property === 'current'
+          ) {
+            lookupLocations.set(instr.lvalue.identifier.id, instr.loc);
+          }
           break;
         }
         case 'LoadLocal': {
           if (refAccessingFunctions.has(instr.value.place.identifier.id)) {
             refAccessingFunctions.add(instr.lvalue.identifier.id);
           }
+          if (isRefValueType(instr.lvalue.identifier)) {
+            const loc = lookupLocations.get(instr.value.place.identifier.id);
+            if (loc !== undefined) {
+              lookupLocations.set(instr.lvalue.identifier.id, loc);
+            }
+          }
           break;
         }
         case 'StoreLocal': {
           if (refAccessingFunctions.has(instr.value.value.identifier.id)) {
             refAccessingFunctions.add(instr.value.lvalue.place.identifier.id);
             refAccessingFunctions.add(instr.lvalue.identifier.id);
           }
+          if (isRefValueType(instr.value.lvalue.place.identifier)) {
+            const loc = lookupLocations.get(instr.value.value.identifier.id);
+            if (loc !== undefined) {
+              lookupLocations.set(instr.value.lvalue.place.identifier.id, loc);
+              lookupLocations.set(instr.lvalue.identifier.id, loc);
+            }
+          }
           break;
         }
         case 'ObjectMethod':
@@ -140,7 +161,11 @@ function validateNoRefAccessInRenderImpl(
                 reason:
                   'This function accesses a ref value (the `current` property), which may not be accessed during render. (https://react.dev/reference/react/useRef)',
                 loc: callee.loc,
-                description: `Function ${printPlace(callee)} accesses a ref`,
+                description:
+                  callee.identifier.name !== null &&
+                  callee.identifier.name.kind === 'named'
+                    ? `Function \`${callee.identifier.name.value}\` accesses a ref`
+                    : null,
                 suggestions: null,
               });
             }
@@ -149,7 +174,7 @@ function validateNoRefAccessInRenderImpl(
                 errors,
                 refAccessingFunctions,
                 operand,
-                operand.loc,
+                lookupLocations.get(operand.identifier.id) ?? operand.loc,
               );
             }
           }
@@ -162,7 +187,7 @@ function validateNoRefAccessInRenderImpl(
               errors,
               refAccessingFunctions,
               operand,
-              operand.loc,
+              lookupLocations.get(operand.identifier.id) ?? operand.loc,
             );
           }
           break;
@@ -175,13 +200,18 @@ function validateNoRefAccessInRenderImpl(
             errors,
             refAccessingFunctions,
             instr.value.object,
-            instr.loc,
+            lookupLocations.get(instr.value.object.identifier.id) ?? instr.loc,
           );
           for (const operand of eachInstructionValueOperand(instr.value)) {
             if (operand === instr.value.object) {
               continue;
             }
-            validateNoRefValueAccess(errors, refAccessingFunctions, operand);
+            validateNoRefValueAccess(
+              errors,
+              refAccessingFunctions,
+              lookupLocations,
+              operand,
+            );
           }
           break;
         }
@@ -190,14 +220,24 @@ function validateNoRefAccessInRenderImpl(
           break;
         default: {
           for (const operand of eachInstructionValueOperand(instr.value)) {
-            validateNoRefValueAccess(errors, refAccessingFunctions, operand);
+            validateNoRefValueAccess(
+              errors,
+              refAccessingFunctions,
+              lookupLocations,
+              operand,
+            );
           }
           break;
         }
       }
     }
     for (const operand of eachTerminalOperand(block.terminal)) {
-      validateNoRefValueAccess(errors, refAccessingFunctions, operand);
+      validateNoRefValueAccess(
+        errors,
+        refAccessingFunctions,
+        lookupLocations,
+        operand,
+      );
     }
   }
 
@@ -211,6 +251,7 @@ function validateNoRefAccessInRenderImpl(
 function validateNoRefValueAccess(
   errors: CompilerError,
   refAccessingFunctions: Set<IdentifierId>,
+  lookupLocations: Map<IdentifierId, SourceLocation>,
   operand: Place,
 ): void {
   if (
@@ -221,8 +262,12 @@ function validateNoRefValueAccess(
       severity: ErrorSeverity.InvalidReact,
       reason:
         'Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)',
-      loc: operand.loc,
-      description: `Cannot access ref value at ${printPlace(operand)}`,
+      loc: lookupLocations.get(operand.identifier.id) ?? operand.loc,
+      description:
+        operand.identifier.name !== null &&
+        operand.identifier.name.kind === 'named'
+          ? `Cannot access ref value \`${operand.identifier.name.value}\``
+          : null,
       suggestions: null,
     });
   }

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-access-ref-during-render.expect.md

@@ -15,10 +15,11 @@ function Component(props) {
 ## Error
 
 ```
+  2 | function Component(props) {
   3 |   const ref = useRef(null);
-  4 |   const value = ref.current;
-> 5 |   return value;
-    |          ^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value at freeze $22:TObject<BuiltInRefValue> (5:5)
+> 4 |   const value = ref.current;
+    |                 ^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (4:4)
+  5 |   return value;
   6 | }
   7 |
 ```

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-disallow-mutating-refs-in-render-transitive.expect.md

@@ -24,7 +24,7 @@ function Component() {
    7 |   };
    8 |   const changeRef = setRef;
 >  9 |   changeRef();
-     |   ^^^^^^^^^ InvalidReact: This function accesses a ref value (the `current` property), which may not be accessed during render. (https://react.dev/reference/react/useRef). Function mutate? $39[11:13]:TObject<BuiltInFunction> accesses a ref (9:9)
+     |   ^^^^^^^^^ InvalidReact: This function accesses a ref value (the `current` property), which may not be accessed during render. (https://react.dev/reference/react/useRef) (9:9)
 
 InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (9:9)
   10 |

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-read-ref-prop-in-render-destructure.expect.md

@@ -14,10 +14,11 @@ function Component({ref}) {
 ## Error
 
 ```
+  1 | // @validateRefAccessDuringRender @compilationMode(infer)
   2 | function Component({ref}) {
-  3 |   const value = ref.current;
-> 4 |   return <div>{value}</div>;
-    |                ^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value at read $17:TObject<BuiltInRefValue> (4:4)
+> 3 |   const value = ref.current;
+    |                 ^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (3:3)
+  4 |   return <div>{value}</div>;
   5 | }
   6 |
 ```

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-read-ref-prop-in-render-property-load.expect.md

@@ -14,10 +14,11 @@ function Component(props) {
 ## Error
 
 ```
+  1 | // @validateRefAccessDuringRender @compilationMode(infer)
   2 | function Component(props) {
-  3 |   const value = props.ref.current;
-> 4 |   return <div>{value}</div>;
-    |                ^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value at read $15:TObject<BuiltInRefValue> (4:4)
+> 3 |   const value = props.ref.current;
+    |                 ^^^^^^^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (3:3)
+  4 |   return <div>{value}</div>;
   5 | }
   6 |
 ```

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-ref-value-as-props.expect.md

@@ -17,7 +17,7 @@ function Component(props) {
   2 | function Component(props) {
   3 |   const ref = useRef(null);
 > 4 |   return <Foo ref={ref.current} />;
-    |                    ^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value at freeze $19:TObject<BuiltInRefValue> (4:4)
+    |                    ^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (4:4)
   5 | }
   6 |
 ```

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-set-and-read-ref-during-render.expect.md

@@ -20,7 +20,7 @@ function Component(props) {
 > 4 |   ref.current = props.value;
     |   ^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (4:4)
 
-InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value at freeze $24:TObject<BuiltInRefValue> (5:5)
+InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (5:5)
   5 |   return ref.current;
   6 | }
   7 |

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-set-and-read-ref-nested-property-during-render.expect.md

@@ -18,9 +18,9 @@ function Component(props) {
   2 | function Component(props) {
   3 |   const ref = useRef({inner: null});
 > 4 |   ref.current.inner = props.value;
-    |   ^^^^^^^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (4:4)
+    |   ^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (4:4)
 
-InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value at freeze $30:TObject<BuiltInRefValue> (5:5)
+InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (5:5)
   5 |   return ref.current.inner;
   6 | }
   7 |

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.return-ref-callback.expect.md

@@ -28,7 +28,7 @@ export const FIXTURE_ENTRYPOINT = {
    8 |   };
    9 |
 > 10 |   return s;
-     |          ^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value at freeze $25:TObject<BuiltInFunction> (10:10)
+     |          ^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (10:10)
   11 | }
   12 |
   13 | export const FIXTURE_ENTRYPOINT = {

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-useCallback-set-ref-nested-property-ref-modified-later-preserve-memoization.expect.md

@@ -34,7 +34,7 @@ export const FIXTURE_ENTRYPOINT = {
   12 |
   13 |   // The ref is modified later, extending its range and preventing memoization of onChange
 > 14 |   ref.current.inner = null;
-     |   ^^^^^^^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (14:14)
+     |   ^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (14:14)
   15 |
   16 |   return <input onChange={onChange} />;
   17 | }

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useCallback-accesses-ref-mutated-later-via-function-preserve-memoization.expect.md

@@ -37,7 +37,7 @@ export const FIXTURE_ENTRYPOINT = {
   15 |     ref.current.inner = null;
   16 |   };
 > 17 |   reset();
-     |   ^^^^^ InvalidReact: This function accesses a ref value (the `current` property), which may not be accessed during render. (https://react.dev/reference/react/useRef). Function mutate? $77[20:22]:TObject<BuiltInFunction> accesses a ref (17:17)
+     |   ^^^^^ InvalidReact: This function accesses a ref value (the `current` property), which may not be accessed during render. (https://react.dev/reference/react/useRef) (17:17)
 
 InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (17:17)
   18 |