fix: report messages for configuration comments correctly by andreahlert · Pull Request #618 · eslint/markdown

andreahlert · 2026-02-05T11:36:15Z

Summary

Previously, ESLint messages that pointed to configuration comments (such as unused disable directive warnings) were being silently dropped because the adjustMessage function returned null for any message pointing to lines before the actual code content.

This change:

Tracks the original HTML comment positions when collecting configuration comments from the Markdown
Builds a mapping from linted code line numbers to original comment info
Maps messages pointing to configuration comment lines back to their original HTML comment locations in the Markdown file

This fixes the issue where --report-unused-disable-directives would not work with the markdown processor, as those warnings were being filtered out.

Before

$ eslint --report-unused-disable-directives README.md
# (no output even when there are unused disable directives)

After

$ eslint --report-unused-disable-directives README.md
README.md
  3:1  warning  Unused eslint-disable directive (no problems were reported from 'no-undef')

Test plan

Added new test suite reportUnusedDisableDirectives with tests for single-line and multi-line HTML comments
All existing 1590 tests pass
Lint passes

Fixes #115

linux-foundation-easycla · 2026-02-05T11:36:23Z

The committers listed above are authorized under a signed CLA.

✅ login: andreahlert / name: André Ahlert (1917fa0, a08617f, aaa9b03, dca9351)

andreahlert · 2026-02-07T20:09:51Z

Hi @nzakas, this fixes a 7-year-old issue (#115) where --report-unused-disable-directives silently drops warnings from HTML comments in Markdown files. CI is green and I've added 4 test cases covering the mapping logic. Would appreciate a review when you get a chance!

andreahlert · 2026-02-08T11:04:25Z

Thanks for the review, @Ari4ka!

nzakas · 2026-02-09T22:16:24Z

@andreahlert thanks, I'll take a look when I'm able.

Did you use AI to generate this PR?

andreahlert · 2026-02-10T20:45:14Z

Hey @nzakas . Everybody says that to me 😅 I write lots of docs at work so it's a habit to be super detailed. And I usually generate an AI template of PR (for headers for exemple), to keep the same looking, but its all my writing.

I'll keep PR descriptions shorter going forward. Lmk if you want me to clarify anything about the implementation!

DMartens · 2026-02-11T14:57:30Z

+			if (commentInfo) {
+				return {
+					...message,
+					line: commentInfo.position.start.line,


Question to the team: Is it okay that the precise location is lost?
For example when a disable directive has multiple rules but only one is unused, the location spans only the name of the unused rule instead of the whole directive.

I'm having a hard time parsing your question. Are you saying that previously we spanned only the unused rule name and this change loses that? If so, I'd say we don't want to lose that.

Or are you saying we didn't span only the unused rule name and this PR does? Then I'd say this is the preferred approach.

To clarify for @nzakas: before this PR those messages were dropped (no report at all). With this PR we report them but the position is the whole HTML comment (start line of the comment in the Markdown), not the exact column of the unused rule name inside the comment. So we’re not losing the previous “span only the unused rule name” behavior; we’re adding reporting with a coarser location.

If the team wants column-level precision (e.g. only the unused rule name) we’d need extra mapping from the linted code columns back to the original comment;

I can explore that in a follow-up if needed.

I agree we should postpone this to a follow-up.

From what I can tell, the current commentInfo type is derived from the Markdown HTML node's position field, which contains full position information including start.line, start.column, end.line, and end.column.

Also, the comment is sanitized by the getComment helper before being pushed into the htmlComments array:

https://github.com/andreahlert/markdown/blob/6855867919761bc1eb57df1c1bbfb5fb4a307d47/src/processor.js#L70-L89

Therefore, the comment syntax should be valid and retain full position information (verified via logging).

Losing the position information causes the following issue: only the first character is highlighted instead of the entire line.

Current

Expected

I believe retaining the full position information here is necessary.

DMartens · 2026-02-11T14:59:25Z

Thank you for submitting a PR for this. It is a really good starting for fixing this.

lumirlumir · 2026-02-15T13:28:51Z

Hi — there's a CI failure. If you have a moment, could you take a look?

Lint was failing because report-unused-disable-directives now correctly reports warnings in .md files; tests/fixtures/long.md has intentional directives that trigger those warnings. Align with main config which already ignores tests/fixtures. Refs eslint#618

andreahlert · 2026-02-23T06:29:03Z

Hi — there's a CI failure. If you have a moment, could you take a look?

Done! Ready for final review.

andreahlert · 2026-03-05T19:18:57Z

@nzakas @DMartens can we have a look on this?

DMartens · 2026-03-05T22:36:27Z

+
 export interface BlockBase {
 	baseIndentText: string;
 	comments: string[];


The comments property should be removed as it is no longer set.

Can you double-check this? It seems like comments is still used.

It seems the current implementation still uses the comments property. Otherwise it throws a type error.

Confirmed, comments is still used in getBlockRangeMap, block.comments.join for assembling linted text, and leadingCommentLines calculation. Kept it as-is.

My point was that comments is unnecessary as the relevant data is available in commentInfos. Further on line 430 the code also recreates the data from comments with wrapComment(commentInfo.text) which would be available on the same index on comments.

Could you also take a look at the comments here when you have a moment?

DMartens · 2026-03-05T22:37:59Z

Sorry for the delay. Other than the small things I mentioned, the changes LGTM.

Lint was failing because report-unused-disable-directives now correctly reports warnings in .md files; tests/fixtures/long.md has intentional directives that trigger those warnings. Align with main config which already ignores tests/fixtures. Refs eslint#618 Signed-off-by: André Ahlert <andre@aex.partners>

@typedef

- Rename CommentInfo to MappedCommentLocation for clarity - Import MappedCommentLocation type from ./types.js instead of inline @typedef - Remove unnecessary * from wrapComment regex character sets - Refactor tests to use beforeEach with shared config - Add reportUnusedDisableDirectives tests under LegacyESLint describe block Signed-off-by: André Ahlert <andre@aex.partners>

andreahlert · 2026-03-12T08:08:09Z

@nzakas Renamed CommentInfo to MappedCommentLocation in both types.ts and processor.js. Also rebased on main and addressed all other review feedback.

andreahlert · 2026-03-12T08:08:21Z

@lumirlumir Rebased on main, all feedback addressed. Tests and lint are passing.

github-actions · 2026-03-22T22:43:13Z

Hi everyone, it looks like we lost track of this pull request. Please review and see what the next steps are. This pull request will auto-close in 7 days without an update.

nzakas

LGTM. Thanks!

Leaving open for @lumirlumir to review.

nzakas · 2026-03-23T20:47:25Z

@andreahlert be sure to double-check the CI failures.

lumirlumir

The test in https://github.com/eslint/markdown/blob/main/tests/types/types.test.ts needs to be updated to fix the CI failure. Running npm run fmt will also resolve the formatting error.

Signed-off-by: André Ahlert <andre@aex.partners>

github-actions · 2026-04-04T22:45:25Z

Hi everyone, it looks like we lost track of this pull request. Please review and see what the next steps are. This pull request will auto-close in 7 days without an update.

fasttime · 2026-04-07T11:05:41Z

This PR needs a new review.

lumirlumir

Apologies for the delay.

This PR needs some dedicated time for me to review. I’ll take a thorough look within 2-3 days. I appreciate your patience.

lumirlumir · 2026-04-09T14:14:19Z

+
 export interface BlockBase {
 	baseIndentText: string;
 	comments: string[];


Could you also take a look at the comments here when you have a moment?

lumirlumir · 2026-04-13T11:04:22Z

 export default defineConfig([
 	globalIgnores(
-		["**/*.js", "**/.cjs", "**/.mjs"],
+		["**/*.js", "**/.cjs", "**/.mjs", "**/tests/fixtures/**"],


I think this comment still hasn't been addressed.

lumirlumir · 2026-04-13T11:44:22Z

+	position: {
+		start: { line: number; column: number };
+		end?: { line: number; column: number };
+	};


Suggested change

position: {

start: { line: number; column: number };

end?: { line: number; column: number };

};

position: Position;

The current position information comes from node.position, which uses the Position type from unist.

htmlComments.push({ text: comment, position: node.position, });

https://github.com/andreahlert/markdown/blob/dca9351e298b2dd18c897774f69776e49442b148/src/processor.js#L355

However, the current position information is hard-coded and not derived from the existing type.

Could we add at the top level import type { Position } from "unist"; and reuse that type?

The current type testing in typest.test.ts relies on this type information, so the type tests will need to be adjusted accordingly.

lumirlumir · 2026-04-13T11:50:09Z

 export interface BlockBase {
 	baseIndentText: string;
 	comments: string[];
+	commentInfos: MappedCommentLocation[];


As noted in #618 (comment), I also think that the commentInfos type can be removed and consolidated into the comments type to minimize changes.

For example, if we remove MappedCommentLocation type and simply create a Comment type as follows:

export interface Comment { text: string; position: Position; }

And update BlockBase as follows:

export interface BlockBase { // ... - comments: string[]; + comments: Comment[]; // ... }

lumirlumir · 2026-04-13T11:56:31Z

+	 * original HTML comment info. This allows us to map messages that point to
+	 * configuration comments back to their original location in the Markdown.
+	 */
+	const commentLineMap = new Map();


Suggested change

const commentLineMap = new Map();

/** @type {...} */

const commentLineMap = new Map();

Could we specify its type using JSDoc syntax? Currently it's declared as const commentLineMap: Map<any, any>, so the type information is lost when other code consumes it.

lumirlumir · 2026-04-13T12:06:04Z

+			if (commentInfo) {
+				return {
+					...message,
+					line: commentInfo.position.start.line,


From what I can tell, the current commentInfo type is derived from the Markdown HTML node's position field, which contains full position information including start.line, start.column, end.line, and end.column.

Also, the comment is sanitized by the getComment helper before being pushed into the htmlComments array:

https://github.com/andreahlert/markdown/blob/6855867919761bc1eb57df1c1bbfb5fb4a307d47/src/processor.js#L70-L89

Therefore, the comment syntax should be valid and retain full position information (verified via logging).

Losing the position information causes the following issue: only the first character is highlighted instead of the entire line.

Current

Expected

I believe retaining the full position information here is necessary.

lumirlumir · 2026-04-28T10:27:18Z

Could you take a look at the merge conflict as well?

eslint-github-bot Bot added the bug label Feb 5, 2026

eslint-github-bot Bot mentioned this pull request Feb 5, 2026

eslint --report-unused-disable-directives does not work with this plugin #115

Open

eslintbot added this to Triage Feb 5, 2026

github-project-automation Bot moved this to Needs Triage in Triage Feb 5, 2026

andreahlert force-pushed the fix/report-unused-disable-directives branch from 31d9362 to dcccf5a Compare February 6, 2026 12:52

Ari4ka approved these changes Feb 8, 2026

View reviewed changes

DMartens reviewed Feb 11, 2026

View reviewed changes

Comment thread docs/processors/markdown.md

DMartens reviewed Feb 11, 2026

View reviewed changes

Comment thread src/processor.js Outdated

DMartens reviewed Feb 11, 2026

View reviewed changes

Comment thread src/processor.js

DMartens reviewed Feb 11, 2026

View reviewed changes

DMartens moved this from Needs Triage to Implementing in Triage Feb 11, 2026

Ari4ka approved these changes Feb 11, 2026

View reviewed changes

Ari4ka approved these changes Feb 17, 2026

View reviewed changes

andreahlert requested review from Ari4ka, DMartens and nzakas February 23, 2026 06:28

Ari4ka approved these changes Feb 23, 2026

View reviewed changes

DMartens reviewed Mar 5, 2026

View reviewed changes

andreahlert added 2 commits March 12, 2026 04:35

andreahlert force-pushed the fix/report-unused-disable-directives branch from e4ed80c to aaa9b03 Compare March 12, 2026 07:48

andreahlert requested review from DMartens, lumirlumir and nzakas March 12, 2026 08:27

github-actions Bot added the Stale label Mar 22, 2026

Ari4ka approved these changes Mar 23, 2026

View reviewed changes

nzakas previously approved these changes Mar 23, 2026

View reviewed changes

nzakas moved this from Implementing to Second Review Needed in Triage Mar 23, 2026

github-actions Bot removed the Stale label Mar 23, 2026

lumirlumir requested changes Mar 25, 2026

View reviewed changes

fix: update type tests for MappedCommentLocation and fix formatting

dca9351

Signed-off-by: André Ahlert <andre@aex.partners>

andreahlert dismissed nzakas’s stale review via dca9351 March 25, 2026 07:19

andreahlert requested review from lumirlumir and nzakas March 25, 2026 07:56

Ari4ka approved these changes Mar 25, 2026

View reviewed changes

github-actions Bot added the Stale label Apr 4, 2026

fasttime removed the Stale label Apr 7, 2026

Ari4ka approved these changes Apr 7, 2026

View reviewed changes

lumirlumir reviewed Apr 9, 2026

View reviewed changes

lumirlumir requested changes Apr 13, 2026

View reviewed changes

lumirlumir added the accepted label Apr 13, 2026

	const commentLineMap = new Map();
	/** @type {...} */
	const commentLineMap = new Map();

Conversation

andreahlert commented Feb 5, 2026

Summary

Before

After

Test plan

Uh oh!

linux-foundation-easycla Bot commented Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andreahlert commented Feb 7, 2026

Uh oh!

andreahlert commented Feb 8, 2026

Uh oh!

nzakas commented Feb 9, 2026

Uh oh!

andreahlert commented Feb 10, 2026

Uh oh!

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!

lumirlumir Apr 13, 2026 • 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!

DMartens commented Feb 11, 2026

Uh oh!

lumirlumir commented Feb 15, 2026

Uh oh!

andreahlert commented Feb 23, 2026

Uh oh!

andreahlert commented Mar 5, 2026

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DMartens commented Mar 5, 2026

Uh oh!

andreahlert commented Mar 12, 2026

Uh oh!

andreahlert commented Mar 12, 2026

Uh oh!

github-actions Bot commented Mar 22, 2026

Uh oh!

nzakas left a comment

Choose a reason for hiding this comment

Uh oh!

nzakas commented Mar 23, 2026

Uh oh!

lumirlumir left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Apr 4, 2026

Uh oh!

fasttime commented Apr 7, 2026

Uh oh!

lumirlumir left a comment

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!

linux-foundation-easycla Bot commented Feb 5, 2026 •

edited

Loading

lumirlumir Apr 13, 2026 •

edited

Loading

lumirlumir Apr 13, 2026 •

edited

Loading

lumirlumir Apr 13, 2026 •

edited

Loading