Merge pull request #20782 from emberjs/bugfix/keyword-shadowing

ef4 · web-flow · commit 5557b11deb8d · 2024-10-23T10:28:40.000-04:00
Fixes Ember keyword shadowing
diff --git a/packages/@ember/-internals/glimmer/tests/integration/helpers/array-test.js b/packages/@ember/-internals/glimmer/tests/integration/helpers/array-test.js
@@ -1,4 +1,10 @@
-import { RenderingTestCase, moduleFor, strip, runTask } from 'internal-test-helpers';
+import {
+  RenderingTestCase,
+  defineComponent,
+  moduleFor,
+  runTask,
+  strip,
+} from 'internal-test-helpers';
 
 import { set } from '@ember/object';
 
@@ -20,6 +26,21 @@ moduleFor(
       this.assertStableRerender();
     }
 
+    ['@test the array helper can be shadowed']() {
+      function array(...list) {
+        return list.map((n) => n * 2);
+      }
+
+      let First = defineComponent({ array }, `{{#each (array 1 2 3) as |n|}}[{{n}}]{{/each}}`);
+
+      let Root = defineComponent(
+        { shadowArray: array, First },
+        `{{#let shadowArray as |array|}}{{#each (array 5 10 15) as |n|}}[{{n}}]{{/each}}{{/let}}<First />`
+      );
+
+      this.renderComponent(Root, { expect: '[10][20][30][2][4][6]' });
+    }
+
     ['@test can have more than one value']() {
       this.render(strip`
       {{#let (array "Sergio" "Robert") as |people|}}
diff --git a/packages/@ember/-internals/glimmer/tests/integration/helpers/fn-test.js b/packages/@ember/-internals/glimmer/tests/integration/helpers/fn-test.js
@@ -1,6 +1,6 @@
 import { set } from '@ember/object';
 import { DEBUG } from '@glimmer/env';
-import { RenderingTestCase, moduleFor, runTask } from 'internal-test-helpers';
+import { RenderingTestCase, defineComponent, moduleFor, runTask } from 'internal-test-helpers';
 import { Component } from '../../utils/helpers';
 
 moduleFor(
@@ -22,6 +22,14 @@ moduleFor(
       });
     }
 
+    '@test fn can be shadowed'() {
+      let First = defineComponent(
+        { fn: boundFn, boundFn, id, invoke },
+        `[{{invoke (fn id 1)}}]{{#let boundFn as |fn|}}[{{invoke (fn id 2)}}{{/let}}]`
+      );
+      this.renderComponent(First, { expect: `[bound:1][bound:2]` });
+    }
+
     '@test updates when arguments change'() {
       this.render(`{{invoke (fn this.myFunc this.arg1 this.arg2)}}`, {
         myFunc(arg1, arg2) {
@@ -209,3 +217,13 @@ moduleFor(
     }
   }
 );
+
+function invoke(fn) {
+  return fn();
+}
+
+function boundFn(fn, ...args) {
+  return () => fn(...args.map((arg) => `bound:${arg}`));
+}
+
+let id = (arg) => arg;
diff --git a/packages/@ember/-internals/glimmer/tests/integration/helpers/get-test.js b/packages/@ember/-internals/glimmer/tests/integration/helpers/get-test.js
@@ -1,4 +1,4 @@
-import { RenderingTestCase, moduleFor, runTask } from 'internal-test-helpers';
+import { RenderingTestCase, defineComponent, moduleFor, runTask } from 'internal-test-helpers';
 
 import { set, get } from '@ember/object';
 
@@ -32,6 +32,17 @@ moduleFor(
       this.assertText('[red] [red]');
     }
 
+    ['@test can be shadowed']() {
+      let get = (obj, key) => `obj.${key}=${obj[key]}`;
+      let obj = { apple: 'red', banana: 'yellow' };
+      let Root = defineComponent(
+        { get, outerGet: get, obj },
+        `[{{get obj 'apple'}}][{{#let outerGet as |get|}}{{get obj 'banana'}}{{/let}}]`
+      );
+
+      this.renderComponent(Root, { expect: '[obj.apple=red][obj.banana=yellow]' });
+    }
+
     ['@test should be able to get an object value with nested static key']() {
       this.render(
         `[{{get this.colors "apple.gala"}}] [{{if true (get this.colors "apple.gala")}}]`,
diff --git a/packages/@ember/-internals/glimmer/tests/integration/helpers/hash-test.js b/packages/@ember/-internals/glimmer/tests/integration/helpers/hash-test.js
@@ -1,4 +1,4 @@
-import { RenderingTestCase, moduleFor, runTask } from 'internal-test-helpers';
+import { RenderingTestCase, defineComponent, moduleFor, runTask } from 'internal-test-helpers';
 
 import { Component } from '../../utils/helpers';
 
@@ -17,6 +17,21 @@ moduleFor(
       this.assertText('Sergio');
     }
 
+    ['@test can be shadowed']() {
+      let hash = (obj) =>
+        Object.entries(obj)
+          .map(([key, value]) => `hash:${key}=${value}`)
+          .join(',');
+      let Root = defineComponent(
+        { hash, shadowHash: hash },
+        `({{hash apple='red' banana='yellow'}}) ({{#let shadowHash as |hash|}}{{hash apple='green'}}{{/let}})`
+      );
+
+      this.renderComponent(Root, {
+        expect: '(hash:apple=red,hash:banana=yellow) (hash:apple=green)',
+      });
+    }
+
     ['@test can have more than one key-value']() {
       this.render(
         `{{#let (hash name="Sergio" lastName="Arbeo") as |person|}}{{person.name}} {{person.lastName}}{{/let}}`
diff --git a/packages/ember-template-compiler/lib/plugins/assert-against-attrs.ts b/packages/ember-template-compiler/lib/plugins/assert-against-attrs.ts
@@ -2,6 +2,7 @@ import { assert, deprecate } from '@ember/debug';
 import type { AST, ASTPlugin } from '@glimmer/syntax';
 import calculateLocationDisplay from '../system/calculate-location-display';
 import type { EmberASTPluginEnvironment } from '../types';
+import { trackLocals } from './utils';
 
 /**
  @module ember
@@ -23,48 +24,16 @@ import type { EmberASTPluginEnvironment } from '../types';
 export default function assertAgainstAttrs(env: EmberASTPluginEnvironment): ASTPlugin {
   let { builders: b } = env.syntax;
   let moduleName = env.meta?.moduleName;
-
-  let stack: string[][] = [[]];
-
-  function updateBlockParamsStack(blockParams: string[]) {
-    let parent = stack[stack.length - 1];
-    assert('has parent', parent);
-    stack.push(parent.concat(blockParams));
-  }
+  let { hasLocal, visitor } = trackLocals(env);
 
   return {
     name: 'assert-against-attrs',
 
     visitor: {
-      Template: {
-        enter(node: AST.Template) {
-          updateBlockParamsStack(node.blockParams);
-        },
-        exit() {
-          stack.pop();
-        },
-      },
-
-      Block: {
-        enter(node: AST.Block) {
-          updateBlockParamsStack(node.blockParams);
-        },
-        exit() {
-          stack.pop();
-        },
-      },
-
-      ElementNode: {
-        enter(node: AST.ElementNode) {
-          updateBlockParamsStack(node.blockParams);
-        },
-        exit() {
-          stack.pop();
-        },
-      },
+      ...visitor,
 
       PathExpression(node: AST.PathExpression): AST.Node | void {
-        if (isAttrs(node, stack[stack.length - 1]!)) {
+        if (isAttrs(node, hasLocal)) {
           assert(
             `Using {{attrs}} to reference named arguments is not supported. {{${
               node.original
@@ -106,12 +75,8 @@ export default function assertAgainstAttrs(env: EmberASTPluginEnvironment): ASTP
   };
 }
 
-function isAttrs(node: AST.PathExpression, symbols: string[]) {
-  return (
-    node.head.type === 'VarHead' &&
-    node.head.name === 'attrs' &&
-    symbols.indexOf(node.head.name) === -1
-  );
+function isAttrs(node: AST.PathExpression, hasLocal: (k: string) => boolean) {
+  return node.head.type === 'VarHead' && node.head.name === 'attrs' && !hasLocal(node.head.name);
 }
 
 function isThisDotAttrs(node: AST.PathExpression) {
diff --git a/packages/ember-template-compiler/lib/plugins/assert-against-named-outlets.ts b/packages/ember-template-compiler/lib/plugins/assert-against-named-outlets.ts
@@ -2,6 +2,7 @@ import { assert } from '@ember/debug';
 import type { AST, ASTPlugin } from '@glimmer/syntax';
 import calculateLocationDisplay from '../system/calculate-location-display';
 import type { EmberASTPluginEnvironment } from '../types';
+import { trackLocals } from './utils';
 
 /**
  @module ember
@@ -15,16 +16,19 @@ import type { EmberASTPluginEnvironment } from '../types';
 */
 export default function assertAgainstNamedOutlets(env: EmberASTPluginEnvironment): ASTPlugin {
   let moduleName = env.meta?.moduleName;
+  let { hasLocal, visitor } = trackLocals(env);
 
   return {
     name: 'assert-against-named-outlets',
 
     visitor: {
+      ...visitor,
       MustacheStatement(node: AST.MustacheStatement) {
         if (
           node.path.type === 'PathExpression' &&
           node.path.original === 'outlet' &&
-          node.params[0]
+          node.params[0] &&
+          !hasLocal('outlet')
         ) {
           let sourceInformation = calculateLocationDisplay(moduleName, node.loc);
           assert(
diff --git a/packages/ember-template-compiler/lib/plugins/assert-input-helper-without-block.ts b/packages/ember-template-compiler/lib/plugins/assert-input-helper-without-block.ts
@@ -2,16 +2,20 @@ import { assert } from '@ember/debug';
 import type { AST, ASTPlugin } from '@glimmer/syntax';
 import calculateLocationDisplay from '../system/calculate-location-display';
 import type { EmberASTPluginEnvironment } from '../types';
-import { isPath } from './utils';
+import { isPath, trackLocals } from './utils';
 
 export default function errorOnInputWithContent(env: EmberASTPluginEnvironment): ASTPlugin {
   let moduleName = env.meta?.moduleName;
+  let { hasLocal, visitor } = trackLocals(env);
 
   return {
     name: 'assert-input-helper-without-block',
 
     visitor: {
+      ...visitor,
       BlockStatement(node: AST.BlockStatement) {
+        if (hasLocal('input')) return;
+
         if (isPath(node.path) && node.path.original === 'input') {
           assert(assertMessage(moduleName, node));
         }
diff --git a/packages/ember-template-compiler/lib/plugins/transform-action-syntax.ts b/packages/ember-template-compiler/lib/plugins/transform-action-syntax.ts
@@ -1,6 +1,6 @@
 import type { AST, ASTPlugin } from '@glimmer/syntax';
 import type { Builders, EmberASTPluginEnvironment } from '../types';
-import { isPath } from './utils';
+import { isPath, trackLocals } from './utils';
 
 /**
  @module ember
@@ -27,36 +27,41 @@ import { isPath } from './utils';
   @class TransformActionSyntax
 */
 
-export default function transformActionSyntax({ syntax }: EmberASTPluginEnvironment): ASTPlugin {
-  let { builders: b } = syntax;
+export default function transformActionSyntax(env: EmberASTPluginEnvironment): ASTPlugin {
+  let { builders: b } = env.syntax;
+  let { hasLocal, visitor } = trackLocals(env);
 
   return {
     name: 'transform-action-syntax',
 
     visitor: {
+      ...visitor,
       ElementModifierStatement(node: AST.ElementModifierStatement) {
-        if (isAction(node)) {
+        if (isAction(node, hasLocal)) {
           insertThisAsFirstParam(node, b);
         }
       },
 
       MustacheStatement(node: AST.MustacheStatement) {
-        if (isAction(node)) {
+        if (isAction(node, hasLocal)) {
           insertThisAsFirstParam(node, b);
         }
       },
 
       SubExpression(node: AST.SubExpression) {
-        if (isAction(node)) {
+        if (isAction(node, hasLocal)) {
           insertThisAsFirstParam(node, b);
         }
       },
     },
   };
 }
 
-function isAction(node: AST.ElementModifierStatement | AST.MustacheStatement | AST.SubExpression) {
-  return isPath(node.path) && node.path.original === 'action';
+function isAction(
+  node: AST.ElementModifierStatement | AST.MustacheStatement | AST.SubExpression,
+  hasLocal: (k: string) => boolean
+) {
+  return isPath(node.path) && node.path.original === 'action' && !hasLocal('action');
 }
 
 function insertThisAsFirstParam(
diff --git a/packages/ember-template-compiler/lib/plugins/transform-each-track-array.ts b/packages/ember-template-compiler/lib/plugins/transform-each-track-array.ts
@@ -1,7 +1,7 @@
 import type { AST, ASTPlugin } from '@glimmer/syntax';
 import { assert } from '@ember/debug';
 import type { EmberASTPluginEnvironment } from '../types';
-import { isPath } from './utils';
+import { isPath, trackLocals } from './utils';
 
 /**
  @module ember
@@ -25,13 +25,15 @@ import { isPath } from './utils';
 */
 export default function transformEachTrackArray(env: EmberASTPluginEnvironment): ASTPlugin {
   let { builders: b } = env.syntax;
+  let { hasLocal, visitor } = trackLocals(env);
 
   return {
     name: 'transform-each-track-array',
 
     visitor: {
+      ...visitor,
       BlockStatement(node: AST.BlockStatement): AST.Node | void {
-        if (isPath(node.path) && node.path.original === 'each') {
+        if (isPath(node.path) && node.path.original === 'each' && !hasLocal('each')) {
           let firstParam = node.params[0];
           assert('has firstParam', firstParam);
 
diff --git a/packages/ember-template-compiler/lib/plugins/transform-resolutions.ts b/packages/ember-template-compiler/lib/plugins/transform-resolutions.ts
@@ -67,7 +67,7 @@ const TARGETS = Object.freeze(['helper', 'modifier']);
 export default function transformResolutions(env: EmberASTPluginEnvironment): ASTPlugin {
   let { builders: b } = env.syntax;
   let moduleName = env.meta?.moduleName;
-  let { hasLocal, node: tracker } = trackLocals();
+  let { hasLocal, node: tracker } = trackLocals(env);
   let seen: Set<AST.Node> | undefined;
 
   return {
diff --git a/packages/ember-template-compiler/lib/plugins/transform-wrap-mount-and-outlet.ts b/packages/ember-template-compiler/lib/plugins/transform-wrap-mount-and-outlet.ts
@@ -37,15 +37,13 @@ import { isPath, trackLocals } from './utils';
 export default function transformWrapMountAndOutlet(env: EmberASTPluginEnvironment): ASTPlugin {
   let { builders: b } = env.syntax;
 
-  let { hasLocal, node } = trackLocals();
+  let { hasLocal, visitor } = trackLocals(env);
 
   return {
     name: 'transform-wrap-mount-and-outlet',
 
     visitor: {
-      Template: node,
-      ElementNode: node,
-      Block: node,
+      ...visitor,
 
       MustacheStatement(node: AST.MustacheStatement): AST.Node | void {
         if (
diff --git a/packages/ember-template-compiler/lib/plugins/utils.ts b/packages/ember-template-compiler/lib/plugins/utils.ts
@@ -1,4 +1,5 @@
 import type { AST } from '@glimmer/syntax';
+import type { EmberASTPluginEnvironment } from '../types';
 
 export function isPath(node: AST.Node): node is AST.PathExpression {
   return node.type === 'PathExpression';
@@ -20,7 +21,11 @@ function getLocalName(node: string | AST.VarHead): string {
   }
 }
 
-export function trackLocals() {
+export function inScope(env: EmberASTPluginEnvironment, name: string): boolean {
+  return Boolean(env.lexicalScope?.(name));
+}
+
+export function trackLocals(env: EmberASTPluginEnvironment) {
   let locals = new Map();
 
   let node = {
@@ -49,7 +54,12 @@ export function trackLocals() {
   };
 
   return {
-    hasLocal: (key: string) => locals.has(key),
+    hasLocal: (key: string) => locals.has(key) || inScope(env, key),
     node,
+    visitor: {
+      Template: node,
+      ElementNode: node,
+      Block: node,
+    },
   };
 }
diff --git a/packages/ember-template-compiler/lib/types.ts b/packages/ember-template-compiler/lib/types.ts
@@ -23,6 +23,7 @@ export interface EmberPrecompileOptions extends PrecompileOptions {
   isProduction?: boolean;
   moduleName?: string;
   plugins?: Plugins;
+  lexicalScope?: (name: string) => boolean;
 }
 
 export type EmberASTPluginEnvironment = ASTPluginEnvironment & EmberPrecompileOptions;
diff --git a/packages/ember-template-compiler/tests/plugins/assert-against-attrs-test.js b/packages/ember-template-compiler/tests/plugins/assert-against-attrs-test.js
diff --git a/packages/ember-template-compiler/tests/plugins/assert-array-test.js b/packages/ember-template-compiler/tests/plugins/assert-array-test.js
diff --git a/packages/ember-template-compiler/tests/plugins/assert-input-helper-without-block-test.js b/packages/ember-template-compiler/tests/plugins/assert-input-helper-without-block-test.js
diff --git a/packages/internal-test-helpers/lib/test-cases/rendering.ts b/packages/internal-test-helpers/lib/test-cases/rendering.ts
