[compiler] update fixtures

The previous error message was generic, because the old style function
signature didn't support a way to specify a reason alongside a freeze
effect. This meant we could only say why a value was frozen for
instructions, but not hooks which use function signatures. By defining a
new aliasing signature for custom hooks we can specify a reason and
provide a better error message.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33513).
* #33571
* #33558
* #33547
* #33543
* #33533
* #33532
* #33530
* #33526
* #33522
* #33518
* #33514
* __->__ #33513

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

@@ -72,7 +72,7 @@ export function lower(
   env: Environment,
   // Bindings captured from the outer function, in case lower() is called recursively (for lambdas)
   bindings: Bindings | null = null,
-  capturedRefs: Array<t.Identifier> = [],
+  capturedRefs: Map<t.Identifier, SourceLocation> = new Map(),
 ): Result<HIRFunction, CompilerError> {
   const builder = new HIRBuilder(env, {
     bindings,
@@ -80,13 +80,13 @@ export function lower(
   });
   const context: HIRFunction['context'] = [];
 
-  for (const ref of capturedRefs ?? []) {
+  for (const [ref, loc] of capturedRefs ?? []) {
     context.push({
       kind: 'Identifier',
       identifier: builder.resolveBinding(ref),
       effect: Effect.Unknown,
       reactive: false,
-      loc: ref.loc ?? GeneratedSource,
+      loc,
     });
   }
 
@@ -3439,10 +3439,12 @@ function lowerFunction(
    * This isn't a problem in practice because use Babel's scope analysis to
    * identify the correct references.
    */
-  const lowering = lower(expr, builder.environment, builder.bindings, [
-    ...builder.context,
-    ...capturedContext,
-  ]);
+  const lowering = lower(
+    expr,
+    builder.environment,
+    builder.bindings,
+    new Map([...builder.context, ...capturedContext]),
+  );
   let loweredFunc: HIRFunction;
   if (lowering.isErr()) {
     lowering
@@ -4160,6 +4162,11 @@ function captureScopes({from, to}: {from: Scope; to: Scope}): Set<Scope> {
   return scopes;
 }
 
+/**
+ * Returns a mapping of "context" identifiers — references to free variables that
+ * will become part of the function expression's `context` array — along with the
+ * source location of their first reference within the function.
+ */
 function gatherCapturedContext(
   fn: NodePath<
     | t.FunctionExpression
@@ -4168,8 +4175,8 @@ function gatherCapturedContext(
     | t.ObjectMethod
   >,
   componentScope: Scope,
-): Array<t.Identifier> {
-  const capturedIds = new Set<t.Identifier>();
+): Map<t.Identifier, SourceLocation> {
+  const capturedIds = new Map<t.Identifier, SourceLocation>();
 
   /*
    * Capture all the scopes from the parent of this function up to and including
@@ -4212,8 +4219,15 @@ function gatherCapturedContext(
 
     // Add the base identifier binding as a dependency.
     const binding = baseIdentifier.scope.getBinding(baseIdentifier.node.name);
-    if (binding !== undefined && pureScopes.has(binding.scope)) {
-      capturedIds.add(binding.identifier);
+    if (
+      binding !== undefined &&
+      pureScopes.has(binding.scope) &&
+      !capturedIds.has(binding.identifier)
+    ) {
+      capturedIds.set(
+        binding.identifier,
+        path.node.loc ?? binding.identifier.loc ?? GeneratedSource,
+      );
     }
   }
 
@@ -4250,7 +4264,7 @@ function gatherCapturedContext(
     },
   });
 
-  return [...capturedIds.keys()];
+  return capturedIds;
 }
 
 function notNull<T>(value: T | null): value is T {

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

@@ -1388,6 +1388,16 @@ export enum ValueReason {
    */
   JsxCaptured = 'jsx-captured',
 
+  /**
+   * Argument to a hook
+   */
+  HookCaptured = 'hook-captured',
+
+  /**
+   * Return value of a hook
+   */
+  HookReturn = 'hook-return',
+
   /**
    * Passed to an effect
    */

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

@@ -106,7 +106,7 @@ export default class HIRBuilder {
   #current: WipBlock;
   #entry: BlockId;
   #scopes: Array<Scope> = [];
-  #context: Array<t.Identifier>;
+  #context: Map<t.Identifier, SourceLocation>;
   #bindings: Bindings;
   #env: Environment;
   #exceptionHandlerStack: Array<BlockId> = [];
@@ -121,7 +121,7 @@ export default class HIRBuilder {
     return this.#env.nextIdentifierId;
   }
 
-  get context(): Array<t.Identifier> {
+  get context(): Map<t.Identifier, SourceLocation> {
     return this.#context;
   }
 
@@ -137,13 +137,13 @@ export default class HIRBuilder {
     env: Environment,
     options?: {
       bindings?: Bindings | null;
-      context?: Array<t.Identifier>;
+      context?: Map<t.Identifier, SourceLocation>;
       entryBlockKind?: BlockKind;
     },
   ) {
     this.#env = env;
     this.#bindings = options?.bindings ?? new Map();
-    this.#context = options?.context ?? [];
+    this.#context = options?.context ?? new Map();
     this.#entry = makeBlockId(env.nextBlockId);
     this.#current = newBlock(this.#entry, options?.entryBlockKind ?? 'block');
   }

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

@@ -1302,6 +1302,34 @@ export const DefaultNonmutatingHook = addHook(
     calleeEffect: Effect.Read,
     hookKind: 'Custom',
     returnValueKind: ValueKind.Frozen,
+    aliasing: {
+      receiver: makeIdentifierId(0),
+      params: [],
+      rest: makeIdentifierId(1),
+      returns: makeIdentifierId(2),
+      temporaries: [],
+      effects: [
+        // Freeze the arguments
+        {
+          kind: 'Freeze',
+          value: signatureArgument(1),
+          reason: ValueReason.HookCaptured,
+        },
+        // Returns a frozen value
+        {
+          kind: 'Create',
+          into: signatureArgument(2),
+          value: ValueKind.Frozen,
+          reason: ValueReason.HookReturn,
+        },
+        // May alias any arguments into the return
+        {
+          kind: 'Alias',
+          from: signatureArgument(1),
+          into: signatureArgument(2),
+        },
+      ],
+    },
   },
   'DefaultNonmutatingHook',
 );

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

@@ -341,6 +341,10 @@ export function getWriteErrorReason(abstractValue: AbstractValue): string {
     return "Mutating a value returned from 'useReducer()', which should not be mutated. Use the dispatch function to update instead";
   } else if (abstractValue.reason.has(ValueReason.Effect)) {
     return 'Updating a value used previously in an effect function or as an effect dependency is not allowed. Consider moving the mutation before calling useEffect()';
+  } else if (abstractValue.reason.has(ValueReason.HookCaptured)) {
+    return 'Updating a value previously passed as an argument to a hook is not allowed. Consider moving the mutation before calling the hook';
+  } else if (abstractValue.reason.has(ValueReason.HookReturn)) {
+    return 'Updating a value returned from a hook is not allowed. Consider moving the mutation into the hook where the value is constructed';
   } else {
     return 'This mutates a variable that React considers immutable';
   }

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

@@ -1980,28 +1980,17 @@ function computeEffectsForLegacySignature(
         break;
       }
       case Effect.ConditionallyMutateIterator: {
-        if (
-          isArrayType(place.identifier) ||
-          isSetType(place.identifier) ||
-          isMapType(place.identifier)
-        ) {
-          effects.push({
-            kind: 'Capture',
-            from: place,
-            into: lvalue,
-          });
-        } else {
-          effects.push({
-            kind: 'Capture',
-            from: place,
-            into: lvalue,
-          });
+        const mutateIterator = conditionallyMutateIterator(place);
+        if (mutateIterator != null) {
+          effects.push(mutateIterator);
+          // TODO: should we always push to captures?
           captures.push(place);
-          effects.push({
-            kind: 'MutateTransitiveConditionally',
-            value: place,
-          });
         }
+        effects.push({
+          kind: 'Capture',
+          from: place,
+          into: lvalue,
+        });
         break;
       }
       case Effect.Freeze: {
@@ -2191,6 +2180,7 @@ function computeEffectsForSignature(
     return null;
   }
   // Build substitutions
+  const mutableSpreads = new Set<IdentifierId>();
   const substitutions: Map<IdentifierId, Array<Place>> = new Map();
   substitutions.set(signature.receiver, [receiver]);
   substitutions.set(signature.returns, [lvalue]);
@@ -2208,6 +2198,13 @@ function computeEffectsForSignature(
       }
       const place = arg.kind === 'Identifier' ? arg : arg.place;
       getOrInsertWith(substitutions, signature.rest, () => []).push(place);
+
+      if (arg.kind === 'Spread') {
+        const mutateIterator = conditionallyMutateIterator(arg.place);
+        if (mutateIterator != null) {
+          mutableSpreads.add(arg.place.identifier.id);
+        }
+      }
     } else {
       const param = params[i];
       substitutions.set(param, [arg]);
@@ -2279,6 +2276,12 @@ function computeEffectsForSignature(
       case 'Freeze': {
         const values = substitutions.get(effect.value.identifier.id) ?? [];
         for (const value of values) {
+          if (mutableSpreads.has(value.identifier.id)) {
+            CompilerError.throwTodo({
+              reason: 'Support spread syntax for hook arguments',
+              loc: value.loc,
+            });
+          }
           effects.push({kind: 'Freeze', value, reason: effect.reason});
         }
         break;

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hook-call-freezes-captured-identifier.expect.md

@@ -32,7 +32,7 @@ export const FIXTURE_ENTRYPOINT = {
   11 |   });
   12 |
 > 13 |   x.value += count;
-     |   ^ InvalidReact: This mutates a variable that React considers immutable (13:13)
+     |   ^ InvalidReact: Updating a value previously passed as an argument to a hook is not allowed. Consider moving the mutation before calling the hook (13:13)
   14 |   return <Stringify x={x} cb={cb} />;
   15 | }
   16 |

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hook-call-freezes-captured-memberexpr.expect.md

@@ -32,7 +32,7 @@ export const FIXTURE_ENTRYPOINT = {
   11 |   });
   12 |
 > 13 |   x.value += count;
-     |   ^ InvalidReact: This mutates a variable that React considers immutable (13:13)
+     |   ^ InvalidReact: Updating a value previously passed as an argument to a hook is not allowed. Consider moving the mutation before calling the hook (13:13)
   14 |   return <Stringify x={x} cb={cb} />;
   15 | }
   16 |

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-non-imported-reanimated-shared-value-writes.expect.md

@@ -27,7 +27,7 @@ function SomeComponent() {
    9 |   return (
   10 |     <Button
 > 11 |       onPress={() => (sharedVal.value = Math.random())}
-     |                       ^^^^^^^^^ InvalidReact: This mutates a variable that React considers immutable. Found mutation of `sharedVal` (11:11)
+     |                       ^^^^^^^^^ InvalidReact: Updating a value returned from a hook is not allowed. Consider moving the mutation into the hook where the value is constructed. Found mutation of `sharedVal` (11:11)
   12 |       title="Randomize"
   13 |     />
   14 |   );

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-valid-functiondecl-hoisting.expect.md

@@ -34,13 +34,13 @@ export const FIXTURE_ENTRYPOINT = {
 ## Error
 
 ```
-  13 |     return bar();
+  11 |
+  12 |   function foo() {
+> 13 |     return bar();
+     |            ^^^ Todo: [PruneHoistedContexts] Rewrite hoisted function references (13:13)
   14 |   }
-> 15 |   function bar() {
-     |            ^^^ Todo: [PruneHoistedContexts] Rewrite hoisted function references (15:15)
+  15 |   function bar() {
   16 |     return 42;
-  17 |   }
-  18 |
 ```
           
       

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit/retry-no-emit.expect.md

@@ -52,7 +52,7 @@ export const FIXTURE_ENTRYPOINT = {
 ## Logs
 
 ```
-{"kind":"CompileError","fnLoc":{"start":{"line":5,"column":0,"index":163},"end":{"line":13,"column":1,"index":357},"filename":"retry-no-emit.ts"},"detail":{"reason":"This mutates a variable that React considers immutable","description":null,"severity":"InvalidReact","suggestions":null,"loc":{"start":{"line":11,"column":2,"index":320},"end":{"line":11,"column":6,"index":324},"filename":"retry-no-emit.ts","identifierName":"arr2"}}}
+{"kind":"CompileError","fnLoc":{"start":{"line":5,"column":0,"index":163},"end":{"line":13,"column":1,"index":357},"filename":"retry-no-emit.ts"},"detail":{"reason":"Updating a value previously passed as an argument to a hook is not allowed. Consider moving the mutation before calling the hook","description":null,"severity":"InvalidReact","suggestions":null,"loc":{"start":{"line":11,"column":2,"index":320},"end":{"line":11,"column":6,"index":324},"filename":"retry-no-emit.ts","identifierName":"arr2"}}}
 {"kind":"AutoDepsDecorations","fnLoc":{"start":{"line":7,"column":2,"index":216},"end":{"line":7,"column":36,"index":250},"filename":"retry-no-emit.ts"},"decorations":[{"start":{"line":7,"column":31,"index":245},"end":{"line":7,"column":34,"index":248},"filename":"retry-no-emit.ts","identifierName":"arr"}]}
 {"kind":"AutoDepsDecorations","fnLoc":{"start":{"line":10,"column":2,"index":274},"end":{"line":10,"column":44,"index":316},"filename":"retry-no-emit.ts"},"decorations":[{"start":{"line":10,"column":25,"index":297},"end":{"line":10,"column":29,"index":301},"filename":"retry-no-emit.ts","identifierName":"arr2"},{"start":{"line":10,"column":25,"index":297},"end":{"line":10,"column":29,"index":301},"filename":"retry-no-emit.ts","identifierName":"arr2"},{"start":{"line":10,"column":35,"index":307},"end":{"line":10,"column":42,"index":314},"filename":"retry-no-emit.ts","identifierName":"propVal"}]}
 {"kind":"CompileSuccess","fnLoc":{"start":{"line":5,"column":0,"index":163},"end":{"line":13,"column":1,"index":357},"filename":"retry-no-emit.ts"},"fnName":"Foo","memoSlots":0,"memoBlocks":0,"memoValues":0,"prunedMemoBlocks":0,"prunedMemoValues":0}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.mutate-frozen-value.expect.md

@@ -19,7 +19,7 @@ function Component({a, b}) {
   3 |   const x = {a};
   4 |   useFreeze(x);
 > 5 |   x.y = true;
-    |   ^ InvalidReact: This mutates a variable that React considers immutable (5:5)
+    |   ^ InvalidReact: Updating a value previously passed as an argument to a hook is not allowed. Consider moving the mutation before calling the hook (5:5)
   6 |   return <div>error</div>;
   7 | }
   8 |

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/retry-no-emit.expect.md

@@ -52,7 +52,7 @@ export const FIXTURE_ENTRYPOINT = {
 ## Logs
 
 ```
-{"kind":"CompileError","fnLoc":{"start":{"line":5,"column":0,"index":195},"end":{"line":13,"column":1,"index":389},"filename":"retry-no-emit.ts"},"detail":{"reason":"This mutates a variable that React considers immutable","description":null,"severity":"InvalidReact","suggestions":null,"loc":{"start":{"line":11,"column":2,"index":352},"end":{"line":11,"column":6,"index":356},"filename":"retry-no-emit.ts","identifierName":"arr2"}}}
+{"kind":"CompileError","fnLoc":{"start":{"line":5,"column":0,"index":195},"end":{"line":13,"column":1,"index":389},"filename":"retry-no-emit.ts"},"detail":{"reason":"Updating a value previously passed as an argument to a hook is not allowed. Consider moving the mutation before calling the hook","description":null,"severity":"InvalidReact","suggestions":null,"loc":{"start":{"line":11,"column":2,"index":352},"end":{"line":11,"column":6,"index":356},"filename":"retry-no-emit.ts","identifierName":"arr2"}}}
 {"kind":"AutoDepsDecorations","fnLoc":{"start":{"line":7,"column":2,"index":248},"end":{"line":7,"column":36,"index":282},"filename":"retry-no-emit.ts"},"decorations":[{"start":{"line":7,"column":31,"index":277},"end":{"line":7,"column":34,"index":280},"filename":"retry-no-emit.ts","identifierName":"arr"}]}
 {"kind":"AutoDepsDecorations","fnLoc":{"start":{"line":10,"column":2,"index":306},"end":{"line":10,"column":44,"index":348},"filename":"retry-no-emit.ts"},"decorations":[{"start":{"line":10,"column":25,"index":329},"end":{"line":10,"column":29,"index":333},"filename":"retry-no-emit.ts","identifierName":"arr2"},{"start":{"line":10,"column":25,"index":329},"end":{"line":10,"column":29,"index":333},"filename":"retry-no-emit.ts","identifierName":"arr2"},{"start":{"line":10,"column":35,"index":339},"end":{"line":10,"column":42,"index":346},"filename":"retry-no-emit.ts","identifierName":"propVal"}]}
 {"kind":"CompileSuccess","fnLoc":{"start":{"line":5,"column":0,"index":195},"end":{"line":13,"column":1,"index":389},"filename":"retry-no-emit.ts"},"fnName":"Foo","memoSlots":0,"memoBlocks":0,"memoValues":0,"prunedMemoBlocks":0,"prunedMemoValues":0}