[BUGFIX release] local variable shadowing assert

Previously we considered the block params to shadow the params, hash,
attributes and named arguments on the same block/element, which was
incorrect.

For example:

```hbs
{{#let (concat "foo" "bar") as |concat|}}
  ...
{{/let}}
```

In this case the assertion code incorrectly believed the `concat` in
the sub-expression invocation was being shadowed. This is now fixed.

Also fixes a bug where we incorrectly transformed mustaches in attribute
positions:

```hbs
{{#let ... as |foo|}}
  <div class={{foo bar}} />
{{/let}}
```

...became...

```hbs
{{#let ... as |foo|}}
  <div class={{component foo bar}} />
{{/let}}
```

This is clearly incorrect and has been fixed here as well.

Fixes #17370

packages/ember-template-compiler/lib/plugins/assert-local-variable-shadowing-helper-invocation.ts

@@ -12,9 +12,9 @@ export default function assertLocalVariableShadowingHelperInvocation(
     name: 'assert-local-variable-shadowing-helper-invocation',
 
     visitor: {
-      BlockStatement: {
-        enter(node: AST.BlockStatement) {
-          locals.push(node.program.blockParams);
+      Program: {
+        enter(node: AST.Program) {
+          locals.push(node.blockParams);
         },
 
         exit() {
@@ -23,51 +23,70 @@ export default function assertLocalVariableShadowingHelperInvocation(
       },
 
       ElementNode: {
-        enter(node: AST.ElementNode) {
-          locals.push(node.blockParams);
-        },
+        keys: {
+          children: {
+            enter(node: AST.ElementNode) {
+              locals.push(node.blockParams);
+            },
 
-        exit() {
-          locals.pop();
+            exit() {
+              locals.pop();
+            },
+          },
         },
       },
 
+      MustacheStatement(node: AST.MustacheStatement) {
+        if (isPath(node.path) && hasArguments(node)) {
+          let name = node.path.parts[0];
+          let type = 'helper';
+
+          assert(
+            `${messageFor(name, type)} ${calculateLocationDisplay(moduleName, node.loc)}`,
+            !isLocalVariable(node.path, locals)
+          );
+        }
+      },
+
       SubExpression(node: AST.SubExpression) {
+        let name = node.path.parts[0];
+        let type = 'helper';
+
         assert(
-          `${messageFor(node)} ${calculateLocationDisplay(moduleName, node.loc)}`,
+          `${messageFor(name, type)} ${calculateLocationDisplay(moduleName, node.loc)}`,
           !isLocalVariable(node.path, locals)
         );
       },
 
       ElementModifierStatement(node: AST.ElementModifierStatement) {
-        // The ElementNode get visited first, but modifiers are more of a sibling
-        // than a child in the lexical scope (we aren't evaluated in its "block")
-        // so any locals introduced by the last element doesn't count
+        let name = node.path.parts[0];
+        let type = 'modifier';
+
         assert(
-          `${messageFor(node)} ${calculateLocationDisplay(moduleName, node.loc)}`,
-          !isLocalVariable(node.path, locals.slice(0, -1))
+          `${messageFor(name, type)} ${calculateLocationDisplay(moduleName, node.loc)}`,
+          !isLocalVariable(node.path, locals)
         );
       },
     },
   };
 }
 
 function isLocalVariable(node: AST.PathExpression, locals: string[][]): boolean {
-  return !node.this && hasLocalVariable(node.parts[0], locals);
+  return !node.this && node.parts.length === 1 && hasLocalVariable(node.parts[0], locals);
 }
 
 function hasLocalVariable(name: string, locals: string[][]): boolean {
   return locals.some(names => names.indexOf(name) !== -1);
 }
 
-function messageFor(node: AST.SubExpression | AST.ElementModifierStatement): string {
-  let type = isSubExpression(node) ? 'helper' : 'modifier';
-  let name = node.path.parts[0];
+function messageFor(name: string, type: string): string {
   return `Cannot invoke the \`${name}\` ${type} because it was shadowed by a local variable (i.e. a block param) with the same name. Please rename the local variable to resolve the conflict.`;
 }
 
-function isSubExpression(
-  node: AST.SubExpression | AST.ElementModifierStatement
-): node is AST.SubExpression {
-  return node.type === 'SubExpression';
+function isPath(node: AST.Node): node is AST.PathExpression {
+  return node.type === 'PathExpression';
+}
+
+function hasArguments(node: AST.MustacheStatement): boolean {
+  return node.params.length > 0 || node.hash.pairs.length > 0;
 }

packages/ember-template-compiler/lib/plugins/transform-component-invocation.ts

@@ -124,13 +124,14 @@ export default function transformComponentInvocation(env: ASTPluginEnvironment):
   let { moduleName } = env.meta;
   let { builders: b } = env.syntax;
   let locals: string[][] = [];
+  let isAttrs = false;
 
   return {
     name: 'transform-component-invocation',
 
     visitor: {
-      ElementNode: {
-        enter(node: AST.ElementNode) {
+      Program: {
+        enter(node: AST.Program) {
           locals.push(node.blockParams);
         },
 
@@ -139,23 +140,39 @@ export default function transformComponentInvocation(env: ASTPluginEnvironment):
         },
       },
 
-      BlockStatement: {
-        enter(node: AST.BlockStatement) {
-          // check this before we push the new locals
-          if (isBlockInvocation(node, locals)) {
-            wrapInComponent(moduleName, node, b);
-          }
-
-          locals.push(node.program.blockParams);
+      ElementNode: {
+        keys: {
+          attributes: {
+            enter() {
+              isAttrs = true;
+            },
+
+            exit() {
+              isAttrs = false;
+            },
+          },
+
+          children: {
+            enter(node: AST.ElementNode) {
+              locals.push(node.blockParams);
+            },
+
+            exit() {
+              locals.pop();
+            },
+          },
         },
+      },
 
-        exit() {
-          locals.pop();
-        },
+      BlockStatement(node: AST.BlockStatement) {
+        // check this before we push the new locals
+        if (isBlockInvocation(node, locals)) {
+          wrapInComponent(moduleName, node, b);
+        }
       },
 
       MustacheStatement(node: AST.MustacheStatement): AST.Node | void {
-        if (isInlineInvocation(node, locals)) {
+        if (!isAttrs && isInlineInvocation(node, locals)) {
           wrapInComponent(moduleName, node, b);
         }
       },