fix: report locations with <CR> linebreaks in `no-reference-like-urls` by mdjermanovic · Pull Request #525 · eslint/markdown

mdjermanovic · 2025-09-15T14:59:42Z

Prerequisites checklist

I have read the contributing guidelines.

What is the purpose of this pull request?

Refactors no-reference-like-urls to visit image and link nodes directly and thus simplify the code, and also fixes report location when the link/image contains lone <CR> as a line break (see the two updated test cases).

What changes did you make? (Give an overview)

Refactored no-reference-like-urls to visit image and link nodes directly.

Related Issues

Discussion #523 (comment)

Is there anything you'd like reviewers to focus on?

Parsing of link/image node texts is still needed because of the [link](<uri>) syntax. The regex could probably be simplified further, but that would be a more complicated change.

mdjermanovic · 2025-09-15T15:03:36Z

tests/rules/no-reference-like-urls.test.js

-					endLine: 1,
-					endColumn: 20,
+					endLine: 2,
+					endColumn: 9,


Looks like there's a bug in findOffsets. Now that this rule is modified to report node location directly from the AST, lone <CR> increases line number.

Yes, exactly. Currently, findOffsets doesn't handle CR line endings.

I plan to fix this in #493 once #376 is merged.

mdjermanovic · 2025-09-15T15:04:03Z

tests/rules/no-reference-like-urls.test.js

-					endLine: 1,
-					endColumn: 21,
+					endLine: 2,
+					endColumn: 9,


Same as #525 (comment)

mdjermanovic · 2025-09-15T20:20:34Z

src/rules/no-reference-like-urls.js


 /** Pattern to match both inline links: `[text](url)` and images: `![alt](url)`, with optional title */
 const linkOrImagePattern =
-	/(?<=(?<!\\)(?:\\{2})*)(?<imageBang>!)?\[(?<label>(?:\\.|[^()\\]|\([\s\S]*\))*?)\]\((?<destination>[ \t]*(?:\r\n?|\n)?(?<![ \t])[ \t]*(?:<[^>]*>|[^ \t()]+))(?:[ \t]*(?:\r\n?|\n)?(?<![ \t])[ \t]*(?<title>"[^"]*"|'[^']*'|\([^)]*\)))?[ \t]*(?:\r\n?|\n)?(?<![ \t])[ \t]*\)(?!\()/gu;


I'm not sure what cases were being checked by (?!\() at the end, but no tests are failing now that I've removed it since the regex gets only the image/link node text so it can't check for a parenthesis after the image/link text.

I also think it's safe to remove it, since I tried to find edge cases but couldn't detect any now that we're inspecting the link and image nodes directly.

lumirlumir

Thanks for taking a look! The updated logic looks great.

I've left a few minor suggestions and noted some false negatives in the regex to discuss.

src/rules/no-reference-like-urls.js

lumirlumir · 2025-09-18T12:32:28Z

tests/rules/no-reference-like-urls.test.js

-					endLine: 1,
-					endColumn: 20,
+					endLine: 2,
+					endColumn: 9,


Yes, exactly. Currently, findOffsets doesn't handle CR line endings.

I plan to fix this in #493 once #376 is merged.

lumirlumir · 2025-09-18T12:42:58Z

src/rules/no-reference-like-urls.js

-function isInSkipRange(index, skipRanges) {
-	return skipRanges.some(range => range[0] <= index && index < range[1]);
-}
+	/(?<imageBang>!)?\[(?<label>(?:\\.|[^()\\]|\([\s\S]*\))*?)\]\((?<destination>[ \t]*(?:\r\n?|\n)?(?<![ \t])[ \t]*(?:<[^>]*>|[^ \t()]+))(?:[ \t]*(?:\r\n?|\n)?(?<![ \t])[ \t]*(?<title>"[^"]*"|'[^']*'|\([^)]*\)))?[ \t]*(?:\r\n?|\n)?(?<![ \t])[ \t]*\)$/u;


[[](](link) [[])](link) ![[](](image) ![[])](image)

According to the AST, the code above are valid links and images.

But, they produces false negatives like the following:

test cases:

{ code: dedent` [[](](mercury) [mercury]: https://example.com/mercury `, output: dedent` [[](][mercury] [mercury]: https://example.com/mercury `, errors: [ { messageId: "referenceLikeUrl", data: { type: "link", prefix: "" }, line: 1, column: 1, endLine: 1, endColumn: 15, }, ], }, { code: dedent` [[])](mercury) [mercury]: https://example.com/mercury `, output: dedent` [[])][mercury] [mercury]: https://example.com/mercury `, errors: [ { messageId: "referenceLikeUrl", data: { type: "link", prefix: "" }, line: 1, column: 1, endLine: 1, endColumn: 15, }, ], }, { code: dedent` ![[](](mercury) [mercury]: https://example.com/mercury `, output: dedent` ![[](][mercury] [mercury]: https://example.com/mercury `, errors: [ { messageId: "referenceLikeUrl", data: { type: "image", prefix: "!" }, line: 1, column: 1, endLine: 1, endColumn: 16, }, ], }, { code: dedent` ![[])](mercury) [mercury]: https://example.com/mercury `, output: dedent` ![[])][mercury] [mercury]: https://example.com/mercury `, errors: [ { messageId: "referenceLikeUrl", data: { type: "image", prefix: "!" }, line: 1, column: 1, endLine: 1, endColumn: 16, }, ], },

result:

The regex could probably be simplified further, but that would be a more complicated change.

But, I'm not sure this fix should be included in this PR.

As you mentioned above, would it be better to address this bug in a separate PR?

But, they produces false negatives like the following

Is this a bug that this PR would introduce, or does it already exist in the current version of this rule in the main branch?

It seems that this bug already exists. It also fails on the main branch.

Then I think we could fix it in a separate PR as it's unrelated to this change (unless this change would make it more difficult to fix this bug and we'd need to revert it?). This change was meant to be just a refactor to simplify the rule, and the <CR> bug fix came as a side effect.

lumirlumir · 2025-09-18T13:15:28Z

src/rules/no-reference-like-urls.js


 /** Pattern to match both inline links: `[text](url)` and images: `![alt](url)`, with optional title */
 const linkOrImagePattern =
-	/(?<=(?<!\\)(?:\\{2})*)(?<imageBang>!)?\[(?<label>(?:\\.|[^()\\]|\([\s\S]*\))*?)\]\((?<destination>[ \t]*(?:\r\n?|\n)?(?<![ \t])[ \t]*(?:<[^>]*>|[^ \t()]+))(?:[ \t]*(?:\r\n?|\n)?(?<![ \t])[ \t]*(?<title>"[^"]*"|'[^']*'|\([^)]*\)))?[ \t]*(?:\r\n?|\n)?(?<![ \t])[ \t]*\)(?!\()/gu;


I also think it's safe to remove it, since I tried to find edge cases but couldn't detect any now that we're inspecting the link and image nodes directly.

Co-authored-by: 루밀LuMir <rpfos@naver.com>

lumirlumir

LGTM, thanks!

Would like one more review before merging.

nzakas

LGTM. Thanks.

fix: report locations with <CR> linebreaks in no-reference-like-urls

de596b2

eslint-github-bot bot added the bug label Sep 15, 2025

eslintbot added this to Triage Sep 15, 2025

github-project-automation bot moved this to Needs Triage in Triage Sep 15, 2025

mdjermanovic commented Sep 15, 2025

View reviewed changes

update regex end

b7da2a1

mdjermanovic commented Sep 15, 2025

View reviewed changes

lumirlumir added the accepted label Sep 17, 2025

lumirlumir self-requested a review September 17, 2025 08:46

lumirlumir moved this from Needs Triage to Triaging in Triage Sep 18, 2025

lumirlumir requested changes Sep 18, 2025

View reviewed changes

lumirlumir moved this from Triaging to Implementing in Triage Sep 18, 2025

mdjermanovic and others added 2 commits September 22, 2025 09:59

add type for node

abc0fca

Co-authored-by: 루밀LuMir <rpfos@naver.com>

update type names

03dcb9e

lumirlumir approved these changes Sep 22, 2025

View reviewed changes

lumirlumir moved this from Implementing to Second Review Needed in Triage Sep 22, 2025

nzakas approved these changes Sep 22, 2025

View reviewed changes

nzakas merged commit 28723c2 into main Sep 22, 2025
23 checks passed

nzakas deleted the refactor-no-reference-like-urls branch September 22, 2025 17:44

github-project-automation bot moved this from Second Review Needed to Complete in Triage Sep 22, 2025

github-actions bot mentioned this pull request Sep 16, 2025

chore: release 7.3.0 🚀 #516

Merged

lumirlumir mentioned this pull request Oct 11, 2025

fix: handle CR in rules to follow CommonMark spec #493

Merged

1 task

fossabot bot mentioned this pull request Oct 24, 2025

Bump @eslint/markdown from 7.2.0 to 7.4.1 ericcornelissen/shescape#2213

Merged

Conversation

mdjermanovic commented Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Prerequisites checklist

What is the purpose of this pull request?

What changes did you make? (Give an overview)

Related Issues

Is there anything you'd like reviewers to focus on?

Uh oh!

mdjermanovic Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mdjermanovic Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lumirlumir left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mdjermanovic Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lumirlumir left a comment

Choose a reason for hiding this comment

Uh oh!

nzakas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mdjermanovic commented Sep 15, 2025 •

edited

Loading

mdjermanovic Sep 15, 2025 •

edited

Loading

mdjermanovic Sep 15, 2025 •

edited

Loading

mdjermanovic Sep 22, 2025 •

edited

Loading