fix(language_server): workspace edits as one batch when source.fixAll.oxc is the context

Sysix · Sysix · commit 427936af9200 · 2025-04-15T20:59:53.000+02:00
diff --git a/crates/oxc_language_server/src/main.rs b/crates/oxc_language_server/src/main.rs
@@ -270,6 +270,8 @@ impl LanguageServer for Backend {
                 r.diagnostic.range == params.range
                     || range_overlaps(params.range, r.diagnostic.range)
             });
+            let mut quick_fixes: Vec<TextEdit> = vec![];
+
             for report in reports {
                 // TODO: Would be better if we had exact rule name from the diagnostic instead of having to parse it.
                 let mut rule_name: Option<String> = None;
@@ -296,30 +298,38 @@ impl LanguageServer for Backend {
                             }
                         }
                     };
-                    code_actions_vec.push(CodeActionOrCommand::CodeAction(CodeAction {
-                        title,
-                        kind: Some(if is_source_fix_all_oxc {
-                            CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC
-                        } else {
-                            CodeActionKind::QUICKFIX
-                        }),
-                        is_preferred: Some(true),
-                        edit: Some(WorkspaceEdit {
-                            #[expect(clippy::disallowed_types)]
-                            changes: Some(std::collections::HashMap::from([(
-                                uri.clone(),
-                                vec![TextEdit {
-                                    range: fixed_content.range,
-                                    new_text: fixed_content.code.clone(),
-                                }],
-                            )])),
-                            ..WorkspaceEdit::default()
-                        }),
-                        disabled: None,
-                        data: None,
-                        diagnostics: None,
-                        command: None,
-                    }));
+
+                    // when source.fixAll.oxc we collect all changes at ones
+                    // and return them as one workspace edit.
+                    // it is possible that one fix will change the range for the next fix
+                    // see oxc-project/oxc#10422
+                    if is_source_fix_all_oxc {
+                        quick_fixes.push(TextEdit {
+                            range: fixed_content.range,
+                            new_text: fixed_content.code.clone(),
+                        });
+                    } else {
+                        code_actions_vec.push(CodeActionOrCommand::CodeAction(CodeAction {
+                            title,
+                            kind: Some(CodeActionKind::QUICKFIX),
+                            is_preferred: Some(true),
+                            edit: Some(WorkspaceEdit {
+                                #[expect(clippy::disallowed_types)]
+                                changes: Some(std::collections::HashMap::from([(
+                                    uri.clone(),
+                                    vec![TextEdit {
+                                        range: fixed_content.range,
+                                        new_text: fixed_content.code.clone(),
+                                    }],
+                                )])),
+                                ..WorkspaceEdit::default()
+                            }),
+                            disabled: None,
+                            data: None,
+                            diagnostics: None,
+                            command: None,
+                        }));
+                    }
                 }
 
                 code_actions_vec.push(
@@ -395,6 +405,23 @@ impl LanguageServer for Backend {
                     command: None,
                 }));
             }
+
+            if is_source_fix_all_oxc && !quick_fixes.is_empty() {
+                code_actions_vec.push(CodeActionOrCommand::CodeAction(CodeAction {
+                    title: "quick fix".to_string(),
+                    kind: Some(CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC),
+                    is_preferred: Some(true),
+                    edit: Some(WorkspaceEdit {
+                        #[expect(clippy::disallowed_types)]
+                        changes: Some(std::collections::HashMap::from([(uri, quick_fixes)])),
+                        ..WorkspaceEdit::default()
+                    }),
+                    disabled: None,
+                    data: None,
+                    diagnostics: None,
+                    command: None,
+                }));
+            }
         }
 
         if code_actions_vec.is_empty() {
diff --git a/editors/vscode/client/extension.spec.ts b/editors/vscode/client/extension.spec.ts
@@ -4,7 +4,9 @@ import {
   commands,
   DiagnosticSeverity,
   languages,
+  Position,
   ProviderResult,
+  Range,
   Uri,
   window,
   workspace,
@@ -128,6 +130,38 @@ suite('code actions', () => {
       ],
     );
   });
+
+  // https://github.com/oxc-project/oxc/issues/10422
+  test('code action `source.fixAll.oxc` on editor.codeActionsOnSave', async () => {
+    let file = Uri.joinPath(WORKSPACE_DIR, 'fixtures', 'file.js');
+    let expectedFile = Uri.joinPath(WORKSPACE_DIR, '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_with_code_actions_on_save');
+    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());
+
+    await workspace.getConfiguration('editor').update('codeActionsOnSave', undefined);
+    await workspace.saveAll();
+  });
 });
 
 suite('E2E Diagnostics', () => {
diff --git a/editors/vscode/client/test-helpers.ts b/editors/vscode/client/test-helpers.ts
@@ -31,8 +31,7 @@ export type OxlintConfig = {
 
 export const WORKSPACE_DIR = workspace.workspaceFolders![0].uri;
 
-const rootOxlintConfigPath = WORKSPACE_DIR + '/.oxlintrc.json';
-const rootOxlintConfigUri = Uri.parse(rootOxlintConfigPath);
+const rootOxlintConfigUri = Uri.joinPath(WORKSPACE_DIR, '.oxlintrc.json');
 
 export async function sleep(ms: number): Promise<void> {
   return new Promise((resolve) => setTimeout(resolve, ms));
@@ -53,15 +52,14 @@ export async function createOxlintConfiguration(configuration: OxlintConfig): Pr
   });
   await workspace.applyEdit(edit);
 }
-
 export async function loadFixture(fixture: string): Promise<void> {
   const absolutePath = path.resolve(`${__dirname}/../fixtures/${fixture}`);
   // do not copy directly into the workspace folder. FileWatcher will detect them as a deletion and stop itself.
-  await workspace.fs.copy(Uri.file(absolutePath), Uri.joinPath(WORKSPACE_DIR, 'diagnostic'), { overwrite: true });
+  await workspace.fs.copy(Uri.file(absolutePath), Uri.joinPath(WORKSPACE_DIR, 'fixtures'), { overwrite: true });
 }
 
 export async function getDiagnostics(file: string): Promise<Diagnostic[]> {
-  const fileUri = Uri.joinPath(WORKSPACE_DIR, 'diagnostic', file);
+  const fileUri = Uri.joinPath(WORKSPACE_DIR, 'fixtures', file);
   await window.showTextDocument(fileUri);
   await sleep(500);
   const diagnostics = languages.getDiagnostics(fileUri);
diff --git a/editors/vscode/fixtures/fixall_with_code_actions_on_save/.oxlintrc.json b/editors/vscode/fixtures/fixall_with_code_actions_on_save/.oxlintrc.json
@@ -0,0 +1,5 @@
+{
+  "rules": {
+      "unicorn/switch-case-braces": "warn"
+  }
+}
diff --git a/editors/vscode/fixtures/fixall_with_code_actions_on_save/expected.txt b/editors/vscode/fixtures/fixall_with_code_actions_on_save/expected.txt
@@ -0,0 +1,72 @@
+ export function processChanges(changeHistory, maxChangesToShow) {
+  if (!changeHistory || !changeHistory.changes) {
+      return [];
+  }
+
+  const { patch = [] } = changeHistory.changes;
+  const summaryItems = [];
+  const processedPaths = new Set();
+  let totalChanges = 0;
+
+  // Process patch operations
+  patch.forEach(({ op, path, value }) => {
+      totalChanges++;
+
+      // Skip if we've reached the display limit
+      if (summaryItems.length >= maxChangesToShow) return;
+
+      // Format path for display
+      const displayPath = formatPathForDisplay(path);
+
+      // Skip duplicates or paths we've already processed
+      if (processedPaths.has(displayPath)) return;
+      processedPaths.add(displayPath);
+
+      // Handle different operation types
+      switch (op) {
+          case 'add': {if (path.match(/^\/references\/\d+$/)) {
+                  if (value && typeof value === 'object' && 'key' in value) {
+                      summaryItems.push({
+                          label: 'References.' + value.key,
+                          displayValue: <span className='text-green-600 font-medium'>Added</span>
+                      });
+                  }
+              } else {
+                  summaryItems.push({
+                      label: displayPath,
+                      displayValue: <span className='text-green-600 font-medium'>Added</span>
+                  });
+              }
+              break;
+          }
+
+          case 'remove': {
+              summaryItems.push({
+                  label: displayPath,
+                  displayValue: <span className='text-red-600 font-medium'>Removed</span>
+              });
+              break;
+          }
+
+          case 'replace': {if (path === '/version') return;
+              summaryItems.push({
+                  label: displayPath,
+                  displayValue: <span className='text-blue-600 font-medium'>Updated</span>
+              });
+              break;
+          }
+
+          default: {
+              summaryItems.push({
+                  label: displayPath,
+                  displayValue: <span className='text-gray-600'>{op}</span>
+              });
+          }
+      }
+  });
+
+  // Add metadata about more changes
+  summaryItems.moreChangesCount = Math.max(0, totalChanges - maxChangesToShow);
+
+  return summaryItems;
+}
diff --git a/editors/vscode/fixtures/fixall_with_code_actions_on_save/file.js b/editors/vscode/fixtures/fixall_with_code_actions_on_save/file.js
@@ -0,0 +1,73 @@
+export function processChanges(changeHistory, maxChangesToShow) {
+  if (!changeHistory || !changeHistory.changes) {
+      return [];
+  }
+
+  const { patch = [] } = changeHistory.changes;
+  const summaryItems = [];
+  const processedPaths = new Set();
+  let totalChanges = 0;
+
+  // Process patch operations
+  patch.forEach(({ op, path, value }) => {
+      totalChanges++;
+
+      // Skip if we've reached the display limit
+      if (summaryItems.length >= maxChangesToShow) return;
+
+      // Format path for display
+      const displayPath = formatPathForDisplay(path);
+
+      // Skip duplicates or paths we've already processed
+      if (processedPaths.has(displayPath)) return;
+      processedPaths.add(displayPath);
+
+      // Handle different operation types
+      switch (op) {
+          case 'add':
+              // Special handling for references
+              if (path.match(/^\/references\/\d+$/)) {
+                  if (value && typeof value === 'object' && 'key' in value) {
+                      summaryItems.push({
+                          label: 'References.' + value.key,
+                          displayValue: <span className='text-green-600 font-medium'>Added</span>
+                      });
+                  }
+              } else {
+                  summaryItems.push({
+                      label: displayPath,
+                      displayValue: <span className='text-green-600 font-medium'>Added</span>
+                  });
+              }
+              break;
+
+          case 'remove':
+              summaryItems.push({
+                  label: displayPath,
+                  displayValue: <span className='text-red-600 font-medium'>Removed</span>
+              });
+              break;
+
+          case 'replace':
+              // Skip version changes as we display them separately
+              if (path === '/version') return;
+
+              summaryItems.push({
+                  label: displayPath,
+                  displayValue: <span className='text-blue-600 font-medium'>Updated</span>
+              });
+              break;
+
+          default:
+              summaryItems.push({
+                  label: displayPath,
+                  displayValue: <span className='text-gray-600'>{op}</span>
+              });
+      }
+  });
+
+  // Add metadata about more changes
+  summaryItems.moreChangesCount = Math.max(0, totalChanges - maxChangesToShow);
+
+  return summaryItems;
+}

-Original file line number
+Diff line change
@@ @@ -0,0 +1,5 @@ @@
 +{
 +  "rules": {
 +      "unicorn/switch-case-braces": "warn"
 +  }
 +}