-
Notifications
You must be signed in to change notification settings - Fork 86
fix: recognize Definition node in no-missing-link-fragments
#603
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
Changes from all commits
5e7545a
ab788c5
bf71061
7506e3c
62513c7
6dc8c3e
f4796bc
a88b8aa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -8,14 +8,14 @@ | |||||||
| //----------------------------------------------------------------------------- | ||||||||
|
|
||||||||
| import GithubSlugger from "github-slugger"; | ||||||||
| import { htmlCommentPattern } from "../util.js"; | ||||||||
| import { stripHtmlComments } from "../util.js"; | ||||||||
|
|
||||||||
| //----------------------------------------------------------------------------- | ||||||||
| // Type Definitions | ||||||||
| //----------------------------------------------------------------------------- | ||||||||
|
|
||||||||
| /** | ||||||||
| * @import { Link } from "mdast"; | ||||||||
| * @import { Definition, Link } from "mdast"; | ||||||||
| * @import { MarkdownRuleDefinition } from "../types.js"; | ||||||||
| * @typedef {"invalidFragment"} NoMissingLinkFragmentsMessageIds | ||||||||
| * @typedef {[{ ignoreCase?: boolean; allowPattern?: string }]} NoMissingLinkFragmentsOptions | ||||||||
|
|
@@ -76,17 +76,16 @@ export default { | |||||||
| }, | ||||||||
|
|
||||||||
| create(context) { | ||||||||
| const [{ allowPattern: allowPatternString, ignoreCase }] = | ||||||||
| context.options; | ||||||||
| const allowPattern = allowPatternString | ||||||||
| ? new RegExp(allowPatternString, "u") | ||||||||
| const [{ allowPattern, ignoreCase }] = context.options; | ||||||||
| const allowPatternOrNull = allowPattern | ||||||||
| ? new RegExp(allowPattern, "u") | ||||||||
| : null; | ||||||||
|
|
||||||||
| const fragmentIds = new Set(["top"]); | ||||||||
| const slugger = new GithubSlugger(); | ||||||||
|
|
||||||||
| /** @type {Array<{node: Link, fragment: string}>} */ | ||||||||
| const linkNodes = []; | ||||||||
| /** @type {Array<Definition | Link>} */ | ||||||||
| const relevantNodes = []; | ||||||||
| /** @type {string} */ | ||||||||
| let headingText; | ||||||||
|
|
||||||||
|
|
@@ -101,58 +100,52 @@ export default { | |||||||
|
|
||||||||
| "heading:exit"() { | ||||||||
| const customIdMatch = headingText.match(customHeadingIdPattern); | ||||||||
| const baseId = customIdMatch | ||||||||
| const id = customIdMatch | ||||||||
| ? customIdMatch.groups.id | ||||||||
| : headingText; | ||||||||
| const finalId = slugger.slug(baseId); | ||||||||
| fragmentIds.add(ignoreCase ? finalId.toLowerCase() : finalId); | ||||||||
|
|
||||||||
| fragmentIds.add(slugger.slug(id)); | ||||||||
| }, | ||||||||
|
|
||||||||
| html(node) { | ||||||||
| // 1. Remove all comments | ||||||||
| const htmlTextWithoutComments = node.value | ||||||||
| .trim() | ||||||||
| .replace(htmlCommentPattern, ""); | ||||||||
| const htmlTextWithoutComments = stripHtmlComments(node.value); | ||||||||
|
|
||||||||
| // 2. Then look for IDs in the remaining text | ||||||||
| for (const match of htmlTextWithoutComments.matchAll( | ||||||||
| htmlIdNamePattern, | ||||||||
| )) { | ||||||||
| const extractedId = match.groups.id; | ||||||||
| const finalId = slugger.slug(extractedId); | ||||||||
| fragmentIds.add( | ||||||||
| ignoreCase ? finalId.toLowerCase() : finalId, | ||||||||
| ); | ||||||||
| const { id } = match.groups; | ||||||||
|
|
||||||||
| fragmentIds.add(slugger.slug(id)); | ||||||||
| } | ||||||||
| }, | ||||||||
|
|
||||||||
| link(node) { | ||||||||
| const url = node.url; | ||||||||
| if (!url || !url.startsWith("#")) { | ||||||||
| return; | ||||||||
| } | ||||||||
| "definition, link"(/** @type {Definition | Link} */ node) { | ||||||||
| const { url } = node; | ||||||||
|
|
||||||||
| const fragment = url.slice(1); | ||||||||
| if (!fragment) { | ||||||||
| // If `url` is empty, `"#"`, or does not start with `"#"`, skip it. | ||||||||
| if (url === "" || url === "#" || !url.startsWith("#")) { | ||||||||
| return; | ||||||||
| } | ||||||||
|
|
||||||||
| linkNodes.push({ node, fragment }); | ||||||||
| relevantNodes.push(node); | ||||||||
| }, | ||||||||
|
Comment on lines
+124
to
133
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a small improvement: I've simplified the logic to reduce a bit of memory usage and computation. Previously, we created a new data structure type as follows: markdown/src/rules/no-missing-link-fragments.js Lines 88 to 89 in 8f92161
However, now we no longer create a new object and instead use an existing object reference; it's small, but it would reduce memory usage and computation. |
||||||||
|
|
||||||||
| "root:exit"() { | ||||||||
| for (const { node, fragment } of linkNodes) { | ||||||||
| /** @type {string} */ | ||||||||
| let decodedFragment; | ||||||||
| for (const node of relevantNodes) { | ||||||||
| const fragment = node.url.slice(1); | ||||||||
| let decodedFragment = fragment; | ||||||||
|
|
||||||||
| // Decode URI component to handle encoded characters such as `%20`. | ||||||||
| try { | ||||||||
| decodedFragment = decodeURIComponent(fragment); | ||||||||
| } catch { | ||||||||
| // fallback if not valid encoding | ||||||||
| decodedFragment = fragment; | ||||||||
| // If decoding fails due to an invalid URI sequence, use the original fragment. | ||||||||
| } | ||||||||
|
|
||||||||
| if ( | ||||||||
| allowPattern?.test(decodedFragment) || | ||||||||
| allowPatternOrNull?.test(decodedFragment) || | ||||||||
| githubLineReferencePattern.test(decodedFragment) | ||||||||
| ) { | ||||||||
| continue; | ||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -25,12 +25,20 @@ const ruleTester = new RuleTester({ | |||
|
|
||||
| ruleTester.run("no-missing-link-fragments", rule, { | ||||
| valid: [ | ||||
| // Basic heading match | ||||
| // Basic heading match with `Link` node | ||||
| dedent` | ||||
| # Heading Name | ||||
| [Link](#heading-name) | ||||
| `, | ||||
|
|
||||
| // Basic heading match with `Definition` node | ||||
| dedent` | ||||
| # Heading Name | ||||
| [Link][reference] | ||||
|
|
||||
| [reference]: #heading-name | ||||
| `, | ||||
|
|
||||
| // Custom heading ID | ||||
| dedent` | ||||
| # Heading Name {#custom-name} | ||||
|
|
@@ -383,7 +391,7 @@ ruleTester.run("no-missing-link-fragments", rule, { | |||
| ], | ||||
|
|
||||
| invalid: [ | ||||
| // Basic invalid case | ||||
| // Basic invalid case with `Link` node | ||||
| { | ||||
| code: dedent` | ||||
| [Invalid](#non-existent) | ||||
|
|
@@ -400,6 +408,25 @@ ruleTester.run("no-missing-link-fragments", rule, { | |||
| ], | ||||
| }, | ||||
|
|
||||
| // Basic invalid case with `Definition` node | ||||
| { | ||||
| code: dedent` | ||||
| [Invalid][reference] | ||||
|
|
||||
| [reference]: #non-existent | ||||
| `, | ||||
| errors: [ | ||||
| { | ||||
| messageId: "invalidFragment", | ||||
| data: { fragment: "non-existent" }, | ||||
| line: 3, | ||||
| column: 1, | ||||
| endLine: 3, | ||||
| endColumn: 27, | ||||
| }, | ||||
| ], | ||||
| }, | ||||
|
|
||||
| // Case-sensitive mismatch (with ignoreCase false option) | ||||
| { | ||||
| code: dedent` | ||||
|
|
@@ -520,6 +547,36 @@ ruleTester.run("no-missing-link-fragments", rule, { | |||
| }, | ||||
| ], | ||||
| }, | ||||
| { | ||||
| code: dedent` | ||||
| [Invalid Format](#l20) | ||||
| `, | ||||
| errors: [ | ||||
| { | ||||
| messageId: "invalidFragment", | ||||
| data: { fragment: "l20" }, | ||||
| line: 1, | ||||
| column: 1, | ||||
| endLine: 1, | ||||
| endColumn: 23, | ||||
| }, | ||||
| ], | ||||
| }, | ||||
| { | ||||
| code: dedent` | ||||
| [Invalid Format](#l20-l30) | ||||
| `, | ||||
| errors: [ | ||||
| { | ||||
| messageId: "invalidFragment", | ||||
| data: { fragment: "l20-l30" }, | ||||
| line: 1, | ||||
| column: 1, | ||||
| endLine: 1, | ||||
| endColumn: 27, | ||||
| }, | ||||
| ], | ||||
| }, | ||||
|
Comment on lines
+550
to
+579
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These test cases were added primarily to ensure that Before adding this test, the suite passed even when I added the
FYI: You can verify that the
https://github.com/eslint/markdown/blob/main/package.json#L4
https://github.com/eslint/markdown/blob/main/package.json#l4 // Unworking. |
||||
|
|
||||
| // Invalid link to suffixed heading that shouldn't exist | ||||
| { | ||||
|
|
||||
Uh oh!
There was an error while loading. Please reload this page.