[compiler] Fix for destructuring with mixed declaration/reassignment

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

@@ -11,6 +11,7 @@ import {
   BasicBlock,
   BlockId,
   Instruction,
+  InstructionKind,
   InstructionValue,
   makeInstructionId,
   Pattern,
@@ -32,6 +33,32 @@ export function* eachInstructionLValue(
   yield* eachInstructionValueLValue(instr.value);
 }
 
+export function* eachInstructionLValueWithKind(
+  instr: ReactiveInstruction,
+): Iterable<[Place, InstructionKind]> {
+  switch (instr.value.kind) {
+    case 'DeclareContext':
+    case 'StoreContext':
+    case 'DeclareLocal':
+    case 'StoreLocal': {
+      yield [instr.value.lvalue.place, instr.value.lvalue.kind];
+      break;
+    }
+    case 'Destructure': {
+      const kind = instr.value.lvalue.kind;
+      for (const place of eachPatternOperand(instr.value.lvalue.pattern)) {
+        yield [place, kind];
+      }
+      break;
+    }
+    case 'PostfixUpdate':
+    case 'PrefixUpdate': {
+      yield [instr.value.lvalue, InstructionKind.Reassign];
+      break;
+    }
+  }
+}
+
 export function* eachInstructionValueLValue(
   value: ReactiveValue,
 ): Iterable<Place> {

compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts

@@ -1359,35 +1359,13 @@ function codegenInstructionNullable(
       value = null;
     } else {
       lvalue = instr.value.lvalue.pattern;
-      let hasReassign = false;
-      let hasDeclaration = false;
       for (const place of eachPatternOperand(lvalue)) {
         if (
           kind !== InstructionKind.Reassign &&
           place.identifier.name === null
         ) {
           cx.temp.set(place.identifier.declarationId, null);
         }
-        const isDeclared = cx.hasDeclared(place.identifier);
-        hasReassign ||= isDeclared;
-        hasDeclaration ||= !isDeclared;
-      }
-      if (hasReassign && hasDeclaration) {
-        CompilerError.invariant(false, {
-          reason:
-            'Encountered a destructuring operation where some identifiers are already declared (reassignments) but others are not (declarations)',
-          description: null,
-          details: [
-            {
-              kind: 'error',
-              loc: instr.loc,
-              message: null,
-            },
-          ],
-          suggestions: null,
-        });
-      } else if (hasReassign) {
-        kind = InstructionKind.Reassign;
       }
       value = codegenPlaceToExpression(cx, instr.value.value);
     }

compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/ExtractScopeDeclarationsFromDestructuring.ts

@@ -19,7 +19,11 @@ import {
   promoteTemporary,
 } from '../HIR';
 import {clonePlaceToTemporary} from '../HIR/HIRBuilder';
-import {eachPatternOperand, mapPatternOperands} from '../HIR/visitors';
+import {
+  eachInstructionLValueWithKind,
+  eachPatternOperand,
+  mapPatternOperands,
+} from '../HIR/visitors';
 import {
   ReactiveFunctionTransform,
   Transformed,
@@ -113,14 +117,18 @@ class Visitor extends ReactiveFunctionTransform<State> {
   ): Transformed<ReactiveStatement> {
     this.visitInstruction(instruction, state);
 
+    let instructionsToProcess: Array<ReactiveInstruction> = [instruction];
+    let result: Transformed<ReactiveStatement> = {kind: 'keep'};
+
     if (instruction.value.kind === 'Destructure') {
       const transformed = transformDestructuring(
         state,
         instruction,
         instruction.value,
       );
       if (transformed) {
-        return {
+        instructionsToProcess = transformed;
+        result = {
           kind: 'replace-many',
           value: transformed.map(instruction => ({
             kind: 'instruction',
@@ -129,7 +137,17 @@ class Visitor extends ReactiveFunctionTransform<State> {
         };
       }
     }
-    return {kind: 'keep'};
+
+    // Update state.declared with declarations from the instruction(s)
+    for (const instr of instructionsToProcess) {
+      for (const [place, kind] of eachInstructionLValueWithKind(instr)) {
+        if (kind !== InstructionKind.Reassign) {
+          state.declared.add(place.identifier.declarationId);
+        }
+      }
+    }
+
+    return result;
   }
 }
 
@@ -144,10 +162,13 @@ function transformDestructuring(
     const isDeclared = state.declared.has(place.identifier.declarationId);
     if (isDeclared) {
       reassigned.add(place.identifier.id);
+    } else {
+      hasDeclaration = true;
     }
-    hasDeclaration ||= !isDeclared;
   }
-  if (reassigned.size === 0 || !hasDeclaration) {
+  if (!hasDeclaration) {
+    // all reassignments
+    destructure.lvalue.kind = InstructionKind.Reassign;
     return null;
   }
   /*

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-invalid-destructuring-reassignment-undefined-variable.expect.md

@@ -0,0 +1,162 @@
+
+## Input
+
+```javascript
+// @flow @compilationMode:"infer"
+'use strict';
+
+function getWeekendDays(user) {
+  return [0, 6];
+}
+
+function getConfig(weekendDays) {
+  return [1, 5];
+}
+
+component Calendar(user, defaultFirstDay, currentDate, view) {
+  const weekendDays = getWeekendDays(user);
+  let firstDay = defaultFirstDay;
+  let daysToDisplay = 7;
+  if (view === 'week') {
+    let lastDay;
+    // this assignment produces invalid code
+    [firstDay, lastDay] = getConfig(weekendDays);
+    daysToDisplay = ((7 + lastDay - firstDay) % 7) + 1;
+  } else if (view === 'day') {
+    firstDay = currentDate.getDayOfWeek();
+    daysToDisplay = 1;
+  }
+
+  return [currentDate, firstDay, daysToDisplay];
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Calendar,
+  params: [
+    {
+      user: {},
+      defaultFirstDay: 1,
+      currentDate: {getDayOfWeek: () => 3},
+      view: 'week',
+    },
+  ],
+  sequentialRenders: [
+    {
+      user: {},
+      defaultFirstDay: 1,
+      currentDate: {getDayOfWeek: () => 3},
+      view: 'week',
+    },
+    {
+      user: {},
+      defaultFirstDay: 1,
+      currentDate: {getDayOfWeek: () => 3},
+      view: 'day',
+    },
+  ],
+};
+
+```
+
+## Code
+
+```javascript
+"use strict";
+import { c as _c } from "react/compiler-runtime";
+
+function getWeekendDays(user) {
+  return [0, 6];
+}
+
+function getConfig(weekendDays) {
+  return [1, 5];
+}
+
+function Calendar(t0) {
+  const $ = _c(12);
+  const { user, defaultFirstDay, currentDate, view } = t0;
+  let daysToDisplay;
+  let firstDay;
+  if (
+    $[0] !== currentDate ||
+    $[1] !== defaultFirstDay ||
+    $[2] !== user ||
+    $[3] !== view
+  ) {
+    const weekendDays = getWeekendDays(user);
+    firstDay = defaultFirstDay;
+    daysToDisplay = 7;
+    if (view === "week") {
+      let lastDay;
+
+      [firstDay, lastDay] = getConfig(weekendDays);
+      daysToDisplay = ((7 + lastDay - firstDay) % 7) + 1;
+    } else {
+      if (view === "day") {
+        let t1;
+        if ($[6] !== currentDate) {
+          t1 = currentDate.getDayOfWeek();
+          $[6] = currentDate;
+          $[7] = t1;
+        } else {
+          t1 = $[7];
+        }
+        firstDay = t1;
+        daysToDisplay = 1;
+      }
+    }
+    $[0] = currentDate;
+    $[1] = defaultFirstDay;
+    $[2] = user;
+    $[3] = view;
+    $[4] = daysToDisplay;
+    $[5] = firstDay;
+  } else {
+    daysToDisplay = $[4];
+    firstDay = $[5];
+  }
+  let t1;
+  if ($[8] !== currentDate || $[9] !== daysToDisplay || $[10] !== firstDay) {
+    t1 = [currentDate, firstDay, daysToDisplay];
+    $[8] = currentDate;
+    $[9] = daysToDisplay;
+    $[10] = firstDay;
+    $[11] = t1;
+  } else {
+    t1 = $[11];
+  }
+  return t1;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Calendar,
+  params: [
+    {
+      user: {},
+      defaultFirstDay: 1,
+      currentDate: { getDayOfWeek: () => 3 },
+      view: "week",
+    },
+  ],
+
+  sequentialRenders: [
+    {
+      user: {},
+      defaultFirstDay: 1,
+      currentDate: { getDayOfWeek: () => 3 },
+      view: "week",
+    },
+    {
+      user: {},
+      defaultFirstDay: 1,
+      currentDate: { getDayOfWeek: () => 3 },
+      view: "day",
+    },
+  ],
+};
+
+```
+      
+### Eval output
+(kind: ok) [{"getDayOfWeek":"[[ function params=0 ]]"},1,5]
+[{"getDayOfWeek":"[[ function params=0 ]]"},3,1]

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-invalid-destructuring-reassignment-undefined-variable.js

@@ -0,0 +1,53 @@
+// @flow @compilationMode:"infer"
+'use strict';
+
+function getWeekendDays(user) {
+  return [0, 6];
+}
+
+function getConfig(weekendDays) {
+  return [1, 5];
+}
+
+component Calendar(user, defaultFirstDay, currentDate, view) {
+  const weekendDays = getWeekendDays(user);
+  let firstDay = defaultFirstDay;
+  let daysToDisplay = 7;
+  if (view === 'week') {
+    let lastDay;
+    // this assignment produces invalid code
+    [firstDay, lastDay] = getConfig(weekendDays);
+    daysToDisplay = ((7 + lastDay - firstDay) % 7) + 1;
+  } else if (view === 'day') {
+    firstDay = currentDate.getDayOfWeek();
+    daysToDisplay = 1;
+  }
+
+  return [currentDate, firstDay, daysToDisplay];
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Calendar,
+  params: [
+    {
+      user: {},
+      defaultFirstDay: 1,
+      currentDate: {getDayOfWeek: () => 3},
+      view: 'week',
+    },
+  ],
+  sequentialRenders: [
+    {
+      user: {},
+      defaultFirstDay: 1,
+      currentDate: {getDayOfWeek: () => 3},
+      view: 'week',
+    },
+    {
+      user: {},
+      defaultFirstDay: 1,
+      currentDate: {getDayOfWeek: () => 3},
+      view: 'day',
+    },
+  ],
+};