Skip to content

Commit

Permalink
Merge pull request #1469 from YusukeHirao/features/#1310
Browse files Browse the repository at this point in the history
Add `no-duplicate-dt` rule
  • Loading branch information
YusukeHirao committed Feb 12, 2024
2 parents 6980833 + 9b9d977 commit d692079
Show file tree
Hide file tree
Showing 18 changed files with 353 additions and 84 deletions.
1 change: 1 addition & 0 deletions packages/@markuplint/config-presets/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ No use deprecated element|You should not use deprecated elements from the viewpo
[Require `doctype`](https://html.spec.whatwg.org/multipage/syntax.html#syntax-doctype)|It has the effect of avoiding quirks mode.|✅|✅|✅|✅|✅|❌|❌|✅|❌|❌|❌|
Must not skip heading levels| |✅|✅|✅|✅|✅|❌|❌|✅|❌|❌|❌|
No use ineffective attr| |✅|✅|✅|✅|✅|❌|❌|✅|❌|❌|❌|
[No duplicate names in `<dl>`](https://html.spec.whatwg.org/multipage/grouping-content.html#the-dl-element:~:text=Within%20a%20single%20dl%20element%2C%20there%20should%20not%20be%20more%20than%20one%20dt%20element%20for%20each%20name)|Within a single dl element, there should not be more than one dt element for each name.|✅|✅|✅|✅|✅|❌|❌|✅|❌|❌|❌|
Allow only **permitted contents**| |✅|✅|✅|✅|✅|❌|❌|✅|❌|❌|❌|
Need **placeholder label option**| |✅|✅|✅|✅|✅|❌|❌|✅|❌|❌|❌|
Require the `datetime` attribute if the content of the `time` element is invalid| |✅|✅|✅|✅|✅|❌|❌|✅|❌|❌|❌|
Expand Down
10 changes: 10 additions & 0 deletions packages/@markuplint/config-presets/src/preset.html-standard.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,16 @@
*/
"ineffective-attr": true,

/**
* No duplicate names in `<dl>`
*
* Within a single dl element, there should not be more than one dt element for each name.
*
* @see https://html.spec.whatwg.org/multipage/grouping-content.html#the-dl-element:~:text=Within%20a%20single%20dl%20element%2C%20there%20should%20not%20be%20more%20than%20one%20dt%20element%20for%20each%20name
*
*/
"no-duplicate-dt": true,

/**
* No refer to no existent **ID**
*/
Expand Down
1 change: 1 addition & 0 deletions packages/@markuplint/i18n/$schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@
"mime type": { "type": "string" },
"minute": { "type": "string" },
"month": { "type": "string" },
"name": { "type": "string" },
"newline": { "type": "string" },
"node": { "type": "string" },
"non-negative integer": { "type": "string" },
Expand Down
1 change: 1 addition & 0 deletions packages/@markuplint/i18n/locales/ja.json
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@
"mime type": "MIMEタイプ",
"minute": "",
"month": "",
"name": "名前",
"newline": "改行",
"node": "ノード",
"non-negative integer": "正の整数",
Expand Down
7 changes: 6 additions & 1 deletion packages/@markuplint/ml-core/src/ml-dom/node/element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3401,11 +3401,16 @@ export class MLElement<T extends RuleConfigValue, O extends PlainData = undefine
* @implements DOM API: `Element`
* @see https://dom.spec.whatwg.org/#ref-for-dom-element-matches%E2%91%A0
*/
matches(selector: string): boolean {
matches(
selector: string,
// eslint-disable-next-line @typescript-eslint/prefer-readonly-parameter-types
scope?: MLParentNode<T, O>,
): boolean {
return (
createSelector(selector, this.ownerMLDocument.specs).match(
// Prioritize the pretender
this.pretenderContext?.type === 'pretender' ? this.pretenderContext.as : this,
scope,
) !== false
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ export abstract class MLParentNode<

const elements = toNodeList(
this._descendantsToArray<MLElement<T, O>>(node => {
if (node.is(node.ELEMENT_NODE) && node.matches(selectors)) {
if (node.is(node.ELEMENT_NODE) && node.matches(selectors, this)) {
return node;
}
}),
Expand Down
2 changes: 2 additions & 0 deletions packages/@markuplint/rules/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import LandmarkRoles from './landmark-roles/index.js';
import NoBooleanAttrValue from './no-boolean-attr-value/index.js';
import NoConsecutiveBr from './no-consecutive-br/index.js';
import NoDefaultValue from './no-default-value/index.js';
import NoDuplicateDt from './no-duplicate-dt/index.js';
import NoEmptyPalpableContent from './no-empty-palpable-content/index.js';
import NoHardCodeId from './no-hard-code-id/index.js';
import NoReferToNonExistentId from './no-refer-to-non-existent-id/index.js';
Expand Down Expand Up @@ -55,6 +56,7 @@ const rules = {
'no-boolean-attr-value': NoBooleanAttrValue,
'no-consecutive-br': NoConsecutiveBr,
'no-default-value': NoDefaultValue,
'no-duplicate-dt': NoDuplicateDt,
'no-empty-palpable-content': NoEmptyPalpableContent,
'no-hard-code-id': NoHardCodeId,
'no-refer-to-non-existent-id': NoReferToNonExistentId,
Expand Down
41 changes: 41 additions & 0 deletions packages/@markuplint/rules/src/no-duplicate-dt/README.ja.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
---
description: '`<dl>`内の名前の重複禁止'
---

# `no-duplicate-dt`

<!-- textlint-disable ja-technical-writing/ja-no-mixed-period -->

`<dl>`内の名前の重複禁止

❌ 間違ったコード例

```html
<dl>
<dt>名前1</dt>
<dd>内容1</dd>
<dt>名前1</dt>
<dd>内容2</dd>
<div>
<dt>名前1</dt>
<dd>内容3</dd>
</div>
</dl>
```

✅ 正しいコード例

```html
<dl>
<dt>名前1</dt>
<dd>内容1</dd>
<dt>名前2</dt>
<dd>内容2</dd>
<div>
<dt>名前3</dt>
<dd>内容3</dd>
</div>
</dl>
```

<!-- textlint-enable ja-technical-writing/ja-no-mixed-period -->
38 changes: 38 additions & 0 deletions packages/@markuplint/rules/src/no-duplicate-dt/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
---
id: no-duplicate-dt
description: No duplicate names in `<dl>`
---

# `no-duplicate-dt`

No duplicate names in `<dl>`.

❌ Examples of **incorrect** code for this rule

```html
<dl>
<dt>Name 1</dt>
<dd>Content 1</dd>
<dt>Name 1</dt>
<dd>Content 2</dd>
<div>
<dt>Name 1</dt>
<dd>Content 3</dd>
</div>
</dl>
```

✅ Examples of **correct** code for this rule

```html
<dl>
<dt>Name 1</dt>
<dd>Content 1</dd>
<dt>Name 2</dt>
<dd>Content 2</dd>
<div>
<dt>Name 3</dt>
<dd>Content 3</dd>
</div>
</dl>
```
84 changes: 84 additions & 0 deletions packages/@markuplint/rules/src/no-duplicate-dt/index.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import { mlRuleTest } from 'markuplint';
import { test, expect } from 'vitest';

import rule from './index.js';

test('Duplicate in content', async () => {
expect(
(
await mlRuleTest(
rule,
`
<dl>
<dt>name1</dt>
<dd>description1</dd>
<dt>name1</dt>
<dd>description2</dd>
</dl>
`,
)
).violations,
).toStrictEqual([
{
severity: 'error',
line: 5,
col: 3,
raw: '<dt>',
message: 'The name duplicated',
},
]);
});

test('Duplicate with <div>', async () => {
expect(
(
await mlRuleTest(
rule,
`
<dl>
<div>
<dt>name1</dt>
<dd>description1</dd>
</div>
<div>
<dt>name1</dt>
<dd>description2</dd>
</div>
</dl>
`,
)
).violations,
).toStrictEqual([
{
severity: 'error',
line: 8,
col: 5,
raw: '<dt>',
message: 'The name duplicated',
},
]);
});

test('Nested', async () => {
expect(
(
await mlRuleTest(
rule,
`
<dl>
<div>
<dt>name1</dt>
<dd>description1</dd>
</div>
<dd>
<dl>
<dt>name1</dt>
<dd>description2</dd>
</dl>
</dd>
</dl>
`,
)
).violations,
).toStrictEqual([]);
});
32 changes: 32 additions & 0 deletions packages/@markuplint/rules/src/no-duplicate-dt/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { createRule } from '@markuplint/ml-core';

export default createRule({
meta: {
category: 'validation',
},
verify({ document, report, t }) {
const dlList = [...document.querySelectorAll('dl')];
for (const dl of dlList) {
const dtList = [...dl.querySelectorAll(':scope > dt, :scope > div > dt')];

const names = new Set<string>();
for (const dt of dtList) {
// TODO: Supoort for alternative text for images and accessible names for contained elements.
const name = dt.textContent?.trim();
if (!name) {
continue;
}
if (name) {
if (names.has(name)) {
report({
scope: dt,
message: t('{0} {1:c}', t('The {0}', 'name'), 'duplicated'),
});
continue;
}
names.add(name);
}
}
}
},
});
29 changes: 29 additions & 0 deletions packages/@markuplint/rules/src/no-duplicate-dt/schema.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
{
"$schema": "http://json-schema.org/draft-07/schema#",
"definitions": {
"value": {
"type": "boolean",
"description": "No duplicate names in `<dl>`",
"description:ja": "`<dl>`内の名前の重複禁止"
}
},
"oneOf": [
{
"$ref": "#/definitions/value"
},
{
"type": "object",
"additionalProperties": false,
"properties": {
"value": { "$ref": "#/definitions/value" },
"severity": {
"$ref": "https://raw.githubusercontent.com/markuplint/markuplint/main/packages/%40markuplint/ml-config/schema.json#/definitions/severity",
"default": "error"
},
"reason": {
"type": "string"
}
}
}
]
}
9 changes: 9 additions & 0 deletions packages/@markuplint/selector/src/selector.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,15 @@ describe('selector matching', () => {
expect(createSelector('header > a').match(a)).toBe(false);
});

test('Child combinator with :scope', () => {
const el = createTestElement('<div><span><a></a></span></div>');
const a = el.children[0].children[0];
expect(a.nodeName).toBe('A');
expect(createSelector('a').match(a)).toBeTruthy();
expect(createSelector(':scope a').match(a, el)).toBeTruthy();
expect(createSelector(':scope > span > a').match(a, el)).toBeTruthy();
});

test('Next-sibling combinator', () => {
const el = createTestElement(`<ul>
<li class="i1"><a class="a1">1</a></li>
Expand Down
4 changes: 2 additions & 2 deletions packages/@markuplint/selector/src/selector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export class Selector {
// eslint-disable-next-line @typescript-eslint/prefer-readonly-parameter-types
scope?: ParentNode | null,
): Specificity | false {
scope = isElement(el) ? el : null;
scope = scope ?? (isElement(el) ? el : null);
const results = this.search(el, scope);
for (const result of results) {
if (result.matched) {
Expand All @@ -51,7 +51,7 @@ export class Selector {
// eslint-disable-next-line @typescript-eslint/prefer-readonly-parameter-types
scope?: ParentNode | null,
) {
scope = isElement(el) ? el : null;
scope = scope ?? (isElement(el) ? el : null);
return this.#ruleset.match(el, scope);
}
}
Expand Down
4 changes: 4 additions & 0 deletions packages/markuplint/src/cli/init/get-default-rules.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,10 @@ test('default-rules', () => {
category: 'style',
defaultValue: false,
},
'no-duplicate-dt': {
category: 'validation',
defaultValue: true,
},
'no-empty-palpable-content': {
category: 'validation',
defaultValue: false,
Expand Down
10 changes: 10 additions & 0 deletions test/fixture/020.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<dl>
<dt>Name 1</dt>
<dd>Definition 1</dd>
<dt>Name 2</dt>
<dd>Definition 2</dd>
<dt>Name 3</dt>
<dd>Definition 3</dd>
<dt>Name 3</dt>
<dd>Definition 3</dd>
</dl>
Loading

0 comments on commit d692079

Please sign in to comment.