Skip to content

Commit eee1726

Browse files
committed
Redo the Folding Range Request on top of the Syntax Tree
Building upon the infrastructure that requests the lexical structure of the document from SwiftSyntax, redo the folding ranges request using the tree directly. This corrects a number of inconsistencies in the tests mostly due to incorrect SourceLocations in the semantic ASTs and the line comment merging logic.
1 parent 8429f71 commit eee1726

File tree

2 files changed

+174
-117
lines changed

2 files changed

+174
-117
lines changed

Sources/SourceKitLSP/Swift/SwiftLanguageServer.swift

Lines changed: 161 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -961,143 +961,200 @@ extension SwiftLanguageServer {
961961
}
962962

963963
public func foldingRange(_ req: Request<FoldingRangeRequest>) {
964-
let keys = self.keys
965-
966964
queue.async {
967965
guard let snapshot = self.documentManager.latestSnapshot(req.params.textDocument.uri) else {
968966
log("failed to find snapshot for url \(req.params.textDocument.uri)")
969967
req.reply(nil)
970968
return
971969
}
972970

973-
let helperDocumentName = "FoldingRanges:" + snapshot.document.uri.pseudoPath
974-
let skreq = SKDRequestDictionary(sourcekitd: self.sourcekitd)
975-
skreq[keys.request] = self.requests.editor_open
976-
skreq[keys.name] = helperDocumentName
977-
skreq[keys.sourcetext] = snapshot.text
978-
skreq[keys.syntactic_only] = 1
971+
guard let sourceFile = snapshot.tokens.syntaxTree else {
972+
log("no lexical structure available for url \(req.params.textDocument.uri)")
973+
req.reply(nil)
974+
return
975+
}
979976

980-
let handle = self.sourcekitd.send(skreq, self.queue) { [weak self] result in
981-
guard let self = self else { return }
977+
final class FoldingRangeFinder: SyntaxVisitor {
978+
private let snapshot: DocumentSnapshot
979+
/// Some ranges might occur multiple times.
980+
/// E.g. for `print("hi")`, `"hi"` is both the range of all call arguments and the range the first argument in the call.
981+
/// It doesn't make sense to report them multiple times, so use a `Set` here.
982+
private var ranges: Set<FoldingRange>
983+
/// The client-imposed limit on the number of folding ranges it would
984+
/// prefer to recieve from the LSP server. If the value is `nil`, there
985+
/// is no preset limit.
986+
private var rangeLimit: Int?
987+
/// If `true`, the client is only capable of folding entire lines. If
988+
/// `false` the client can handle folding ranges.
989+
private var lineFoldingOnly: Bool
990+
991+
init(snapshot: DocumentSnapshot, rangeLimit: Int?, lineFoldingOnly: Bool) {
992+
self.snapshot = snapshot
993+
self.ranges = []
994+
self.rangeLimit = rangeLimit
995+
self.lineFoldingOnly = lineFoldingOnly
996+
super.init(viewMode: .sourceAccurate)
997+
}
982998

983-
defer {
984-
let closeHelperReq = SKDRequestDictionary(sourcekitd: self.sourcekitd)
985-
closeHelperReq[keys.request] = self.requests.editor_close
986-
closeHelperReq[keys.name] = helperDocumentName
987-
_ = self.sourcekitd.send(closeHelperReq, .global(qos: .utility), reply: { _ in })
999+
override func visit(_ node: TokenSyntax) -> SyntaxVisitorContinueKind {
1000+
// Index comments, so we need to see at least '/*', or '//'.
1001+
if node.leadingTriviaLength.utf8Length > 2 {
1002+
self.addTrivia(from: node, node.leadingTrivia)
1003+
}
1004+
1005+
if node.trailingTriviaLength.utf8Length > 2 {
1006+
self.addTrivia(from: node, node.trailingTrivia)
1007+
}
1008+
1009+
return .visitChildren
9881010
}
9891011

990-
guard let dict = result.success else {
991-
req.reply(.failure(ResponseError(result.failure!)))
992-
return
1012+
private func addTrivia(from node: TokenSyntax, _ trivia: Trivia) {
1013+
var start = node.position.utf8Offset
1014+
var lineCommentStart: Int? = nil
1015+
func flushLineComment(_ offset: Int = 0) {
1016+
if let lineCommentStart = lineCommentStart {
1017+
_ = self.addFoldingRange(
1018+
start: lineCommentStart,
1019+
end: start + offset,
1020+
kind: .comment)
1021+
}
1022+
lineCommentStart = nil
1023+
}
1024+
1025+
for piece in node.leadingTrivia {
1026+
defer { start += piece.sourceLength.utf8Length }
1027+
switch piece {
1028+
case .blockComment(_):
1029+
flushLineComment()
1030+
_ = self.addFoldingRange(
1031+
start: start,
1032+
end: start + piece.sourceLength.utf8Length,
1033+
kind: .comment)
1034+
case .docBlockComment(_):
1035+
flushLineComment()
1036+
_ = self.addFoldingRange(
1037+
start: start,
1038+
end: start + piece.sourceLength.utf8Length,
1039+
kind: .comment)
1040+
case .lineComment(_), .docLineComment(_):
1041+
if lineCommentStart == nil {
1042+
lineCommentStart = start
1043+
}
1044+
case .newlines(1), .carriageReturns(1), .spaces(_), .tabs(_):
1045+
if lineCommentStart != nil {
1046+
continue
1047+
} else {
1048+
flushLineComment()
1049+
}
1050+
default:
1051+
flushLineComment()
1052+
continue
1053+
}
1054+
}
1055+
1056+
flushLineComment()
9931057
}
9941058

995-
guard let syntaxMap: SKDResponseArray = dict[self.keys.syntaxmap],
996-
let substructure: SKDResponseArray = dict[self.keys.substructure] else {
997-
return req.reply([])
1059+
override func visit(_ node: CodeBlockSyntax) -> SyntaxVisitorContinueKind {
1060+
return self.addFoldingRange(
1061+
start: node.statements.position.utf8Offset,
1062+
end: node.rightBrace.positionAfterSkippingLeadingTrivia.utf8Offset)
9981063
}
9991064

1000-
/// Some ranges might occur multiple times.
1001-
/// E.g. for `print("hi")`, `"hi"` is both the range of all call arguments and the range the first argument in the call.
1002-
/// It doesn't make sense to report them multiple times, so use a `Set` here.
1003-
var ranges: Set<FoldingRange> = []
1065+
override func visit(_ node: MemberDeclBlockSyntax) -> SyntaxVisitorContinueKind {
1066+
return self.addFoldingRange(
1067+
start: node.members.position.utf8Offset,
1068+
end: node.rightBrace.positionAfterSkippingLeadingTrivia.utf8Offset)
1069+
}
10041070

1005-
var hasReachedLimit: Bool {
1006-
let capabilities = self.clientCapabilities.textDocument?.foldingRange
1007-
guard let rangeLimit = capabilities?.rangeLimit else {
1008-
return false
1009-
}
1010-
return ranges.count >= rangeLimit
1071+
override func visit(_ node: ClosureExprSyntax) -> SyntaxVisitorContinueKind {
1072+
return self.addFoldingRange(
1073+
start: node.statements.position.utf8Offset,
1074+
end: node.rightBrace.positionAfterSkippingLeadingTrivia.utf8Offset)
10111075
}
10121076

1013-
// If the limit is less than one, do nothing.
1014-
guard hasReachedLimit == false else {
1015-
req.reply([])
1016-
return
1077+
override func visit(_ node: AccessorBlockSyntax) -> SyntaxVisitorContinueKind {
1078+
return self.addFoldingRange(
1079+
start: node.accessors.position.utf8Offset,
1080+
end: node.rightBrace.positionAfterSkippingLeadingTrivia.utf8Offset)
10171081
}
10181082

1019-
// Merge successive comments into one big comment by adding their lengths.
1020-
var currentComment: (offset: Int, length: Int)? = nil
1083+
override func visit(_ node: SwitchStmtSyntax) -> SyntaxVisitorContinueKind {
1084+
return self.addFoldingRange(
1085+
start: node.cases.position.utf8Offset,
1086+
end: node.rightBrace.positionAfterSkippingLeadingTrivia.utf8Offset)
1087+
}
10211088

1022-
syntaxMap.forEach { _, value in
1023-
if let kind: sourcekitd_uid_t = value[self.keys.kind],
1024-
kind.isCommentKind(self.values),
1025-
let offset: Int = value[self.keys.offset],
1026-
let length: Int = value[self.keys.length]
1027-
{
1028-
if let comment = currentComment, comment.offset + comment.length == offset {
1029-
currentComment!.length += length
1030-
return true
1031-
}
1032-
if let comment = currentComment {
1033-
self.addFoldingRange(offset: comment.offset, length: comment.length, kind: .comment, in: snapshot, toSet: &ranges)
1034-
}
1035-
currentComment = (offset: offset, length: length)
1036-
}
1037-
return hasReachedLimit == false
1089+
override func visit(_ node: FunctionCallExprSyntax) -> SyntaxVisitorContinueKind {
1090+
return self.addFoldingRange(
1091+
start: node.argumentList.position.utf8Offset,
1092+
end: node.argumentList.endPosition.utf8Offset)
10381093
}
10391094

1040-
// Add the last stored comment.
1041-
if let comment = currentComment, hasReachedLimit == false {
1042-
self.addFoldingRange(offset: comment.offset, length: comment.length, kind: .comment, in: snapshot, toSet: &ranges)
1043-
currentComment = nil
1095+
override func visit(_ node: SubscriptExprSyntax) -> SyntaxVisitorContinueKind {
1096+
return self.addFoldingRange(
1097+
start: node.argumentList.position.utf8Offset,
1098+
end: node.argumentList.endPosition.utf8Offset)
10441099
}
10451100

1046-
var structureStack: [SKDResponseArray] = [substructure]
1047-
while !hasReachedLimit, let substructure = structureStack.popLast() {
1048-
substructure.forEach { _, value in
1049-
if let offset: Int = value[self.keys.bodyoffset],
1050-
let length: Int = value[self.keys.bodylength],
1051-
length > 0
1052-
{
1053-
self.addFoldingRange(offset: offset, length: length, in: snapshot, toSet: &ranges)
1054-
if hasReachedLimit {
1055-
return false
1056-
}
1057-
}
1058-
if let substructure: SKDResponseArray = value[self.keys.substructure] {
1059-
structureStack.append(substructure)
1101+
__consuming func finalize() -> Set<FoldingRange> {
1102+
return self.ranges
1103+
}
1104+
1105+
private func addFoldingRange(start: Int, end: Int, kind: FoldingRangeKind? = nil) -> SyntaxVisitorContinueKind {
1106+
if let limit = self.rangeLimit, self.ranges.count >= limit {
1107+
return .skipChildren
1108+
}
1109+
1110+
guard let start: Position = snapshot.positionOf(utf8Offset: start),
1111+
let end: Position = snapshot.positionOf(utf8Offset: end) else {
1112+
log("folding range failed to retrieve position of \(snapshot.document.uri): \(start)-\(end)", level: .warning)
1113+
return .visitChildren
1114+
}
1115+
let range: FoldingRange
1116+
if lineFoldingOnly {
1117+
// Since the client cannot fold less than a single line, if the
1118+
// fold would span 1 line there's no point in reporting it.
1119+
guard end.line > start.line else {
1120+
return .visitChildren
10601121
}
1061-
return true
1122+
1123+
// If the client only supports folding full lines, don't report
1124+
// the end of the range since there's nothing they could do with it.
1125+
range = FoldingRange(startLine: start.line,
1126+
startUTF16Index: nil,
1127+
endLine: end.line,
1128+
endUTF16Index: nil,
1129+
kind: kind)
1130+
} else {
1131+
range = FoldingRange(startLine: start.line,
1132+
startUTF16Index: start.utf16index,
1133+
endLine: end.line,
1134+
endUTF16Index: end.utf16index,
1135+
kind: kind)
10621136
}
1137+
ranges.insert(range)
1138+
return .visitChildren
10631139
}
1064-
1065-
req.reply(ranges.sorted())
10661140
}
10671141

1068-
// FIXME: cancellation
1069-
_ = handle
1070-
}
1071-
}
1072-
1073-
func addFoldingRange(offset: Int, length: Int, kind: FoldingRangeKind? = nil, in snapshot: DocumentSnapshot, toSet ranges: inout Set<FoldingRange>) {
1074-
guard let start: Position = snapshot.positionOf(utf8Offset: offset),
1075-
let end: Position = snapshot.positionOf(utf8Offset: offset + length) else {
1076-
log("folding range failed to retrieve position of \(snapshot.document.uri): \(offset)-\(offset + length)", level: .warning)
1077-
return
1078-
}
1079-
let capabilities = clientCapabilities.textDocument?.foldingRange
1080-
let range: FoldingRange
1081-
// If the client only supports folding full lines, ignore the end character's line.
1082-
if capabilities?.lineFoldingOnly == true {
1083-
let lastLineToFold = end.line - 1
1084-
if lastLineToFold <= start.line {
1142+
let capabilities = self.clientCapabilities.textDocument?.foldingRange
1143+
// If the limit is less than one, do nothing.
1144+
if let limit = capabilities?.rangeLimit, limit <= 0 {
1145+
req.reply([])
10851146
return
1086-
} else {
1087-
range = FoldingRange(startLine: start.line,
1088-
startUTF16Index: nil,
1089-
endLine: lastLineToFold,
1090-
endUTF16Index: nil,
1091-
kind: kind)
10921147
}
1093-
} else {
1094-
range = FoldingRange(startLine: start.line,
1095-
startUTF16Index: start.utf16index,
1096-
endLine: end.line,
1097-
endUTF16Index: end.utf16index,
1098-
kind: kind)
1148+
1149+
let rangeFinder = FoldingRangeFinder(
1150+
snapshot: snapshot,
1151+
rangeLimit: capabilities?.rangeLimit,
1152+
lineFoldingOnly: capabilities?.lineFoldingOnly ?? false)
1153+
rangeFinder.walk(sourceFile)
1154+
let ranges = rangeFinder.finalize()
1155+
1156+
req.reply(ranges.sorted())
10991157
}
1100-
ranges.insert(range)
11011158
}
11021159

11031160
public func codeAction(_ req: Request<CodeActionRequest>) {

Tests/SourceKitLSPTests/FoldingRangeTests.swift

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -39,18 +39,17 @@ final class FoldingRangeTests: XCTestCase {
3939
let ranges = try withExtendedLifetime(ws) { try ws.sk.sendSync(request) }
4040

4141
XCTAssertEqual(ranges, [
42-
FoldingRange(startLine: 0, startUTF16Index: 0, endLine: 2, endUTF16Index: 0, kind: .comment),
42+
FoldingRange(startLine: 0, startUTF16Index: 0, endLine: 1, endUTF16Index: 18, kind: .comment),
4343
FoldingRange(startLine: 3, startUTF16Index: 0, endLine: 13, endUTF16Index: 2, kind: .comment),
4444
FoldingRange(startLine: 14, startUTF16Index: 10, endLine: 27, endUTF16Index: 0, kind: nil),
45-
FoldingRange(startLine: 15, startUTF16Index: 2, endLine: 16, endUTF16Index: 0, kind: .comment),
46-
FoldingRange(startLine: 16, startUTF16Index: 2, endLine: 17, endUTF16Index: 0, kind: .comment),
45+
FoldingRange(startLine: 15, startUTF16Index: 2, endLine: 17, endUTF16Index: 2, kind: .comment),
4746
FoldingRange(startLine: 17, startUTF16Index: 2, endLine: 19, endUTF16Index: 4, kind: .comment),
4847
FoldingRange(startLine: 22, startUTF16Index: 21, endLine: 25, endUTF16Index: 2, kind: nil),
49-
FoldingRange(startLine: 23, startUTF16Index: 22, endLine: 23, endUTF16Index: 30, kind: nil),
48+
FoldingRange(startLine: 23, startUTF16Index: 23, endLine: 23, endUTF16Index: 30, kind: nil),
5049
FoldingRange(startLine: 26, startUTF16Index: 2, endLine: 26, endUTF16Index: 10, kind: .comment),
51-
FoldingRange(startLine: 29, startUTF16Index: 0, endLine: 32, endUTF16Index: 0, kind: .comment),
52-
FoldingRange(startLine: 33, startUTF16Index: 0, endLine: 36, endUTF16Index: 0, kind: .comment),
53-
FoldingRange(startLine: 37, startUTF16Index: 0, endLine: 38, endUTF16Index: 0, kind: .comment),
50+
FoldingRange(startLine: 29, startUTF16Index: 0, endLine: 31, endUTF16Index: 2, kind: .comment),
51+
FoldingRange(startLine: 33, startUTF16Index: 0, endLine: 35, endUTF16Index: 2, kind: .comment),
52+
FoldingRange(startLine: 37, startUTF16Index: 0, endLine: 37, endUTF16Index: 32, kind: .comment),
5453
FoldingRange(startLine: 39, startUTF16Index: 0, endLine: 39, endUTF16Index: 11, kind: .comment),
5554
])
5655
}
@@ -66,10 +65,11 @@ final class FoldingRangeTests: XCTestCase {
6665

6766
XCTAssertEqual(ranges, [
6867
FoldingRange(startLine: 0, endLine: 1, kind: .comment),
69-
FoldingRange(startLine: 3, endLine: 12, kind: .comment),
70-
FoldingRange(startLine: 14, endLine: 26, kind: nil),
71-
FoldingRange(startLine: 17, endLine: 18, kind: .comment),
72-
FoldingRange(startLine: 22, endLine: 24, kind: nil),
68+
FoldingRange(startLine: 3, endLine: 13, kind: .comment),
69+
FoldingRange(startLine: 14, endLine: 27, kind: nil),
70+
FoldingRange(startLine: 15, endLine: 17, kind: .comment),
71+
FoldingRange(startLine: 17, endLine: 19, kind: .comment),
72+
FoldingRange(startLine: 22, endLine: 25, kind: nil),
7373
FoldingRange(startLine: 29, endLine: 31, kind: .comment),
7474
FoldingRange(startLine: 33, endLine: 35, kind: .comment),
7575
])
@@ -90,8 +90,8 @@ final class FoldingRangeTests: XCTestCase {
9090
try performTest(withRangeLimit: -100, expecting: 0)
9191
try performTest(withRangeLimit: 0, expecting: 0)
9292
try performTest(withRangeLimit: 4, expecting: 4)
93-
try performTest(withRangeLimit: 5000, expecting: 13)
94-
try performTest(withRangeLimit: nil, expecting: 13)
93+
try performTest(withRangeLimit: 5000, expecting: 12)
94+
try performTest(withRangeLimit: nil, expecting: 12)
9595
}
9696

9797
func testNoRanges() throws {

0 commit comments

Comments
 (0)