[BUGFIX beta] Local variables should win over helpers

```hbs
{{#let this.foo as |foo|}}
  {{foo 1 2 3}}
    ~~~ local variable invocation!
{{/let}}
```

Previously, this would have tried to resolve and invoke the helper `foo`,
ignoring the presence of the `foo` local variable. This is inconsistent
with our general lexical lookup rules. This will now invoke `foo` local
variable as a contextual component.

(In other words, this removes the remaining "dot rule" exceptions for
contextual components, as per [RFC #311](https://github.com/emberjs/rfcs/blob/master/text/0311-angle-bracket-invocation.md).)

This commit also fixed another related issue:

```hbs
{{#let (hash foo="foo-bar") as |h|}}
  {{h.foo 1 2 3}}
    ~~~~~ this is a string, not a component!
{{/let}}
```

Previously, this would have tried to resolve and invoke the `foo-bar`
component. This is an unintended consequence (i.e. a bug) of the "dot
rule" implementation. This will now raise a `TypeError` in development
mode and result in undefined behavior in production builds (probably
some other runtime error deep inside the Glimmer VM internals).

Fixes #17121.

packages/@ember/-internals/glimmer/lib/helpers/-assert-implicit-component-helper-argument.ts

@@ -0,0 +1,36 @@
+import { DEBUG } from '@glimmer/env';
+import { Opaque } from '@glimmer/interfaces';
+import { Tag, VersionedPathReference } from '@glimmer/reference';
+import { Arguments, Helper, VM } from '@glimmer/runtime';
+import { Maybe } from '@glimmer/util';
+
+let helper: Maybe<Helper> = undefined;
+
+if (DEBUG) {
+  class ComponentAssertionReference implements VersionedPathReference<Opaque> {
+    public tag: Tag;
+
+    constructor(private component: VersionedPathReference<Opaque>, private message: string) {
+      this.tag = component.tag;
+    }
+
+    value(): Opaque {
+      let value = this.component.value();
+
+      if (typeof value === 'string') {
+        throw new TypeError(this.message);
+      }
+
+      return value;
+    }
+
+    get(property: string): VersionedPathReference<Opaque> {
+      return this.component.get(property);
+    }
+  }
+
+  helper = (_vm: VM, args: Arguments) =>
+    new ComponentAssertionReference(args.positional.at(0), args.positional.at(1).value() as string);
+}
+
+export default helper;

packages/@ember/-internals/glimmer/lib/resolver.ts

@@ -5,6 +5,7 @@ import { lookupComponent, lookupPartial, OwnedTemplateMeta } from '@ember/-inter
 import { EMBER_MODULE_UNIFICATION, GLIMMER_CUSTOM_COMPONENT_MANAGER } from '@ember/canary-features';
 import { assert } from '@ember/debug';
 import { _instrumentStart } from '@ember/instrumentation';
+import { DEBUG } from '@glimmer/env';
 import {
   ComponentDefinition,
   Opaque,
@@ -18,6 +19,7 @@ import { CurlyComponentDefinition } from './component-managers/curly';
 import { CustomManagerDefinition, ManagerDelegate } from './component-managers/custom';
 import { TemplateOnlyComponentDefinition } from './component-managers/template-only';
 import { isHelperFactory, isSimpleHelper } from './helper';
+import { default as componentAssertionHelper } from './helpers/-assert-implicit-component-helper-argument';
 import { default as classHelper } from './helpers/-class';
 import { default as htmlSafeHelper } from './helpers/-html-safe';
 import { default as inputTypeHelper } from './helpers/-input-type';
@@ -76,6 +78,10 @@ const BUILTINS_HELPERS = {
   '-outlet': outletHelper,
 };
 
+if (DEBUG) {
+  BUILTINS_HELPERS['-assert-implicit-component-helper-argument'] = componentAssertionHelper;
+}
+
 const BUILTIN_MODIFIERS = {
   action: { manager: new ActionModifierManager(), state: null },
 };

packages/@ember/-internals/glimmer/tests/integration/components/contextual-components-test.js

@@ -1387,6 +1387,59 @@ moduleFor(
         this.render('{{component (component "textarea" type="text")}}');
       }, 'You cannot use `textarea` as a component name.');
     }
+
+    ['@test GH#17121 local variable should win over helper (without arguments)']() {
+      this.registerHelper('foo', () => 'foo helper');
+
+      this.registerComponent('foo-bar', { template: 'foo-bar component' });
+
+      this.render(strip`
+        {{#let (component 'foo-bar') as |foo|}}
+          {{foo}}
+        {{/let}}
+      `);
+
+      this.assertText('foo-bar component');
+
+      this.assertStableRerender();
+    }
+
+    ['@test GH#17121 local variable should win over helper (with arguments)']() {
+      this.registerHelper('foo', params => `foo helper: ${params.join(' ')}`);
+
+      this.registerComponent('foo-bar', {
+        ComponentClass: Component.extend().reopenClass({
+          positionalParams: 'params',
+        }),
+        template: 'foo-bar component:{{#each params as |param|}} {{param}}{{/each}}',
+      });
+
+      this.render(strip`
+        {{#let (component 'foo-bar') as |foo|}}
+          {{foo 1 2 3}}
+        {{/let}}
+      `);
+
+      this.assertText('foo-bar component: 1 2 3');
+
+      this.assertStableRerender();
+    }
+
+    ['@test GH#17121 implicit component invocations should not perform string lookup'](assert) {
+      this.registerComponent('foo-bar', { template: 'foo-bar component' });
+
+      assert.throws(
+        () =>
+          this.render(strip`
+          {{#let 'foo-bar' as |foo|}}
+            {{foo 1 2 3}}
+          {{/let}}
+        `),
+        new TypeError(
+          "expected `foo` to be a contextual component but found a string. Did you mean `(component foo)`? ('-top-level' @ L1:C29) "
+        )
+      );
+    }
   }
 );
 

packages/ember-template-compiler/lib/plugins/index.ts

@@ -6,7 +6,7 @@ import DeprecateSendAction from './deprecate-send-action';
 import TransformActionSyntax from './transform-action-syntax';
 import TransformAngleBracketComponents from './transform-angle-bracket-components';
 import TransformAttrsIntoArgs from './transform-attrs-into-args';
-import TransformDotComponentInvocation from './transform-dot-component-invocation';
+import TransformComponentInvocation from './transform-component-invocation';
 import TransformEachInIntoEach from './transform-each-in-into-each';
 import TransformHasBlockSyntax from './transform-has-block-syntax';
 import TransformInElement from './transform-in-element';
@@ -22,7 +22,7 @@ import { ASTPlugin, ASTPluginEnvironment } from '@glimmer/syntax';
 export type APluginFunc = (env: ASTPluginEnvironment) => ASTPlugin | undefined;
 
 const transforms: Array<APluginFunc> = [
-  TransformDotComponentInvocation,
+  TransformComponentInvocation,
   TransformAngleBracketComponents,
   TransformTopLevelComponents,
   TransformInlineLinkTo,

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

@@ -0,0 +1,212 @@
+import { DEBUG } from '@glimmer/env';
+import { AST, ASTPlugin, ASTPluginEnvironment } from '@glimmer/syntax';
+import calculateLocationDisplay from '../system/calculate-location-display';
+import { Builders } from '../types';
+
+/**
+  Transforms unambigious invocations of closure components to be wrapped with
+  the component helper.
+
+  ```handlebars
+  {{!-- this.foo is not a legal helper/component name --}}
+  {{this.foo "with" some="args"}}
+  ```
+
+  with
+
+  ```handlebars
+  {{component this.foo "with" some="args"}}
+  ```
+
+  and
+
+  ```handlebars
+  {{!-- this.foo is not a legal helper/component name --}}
+  {{#this.foo}}...{{/this.foo}}
+  ```
+
+  with
+
+  ```handlebars
+  {{#component this.foo}}...{{/component}}
+  ```
+
+  and
+
+  ```handlebars
+  {{!-- foo.bar is not a legal helper/component name --}}
+  {{foo.bar "with" some="args"}}
+  ```
+
+  with
+
+  ```handlebars
+  {{component foo.bar "with" some="args"}}
+  ```
+
+  and
+
+  ```handlebars
+  {{!-- foo.bar is not a legal helper/component name --}}
+  {{#foo.bar}}...{{/foo.bar}}
+  ```
+
+  with
+
+  ```handlebars
+  {{#component foo.bar}}...{{/component}}
+  ```
+
+  and
+
+  ```handlebars
+  {{#let ... as |foo|}}
+    {{!-- foo is a local variable --}}
+    {{foo "with" some="args"}}
+  {{/let}}
+  ```
+
+  with
+
+  ```handlebars
+  {{#let ... as |foo|}}
+    {{component foo "with" some="args"}}
+  {{/let}}
+  ```
+
+  and
+
+  ```handlebars
+  {{#let ... as |foo|}}
+    {{!-- foo is a local variable --}}
+    {{#foo}}...{{/foo}}
+  {{/let}}
+  ```
+
+  with
+
+  ```handlebars
+  {{#let ... as |foo|}}
+    {{#component foo}}...{{/component}}
+  {{/let}}
+  ```
+
+  @private
+  @class TransFormComponentInvocation
+*/
+export default function transformComponentInvocation(env: ASTPluginEnvironment): ASTPlugin {
+  let { moduleName } = env.meta;
+  let { builders: b } = env.syntax;
+  let locals: string[][] = [];
+
+  return {
+    name: 'transform-component-invocation',
+
+    visitor: {
+      ElementNode: {
+        enter(node: AST.ElementNode) {
+          locals.push(node.blockParams);
+        },
+
+        exit() {
+          locals.pop();
+        },
+      },
+
+      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);
+        },
+
+        exit() {
+          locals.pop();
+        },
+      },
+
+      MustacheStatement(node: AST.MustacheStatement): AST.Node | void {
+        if (isInlineInvocation(node, locals)) {
+          wrapInComponent(moduleName, node, b);
+        }
+      },
+    },
+  };
+}
+
+function isInlineInvocation(node: AST.MustacheStatement, locals: string[][]): boolean {
+  let { path } = node;
+  return isPath(path) && isIllegalName(path, locals) && hasArguments(node);
+}
+
+function isPath(node: AST.PathExpression | AST.Literal): node is AST.PathExpression {
+  return node.type === 'PathExpression';
+}
+
+function isIllegalName(node: AST.PathExpression, locals: string[][]): boolean {
+  return isThisPath(node) || isDotPath(node) || isLocalVariable(node, locals);
+}
+
+function isThisPath(node: AST.PathExpression): boolean {
+  return node.this === true;
+}
+
+function isDotPath(node: AST.PathExpression): boolean {
+  return node.parts.length > 1;
+}
+
+function isLocalVariable(node: AST.PathExpression, locals: string[][]): boolean {
+  return !node.this && hasLocalVariable(node.parts[0], locals);
+}
+
+function hasLocalVariable(name: string, locals: string[][]): boolean {
+  return locals.some(names => names.indexOf(name) !== -1);
+}
+
+function hasArguments(node: AST.MustacheStatement): boolean {
+  return node.params.length > 0 || node.hash.pairs.length > 0;
+}
+
+function isBlockInvocation(node: AST.BlockStatement, locals: string[][]): boolean {
+  return isIllegalName(node.path, locals);
+}
+
+let wrapInAssertion: (
+  moduleName: string,
+  node: AST.PathExpression,
+  builder: Builders
+) => AST.Expression;
+
+if (DEBUG) {
+  wrapInAssertion = (moduleName, node, b) => {
+    let error = b.string(
+      `expected \`${
+        node.original
+      }\` to be a contextual component but found a string. Did you mean \`(component ${
+        node.original
+      })\`? ${calculateLocationDisplay(moduleName, node.loc)}`
+    );
+
+    return b.sexpr(
+      b.path('-assert-implicit-component-helper-argument'),
+      [node, error],
+      b.hash(),
+      node.loc
+    );
+  };
+} else {
+  wrapInAssertion = (_, node) => node;
+}
+
+function wrapInComponent(
+  moduleName: string,
+  node: AST.MustacheStatement | AST.BlockStatement,
+  b: Builders
+) {
+  let component = wrapInAssertion(moduleName, node.path as AST.PathExpression, b);
+  node.path = b.path('component');
+  node.params.unshift(component);
+}