Skip to content

fix: false negatives and positives in no-reversed-media-syntax #473

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

Merged
merged 12 commits into from
Jul 30, 2025

Conversation

lumirlumir
Copy link
Member

@lumirlumir lumirlumir commented Jul 18, 2025

Prerequisites checklist

What is the purpose of this pull request?

Hello,

In this PR, I have addressed both false negatives and false positives in no-reversed-media-syntax.

  • False Negatives:

    Currently, the no-reversed-media-syntax rule does not detect error-prone syntax in Heading and TableCell nodes.

    AST

    image

    Heading and TableCell nodes can contain Link or Image nodes, but the current logic does not report them.

    This behavior is also consistent with Markdownlint:

    image

    So, I've fixed them to be reported correctly.

  • False Positives:

    For cases involving HTML nodes, the logic should not report them; however, the current logic is producing false positives.

    So, I've added HTML nodes to the skip range and excluded them when reporting errors.

    For example:

    hi <span class="foo(bar)[baz]">hi</span>

What changes did you make? (Give an overview)

Related Issues

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

@github-project-automation github-project-automation bot moved this to Needs Triage in Triage Jul 18, 2025
@lumirlumir lumirlumir changed the title fix: Heading/TableCell false negatives in no-reversed-media-syntax fix: false negatives and positives in no-reversed-media-syntax Jul 18, 2025
Comment on lines +106 to +171
function findReversedMediaSyntax(node) {
const text = context.sourceCode.getText(node);
const skipRanges = findSkipRanges(node);
let match;

while ((match = reversedPattern.exec(text)) !== null) {
const [reversedSyntax, label, url] = match;
const matchIndex = match.index;
const matchLength = reversedSyntax.length;

if (
isInSkipRange(
matchIndex + node.position.start.offset,
skipRanges,
)
) {
continue;
}

const {
lineOffset: startLineOffset,
columnOffset: startColumnOffset,
} = findOffsets(text, matchIndex);
const {
lineOffset: endLineOffset,
columnOffset: endColumnOffset,
} = findOffsets(text, matchIndex + matchLength);

const baseColumn = 1;
const nodeStartLine = node.position.start.line;
const nodeStartColumn = node.position.start.column;
const startLine = nodeStartLine + startLineOffset;
const endLine = nodeStartLine + endLineOffset;
const startColumn =
(startLine === nodeStartLine
? nodeStartColumn
: baseColumn) + startColumnOffset;
const endColumn =
(endLine === nodeStartLine ? nodeStartColumn : baseColumn) +
endColumnOffset;

context.report({
loc: {
start: {
line: startLine,
column: startColumn,
},
messageId: "reversedSyntax",
fix(fixer) {
const startOffset =
node.position.start.offset + matchIndex;
const endOffset = startOffset + matchLength;

return fixer.replaceTextRange(
[startOffset, endOffset],
`[${label}](${url})`,
);
end: {
line: endLine,
column: endColumn,
},
});
}
},
messageId: "reversedSyntax",
fix(fixer) {
const startOffset =
node.position.start.offset + matchIndex;
const endOffset = startOffset + matchLength;

return fixer.replaceTextRange(
[startOffset, endOffset],
`[${label}](${url})`,
);
},
});
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

The git diff shows some changes here, but I didn't change the logic itself.

I just wrapped the existing logic in a single function and reused it.

@lumirlumir lumirlumir marked this pull request as ready for review July 22, 2025 06:34
@lumirlumir lumirlumir requested a review from a team July 22, 2025 06:34
nzakas
nzakas previously approved these changes Jul 23, 2025
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
Copy link
Member

nzakas commented Jul 23, 2025

It looks like there's a merge conflict. Can you take a look?

@lumirlumir lumirlumir moved this from Needs Triage to Second Review Needed in Triage Jul 24, 2025
@lumirlumir
Copy link
Member Author

I've resolved the merge conflicts.

@lumirlumir lumirlumir requested a review from a team July 25, 2025 11:29
@nzakas nzakas merged commit c7a4d64 into main Jul 30, 2025
23 checks passed
@nzakas nzakas deleted the fix-false-negatives-no-reversed-media-syntax branch July 30, 2025 14:34
@github-project-automation github-project-automation bot moved this from Second Review Needed to Complete in Triage Jul 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Complete
Development

Successfully merging this pull request may close these issues.

2 participants