Merge pull request #17137 from emberjs/assert-local-variable-shadowin…

…g-modifier-invocation

[BUGFIX beta] Assert when local variables shadow modifier invocations

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

@@ -38,6 +38,16 @@ export default function assertLocalVariableShadowingHelperInvocation(
           !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
+        assert(
+          `${messageFor(node)} ${calculateLocationDisplay(moduleName, node.loc)}`,
+          !isLocalVariable(node.path, locals.slice(0, -1))
+        );
+      },
     },
   };
 }
@@ -50,7 +60,14 @@ function hasLocalVariable(name: string, locals: string[][]): boolean {
   return locals.some(names => names.indexOf(name) !== -1);
 }
 
-function messageFor(node: AST.SubExpression): string {
+function messageFor(node: AST.SubExpression | AST.ElementModifierStatement): string {
+  let type = isSubExpression(node) ? 'helper' : 'modifier';
   let name = node.path.parts[0];
-  return `Cannot invoke the \`${name}\` helper 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.`;
+  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';
 }

packages/ember-template-compiler/tests/plugins/assert-local-variable-shadowing-helper-invocation-test.js

@@ -224,5 +224,118 @@ moduleFor(
         { moduleName: 'baz/foo-bar' }
       );
     }
+
+    [`@test block statements shadowing modifier invocations`]() {
+      expectAssertion(() => {
+        compile(
+          `
+          {{#let foo as |foo|}}
+            <div {{foo}} />
+          {{/let}}`,
+          { moduleName: 'baz/foo-bar' }
+        );
+      }, `Cannot invoke the \`foo\` modifier 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. ('baz/foo-bar' @ L3:C17) `);
+
+      expectAssertion(() => {
+        compile(
+          `
+          {{#let foo as |foo|}}
+            <div {{foo bar baz}} />
+          {{/let}}`,
+          { moduleName: 'baz/foo-bar' }
+        );
+      }, `Cannot invoke the \`foo\` modifier 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. ('baz/foo-bar' @ L3:C17) `);
+
+      // Not shadowed
+
+      compile(
+        `
+        {{#let foo as |foo|}}{{/let}}
+        <div {{foo}} />
+        <div {{foo bar baz}} />`,
+        { moduleName: 'baz/foo-bar' }
+      );
+    }
+
+    [`@test element nodes shadowing modifier invocations`]() {
+      expectAssertion(() => {
+        compile(
+          `
+          <Foo as |foo|>
+            <div {{foo}} />
+          </Foo>`,
+          { moduleName: 'baz/foo-bar' }
+        );
+      }, `Cannot invoke the \`foo\` modifier 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. ('baz/foo-bar' @ L3:C17) `);
+
+      expectAssertion(() => {
+        compile(
+          `
+          <Foo as |foo|>
+            <div {{foo bar baz}} />
+          </Foo>`,
+          { moduleName: 'baz/foo-bar' }
+        );
+      }, `Cannot invoke the \`foo\` modifier 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. ('baz/foo-bar' @ L3:C17) `);
+
+      // Not shadowed
+
+      compile(
+        `
+        <Foo as |foo|></Foo>
+        <div {{foo}} />
+        <div {{foo bar baz}} />`,
+        { moduleName: 'baz/foo-bar' }
+      );
+    }
+
+    [`@test deeply nested modifier invocations`]() {
+      expectAssertion(() => {
+        compile(
+          `
+          {{#let foo as |foo|}}
+            <FooBar as |bar|>
+              {{#each items as |baz|}}
+                <div {{foo}} />
+              {{/each}}
+            </FooBar>
+          {{/let}}`,
+          { moduleName: 'baz/foo-bar' }
+        );
+      }, `Cannot invoke the \`foo\` modifier 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. ('baz/foo-bar' @ L5:C21) `);
+
+      expectAssertion(() => {
+        compile(
+          `
+          {{#let foo as |foo|}}
+            <FooBar as |bar|>
+              {{#each items as |baz|}}
+                <div {{foo bar baz}} />
+              {{/each}}
+            </FooBar>
+          {{/let}}`,
+          { moduleName: 'baz/foo-bar' }
+        );
+      }, `Cannot invoke the \`foo\` modifier 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. ('baz/foo-bar' @ L5:C21) `);
+
+      // Not shadowed
+
+      compile(
+        `
+        {{#let foo as |foo|}}
+          <FooBar as |bar|>
+            {{#each items as |baz|}}
+            {{/each}}
+            <div {{baz}} />
+            <div {{baz bat}} />
+          </FooBar>
+          <div {{bar}} />
+          <div {{bar baz bat}} />
+        {{/let}}
+        <div {{foo}} />
+        <div {{foo bar baz bat}} />`,
+        { moduleName: 'baz/foo-bar' }
+      );
+    }
   }
 );