Skip to content

feat(linter/plugins): implement SourceCode.getDisableDirectives method#21029

Merged
camc314 merged 4 commits into
oxc-project:mainfrom
KuSh:get-disable-directives
May 4, 2026
Merged

feat(linter/plugins): implement SourceCode.getDisableDirectives method#21029
camc314 merged 4 commits into
oxc-project:mainfrom
KuSh:get-disable-directives

Conversation

@KuSh
Copy link
Copy Markdown
Contributor

@KuSh KuSh commented Apr 4, 2026

Used by eslint-plugin-unicorn

It crashes when trying to use it through jsPlugins (for unicorn/expiring-todo-comments)

ESLint documentation says its optional but unicorn doesn't seem to guard against its absence

@github-actions github-actions Bot added A-linter Area - Linter A-cli Area - CLI A-linter-plugins Area - Linter JS plugins C-enhancement Category - New feature or request labels Apr 4, 2026
@KuSh KuSh force-pushed the get-disable-directives branch 4 times, most recently from 9d9ee06 to 91545c1 Compare April 4, 2026 02:04
@KuSh KuSh changed the title feat(linter/plugins): expose sourceCode.getDisableDirectives() method feat(linter/plugins): expose SourceCode.getDisableDirectives() method Apr 4, 2026
@KuSh KuSh force-pushed the get-disable-directives branch from 91545c1 to 053bc11 Compare April 4, 2026 02:10
@KuSh KuSh force-pushed the get-disable-directives branch from 053bc11 to 41e1f45 Compare April 7, 2026 20:49
@overlookmotel
Copy link
Copy Markdown
Member

overlookmotel commented Apr 23, 2026

Sorry I'm so slow to get to this PR.

As I mentioned on #21492, ideal solution would be to pass the disable comments from Rust side (as we're already doing the work of parsing comments there), but I'm not sure how easy that would be.

A pure-JS solution like in this PR is a good interim solution (and who knows how long it'll be until we get to doing it "properly", so "interim" might actually mean quite a long time).

All that's lacking, I think, is:

  1. Tests (apps/oxlint/test/fixtures).
  2. A citation showing the match patterns match either (a) ESLint or (b) whatever pattern we use on Rust side in Oxlint.

@KuSh Would you have the time and inclination to add those to this PR?

@KuSh
Copy link
Copy Markdown
Contributor Author

KuSh commented Apr 23, 2026

Yes sure, I'll see if I find time in the next weeks! Thanks for the feedback @overlookmotel

I thought of trying to pass the disable comments from Rust side, but as that part seems to be touchy and hard to optimize, and that specific function doesn't seem to be used a lot by plugins. I tought I would propose a simpler first version to have feedback ^^

@KuSh KuSh force-pushed the get-disable-directives branch from 41e1f45 to 7e58a8c Compare April 25, 2026 21:51
@KuSh
Copy link
Copy Markdown
Contributor Author

KuSh commented Apr 25, 2026

@overlookmotel I've added a fixture test. I've also added a comment about what patterns match. I hope that's what you had in mind.

@christophehurpeau
Copy link
Copy Markdown

This is the last thing missing to fully migrate to oxlint. If this PR is merged it would be much appreciated 🙏

@camc314 camc314 changed the title feat(linter/plugins): expose SourceCode.getDisableDirectives() method feat(linter/plugins): implement SourceCode.getDisableDirectives method May 4, 2026
@camc314
Copy link
Copy Markdown
Contributor

camc314 commented May 4, 2026

As I mentioned on #21492, ideal solution would be to pass the disable comments from Rust side (as we're already doing the work of parsing comments there), but I'm not sure how easy that would be.

This is going to be a bunch of extra work, which I don't think is currently worth it. We can address this in future

A pure-JS solution like in this PR is a good interim solution (and who knows how long it'll be until we get to doing it "properly", so "interim" might actually mean quite a long time).

This has been implemented - I've pushed up a follow up commit added support for the enable comments.

All that's lacking, I think, is:

  1. Tests (apps/oxlint/test/fixtures).
  2. A citation showing the match patterns match either (a) ESLint or (b) whatever pattern we use on Rust side in Oxlint.

@ KuSh Would you have the time and inclination to add those to this PR?

I think both of these should be done!

@camc314
Copy link
Copy Markdown
Contributor

camc314 commented May 4, 2026

@overlookmotel going to merge this now-ish, if you could take a retrospective look that would be appreciated!

thanks!

Comment thread apps/oxlint/src-js/plugins/directives.ts
@camc314 camc314 assigned camc314 and unassigned overlookmotel May 4, 2026
Copy link
Copy Markdown
Contributor

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 PR adds SourceCode.getDisableDirectives() to the JS plugin SourceCode surface so ESLint-compatible plugins (notably unicorn helpers) can inspect inline disable/enable comments instead of crashing when the method is missing.

Changes:

  • Adds a new directives.ts helper that parses eslint-* / oxlint-* disable and enable comments into { problems, directives }.
  • Exposes getDisableDirectives on the shared JS plugin SOURCE_CODE object.
  • Adds a new CLI fixture covering plugin access to the method and snapshotting the reported directive counts.

Reviewed changes

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

Show a summary per file
File Description
apps/oxlint/test/fixtures/getDisableDirectives/plugin.ts New fixture plugin that calls sourceCode.getDisableDirectives() and asserts returned counts/shape.
apps/oxlint/test/fixtures/getDisableDirectives/output.snap.md Snapshot for the new fixture’s expected lint output.
apps/oxlint/test/fixtures/getDisableDirectives/files/test.js Test input containing various oxlint/eslint disable and enable directives.
apps/oxlint/test/fixtures/getDisableDirectives/.oxlintrc.json Enables the new fixture plugin/rule.
apps/oxlint/src-js/plugins/source_code.ts Wires the new directive helper onto the public SourceCode API.
apps/oxlint/src-js/plugins/directives.ts Implements directive parsing and malformed-comment problem collection.

Comment thread apps/oxlint/src-js/plugins/directives.ts
Comment thread apps/oxlint/src-js/plugins/directives.ts
Copy link
Copy Markdown
Contributor

@camc314 camc314 left a comment

Choose a reason for hiding this comment

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

thank you!

@camc314 camc314 merged commit 1884833 into oxc-project:main May 4, 2026
28 checks passed
@KuSh KuSh deleted the get-disable-directives branch May 4, 2026 20:34
@KuSh
Copy link
Copy Markdown
Contributor Author

KuSh commented May 7, 2026

@camc314 follow up PR #22238 as this wasn't enough to make unicorn work (at least for expiring-todo-comments rule)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-cli Area - CLI A-linter Area - Linter A-linter-plugins Area - Linter JS plugins C-enhancement Category - New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants