Skip to content

Conversation

@lumirlumir
Copy link
Member

@lumirlumir lumirlumir commented Dec 9, 2025

Prerequisites checklist

What is the purpose of this pull request?

Which language are you using?

CommonMark and GFM.

What did you do?

I expected the following examples to be ignored and not to generate errors, but they generate errors (false positives).

Within each section of the code snippet there are HTML comments, HTML, images, and inline code, so these parts shouldn't be interpreted as error-prone link syntax ([description](URL)).

image image

What did you expect to happen?

I expected the above examples to be ignored and not to generate errors, but they generate errors (false positives).

Link to minimal reproducible Example

The following Markdown code may help identify the problem:

<!-- eslint markdown/no-reversed-media-syntax: "error" -->

( <!-- hi)[ --> ]

( <img data-custom = ")[" alt="alt"> ]

( ![image)[]]()

(`)[`]

(`)[]`

What changes did you make? (Give an overview)

In this PR, I've fixed false positives for inline elements in no-reversed-media-syntax.

The previous logic only checked the startOffset of the ignored position, as shown below:

if (isInSkipRange(startOffset, skipRanges)) {
continue;
}

So, the following code was ignored as expected:

`()[]`
^^^^^^ These offsets are in the ignored positions.

`()[]`
-^---- This offset is being checked, so it's working as expected.

However, for the following snippets it doesn't work as expected and produces false positives:

(`)[`]
-^^^^- These offsets are in the ignored positions.

(`)[`]
^----- This offset is being checked, so it's NOT working as expected.
(`)[]`
-^^^^^ These offsets are in the ignored positions.

(`)[]`
^----- This offset is being checked, so it's NOT working as expected.

A concrete way to resolve this problem is to test the regex against whitespace-masked text, rather than checking only the startOffset.

For example, the previous logic checked the following code, which caused false positives:

(`)[`]
^^^^^^ All text will be tested against the `reversedPattern` regex.

However, after this change, the text is tested against whitespace-masked text, so it no longer causes false positives.

(    ]
^----^ Now, The text is whitespace-masked.

The whitespace-masking idea was inspired by the existing no-space-in-emphasis rule, and I've modified its logic slightly.

"heading, paragraph, tableCell"(
/** @type {Heading | Paragraph | TableCell} */ node,
) {
const [startOffset, endOffset] = sourceCode.getRange(node);
// Initialize `buffer` with a whitespace-masked character array.
buffer = new Array(endOffset - startOffset).fill(" ");
},
":matches(heading, paragraph, tableCell) > text"(
/** @type {Text} */ node,
) {
const [startOffset, endOffset] = sourceCode.getRange(node);
const parentNodeStartOffset = // Parent node can be `Heading`, `Paragraph`, or `TableCell`.
sourceCode.getParent(node).position.start.offset;
// Add the content of a `Text` node into the current buffer at the correct offsets.
for (let i = startOffset; i < endOffset; i++) {
buffer[i - parentNodeStartOffset] = sourceCode.text[i];
}
},
":matches(heading, paragraph, tableCell):exit"(
/** @type {Heading | Paragraph | TableCell} */ node,
) {
const maskedText = buffer.join("");
/** @type {Map<string, SourceRange[]>} */
const markerGroups = new Map();

Related Issues

N/A

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

The findReversedMediaSyntax helper function was only used in one place, so I've moved it directly under :matches(heading, paragraph, tableCell):exit.

@eslint-github-bot eslint-github-bot bot added the bug label Dec 9, 2025
@eslintbot eslintbot added this to Triage Dec 9, 2025
@github-project-automation github-project-automation bot moved this to Needs Triage in Triage Dec 9, 2025
@lumirlumir lumirlumir moved this from Needs Triage to Implementing in Triage Dec 9, 2025
@lumirlumir lumirlumir moved this from Implementing to Needs Triage in Triage Dec 9, 2025
@lumirlumir lumirlumir marked this pull request as ready for review December 9, 2025 11:01
Copilot AI review requested due to automatic review settings December 9, 2025 11:01
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request fixes false positives in the no-reversed-media-syntax rule when inline elements (HTML comments, HTML tags, images, inline code) appear in patterns that superficially resemble reversed media syntax.

Key Changes:

  • Replaced skip-range checking with whitespace masking approach
  • Inline elements are masked with spaces before pattern matching
  • Fix generation now extracts from original source using positions from masked matches

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/rules/no-reversed-media-syntax.js Refactored to use whitespace masking instead of skip ranges; removed unused helper function and import; inlined logic into visitor methods
tests/rules/no-reversed-media-syntax.test.js Added 5 test cases covering false positives with HTML comments, HTML tags, images, and inline code

The implementation is sound and correctly handles the reported false positive cases. The whitespace masking approach is borrowed from the existing no-space-in-emphasis rule and effectively prevents inline elements from interfering with pattern matching while preserving the original content for fix generation. The test coverage adequately validates the fix for the reported issues.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

@nzakas nzakas merged commit 8538c10 into main Dec 10, 2025
30 checks passed
@nzakas nzakas deleted the fix-false-positives-for-inline-elements-in-no-reversed-media-syntax branch December 10, 2025 15:50
@github-project-automation github-project-automation bot moved this from Needs Triage to Complete in Triage Dec 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Complete

Development

Successfully merging this pull request may close these issues.

3 participants