Skip to content

Commit

Permalink
[JVSC-249] Fix code folding in VS Code due to line-only folding limit…
Browse files Browse the repository at this point in the history 
…ation

Added the backport of the Netbeans PR apache/netbeans#7750 patch which allows supporting LSP clients
which have line-only folding support.
- This is the case of VS Code as referenced in microsoft/vscode#50840.

Closes oracle#249

Signed-off-by: Siddharth Srinivasan <siddharth.srinivasan@oracle.com>
  • Loading branch information
@sid-srini
sid-srini committed Oct 26, 2024
1 parent 08ed2f7 commit 68c7ae0
Show file tree
Hide file tree
Showing 2 changed files with 294 additions and 0 deletions.
1 change: 1 addition & 0 deletions build.xml
View file
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/mvn-sh.diff
patches/generate-dependencies.diff
patches/rename-debugger.diff
Expand Down
293 changes: 293 additions & 0 deletions patches/7750.diff
View file
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;
+ }
}

0 comments on commit 68c7ae0

Please sign in to comment.