Skip to content

Commit

Permalink
Fix code-folding for LSP clients that support line-folding only
Browse files Browse the repository at this point in the history 
1. Added `TextDocumentServiceImpl.convertToLineOnlyFolds` which:
    - accepts code-folds specified as fine-grained Position-based (line, column) ranges, and,
    - changes them to coarse-grained line-only ranges.
    - The line-only ranges computed uphold the code-folding invariant that: *a fold **does not end** at the same point **where** another fold **starts**. *

2. This is used in `TextDocumentServiceImpl.foldingRange()` when the client capabilities have `FoldingRangeClientCapabilities.lineFoldingOnly = true`.

Signed-off-by: Siddharth Srinivasan <[email protected]>
  • Loading branch information
@sid-srini
sid-srini committed Sep 17, 2024
1 parent 7bdf651 commit d371a8c
Show file tree
Hide file tree
Showing 2 changed files with 222 additions and 2 deletions.
82 changes: 80 additions & 2 deletions ...lsp.server/src/org/netbeans/modules/java/lsp/server/protocol/TextDocumentServiceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,17 @@
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.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;
Expand Down Expand Up @@ -1550,12 +1554,13 @@ public CompletableFuture<List<FoldingRange>> foldingRange(FoldingRangeRequestPar
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);
Expand Down Expand Up @@ -1600,14 +1605,87 @@ private FoldingRange createFold(int start, int end, String kind) {
});
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);
}
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);
Expand Down
142 changes: 142 additions & 0 deletions ...t/unit/src/org/netbeans/modules/java/lsp/server/protocol/TextDocumentServiceImplTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -117,4 +122,141 @@ public void changedUpdate(DocumentEvent e) {
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 d371a8c

Please sign in to comment.