Skip to content

Commit 4e5c4a9

Browse files
sebthomsebthom
authored
fix: fall back to next LS when formatting returns no edits (#1433)
1 parent 70e805e commit 4e5c4a9

4 files changed

Lines changed: 45 additions & 20 deletions

File tree

‎org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/format/FormatTest.java‎

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import java.util.ArrayList;
2222
import java.util.Collections;
2323
import java.util.ConcurrentModificationException;
24+
import java.util.List;
2425
import java.util.Optional;
2526
import java.util.concurrent.ExecutionException;
2627

@@ -57,6 +58,29 @@ public void testFormattingInvalidDocument() throws Exception {
5758
assertTrue(edits.isEmpty());
5859
}
5960

61+
/**
62+
* Verifies that when the language server reports no formatting edits
63+
* (empty edit list), {@link LSPFormatter} returns an empty Optional
64+
* and leaves the document content unchanged.
65+
*/
66+
@Test
67+
public void testFormattingEmptyEditsYieldEmptyOptional() throws Exception {
68+
MockLanguageServer.INSTANCE.setFormattingTextEdits(Collections.emptyList());
69+
70+
IFile file = TestUtils.createUniqueTestFile(project, "Formatting Other Text");
71+
IEditorPart editor = TestUtils.openEditor(file);
72+
ITextViewer viewer = LSPEclipseUtils.getTextViewer(editor);
73+
74+
final var formatter = new LSPFormatter();
75+
ISelection selection = viewer.getSelectionProvider().getSelection();
76+
77+
Optional<VersionedEdits> edits = formatter.requestFormatting(viewer.getDocument(), (ITextSelection) selection).get();
78+
assertTrue(edits.isEmpty());
79+
assertEquals("Formatting Other Text", viewer.getDocument().get());
80+
81+
TestUtils.closeEditor(editor, false);
82+
}
83+
6084
@Test
6185
public void testFormattingNoChanges() throws Exception {
6286
MockLanguageServer.INSTANCE.setFormattingTextEdits(Collections.emptyList());
@@ -69,14 +93,10 @@ public void testFormattingNoChanges() throws Exception {
6993
ISelection selection = viewer.getSelectionProvider().getSelection();
7094

7195
Optional<VersionedEdits> edits = formatter.requestFormatting(viewer.getDocument(), (ITextSelection) selection).get();
72-
assertTrue(edits.isPresent());
73-
editor.getSite().getShell().getDisplay().syncExec(() -> {
74-
try {
75-
edits.get().apply();
76-
} catch (ConcurrentModificationException | BadLocationException e) {
77-
fail(e.getMessage());
78-
}
79-
});
96+
97+
// No formatting edits produced -> no VersionedEdits returned
98+
assertTrue(edits.isEmpty());
99+
80100
final var textEditor = (ITextEditor) editor;
81101
textEditor.getDocumentProvider().getDocument(textEditor.getEditorInput());
82102
assertEquals("Formatting Other Text", viewer.getDocument().get());
@@ -243,7 +263,8 @@ public void testSelectiveFormattingWithIncapableServer() throws Exception {
243263
@Test
244264
public void testOutdatedFormatting()
245265
throws CoreException, InterruptedException, ExecutionException, BadLocationException {
246-
MockLanguageServer.INSTANCE.setFormattingTextEdits(Collections.emptyList());
266+
// Use a non-empty edit list so that a VersionedEdits is produced
267+
MockLanguageServer.INSTANCE.setFormattingTextEdits(List.of(new TextEdit(new Range(new Position(0, 0), new Position(0, 0)), "X")));
247268

248269
IFile file = TestUtils.createUniqueTestFile(project, "Formatting Other Text");
249270
IEditorPart editor = TestUtils.openEditor(file);

‎org.eclipse.lsp4e/META-INF/MANIFEST.MF‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
22
Bundle-ManifestVersion: 2
33
Bundle-Name: Language Server Protocol client for Eclipse IDE (Incubation)
44
Bundle-SymbolicName: org.eclipse.lsp4e;singleton:=true
5-
Bundle-Version: 0.19.2.qualifier
5+
Bundle-Version: 0.19.3.qualifier
66
Bundle-RequiredExecutionEnvironment: JavaSE-21
77
Require-Bundle: org.eclipse.core.runtime;bundle-version="3.12.0",
88
org.eclipse.equinox.common;bundle-version="3.8.0",

‎org.eclipse.lsp4e/pom.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
</parent>
1111
<artifactId>org.eclipse.lsp4e</artifactId>
1212
<packaging>eclipse-plugin</packaging>
13-
<version>0.19.2-SNAPSHOT</version>
13+
<version>0.19.3-SNAPSHOT</version>
1414

1515
<build>
1616
<plugins>

‎org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/format/LSPFormatter.java‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,11 @@
1414
package org.eclipse.lsp4e.operations.format;
1515

1616
import java.net.URI;
17+
import java.util.List;
1718
import java.util.Optional;
1819
import java.util.concurrent.CompletableFuture;
1920

21+
import org.eclipse.jdt.annotation.Nullable;
2022
import org.eclipse.jface.preference.IPreferenceStore;
2123
import org.eclipse.jface.text.BadLocationException;
2224
import org.eclipse.jface.text.IDocument;
@@ -31,6 +33,7 @@
3133
import org.eclipse.lsp4j.FormattingOptions;
3234
import org.eclipse.lsp4j.ServerCapabilities;
3335
import org.eclipse.lsp4j.TextDocumentIdentifier;
36+
import org.eclipse.lsp4j.TextEdit;
3437
import org.eclipse.ui.editors.text.EditorsUI;
3538
import org.eclipse.ui.texteditor.AbstractDecoratedTextEditorPreferenceConstants;
3639

@@ -49,20 +52,21 @@ public CompletableFuture<Optional<VersionedEdits>> requestFormatting(IDocument d
4952

5053
DocumentFormattingParams params = getFullFormatParams(formatOptions, docId);
5154

52-
// TODO: Could refine this algorithm: at present this grabs the first non-null response but the most functional
53-
// implementation (if a text selection is present) would try all the servers in turn to see if they supported
54-
// range formatting, falling back to a full format if unavailable
55+
// **NOTE:** We let LanguageServers.computeFirst() see the *raw* edit lists so that servers which
56+
// advertise formatting but return no edits (empty list) are treated as "no result" and formatting
57+
// can fall through to the next server (e.g. Vue LS after TS LS on .vue files).
5558
long modificationStamp = DocumentUtil.getDocumentModificationStamp(document);
5659
return executor.computeFirst((w, ls) -> w.getServerCapabilitiesAsync().thenCompose(capabilities -> {
5760
if (isDocumentRangeFormattingSupported(capabilities) && (textSelection.getLength() > 0 || !isDocumentFormattingSupported(capabilities))) {
58-
return ls.getTextDocumentService().rangeFormatting(rangeParams)
59-
.thenApply(edits -> new VersionedEdits(modificationStamp, edits, document));
61+
return (CompletableFuture<@Nullable List<? extends TextEdit>>) ls.getTextDocumentService()
62+
.rangeFormatting(rangeParams);
6063
} else if (isDocumentFormattingSupported(capabilities)) {
61-
return ls.getTextDocumentService().formatting(params)
62-
.thenApply(edits -> new VersionedEdits(modificationStamp, edits, document));
64+
return (CompletableFuture<@Nullable List<? extends TextEdit>>) ls.getTextDocumentService()
65+
.formatting(params);
6366
}
64-
return CompletableFuture.<VersionedEdits>completedFuture(null);
65-
}));
67+
return CompletableFuture.completedFuture(null);
68+
})).thenApply(
69+
optionalEdits -> optionalEdits.map(edits -> new VersionedEdits(modificationStamp, edits, document)));
6670
}
6771

6872
public static DocumentFormattingParams getFullFormatParams(FormattingOptions formatOptions,

0 commit comments

Comments
 (0)