Skip to content

Commit cea33a5

Browse files
committed
fix(language_server): don't apply "ignore this rule" fixes for fixAll code action + command
1 parent 55ebb8b commit cea33a5

File tree

6 files changed

+88
-2
lines changed

6 files changed

+88
-2
lines changed

crates/oxc_language_server/src/code_actions.rs

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use log::debug;
12
use tower_lsp_server::lsp_types::{CodeAction, CodeActionKind, TextEdit, Uri, WorkspaceEdit};
23

34
use crate::linter::error_with_position::{DiagnosticReport, FixedContent, PossibleFixContent};
@@ -87,7 +88,29 @@ pub fn apply_all_fix_code_action<'a>(
8788
PossibleFixContent::Single(fixed_content) => Some(fixed_content),
8889
// For multiple fixes, we take the first one as a representative fix.
8990
// Applying all possible fixes at once is not possible in this context.
90-
PossibleFixContent::Multiple(multi) => multi.first(),
91+
PossibleFixContent::Multiple(multi) => {
92+
if multi.len() > 2 {
93+
multi.first()
94+
} else {
95+
debug!("Multiple fixes found, but only ignore fixes available");
96+
#[cfg(debug_assertions)]
97+
{
98+
if !multi.is_empty() {
99+
debug_assert!(multi[0].message.as_ref().is_some());
100+
debug_assert!(
101+
multi[0].message.as_ref().unwrap().starts_with("Disable")
102+
);
103+
debug_assert!(
104+
multi[0].message.as_ref().unwrap().ends_with("for this line")
105+
);
106+
}
107+
}
108+
109+
// this fix is only for "ignore this line/file" fixes
110+
// do not apply them for "fix all" code action
111+
None
112+
}
113+
}
91114
};
92115

93116
if let Some(fixed_content) = &fix {

crates/oxc_language_server/src/worker.rs

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,28 @@ impl WorkspaceWorker {
294294
PossibleFixContent::Single(fixed_content) => Some(fixed_content),
295295
// For multiple fixes, we take the first one as a representative fix.
296296
// Applying all possible fixes at once is not possible in this context.
297-
PossibleFixContent::Multiple(multi) => multi.first(),
297+
PossibleFixContent::Multiple(multi) => {
298+
if multi.len() > 2 {
299+
multi.first()
300+
} else {
301+
debug!("Multiple fixes found, but only ignore fixes available");
302+
#[cfg(debug_assertions)]
303+
{
304+
if !multi.is_empty() {
305+
debug_assert!(multi[0].message.as_ref().is_some());
306+
debug_assert!(
307+
multi[0].message.as_ref().unwrap().starts_with("Disable")
308+
);
309+
debug_assert!(
310+
multi[0].message.as_ref().unwrap().ends_with("for this line")
311+
);
312+
}
313+
}
314+
// this fix is only for "ignore this line/file" fixes
315+
// do not apply them for "fix all" code action
316+
None
317+
}
318+
}
298319
};
299320

300321
if let Some(fixed_content) = &fix {
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"rules": {
3+
"no-const-assign ": "error"
4+
}
5+
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
const a = 0;
2+
a = 1;
3+
4+
console.log(a);
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
const a = 0;
2+
a = 1;
3+
4+
console.log(a);

editors/vscode/tests/code_actions.spec.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,35 @@ suite('code actions', () => {
106106
strictEqual(content.toString(), expected.toString());
107107
});
108108

109+
// https://discord.com/channels/1079625926024900739/1080723403595591700/1422191300395929620
110+
test('code action `source.fixAll.oxc` ignores "ignore this rule for this line/file"', async () => {
111+
let file = Uri.joinPath(fixturesWorkspaceUri(), 'fixtures', 'file2.js');
112+
let expectedFile = Uri.joinPath(fixturesWorkspaceUri(), 'fixtures', 'expected.txt');
113+
114+
await workspace.getConfiguration('editor').update('codeActionsOnSave', {
115+
'source.fixAll.oxc': 'always',
116+
});
117+
await workspace.saveAll();
118+
119+
const range = new Range(new Position(0, 0), new Position(0, 0));
120+
const edit = new WorkspaceEdit();
121+
edit.replace(file, range, ' ');
122+
123+
await sleep(1000);
124+
125+
await loadFixture('fixall_code_action_ignore_only_disable_fix');
126+
await workspace.openTextDocument(file);
127+
await workspace.applyEdit(edit);
128+
await sleep(1000);
129+
await workspace.saveAll();
130+
await sleep(500);
131+
132+
const content = await workspace.fs.readFile(file);
133+
const expected = await workspace.fs.readFile(expectedFile);
134+
135+
strictEqual(content.toString(), expected.toString());
136+
});
137+
109138
test('changing configuration flag "fix_kind" will reveal more code actions', async () => {
110139
await loadFixture('changing_fix_kind');
111140
const fileUri = Uri.joinPath(fixturesWorkspaceUri(), 'fixtures', 'for_direction.ts');

0 commit comments

Comments
 (0)