Skip to content

Commit 57e2a0c

Browse files
SysixSysix
committed
fix(language_server): make unused directives fixable again
1 parent a1cda35 commit 57e2a0c

File tree

3 files changed

+165
-59
lines changed

3 files changed

+165
-59
lines changed

‎crates/oxc_language_server/src/linter/isolated_lint_handler.rs‎

Lines changed: 78 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@ use tower_lsp_server::{UriExt, lsp_types::Uri};
1010

1111
use oxc_allocator::Allocator;
1212
use oxc_linter::{
13-
AllowWarnDeny, ConfigStore, DirectivesStore, DisableDirectives, LINTABLE_EXTENSIONS,
14-
LintOptions, LintService, LintServiceOptions, Linter, Message, RuntimeFileSystem,
15-
create_unused_directives_diagnostics, read_to_arena_str, read_to_string,
13+
AllowWarnDeny, ConfigStore, DirectivesStore, DisableDirectives, Fix, LINTABLE_EXTENSIONS,
14+
LintOptions, LintService, LintServiceOptions, Linter, Message, PossibleFixes, RuleCommentType,
15+
RuntimeFileSystem, read_to_arena_str, read_to_string,
1616
};
1717

1818
use super::error_with_position::{
@@ -130,7 +130,7 @@ impl IsolatedLintHandler {
130130
&& let Some(directives) = self.directives_coordinator.get(path)
131131
{
132132
messages.extend(
133-
self.create_unused_directives_messages(&directives, severity)
133+
create_unused_directives_messages(&directives, severity, source_text)
134134
.iter()
135135
.map(|message| message_to_lsp_diagnostic(message, uri, source_text, rope)),
136136
);
@@ -139,20 +139,6 @@ impl IsolatedLintHandler {
139139
messages
140140
}
141141

142-
#[expect(clippy::unused_self)]
143-
fn create_unused_directives_messages(
144-
&self,
145-
directives: &DisableDirectives,
146-
severity: AllowWarnDeny,
147-
) -> Vec<Message> {
148-
let diagnostics = create_unused_directives_diagnostics(directives, severity);
149-
diagnostics
150-
.into_iter()
151-
// TODO: unused directives should be fixable, `RuleCommentRule.create_fix()` can be used
152-
.map(|diagnostic| Message::new(diagnostic, oxc_linter::PossibleFixes::None))
153-
.collect()
154-
}
155-
156142
fn should_lint_path(path: &Path) -> bool {
157143
static WANTED_EXTENSIONS: OnceLock<FxHashSet<&'static str>> = OnceLock::new();
158144
let wanted_exts =
@@ -163,3 +149,77 @@ impl IsolatedLintHandler {
163149
.is_some_and(|ext| wanted_exts.contains(ext))
164150
}
165151
}
152+
153+
/// Almost the same as [oxc_linter::create_unused_directives_diagnostics], but returns `Message`s
154+
/// with a `PossibleFixes` instead of `OxcDiagnostic`s.
155+
fn create_unused_directives_messages(
156+
directives: &DisableDirectives,
157+
severity: AllowWarnDeny,
158+
source_text: &str,
159+
) -> Vec<Message> {
160+
use oxc_diagnostics::OxcDiagnostic;
161+
162+
let mut diagnostics = Vec::new();
163+
let fix_message = "remove unused disable directive";
164+
165+
let severity = if severity == AllowWarnDeny::Deny {
166+
oxc_diagnostics::Severity::Error
167+
} else {
168+
oxc_diagnostics::Severity::Warning
169+
};
170+
171+
// Report unused disable comments
172+
let unused_disable = directives.collect_unused_disable_comments();
173+
for unused_comment in unused_disable {
174+
let span = unused_comment.span;
175+
match unused_comment.r#type {
176+
RuleCommentType::All => {
177+
diagnostics.push(Message::new(
178+
OxcDiagnostic::warn(
179+
"Unused eslint-disable directive (no problems were reported).",
180+
)
181+
.with_label(span)
182+
.with_severity(severity),
183+
PossibleFixes::Single(Fix::delete(span).with_message(fix_message)),
184+
));
185+
}
186+
RuleCommentType::Single(rules) => {
187+
for rule in rules {
188+
let rule_message = format!(
189+
"Unused eslint-disable directive (no problems were reported from {}).",
190+
rule.rule_name
191+
);
192+
diagnostics.push(Message::new(
193+
OxcDiagnostic::warn(rule_message)
194+
.with_label(rule.name_span)
195+
.with_severity(severity),
196+
PossibleFixes::Single(
197+
rule.create_fix(source_text, span).with_message(fix_message),
198+
),
199+
));
200+
}
201+
}
202+
}
203+
}
204+
205+
// Report unused enable comments
206+
let unused_enable = directives.unused_enable_comments();
207+
for (rule_name, span) in unused_enable {
208+
let message = if let Some(rule_name) = rule_name {
209+
format!(
210+
"Unused eslint-enable directive (no matching eslint-disable directives were found for {rule_name})."
211+
)
212+
} else {
213+
"Unused eslint-enable directive (no matching eslint-disable directives were found)."
214+
.to_string()
215+
};
216+
diagnostics.push(Message::new(
217+
OxcDiagnostic::warn(message).with_label(*span).with_severity(severity),
218+
// TODO: fixer
219+
// copy the structure of disable directives
220+
PossibleFixes::None,
221+
));
222+
}
223+
224+
diagnostics
225+
}

‎crates/oxc_language_server/src/snapshots/fixtures_linter_unused_disabled_directives@test.js.snap‎

Lines changed: 54 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,24 @@ related_information[0].location.range: Range { start: Position { line: 0, charac
127127
severity: Some(Error)
128128
source: Some("oxc")
129129
tags: None
130-
fixed: None
130+
fixed: Single(
131+
FixedContent {
132+
message: Some(
133+
"remove unused disable directive",
134+
),
135+
code: "",
136+
range: Range {
137+
start: Position {
138+
line: 0,
139+
character: 2,
140+
},
141+
end: Position {
142+
line: 0,
143+
character: 56,
144+
},
145+
},
146+
},
147+
)
131148

132149

133150
code: ""
@@ -140,7 +157,24 @@ related_information[0].location.range: Range { start: Position { line: 5, charac
140157
severity: Some(Error)
141158
source: Some("oxc")
142159
tags: None
143-
fixed: None
160+
fixed: Single(
161+
FixedContent {
162+
message: Some(
163+
"remove unused disable directive",
164+
),
165+
code: "",
166+
range: Range {
167+
start: Position {
168+
line: 5,
169+
character: 39,
170+
},
171+
end: Position {
172+
line: 5,
173+
character: 52,
174+
},
175+
},
176+
},
177+
)
144178

145179

146180
code: ""
@@ -153,4 +187,21 @@ related_information[0].location.range: Range { start: Position { line: 8, charac
153187
severity: Some(Error)
154188
source: Some("oxc")
155189
tags: None
156-
fixed: None
190+
fixed: Single(
191+
FixedContent {
192+
message: Some(
193+
"remove unused disable directive",
194+
),
195+
code: "",
196+
range: Range {
197+
start: Position {
198+
line: 8,
199+
character: 2,
200+
},
201+
end: Position {
202+
line: 8,
203+
character: 52,
204+
},
205+
},
206+
},
207+
)

‎editors/vscode/tests/code_actions.spec.ts‎

Lines changed: 33 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -175,42 +175,37 @@ suite('code actions', () => {
175175
strictEqual(quickFixesWithFix.length, 3);
176176
});
177177

178-
// TODO(camc314): FIXME
179-
// test('changing configuration "unusedDisableDirectives" will reveal more code actions', async () => {
180-
// await loadFixture('changing_unused_disable_directives');
181-
// const fileUri = Uri.joinPath(fixturesWorkspaceUri(), 'fixtures', 'unused_disable_directives.js');
182-
// await window.showTextDocument(fileUri);
183-
// const codeActionsNoFix: ProviderResult<Array<CodeAction>> = await commands.executeCommand(
184-
// 'vscode.executeCodeActionProvider',
185-
// fileUri,
186-
// {
187-
// start: { line: 0, character: 0 },
188-
// end: { line: 0, character: 10 },
189-
// },
190-
// );
191-
192-
// assert(Array.isArray(codeActionsNoFix));
193-
// const quickFixesNoFix = codeActionsNoFix.filter(
194-
// (action) => action.kind?.value === 'quickfix',
195-
// );
196-
// strictEqual(quickFixesNoFix.length, 0);
197-
198-
// await workspace.getConfiguration('oxc').update('unusedDisableDirectives', "warn");
199-
// await workspace.saveAll();
200-
201-
// const codeActionsWithFix: ProviderResult<Array<CodeAction>> = await commands.executeCommand(
202-
// 'vscode.executeCodeActionProvider',
203-
// fileUri,
204-
// {
205-
// start: { line: 0, character: 2 },
206-
// end: { line: 0, character: 10 },
207-
// },
208-
// );
209-
210-
// assert(Array.isArray(codeActionsWithFix));
211-
// const quickFixesWithFix = codeActionsWithFix.filter(
212-
// (action) => action.kind?.value === 'quickfix',
213-
// );
214-
// strictEqual(quickFixesWithFix.length, 1);
215-
// });
178+
test('changing configuration "unusedDisableDirectives" will reveal more code actions', async () => {
179+
await loadFixture('changing_unused_disable_directives');
180+
const fileUri = Uri.joinPath(fixturesWorkspaceUri(), 'fixtures', 'unused_disable_directives.js');
181+
await window.showTextDocument(fileUri);
182+
const codeActionsNoFix: ProviderResult<Array<CodeAction>> = await commands.executeCommand(
183+
'vscode.executeCodeActionProvider',
184+
fileUri,
185+
{
186+
start: { line: 0, character: 0 },
187+
end: { line: 0, character: 10 },
188+
},
189+
);
190+
assert(Array.isArray(codeActionsNoFix));
191+
const quickFixesNoFix = codeActionsNoFix.filter(
192+
(action) => action.kind?.value === 'quickfix',
193+
);
194+
strictEqual(quickFixesNoFix.length, 0);
195+
await workspace.getConfiguration('oxc').update('unusedDisableDirectives', "warn");
196+
await workspace.saveAll();
197+
const codeActionsWithFix: ProviderResult<Array<CodeAction>> = await commands.executeCommand(
198+
'vscode.executeCodeActionProvider',
199+
fileUri,
200+
{
201+
start: { line: 0, character: 2 },
202+
end: { line: 0, character: 10 },
203+
},
204+
);
205+
assert(Array.isArray(codeActionsWithFix));
206+
const quickFixesWithFix = codeActionsWithFix.filter(
207+
(action) => action.kind?.value === 'quickfix',
208+
);
209+
strictEqual(quickFixesWithFix.length, 1);
210+
});
216211
});

0 commit comments

Comments
 (0)