Address code review feedback - improve autofixes and handle edge cases

Co-authored-by: NullVoxPopuli <[email protected]>

Author: Copilot

lib/rules/template-no-duplicate-attributes.js

@@ -35,8 +35,19 @@ module.exports = {
             messageId,
             data: { name: key },
             fix(fixer) {
-              // Remove the duplicate attribute
-              return fixer.remove(attr);
+              // Remove the duplicate attribute including preceding whitespace
+              const sourceCode = context.sourceCode;
+              const text = sourceCode.getText();
+              const attrStart = attr.range[0];
+              const attrEnd = attr.range[1];
+              
+              // Look for whitespace before the attribute
+              let removeStart = attrStart;
+              while (removeStart > 0 && /\s/.test(text[removeStart - 1])) {
+                removeStart--;
+              }
+              
+              return fixer.removeRange([removeStart, attrEnd]);
             },
           });
         } else {

lib/rules/template-no-positive-tabindex.js

@@ -79,7 +79,6 @@ module.exports = {
     schema: [],
     messages: {
       positive: 'Avoid positive integer values for tabindex.',
-      invalid: 'Tabindex values must be negative numeric.',
     },
   },
 
@@ -94,14 +93,9 @@ module.exports = {
 
         const tabindexValue = parseTabindexValue(tabindexAttr.value);
 
-        if (tabindexValue === null) {
-          // Can't determine the value, might be a variable reference
-          // Report as potentially invalid
-          context.report({
-            node: tabindexAttr,
-            messageId: 'invalid',
-          });
-        } else if (tabindexValue > 0) {
+        // Only report if we can determine the value and it's positive
+        // Dynamic values (variables) cannot be validated at lint time
+        if (tabindexValue !== null && tabindexValue > 0) {
           context.report({
             node: tabindexAttr,
             messageId: 'positive',

lib/rules/template-require-button-type.js

@@ -66,11 +66,15 @@ module.exports = {
               node: typeAttr,
               messageId: 'invalid',
               fix(fixer) {
-                return fixer.replaceText(typeAttr, 'type="button"');
+                // Use context-aware default like we do for missing type
+                const defaultType = hasParentForm(node) ? 'submit' : 'button';
+                return fixer.replaceText(typeAttr, `type="${defaultType}"`);
               },
             });
           }
         }
+        // Note: Dynamic type values (e.g., type={{this.dynamicType}}) cannot be
+        // validated at lint time, so we don't report them as errors
       },
     };
   },

tests/lib/rules/template-no-duplicate-attributes.js

@@ -38,7 +38,7 @@ ruleTester.run('template-no-duplicate-attributes', rule, {
         <div class="foo" class="bar"></div>
       </template>`,
       output: `<template>
-        <div class="foo" ></div>
+        <div class="foo"></div>
       </template>`,
       errors: [{
         message: "Duplicate attribute 'class' found in the Element.",
@@ -50,7 +50,7 @@ ruleTester.run('template-no-duplicate-attributes', rule, {
         <input type="text" disabled type="email" />
       </template>`,
       output: `<template>
-        <input type="text" disabled  />
+        <input type="text" disabled />
       </template>`,
       errors: [{
         message: "Duplicate attribute 'type' found in the Element.",
@@ -62,7 +62,7 @@ ruleTester.run('template-no-duplicate-attributes', rule, {
         {{helper foo="bar" foo="baz"}}
       </template>`,
       output: `<template>
-        {{helper foo="bar" }}
+        {{helper foo="bar"}}
       </template>`,
       errors: [{
         message: "Duplicate attribute 'foo' found in the MustacheStatement.",
@@ -76,7 +76,7 @@ ruleTester.run('template-no-duplicate-attributes', rule, {
         {{/if}}
       </template>`,
       output: `<template>
-        {{#if condition key="a" }}
+        {{#if condition key="a"}}
           content
         {{/if}}
       </template>`,

tests/lib/rules/template-require-button-type.js

@@ -71,5 +71,21 @@ ruleTester.run('template-require-button-type', rule, {
         type: 'GlimmerAttrNode',
       }],
     },
+    {
+      code: `<template>
+        <form>
+          <button type="invalid">Submit</button>
+        </form>
+      </template>`,
+      output: `<template>
+        <form>
+          <button type="submit">Submit</button>
+        </form>
+      </template>`,
+      errors: [{
+        message: 'Button type must be "button", "submit", or "reset"',
+        type: 'GlimmerAttrNode',
+      }],
+    },
   ],
 });