From 0324785df21d52392feb73a97731cbbf5e45f6c3 Mon Sep 17 00:00:00 2001 From: Dietrich Travkin <10887297+travkin79@users.noreply.github.com> Date: Thu, 3 Oct 2024 06:35:13 +0200 Subject: [PATCH] Fix selective formatting (issue #1127) (#1128) * Fix selective formatting (#1127) Also adapt mock server to support range formatting and add tests. * Bump versions --- org.eclipse.lsp4e.test/META-INF/MANIFEST.MF | 2 +- org.eclipse.lsp4e.test/pom.xml | 2 +- .../eclipse/lsp4e/test/format/FormatTest.java | 124 ++++++++++++++++++ .../META-INF/MANIFEST.MF | 2 +- .../lsp4e/tests/mock/MockLanguageServer.java | 1 + .../tests/mock/MockTextDocumentService.java | 23 +++- .../lsp4e/operations/format/LSPFormatter.java | 6 +- 7 files changed, 153 insertions(+), 7 deletions(-) diff --git a/org.eclipse.lsp4e.test/META-INF/MANIFEST.MF b/org.eclipse.lsp4e.test/META-INF/MANIFEST.MF index 43c576ffe..c1fa33639 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.15.18.qualifier +Bundle-Version: 0.15.19.qualifier Fragment-Host: org.eclipse.lsp4e Bundle-Vendor: Eclipse.org Bundle-RequiredExecutionEnvironment: JavaSE-17 diff --git a/org.eclipse.lsp4e.test/pom.xml b/org.eclipse.lsp4e.test/pom.xml index 1e38624a8..1f5ba2b79 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.15.18-SNAPSHOT + 0.15.19-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 2e711cae9..ab0cb24c0 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 @@ -36,6 +36,7 @@ import org.eclipse.lsp4e.tests.mock.MockLanguageServer; import org.eclipse.lsp4j.Position; import org.eclipse.lsp4j.Range; +import org.eclipse.lsp4j.ServerCapabilities; import org.eclipse.lsp4j.TextEdit; import org.eclipse.ui.IEditorPart; import org.eclipse.ui.texteditor.ITextEditor; @@ -111,6 +112,129 @@ public void testFormatting() TestUtils.closeEditor(editor, false); } + + @Test + public void testSelectiveFormatting() throws Exception { + String fileContent = "Line 1\nLine 2\n\nText to be formatted.\nLine 5"; + + final var formattingTextEdits = new ArrayList(); + formattingTextEdits.add(new TextEdit(new Range(new Position(0, 5), new Position(0, 5)), " changed")); + formattingTextEdits.add(new TextEdit(new Range(new Position(3, 10), new Position(3, 11)), "\n")); + MockLanguageServer.INSTANCE.setFormattingTextEdits(formattingTextEdits); + + IFile file = TestUtils.createUniqueTestFile(project, fileContent); + IEditorPart editor = TestUtils.openEditor(file); + ITextViewer viewer = LSPEclipseUtils.getTextViewer(editor); + + viewer.setSelectedRange(viewer.getDocument().getLineOffset(3), viewer.getDocument().getLineLength(3)); + + final var formatter = new LSPFormatter(); + ISelection selection = viewer.getSelectionProvider().getSelection(); + Optional edits = formatter.requestFormatting(viewer.getDocument(), (ITextSelection) selection).get(); + + // only the edits in the selection range are expected to be returned + assertTrue(edits.isPresent()); + assertEquals(1, edits.get().data.size()); + + editor.getSite().getShell().getDisplay().syncExec(() -> { + try { + edits.get().apply(); + } catch (ConcurrentModificationException | BadLocationException e) { + fail(e.getMessage()); + } + }); + + final var textEditor = (ITextEditor) editor; + textEditor.getDocumentProvider().getDocument(textEditor.getEditorInput()); + String formattedText = viewer.getDocument().get(); + assertEquals("Line 1\nLine 2\n\nText to be\nformatted.\nLine 5", formattedText); + + TestUtils.closeEditor(editor, false); + } + + @Test + public void testSelectiveFormattingWithEmptySelection() throws Exception { + String fileContent = "Line 1\nLine 2\n\nText to be formatted.\nLine 5"; + + final var formattingTextEdits = new ArrayList(); + formattingTextEdits.add(new TextEdit(new Range(new Position(0, 6), new Position(0, 6)), " changed")); + formattingTextEdits.add(new TextEdit(new Range(new Position(3, 10), new Position(3, 11)), "\n")); + MockLanguageServer.INSTANCE.setFormattingTextEdits(formattingTextEdits); + + IFile file = TestUtils.createUniqueTestFile(project, fileContent); + IEditorPart editor = TestUtils.openEditor(file); + ITextViewer viewer = LSPEclipseUtils.getTextViewer(editor); + + viewer.setSelectedRange(viewer.getDocument().getLineOffset(3), 0); + + final var formatter = new LSPFormatter(); + ISelection selection = viewer.getSelectionProvider().getSelection(); + Optional edits = formatter.requestFormatting(viewer.getDocument(), (ITextSelection) selection).get(); + + assertTrue(edits.isPresent()); + assertEquals(formattingTextEdits.size(), edits.get().data.size()); + + editor.getSite().getShell().getDisplay().syncExec(() -> { + try { + edits.get().apply(); + } catch (ConcurrentModificationException | BadLocationException e) { + fail(e.getMessage()); + } + }); + + final var textEditor = (ITextEditor) editor; + textEditor.getDocumentProvider().getDocument(textEditor.getEditorInput()); + String formattedText = viewer.getDocument().get(); + assertEquals("Line 1 changed\nLine 2\n\nText to be\nformatted.\nLine 5", formattedText); + + TestUtils.closeEditor(editor, false); + } + + private static ServerCapabilities customServerWithoutRangeFormatting() { + ServerCapabilities capabilities = MockLanguageServer.defaultServerCapabilities(); + capabilities.setDocumentRangeFormattingProvider(false); + return capabilities; + } + + @Test + public void testSelectiveFormattingWithIncapableServer() throws Exception { + MockLanguageServer.reset(FormatTest::customServerWithoutRangeFormatting); + + String fileContent = "Line 1\nLine 2\n\nText to be formatted.\nLine 5"; + + final var formattingTextEdits = new ArrayList(); + formattingTextEdits.add(new TextEdit(new Range(new Position(0, 6), new Position(0, 6)), " changed")); + formattingTextEdits.add(new TextEdit(new Range(new Position(3, 10), new Position(3, 11)), "\n")); + MockLanguageServer.INSTANCE.setFormattingTextEdits(formattingTextEdits); + + IFile file = TestUtils.createUniqueTestFile(project, fileContent); + IEditorPart editor = TestUtils.openEditor(file); + ITextViewer viewer = LSPEclipseUtils.getTextViewer(editor); + + viewer.setSelectedRange(viewer.getDocument().getLineOffset(3), viewer.getDocument().getLineLength(3)); + + final var formatter = new LSPFormatter(); + ISelection selection = viewer.getSelectionProvider().getSelection(); + Optional edits = formatter.requestFormatting(viewer.getDocument(), (ITextSelection) selection).get(); + + assertTrue(edits.isPresent()); + assertEquals(formattingTextEdits.size(), edits.get().data.size()); + + editor.getSite().getShell().getDisplay().syncExec(() -> { + try { + edits.get().apply(); + } catch (ConcurrentModificationException | BadLocationException e) { + fail(e.getMessage()); + } + }); + + final var textEditor = (ITextEditor) editor; + textEditor.getDocumentProvider().getDocument(textEditor.getEditorInput()); + String formattedText = viewer.getDocument().get(); + assertEquals("Line 1 changed\nLine 2\n\nText to be\nformatted.\nLine 5", formattedText); + + TestUtils.closeEditor(editor, false); + } @Test public void testOutdatedFormatting() diff --git a/org.eclipse.lsp4e.tests.mock/META-INF/MANIFEST.MF b/org.eclipse.lsp4e.tests.mock/META-INF/MANIFEST.MF index 5681dd2f6..dcb03d46b 100644 --- a/org.eclipse.lsp4e.tests.mock/META-INF/MANIFEST.MF +++ b/org.eclipse.lsp4e.tests.mock/META-INF/MANIFEST.MF @@ -2,7 +2,7 @@ Manifest-Version: 1.0 Bundle-ManifestVersion: 2 Bundle-Name: Mock Language Server to test LSP4E Bundle-SymbolicName: org.eclipse.lsp4e.tests.mock -Bundle-Version: 0.16.11.qualifier +Bundle-Version: 0.16.12.qualifier Bundle-Vendor: Eclipse LSP4E Bundle-RequiredExecutionEnvironment: JavaSE-17 Require-Bundle: org.eclipse.lsp4j;bundle-version="[0.23.0,0.24.0)", diff --git a/org.eclipse.lsp4e.tests.mock/src/org/eclipse/lsp4e/tests/mock/MockLanguageServer.java b/org.eclipse.lsp4e.tests.mock/src/org/eclipse/lsp4e/tests/mock/MockLanguageServer.java index 6e1a1decc..0d467a825 100644 --- a/org.eclipse.lsp4e.tests.mock/src/org/eclipse/lsp4e/tests/mock/MockLanguageServer.java +++ b/org.eclipse.lsp4e.tests.mock/src/org/eclipse/lsp4e/tests/mock/MockLanguageServer.java @@ -170,6 +170,7 @@ public static ServerCapabilities defaultServerCapabilities() { capabilities.setTypeDefinitionProvider(Boolean.TRUE); capabilities.setReferencesProvider(true); capabilities.setDocumentFormattingProvider(true); + capabilities.setDocumentRangeFormattingProvider(true); capabilities.setCodeActionProvider(Boolean.TRUE); capabilities.setCodeLensProvider(new CodeLensOptions(true)); capabilities.setDocumentLinkProvider(new DocumentLinkOptions()); diff --git a/org.eclipse.lsp4e.tests.mock/src/org/eclipse/lsp4e/tests/mock/MockTextDocumentService.java b/org.eclipse.lsp4e.tests.mock/src/org/eclipse/lsp4e/tests/mock/MockTextDocumentService.java index ee17a0a94..7b708da87 100644 --- a/org.eclipse.lsp4e.tests.mock/src/org/eclipse/lsp4e/tests/mock/MockTextDocumentService.java +++ b/org.eclipse.lsp4e.tests.mock/src/org/eclipse/lsp4e/tests/mock/MockTextDocumentService.java @@ -26,6 +26,7 @@ import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.CopyOnWriteArrayList; import java.util.function.Function; +import java.util.stream.Collectors; import org.eclipse.lsp4j.CodeAction; import org.eclipse.lsp4j.CodeActionParams; @@ -225,7 +226,27 @@ public CompletableFuture> formatting(DocumentFormatting @Override public CompletableFuture> rangeFormatting(DocumentRangeFormattingParams params) { - return CompletableFuture.completedFuture(null); + Range range = params.getRange(); + List rangeEdits = mockFormattingTextEdits.stream() + .filter(edit -> inRange(range, edit.getRange())).collect(Collectors.toList()); + + return CompletableFuture.completedFuture(rangeEdits); + } + + private boolean inRange(Range containingRange, Range includedRange) { + if (containingRange == null || includedRange == null) { + throw new IllegalArgumentException(); + } + + if (containingRange.getStart().getLine() <= includedRange.getStart().getLine() + && containingRange.getEnd().getLine() >= includedRange.getEnd().getLine()) { + return (containingRange.getStart().getLine() != includedRange.getStart().getLine() + || containingRange.getStart().getCharacter() <= includedRange.getStart().getCharacter()) + && (containingRange.getEnd().getLine() != includedRange.getEnd().getLine() + || containingRange.getEnd().getCharacter() >= includedRange.getEnd().getCharacter()); + } + + return false; } @Override 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 fc7bf21b4..c94a620db 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 @@ -56,14 +56,14 @@ public CompletableFuture> requestFormatting(IDocument d // range formatting, falling back to a full format if unavailable long modificationStamp = DocumentUtil.getDocumentModificationStamp(document); return executor.computeFirst((w, ls) -> w.getServerCapabilitiesAsync().thenCompose(capabilities -> { - if (textSelection.getLength() == 0 && isDocumentRangeFormattingSupported(capabilities) - && !(isDocumentFormattingSupported(capabilities))) { + if (textSelection.getLength() != 0 && isDocumentRangeFormattingSupported(capabilities)) { return ls.getTextDocumentService().rangeFormatting(rangeParams) .thenApply(edits -> new VersionedEdits(modificationStamp, edits, document)); - } else { + } else if (isDocumentFormattingSupported(capabilities)) { return ls.getTextDocumentService().formatting(params) .thenApply(edits -> new VersionedEdits(modificationStamp, edits, document)); } + return CompletableFuture.completedFuture(null); })); }