Skip to content

Commit 2e5a6d8

Browse files
sebthomsebthom
committed
fix: fall back to next LS when formatting returns no edits
1 parent 7ef482c commit 2e5a6d8

6 files changed

Lines changed: 49 additions & 28 deletions

File tree

‎org.eclipse.lsp4e.test/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: Tests for language server bundle (Incubation)
44
Bundle-SymbolicName: org.eclipse.lsp4e.test;singleton:=true
5-
Bundle-Version: 0.16.3.qualifier
5+
Bundle-Version: 0.16.4.qualifier
66
Fragment-Host: org.eclipse.lsp4e
77
Bundle-Vendor: Eclipse LSP4E
88
Bundle-RequiredExecutionEnvironment: JavaSE-21

‎org.eclipse.lsp4e.test/pom.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
</parent>
99
<artifactId>org.eclipse.lsp4e.test</artifactId>
1010
<packaging>eclipse-test-plugin</packaging>
11-
<version>0.16.3-SNAPSHOT</version>
11+
<version>0.16.4-SNAPSHOT</version>
1212

1313
<properties>
1414
<os-jvm-flags /> <!-- for the default case -->

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

Lines changed: 32 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -11,16 +11,13 @@
1111
*******************************************************************************/
1212
package org.eclipse.lsp4e.test.format;
1313

14-
import static org.eclipse.lsp4e.test.utils.TestUtils.numberOfChangesIs;
15-
import static org.eclipse.lsp4e.test.utils.TestUtils.waitForAndAssertCondition;
16-
import static org.junit.jupiter.api.Assertions.assertEquals;
17-
import static org.junit.jupiter.api.Assertions.assertThrows;
18-
import static org.junit.jupiter.api.Assertions.assertTrue;
19-
import static org.junit.jupiter.api.Assertions.fail;
14+
import static org.eclipse.lsp4e.test.utils.TestUtils.*;
15+
import static org.junit.jupiter.api.Assertions.*;
2016

2117
import java.util.ArrayList;
2218
import java.util.Collections;
2319
import java.util.ConcurrentModificationException;
20+
import java.util.List;
2421
import java.util.Optional;
2522
import java.util.concurrent.ExecutionException;
2623

@@ -57,6 +54,29 @@ public void testFormattingInvalidDocument() throws Exception {
5754
assertTrue(edits.isEmpty());
5855
}
5956

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

7191
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-
});
92+
93+
// No formatting edits produced -> no VersionedEdits returned
94+
assertTrue(edits.isEmpty());
95+
8096
final var textEditor = (ITextEditor) editor;
8197
textEditor.getDocumentProvider().getDocument(textEditor.getEditorInput());
8298
assertEquals("Formatting Other Text", viewer.getDocument().get());
@@ -243,7 +259,8 @@ public void testSelectiveFormattingWithIncapableServer() throws Exception {
243259
@Test
244260
public void testOutdatedFormatting()
245261
throws CoreException, InterruptedException, ExecutionException, BadLocationException {
246-
MockLanguageServer.INSTANCE.setFormattingTextEdits(Collections.emptyList());
262+
// Use a non-empty edit list so that a VersionedEdits is produced
263+
MockLanguageServer.INSTANCE.setFormattingTextEdits(List.of(new TextEdit(new Range(new Position(0, 0), new Position(0, 0)), "X")));
247264

248265
IFile file = TestUtils.createUniqueTestFile(project, "Formatting Other Text");
249266
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)