-
-
Notifications
You must be signed in to change notification settings - Fork 426
feat(linter): warn when data-cite targets a spec available in xref #5295
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
Draft
marcoscaceres
wants to merge
4
commits into
main
Choose a base branch
from
feat/3561-data-cite-xref-linter
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
11c7c7f
feat(linter): warn when data-cite targets a spec available in xref
marcoscaceres 831ee20
refactor(linter/prefer-xref): simplify and fix severity support
marcoscaceres 8858f2f
fix(linter/prefer-xref): add JSDoc type casts for TypeScript strict mode
marcoscaceres 96974dc
feat(linter/prefer-xref): register in all profiles, add Czech transla…
marcoscaceres File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,119 @@ | ||
| // @ts-check | ||
| import { docLink, getIntlData, showError, showWarning } from "../utils.js"; | ||
| import { profiles } from "../xref.js"; | ||
|
|
||
| const ruleName = "prefer-xref"; | ||
| export const name = "core/linter-rules/prefer-xref"; | ||
|
|
||
| /** @satisfies {Record<string, { msg(specKey: string): string; readonly hint: string }>} */ | ||
| const localizationStrings = { | ||
| en: { | ||
| msg(specKey) { | ||
| return `Spec \`${specKey}\` is available in xref. Consider using shorthand syntax (e.g. \`[= term =]\`) instead of \`data-cite="${specKey}#…"\`.`; | ||
| }, | ||
| get hint() { | ||
| return docLink`Using ${"[xref]"} shorthand syntax is shorter, spec-version-agnostic, and lets ReSpec verify the term exists. To silence this warning for a specific element, add \`class="lint-ignore"\`. To disable this rule entirely, set \`lint: { "${ruleName}": false }\` in your \`respecConfig\`.`; | ||
| }, | ||
| }, | ||
| }; | ||
| const l10n = getIntlData(localizationStrings); | ||
|
|
||
| /** | ||
| * @param {Conf["xref"]} xref | ||
| * @returns {Set<string> | null} | ||
| */ | ||
| function getXrefSpecSet(xref) { | ||
| if (!xref || xref === true) return null; | ||
|
|
||
| /** @type {Set<string>} */ | ||
| const specs = new Set(); | ||
|
|
||
| if (typeof xref === "string") { | ||
| const profile = xref.toLowerCase(); | ||
| if (profile in profiles) { | ||
| /** @type {Record<string, string[]>} */ (profiles)[profile].forEach( | ||
| (/** @type {string} */ s) => specs.add(s.toUpperCase()) | ||
| ); | ||
| } | ||
| return specs.size ? specs : null; | ||
| } | ||
|
|
||
| if (Array.isArray(xref)) { | ||
| xref.forEach(s => specs.add(s.toUpperCase())); | ||
| return specs.size ? specs : null; | ||
| } | ||
|
|
||
| if (typeof xref === "object") { | ||
| const { profile, specs: specList } = | ||
| /** @type {{ profile?: string; specs?: string[] }} */ (xref); | ||
| if (profile) { | ||
| const key = profile.toLowerCase(); | ||
| if (key in profiles) { | ||
| /** @type {Record<string, string[]>} */ (profiles)[key].forEach( | ||
| (/** @type {string} */ s) => specs.add(s.toUpperCase()) | ||
| ); | ||
| } | ||
| } | ||
| if (specList) { | ||
| specList.forEach(s => specs.add(s.toUpperCase())); | ||
| } | ||
| return specs.size ? specs : null; | ||
| } | ||
|
|
||
| return null; | ||
| } | ||
|
|
||
| /** | ||
| * @param {string} rawCite | ||
| * @returns {string} | ||
| */ | ||
| function extractSpecKey(rawCite) { | ||
| return rawCite.replace(/^[?!]/, "").split(/[/#]/)[0].toUpperCase(); | ||
| } | ||
|
|
||
| /** | ||
| * @param {Conf} conf | ||
| */ | ||
| export function run(conf) { | ||
| // @ts-expect-error -- LintConfig can be false; ?. only short-circuits null/undefined in TS | ||
| if (!conf.lint?.[ruleName]) { | ||
| return; | ||
| } | ||
|
|
||
| if (!conf.xref) { | ||
| return; | ||
| } | ||
|
|
||
| const xrefSpecSet = getXrefSpecSet(conf.xref); | ||
| if (!xrefSpecSet) { | ||
| return; | ||
| } | ||
|
|
||
| /** @type {NodeListOf<HTMLElement>} */ | ||
| const elems = document.querySelectorAll( | ||
| ":is(a, dfn)[data-cite*='#']:not([data-cite^='#']):not(.lint-ignore)" | ||
| ); | ||
|
|
||
| /** @type {Map<string, HTMLElement[]>} */ | ||
| const offenders = new Map(); | ||
|
|
||
| elems.forEach(elem => { | ||
| const rawCite = /** @type {string} */ (elem.dataset.cite); | ||
| const specKey = extractSpecKey(rawCite); | ||
| if (!specKey || !xrefSpecSet.has(specKey)) return; | ||
|
|
||
| if (!offenders.has(specKey)) { | ||
| offenders.set(specKey, []); | ||
| } | ||
| /** @type {HTMLElement[]} */ (offenders.get(specKey)).push(elem); | ||
|
github-code-quality[bot] marked this conversation as resolved.
Fixed
marcoscaceres marked this conversation as resolved.
|
||
| }); | ||
|
marcoscaceres marked this conversation as resolved.
|
||
|
|
||
| // @ts-expect-error -- ruleName is a string literal but LintConfig index signature doesn't cover it | ||
| const logger = conf.lint?.[ruleName] === "error" ? showError : showWarning; | ||
| offenders.forEach((elements, specKey) => { | ||
| logger(l10n.msg(specKey), name, { | ||
| hint: l10n.hint, | ||
| elements, | ||
| }); | ||
| }); | ||
|
marcoscaceres marked this conversation as resolved.
|
||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,243 @@ | ||
| "use strict"; | ||
|
|
||
| import { | ||
| flushIframes, | ||
| makeRSDoc, | ||
| makeStandardOps, | ||
| warningFilters, | ||
| } from "../../SpecHelper.js"; | ||
|
|
||
| describe("Core — linter-rules - prefer-xref", () => { | ||
| const name = "core/linter-rules/prefer-xref"; | ||
| const lintWarnings = warningFilters.filter(name); | ||
|
|
||
| afterAll(() => { | ||
| flushIframes(); | ||
| }); | ||
|
|
||
| it("does not warn when the rule is turned off", async () => { | ||
| const body = ` | ||
| <section> | ||
| <h2>Test</h2> | ||
| <p><a data-cite="HTML#the-a-element">a element</a></p> | ||
| </section>`; | ||
| const ops = makeStandardOps( | ||
| { lint: { "prefer-xref": false }, xref: "web-platform" }, | ||
| body | ||
| ); | ||
| const doc = await makeRSDoc(ops); | ||
| expect(lintWarnings(doc)).toHaveSize(0); | ||
| }); | ||
|
|
||
| it("does not warn when xref is not enabled", async () => { | ||
| const body = ` | ||
| <section> | ||
| <h2>Test</h2> | ||
| <p><a data-cite="HTML#the-a-element">a element</a></p> | ||
| </section>`; | ||
| const ops = makeStandardOps( | ||
| { lint: { "prefer-xref": true }, xref: false }, | ||
| body | ||
| ); | ||
| const doc = await makeRSDoc(ops); | ||
| expect(lintWarnings(doc)).toHaveSize(0); | ||
| }); | ||
|
|
||
| it("does not warn when xref is enabled with no spec list (xref: true)", async () => { | ||
| // When xref=true we cannot determine coverage without a network call, | ||
| // so we conservatively skip the check. | ||
| const body = ` | ||
| <section> | ||
| <h2>Test</h2> | ||
| <p><a data-cite="HTML#the-a-element">a element</a></p> | ||
| </section>`; | ||
| const ops = makeStandardOps( | ||
| { lint: { "prefer-xref": true }, xref: true }, | ||
| body | ||
| ); | ||
| const doc = await makeRSDoc(ops); | ||
| expect(lintWarnings(doc)).toHaveSize(0); | ||
| }); | ||
|
|
||
| it("warns for data-cite with fragment on a spec in the xref profile", async () => { | ||
| const body = ` | ||
| <section> | ||
| <h2>Test</h2> | ||
| <p><a data-cite="HTML#the-a-element">a element</a></p> | ||
| </section>`; | ||
| const ops = makeStandardOps( | ||
| { lint: { "prefer-xref": true }, xref: "web-platform" }, | ||
| body | ||
| ); | ||
| const doc = await makeRSDoc(ops); | ||
| const warnings = lintWarnings(doc); | ||
| expect(warnings).toHaveSize(1); | ||
| expect(warnings[0].message).toContain("HTML"); | ||
| }); | ||
|
|
||
| it("warns once per spec key, grouping all offending elements together", async () => { | ||
| const body = ` | ||
| <section> | ||
| <h2>Test</h2> | ||
| <p><a data-cite="HTML#the-a-element">a element</a></p> | ||
| <p><a data-cite="HTML#the-p-element">p element</a></p> | ||
| <p><a data-cite="INFRA#list">list</a></p> | ||
| </section>`; | ||
| const ops = makeStandardOps( | ||
| { lint: { "prefer-xref": true }, xref: "web-platform" }, | ||
| body | ||
| ); | ||
| const doc = await makeRSDoc(ops); | ||
| const warnings = lintWarnings(doc); | ||
| // One warning per spec key (HTML and INFRA each get one warning). | ||
| expect(warnings).toHaveSize(2); | ||
| const specKeys = warnings.map(w => w.message).join(" "); | ||
| expect(specKeys).toContain("HTML"); | ||
| expect(specKeys).toContain("INFRA"); | ||
| }); | ||
|
|
||
| it("does not warn for data-cite without a fragment (context-only use)", async () => { | ||
| // data-cite="HTML" (no #) is a legitimate context hint — xref uses it. | ||
| const body = ` | ||
| <section data-cite="HTML"> | ||
| <h2>Test</h2> | ||
| <p><a>event handler</a></p> | ||
| </section>`; | ||
| const ops = makeStandardOps( | ||
| { lint: { "prefer-xref": true }, xref: "web-platform" }, | ||
| body | ||
| ); | ||
| const doc = await makeRSDoc(ops); | ||
| expect(lintWarnings(doc)).toHaveSize(0); | ||
| }); | ||
|
|
||
| it("does not warn for specs not in the configured xref list", async () => { | ||
| const body = ` | ||
| <section> | ||
| <h2>Test</h2> | ||
| <p><a data-cite="SOME-OTHER-SPEC#a-fragment">some term</a></p> | ||
| </section>`; | ||
| const ops = makeStandardOps( | ||
| { lint: { "prefer-xref": true }, xref: "web-platform" }, | ||
| body | ||
| ); | ||
| const doc = await makeRSDoc(ops); | ||
| expect(lintWarnings(doc)).toHaveSize(0); | ||
| }); | ||
|
|
||
| it("warns for specs in a custom xref array", async () => { | ||
| const body = ` | ||
| <section> | ||
| <h2>Test</h2> | ||
| <p><a data-cite="MYSPEC#some-term">some term</a></p> | ||
| </section>`; | ||
| const ops = makeStandardOps( | ||
| { lint: { "prefer-xref": true }, xref: ["MYSPEC"] }, | ||
| body | ||
| ); | ||
| const doc = await makeRSDoc(ops); | ||
| const warnings = lintWarnings(doc); | ||
| expect(warnings).toHaveSize(1); | ||
| expect(warnings[0].message).toContain("MYSPEC"); | ||
| }); | ||
|
|
||
| it("warns for specs in a custom xref object with specs array", async () => { | ||
| const body = ` | ||
| <section> | ||
| <h2>Test</h2> | ||
| <p><a data-cite="DOM#eventtarget">EventTarget</a></p> | ||
| </section>`; | ||
| const ops = makeStandardOps( | ||
| { | ||
| lint: { "prefer-xref": true }, | ||
| xref: { profile: "web-platform", specs: ["DOM"] }, | ||
| }, | ||
| body | ||
| ); | ||
| const doc = await makeRSDoc(ops); | ||
| const warnings = lintWarnings(doc); | ||
| expect(warnings).toHaveSize(1); | ||
| expect(warnings[0].message).toContain("DOM"); | ||
| }); | ||
|
|
||
| it("ignores elements with class='lint-ignore'", async () => { | ||
| const body = ` | ||
| <section> | ||
| <h2>Test</h2> | ||
| <p><a class="lint-ignore" data-cite="HTML#the-a-element">a element</a></p> | ||
| <p><a data-cite="HTML#the-p-element">p element</a></p> | ||
| </section>`; | ||
| const ops = makeStandardOps( | ||
| { lint: { "prefer-xref": true }, xref: "web-platform" }, | ||
| body | ||
| ); | ||
| const doc = await makeRSDoc(ops); | ||
| const warnings = lintWarnings(doc); | ||
| // Only the second element (without lint-ignore) should warn. | ||
| expect(warnings).toHaveSize(1); | ||
| expect(warnings[0].elements).toHaveSize(1); | ||
| }); | ||
|
|
||
| it("handles case-insensitive spec key matching", async () => { | ||
| // Authors may write lowercase "html" — should still match "HTML" in the profile. | ||
| const body = ` | ||
| <section> | ||
| <h2>Test</h2> | ||
| <p><a data-cite="html#the-a-element">a element</a></p> | ||
| </section>`; | ||
| const ops = makeStandardOps( | ||
| { lint: { "prefer-xref": true }, xref: "web-platform" }, | ||
| body | ||
| ); | ||
| const doc = await makeRSDoc(ops); | ||
| const warnings = lintWarnings(doc); | ||
| expect(warnings).toHaveSize(1); | ||
| }); | ||
|
|
||
| it("handles informative prefix '?' in data-cite value", async () => { | ||
| const body = ` | ||
| <section> | ||
| <h2>Test</h2> | ||
| <p><a data-cite="?HTML#the-a-element">a element</a></p> | ||
| </section>`; | ||
| const ops = makeStandardOps( | ||
| { lint: { "prefer-xref": true }, xref: "web-platform" }, | ||
| body | ||
| ); | ||
| const doc = await makeRSDoc(ops); | ||
| const warnings = lintWarnings(doc); | ||
| expect(warnings).toHaveSize(1); | ||
| expect(warnings[0].message).toContain("HTML"); | ||
| }); | ||
|
|
||
| it("warns for dfn elements with data-cite fragment in xref specs", async () => { | ||
| const body = ` | ||
| <section> | ||
| <h2>Test</h2> | ||
| <p><dfn data-cite="WEBIDL#dfn-interface">interface</dfn></p> | ||
| </section>`; | ||
| const ops = makeStandardOps( | ||
| { lint: { "prefer-xref": true }, xref: "web-platform" }, | ||
| body | ||
| ); | ||
| const doc = await makeRSDoc(ops); | ||
| const warnings = lintWarnings(doc); | ||
| expect(warnings).toHaveSize(1); | ||
| expect(warnings[0].message).toContain("WEBIDL"); | ||
| }); | ||
|
|
||
| it("does not warn for self-referencing data-cite (data-cite='#foo')", async () => { | ||
| const body = ` | ||
| <section> | ||
| <h2>Test</h2> | ||
| <dfn id="something">something</dfn> | ||
| <p><a data-cite="#something">something</a></p> | ||
| </section>`; | ||
| const ops = makeStandardOps( | ||
| { lint: { "prefer-xref": true }, xref: "web-platform" }, | ||
| body | ||
| ); | ||
| const doc = await makeRSDoc(ops); | ||
| expect(lintWarnings(doc)).toHaveSize(0); | ||
| }); | ||
| }); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.