From 5f24a2c0549c3694cee27d3b22ea4e7ed73d0c91 Mon Sep 17 00:00:00 2001 From: Sebastian Thomschke Date: Fri, 28 Nov 2025 23:18:33 +0100 Subject: [PATCH] fix: fall back to next LS when formatting returns no edits --- org.eclipse.lsp4e.test/META-INF/MANIFEST.MF | 2 +- org.eclipse.lsp4e.test/pom.xml | 2 +- .../eclipse/lsp4e/test/format/FormatTest.java | 39 ++++++++++++++----- org.eclipse.lsp4e/META-INF/MANIFEST.MF | 2 +- org.eclipse.lsp4e/pom.xml | 2 +- .../lsp4e/operations/format/LSPFormatter.java | 22 ++++++----- 6 files changed, 47 insertions(+), 22 deletions(-) diff --git a/org.eclipse.lsp4e.test/META-INF/MANIFEST.MF b/org.eclipse.lsp4e.test/META-INF/MANIFEST.MF index d26e58a97..6369d95b0 100644 --- a/org.eclipse.lsp4e.test/META-INF/MANIFEST.MF +++ b/org.eclipse.lsp4e.test/META-INF/MANIFEST.MF @@ -2,7 +2,7 @@ Manifest-Version: 1.0 Bundle-ManifestVersion: 2 Bundle-Name: Tests for language server bundle (Incubation) Bundle-SymbolicName: org.eclipse.lsp4e.test;singleton:=true -Bundle-Version: 0.16.3.qualifier +Bundle-Version: 0.16.4.qualifier Fragment-Host: org.eclipse.lsp4e Bundle-Vendor: Eclipse LSP4E Bundle-RequiredExecutionEnvironment: JavaSE-21 diff --git a/org.eclipse.lsp4e.test/pom.xml b/org.eclipse.lsp4e.test/pom.xml index 53f4e06be..d70109e46 100644 --- a/org.eclipse.lsp4e.test/pom.xml +++ b/org.eclipse.lsp4e.test/pom.xml @@ -8,7 +8,7 @@ org.eclipse.lsp4e.test eclipse-test-plugin - 0.16.3-SNAPSHOT + 0.16.4-SNAPSHOT diff --git a/org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/format/FormatTest.java b/org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/format/FormatTest.java index 0a22092d7..464d94529 100644 --- a/org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/format/FormatTest.java +++ b/org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/format/FormatTest.java @@ -21,6 +21,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.ConcurrentModificationException; +import java.util.List; import java.util.Optional; import java.util.concurrent.ExecutionException; @@ -57,6 +58,29 @@ public void testFormattingInvalidDocument() throws Exception { assertTrue(edits.isEmpty()); } + /** + * Verifies that when the language server reports no formatting edits + * (empty edit list), {@link LSPFormatter} returns an empty Optional + * and leaves the document content unchanged. + */ + @Test + public void testFormattingEmptyEditsYieldEmptyOptional() throws Exception { + MockLanguageServer.INSTANCE.setFormattingTextEdits(Collections.emptyList()); + + IFile file = TestUtils.createUniqueTestFile(project, "Formatting Other Text"); + IEditorPart editor = TestUtils.openEditor(file); + ITextViewer viewer = LSPEclipseUtils.getTextViewer(editor); + + final var formatter = new LSPFormatter(); + ISelection selection = viewer.getSelectionProvider().getSelection(); + + Optional edits = formatter.requestFormatting(viewer.getDocument(), (ITextSelection) selection).get(); + assertTrue(edits.isEmpty()); + assertEquals("Formatting Other Text", viewer.getDocument().get()); + + TestUtils.closeEditor(editor, false); + } + @Test public void testFormattingNoChanges() throws Exception { MockLanguageServer.INSTANCE.setFormattingTextEdits(Collections.emptyList()); @@ -69,14 +93,10 @@ public void testFormattingNoChanges() throws Exception { ISelection selection = viewer.getSelectionProvider().getSelection(); Optional edits = formatter.requestFormatting(viewer.getDocument(), (ITextSelection) selection).get(); - assertTrue(edits.isPresent()); - editor.getSite().getShell().getDisplay().syncExec(() -> { - try { - edits.get().apply(); - } catch (ConcurrentModificationException | BadLocationException e) { - fail(e.getMessage()); - } - }); + + // No formatting edits produced -> no VersionedEdits returned + assertTrue(edits.isEmpty()); + final var textEditor = (ITextEditor) editor; textEditor.getDocumentProvider().getDocument(textEditor.getEditorInput()); assertEquals("Formatting Other Text", viewer.getDocument().get()); @@ -243,7 +263,8 @@ public void testSelectiveFormattingWithIncapableServer() throws Exception { @Test public void testOutdatedFormatting() throws CoreException, InterruptedException, ExecutionException, BadLocationException { - MockLanguageServer.INSTANCE.setFormattingTextEdits(Collections.emptyList()); + // Use a non-empty edit list so that a VersionedEdits is produced + MockLanguageServer.INSTANCE.setFormattingTextEdits(List.of(new TextEdit(new Range(new Position(0, 0), new Position(0, 0)), "X"))); IFile file = TestUtils.createUniqueTestFile(project, "Formatting Other Text"); IEditorPart editor = TestUtils.openEditor(file); diff --git a/org.eclipse.lsp4e/META-INF/MANIFEST.MF b/org.eclipse.lsp4e/META-INF/MANIFEST.MF index 8575497e9..052db6dbc 100644 --- a/org.eclipse.lsp4e/META-INF/MANIFEST.MF +++ b/org.eclipse.lsp4e/META-INF/MANIFEST.MF @@ -2,7 +2,7 @@ Manifest-Version: 1.0 Bundle-ManifestVersion: 2 Bundle-Name: Language Server Protocol client for Eclipse IDE (Incubation) Bundle-SymbolicName: org.eclipse.lsp4e;singleton:=true -Bundle-Version: 0.19.2.qualifier +Bundle-Version: 0.19.3.qualifier Bundle-RequiredExecutionEnvironment: JavaSE-21 Require-Bundle: org.eclipse.core.runtime;bundle-version="3.12.0", org.eclipse.equinox.common;bundle-version="3.8.0", diff --git a/org.eclipse.lsp4e/pom.xml b/org.eclipse.lsp4e/pom.xml index 7bde4b2e6..249fff5a6 100644 --- a/org.eclipse.lsp4e/pom.xml +++ b/org.eclipse.lsp4e/pom.xml @@ -10,7 +10,7 @@ org.eclipse.lsp4e eclipse-plugin - 0.19.2-SNAPSHOT + 0.19.3-SNAPSHOT diff --git a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/format/LSPFormatter.java b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/format/LSPFormatter.java index 0610567d7..3005b5146 100644 --- a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/format/LSPFormatter.java +++ b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/format/LSPFormatter.java @@ -14,9 +14,11 @@ package org.eclipse.lsp4e.operations.format; import java.net.URI; +import java.util.List; import java.util.Optional; import java.util.concurrent.CompletableFuture; +import org.eclipse.jdt.annotation.Nullable; import org.eclipse.jface.preference.IPreferenceStore; import org.eclipse.jface.text.BadLocationException; import org.eclipse.jface.text.IDocument; @@ -31,6 +33,7 @@ import org.eclipse.lsp4j.FormattingOptions; import org.eclipse.lsp4j.ServerCapabilities; import org.eclipse.lsp4j.TextDocumentIdentifier; +import org.eclipse.lsp4j.TextEdit; import org.eclipse.ui.editors.text.EditorsUI; import org.eclipse.ui.texteditor.AbstractDecoratedTextEditorPreferenceConstants; @@ -49,20 +52,21 @@ public CompletableFuture> requestFormatting(IDocument d DocumentFormattingParams params = getFullFormatParams(formatOptions, docId); - // TODO: Could refine this algorithm: at present this grabs the first non-null response but the most functional - // implementation (if a text selection is present) would try all the servers in turn to see if they supported - // range formatting, falling back to a full format if unavailable + // **NOTE:** We let LanguageServers.computeFirst() see the *raw* edit lists so that servers which + // advertise formatting but return no edits (empty list) are treated as "no result" and formatting + // can fall through to the next server (e.g. Vue LS after TS LS on .vue files). long modificationStamp = DocumentUtil.getDocumentModificationStamp(document); return executor.computeFirst((w, ls) -> w.getServerCapabilitiesAsync().thenCompose(capabilities -> { if (isDocumentRangeFormattingSupported(capabilities) && (textSelection.getLength() > 0 || !isDocumentFormattingSupported(capabilities))) { - return ls.getTextDocumentService().rangeFormatting(rangeParams) - .thenApply(edits -> new VersionedEdits(modificationStamp, edits, document)); + return (CompletableFuture<@Nullable List>) ls.getTextDocumentService() + .rangeFormatting(rangeParams); } else if (isDocumentFormattingSupported(capabilities)) { - return ls.getTextDocumentService().formatting(params) - .thenApply(edits -> new VersionedEdits(modificationStamp, edits, document)); + return (CompletableFuture<@Nullable List>) ls.getTextDocumentService() + .formatting(params); } - return CompletableFuture.completedFuture(null); - })); + return CompletableFuture.completedFuture(null); + })).thenApply( + optionalEdits -> optionalEdits.map(edits -> new VersionedEdits(modificationStamp, edits, document))); } public static DocumentFormattingParams getFullFormatParams(FormattingOptions formatOptions,