From adff0694d0c6eb2eaefb5e32f7467c1ac6832eb4 Mon Sep 17 00:00:00 2001 From: Sysix <3897725+Sysix@users.noreply.github.com> Date: Mon, 29 Sep 2025 20:37:43 +0000 Subject: [PATCH] fix(language_server): don't apply "ignore this rule" fixes for fixAll code action + command (#14243) This PR fixes a bug where on `source.fixAll.oxc` code action or `oxc.fixAll` command will append `// oxlint-disable ...` comments to the file. This bug was introduced in https://github.com/oxc-project/oxc/pull/14183 --- .../oxc_language_server/src/code_actions.rs | 26 ++++++++++++++++- crates/oxc_language_server/src/worker.rs | 24 ++++++++++++++- .../.oxlintrc.json | 5 ++++ .../expected.txt | 4 +++ .../file2.js | 4 +++ editors/vscode/tests/code_actions.spec.ts | 29 +++++++++++++++++++ 6 files changed, 90 insertions(+), 2 deletions(-) create mode 100644 editors/vscode/fixtures/fixall_code_action_ignore_only_disable_fix/.oxlintrc.json create mode 100644 editors/vscode/fixtures/fixall_code_action_ignore_only_disable_fix/expected.txt create mode 100644 editors/vscode/fixtures/fixall_code_action_ignore_only_disable_fix/file2.js diff --git a/crates/oxc_language_server/src/code_actions.rs b/crates/oxc_language_server/src/code_actions.rs index 363e070f67833..a5a4763d21705 100644 --- a/crates/oxc_language_server/src/code_actions.rs +++ b/crates/oxc_language_server/src/code_actions.rs @@ -1,3 +1,4 @@ +use log::debug; use tower_lsp_server::lsp_types::{CodeAction, CodeActionKind, TextEdit, Uri, WorkspaceEdit}; use crate::linter::error_with_position::{DiagnosticReport, FixedContent, PossibleFixContent}; @@ -87,7 +88,30 @@ pub fn apply_all_fix_code_action<'a>( PossibleFixContent::Single(fixed_content) => Some(fixed_content), // For multiple fixes, we take the first one as a representative fix. // Applying all possible fixes at once is not possible in this context. - PossibleFixContent::Multiple(multi) => multi.first(), + PossibleFixContent::Multiple(multi) => { + // for a real linter fix, we expect at least 3 fixes + if multi.len() > 2 { + multi.first() + } else { + debug!("Multiple fixes found, but only ignore fixes available"); + #[cfg(debug_assertions)] + { + if !multi.is_empty() { + debug_assert!(multi[0].message.as_ref().is_some()); + debug_assert!( + multi[0].message.as_ref().unwrap().starts_with("Disable") + ); + debug_assert!( + multi[0].message.as_ref().unwrap().ends_with("for this line") + ); + } + } + + // this fix is only for "ignore this line/file" fixes + // do not apply them for "fix all" code action + None + } + } }; if let Some(fixed_content) = &fix { diff --git a/crates/oxc_language_server/src/worker.rs b/crates/oxc_language_server/src/worker.rs index 56c28b34f4ca3..d21a2377636b2 100644 --- a/crates/oxc_language_server/src/worker.rs +++ b/crates/oxc_language_server/src/worker.rs @@ -294,7 +294,29 @@ impl WorkspaceWorker { PossibleFixContent::Single(fixed_content) => Some(fixed_content), // For multiple fixes, we take the first one as a representative fix. // Applying all possible fixes at once is not possible in this context. - PossibleFixContent::Multiple(multi) => multi.first(), + PossibleFixContent::Multiple(multi) => { + // for a real linter fix, we expect at least 3 fixes + if multi.len() > 2 { + multi.first() + } else { + debug!("Multiple fixes found, but only ignore fixes available"); + #[cfg(debug_assertions)] + { + if !multi.is_empty() { + debug_assert!(multi[0].message.as_ref().is_some()); + debug_assert!( + multi[0].message.as_ref().unwrap().starts_with("Disable") + ); + debug_assert!( + multi[0].message.as_ref().unwrap().ends_with("for this line") + ); + } + } + // this fix is only for "ignore this line/file" fixes + // do not apply them for "fix all" code action + None + } + } }; if let Some(fixed_content) = &fix { diff --git a/editors/vscode/fixtures/fixall_code_action_ignore_only_disable_fix/.oxlintrc.json b/editors/vscode/fixtures/fixall_code_action_ignore_only_disable_fix/.oxlintrc.json new file mode 100644 index 0000000000000..caf3ba0f6d5c7 --- /dev/null +++ b/editors/vscode/fixtures/fixall_code_action_ignore_only_disable_fix/.oxlintrc.json @@ -0,0 +1,5 @@ +{ + "rules": { + "no-const-assign": "error" + } +} diff --git a/editors/vscode/fixtures/fixall_code_action_ignore_only_disable_fix/expected.txt b/editors/vscode/fixtures/fixall_code_action_ignore_only_disable_fix/expected.txt new file mode 100644 index 0000000000000..c505f87176a82 --- /dev/null +++ b/editors/vscode/fixtures/fixall_code_action_ignore_only_disable_fix/expected.txt @@ -0,0 +1,4 @@ + const a = 0; +a = 1; + +console.log(a); diff --git a/editors/vscode/fixtures/fixall_code_action_ignore_only_disable_fix/file2.js b/editors/vscode/fixtures/fixall_code_action_ignore_only_disable_fix/file2.js new file mode 100644 index 0000000000000..45152a30ba412 --- /dev/null +++ b/editors/vscode/fixtures/fixall_code_action_ignore_only_disable_fix/file2.js @@ -0,0 +1,4 @@ +const a = 0; +a = 1; + +console.log(a); diff --git a/editors/vscode/tests/code_actions.spec.ts b/editors/vscode/tests/code_actions.spec.ts index 5ad2733764400..6f0191fab2f5e 100644 --- a/editors/vscode/tests/code_actions.spec.ts +++ b/editors/vscode/tests/code_actions.spec.ts @@ -106,6 +106,35 @@ suite('code actions', () => { strictEqual(content.toString(), expected.toString()); }); + // https://discord.com/channels/1079625926024900739/1080723403595591700/1422191300395929620 + test('code action `source.fixAll.oxc` ignores "ignore this rule for this line/file"', async () => { + let file = Uri.joinPath(fixturesWorkspaceUri(), 'fixtures', 'file2.js'); + let expectedFile = Uri.joinPath(fixturesWorkspaceUri(), 'fixtures', 'expected.txt'); + + await workspace.getConfiguration('editor').update('codeActionsOnSave', { + 'source.fixAll.oxc': 'always', + }); + await workspace.saveAll(); + + const range = new Range(new Position(0, 0), new Position(0, 0)); + const edit = new WorkspaceEdit(); + edit.replace(file, range, ' '); + + await sleep(1000); + + await loadFixture('fixall_code_action_ignore_only_disable_fix'); + await workspace.openTextDocument(file); + await workspace.applyEdit(edit); + await sleep(1000); + await workspace.saveAll(); + await sleep(500); + + const content = await workspace.fs.readFile(file); + const expected = await workspace.fs.readFile(expectedFile); + + strictEqual(content.toString(), expected.toString()); + }); + test('changing configuration flag "fix_kind" will reveal more code actions', async () => { await loadFixture('changing_fix_kind'); const fileUri = Uri.joinPath(fixturesWorkspaceUri(), 'fixtures', 'for_direction.ts');