Fix template-no-action false positive in GJS/GTS

johanrd · johanrd · commit d8426e43614c · 2026-04-13T13:12:53.000+02:00
`action` is an ambient strict-mode keyword in Ember (registered in
STRICT_MODE_KEYWORDS at @ember/template-compiler/lib/plugins/index.ts),
so `{{action this.x}}` works in .gjs/.gts templates without an import.
The rule should still flag those uses — its purpose is to discourage
the deprecated keyword everywhere — but skip cases where `action`
resolves to a JS-scope binding or a template block param:

- `import action from './my-action-helper'; {{action this.x}}` (user import)
- `const action = (h) =&gt; () =&gt; h(); {{action this.x}}` (local declaration)
- `{{#each items as |action|}}{{action this.x}}{{/each}}` (block-param shadow)
- `&lt;Foo as |action|&gt;{{action this.x}}&lt;/Foo&gt;` (element-block-param shadow)

Add the same JS-scope-walk + block-param tracking pattern used by
template-no-log, template-no-unbound, etc.: walk
sourceCode.getScope().variables for the JS side, and push/pop
GlimmerBlockStatement.program.blockParams and GlimmerElementNode.blockParams
for template-side scopes. The `@action` and `this.action` head-type checks
are unchanged (those are JS-side namespaces, not the template keyword).

Tests: 22 (was 8) — adds 9 new valid cases (5 JS-scope, 4 block-param) and
5 new invalid cases (3 ambient-keyword in .gjs/.gts, 1 unrelated import
does not mask, 1 ambient-after-block-exit).

Docs updated to document the strict-mode behavior.
diff --git a/docs/rules/template-no-action.md b/docs/rules/template-no-action.md
@@ -42,6 +42,25 @@ Examples of **correct** code for this rule:
 </template>
 ```
 
+```gjs
+import action from './my-action-helper';
+<template>
+  {{action this.handleClick}}
+</template>
+```
+
+```gjs
+<template>
+  {{#each items as |action|}}
+    <button {{action this.handleClick}}>x</button>
+  {{/each}}
+</template>
+```
+
+## Strict-mode behavior
+
+`action` is an ambient strict-mode keyword in Ember (registered in `STRICT_MODE_KEYWORDS`), so `{{action this.x}}` works in `.gjs`/`.gts` templates without an explicit import. The rule still flags those uses to discourage the deprecated keyword — but skips reports when `action` resolves to a JS-scope binding (an import or local declaration) or a template block param.
+
 ## Migration
 
 - Replace `(action "methodName")` with method references or `(fn this.methodName)`
diff --git a/lib/rules/template-no-action.js b/lib/rules/template-no-action.js
@@ -1,20 +1,19 @@
-function isActionHelper(node) {
+function isActionHelperPath(node) {
   if (!node.path || node.path.type !== 'GlimmerPathExpression') {
     return false;
   }
 
   // Check if it's the action helper (not this.action or @action)
   const path = node.path;
-  if (path.original === 'action') {
-    // Check head.type to avoid deprecated data/this properties
-    const head = path.head;
-    if (head && (head.type === 'AtHead' || head.type === 'ThisHead')) {
-      return false;
-    }
-    return true;
+  if (path.original !== 'action') {
+    return false;
   }
-
-  return false;
+  // Avoid deprecated data/this properties — those are not the helper.
+  const head = path.head;
+  if (head && (head.type === 'AtHead' || head.type === 'ThisHead')) {
+    return false;
+  }
+  return true;
 }
 
 /** @type {import('eslint').Rule.RuleModule} */
@@ -39,9 +38,79 @@ module.exports = {
   },
 
   create(context) {
+    // `action` is an ambient strict-mode keyword in Ember (registered in
+    // STRICT_MODE_KEYWORDS), so `{{action this.x}}` works in .gjs/.gts without
+    // an import. We still want to flag the ambient keyword everywhere — but
+    // skip the report when `action` resolves to a JS binding (e.g.
+    // `import action from './my-action'`) or a template block param
+    // (e.g. `{{#each items as |action|}}{{action this.x}}{{/each}}`).
+    const sourceCode = context.sourceCode;
+    const localScopes = [];
+
+    function isJsScopeVariable(node) {
+      if (!sourceCode) {
+        return false;
+      }
+      try {
+        let scope = sourceCode.getScope(node);
+        while (scope) {
+          if (scope.variables.some((v) => v.name === 'action')) {
+            return true;
+          }
+          scope = scope.upper;
+        }
+      } catch {
+        // sourceCode.getScope may not be available in .hbs-only mode; ignore.
+      }
+      return false;
+    }
+
+    function pushLocals(params) {
+      localScopes.push(new Set(params || []));
+    }
+
+    function popLocals() {
+      localScopes.pop();
+    }
+
+    function isLocalActionBinding() {
+      for (const scope of localScopes) {
+        if (scope.has('action')) {
+          return true;
+        }
+      }
+      return false;
+    }
+
+    function shouldFlag(node) {
+      return isActionHelperPath(node) && !isLocalActionBinding() && !isJsScopeVariable(node);
+    }
+
     return {
+      GlimmerBlockStatement(node) {
+        if (node.program?.blockParams?.length > 0) {
+          pushLocals(node.program.blockParams);
+        }
+      },
+      'GlimmerBlockStatement:exit'(node) {
+        if (node.program?.blockParams?.length > 0) {
+          popLocals();
+        }
+      },
+
+      GlimmerElementNode(node) {
+        if (node.blockParams?.length > 0) {
+          pushLocals(node.blockParams);
+        }
+      },
+      'GlimmerElementNode:exit'(node) {
+        if (node.blockParams?.length > 0) {
+          popLocals();
+        }
+      },
+
       GlimmerSubExpression(node) {
-        if (isActionHelper(node)) {
+        if (shouldFlag(node)) {
           context.report({
             node,
             messageId: 'subExpression',
@@ -50,7 +119,7 @@ module.exports = {
       },
 
       GlimmerMustacheStatement(node) {
-        if (isActionHelper(node)) {
+        if (shouldFlag(node)) {
           context.report({
             node,
             messageId: 'mustache',
@@ -59,7 +128,7 @@ module.exports = {
       },
 
       GlimmerElementModifierStatement(node) {
-        if (isActionHelper(node)) {
+        if (shouldFlag(node)) {
           context.report({
             node,
             messageId: 'modifier',
diff --git a/tests/lib/rules/template-no-action.js b/tests/lib/rules/template-no-action.js
@@ -28,6 +28,37 @@ ruleTester.run('template-no-action', rule, {
     `<template>
       {{@action}}
     </template>`,
+
+    // GJS/GTS: a JS-scope binding shadows the ambient `action` keyword. The
+    // user has imported (or declared) their own `action`, so flagging would
+    // corrupt their reference.
+    {
+      filename: 'test.gjs',
+      code: "import action from './my-action';\n<template>{{action this.handleClick}}</template>",
+    },
+    {
+      filename: 'test.gjs',
+      code: "import action from './my-action';\n<template>{{(action this.handleClick)}}</template>",
+    },
+    {
+      filename: 'test.gjs',
+      code: "import action from './my-action';\n<template><button {{action this.handleClick}}>x</button></template>",
+    },
+    {
+      filename: 'test.gts',
+      code: "import action from './my-action';\n<template>{{action this.handleClick}}</template>",
+    },
+    {
+      filename: 'test.gjs',
+      code: 'const action = (h) => () => h();\n<template>{{action this.handleClick}}</template>',
+    },
+
+    // Template block-param shadowing — `action` is the iterator/let-bound
+    // value, not the ambient keyword.
+    '<template>{{#each items as |action|}}{{action this.x}}{{/each}}</template>',
+    '<template>{{#let (component "x") as |action|}}{{action this.x}}{{/let}}</template>',
+    '<template>{{#each items as |action|}}<button {{action this.x}}>x</button>{{/each}}</template>',
+    '<template><Foo as |action|>{{action this.x}}</Foo></template>',
   ],
 
   invalid: [
@@ -83,5 +114,44 @@ ruleTester.run('template-no-action', rule, {
         },
       ],
     },
+
+    // GJS/GTS: ambient `action` keyword usage with no shadowing import or
+    // block param. Still flagged.
+    {
+      filename: 'test.gjs',
+      code: '<template>{{action "save"}}</template>',
+      output: null,
+      errors: [{ messageId: 'mustache', type: 'GlimmerMustacheStatement' }],
+    },
+    {
+      filename: 'test.gjs',
+      code: '<template><button {{on "click" (action "save")}}>x</button></template>',
+      output: null,
+      errors: [{ messageId: 'subExpression', type: 'GlimmerSubExpression' }],
+    },
+    {
+      filename: 'test.gts',
+      code: '<template><button {{action "submit"}}>x</button></template>',
+      output: null,
+      errors: [{ messageId: 'modifier', type: 'GlimmerElementModifierStatement' }],
+    },
+
+    // Unrelated JS bindings do NOT mask the rule. Only a binding named
+    // `action` should be respected.
+    {
+      filename: 'test.gjs',
+      code: "import handler from './handler';\n<template>{{action this.x}}</template>",
+      output: null,
+      errors: [{ messageId: 'mustache' }],
+    },
+
+    // Ambient `action` outside a block-param scope is still flagged, even
+    // when an inner block legitimately shadows it.
+    {
+      filename: 'test.gjs',
+      code: '<template>{{#each items as |action|}}{{action this.x}}{{/each}}{{action this.y}}</template>',
+      output: null,
+      errors: [{ messageId: 'mustache' }],
+    },
   ],
 });
