Post-merge-review: Fix template-no-implicit-this: callee detection, block-param scoping, bare {{this}}

lib/rules/template-no-implicit-this.js

@@ -31,40 +31,65 @@ const BUILT_INS = new Set([
   'rootURL',
 ]);
 
-// Control-flow built-ins whose params should not be flagged
-const CONTROL_FLOW_HELPERS = new Set([
-  'if',
-  'unless',
-  'each',
-  'let',
-  'with',
-  'each-in',
-  'concat',
-  'get',
-  'array',
-  'hash',
-  'log',
+// Node types that have a `path` property pointing to a callee PathExpression
+const CALLEE_PARENT_TYPES = new Set([
+  'GlimmerMustacheStatement',
+  'GlimmerSubExpression',
+  'GlimmerBlockStatement',
+  'GlimmerElementModifierStatement',
 ]);
 
-function isMustacheCalleeWithArgs(node) {
+// Callees are always valid for SubExpression/Block/Modifier; for Mustache,
+// only when the mustache has args (bare {{foo}} is still ambiguous).
+function isCalleePosition(node) {
   const parent = node.parent;
-  if (parent.path !== node) {
+  if (!parent || !CALLEE_PARENT_TYPES.has(parent.type) || parent.path !== node) {
     return false;
   }
-  if (parent.params && parent.params.length > 0) {
+  if (parent.type !== 'GlimmerMustacheStatement') {
     return true;
   }
-  return Boolean(parent.hash && parent.hash.pairs && parent.hash.pairs.length > 0);
+  const hasParams = parent.params && parent.params.length > 0;
+  const hasHash = parent.hash && parent.hash.pairs && parent.hash.pairs.length > 0;
+  return hasParams || hasHash;
 }
 
-function isControlFlowParam(node) {
-  const callee = node.parent.path?.original;
-  return CONTROL_FLOW_HELPERS.has(callee) && node.parent.params?.includes(node);
+// Returns true if the path root resolves to a JS binding (import, const,
+// param, etc.). Walks scope.variables by name so it catches Glimmer built-in
+// names (e.g. log, outlet) that don't surface in scope.references.
+function isJsScopeVariable(node, sourceCode) {
+  if (!sourceCode || !node.original) {
+    return false;
+  }
+  const name = node.original.split('.')[0];
+  try {
+    let scope = sourceCode.getScope(node);
+    while (scope) {
+      if (scope.variables.some((v) => v.name === name)) {
+        return true;
+      }
+      scope = scope.upper;
+    }
+  } catch {
+    // sourceCode.getScope may not be available in .hbs-only mode; ignore.
+  }
+  return false;
 }
 
-function isBlockParamPath(node, path) {
-  const blockParams = node.parent.program?.blockParams || [];
-  return blockParams.includes(path.split('.')[0]);
+// Walks ancestors collecting block params from GlimmerBlockStatement nodes.
+function isLocalBlockParam(node, pathRoot) {
+  let current = node.parent;
+  while (current) {
+    // GlimmerBlockStatement nodes carry block params in program.blockParams
+    if (current.type === 'GlimmerBlockStatement') {
+      const blockParams = current.program?.blockParams || current.blockParams || [];
+      if (blockParams.includes(pathRoot)) {
+        return true;
+      }
+    }
+    current = current.parent;
+  }
+  return false;
 }
 
 /** @type {import('eslint').Rule.RuleModule} */
@@ -104,13 +129,14 @@ module.exports = {
 
   create(context) {
     const allowList = context.options[0]?.allow || [];
+    const sourceCode = context.sourceCode;
 
     return {
       GlimmerPathExpression(node) {
         const path = node.original;
 
         // Skip if path starts with @ (named arg) or this. (explicit)
-        if (path.startsWith('@') || path.startsWith('this.')) {
+        if (path.startsWith('@') || path.startsWith('this.') || path === 'this') {
           return;
         }
 
@@ -124,38 +150,34 @@ module.exports = {
           return;
         }
 
-        // Skip single identifiers that are the callee of a helper-like MustacheStatement
-        if (node.parent && node.parent.type === 'GlimmerMustacheStatement') {
-          if (isMustacheCalleeWithArgs(node)) {
-            return;
-          }
-          if (isControlFlowParam(node)) {
-            return;
-          }
+        // Skip if it looks like a component (PascalCase)
+        const firstPart = path.split('.')[0];
+        if (firstPart[0] === firstPart[0].toUpperCase()) {
+          return;
         }
 
-        // Skip paths that are part of block params
-        if (node.parent && node.parent.type === 'GlimmerBlockStatement') {
-          if (isBlockParamPath(node, path)) {
-            return;
-          }
+        // Skip callees of call-like expressions (SubExpression, BlockStatement,
+        // ElementModifierStatement always; MustacheStatement only with args)
+        if (isCalleePosition(node)) {
+          return;
         }
 
-        // Report ambiguous paths that should use this. or @
-        if (!path.includes('.') || !path.startsWith('this.')) {
-          const firstPart = path.split('.')[0];
-
-          // Skip if it looks like a component (PascalCase)
-          if (firstPart[0] === firstPart[0].toUpperCase()) {
-            return;
-          }
+        // Skip paths whose root is a JS scope binding (import/const/param) —
+        // this is how GJS/GTS references external helpers, components, values.
+        if (isJsScopeVariable(node, sourceCode)) {
+          return;
+        }
 
-          context.report({
-            node,
-            messageId: 'noImplicitThis',
-            data: { path },
-          });
+        // Skip paths whose root is an in-scope block param
+        if (isLocalBlockParam(node, firstPart)) {
+          return;
         }
+
+        context.report({
+          node,
+          messageId: 'noImplicitThis',
+          data: { path },
+        });
       },
     };
   },

tests/lib/rules/template-no-implicit-this.js

@@ -23,9 +23,28 @@ ruleTester.run('template-no-implicit-this', rule, {
     '<template>{{outlet}}</template>',
     '<template>{{has-block}}</template>',
 
-    // Helpers with params
-    '<template>{{if condition "yes" "no"}}</template>',
-    '<template>{{each items}}</template>',
+    // Named-argument control-flow helpers not flagged
+    '<template>{{if @condition "yes" "no"}}</template>',
+    '<template>{{each @items}}</template>',
+
+    // SubExpression, modifier, block callees not flagged
+    '<template>{{echo (my-helper @arg)}}</template>',
+    '<template><div {{my-modifier @arg}}></div></template>',
+    '<template>{{#my-component}}{{/my-component}}</template>',
+
+    // Bare {{this}} is not ambiguous
+    '<template>{{this}}</template>',
+
+    // Block params in nested scopes
+    '<template>{{#each @items as |item|}}{{item.name}}{{/each}}</template>',
+
+    // JS scope bindings (imports, const, let, params) are valid references in GJS/GTS
+    `const condition = false;
+     export default <template>{{if condition "yes" "no"}}</template>;`,
+    `import helper from './my-helper';
+     export default <template>{{helper}}</template>;`,
+    `const items = [1, 2, 3];
+     export default <template>{{#each items as |item|}}{{item}}{{/each}}</template>;`,
 
     // Components (PascalCase)
     '<template>{{MyComponent}}</template>',
@@ -52,6 +71,12 @@ ruleTester.run('template-no-implicit-this', rule, {
       output: null,
       errors: [{ messageId: 'noImplicitThis' }],
     },
+    // Control-flow helper args with no JS binding are still ambiguous
+    {
+      code: '<template>{{if condition "yes" "no"}}</template>',
+      output: null,
+      errors: [{ messageId: 'noImplicitThis' }],
+    },
     {
       code: '<template>{{book-details}}</template>',
       output: null,
@@ -130,6 +155,7 @@ hbsRuleTester.run('template-no-implicit-this', rule, {
     '{{@book.author}}',
 
     // Explicit this
+    '{{this}}',
     '{{this.book}}',
     '{{this.book.author}}',
 
@@ -140,6 +166,24 @@ hbsRuleTester.run('template-no-implicit-this', rule, {
     // Helpers invoked with positional arguments (callee is not flagged)
     '<MyComponent @prop={{can "edit" @model}} />',
 
+    // SubExpression callees should not be flagged
+    '{{echo (my-helper @arg)}}',
+    '{{echo (some-util "value")}}',
+
+    // ElementModifierStatement callees should not be flagged
+    '<div {{my-modifier @arg}}></div>',
+    '<div {{some-modifier "value"}}></div>',
+
+    // BlockStatement callees should not be flagged
+    '{{#my-component}}{{/my-component}}',
+    '{{#some-layout title="Hi"}}content{{/some-layout}}',
+
+    // Block params should be recognized in nested scopes
+    '{{#each @items as |item|}}{{item.name}}{{/each}}',
+    '{{#each @items as |item|}}{{item}}{{/each}}',
+    '{{#let @foo as |bar|}}{{bar.baz}}{{/let}}',
+    '{{#each @items as |item|}}{{#each item.children as |child|}}{{child.name}}{{/each}}{{/each}}',
+
     // PascalCase components
     '<WelcomePage />',
     '<MyComponent @prop={{@value}} />',