Skip to content
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

Jump to bottom

Fix code-folding for LSP clients that support line-folding only #7750

Merged
merged 1 commit into from
Oct 29, 2024
Merged

Fix code-folding for LSP clients that support line-folding only #7750

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
82 changes: 80 additions & 2 deletions ...lsp.server/src/org/netbeans/modules/java/lsp/server/protocol/TextDocumentServiceImpl.java
View file
Open in desktop
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
Open in desktop
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;
}
}