fix(linter): remove entire comment line when deleting unused disable directives#19804
fix(linter): remove entire comment line when deleting unused disable directives#19804wagenet wants to merge 6 commits intooxc-project:mainfrom
Conversation
…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>
…e_span Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
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) |
Merging this PR will not alter performance
Comparing Footnotes
|
…st submodule Co-authored-by: wagenet <9835+wagenet@users.noreply.github.com>
fix(linter): address review feedback on full_comment_delete_span
|
fixed by #21092 |
Summary
eslint-disable/oxlint-disablecomments, the auto-fix previously only deleted the comment content (viaComment::content_span()), leaving behind empty//or/* */delimitersfull_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--fixpath and the LSP code action pathTest plan
full_comment_delete_spancovering line/block comments, inline vs own-line, start/end of file, CRLF, leading whitespacecargo test -p oxc_linter(981 tests pass)cargo test -p oxlint(254 tests pass, snapshot updated)cargo clippy -p oxc_linter -p oxlintclean🤖 Generated with Claude Code