Improve remaining InvalidReact error messages

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

@@ -2551,9 +2551,9 @@ function lowerIdentifierForAssignment(
     if (kind === InstructionKind.Reassign) {
       // Trying to reassign a global is not allowed
       builder.errors.push({
-        reason: `(BuildHIR::lowerAssignment) Assigning to an identifier defined outside the function scope is not supported.`,
+        reason: `This reassigns a variable which was not defined inside of the component. Components should be pure and side-effect free. If this variable is used in rendering, use useState instead. (https://react.dev/learn/keeping-components-pure)`,
         severity: ErrorSeverity.InvalidReact,
-        loc: path.node.loc ?? null,
+        loc: path.parentPath.node.loc ?? null,
         suggestions: null,
       });
     } else {

compiler/forget/packages/babel-plugin-react-forget/src/Inference/InferReferenceEffects.ts

@@ -25,11 +25,9 @@ import {
 } from "../HIR/HIR";
 import { FunctionSignature } from "../HIR/ObjectShape";
 import {
-  printIdentifier,
   printMixedHIR,
   printPlace,
   printSourceLocation,
-  printType,
 } from "../HIR/PrintHIR";
 import {
   eachInstructionOperand,
@@ -324,10 +322,11 @@ class InferenceState {
           effect = Effect.Mutate;
         } else {
           CompilerError.invalidReact({
-            reason: `InferReferenceEffects: inferred mutation of known immutable value`,
-            description: `Found mutation of ${printIdentifier(
-              place.identifier
-            )}${printType(place.identifier.type)} (${valueKind})`,
+            reason: `This mutates a variable after it was passed to React, which means that React cannot observe changes to it`,
+            description:
+              place.identifier.name !== null
+                ? `Found mutation of ${place.identifier.name}`
+                : null,
             loc: place.loc,
             suggestions: null,
           });
@@ -340,10 +339,11 @@ class InferenceState {
           valueKind !== ValueKind.Context
         ) {
           CompilerError.invalidReact({
-            reason: `InferReferenceEffects: inferred mutation of known immutable value`,
-            description: `Found mutation of ${printIdentifier(
-              place.identifier
-            )}${printType(place.identifier.type)} (${valueKind})`,
+            reason: `This mutates a variable after it was passed to React, which means that React cannot observe changes to it`,
+            description:
+              place.identifier.name !== null
+                ? `Found mutation of ${place.identifier.name}`
+                : null,
             loc: place.loc,
             suggestions: null,
           });

compiler/forget/packages/babel-plugin-react-forget/src/Validation/ValidateFrozenLambdas.ts

@@ -132,11 +132,16 @@ function validateOperand(
       state.temporaries.get(operand.identifier.id) ?? operand.identifier.id;
     const lambda = state.lambdas.get(operandId);
     if (lambda !== undefined) {
+      // TODO: these seem to always be null, we should try to preserve original names from source
+      const description =
+        lambda.name !== null && operand.identifier.name !== null
+          ? `\`${lambda.name}\` is a function that may mutate \`${operand.identifier.name}\`. If you must mutate \`${operand.identifier.name}\` try using a React API like useState and use its setter function instead`
+          : null;
       return new CompilerErrorDetail({
-        description: null,
+        description,
         loc: typeof operand.loc !== "symbol" ? operand.loc : null,
         reason:
-          "Cannot use a mutable function where an immutable value is expected",
+          "This mutates a variable that is managed by React, where an immutable value or a function was expected",
         severity: ErrorSeverity.InvalidReact,
         suggestions: null,
       });

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

@@ -107,7 +107,7 @@ function validateNonRefValue(error: CompilerError, operand: Place): void {
         description: `Cannot access ref value at ${printPlace(operand)}`,
         loc: typeof operand.loc !== "symbol" ? operand.loc : null,
         reason:
-          "Ref values (the `current` property) may not be accessed during render",
+          "Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)",
         severity: ErrorSeverity.InvalidReact,
         suggestions: null,
       })
@@ -123,7 +123,7 @@ function validateNonRefObject(error: CompilerError, operand: Place): void {
         description: `Cannot access ref object at ${printPlace(operand)}`,
         loc: typeof operand.loc !== "symbol" ? operand.loc : null,
         reason:
-          "Ref values may not be passed to functions because they could read the ref value (`current` property) during render",
+          "Ref values may not be passed to functions because they could read the ref value (`current` property) during render. (https://react.dev/reference/react/useRef)",
         severity: ErrorSeverity.InvalidReact,
         suggestions: null,
       })

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

@@ -15,7 +15,7 @@ function Component(props) {
 ## Error
 
 ```
-[ReactForget] InvalidReact: Ref values (the `current` property) may not be accessed during render. Cannot access ref value at freeze $22:TObject<BuiltInRefValue> (5:5)
+[ReactForget] 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)
 ```
           
       

compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-array-push-frozen.expect.md

@@ -15,7 +15,7 @@ function Component(props) {
 ## Error
 
 ```
-[ReactForget] InvalidReact: InferReferenceEffects: inferred mutation of known immutable value. Found mutation of $19:TObject<BuiltInArray> (frozen) (4:4)
+[ReactForget] InvalidReact: This mutates a variable after it was passed to React, which means that React cannot observe changes to it (4:4)
 ```
           
       

compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-capture-func-passed-to-jsx.expect.md

@@ -20,7 +20,7 @@ function component(a, b) {
 ## Error
 
 ```
-[ReactForget] InvalidReact: Cannot use a mutable function where an immutable value is expected (8:8)
+[ReactForget] InvalidReact: This mutates a variable that is managed by React, where an immutable value or a function was expected (8:8)
 ```
           
       

compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-computed-store-to-frozen-value.expect.md

@@ -16,7 +16,7 @@ function Component(props) {
 ## Error
 
 ```
-[ReactForget] InvalidReact: InferReferenceEffects: inferred mutation of known immutable value. Found mutation of $21 (frozen) (5:5)
+[ReactForget] InvalidReact: This mutates a variable after it was passed to React, which means that React cannot observe changes to it (5:5)
 ```
           
       

compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-delete-computed-property-of-frozen-value.expect.md

@@ -16,7 +16,7 @@ function Component(props) {
 ## Error
 
 ```
-[ReactForget] InvalidReact: InferReferenceEffects: inferred mutation of known immutable value. Found mutation of $19 (frozen) (5:5)
+[ReactForget] InvalidReact: This mutates a variable after it was passed to React, which means that React cannot observe changes to it (5:5)
 ```
           
       

compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-delete-property-of-frozen-value.expect.md

@@ -16,7 +16,7 @@ function Component(props) {
 ## Error
 
 ```
-[ReactForget] InvalidReact: InferReferenceEffects: inferred mutation of known immutable value. Found mutation of $18 (frozen) (5:5)
+[ReactForget] InvalidReact: This mutates a variable after it was passed to React, which means that React cannot observe changes to it (5:5)
 ```
           
       

compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-destructure-assignment-to-global.expect.md

@@ -13,7 +13,7 @@ function useFoo(props) {
 ## Error
 
 ```
-[ReactForget] InvalidReact: (BuildHIR::lowerAssignment) Assigning to an identifier defined outside the function scope is not supported. (2:2)
+[ReactForget] InvalidReact: This reassigns a variable which was not defined inside of the component. Components should be pure and side-effect free. If this variable is used in rendering, use useState instead. (https://react.dev/learn/keeping-components-pure) (2:2)
 ```
           
       

compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-destructure-to-local-global-variables.expect.md

@@ -15,7 +15,7 @@ function Component(props) {
 ## Error
 
 ```
-[ReactForget] InvalidReact: (BuildHIR::lowerAssignment) Assigning to an identifier defined outside the function scope is not supported. (3:3)
+[ReactForget] InvalidReact: This reassigns a variable which was not defined inside of the component. Components should be pure and side-effect free. If this variable is used in rendering, use useState instead. (https://react.dev/learn/keeping-components-pure) (3:3)
 ```
           
       

compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-freeze-conditionally-mutable-lambda.expect.md

@@ -25,7 +25,7 @@ function Component(props) {
 ## Error
 
 ```
-[ReactForget] InvalidReact: Cannot use a mutable function where an immutable value is expected (15:15)
+[ReactForget] InvalidReact: This mutates a variable that is managed by React, where an immutable value or a function was expected (15:15)
 ```
           
       

compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-freeze-mutable-lambda-mutate-local.expect.md

@@ -18,7 +18,7 @@ function Component(props) {
 ## Error
 
 ```
-[ReactForget] InvalidReact: Cannot use a mutable function where an immutable value is expected (8:8)
+[ReactForget] InvalidReact: This mutates a variable that is managed by React, where an immutable value or a function was expected (8:8)
 ```
           
       

compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-freeze-mutable-lambda-reassign-local.expect.md

@@ -16,7 +16,7 @@ function Component(props) {
 ## Error
 
 ```
-[ReactForget] InvalidReact: Cannot use a mutable function where an immutable value is expected (6:6)
+[ReactForget] InvalidReact: This mutates a variable that is managed by React, where an immutable value or a function was expected (6:6)
 ```
           
       

compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-function-expression-mutates-immutable-value.expect.md

@@ -18,7 +18,7 @@ function Component(props) {
 ## Error
 
 ```
-[ReactForget] InvalidReact: InferReferenceEffects: inferred mutation of known immutable value. Found mutation of $39 (frozen) (5:5)
+[ReactForget] InvalidReact: This mutates a variable after it was passed to React, which means that React cannot observe changes to it (5:5)
 ```
           
       

compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-mutate-after-aliased-freeze.expect.md

@@ -25,7 +25,7 @@ function Component(props) {
 ## Error
 
 ```
-[ReactForget] InvalidReact: InferReferenceEffects: inferred mutation of known immutable value. Found mutation of $44:TObject<BuiltInArray> (frozen) (13:13)
+[ReactForget] InvalidReact: This mutates a variable after it was passed to React, which means that React cannot observe changes to it (13:13)
 ```
           
       

compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-mutate-after-freeze.expect.md

@@ -19,7 +19,7 @@ function Component(props) {
 ## Error
 
 ```
-[ReactForget] InvalidReact: InferReferenceEffects: inferred mutation of known immutable value. Found mutation of $26:TObject<BuiltInArray> (frozen) (7:7)
+[ReactForget] InvalidReact: This mutates a variable after it was passed to React, which means that React cannot observe changes to it (7:7)
 ```
           
       

compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-pass-ref-to-function.expect.md

@@ -14,7 +14,7 @@ function Component(props) {
 ## Error
 
 ```
-[ReactForget] InvalidReact: Ref values may not be passed to functions because they could read the ref value (`current` property) during render. Cannot access ref object at mutate? $21[6:8]:TObject<BuiltInUseRefId> (3:3)
+[ReactForget] InvalidReact: Ref values may not be passed to functions because they could read the ref value (`current` property) during render. (https://react.dev/reference/react/useRef). Cannot access ref object at mutate? $21[6:8]:TObject<BuiltInUseRefId> (3:3)
 ```
           
       

compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-property-store-to-frozen-value.expect.md

@@ -16,7 +16,7 @@ function Component(props) {
 ## Error
 
 ```
-[ReactForget] InvalidReact: InferReferenceEffects: inferred mutation of known immutable value. Found mutation of $20 (frozen) (5:5)
+[ReactForget] InvalidReact: This mutates a variable after it was passed to React, which means that React cannot observe changes to it (5:5)
 ```
           
       

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

@@ -18,7 +18,7 @@ function Component(props) {
 ## Error
 
 ```
-[ReactForget] InvalidReact: Ref values (the `current` property) may not be accessed during render. Cannot access ref value at capture $42[6:16]:TObject<BuiltInRefValue> (5:5)
+[ReactForget] InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value at capture $42[6:16]:TObject<BuiltInRefValue> (5:5)
 ```
           
       

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

@@ -13,7 +13,7 @@ function Component(props) {
 ## Error
 
 ```
-[ReactForget] InvalidReact: Ref values (the `current` property) may not be accessed during render. Cannot access ref value at freeze $19:TObject<BuiltInRefValue> (3:3)
+[ReactForget] 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> (3:3)
 ```
           
       

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

@@ -14,9 +14,9 @@ function Component(props) {
 ## Error
 
 ```
-[ReactForget] InvalidReact: Ref values may not be passed to functions because they could read the ref value (`current` property) during render. Cannot access ref object at store $21[7:9]:TObject<BuiltInUseRefId> (3:3)
+[ReactForget] InvalidReact: Ref values may not be passed to functions because they could read the ref value (`current` property) during render. (https://react.dev/reference/react/useRef). Cannot access ref object at store $21[7:9]:TObject<BuiltInUseRefId> (3:3)
 
-[ReactForget] InvalidReact: Ref values (the `current` property) may not be accessed during render. Cannot access ref value at freeze $24:TObject<BuiltInRefValue> (4:4)
+[ReactForget] 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> (4:4)
 ```
           
       

compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-use-ref-added-to-dep-without-type-info.expect.md

@@ -21,7 +21,7 @@ function Foo({ a }) {
 ## Error
 
 ```
-[ReactForget] InvalidReact: Ref values may not be passed to functions because they could read the ref value (`current` property) during render. Cannot access ref object at capture $29:TObject<BuiltInUseRefId> (4:4)
+[ReactForget] InvalidReact: Ref values may not be passed to functions because they could read the ref value (`current` property) during render. (https://react.dev/reference/react/useRef). Cannot access ref object at capture $29:TObject<BuiltInUseRefId> (4:4)
 ```
           
       

compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.reassignment-to-global.expect.md

@@ -0,0 +1,22 @@
+
+## Input
+
+```javascript
+function Component() {
+  // Cannot assign to globals
+  someUnknownGlobal = true;
+  moduleLocal = true;
+}
+
+```
+
+
+## Error
+
+```
+[ReactForget] InvalidReact: This reassigns a variable which was not defined inside of the component. Components should be pure and side-effect free. If this variable is used in rendering, use useState instead. (https://react.dev/learn/keeping-components-pure) (3:3)
+
+[ReactForget] InvalidReact: This reassigns a variable which was not defined inside of the component. Components should be pure and side-effect free. If this variable is used in rendering, use useState instead. (https://react.dev/learn/keeping-components-pure) (4:4)
+```
+          
+      

compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.reassignment-to-global.js

@@ -0,0 +1,5 @@
+function Component() {
+  // Cannot assign to globals
+  someUnknownGlobal = true;
+  moduleLocal = true;
+}

compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-kitchensink.expect.md

@@ -64,10 +64,6 @@ function foo([a, b], { c, d, e = "e" }, f = "f", ...args) {
     }
   }
 
-  // Cannot assign to globals
-  someUnknownGlobal = true;
-  moduleLocal = true;
-
   function component(a) {
     // Add support for function declarations once we support `var` hoisting.
     function t() {}
@@ -126,10 +122,6 @@ let moduleLocal = false;
 [ReactForget] Todo: (BuildHIR::node.lowerReorderableExpression) Expression type 'CallExpression' cannot be safely reordered (55:55)
 
 [ReactForget] Todo: (BuildHIR::node.lowerReorderableExpression) Expression type 'BinaryExpression' cannot be safely reordered (53:53)
-
-[ReactForget] InvalidReact: (BuildHIR::lowerAssignment) Assigning to an identifier defined outside the function scope is not supported. (64:64)
-
-[ReactForget] InvalidReact: (BuildHIR::lowerAssignment) Assigning to an identifier defined outside the function scope is not supported. (65:65)
 ```
           
       

compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-kitchensink.js

@@ -60,10 +60,6 @@ function foo([a, b], { c, d, e = "e" }, f = "f", ...args) {
     }
   }
 
-  // Cannot assign to globals
-  someUnknownGlobal = true;
-  moduleLocal = true;
-
   function component(a) {
     // Add support for function declarations once we support `var` hoisting.
     function t() {}

compiler/forget/packages/eslint-plugin-react-forget/__tests__/ReactForgetDiagnostics-test.ts

@@ -72,7 +72,7 @@ const tests: ForgetTestCases = {
       errors: [
         {
           message:
-            "[ReactForget] InvalidReact: Ref values (the `current` property) may not be accessed during render. Cannot access ref value at freeze $22:TObject<BuiltInRefValue> (6:6)",
+            "[ReactForget] 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> (6:6)",
           line: 6,
           column: 10,
           endColumn: 15,