From 644dfd43a295343319e7173933f47ce2846d986d Mon Sep 17 00:00:00 2001 From: Sysix <3897725+Sysix@users.noreply.github.com> Date: Wed, 22 Oct 2025 10:39:33 +0000 Subject: [PATCH] fix(language_server): make unused directives fixable again (#14872) https://github.com/oxc-project/oxc/pull/14052 created diagnostics without a fix. Fixed it for the language server, maybe the CLI should support this too with `--fix --report-unused-disable-directives`. It was at least possible before the PR. --- .../src/linter/isolated_lint_handler.rs | 96 +++++++++++++++---- ...er_unused_disabled_directives@test.js.snap | 57 ++++++++++- editors/vscode/tests/code_actions.spec.ts | 71 +++++++------- 3 files changed, 165 insertions(+), 59 deletions(-) diff --git a/crates/oxc_language_server/src/linter/isolated_lint_handler.rs b/crates/oxc_language_server/src/linter/isolated_lint_handler.rs index e5c5ff86706b3..fb0e76ed6931b 100644 --- a/crates/oxc_language_server/src/linter/isolated_lint_handler.rs +++ b/crates/oxc_language_server/src/linter/isolated_lint_handler.rs @@ -10,9 +10,9 @@ use tower_lsp_server::{UriExt, lsp_types::Uri}; use oxc_allocator::Allocator; use oxc_linter::{ - AllowWarnDeny, ConfigStore, DirectivesStore, DisableDirectives, LINTABLE_EXTENSIONS, - LintOptions, LintService, LintServiceOptions, Linter, Message, RuntimeFileSystem, - create_unused_directives_diagnostics, read_to_arena_str, read_to_string, + AllowWarnDeny, ConfigStore, DirectivesStore, DisableDirectives, Fix, LINTABLE_EXTENSIONS, + LintOptions, LintService, LintServiceOptions, Linter, Message, PossibleFixes, RuleCommentType, + RuntimeFileSystem, read_to_arena_str, read_to_string, }; use super::error_with_position::{ @@ -130,7 +130,7 @@ impl IsolatedLintHandler { && let Some(directives) = self.directives_coordinator.get(path) { messages.extend( - self.create_unused_directives_messages(&directives, severity) + create_unused_directives_messages(&directives, severity, source_text) .iter() .map(|message| message_to_lsp_diagnostic(message, uri, source_text, rope)), ); @@ -139,20 +139,6 @@ impl IsolatedLintHandler { messages } - #[expect(clippy::unused_self)] - fn create_unused_directives_messages( - &self, - directives: &DisableDirectives, - severity: AllowWarnDeny, - ) -> Vec { - let diagnostics = create_unused_directives_diagnostics(directives, severity); - diagnostics - .into_iter() - // TODO: unused directives should be fixable, `RuleCommentRule.create_fix()` can be used - .map(|diagnostic| Message::new(diagnostic, oxc_linter::PossibleFixes::None)) - .collect() - } - fn should_lint_path(path: &Path) -> bool { static WANTED_EXTENSIONS: OnceLock> = OnceLock::new(); let wanted_exts = @@ -163,3 +149,77 @@ impl IsolatedLintHandler { .is_some_and(|ext| wanted_exts.contains(ext)) } } + +/// Almost the same as [oxc_linter::create_unused_directives_diagnostics], but returns `Message`s +/// with a `PossibleFixes` instead of `OxcDiagnostic`s. +fn create_unused_directives_messages( + directives: &DisableDirectives, + severity: AllowWarnDeny, + source_text: &str, +) -> Vec { + use oxc_diagnostics::OxcDiagnostic; + + let mut diagnostics = Vec::new(); + let fix_message = "remove unused disable directive"; + + let severity = if severity == AllowWarnDeny::Deny { + oxc_diagnostics::Severity::Error + } else { + oxc_diagnostics::Severity::Warning + }; + + // Report unused disable comments + let unused_disable = directives.collect_unused_disable_comments(); + for unused_comment in unused_disable { + let span = unused_comment.span; + match unused_comment.r#type { + RuleCommentType::All => { + diagnostics.push(Message::new( + OxcDiagnostic::warn( + "Unused eslint-disable directive (no problems were reported).", + ) + .with_label(span) + .with_severity(severity), + PossibleFixes::Single(Fix::delete(span).with_message(fix_message)), + )); + } + RuleCommentType::Single(rules) => { + for rule in rules { + let rule_message = format!( + "Unused eslint-disable directive (no problems were reported from {}).", + rule.rule_name + ); + diagnostics.push(Message::new( + OxcDiagnostic::warn(rule_message) + .with_label(rule.name_span) + .with_severity(severity), + PossibleFixes::Single( + rule.create_fix(source_text, span).with_message(fix_message), + ), + )); + } + } + } + } + + // Report unused enable comments + let unused_enable = directives.unused_enable_comments(); + for (rule_name, span) in unused_enable { + let message = if let Some(rule_name) = rule_name { + format!( + "Unused eslint-enable directive (no matching eslint-disable directives were found for {rule_name})." + ) + } else { + "Unused eslint-enable directive (no matching eslint-disable directives were found)." + .to_string() + }; + diagnostics.push(Message::new( + OxcDiagnostic::warn(message).with_label(*span).with_severity(severity), + // TODO: fixer + // copy the structure of disable directives + PossibleFixes::None, + )); + } + + diagnostics +} diff --git a/crates/oxc_language_server/src/snapshots/fixtures_linter_unused_disabled_directives@test.js.snap b/crates/oxc_language_server/src/snapshots/fixtures_linter_unused_disabled_directives@test.js.snap index 78480e072ce5b..628c4ff2aea84 100644 --- a/crates/oxc_language_server/src/snapshots/fixtures_linter_unused_disabled_directives@test.js.snap +++ b/crates/oxc_language_server/src/snapshots/fixtures_linter_unused_disabled_directives@test.js.snap @@ -127,7 +127,24 @@ related_information[0].location.range: Range { start: Position { line: 0, charac severity: Some(Error) source: Some("oxc") tags: None -fixed: None +fixed: Single( + FixedContent { + message: Some( + "remove unused disable directive", + ), + code: "", + range: Range { + start: Position { + line: 0, + character: 2, + }, + end: Position { + line: 0, + character: 56, + }, + }, + }, +) code: "" @@ -140,7 +157,24 @@ related_information[0].location.range: Range { start: Position { line: 5, charac severity: Some(Error) source: Some("oxc") tags: None -fixed: None +fixed: Single( + FixedContent { + message: Some( + "remove unused disable directive", + ), + code: "", + range: Range { + start: Position { + line: 5, + character: 39, + }, + end: Position { + line: 5, + character: 52, + }, + }, + }, +) code: "" @@ -153,4 +187,21 @@ related_information[0].location.range: Range { start: Position { line: 8, charac severity: Some(Error) source: Some("oxc") tags: None -fixed: None +fixed: Single( + FixedContent { + message: Some( + "remove unused disable directive", + ), + code: "", + range: Range { + start: Position { + line: 8, + character: 2, + }, + end: Position { + line: 8, + character: 52, + }, + }, + }, +) diff --git a/editors/vscode/tests/code_actions.spec.ts b/editors/vscode/tests/code_actions.spec.ts index f2885cdf779b0..c01c806cc92cf 100644 --- a/editors/vscode/tests/code_actions.spec.ts +++ b/editors/vscode/tests/code_actions.spec.ts @@ -175,42 +175,37 @@ suite('code actions', () => { strictEqual(quickFixesWithFix.length, 3); }); - // TODO(camc314): FIXME - // test('changing configuration "unusedDisableDirectives" will reveal more code actions', async () => { - // await loadFixture('changing_unused_disable_directives'); - // const fileUri = Uri.joinPath(fixturesWorkspaceUri(), 'fixtures', 'unused_disable_directives.js'); - // await window.showTextDocument(fileUri); - // const codeActionsNoFix: ProviderResult> = await commands.executeCommand( - // 'vscode.executeCodeActionProvider', - // fileUri, - // { - // start: { line: 0, character: 0 }, - // end: { line: 0, character: 10 }, - // }, - // ); - - // assert(Array.isArray(codeActionsNoFix)); - // const quickFixesNoFix = codeActionsNoFix.filter( - // (action) => action.kind?.value === 'quickfix', - // ); - // strictEqual(quickFixesNoFix.length, 0); - - // await workspace.getConfiguration('oxc').update('unusedDisableDirectives', "warn"); - // await workspace.saveAll(); - - // const codeActionsWithFix: ProviderResult> = await commands.executeCommand( - // 'vscode.executeCodeActionProvider', - // fileUri, - // { - // start: { line: 0, character: 2 }, - // end: { line: 0, character: 10 }, - // }, - // ); - - // assert(Array.isArray(codeActionsWithFix)); - // const quickFixesWithFix = codeActionsWithFix.filter( - // (action) => action.kind?.value === 'quickfix', - // ); - // strictEqual(quickFixesWithFix.length, 1); - // }); + test('changing configuration "unusedDisableDirectives" will reveal more code actions', async () => { + await loadFixture('changing_unused_disable_directives'); + const fileUri = Uri.joinPath(fixturesWorkspaceUri(), 'fixtures', 'unused_disable_directives.js'); + await window.showTextDocument(fileUri); + const codeActionsNoFix: ProviderResult> = await commands.executeCommand( + 'vscode.executeCodeActionProvider', + fileUri, + { + start: { line: 0, character: 0 }, + end: { line: 0, character: 10 }, + }, + ); + assert(Array.isArray(codeActionsNoFix)); + const quickFixesNoFix = codeActionsNoFix.filter( + (action) => action.kind?.value === 'quickfix', + ); + strictEqual(quickFixesNoFix.length, 0); + await workspace.getConfiguration('oxc').update('unusedDisableDirectives', "warn"); + await workspace.saveAll(); + const codeActionsWithFix: ProviderResult> = await commands.executeCommand( + 'vscode.executeCodeActionProvider', + fileUri, + { + start: { line: 0, character: 2 }, + end: { line: 0, character: 10 }, + }, + ); + assert(Array.isArray(codeActionsWithFix)); + const quickFixesWithFix = codeActionsWithFix.filter( + (action) => action.kind?.value === 'quickfix', + ); + strictEqual(quickFixesWithFix.length, 1); + }); });