fix(linter): mark complete comment for unused disable directives + lsp fix by Copilot · Pull Request #21092 · oxc-project/oxc

Copilot · 2026-04-06T15:21:40Z

When auto-fixing unused eslint-disable / oxlint-disable directives, only the comment content was stripped — leaving behind empty comment tokens (// or /**/) instead of removing the line entirely.

Root cause: The comment_span stored in DisabledRule was set to comment.content_span() (the text inside the delimiters), so Fix::delete(span) left the // prefix and */ suffix in place.

Changes

disable_directives.rs — Added DisableDirectivesBuilder::compute_comment_fix_span static helper and a fix_span field alongside comment_span in both DisabledRule::All and DisabledRule::Single:
- comment_span = comment.span (full outer comment span, used for diagnostic labels — no line extension)
- fix_span = line-aware span: the entire line (indentation + newline included) when the comment is the only non-whitespace content on its line; otherwise the full outer comment span (including // / /* */ delimiters)
- DisableRuleComment gains a matching fix_span field; its span field remains the original comment span for diagnostics
context/host.rs and apps/oxlint/src/lsp/error_with_position.rs — use span for .with_label (diagnostic highlighting) and fix_span for Fix::delete (the actual edit)

Before / after:

// Before fix applied:
// eslint-disable-next-line no-console   →   //
/* eslint-disable no-console */          →   /**/

// After fix applied:
// eslint-disable-next-line no-console   →   (line removed)
/* eslint-disable no-console */          →   (line removed)

Multi-rule comments where only some rules are unused are unaffected — the existing per-rule comma-aware removal logic (RuleCommentType::Single) continues to work correctly.

camc314 · 2026-04-06T15:27:34Z

@Sysix maybe #19804 ?

I haven't reviewed

Sysix · 2026-04-06T15:31:38Z

@camc314 oh I didn't know there was a PR already created :)
I hope copilot will just adjust the DisabledRule:All comment span 🤞

camc314 · 2026-04-06T15:33:00Z

I hope copilot will just adjust the DisabledRule:All comment span 🤞

Probably a better approach.

Feel free to close whichever has the worst approach.

Sysix · 2026-04-06T16:09:20Z

@copilot try to avoid changing the LSP side. The fix would "maybe" later apply to CLI too.
My first guess is DisabledRule::All comment span assignement in fn build_impl(&mut self, source_text: &str, comments: &[Comment]).
The comment span is not comment.content_span, it is comment.span, which will cover line and block comments.
Try to refactore the code

Copilot · 2026-04-06T16:40:04Z

@copilot try to avoid changing the LSP side. The fix would "maybe" later apply to CLI too.
My first guess is DisabledRule::All comment span assignement in `fn build_impl(&mut self, source_text: &st...

Refactored in 5c19ae0. The create_line_fix method is removed. Instead, DisableDirectivesBuilder::build_impl now computes a comment_fix_span for every disable directive (via the new compute_comment_fix_span static helper) and stores that in both DisabledRule::All and DisabledRule::Single. The span is the full outer comment span (including ////* */ delimiters), expanded to the whole line (indentation + newline) when the comment is the only content on that line. The existing Fix::delete(span) calls in host.rs and error_with_position.rs are unchanged and just work correctly.

Sysix · 2026-04-06T17:49:53Z

@copilot the diagnostic span should not include the line breaks checks. It only should be the original comment span

Copilot · 2026-04-06T18:04:12Z

@copilot the diagnostic span should not include the line breaks checks. It only should be the original comment span

Fixed in ad72d4b. DisableRuleComment now has two separate spans:

span = original comment.span (outer comment only, no line extension) — used for .with_label / diagnostic range
fix_span = line-aware span (whole line when comment is alone, outer span otherwise) — used for Fix::delete

The diagnostic range in the LSP snapshot now correctly shows only the comment bounds (e.g. {line:0, char:0}–{line:0, char:56}), while the code action TextEdit still deletes the entire line ({line:0, char:0}–{line:1, char:0}).

Sysix · 2026-04-07T17:59:08Z

@copilot fix the lint errors

codspeed-hq · 2026-04-07T18:04:20Z

Merging this PR will not alter performance

✅ 4 untouched benchmarks
⏩ 47 skipped benchmarks¹

_{Comparing copilot/fix-unused-eslint-disable-directives (2ef6f8d) with main (8c51121)}

47 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

Copilot · 2026-04-07T18:12:45Z

@copilot fix the lint errors

Fixed in c5dd96c:

Replaced .map_or(true, |(_, before)| before.trim().is_empty()) with .is_none_or(|(_, before)| before.trim().is_empty()) in two test locations
Updated the NAPI TypeScript snapshot for the new diagnostic range (full outer comment span)
Restored the insta Rust snapshot trailing whitespace that was lost in the merge conflict resolution

Sysix · 2026-04-07T19:11:06Z

@camc314 I do not like this solution either, but it is imo "cleaner" to handle this on the directives side, instead of LSP.
Adjusted the diagnostics span as a little bonus too 🥳

Feel free to merge one of them :) ,
I looked into #19804, there are some assumptions about the comment spans (+- 2 chars to get the comment span).

camc314

💪 nice!

camc314 · 2026-04-07T20:54:32Z

Merge activity

Apr 7, 8:54 PM UTC: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
Apr 7, 8:54 PM UTC: camc314 added this pull request to the Graphite merge queue.
Apr 7, 8:58 PM UTC: Merged by the Graphite merge queue.

…p fix (#21092) When auto-fixing unused `eslint-disable` / `oxlint-disable` directives, only the comment *content* was stripped — leaving behind empty comment tokens (`//` or `/**/`) instead of removing the line entirely. **Root cause:** The `comment_span` stored in `DisabledRule` was set to `comment.content_span()` (the text inside the delimiters), so `Fix::delete(span)` left the `//` prefix and `*/` suffix in place. ## Changes - **`disable_directives.rs`** — Added `DisableDirectivesBuilder::compute_comment_fix_span` static helper and a `fix_span` field alongside `comment_span` in both `DisabledRule::All` and `DisabledRule::Single`: - `comment_span` = `comment.span` (full outer comment span, used for **diagnostic labels** — no line extension) - `fix_span` = line-aware span: the **entire line** (indentation + newline included) when the comment is the only non-whitespace content on its line; otherwise the full outer comment span (including `//` / `/* */` delimiters) - `DisableRuleComment` gains a matching `fix_span` field; its `span` field remains the original comment span for diagnostics - **`context/host.rs`** and **`apps/oxlint/src/lsp/error_with_position.rs`** — use `span` for `.with_label` (diagnostic highlighting) and `fix_span` for `Fix::delete` (the actual edit) **Before / after:** ```js // Before fix applied: // eslint-disable-next-line no-console → // /* eslint-disable no-console */ → /**/ // After fix applied: // eslint-disable-next-line no-console → (line removed) /* eslint-disable no-console */ → (line removed) ``` Multi-rule comments where only *some* rules are unused are unaffected — the existing per-rule comma-aware removal logic (`RuleCommentType::Single`) continues to work correctly.

# Oxlint ### 💥 BREAKING CHANGES - 382958a span: [**BREAKING**] Remove re-exports of string types from `oxc_span` crate (#21246) (overlookmotel) - c4aedfa str: [**BREAKING**] Add `static_ident!` macro (#21245) (overlookmotel) - 7354f3c linter: [**BREAKING**] Error on no matched files (#21144) (camc314) ### 🚀 Features - 91f2c79 linter/eslint-jest-plugin: Implemented `prefer-importing-jest-globals` rule (#21303) (Said Atrahouch) - a02f32c linter: Add release version for existing rules (#21363) (camchenry) - b9e93da linter: Allow tagging rules with release version (#21362) (camchenry) - f99ecda oxlint: Gate `vite.config.ts` recognition behind `VP_VERSION` env var (#21298) (leaysgur) - cf459d3 linter: Implement suggestion for `no-empty-function` rule (#21347) (Mikhail Baev) - 7213d61 linter: Adding pending suggestions fix to `valid_expect` rules. (#21249) (Said Atrahouch) - ae45312 linter: Introduce `--type-check-only` flag (#21184) (camc314) - 1ce8b90 linter: Implemented `valid-expect-in-promise` vitest and jest rule (#21170) (Said Atrahouch) - 39f7fda linter: Add auto-fix to `unicorn/prefer-default-parameters` (#21166) (yefan) - 15574bc linter/unicorn: Implement consistent-template-literal-escape (#21126) (AliceLanniste) - c5c8c03 linter/prefer-readonly-parameter-types: Move rule from nursery to pedantic (#21114) (camc314) - 1893be1 linter/no-useless-default-assignment: Move rule from nursery to correctness (#21113) (camc314) - 5462ff9 linter/strict-void-return: Move rule from nursery to pedantic (#21115) (camc314) - c2989bd linter/no-unnecessary-type-parameters: Move rule from nursery to suspicious (#21112) (camc314) - 79d339a linter/no-unnecessary-qualifier: Move rule from nursery to style (#21111) (camc314) ### 🐛 Bug Fixes - b577efc linter/unicorn: Handle optional chaining in `prefer-array-flat` and `no-invalid-remove-event-listener` (#21299) (Mikhail Baev) - 5e55735 oxlint/lsp: Skip .git directories in LSP walkers (#21316) (camc314) - ec7f6ed oxlint, oxfmt: Apply `check_for_writer_error` to `.flush()` (#21343) (Craig Morrison) - a17a08a linter/no-useless-assignment: Handle continue edges in loop analysis (#21358) (camc314) - a0eac12 linter/array-type: Move match to first stmt (#21357) (camc314) - 1b3abc3 linter: Exclude boundary tokens from JSXText whitespace check in isSpaceBetweenTokens (#21313) (bab) - ecbcf5e linter: More info to summary output for GitHub formatter (#21330) (Théo LUDWIG) - a0a8c62 linter/no-fallthrough: Check from start of switch case for empty lines (#21324) (Josh Cartmell) - 36f0bc4 linter/no-cycle: Report all cyclic dependencies inside a file (#21259) (camc314) - 3f80536 linter: Ignore regex flags other than `g`/`u`/`v` in `prefer-string-replace-all` (#21203) (bab) - f21d3aa linter/unicorn: Report on optional in `require-number-to-fixed-digits-argument` rule (#21207) (Mikhail Baev) - af8e122 linter: Render each config error as a separate diagnostic (#21120) (bab) - a950f55 linter/unicorn: Do not report on optionals in `no-single-promise-in-promise-methods` (#21157) (Mikhail Baev) - 472f8ee linter: Mark complete comment for unused disable directives + lsp fix (#21092) (copilot-swe-agent) - edd0865 linter/no-array-index-key: False positive when index is inside an expression within a template literal (#21123) (bab) - 7e8d520 linter/unicorn: Report on optional `foo?.postMessage` in `require-post-message-target-origin` rule (#21104) (Mikhail Baev) ### ⚡ Performance - addcd02 napi/parser, linter/plugins: Raw transfer deserializer for `Vec`s use shift instead of multiply where possible (#21142) (overlookmotel) - 3068ded napi/parser, linter/plugins: Shift before add when calculating positions in raw transfer deserializer (#21141) (overlookmotel) - eb400b8 napi/parser, linter/plugins: Remove `uint32` buffer view (#21140) (overlookmotel) - 7a86613 linter/plugins: Use `Int32Array`s for tokens and comments buffers (#21136) (overlookmotel) - 8c51121 napi/parser, linter/plugins: Raw transfer deserialize `Span` fields as `i32`s (#21135) (overlookmotel) - bc1bcdd napi/parser, linter/plugins: Inline trivial raw transfer field deserializers into node object definitions (#21134) (overlookmotel) - c0278ab napi/parser, linter/plugins: Use `Int32Array` in raw transfer deserializer (#21132) (overlookmotel) - 43482c7 linter/plugins: Use `>>` not `>>>` in binary search loops (#21129) (overlookmotel) ### 📚 Documentation - 7888280 linter: Move config docs for `no-restricted-exports` (#21360) (camchenry) - 162d26c linter: Improve docs for `typescript/array-type` (#21356) (camchenry) - a2dbaec linter: Add missing docs for options for `typescript/class-literal-property-style` (#21355) (camchenry) - 79593eb linter: Improve docs for `typescript/consistent-type-assertions` (#21353) (camchenry) - f9d20d2 linter: Move config option docs for `typescript/no-empty-object-type` (#21352) (camchenry) - a8f650d linter: Add missing config option docs for `prefer-string-start-ends-with` (#21332) (camchenry) - cfd8a4f linter: Don't rely on old eslint doc for available globals (#21334) (Nicolas Le Cam) - 03865fa linter: Jest/prefer-snapshot-hint: add doc comment for snapshot hint mode (#21290) (camchenry) - a6fe09b linter: Add missing docs for config options in `react` plugin (#21289) (camchenry) - 60eaf47 linter: Add missing docs for config options in unicorn plugin (#21288) (camchenry) - c3c2055 linter: `jsx-a11y/label-has-associated-control`: document the `assert` options (#21287) (camchenry) - a928ed9 linter: Add missing config docs for vitest plugin rules (#21285) (camchenry) - 7e07c7c linter: `id-length`: move enum docs to doc comments (#21281) (camchenry) - 9746bdf linter: Add missing docs for `class-methods-use-this` config (#21278) (camchenry) - 6ffe7a5 linter: Move docs for `Target` variant onto enum (#21277) (camchenry) - 305350d linter/plugins: Correct comments (#21130) (overlookmotel) # Oxfmt ### 💥 BREAKING CHANGES - 382958a span: [**BREAKING**] Remove re-exports of string types from `oxc_span` crate (#21246) (overlookmotel) ### 🚀 Features - e3081e1 oxfmt: Gate `vite.config.ts` recognition behind `VP_VERSION` env var (#21295) (leaysgur) - 5b0b573 oxfmt: Update prettier to 3.8.2 (#21294) (leaysgur) - 0d67834 oxfmt: Show hint for all files are ignored case (#21154) (leaysgur) ### 🐛 Bug Fixes - 2871fc2 oxfmt: Non idempotent formatting on comments in TS (#20449) (Cat Chen) - ec7f6ed oxlint, oxfmt: Apply `check_for_writer_error` to `.flush()` (#21343) (Craig Morrison) - 1a8c225 formatter: Preserve newline between self-closing JSX element and single-char text (#21149) (Justin Mecham) - 407b725 oxfmt: Indent dangling comments in empty enum with block indent (#21163) (Leonabcd123) - d13fd37 formatter: Remove extra outer parentheses on return with JSDoc type cast (#21109) (bab) - 22babde oxfmt: Fix unicode char escaping (#21162) (leaysgur) - 4da53e5 formatter: Preserve trailing comma in TSX arrow functions with default type params (#21151) (Justin Mecham) - 94fe774 oxfmt: Handle paths with consecutive leading slashes (#21155) (leaysgur) - 50c389b oxfmt: Support `.editorconfig` `quote_type` (#20989) (leaysgur) ### ⚡ Performance - 0ce619f formatter: Use `Allocator::alloc_concat_strs_array` instead of `StringBuilder::from_strs_array_in` (#21339) (overlookmotel)

Copilot AI assigned Copilot and Sysix Apr 6, 2026

Copilot started work on behalf of Sysix April 6, 2026 15:21 View session

Copilot AI linked an issue Apr 6, 2026 that may be closed by this pull request

oxlint(lsp): Auto-fix for unused eslint-disable directives should delete the line, not leave blank comments #21088

Closed

2 tasks

Copilot AI changed the title ~~[WIP] Fix auto-fix for unused eslint-disable directives~~ fix(lsp): delete whole line when removing unused disable directives Apr 6, 2026

Copilot AI requested a review from Sysix April 6, 2026 15:43

Copilot finished work on behalf of Sysix April 6, 2026 15:43

Copilot started work on behalf of Sysix April 6, 2026 16:09 View session

Copilot AI changed the title ~~fix(lsp): delete whole line when removing unused disable directives~~ fix(linter): delete whole line when removing unused disable directives Apr 6, 2026

Copilot finished work on behalf of Sysix April 6, 2026 16:41

Copilot started work on behalf of Sysix April 6, 2026 17:50 View session

Copilot finished work on behalf of Sysix April 6, 2026 18:05

github-actions Bot added A-linter Area - Linter A-cli Area - CLI A-editor Area - Editor and Language Server C-bug Category - Bug labels Apr 7, 2026

Copilot started work on behalf of Sysix April 7, 2026 17:59 View session

Copilot finished work on behalf of Sysix April 7, 2026 18:13

github-actions Bot added the A-linter-plugins Area - Linter JS plugins label Apr 7, 2026

Sysix marked this pull request as ready for review April 7, 2026 19:08

Sysix requested review from camc314 and overlookmotel as code owners April 7, 2026 19:08

Sysix approved these changes Apr 7, 2026

View reviewed changes

Sysix changed the title ~~fix(linter): delete whole line when removing unused disable directives~~ fix(linter): mark complete comment for unused disable directives + lsp fix Apr 7, 2026

camc314 approved these changes Apr 7, 2026

View reviewed changes

camc314 added the 0-merge Merge with Graphite Merge Queue label Apr 7, 2026

camc314 mentioned this pull request Apr 7, 2026

fix(linter): remove entire comment line when deleting unused disable directives #19804

Closed

4 tasks

camc314 reviewed Apr 7, 2026

View reviewed changes

graphite-app Bot force-pushed the copilot/fix-unused-eslint-disable-directives branch from 2ef6f8d to 472f8ee Compare April 7, 2026 20:54

graphite-app Bot merged commit 472f8ee into main Apr 7, 2026
25 of 26 checks passed

graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label Apr 7, 2026

graphite-app Bot deleted the copilot/fix-unused-eslint-disable-directives branch April 7, 2026 20:58

Boshen mentioned this pull request Apr 13, 2026

release(apps): oxlint v1.60.0 && oxfmt v0.45.0 #21375

Merged

Uh oh!

Conversation

Copilot AI commented Apr 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Uh oh!

camc314 commented Apr 6, 2026

Uh oh!

Sysix commented Apr 6, 2026

Uh oh!

camc314 commented Apr 6, 2026

Uh oh!

Sysix commented Apr 6, 2026

Uh oh!

Copilot AI commented Apr 6, 2026

Uh oh!

Sysix commented Apr 6, 2026

Uh oh!

Copilot AI commented Apr 6, 2026

Uh oh!

Sysix commented Apr 7, 2026

Uh oh!

codspeed-hq Bot commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Footnotes

Uh oh!

Copilot AI commented Apr 7, 2026

Uh oh!

Sysix commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

camc314 left a comment

Choose a reason for hiding this comment

Uh oh!

camc314 commented Apr 7, 2026 • edited by graphite-app Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge activity

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Apr 6, 2026 •

edited

Loading

codspeed-hq Bot commented Apr 7, 2026 •

edited

Loading

Sysix commented Apr 7, 2026 •

edited

Loading

camc314 commented Apr 7, 2026 •

edited by graphite-app Bot

Loading