[Compiler] Don't count a setState in the dependency array of the effect it is called on as a usage (facebook#35134)

Summary:
The validation only allows setState declaration as a usage outside of
the effect.

Another edge case is that if you add the setState being validated in the
dependency array you also make the validation opt out since it counts as
a usage outside of the effect.

Added a bit of logic to consider the effect's deps when creating the
cache for setState usages within the effect

Test Plan:
Added a fixture

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35134).
* facebook#35135
* __->__ facebook#35134

Author: jorge-cab

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

@@ -22,6 +22,7 @@ import {
   BasicBlock,
   isUseRefType,
   SourceLocation,
+  ArrayExpression,
 } from '../HIR';
 import {eachInstructionLValue, eachInstructionOperand} from '../HIR/visitors';
 import {isMutable} from '../ReactiveScopes/InferReactiveScopeVariables';
@@ -36,11 +37,17 @@ type DerivationMetadata = {
   isStateSource: boolean;
 };
 
+type EffectMetadata = {
+  effect: HIRFunction;
+  dependencies: ArrayExpression;
+};
+
 type ValidationContext = {
   readonly functions: Map<IdentifierId, FunctionExpression>;
+  readonly candidateDependencies: Map<IdentifierId, ArrayExpression>;
   readonly errors: CompilerError;
   readonly derivationCache: DerivationCache;
-  readonly effects: Set<HIRFunction>;
+  readonly effectsCache: Map<IdentifierId, EffectMetadata>;
   readonly setStateLoads: Map<IdentifierId, IdentifierId | null>;
   readonly setStateUsages: Map<IdentifierId, Set<SourceLocation>>;
 };
@@ -175,18 +182,20 @@ export function validateNoDerivedComputationsInEffects_exp(
   fn: HIRFunction,
 ): Result<void, CompilerError> {
   const functions: Map<IdentifierId, FunctionExpression> = new Map();
+  const candidateDependencies: Map<IdentifierId, ArrayExpression> = new Map();
   const derivationCache = new DerivationCache();
   const errors = new CompilerError();
-  const effects: Set<HIRFunction> = new Set();
+  const effectsCache: Map<IdentifierId, EffectMetadata> = new Map();
 
   const setStateLoads: Map<IdentifierId, IdentifierId> = new Map();
   const setStateUsages: Map<IdentifierId, Set<SourceLocation>> = new Map();
 
   const context: ValidationContext = {
     functions,
+    candidateDependencies,
     errors,
     derivationCache,
-    effects,
+    effectsCache,
     setStateLoads,
     setStateUsages,
   };
@@ -229,8 +238,8 @@ export function validateNoDerivedComputationsInEffects_exp(
     isFirstPass = false;
   } while (context.derivationCache.snapshot());
 
-  for (const effect of effects) {
-    validateEffect(effect, context);
+  for (const [, effect] of effectsCache) {
+    validateEffect(effect.effect, effect.dependencies, context);
   }
 
   return errors.asResult();
@@ -354,8 +363,14 @@ function recordInstructionDerivations(
       value.args[1].kind === 'Identifier'
     ) {
       const effectFunction = context.functions.get(value.args[0].identifier.id);
-      if (effectFunction != null) {
-        context.effects.add(effectFunction.loweredFunc.func);
+      const deps = context.candidateDependencies.get(
+        value.args[1].identifier.id,
+      );
+      if (effectFunction != null && deps != null) {
+        context.effectsCache.set(value.args[0].identifier.id, {
+          effect: effectFunction.loweredFunc.func,
+          dependencies: deps,
+        });
       }
     } else if (isUseStateType(lvalue.identifier) && value.args.length > 0) {
       typeOfValue = 'fromState';
@@ -367,6 +382,8 @@ function recordInstructionDerivations(
       );
       return;
     }
+  } else if (value.kind === 'ArrayExpression') {
+    context.candidateDependencies.set(lvalue.identifier.id, value);
   }
 
   for (const operand of eachInstructionOperand(instr)) {
@@ -596,6 +613,7 @@ function getFnLocalDeps(
 
 function validateEffect(
   effectFunction: HIRFunction,
+  dependencies: ArrayExpression,
   context: ValidationContext,
 ): void {
   const seenBlocks: Set<BlockId> = new Set();
@@ -612,6 +630,16 @@ function validateEffect(
     Set<SourceLocation>
   > = new Map();
 
+  // Consider setStates in the effect's dependency array as being part of effectSetStateUsages
+  for (const dep of dependencies.elements) {
+    if (dep.kind === 'Identifier') {
+      const root = getRootSetState(dep.identifier.id, context.setStateLoads);
+      if (root !== null) {
+        effectSetStateUsages.set(root, new Set([dep.loc]));
+      }
+    }
+  }
+
   let cleanUpFunctionDeps: Set<IdentifierId> | undefined;
 
   const globals: Set<IdentifierId> = new Set();

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/effect-used-in-dep-array-still-errors.expect.md

@@ -0,0 +1,63 @@
+
+## Input
+
+```javascript
+// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly
+
+function Component({prop}) {
+  const [s, setS] = useState(0);
+  useEffect(() => {
+    setS(prop);
+  }, [prop, setS]);
+
+  return <div>{prop}</div>;
+}
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp @loggerTestOnly
+
+function Component(t0) {
+  const $ = _c(5);
+  const { prop } = t0;
+  const [, setS] = useState(0);
+  let t1;
+  let t2;
+  if ($[0] !== prop) {
+    t1 = () => {
+      setS(prop);
+    };
+    t2 = [prop, setS];
+    $[0] = prop;
+    $[1] = t1;
+    $[2] = t2;
+  } else {
+    t1 = $[1];
+    t2 = $[2];
+  }
+  useEffect(t1, t2);
+  let t3;
+  if ($[3] !== prop) {
+    t3 = <div>{prop}</div>;
+    $[3] = prop;
+    $[4] = t3;
+  } else {
+    t3 = $[4];
+  }
+  return t3;
+}
+
+```
+
+## Logs
+
+```
+{"kind":"CompileError","detail":{"options":{"description":"Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user\n\nThis setState call is setting a derived value that depends on the following reactive sources:\n\nProps: [prop]\n\nData Flow Tree:\n└── prop (Prop)\n\nSee: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state","category":"EffectDerivationsOfState","reason":"You might not need an effect. Derive values in render, not effects.","details":[{"kind":"error","loc":{"start":{"line":6,"column":4,"index":150},"end":{"line":6,"column":8,"index":154},"filename":"effect-used-in-dep-array-still-errors.ts","identifierName":"setS"},"message":"This should be computed during render, not in an effect"}]}},"fnLoc":null}
+{"kind":"CompileSuccess","fnLoc":{"start":{"line":3,"column":0,"index":64},"end":{"line":10,"column":1,"index":212},"filename":"effect-used-in-dep-array-still-errors.ts"},"fnName":"Component","memoSlots":5,"memoBlocks":2,"memoValues":3,"prunedMemoBlocks":0,"prunedMemoValues":0}
+```
+      
+### Eval output
+(kind: exception) Fixture not implemented

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/effect-used-in-dep-array-still-errors.js

@@ -0,0 +1,10 @@
+// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly
+
+function Component({prop}) {
+  const [s, setS] = useState(0);
+  useEffect(() => {
+    setS(prop);
+  }, [prop, setS]);
+
+  return <div>{prop}</div>;
+}