Revert "[compiler] Fix VariableDeclarator source location (#35129)" (#35346)

This broke main.

Author: sebmarkbage

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

@@ -4026,7 +4026,6 @@ function lowerAssignment(
           pattern: {
             kind: 'ArrayPattern',
             items,
-            loc: lvalue.node.loc ?? GeneratedSource,
           },
         },
         value,
@@ -4204,7 +4203,6 @@ function lowerAssignment(
           pattern: {
             kind: 'ObjectPattern',
             properties,
-            loc: lvalue.node.loc ?? GeneratedSource,
           },
         },
         value,

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

@@ -694,13 +694,11 @@ export type SpreadPattern = {
 export type ArrayPattern = {
   kind: 'ArrayPattern';
   items: Array<Place | SpreadPattern | Hole>;
-  loc: SourceLocation;
 };
 
 export type ObjectPattern = {
   kind: 'ObjectPattern';
   properties: Array<ObjectProperty | SpreadPattern>;
-  loc: SourceLocation;
 };
 
 export type ObjectPropertyKey =

compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineJsx.ts

@@ -515,7 +515,6 @@ function emitDestructureProps(
         pattern: {
           kind: 'ObjectPattern',
           properties,
-          loc: GeneratedSource,
         },
         kind: InstructionKind.Let,
       },

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

@@ -702,7 +702,7 @@ function codegenReactiveScope(
     outputComments.push(name.name);
     if (!cx.hasDeclared(identifier)) {
       statements.push(
-        t.variableDeclaration('let', [createVariableDeclarator(name, null)]),
+        t.variableDeclaration('let', [t.variableDeclarator(name)]),
       );
     }
     cacheLoads.push({name, index, value: wrapCacheDep(cx, name)});
@@ -1387,7 +1387,7 @@ function codegenInstructionNullable(
           suggestions: null,
         });
         return createVariableDeclaration(instr.loc, 'const', [
-          createVariableDeclarator(codegenLValue(cx, lvalue), value),
+          t.variableDeclarator(codegenLValue(cx, lvalue), value),
         ]);
       }
       case InstructionKind.Function: {
@@ -1451,7 +1451,7 @@ function codegenInstructionNullable(
           suggestions: null,
         });
         return createVariableDeclaration(instr.loc, 'let', [
-          createVariableDeclarator(codegenLValue(cx, lvalue), value),
+          t.variableDeclarator(codegenLValue(cx, lvalue), value),
         ]);
       }
       case InstructionKind.Reassign: {
@@ -1691,9 +1691,6 @@ function withLoc<T extends (...args: Array<any>) => t.Node>(
   };
 }
 
-const createIdentifier = withLoc(t.identifier);
-const createArrayPattern = withLoc(t.arrayPattern);
-const createObjectPattern = withLoc(t.objectPattern);
 const createBinaryExpression = withLoc(t.binaryExpression);
 const createExpressionStatement = withLoc(t.expressionStatement);
 const _createLabelledStatement = withLoc(t.labeledStatement);
@@ -1725,31 +1722,6 @@ const createTryStatement = withLoc(t.tryStatement);
 const createBreakStatement = withLoc(t.breakStatement);
 const createContinueStatement = withLoc(t.continueStatement);
 
-function createVariableDeclarator(
-  id: t.LVal,
-  init?: t.Expression | null,
-): t.VariableDeclarator {
-  const node = t.variableDeclarator(id, init);
-
-  /*
-   * The variable declarator location is not preserved in HIR, however, we can use the
-   * start location of the id and the end location of the init to recreate the
-   * exact original variable declarator location.
-   *
-   * Or if init is null, we likely have a declaration without an initializer, so we can use the id.loc.end as the end location.
-   */
-  if (id.loc && (init === null || init?.loc)) {
-    node.loc = {
-      start: id.loc.start,
-      end: init?.loc?.end ?? id.loc.end,
-      filename: id.loc.filename,
-      identifierName: undefined,
-    };
-  }
-
-  return node;
-}
-
 function createHookGuard(
   guard: ExternalFunction,
   context: ProgramContext,
@@ -1857,7 +1829,7 @@ function codegenInstruction(
       );
     } else {
       return createVariableDeclaration(instr.loc, 'const', [
-        createVariableDeclarator(
+        t.variableDeclarator(
           convertIdentifier(instr.lvalue.identifier),
           expressionValue,
         ),
@@ -2784,7 +2756,7 @@ function codegenArrayPattern(
 ): t.ArrayPattern {
   const hasHoles = !pattern.items.every(e => e.kind !== 'Hole');
   if (hasHoles) {
-    const result = createArrayPattern(pattern.loc, []);
+    const result = t.arrayPattern([]);
     /*
      * Older versions of babel have a validation bug fixed by
      * https://github.com/babel/babel/pull/10917
@@ -2805,8 +2777,7 @@ function codegenArrayPattern(
     }
     return result;
   } else {
-    return createArrayPattern(
-      pattern.loc,
+    return t.arrayPattern(
       pattern.items.map(item => {
         if (item.kind === 'Hole') {
           return null;
@@ -2826,8 +2797,7 @@ function codegenLValue(
       return codegenArrayPattern(cx, pattern);
     }
     case 'ObjectPattern': {
-      return createObjectPattern(
-        pattern.loc,
+      return t.objectPattern(
         pattern.properties.map(property => {
           if (property.kind === 'ObjectProperty') {
             const key = codegenObjectPropertyKey(cx, property.key);
@@ -2946,7 +2916,7 @@ function convertIdentifier(identifier: Identifier): t.Identifier {
       suggestions: null,
     },
   );
-  return createIdentifier(identifier.loc, identifier.name.value);
+  return t.identifier(identifier.name.value);
 }
 
 function compareScopeDependency(

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

@@ -27,11 +27,7 @@ import {Result} from '../Utils/Result';
 
 /**
  * Some common node types that are important for coverage tracking.
- * Based on istanbul-lib-instrument + some other common nodes we expect to be present in the generated AST.
- *
- * Note: For VariableDeclaration, VariableDeclarator, and Identifier, we enforce stricter validation
- * that requires both the source location AND node type to match in the generated AST. This ensures
- * that variable declarations maintain their structural integrity through compilation.
+ * Based on istanbul-lib-instrument
  */
 const IMPORTANT_INSTRUMENTED_TYPES = new Set([
   'ArrowFunctionExpression',
@@ -58,14 +54,6 @@ const IMPORTANT_INSTRUMENTED_TYPES = new Set([
   'LabeledStatement',
   'ConditionalExpression',
   'LogicalExpression',
-
-  /**
-   * Note: these aren't important for coverage tracking,
-   * but we still want to track them to ensure we aren't regressing them when
-   * we fix the source location tracking for other nodes.
-   */
-  'VariableDeclaration',
-  'Identifier',
 ]);
 
 /**
@@ -126,13 +114,10 @@ export function validateSourceLocations(
 ): Result<void, CompilerError> {
   const errors = new CompilerError();
 
-  /*
-   * Step 1: Collect important locations from the original source
-   * Note: Multiple node types can share the same location (e.g. VariableDeclarator and Identifier)
-   */
+  // Step 1: Collect important locations from the original source
   const importantOriginalLocations = new Map<
     string,
-    {loc: t.SourceLocation; nodeTypes: Set<string>}
+    {loc: t.SourceLocation; nodeType: string}
   >();
 
   func.traverse({
@@ -152,31 +137,20 @@ export function validateSourceLocations(
       // Collect the location if it exists
       if (node.loc) {
         const key = locationKey(node.loc);
-        const existing = importantOriginalLocations.get(key);
-        if (existing) {
-          existing.nodeTypes.add(node.type);
-        } else {
-          importantOriginalLocations.set(key, {
-            loc: node.loc,
-            nodeTypes: new Set([node.type]),
-          });
-        }
+        importantOriginalLocations.set(key, {
+          loc: node.loc,
+          nodeType: node.type,
+        });
       }
     },
   });
 
-  // Step 2: Collect all locations from the generated AST with their node types
-  const generatedLocations = new Map<string, Set<string>>();
+  // Step 2: Collect all locations from the generated AST
+  const generatedLocations = new Set<string>();
 
   function collectGeneratedLocations(node: t.Node): void {
     if (node.loc) {
-      const key = locationKey(node.loc);
-      const nodeTypes = generatedLocations.get(key);
-      if (nodeTypes) {
-        nodeTypes.add(node.type);
-      } else {
-        generatedLocations.set(key, new Set([node.type]));
-      }
+      generatedLocations.add(locationKey(node.loc));
     }
 
     // Use Babel's VISITOR_KEYS to traverse only actual node properties
@@ -209,86 +183,22 @@ export function validateSourceLocations(
     collectGeneratedLocations(outlined.fn.body);
   }
 
-  /*
-   * Step 3: Validate that all important locations are preserved
-   * For certain node types, also validate that the node type matches
-   */
-  const strictNodeTypes = new Set([
-    'VariableDeclaration',
-    'VariableDeclarator',
-    'Identifier',
-  ]);
-
-  const reportMissingLocation = (
-    loc: t.SourceLocation,
-    nodeType: string,
-  ): void => {
-    errors.pushDiagnostic(
-      CompilerDiagnostic.create({
-        category: ErrorCategory.Todo,
-        reason: 'Important source location missing in generated code',
-        description:
-          `Source location for ${nodeType} is missing in the generated output. This can cause coverage instrumentation ` +
-          `to fail to track this code properly, resulting in inaccurate coverage reports.`,
-      }).withDetails({
-        kind: 'error',
-        loc,
-        message: null,
-      }),
-    );
-  };
-
-  const reportWrongNodeType = (
-    loc: t.SourceLocation,
-    expectedType: string,
-    actualTypes: Set<string>,
-  ): void => {
-    errors.pushDiagnostic(
-      CompilerDiagnostic.create({
-        category: ErrorCategory.Todo,
-        reason:
-          'Important source location has wrong node type in generated code',
-        description:
-          `Source location for ${expectedType} exists in the generated output but with wrong node type(s): ${Array.from(actualTypes).join(', ')}. ` +
-          `This can cause coverage instrumentation to fail to track this code properly, resulting in inaccurate coverage reports.`,
-      }).withDetails({
-        kind: 'error',
-        loc,
-        message: null,
-      }),
-    );
-  };
-
-  for (const [key, {loc, nodeTypes}] of importantOriginalLocations) {
-    const generatedNodeTypes = generatedLocations.get(key);
-
-    if (!generatedNodeTypes) {
-      // Location is completely missing
-      reportMissingLocation(loc, Array.from(nodeTypes).join(', '));
-    } else {
-      // Location exists, check each node type
-      for (const nodeType of nodeTypes) {
-        if (
-          strictNodeTypes.has(nodeType) &&
-          !generatedNodeTypes.has(nodeType)
-        ) {
-          /*
-           * For strict node types, the specific node type must be present
-           * Check if any generated node type is also an important original node type
-           */
-          const hasValidNodeType = Array.from(generatedNodeTypes).some(
-            genType => nodeTypes.has(genType),
-          );
-
-          if (hasValidNodeType) {
-            // At least one generated node type is valid (also in original), so this is just missing
-            reportMissingLocation(loc, nodeType);
-          } else {
-            // None of the generated node types are in original - this is wrong node type
-            reportWrongNodeType(loc, nodeType, generatedNodeTypes);
-          }
-        }
-      }
+  // Step 3: Validate that all important locations are preserved
+  for (const [key, {loc, nodeType}] of importantOriginalLocations) {
+    if (!generatedLocations.has(key)) {
+      errors.pushDiagnostic(
+        CompilerDiagnostic.create({
+          category: ErrorCategory.Todo,
+          reason: 'Important source location missing in generated code',
+          description:
+            `Source location for ${nodeType} is missing in the generated output. This can cause coverage instrumentation ` +
+            `to fail to track this code properly, resulting in inaccurate coverage reports.`,
+        }).withDetails({
+          kind: 'error',
+          loc,
+          message: null,
+        }),
+      );
     }
   }
 

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/alias-capture-in-method-receiver-and-mutate.expect.md

@@ -35,8 +35,10 @@ function Component() {
   let t0;
   if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
     const a = makeObject_Primitives();
+
     const x = [];
     x.push(a);
+
     mutate(x);
     t0 = [x, a];
     $[0] = t0;

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/alias-capture-in-method-receiver.expect.md

@@ -33,6 +33,7 @@ function Component() {
   if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
     const x = [];
     x.push(a);
+
     t1 = [x, a];
     $[1] = t1;
   } else {

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/aliased-nested-scope-fn-expr.expect.md

@@ -85,10 +85,14 @@ function Component(t0) {
   let t1;
   if ($[0] !== prop) {
     const obj = shallowCopy(prop);
+
     const aliasedObj = identity(obj);
+
     const getId = () => obj.id;
+
     mutate(aliasedObj);
     setPropertyByKey(aliasedObj, "id", prop.id + 1);
+
     t1 = <Stringify getId={getId} shouldInvokeFns={true} />;
     $[0] = prop;
     $[1] = t1;

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/aliased-nested-scope-truncated-dep.expect.md

@@ -181,9 +181,12 @@ function Component(t0) {
   if ($[0] !== prop) {
     const obj = shallowCopy(prop);
     const aliasedObj = identity(obj);
+
     const id = [obj.id];
+
     mutate(aliasedObj);
     setPropertyByKey(aliasedObj, "id", prop.id + 1);
+
     t1 = <Stringify id={id} />;
     $[0] = prop;
     $[1] = t1;

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/align-scopes-within-nested-valueblock-in-array.expect.md

@@ -54,6 +54,7 @@ function Foo(t0) {
   let t1;
   if ($[0] !== cond1 || $[1] !== cond2) {
     const arr = makeArray({ a: 2 }, 2, []);
+
     t1 = cond1 ? (
       <>
         <div>{identity("foo")}</div>