diff --git a/package-lock.json b/package-lock.json index cdeaedcaf..bf9a0672c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "htmlhint", - "version": "1.6.2", + "version": "1.6.3", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "htmlhint", - "version": "1.6.2", + "version": "1.6.3", "license": "MIT", "dependencies": { "async": "3.2.6", diff --git a/package.json b/package.json index e4275d777..8199d252d 100644 --- 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 index 66ccf4d10..60ace1459 100644 --- 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 index e5b57fbaa..df00e8669 100644 --- 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 = '
Test
' + 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 = '
Test
' + 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 = '' + 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 = 'Test' 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 = '' - 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 = - '
Test
' + '
Test
' 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 = - '' + 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 = '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 = - '' - 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 = - '' - const messages = HTMLHint.verify(code, ruleOptions) - expect(messages.length).toBe(0) - }) + // This should now trigger an error with our custom config + const code = '
Test
' + 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 = - '' - const messages = HTMLHint.verify(code, ruleOptions) - expect(messages.length).toBe(0) + // Class should no longer be checked with custom config + const code2 = '
Test
' + const messages2 = HTMLHint.verify(code2, customOptions) + expect(messages2.length).toBe(0) }) - it('Sizes attribute should be skipped entirely', () => { - const code = - '' - 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 = - "" - const messages = HTMLHint.verify(code, ruleOptions) - expect(messages.length).toBe(0) + // Class should still be checked + const code1 = '
Test
' + 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 = '
Test
' + 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 = '
Test
' + 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 index 3fa2f5df6..737641586 100644 --- 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)_ + +- Improve [`attr-value-no-duplication`](/rules/attr-value-no-duplication/) logic + ## 1.6.2 _(2025-06-18)_ - 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 index 29abea84c..e2d2646c7 100644 --- 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 changelog to see what's new. + v1.6.3 is now available! Check the changelog 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 index 9355242a7..ebd52ea5a 100644 --- a/website/src/content/docs/rules/attr-value-no-duplication.mdx +++ b/website/src/content/docs/rules/attr-value-no-duplication.mdx @@ -8,62 +8,44 @@ 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: ## 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 -
Content
-``` +By default, this rule only checks the `class` attribute for duplicate values: -```html -Content -``` +- `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 -
Button
+
Content
``` ```html - + + ``` -## 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
Content
``` -```html -Content -``` - -```html -
Button
-``` - ## 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 @@ -71,3 +53,21 @@ Having duplicate values in attributes like `class` or custom data attributes can - 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 index 56c3173bd..35a49074a 100644 --- a/website/src/content/docs/rules/tagname-lowercase.mdx +++ b/website/src/content/docs/rules/tagname-lowercase.mdx @@ -14,8 +14,7 @@ Level: - `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