Skip to content

fix(language_server): make unused directives fixable again #14872

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 22, 2025
+165 −59
Merged

fix(language_server): make unused directives fixable again #14872

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 78 additions & 18 deletions crates/oxc_language_server/src/linter/isolated_lint_handler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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)),
);
Expand All @@ -139,20 +139,6 @@ impl IsolatedLintHandler {
messages
}

#[expect(clippy::unused_self)]
fn create_unused_directives_messages(
&self,
directives: &DisableDirectives,
severity: AllowWarnDeny,
) -> Vec<Message> {
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<FxHashSet<&'static str>> = OnceLock::new();
let wanted_exts =
Expand All @@ -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<Message> {
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
}
57 changes: 54 additions & 3 deletions ...oxc_language_server/src/snapshots/fixtures_linter_unused_disabled_directives@test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: ""
Expand All @@ -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: ""
Expand All @@ -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,
},
},
},
)
71 changes: 33 additions & 38 deletions editors/vscode/tests/code_actions.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<Array<CodeAction>> = 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<Array<CodeAction>> = 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<Array<CodeAction>> = 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<Array<CodeAction>> = 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);
});
});
Loading