Skip to content

[JVSC-249] Fix code folding in VS Code due to line-only folding limitation #304

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 28, 2024
Merged

[JVSC-249] Fix code folding in VS Code due to line-only folding limitation #304

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions build.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
patches/7722.diff
patches/7724.diff
patches/7733.diff
patches/7750.diff
patches/7910.diff
patches/mvn-sh.diff
patches/generate-dependencies.diff
Expand Down
293 changes: 293 additions & 0 deletions patches/7750.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,293 @@
diff --git a/java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/protocol/TextDocumentServiceImpl.java b/java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/protocol/TextDocumentServiceImpl.java
index c112b4eb73..ff1fe7f903 100644
--- a/java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/protocol/TextDocumentServiceImpl.java
+++ b/java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/protocol/TextDocumentServiceImpl.java
@@ -44,14 +44,18 @@ import java.net.MalformedURLException;
import java.net.URISyntaxException;
import java.nio.file.Path;
import java.nio.file.Paths;
+import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
+import java.util.Comparator;
+import java.util.Deque;
import java.util.EnumMap;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.HashSet;
+import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Locale;
@@ -1561,12 +1565,13 @@ public class TextDocumentServiceImpl implements TextDocumentService, LanguageCli
if (source == null) {
return CompletableFuture.completedFuture(Collections.emptyList());
}
+ final boolean lineFoldingOnly = client.getNbCodeCapabilities().getClientCapabilities().getTextDocument().getFoldingRange().getLineFoldingOnly() == Boolean.TRUE;
CompletableFuture<List<FoldingRange>> result = new CompletableFuture<>();
try {
source.runUserActionTask(cc -> {
cc.toPhase(JavaSource.Phase.RESOLVED);
Document doc = cc.getSnapshot().getSource().getDocument(true);
- JavaElementFoldVisitor v = new JavaElementFoldVisitor(cc, cc.getCompilationUnit(), cc.getTrees().getSourcePositions(), doc, new FoldCreator<FoldingRange>() {
+ JavaElementFoldVisitor<FoldingRange> v = new JavaElementFoldVisitor<>(cc, cc.getCompilationUnit(), cc.getTrees().getSourcePositions(), doc, new FoldCreator<FoldingRange>() {
@Override
public FoldingRange createImportsFold(int start, int end) {
return createFold(start, end, FoldingRangeKind.Imports);
@@ -1611,7 +1616,10 @@ public class TextDocumentServiceImpl implements TextDocumentService, LanguageCli
});
v.checkInitialFold();
v.scan(cc.getCompilationUnit(), null);
- result.complete(v.getFolds());
+ List<FoldingRange> folds = v.getFolds();
+ if (lineFoldingOnly)
+ folds = convertToLineOnlyFolds(folds);
+ result.complete(folds);
}, true);
} catch (IOException ex) {
result.completeExceptionally(ex);
@@ -1619,6 +1627,76 @@ public class TextDocumentServiceImpl implements TextDocumentService, LanguageCli
return result;
}

+ /**
+ * Converts a list of code-folds to a line-only Range form, in place of the
+ * finer-grained form of {@linkplain Position Position-based} (line, column) Ranges.
+ * <p>
+ * This is needed for LSP clients that do not support the finer grained Range
+ * specification. This is expected to be advertised by the client in
+ * {@code FoldingRangeClientCapabilities.lineFoldingOnly}.
+ *
+ * @implSpec The line-only ranges computed uphold the code-folding invariant that:
+ * <em>a fold <b>does not end</b> at the same point <b>where</b> another fold <b>starts</b></em>.
+ *
+ * @implNote This is performed in {@code O(n log n) + O(n)} time and {@code O(n)} space for the returned list.
+ *
+ * @param folds List of code-folding ranges computed for a textDocument,
+ * containing fine-grained {@linkplain Position Position-based}
+ * (line, column) ranges.
+ * @return List of code-folding ranges computed for a textDocument,
+ * containing coarse-grained line-only ranges.
+ *
+ * @see <a href="https://microsoft.github.io/language-server-protocol/specifications/specification-current/#foldingRangeClientCapabilities">
+ * LSP FoldingRangeClientCapabilities</a>
+ */
+ static List<FoldingRange> convertToLineOnlyFolds(List<FoldingRange> folds) {
+ if (folds != null && folds.size() > 1) {
+ // Ensure that the folds are sorted in increasing order of their start position
+ folds = new ArrayList<>(folds);
+ folds.sort(Comparator.comparingInt(FoldingRange::getStartLine)
+ .thenComparing(FoldingRange::getStartCharacter));
+ // Maintain a stack of enclosing folds
+ Deque<FoldingRange> enclosingFolds = new ArrayDeque<>();
+ for (FoldingRange fold : folds) {
+ FoldingRange last;
+ while ((last = enclosingFolds.peek()) != null &&
+ (last.getEndLine() < fold.getEndLine() ||
+ (last.getEndLine() == fold.getEndLine() && last.getEndCharacter() < fold.getEndCharacter()))) {
+ // The last enclosingFold does not enclose this fold.
+ // Due to sortedness of the folds, last also ends before this fold starts.
+ enclosingFolds.pop();
+ // If needed, adjust last to end on a line prior to this fold start
+ if (last.getEndLine() == fold.getStartLine()) {
+ last.setEndLine(last.getEndLine() - 1);
+ }
+ last.setEndCharacter(null); // null denotes the end of the line.
+ last.setStartCharacter(null); // null denotes the end of the line.
+ }
+ enclosingFolds.push(fold);
+ }
+ // empty the stack; since each fold completely encloses the next higher one.
+ FoldingRange fold;
+ while ((fold = enclosingFolds.poll()) != null) {
+ fold.setEndCharacter(null); // null denotes the end of the line.
+ fold.setStartCharacter(null); // null denotes the end of the line.
+ }
+ // Remove invalid or duplicate folds
+ Iterator<FoldingRange> it = folds.iterator();
+ FoldingRange prev = null;
+ while(it.hasNext()) {
+ FoldingRange next = it.next();
+ if (next.getEndLine() <= next.getStartLine() ||
+ (prev != null && prev.equals(next))) {
+ it.remove();
+ } else {
+ prev = next;
+ }
+ }
+ }
+ return folds;
+ }
+
+
@Override
public void didOpen(DidOpenTextDocumentParams params) {
LOG.log(Level.FINER, "didOpen: {0}", params);
diff --git a/java/java.lsp.server/test/unit/src/org/netbeans/modules/java/lsp/server/protocol/TextDocumentServiceImplTest.java b/java/java.lsp.server/test/unit/src/org/netbeans/modules/java/lsp/server/protocol/TextDocumentServiceImplTest.java
index 0f2bda50ae..06fd93d3e5 100644
--- a/java/java.lsp.server/test/unit/src/org/netbeans/modules/java/lsp/server/protocol/TextDocumentServiceImplTest.java
+++ b/java/java.lsp.server/test/unit/src/org/netbeans/modules/java/lsp/server/protocol/TextDocumentServiceImplTest.java
@@ -18,14 +18,19 @@
*/
package org.netbeans.modules.java.lsp.server.protocol;

+import java.util.Collections;
+import java.util.List;
import java.util.concurrent.atomic.AtomicInteger;
import javax.swing.event.DocumentEvent;
import javax.swing.event.DocumentListener;
import javax.swing.text.BadLocationException;
import javax.swing.text.Document;
import javax.swing.text.PlainDocument;
+import org.eclipse.lsp4j.FoldingRange;
import org.netbeans.junit.NbTestCase;

+import static org.netbeans.modules.java.lsp.server.protocol.TextDocumentServiceImpl.convertToLineOnlyFolds;
+
public class TextDocumentServiceImplTest extends NbTestCase {

public TextDocumentServiceImplTest(String name) {
@@ -117,4 +122,141 @@ public class TextDocumentServiceImplTest extends NbTestCase {
fail(String.valueOf(e));
}
}
+
+ public void testConvertToLineOnlyFolds() {
+ assertNull(convertToLineOnlyFolds(null));
+ assertEquals(0, convertToLineOnlyFolds(Collections.emptyList()).size());
+ List<FoldingRange> inputFolds, outputFolds;
+ inputFolds = Collections.singletonList(createRange(10, 20));
+ assertEquals(inputFolds, convertToLineOnlyFolds(inputFolds));
+
+ // test stable sort by start index
+ inputFolds = List.of(createRange(10, 20, 9, 9), createRange(5, 9, 9, 9), createRange(10, 19, 9, 9), createRange(10, 14, 13, 13));
+ outputFolds = List.of(createRange(5, 9), createRange(10, 20), createRange(10, 19), createRange(10, 14));
+ assertEquals(outputFolds, convertToLineOnlyFolds(inputFolds));
+
+ // test already disjoint folds
+ inputFolds = List.of(createRange(10, 20, 9, 9), createRange(5, 9, 9, 9), createRange(15, 19, 13, 13), createRange(10, 14, 13, 13));
+ outputFolds = List.of(createRange(5, 9), createRange(10, 20), createRange(10, 14), createRange(15, 19));
+ assertEquals(outputFolds, convertToLineOnlyFolds(inputFolds));
+
+ // test invariant of range.endLine: there exists no otherRange.startLine == range.endLine.
+ inputFolds = List.of(createRange(10, 20, 35, 9), createRange(5, 10, 12, 9), createRange(15, 19, 20, 13), createRange(10, 15, 51, 13));
+ assertEquals(outputFolds, convertToLineOnlyFolds(inputFolds));
+
+ // test a complex example of a full file:
+//import java.util.ArrayList;
+//import java.util.Collection;
+//import java.util.Collections;
+//
+///**
+// * A top-class action performer
+// *
+// * @since 1.1
+// */
+//public class TopClass {
+//
+// private final String action;
+// private final int index;
+//
+// /**
+// * @param action Top action to be done
+// */
+// public TopClass(String action) {
+// this(action, 0);
+// }
+//
+// /**
+// * @param action Top action to be done
+// * @param index Action index
+// */
+// public TopClass(String action, int index) {
+// this.action = action;
+// this.index = index;
+// }
+//
+// public void doSomethingTopClass(TopClass tc) {
+// // what can we do
+// {
+// if (tc == this) {
+// return;
+// } else if (tc.getClass() == this.getClass()) {
+// } else if (tc.getClass().isAssignableFrom(this.getClass())) {
+//
+// } else {
+// if (true) {
+// switch (tc) {
+// default: { /* this is some comment */ ; }
+// /// some outside default
+// }
+// } else { if (true) { { /* some */ } { /* bad blocks */ }
+// }}
+// /* done */
+// }
+// }
+// tc.doSomethingTopClass(tc);
+// }
+//
+// public class InnerClass {
+// @Override
+// public String toString() {
+// StringBuilder sb = new StringBuilder();
+// sb.append("InnerClass{");
+// sb.append("action=").append(action);
+// sb.append(", index=").append(index);
+// sb.append('}');
+// return sb.toString();
+// }
+// }
+//}
+ inputFolds = List.of(
+ createRange(27, 30, 48, 5),
+ createRange(0, 3, 7, 30),
+ createRange(32, 52, 51, 5),
+ createRange(37, 38, 59, 13),
+ createRange(34, 50, 10, 9),
+ createRange(46, 46, 39, 51),
+ createRange(35, 37, 30, 13),
+ createRange(38, 40, 74, 13),
+ createRange(40, 49, 21, 13),
+ createRange(46, 47, 37, 17),
+ createRange(41, 46, 28, 17),
+ createRange(42, 45, 34, 21),
+ createRange(11, 66, 24, 1),
+ createRange(43, 43, 35, 65),
+ createRange(46, 47, 25, 18),
+ createRange(54, 64, 30, 5),
+ createRange(46, 46, 54, 72),
+ createRange(6, 10, 4, 1),
+ createRange(56, 63, 35, 9)
+ );
+ outputFolds = List.of(
+ createRange(0, 3),
+ createRange(6, 10),
+ createRange(11, 66),
+ createRange(27, 30),
+ createRange(32, 52),
+ createRange(34, 50),
+ createRange(35, 36),
+ createRange(38, 39),
+ createRange(40, 49),
+ createRange(41, 45),
+ createRange(42, 45),
+ createRange(46, 47),
+ createRange(54, 64),
+ createRange(56, 63)
+ );
+ assertEquals(outputFolds, convertToLineOnlyFolds(inputFolds));
+ }
+
+ private static FoldingRange createRange(int startLine, int endLine) {
+ return new FoldingRange(startLine, endLine);
+ }
+
+ private static FoldingRange createRange(int startLine, int endLine, Integer startColumn, Integer endColumn) {
+ FoldingRange foldingRange = new FoldingRange(startLine, endLine);
+ foldingRange.setStartCharacter(startColumn);
+ foldingRange.setEndCharacter(endColumn);
+ return foldingRange;
+ }
}