Merge pull request #2654 from johanrd/night_fix/template-no-action-modifiers

Post-merge-review: Fix `template-no-action-modifiers` autofix: skip when hash pairs are present

Author: NullVoxPopuli

lib/rules/template-no-action-modifiers.js

@@ -49,33 +49,46 @@ module.exports = {
         node.path.head?.type !== 'AtHead' &&
         node.path.head?.type !== 'ThisHead'
       ) {
-        // Only offer autofix when the first param is a path expression
+        // Only offer autofix when the first param is a path expression.
+        // If hash pairs are present, only fix when the sole hash pair is `on="<event>"` —
+        // we can read the event name from there and drop the pair in the output.
         const maybePath = node.params?.[0];
-        const canFix = maybePath && maybePath.type === 'GlimmerPathExpression';
+        const hashPairs = node.hash?.pairs || [];
+        const onPair = hashPairs.find((p) => p.key === 'on');
+        const otherPairs = hashPairs.filter((p) => p.key !== 'on');
+
+        const canFix =
+          maybePath &&
+          maybePath.type === 'GlimmerPathExpression' &&
+          otherPairs.length === 0 &&
+          (onPair === undefined || onPair.value.type === 'GlimmerStringLiteral');
 
         context.report({
           node,
           messageId: 'noActionModifier',
           fix: canFix
             ? (fixer) => {
+                const eventName =
+                  onPair && onPair.value.type === 'GlimmerStringLiteral'
+                    ? onPair.value.value
+                    : 'click';
+
                 const args = node.params.slice(1);
                 const pathText = sourceCode.getText(maybePath);
 
                 let replacement;
                 if (args.length === 0) {
                   // {{action this.handleClick}} → {{on "click" this.handleClick}}
-                  replacement = `on "click" ${pathText}`;
+                  // {{action this.handleClick on="submit"}} → {{on "submit" this.handleClick}}
+                  replacement = `on "${eventName}" ${pathText}`;
                 } else {
                   // {{action this.handleClick "arg"}} → {{on "click" (fn this.handleClick "arg")}}
                   const argsText = args.map((a) => sourceCode.getText(a)).join(' ');
-                  replacement = `on "click" (fn ${pathText} ${argsText})`;
+                  replacement = `on "${eventName}" (fn ${pathText} ${argsText})`;
                 }
 
-                const lastParam = node.params.at(-1);
-                return fixer.replaceTextRange(
-                  [node.path.range[0], lastParam.range[1]],
-                  replacement
-                );
+                // Replace from start of `action` to just before `}}`, covering any hash pairs
+                return fixer.replaceTextRange([node.path.range[0], node.range[1] - 2], replacement);
               }
             : null,
         });

tests/lib/rules/template-no-action-modifiers.js

@@ -61,5 +61,23 @@ ruleTester.run('template-no-action-modifiers', rule, {
         '<template><button {{on "click" (fn this.handleClick "arg1" "arg2")}}>Save</button></template>',
       errors: [{ messageId: 'noActionModifier' }],
     },
+    {
+      // Path expression with on="click" hash — autofix reads event from hash and drops it
+      code: '<template><button {{action this.handleClick on="click"}}>Save</button></template>',
+      output: '<template><button {{on "click" this.handleClick}}>Save</button></template>',
+      errors: [{ messageId: 'noActionModifier' }],
+    },
+    {
+      // Path expression with on="submit" hash — autofix reads event from hash and drops it
+      code: '<template><form {{action this.handleSubmit on="submit"}}>Submit</form></template>',
+      output: '<template><form {{on "submit" this.handleSubmit}}>Submit</form></template>',
+      errors: [{ messageId: 'noActionModifier' }],
+    },
+    {
+      // Non-`on` hash pair present — no autofix (can't safely translate other hash pairs)
+      code: '<template><button {{action this.handleClick bubbles=false}}>Save</button></template>',
+      output: null,
+      errors: [{ messageId: 'noActionModifier' }],
+    },
   ],
 });