propagate declaration locations through HIR pipeline for hoisted variables

Author: nathanmarks

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

@@ -931,7 +931,7 @@ function lowerStatement(
                   kind: InstructionKind.Let,
                   place,
                 },
-                loc: id.node.loc ?? GeneratedSource,
+                loc: stmt.node.loc ?? GeneratedSource,
               });
             } else {
               const typeAnnotation = id.get('typeAnnotation');
@@ -952,7 +952,7 @@ function lowerStatement(
                   place,
                 },
                 type,
-                loc: id.node.loc ?? GeneratedSource,
+                loc: stmt.node.loc ?? GeneratedSource,
               });
             }
           }

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

@@ -1649,6 +1649,7 @@ export type ReactiveScopeDependencies = Set<ReactiveScopeDependency>;
 export type ReactiveScopeDeclaration = {
   identifier: Identifier;
   scope: ReactiveScope; // the scope in which the variable was originally declared
+  loc: SourceLocation;
 };
 
 const opaquePropertyLiteral = Symbol();

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

@@ -28,6 +28,7 @@ import {
   LoadContext,
   TInstruction,
   FunctionExpression,
+  SourceLocation,
   ObjectMethod,
   PropertyLiteral,
   convertHoistedLValueKind,
@@ -393,6 +394,7 @@ function getProperty(
 type Decl = {
   id: InstructionId;
   scope: Stack<ReactiveScope>;
+  loc: SourceLocation;
 };
 
 export class DependencyCollectionContext {
@@ -602,6 +604,7 @@ export class DependencyCollectionContext {
           scope.declarations.set(maybeDependency.identifier.id, {
             identifier: maybeDependency.identifier,
             scope: originalDeclaration.scope.value!,
+            loc: originalDeclaration.loc,
           });
         }
       });
@@ -686,6 +689,7 @@ export function handleInstruction(
   context.declare(lvalue.identifier, {
     id,
     scope: context.currentScope,
+    loc: instr.loc,
   });
   if (
     context.isDeferredDependency({kind: HIRValue.Instruction, value: instr})
@@ -702,6 +706,7 @@ export function handleInstruction(
     context.declare(value.lvalue.place.identifier, {
       id,
       scope: context.currentScope,
+      loc: instr.loc,
     });
   } else if (value.kind === 'DeclareLocal' || value.kind === 'DeclareContext') {
     /*
@@ -718,6 +723,7 @@ export function handleInstruction(
       context.declare(value.lvalue.place.identifier, {
         id,
         scope: context.currentScope,
+        loc: instr.loc,
       });
     }
   } else if (value.kind === 'Destructure') {
@@ -729,6 +735,7 @@ export function handleInstruction(
       context.declare(place.identifier, {
         id,
         scope: context.currentScope,
+        loc: instr.loc,
       });
     }
   } else if (value.kind === 'StoreContext') {
@@ -745,6 +752,7 @@ export function handleInstruction(
       context.declare(value.lvalue.place.identifier, {
         id,
         scope: context.currentScope,
+        loc: instr.loc,
       });
     }
 
@@ -775,11 +783,13 @@ function collectDependencies(
       context.declare(param.identifier, {
         id: makeInstructionId(0),
         scope: empty(),
+        loc: param.identifier.loc,
       });
     } else {
       context.declare(param.place.identifier, {
         id: makeInstructionId(0),
         scope: empty(),
+        loc: param.place.loc,
       });
     }
   }
@@ -812,6 +822,7 @@ function collectDependencies(
           context.declare(instr.lvalue.identifier, {
             id: instr.id,
             scope: context.currentScope,
+            loc: instr.loc,
           });
           /**
            * Recursively visit the inner function to extract dependencies there

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

@@ -628,6 +628,7 @@ function inferDependencies(
     context.declare(dep.identifier, {
       id: makeInstructionId(0),
       scope: empty(),
+      loc: dep.identifier.loc,
     });
   }
   const placeholderScope: ReactiveScope = {
@@ -694,6 +695,7 @@ function inferDependenciesInFn(
         context.declare(instr.lvalue.identifier, {
           id: instr.id,
           scope: context.currentScope,
+          loc: instr.loc,
         });
         /**
          * Recursively visit the inner function to extract dependencies

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

@@ -411,6 +411,7 @@ export function inlineJsxTransform(
           scope.declarations.set(decl.identifier.id, {
             identifier: newDecl,
             scope: decl.scope,
+            loc: decl.loc,
           });
         }
       }

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

@@ -675,8 +675,8 @@ function codegenReactiveScope(
   }
   let firstOutputIndex: number | null = null;
 
-  for (const [, {identifier}] of [...scope.declarations].sort(([, a], [, b]) =>
-    compareScopeDeclaration(a, b),
+  for (const [, {identifier, loc}] of [...scope.declarations].sort(
+    ([, a], [, b]) => compareScopeDeclaration(a, b),
   )) {
     const index = cx.nextCacheIndex;
     if (firstOutputIndex === null) {
@@ -702,7 +702,9 @@ function codegenReactiveScope(
     outputComments.push(name.name);
     if (!cx.hasDeclared(identifier)) {
       statements.push(
-        t.variableDeclaration('let', [createVariableDeclarator(name, null)]),
+        createVariableDeclaration(loc, 'let', [
+          createVariableDeclarator(name, null),
+        ]),
       );
     }
     cacheLoads.push({name, index, value: wrapCacheDep(cx, name)});

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

@@ -156,6 +156,7 @@ class Transform extends ReactiveFunctionTransform<State> {
         scopeBlock.scope.declarations.set(earlyReturnValue.value.id, {
           identifier: earlyReturnValue.value,
           scope: scopeBlock.scope,
+          loc: earlyReturnValue.loc,
         });
 
         const instructions = scopeBlock.instructions;

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-missing-source-locations.expect.md

@@ -52,7 +52,7 @@ function Component({prop1, prop2}) {
 ## Error
 
 ```
-Found 25 errors:
+Found 23 errors:
 
 Todo: Important source location missing in generated code
 
@@ -69,19 +69,6 @@ error.todo-missing-source-locations.ts:4:9
 
 Todo: Important source location missing in generated code
 
-Source location for VariableDeclaration is missing in the generated output. This can cause coverage instrumentation to fail to track this code properly, resulting in inaccurate coverage reports..
-
-error.todo-missing-source-locations.ts:9:2
-   7 |   const arr = [x, y];
-   8 |   const obj = {x, y};
->  9 |   let destA, destB;
-     |   ^^^^^^^^^^^^^^^^^
-  10 |   if (y > 5) {
-  11 |     [destA, destB] = arr;
-  12 |   }
-
-Todo: Important source location missing in generated code
-
 Source location for ExpressionStatement is missing in the generated output. This can cause coverage instrumentation to fail to track this code properly, resulting in inaccurate coverage reports..
 
 error.todo-missing-source-locations.ts:11:4
@@ -121,19 +108,6 @@ error.todo-missing-source-locations.ts:15:15
 
 Todo: Important source location missing in generated code
 
-Source location for VariableDeclaration is missing in the generated output. This can cause coverage instrumentation to fail to track this code properly, resulting in inaccurate coverage reports..
-
-error.todo-missing-source-locations.ts:16:2
-  14 |   const [a, b] = arr;
-  15 |   const {x: c, y: d} = obj;
-> 16 |   let sound;
-     |   ^^^^^^^^^^
-  17 |
-  18 |   if (y > 10) {
-  19 |     sound = 'woof';
-
-Todo: Important source location missing in generated code
-
 Source location for ExpressionStatement is missing in the generated output. This can cause coverage instrumentation to fail to track this code properly, resulting in inaccurate coverage reports..
 
 error.todo-missing-source-locations.ts:19:4