fix: improve attr-value-no-duplication logic (#1668)

coliff · web-flow · commit 97a1adbff558 · 2025-06-19T00:27:41.000+09:00
* fix: improve attr-value-no-duplication logic

* Update tagname-lowercase.mdx
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "htmlhint",
-  "version": "1.6.2",
+  "version": "1.6.3",
   "description": "The Static Code Analysis Tool for your HTML",
   "keywords": [
     "html",
diff --git a/src/core/rules/attr-value-no-duplication.ts b/src/core/rules/attr-value-no-duplication.ts
@@ -2,62 +2,52 @@ import { Rule } from '../types'
 
 export default {
   id: 'attr-value-no-duplication',
-  description: 'Attribute values should not contain duplicates.',
-  init(parser, reporter) {
+  description:
+    'Class attributes should not contain duplicate values. Other attributes can be checked via configuration.',
+  init(parser, reporter, options) {
+    // Default attributes to check - by default, only check class
+    const defaultAttributesToCheck = ['class']
+
+    // Allow custom configuration of attributes to check
+    const attributesToCheck = Array.isArray(options)
+      ? options
+      : defaultAttributesToCheck
+
     parser.addListener('tagstart', (event) => {
       const attrs = event.attrs
       let attr
       const col = event.col + event.tagName.length + 1
 
-      // Skip SVG elements entirely
-      if (event.tagName.toLowerCase() === 'svg') {
-        return
-      }
-
       for (let i = 0, l = attrs.length; i < l; i++) {
         attr = attrs[i]
+        const attrName = attr.name.toLowerCase()
 
-        // Skip content, media, and style attributes entirely
-        if (
-          attr.name.toLowerCase() === 'content' ||
-          attr.name.toLowerCase() === 'd' ||
-          attr.name.toLowerCase() === 'media' ||
-          attr.name.toLowerCase() === 'sizes' ||
-          attr.name.toLowerCase() === 'src' ||
-          attr.name.toLowerCase() === 'style' ||
-          attr.name.toLowerCase().startsWith('on') // Skip all event handlers (onclick, onchange, etc.)
-        ) {
+        // Strict check - only process attributes in our allowlist
+        if (!attributesToCheck.includes(attrName)) {
           continue
         }
 
-        if (attr.value) {
-          let values: string[]
-          if (attr.name.toLowerCase() === 'media') {
-            // For media, treat each comma-separated part as a whole
-            values = attr.value
-              .split(',')
-              .map((part) => part.trim())
-              .filter(Boolean)
-          } else {
-            // For other attributes, split by whitespace only
-            values = attr.value.trim().split(/\s+/)
-          }
+        // Only process attributes with values containing spaces
+        if (!attr.value || !/\s/.test(attr.value)) {
+          continue
+        }
 
-          const duplicateMap: { [value: string]: boolean } = {}
+        // Split by whitespace - this is appropriate for class, id, role, etc.
+        const values = attr.value.trim().split(/\s+/)
+        const duplicateMap: { [value: string]: boolean } = {}
 
-          for (const value of values) {
-            if (duplicateMap[value] === true) {
-              reporter.error(
-                `Duplicate value [ ${value} ] was found in attribute [ ${attr.name} ].`,
-                event.line,
-                col + attr.index,
-                this,
-                attr.raw
-              )
-              break // Only report the first duplicate found per attribute
-            }
-            duplicateMap[value] = true
+        for (const value of values) {
+          if (value && duplicateMap[value] === true) {
+            reporter.error(
+              `Duplicate value [ ${value} ] was found in attribute [ ${attr.name} ].`,
+              event.line,
+              col + attr.index,
+              this,
+              attr.raw
+            )
+            break // Only report the first duplicate found per attribute
           }
+          duplicateMap[value] = true
         }
       }
     })
diff --git a/test/rules/attr-value-no-duplication.spec.js b/test/rules/attr-value-no-duplication.spec.js
@@ -18,16 +18,28 @@ describe(`Rules: ${ruleId}`, () => {
     )
   })
 
-  it('Duplicate values in data attribute should result in an error', () => {
+  it('Duplicate values in id attribute should NOT result in an error with default config', () => {
+    const code = '<div id="section1 main section1">Test</div>'
+    const messages = HTMLHint.verify(code, ruleOptions)
+    expect(messages.length).toBe(0)
+  })
+
+  it('Duplicate values in role attribute should NOT result in an error with default config', () => {
+    const code = '<div role="button navigation button">Test</div>'
+    const messages = HTMLHint.verify(code, ruleOptions)
+    expect(messages.length).toBe(0)
+  })
+
+  it('Duplicate values in name attribute should NOT result in an error with default config', () => {
+    const code = '<input name="username form1 username">'
+    const messages = HTMLHint.verify(code, ruleOptions)
+    expect(messages.length).toBe(0)
+  })
+
+  it('Duplicate values in data attribute should not result in an error with default config', () => {
     const code = '<span data-attributes="dark light dark">Test</span>'
     const messages = HTMLHint.verify(code, ruleOptions)
-    expect(messages.length).toBe(1)
-    expect(messages[0].rule.id).toBe(ruleId)
-    expect(messages[0].line).toBe(1)
-    expect(messages[0].col).toBe(6)
-    expect(messages[0].message).toBe(
-      'Duplicate value [ dark ] was found in attribute [ data-attributes ].'
-    )
+    expect(messages.length).toBe(0)
   })
 
   it('No duplicate values should not result in an error', () => {
@@ -65,58 +77,65 @@ describe(`Rules: ${ruleId}`, () => {
     )
   })
 
-  it('SVG elements should be skipped entirely', () => {
-    const code = '<svg class="icon icon icon" viewBox="0 0 24 24"></svg>'
-    const messages = HTMLHint.verify(code, ruleOptions)
-    expect(messages.length).toBe(0)
-  })
-
-  it('Style attributes should be skipped entirely', () => {
+  it('Angular directive attributes should not result in an error', () => {
     const code =
-      '<div style="width: 2rem; height: 2rem; width: 2rem;">Test</div>'
+      '<div [ngClass]="\'btn btn\'" *ngIf="condition condition">Test</div>'
     const messages = HTMLHint.verify(code, ruleOptions)
     expect(messages.length).toBe(0)
   })
 
-  it('CSS media queries with commas should not be flagged as duplicates', () => {
-    const code =
-      '<link rel="stylesheet" href="css/test.css" media="all and (-ms-high-contrast: active), (-ms-high-contrast: none)">'
+  it('Alt attributes with duplicate words should not result in an error', () => {
+    // This has the word "a" repeated multiple times which would trigger an error if 'alt' was checked
+    const code = '<img src="image.jpg" alt="A cat and a dog and a ball">'
     const messages = HTMLHint.verify(code, ruleOptions)
     expect(messages.length).toBe(0)
   })
 
-  it('Media attribute with actual duplicates should be skipped', () => {
-    const code =
-      '<link rel="stylesheet" href="css/test.css" media="screen screen">'
-    const messages = HTMLHint.verify(code, ruleOptions)
-    expect(messages.length).toBe(0)
-  })
+  it('Custom attribute configuration should work as expected', () => {
+    const customOptions = {}
+    customOptions[ruleId] = ['data-test', 'aria-label']
 
-  it('Content attribute should be skipped entirely', () => {
-    const code =
-      '<meta name="keywords" content="HTML, CSS, JavaScript, HTML, responsive design">'
-    const messages = HTMLHint.verify(code, ruleOptions)
-    expect(messages.length).toBe(0)
-  })
+    // This should now trigger an error with our custom config
+    const code = '<div data-test="unit test unit">Test</div>'
+    const messages = HTMLHint.verify(code, customOptions)
+    expect(messages.length).toBe(1)
+    expect(messages[0].rule.id).toBe(ruleId)
+    expect(messages[0].message).toBe(
+      'Duplicate value [ unit ] was found in attribute [ data-test ].'
+    )
 
-  it('Script src attribute should be skipped entirely', () => {
-    const code =
-      '<script src="data:text/javascript,window.analytics = window.analytics || function() { (window.analytics.q = window.analytics.q || []).push(arguments) }"></script>'
-    const messages = HTMLHint.verify(code, ruleOptions)
-    expect(messages.length).toBe(0)
+    // Class should no longer be checked with custom config
+    const code2 = '<div class="btn primary btn">Test</div>'
+    const messages2 = HTMLHint.verify(code2, customOptions)
+    expect(messages2.length).toBe(0)
   })
 
-  it('Sizes attribute should be skipped entirely', () => {
-    const code =
-      '<source type="" sizes="(min-width: 1rem) 1vw, (min-width: 2rem) 2vw" srcset="">'
-    const messages = HTMLHint.verify(code, ruleOptions)
-    expect(messages.length).toBe(0)
-  })
+  it('Extended custom configuration should work as expected', () => {
+    const extendedOptions = {}
+    extendedOptions[ruleId] = ['class', 'id', 'role', 'name']
 
-  it('Event handler attributes should be skipped entirely', () => {
-    const code =
-      "<button onclick=\"trackEvent('click'); validateForm(); trackEvent('click');\">Submit</button>"
-    const messages = HTMLHint.verify(code, ruleOptions)
-    expect(messages.length).toBe(0)
+    // Class should still be checked
+    const code1 = '<div class="btn primary btn">Test</div>'
+    const messages1 = HTMLHint.verify(code1, extendedOptions)
+    expect(messages1.length).toBe(1)
+    expect(messages1[0].message).toBe(
+      'Duplicate value [ btn ] was found in attribute [ class ].'
+    )
+
+    // Id should now be checked
+    const code2 = '<div id="section1 main section1">Test</div>'
+    const messages2 = HTMLHint.verify(code2, extendedOptions)
+    expect(messages2.length).toBe(1)
+    expect(messages2[0].message).toBe(
+      'Duplicate value [ section1 ] was found in attribute [ id ].'
+    )
+
+    // Role should now be checked
+    const code3 = '<div role="button navigation button">Test</div>'
+    const messages3 = HTMLHint.verify(code3, extendedOptions)
+    expect(messages3.length).toBe(1)
+    expect(messages3[0].message).toBe(
+      'Duplicate value [ button ] was found in attribute [ role ].'
+    )
   })
 })
diff --git a/website/src/content/docs/changelog.mdx b/website/src/content/docs/changelog.mdx
@@ -5,6 +5,10 @@ description: The release notes for HTMLHint, see what's changed with each new ve
 
 import { Badge } from '@astrojs/starlight/components'
 
+## 1.6.3 _(2025-06-18)_
+
+- <Badge text="Fix" size="small" variant="danger" /> Improve [`attr-value-no-duplication`](/rules/attr-value-no-duplication/) logic
+
 ## 1.6.2 _(2025-06-18)_
 
 - <Badge text="Fix" size="small" variant="danger" /> Improve [`attr-value-no-duplication`](/rules/attr-value-no-duplication/) logic
diff --git a/website/src/content/docs/index.mdx b/website/src/content/docs/index.mdx
@@ -14,7 +14,7 @@ hero:
       variant: minimal
 banner:
   content: |
-    v1.6.2 is now available! Check the <a href="/changelog/">changelog</a> to see what's new.
+    v1.6.3 is now available! Check the <a href="/changelog/">changelog</a> to see what's new.
 tableOfContents: false
 lastUpdated: false
 editUrl: false
diff --git a/website/src/content/docs/rules/attr-value-no-duplication.mdx b/website/src/content/docs/rules/attr-value-no-duplication.mdx
@@ -8,66 +8,66 @@ sidebar:
 
 import { Badge } from '@astrojs/starlight/components';
 
-Attribute values should not contain duplicates.
+Class attributes should not contain duplicate values. Other attributes can be checked via configuration.
 
 Level: <Badge text="Error" variant="danger" />
 
 ## Config value
 
-- `true`: enable rule
+- `true`: enable rule with default attributes (only class)
+- `['attr1', 'attr2', ...]`: specify custom list of attributes to check
 - `false`: disable rule
 
-### The following patterns are **not** considered rule violations
+## Default attributes checked
 
-```html
-<div class="container fluid small">Content</div>
-```
+By default, this rule only checks the `class` attribute for duplicate values:
 
-```html
-<span data-toggle="modal">Content</span>
-```
+- `class` - CSS class names should not be repeated
+
+Other attributes can be checked by providing a custom configuration.
+
+### The following patterns are **not** considered rule violations
 
 ```html
-<div class="btn btn-primary btn-large">Button</div>
+<div class="container fluid small">Content</div>
 ```
 
 ```html
-<source type="" sizes="(min-width: 1rem) 1vw, (min-width: 2rem) 2vw" srcset="">
+<!-- data-attributes not checked by default -->
+<input data-attribute="apple banana apple">
 ```
 
-## Excluded attributes
-
-The following attributes are excluded from this rule and will not trigger errors even if they contain duplicate values:
-
-- `content` - Common for meta tags that may contain duplicate keywords
-- `d` - Used in SVG path data which may contain repetitive pattern data
-- `media` - Used for media queries which have special parsing rules
-- `on*` - All event handlers (onclick, onkeydown, etc.) that may contain legitimate duplicate JavaScript operations
-- `sizes` - Used in responsive images with complex viewport conditions
-- `src` - URLs and data URIs that might legitimately contain repeated patterns
-- `style` - Inline CSS which has its own syntax and may contain duplicate property names
-
 ### The following patterns are considered rule violations:
 
 ```html
 <div class="d-none small d-none">Content</div>
 ```
 
-```html
-<span data-attributes="dark light dark">Content</span>
-```
-
-```html
-<div class="btn  primary btn">Button</div>
-```
-
 ## Why does this rule exist?
 
-Having duplicate values in attributes like `class` or custom data attributes can:
+Having duplicate values in class attributes can:
 
 - Make the markup unnecessarily verbose
 - Cause confusion during development
 - Lead to inefficient CSS specificity calculations
 - Indicate potential copy-paste errors or oversight
 
 This rule helps maintain clean, efficient markup by catching these duplicates early.
+
+## Custom configuration
+
+You can customize which attributes to check by providing an array:
+
+```json
+{
+  "attr-value-no-duplication": ["class", "id", "name", "role"]
+}
+```
+
+```json
+{
+  "attr-value-no-duplication": ["data-test", "aria-label", "custom-attr"]
+}
+```
+
+This allows you to focus on attributes specific to your project needs.
diff --git a/website/src/content/docs/rules/tagname-lowercase.mdx b/website/src/content/docs/rules/tagname-lowercase.mdx
@@ -14,8 +14,7 @@ Level: <Badge text="Error" variant="danger" />
 
 - `true`: enable rule
 - `false`: disable rule
-
-3. ['clipPath', 'test']: Ignore some tagname name
+- `['clipPath', 'data-Test']`: Ignore some tagname name
 
 ### The following patterns are **not** considered rule violations
 

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"name": "htmlhint",`
`3`		`- "version": "1.6.2",`
	`3`	`+ "version": "1.6.3",`
`4`	`4`	`"description": "The Static Code Analysis Tool for your HTML",`
`5`	`5`	`"keywords": [`
`6`	`6`	`"html",`