Share options.locals by reference so scope.crawl mutations are visible

This fixes two smoke-test failures from commit 571a94d:

  {{fn}} as keyword (but it is shadowed): it works
  {{on}} as keyword (but it is shadowed): it works

These `.gjs` tests declare a JS-level lexical (`const fn = ...` or
`const on = ...`) and expect the template reference to resolve to the
shadowing local, not the built-in keyword. They passed on main but
regressed on the PR because of how `preprocess` wired
`template.blockParams`.

## Root cause

`babel-plugin-ember-template-compilation` passes `options.locals` as a
live `readOnlyArray` proxy over its internal `ScopeLocals` array. The
plugin order at precompile time is, roughly:

    [embroider macros, scope.crawl, AutoImportBuiltins, ...other ember
     strict-mode transforms]

`scope.crawl` walks the template, detects `PathExpression` upvars that
have matching JS-level bindings (via `astNodeHasBinding` and Babel scope
lookup), and calls `scope.add(name)` for each — mutating the underlying
locals array. When it runs *before* `AutoImportBuiltins`, its mutations
need to be visible through `template.blockParams` so that
`AutoImportBuiltins`' `trackLocals.hasLocal('fn')` returns `true` for a
shadowed lexical and skips the rewrite.

Main achieves this by passing `options.locals` directly to
`b.template({ blockParams: options.locals, ... })` in
`handlebars-node-visitors.ts`. The assignment is by reference: the
Template literally points at the proxy, and later reads see the latest
state of the underlying array in real time.

The PR's preprocess was instead doing:

    template.blockParams = [...options.locals];

That takes a *snapshot* of the proxy at preprocess-start time (when it
is still empty). When `scope.crawl` later mutates the underlying array,
`template.blockParams` stays frozen at `[]`. `AutoImportBuiltins` then
sees an empty locals table, fails to detect the shadow, calls
`jsutils.bindImport('fn')` which — because the JS scope has `const fn`
— imports `@ember/helper#fn` under a renamed local like `fn0`, and
rewrites `node.path.original = 'fn0'`. The `buildLegacyPath` setter
rebuilds `head` to `VarHead{ name: 'fn0' }`, the v2 normalizer resolves
`fn0` as a lexical, and the runtime ends up calling the imported helper
instead of the shadowing `const fn`. Hence the failure.

## Fix

Assign `options.locals` to `template.blockParams` by reference, matching
main's behavior. The post-plugin "refresh" block is no longer needed
since the live proxy already tracks mutations.

Also drops the `parts` accessor cast-free behavior: the narrowing uses
`if (options.locals)` rather than a length check, so an empty array
still points at the proxy and picks up later mutations.

## Verified

- Smoke tests: `{{fn}}/{{on}} as keyword (but it is shadowed)` — pass in
  all 3 classic-basics scenarios (19/19 tests, 0 fail × 3).
- Browser test suite: 9138/9138 pass.
- Lint, prettier, build:types, type-check:internals — all clean.
- `auto-import-builtins.ts` unchanged from main.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

Author: NullVoxPopuli

packages/@ember/template-compiler/lib/plugins/auto-import-builtins.ts

@@ -51,24 +51,7 @@ function rewriteKeyword(
       name,
     });
   } else if (env.meta?.emberRuntime) {
-    const rewritten = env.meta.emberRuntime.lookupKeyword(name);
-    node.path.original = rewritten;
-    // Also update head and tail so the v2 normalizer resolves correctly.
-    // e.g., "__ember_keywords__.on" → head="__ember_keywords__", tail=["on"]
-    // (The Peggy grammar emits PathExpression nodes as plain objects where
-    // `original`, `head`, and `tail` are independent data properties, so we
-    // need to keep them in sync by hand here.)
-    const parts = rewritten.split('.');
-    if (parts.length >= 2) {
-      const headName = parts[0] as string;
-      node.path.head = {
-        type: 'VarHead',
-        name: headName,
-        original: headName,
-        loc: node.path.head.loc,
-      } as AST.VarHead;
-      node.path.tail = parts.slice(1);
-    }
+    node.path.original = env.meta.emberRuntime.lookupKeyword(name);
   }
 }
 

packages/@ember/template-compiler/lib/plugins/utils.ts

@@ -1,12 +1,12 @@
 import type { AST } from '@glimmer/syntax';
 import type { EmberASTPluginEnvironment } from '../types';
 
-export function isPath(node: AST.Node | null | undefined): node is AST.PathExpression {
-  return node != null && node.type === 'PathExpression';
+export function isPath(node: AST.Node): node is AST.PathExpression {
+  return node.type === 'PathExpression';
 }
 
-export function isSubExpression(node: AST.Node | null | undefined): node is AST.SubExpression {
-  return node != null && node.type === 'SubExpression';
+export function isSubExpression(node: AST.Node): node is AST.SubExpression {
+  return node.type === 'SubExpression';
 }
 
 export function isStringLiteral(node: AST.Expression): node is AST.StringLiteral {

packages/@glimmer/syntax/lib/parser/tokenizer-event-handlers.ts

@@ -11,6 +11,7 @@ import * as src from '../source/api';
 import { generateSyntaxError } from '../syntax-error';
 import traverse from '../traversal/traverse';
 import Walker from '../traversal/walker';
+import { buildLegacyPath } from '../v1/legacy-interop';
 import publicBuilder from '../v1/public-builders';
 
 // ============================================================================
@@ -472,6 +473,8 @@ function isPeggySpan(
 
 /**
  * Recursively convert plain {start, end} location objects to SourceSpan instances.
+ * PathExpression nodes are upgraded via buildLegacyPath so that setting
+ * `node.original` propagates to `head`/`tail`.
  */
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
 function convertLocations(node: any, source: src.Source): any {
@@ -485,6 +488,7 @@ function convertLocations(node: any, source: src.Source): any {
   let result: any = {};
 
   for (let key of Object.keys(node)) {
+    if (node.type === 'PathExpression' && key === 'original') continue;
     let val = node[key];
     if ((key === 'loc' || key === 'openTag' || key === 'closeTag') && isPeggySpan(val)) {
       result[key] = peggySpanToSourceSpan(source, val);
@@ -495,6 +499,14 @@ function convertLocations(node: any, source: src.Source): any {
     }
   }
 
+  if (result.type === 'PathExpression') {
+    return buildLegacyPath({
+      head: result.head,
+      tail: result.tail,
+      loc: result.loc,
+    });
+  }
+
   return result;
 }
 
@@ -730,13 +742,20 @@ export function preprocess(
     stripStandalone(template.body);
   }
 
-  // Pre-seed template.blockParams from options.locals so that AST plugins
-  // (e.g. transform-each-track-array) can see lexical scope variables via
-  // trackLocals. The scope crawl plugin from babel-plugin-ember-template-
-  // compilation may add more locals during execution, so we refresh again
-  // after all plugins have run.
-  if (options.locals && options.locals.length > 0) {
-    template.blockParams = [...options.locals];
+  // Point template.blockParams at options.locals BY REFERENCE, not by copy.
+  //
+  // babel-plugin-ember-template-compilation passes `options.locals` as a
+  // readOnlyArray proxy over its internal ScopeLocals array. As plugins run
+  // (notably `scope.crawl` from the babel plugin), they mutate that underlying
+  // array via `scope.add(name)`. Because the proxy is live, template.blockParams
+  // automatically reflects those mutations — so by the time later plugins like
+  // auto-import-builtins read `template.blockParams` in their trackLocals pass,
+  // they correctly see the lexical scope variables that were discovered by
+  // earlier plugins. If we copy here (e.g. `[...options.locals]`), we freeze
+  // the snapshot at preprocess-start time and auto-import-builtins ends up
+  // rewriting names that should have been recognized as shadowed locals.
+  if (options.locals) {
+    template.blockParams = options.locals;
   }
 
   // Run AST plugins
@@ -748,11 +767,5 @@ export function preprocess(
     }
   }
 
-  // Refresh blockParams from options.locals after plugins run — the scope
-  // crawl plugin may have populated additional locals during execution.
-  if (options.locals && options.locals.length > 0) {
-    template.blockParams = [...options.locals];
-  }
-
   return template;
 }