[compiler] Improve ref validation error message

Improves the error message for ValidateNoRefAccessInRender, using the new diagnostic type as well as providing a longer but succinct summary of what refs are for and why they're unsafe to access in render.

Author: josephsavona

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

@@ -5,7 +5,11 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-import {CompilerError, ErrorSeverity} from '../CompilerError';
+import {
+  CompilerDiagnostic,
+  CompilerError,
+  ErrorSeverity,
+} from '../CompilerError';
 import {
   BlockId,
   HIRFunction,
@@ -385,28 +389,40 @@ function validateNoRefAccessInRenderImpl(
             const hookKind = getHookKindForType(fn.env, callee.identifier.type);
             let returnType: RefAccessType = {kind: 'None'};
             const fnType = env.get(callee.identifier.id);
+            let didError = false;
             if (fnType?.kind === 'Structure' && fnType.fn !== null) {
               returnType = fnType.fn.returnType;
               if (fnType.fn.readRefEffect) {
-                errors.push({
-                  severity: ErrorSeverity.InvalidReact,
-                  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:
-                    callee.identifier.name !== null &&
-                    callee.identifier.name.kind === 'named'
-                      ? `Function \`${callee.identifier.name.value}\` accesses a ref`
-                      : null,
-                  suggestions: null,
-                });
+                didError = true;
+                errors.pushDiagnostic(
+                  CompilerDiagnostic.create({
+                    severity: ErrorSeverity.InvalidReact,
+                    category: 'Cannot access refs during render',
+                    description: ERROR_DESCRIPTION,
+                  }).withDetail({
+                    kind: 'error',
+                    loc: callee.loc,
+                    message: `This function accesses a ref value`,
+                  }),
+                );
               }
             }
-            for (const operand of eachInstructionValueOperand(instr.value)) {
-              if (hookKind != null) {
-                validateNoDirectRefValueAccess(errors, operand, env);
-              } else {
-                validateNoRefAccess(errors, env, operand, operand.loc);
+            if (!didError) {
+              /*
+               * If we already reported an error on this instruction, don't report
+               * duplicate errors
+               */
+              for (const operand of eachInstructionValueOperand(instr.value)) {
+                if (hookKind != null) {
+                  validateNoDirectRefValueAccess(errors, operand, env);
+                } else {
+                  validateNoRefPassedToFunction(
+                    errors,
+                    env,
+                    operand,
+                    operand.loc,
+                  );
+                }
               }
             }
             env.set(instr.lvalue.identifier.id, returnType);
@@ -449,7 +465,7 @@ function validateNoRefAccessInRenderImpl(
             ) {
               safeBlocks.delete(block.id);
             } else {
-              validateNoRefAccess(errors, env, instr.value.object, instr.loc);
+              validateNoRefUpdate(errors, env, instr.value.object, instr.loc);
             }
             for (const operand of eachInstructionValueOperand(instr.value)) {
               if (operand === instr.value.object) {
@@ -583,18 +599,17 @@ function destructure(
 
 function guardCheck(errors: CompilerError, operand: Place, env: Env): void {
   if (env.get(operand.identifier.id)?.kind === 'Guard') {
-    errors.push({
-      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:
-        operand.identifier.name !== null &&
-        operand.identifier.name.kind === 'named'
-          ? `Cannot access ref value \`${operand.identifier.name.value}\``
-          : null,
-      suggestions: null,
-    });
+    errors.pushDiagnostic(
+      CompilerDiagnostic.create({
+        severity: ErrorSeverity.InvalidReact,
+        category: 'Cannot access refs during render',
+        description: ERROR_DESCRIPTION,
+      }).withDetail({
+        kind: 'error',
+        loc: operand.loc,
+        message: `Cannot access ref value during render`,
+      }),
+    );
   }
 }
 
@@ -608,22 +623,47 @@ function validateNoRefValueAccess(
     type?.kind === 'RefValue' ||
     (type?.kind === 'Structure' && type.fn?.readRefEffect)
   ) {
-    errors.push({
-      severity: ErrorSeverity.InvalidReact,
-      reason:
-        'Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)',
-      loc: (type.kind === 'RefValue' && type.loc) || operand.loc,
-      description:
-        operand.identifier.name !== null &&
-        operand.identifier.name.kind === 'named'
-          ? `Cannot access ref value \`${operand.identifier.name.value}\``
-          : null,
-      suggestions: null,
-    });
+    errors.pushDiagnostic(
+      CompilerDiagnostic.create({
+        severity: ErrorSeverity.InvalidReact,
+        category: 'Cannot access refs during render',
+        description: ERROR_DESCRIPTION,
+      }).withDetail({
+        kind: 'error',
+        loc: (type.kind === 'RefValue' && type.loc) || operand.loc,
+        message: `Cannot access ref value during render`,
+      }),
+    );
+  }
+}
+
+function validateNoRefPassedToFunction(
+  errors: CompilerError,
+  env: Env,
+  operand: Place,
+  loc: SourceLocation,
+): void {
+  const type = destructure(env.get(operand.identifier.id));
+  if (
+    type?.kind === 'Ref' ||
+    type?.kind === 'RefValue' ||
+    (type?.kind === 'Structure' && type.fn?.readRefEffect)
+  ) {
+    errors.pushDiagnostic(
+      CompilerDiagnostic.create({
+        severity: ErrorSeverity.InvalidReact,
+        category: 'Cannot access refs during render',
+        description: ERROR_DESCRIPTION,
+      }).withDetail({
+        kind: 'error',
+        loc: (type.kind === 'RefValue' && type.loc) || loc,
+        message: `Passing a ref to a function may read its value during render`,
+      }),
+    );
   }
 }
 
-function validateNoRefAccess(
+function validateNoRefUpdate(
   errors: CompilerError,
   env: Env,
   operand: Place,
@@ -635,18 +675,17 @@ function validateNoRefAccess(
     type?.kind === 'RefValue' ||
     (type?.kind === 'Structure' && type.fn?.readRefEffect)
   ) {
-    errors.push({
-      severity: ErrorSeverity.InvalidReact,
-      reason:
-        'Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)',
-      loc: (type.kind === 'RefValue' && type.loc) || loc,
-      description:
-        operand.identifier.name !== null &&
-        operand.identifier.name.kind === 'named'
-          ? `Cannot access ref value \`${operand.identifier.name.value}\``
-          : null,
-      suggestions: null,
-    });
+    errors.pushDiagnostic(
+      CompilerDiagnostic.create({
+        severity: ErrorSeverity.InvalidReact,
+        category: 'Cannot access refs during render',
+        description: ERROR_DESCRIPTION,
+      }).withDetail({
+        kind: 'error',
+        loc: (type.kind === 'RefValue' && type.loc) || loc,
+        message: `Cannot update ref during render`,
+      }),
+    );
   }
 }
 
@@ -657,17 +696,22 @@ function validateNoDirectRefValueAccess(
 ): void {
   const type = destructure(env.get(operand.identifier.id));
   if (type?.kind === 'RefValue') {
-    errors.push({
-      severity: ErrorSeverity.InvalidReact,
-      reason:
-        'Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)',
-      loc: type.loc ?? operand.loc,
-      description:
-        operand.identifier.name !== null &&
-        operand.identifier.name.kind === 'named'
-          ? `Cannot access ref value \`${operand.identifier.name.value}\``
-          : null,
-      suggestions: null,
-    });
+    errors.pushDiagnostic(
+      CompilerDiagnostic.create({
+        severity: ErrorSeverity.InvalidReact,
+        category: 'Cannot access refs during render',
+        description: ERROR_DESCRIPTION,
+      }).withDetail({
+        kind: 'error',
+        loc: type.loc ?? operand.loc,
+        message: `Cannot access ref value during render`,
+      }),
+    );
   }
 }
+
+const ERROR_DESCRIPTION =
+  '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)';

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.capture-ref-for-mutation.expect.md

@@ -32,48 +32,30 @@ export const FIXTURE_ENTRYPOINT = {
 ## Error
 
 ```
-Found 4 errors:
+Found 2 errors:
 
-Error: This function accesses a ref value (the `current` property), which may not be accessed during render. (https://react.dev/reference/react/useRef)
+Error: Cannot access refs during render
 
-error.capture-ref-for-mutation.ts:12:13
-  10 |   };
-  11 |   const moveLeft = {
-> 12 |     handler: handleKey('left')(),
-     |              ^^^^^^^^^^^^^^^^^ This function accesses a ref value (the `current` property), which may not be accessed during render. (https://react.dev/reference/react/useRef)
-  13 |   };
-  14 |   const moveRight = {
-  15 |     handler: handleKey('right')(),
-
-Error: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)
+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.capture-ref-for-mutation.ts:12:13
   10 |   };
   11 |   const moveLeft = {
 > 12 |     handler: handleKey('left')(),
-     |              ^^^^^^^^^^^^^^^^^ Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)
+     |              ^^^^^^^^^^^^^^^^^ This function accesses a ref value
   13 |   };
   14 |   const moveRight = {
   15 |     handler: handleKey('right')(),
 
-Error: This function accesses a ref value (the `current` property), which may not be accessed during render. (https://react.dev/reference/react/useRef)
-
-error.capture-ref-for-mutation.ts:15:13
-  13 |   };
-  14 |   const moveRight = {
-> 15 |     handler: handleKey('right')(),
-     |              ^^^^^^^^^^^^^^^^^^ This function accesses a ref value (the `current` property), which may not be accessed during render. (https://react.dev/reference/react/useRef)
-  16 |   };
-  17 |   return [moveLeft, moveRight];
-  18 | }
+Error: Cannot access refs during render
 
-Error: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)
+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.capture-ref-for-mutation.ts:15:13
   13 |   };
   14 |   const moveRight = {
 > 15 |     handler: handleKey('right')(),
-     |              ^^^^^^^^^^^^^^^^^^ Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)
+     |              ^^^^^^^^^^^^^^^^^^ This function accesses a ref value
   16 |   };
   17 |   return [moveLeft, moveRight];
   18 | }

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

@@ -22,24 +22,28 @@ export const FIXTURE_ENTRYPOINT = {
 ```
 Found 2 errors:
 
-Error: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)
+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.hook-ref-value.ts:5:23
   3 | function Component(props) {
   4 |   const ref = useRef();
 > 5 |   useEffect(() => {}, [ref.current]);
-    |                        ^^^^^^^^^^^ Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)
+    |                        ^^^^^^^^^^^ Cannot access ref value during render
   6 | }
   7 |
   8 | export const FIXTURE_ENTRYPOINT = {
 
-Error: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)
+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.hook-ref-value.ts:5:23
   3 | function Component(props) {
   4 |   const ref = useRef();
 > 5 |   useEffect(() => {}, [ref.current]);
-    |                        ^^^^^^^^^^^ Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)
+    |                        ^^^^^^^^^^^ Cannot access ref value during render
   6 | }
   7 |
   8 | export const FIXTURE_ENTRYPOINT = {

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

@@ -17,13 +17,15 @@ function Component(props) {
 ```
 Found 1 error:
 
-Error: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)
+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.invalid-access-ref-during-render.ts:4:16
   2 | function Component(props) {
   3 |   const ref = useRef(null);
 > 4 |   const value = ref.current;
-    |                 ^^^^^^^^^^^ Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)
+    |                 ^^^^^^^^^^^ Cannot access ref value during render
   5 |   return value;
   6 | }
   7 |

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-aliased-ref-in-callback-invoked-during-render-.expect.md

@@ -21,13 +21,15 @@ function Component(props) {
 ```
 Found 1 error:
 
-Error: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)
+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.invalid-aliased-ref-in-callback-invoked-during-render-.ts:9:33
    7 |     return <Foo item={item} current={current} />;
    8 |   };
 >  9 |   return <Items>{props.items.map(item => renderItem(item))}</Items>;
-     |                                  ^^^^^^^^^^^^^^^^^^^^^^^^ Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)
+     |                                  ^^^^^^^^^^^^^^^^^^^^^^^^ Passing a ref to a function may read its value during render
   10 | }
   11 |
 ```

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

@@ -20,12 +20,14 @@ component Example() {
 ```
 Found 1 error:
 
-Error: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)
+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)
 
   4 | component Example() {
   5 |   const fooRef = makeObject_Primitives();
 > 6 |   fooRef.current = true;
-    |   ^^^^^^^^^^^^^^ Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)
+    |   ^^^^^^^^^^^^^^ Cannot update ref during render
   7 |
   8 |   return <Stringify foo={fooRef} />;
   9 | }

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

@@ -18,13 +18,15 @@ function Component() {
 ```
 Found 1 error:
 
-Error: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)
+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.invalid-disallow-mutating-ref-in-render.ts:4:2
   2 | function Component() {
   3 |   const ref = useRef(null);
 > 4 |   ref.current = false;
-    |   ^^^^^^^^^^^ Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)
+    |   ^^^^^^^^^^^ Cannot update ref during render
   5 |
   6 |   return <button ref={ref} />;
   7 | }