Skip to content

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

Closed
wagenet wants to merge 6 commits intooxc-project:mainfrom
wagenet:stray-comments
Closed

fix(linter): remove entire comment line when deleting unused disable directives#19804
wagenet wants to merge 6 commits intooxc-project:mainfrom
wagenet:stray-comments

Conversation

@wagenet
Copy link
Copy Markdown
Contributor

@wagenet wagenet commented Feb 27, 2026

Summary

  • When removing unused eslint-disable/oxlint-disable comments, the auto-fix previously only deleted the comment content (via Comment::content_span()), leaving behind empty // or /* */ delimiters
  • Adds full_comment_delete_span() helper that expands the deletion span to include comment delimiters and, when the comment is the sole content on its line, the entire line
  • Applied in both the CLI --fix path and the LSP code action path

Test plan

  • 8 new unit tests for full_comment_delete_span covering line/block comments, inline vs own-line, start/end of file, CRLF, leading whitespace
  • cargo test -p oxc_linter (981 tests pass)
  • cargo test -p oxlint (254 tests pass, snapshot updated)
  • cargo clippy -p oxc_linter -p oxlint clean

🤖 Generated with Claude Code

…directives

When removing unused `eslint-disable`/`oxlint-disable` comments, the fix
previously only deleted the comment content (via `Comment::content_span()`),
leaving behind empty `//` or `/* */` delimiters. This adds
`full_comment_delete_span()` which expands the deletion span to include
delimiters and, when the comment is the sole content on its line, the
entire line. Applied in both the CLI `--fix` path and the LSP path.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings February 27, 2026 04:13
@wagenet wagenet requested a review from camc314 as a code owner February 27, 2026 04:13
@github-actions github-actions Bot added A-linter Area - Linter A-cli Area - CLI C-bug Category - Bug labels Feb 27, 2026
…e_span

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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 fixes an issue where auto-fix for unused eslint-disable/oxlint-disable comments would leave behind empty comment delimiters (// or /* */). The fix introduces a new helper function that expands the deletion span to include comment delimiters and, when appropriate, the entire line containing the comment.

Changes:

  • Added full_comment_delete_span() helper function to expand comment content spans to full comment deletion spans
  • Applied the helper in both CLI (context/host.rs) and LSP (error_with_position.rs) code paths for unused disable directive fixes
  • Added comprehensive test coverage with 8 unit tests covering various edge cases

Reviewed changes

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

Show a summary per file
File Description
crates/oxc_linter/src/lib.rs Exported the new full_comment_delete_span function
crates/oxc_linter/src/disable_directives.rs Implemented full_comment_delete_span() with logic to expand comment spans and comprehensive unit tests
crates/oxc_linter/src/context/host.rs Updated CLI auto-fix path to use full_comment_delete_span for RuleCommentType::All cases
apps/oxlint/src/lsp/error_with_position.rs Updated LSP code action path to use full_comment_delete_span for RuleCommentType::All cases
apps/oxlint/src/lsp/snapshots/fixtures_lsp_unused_disabled_directives@test.js.snap Updated snapshot to reflect new deletion behavior (entire lines instead of just comment content)

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Feb 27, 2026

Merging this PR will not alter performance

✅ 4 untouched benchmarks
⏩ 51 skipped benchmarks1


Comparing wagenet:stray-comments (a212ecb) with main (81bab90)

Open in CodSpeed

Footnotes

  1. 51 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.

Comment thread crates/oxc_linter/src/disable_directives.rs Outdated
Comment thread crates/oxc_linter/src/disable_directives.rs Outdated
@camc314 camc314 self-assigned this Feb 27, 2026
Copilot AI and others added 3 commits February 27, 2026 19:20
…st submodule

Co-authored-by: wagenet <9835+wagenet@users.noreply.github.com>
fix(linter): address review feedback on full_comment_delete_span
@camc314
Copy link
Copy Markdown
Contributor

camc314 commented Apr 7, 2026

fixed by #21092

@camc314 camc314 closed this Apr 7, 2026
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 C-bug Category - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants