Skip to content

Commit 604bf2f

Browse files
DanTupCommit Queue
authored and
Commit Queue
committed
[analysis_server] Don't overlap folding regions when lineFoldingOnly
+ convert LSP folding tests to use TestCode parser. A change in VS Code means two folding regions are no longer allowed to end/start on the same line (the second range is silently dropped). This truncates folding regions if they end on the same line that another starts to end on the line before (but only if a client only supports line-folding mode). Fixes Dart-Code/Dart-Code#4121. Change-Id: Ic26f58f84c44a01ae5157c336ed0f207d1c0eeb8 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/261900 Reviewed-by: Brian Wilkerson <[email protected]> Commit-Queue: Brian Wilkerson <[email protected]>
1 parent f70ca8e commit 604bf2f

File tree

6 files changed

+271
-185
lines changed

6 files changed

+271
-185
lines changed

pkg/analysis_server/lib/src/lsp/client_capabilities.dart

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ class LspClientCapabilities {
7171
final bool typeDefinitionLocationLink;
7272
final bool hierarchicalSymbols;
7373
final bool diagnosticCodeDescription;
74+
final bool lineFoldingOnly;
7475
final Set<CodeActionKind> codeActionKinds;
7576
final Set<CompletionItemTag> completionItemTags;
7677
final Set<DiagnosticTag> diagnosticTags;
@@ -142,6 +143,8 @@ class LspClientCapabilities {
142143
final hoverContentFormats = _listToNullableSet(hover?.contentFormat);
143144
final insertReplaceCompletionRanges =
144145
completionItem?.insertReplaceSupport ?? false;
146+
final lineFoldingOnly =
147+
textDocument?.foldingRange?.lineFoldingOnly ?? false;
145148
final literalCodeActions = codeActionLiteral != null;
146149
final renameValidation = textDocument?.rename?.prepareSupport ?? false;
147150
final signatureHelpDocumentationFormats =
@@ -170,6 +173,7 @@ class LspClientCapabilities {
170173
definitionLocationLink: definitionLocationLink,
171174
typeDefinitionLocationLink: typeDefinitionLocationLink,
172175
hierarchicalSymbols: hierarchicalSymbols,
176+
lineFoldingOnly: lineFoldingOnly,
173177
diagnosticCodeDescription: diagnosticCodeDescription,
174178
codeActionKinds: codeActionKinds,
175179
completionItemTags: completionItemTags,
@@ -204,6 +208,7 @@ class LspClientCapabilities {
204208
required this.definitionLocationLink,
205209
required this.typeDefinitionLocationLink,
206210
required this.hierarchicalSymbols,
211+
required this.lineFoldingOnly,
207212
required this.diagnosticCodeDescription,
208213
required this.codeActionKinds,
209214
required this.completionItemTags,

pkg/analysis_server/lib/src/lsp/handlers/handler_folding.dart

Lines changed: 72 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,13 @@ class FoldingHandler
2222
@override
2323
Future<ErrorOr<List<FoldingRange>>> handle(FoldingRangeParams params,
2424
MessageInfo message, CancellationToken token) async {
25+
final clientCapabilities = server.clientCapabilities;
26+
if (clientCapabilities == null) {
27+
// This should not happen unless a client misbehaves.
28+
return serverNotInitializedError;
29+
}
30+
31+
final lineFoldingOnly = clientCapabilities.lineFoldingOnly;
2532
final path = pathOfDoc(params.textDocument);
2633

2734
return path.mapResult((path) async {
@@ -53,9 +60,71 @@ class FoldingHandler
5360
final regions =
5461
notificationManager.merger.mergeFoldingRegions(partialResults);
5562

56-
return success(
57-
regions.map((region) => toFoldingRange(lineInfo!, region)).toList(),
58-
);
63+
// Ensure sorted by offset for when looking for overlapping ranges in
64+
// line mode below.
65+
regions.sort((r1, r2) => r1.offset.compareTo(r2.offset));
66+
67+
final foldingRanges = regions
68+
.map((region) =>
69+
_toFoldingRange(lineInfo!, region, lineOnly: lineFoldingOnly))
70+
.toList();
71+
72+
// When in line-only mode, ranges that end on the same line that another
73+
// ranges starts should be truncated to be on the line before (and if this
74+
// leave them spanning only a single line, should be removed).
75+
if (lineFoldingOnly) {
76+
_compensateForLineFolding(foldingRanges);
77+
}
78+
79+
return success(foldingRanges);
5980
});
6081
}
82+
83+
/// Adjust [foldingRanges] taking into count additional rules for line
84+
/// folding.
85+
///
86+
/// When character folding is supported, a range may start on the same line
87+
/// that another ends (as long as they don't overlap).
88+
///
89+
/// When only line folding is supported, ranges must not end on the same line
90+
/// that another starts. In this case, we shrink the previous range (and if
91+
/// this makes it a single line, remove it).
92+
void _compensateForLineFolding(List<FoldingRange> foldingRanges) {
93+
// Loop over items expect last (`-1`). We can skip the last item because
94+
// it has no next item.
95+
for (var i = 0; i < foldingRanges.length - 1; i++) {
96+
final range = foldingRanges[i];
97+
final next = foldingRanges[i + 1];
98+
// If this item runs into the next...
99+
if (range.endLine >= next.startLine) {
100+
// Truncate it to end on the line before.
101+
final newEndLine = next.startLine - 1;
102+
103+
// If it no longer needs to be a folding range at all, remove it.
104+
if (newEndLine <= range.startLine) {
105+
foldingRanges.removeAt(i);
106+
i--;
107+
continue;
108+
}
109+
110+
foldingRanges[i] = FoldingRange(
111+
startLine: range.startLine,
112+
endLine: newEndLine,
113+
kind: range.kind,
114+
);
115+
}
116+
}
117+
}
118+
119+
FoldingRange _toFoldingRange(LineInfo lineInfo, FoldingRegion region,
120+
{required bool lineOnly}) {
121+
final range = toRange(lineInfo, region.offset, region.length);
122+
return FoldingRange(
123+
startLine: range.start.line,
124+
startCharacter: lineOnly ? null : range.start.character,
125+
endLine: range.end.line,
126+
endCharacter: lineOnly ? null : range.end.character,
127+
kind: toFoldingRangeKind(region.kind),
128+
);
129+
}
61130
}

pkg/analysis_server/lib/src/lsp/mapping.dart

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1079,17 +1079,6 @@ lsp.FlutterOutlineAttribute toFlutterOutlineAttribute(
10791079
: null);
10801080
}
10811081

1082-
lsp.FoldingRange toFoldingRange(
1083-
server.LineInfo lineInfo, server.FoldingRegion region) {
1084-
final range = toRange(lineInfo, region.offset, region.length);
1085-
return lsp.FoldingRange(
1086-
startLine: range.start.line,
1087-
startCharacter: range.start.character,
1088-
endLine: range.end.line,
1089-
endCharacter: range.end.character,
1090-
kind: toFoldingRangeKind(region.kind));
1091-
}
1092-
10931082
lsp.FoldingRangeKind? toFoldingRangeKind(server.FoldingKind kind) {
10941083
switch (kind) {
10951084
case server.FoldingKind.COMMENT:

0 commit comments

Comments
 (0)