Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 18, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
74 changes: 32 additions & 42 deletions src/core/rules/attr-value-no-duplication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
})
Expand Down
113 changes: 66 additions & 47 deletions test/rules/attr-value-no-duplication.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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 ].'
)
})
})
4 changes: 4 additions & 0 deletions website/src/content/docs/changelog.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion website/src/content/docs/index.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
64 changes: 32 additions & 32 deletions website/src/content/docs/rules/attr-value-no-duplication.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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.
3 changes: 1 addition & 2 deletions website/src/content/docs/rules/tagname-lowercase.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down