Repository: facebook/react
Pull request: [compiler] More complete validation against locals being reassigned after render

Stack from [ghstack](https://github.com/ezyang/ghstack) (oldest at bottom):
* __->__ #30540

Summary:
This diff extends the existing work on validating against locals being reassigned after render, by propagating the reassignment "effect" into the lvalues of instructions when the rvalue operands include values known to cause reassignments. In particular, this "closes the loop" for function definitions and function calls: a function that returns a function that reassigns will be considered to also perform reassignments, but previous to this we didn't consider the result of a `Call` of a function that reassigns to itself be a value that reassigns.

This causes a number of new bailouts in test cases, all of which appear to me to be legit.

Changed files:
filename: compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateLocalsNotReassignedAfterRender.ts
patch: @@ -12,6 +12,7 @@ import {
   eachInstructionValueOperand,
   eachTerminalOperand,
 } from '../HIR/visitors';
+import {getFunctionCallSignature} from '../Inference/InferReferenceEffects';
 
 /**
  * Validates that local variables cannot be reassigned after render.
@@ -132,7 +133,26 @@ function getContextReassignment(
           break;
         }
         default: {
-          for (const operand of eachInstructionValueOperand(value)) {
+          let operands = eachInstructionValueOperand(value);
+          // If we're calling a function that doesn't let its arguments escape, only test the callee
+          if (value.kind === 'CallExpression') {
+            const signature = getFunctionCallSignature(
+              fn.env,
+              value.callee.identifier.type,
+            );
+            if (signature?.noAlias) {
+              operands = [value.callee];
+            }
+          } else if (value.kind === 'MethodCall') {
+            const signature = getFunctionCallSignature(
+              fn.env,
+              value.property.identifier.type,
+            );
+            if (signature?.noAlias) {
+              operands = [value.receiver, value.property];
+            }
+          }
+          for (const operand of operands) {
             CompilerError.invariant(operand.effect !== Effect.Unknown, {
               reason: `Expected effects to be inferred prior to ValidateLocalsNotReassignedAfterRender`,
               loc: operand.loc,
@@ -148,6 +168,10 @@ function getContextReassignment(
               if (operand.effect === Effect.Freeze) {
                 return reassignment;
               } else {
+                /*
+                 * If the operand is not frozen but it does reassign, then the lvalues
+                 * of the instruction could also be reassigning
+                 */
                 for (const lval of eachInstructionLValue(instr)) {
                   reassigningFunctions.set(lval.identifier.id, reassignment);
                 }

filename: compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.dont-memoize-array-with-capturing-map-after-hook.expect.md
patch: @@ -1,47 +0,0 @@
-
-## Input
-
-```javascript
-import {useEffect, useState} from 'react';
-import {mutate} from 'shared-runtime';
-
-function Component(props) {
-  const x = [{...props.value}];
-  useEffect(() => {}, []);
-  const onClick = () => {
-    console.log(x.length);
-  };
-  let y;
-  return (
-    <div onClick={onClick}>
-      {x.map(item => {
-        y = item;
-        return <span key={item.id}>{item.text}</span>;
-      })}
-      {mutate(y)}
-    </div>
-  );
-}
-
-export const FIXTURE_ENTRYPOINT = {
-  fn: Component,
-  params: [{value: {id: 0, text: 'Hello!'}}],
-  isComponent: true,
-};
-
-```
-
-
-## Error
-
-```
-  12 |     <div onClick={onClick}>
-  13 |       {x.map(item => {
-> 14 |         y = item;
-     |         ^ InvalidReact: Reassigning a variable after render has completed can cause inconsistent behavior on subsequent renders. Consider using state instead. Variable `y` cannot be reassigned after render (14:14)
-  15 |         return <span key={item.id}>{item.text}</span>;
-  16 |       })}
-  17 |       {mutate(y)}
-```
-          
-      
\ No newline at end of file

filename: compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-dont-memoize-array-with-capturing-map-after-hook.expect.md
patch: @@ -0,0 +1,91 @@
+
+## Input
+
+```javascript
+import {useEffect, useState} from 'react';
+import {mutate} from 'shared-runtime';
+
+function Component(props) {
+  const x = [{...props.value}];
+  useEffect(() => {}, []);
+  const onClick = () => {
+    console.log(x.length);
+  };
+  let y;
+  return (
+    <div onClick={onClick}>
+      {x.map(item => {
+        y = item;
+        return <span key={item.id}>{item.text}</span>;
+      })}
+      {mutate(y)}
+    </div>
+  );
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{value: {id: 0, text: 'Hello!'}}],
+  isComponent: true,
+};
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime";
+import { useEffect, useState } from "react";
+import { mutate } from "shared-runtime";
+
+function Component(props) {
+  const $ = _c(5);
+  const x = [{ ...props.value }];
+  let t0;
+  if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
+    t0 = [];
+    $[0] = t0;
+  } else {
+    t0 = $[0];
+  }
+  useEffect(_temp, t0);
+  const onClick = () => {
+    console.log(x.length);
+  };
+
+  let y;
+
+  const t1 = x.map((item) => {
+    y = item;
+    return <span key={item.id}>{item.text}</span>;
+  });
+  const t2 = mutate(y);
+  let t3;
+  if ($[1] !== onClick || $[2] !== t1 || $[3] !== t2) {
+    t3 = (
+      <div onClick={onClick}>
+        {t1}
+        {t2}
+      </div>
+    );
+    $[1] = onClick;
+    $[2] = t1;
+    $[3] = t2;
+    $[4] = t3;
+  } else {
+    t3 = $[4];
+  }
+  return t3;
+}
+function _temp() {}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{ value: { id: 0, text: "Hello!" } }],
+  isComponent: true,
+};
+
+```
+      
+### Eval output
+(kind: ok) <div><span>Hello!</span></div>
\ No newline at end of file