Skip to content

Commit fef3cb8

Browse files
ahoppenahoppen
committed
Sort results returned by the index
rdar://123487052
1 parent 6087ce5 commit fef3cb8

File tree

6 files changed

+138
-27
lines changed

6 files changed

+138
-27
lines changed

‎Sources/LanguageServerProtocol/SupportTypes/Location.swift

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,16 @@
1313
/// Range within a particular document.
1414
///
1515
/// For a location where the document is implied, use `Position` or `Range<Position>`.
16-
public struct Location: ResponseType, Hashable, Codable, CustomDebugStringConvertible {
16+
public struct Location: ResponseType, Hashable, Codable, CustomDebugStringConvertible, Comparable {
17+
public static func < (lhs: Location, rhs: Location) -> Bool {
18+
if lhs.uri != rhs.uri {
19+
return lhs.uri.stringValue < rhs.uri.stringValue
20+
}
21+
if lhs.range.lowerBound != rhs.range.lowerBound {
22+
return lhs.range.lowerBound < rhs.range.lowerBound
23+
}
24+
return lhs.range.upperBound < rhs.range.upperBound
25+
}
1726

1827
public var uri: DocumentURI
1928

‎Sources/SourceKitLSP/SourceKitServer.swift

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1606,7 +1606,7 @@ extension SourceKitServer {
16061606

16071607
/// Find all symbols in the workspace that include a string in their name.
16081608
/// - returns: An array of SymbolOccurrences that match the string.
1609-
func findWorkspaceSymbols(matching: String) -> [SymbolOccurrence] {
1609+
func findWorkspaceSymbols(matching: String) throws -> [SymbolOccurrence] {
16101610
// Ignore short queries since they are:
16111611
// - noisy and slow, since they can match many symbols
16121612
// - normally unintentional, triggered when the user types slowly or if the editor doesn't
@@ -1623,25 +1623,24 @@ extension SourceKitServer {
16231623
subsequence: true,
16241624
ignoreCase: true
16251625
) { symbol in
1626+
if Task.isCancelled {
1627+
return false
1628+
}
16261629
guard !symbol.location.isSystem && !symbol.roles.contains(.accessorOf) else {
16271630
return true
16281631
}
16291632
symbolOccurrenceResults.append(symbol)
1630-
// FIXME: Once we have cancellation support, we should fetch all results and take the top
1631-
// `maxWorkspaceSymbolResults` symbols but bail if cancelled.
1632-
//
1633-
// Until then, take the first `maxWorkspaceSymbolResults` symbols to limit the impact of
1634-
// queries which match many symbols.
1635-
return symbolOccurrenceResults.count < maxWorkspaceSymbolResults
1633+
return true
16361634
}
1635+
try Task.checkCancellation()
16371636
}
1638-
return symbolOccurrenceResults
1637+
return symbolOccurrenceResults.sorted()
16391638
}
16401639

16411640
/// Handle a workspace/symbol request, returning the SymbolInformation.
16421641
/// - returns: An array with SymbolInformation for each matching symbol in the workspace.
16431642
func workspaceSymbols(_ req: WorkspaceSymbolsRequest) async throws -> [WorkspaceSymbolItem]? {
1644-
let symbols = findWorkspaceSymbols(matching: req.query).map(WorkspaceSymbolItem.init)
1643+
let symbols = try findWorkspaceSymbols(matching: req.query).map(WorkspaceSymbolItem.init)
16451644
return symbols
16461645
}
16471646

@@ -1869,6 +1868,7 @@ extension SourceKitServer {
18691868
}
18701869
return indexToLSPLocation(occurrence.location)
18711870
}
1871+
.sorted()
18721872
}
18731873

18741874
/// Returns the result of a `DefinitionRequest` by running a `SymbolInfoRequest`, inspecting
@@ -1972,7 +1972,7 @@ extension SourceKitServer {
19721972
occurrences = index.occurrences(relatedToUSR: usr, roles: .overrideOf)
19731973
}
19741974

1975-
return .locations(occurrences.compactMap { indexToLSPLocation($0.location) })
1975+
return .locations(occurrences.compactMap { indexToLSPLocation($0.location) }.sorted())
19761976
}
19771977

19781978
func references(
@@ -1996,7 +1996,7 @@ extension SourceKitServer {
19961996
if req.context.includeDeclaration {
19971997
roles.formUnion([.declaration, .definition])
19981998
}
1999-
return index.occurrences(ofUSR: usr, roles: roles).compactMap { indexToLSPLocation($0.location) }
1999+
return index.occurrences(ofUSR: usr, roles: roles).compactMap { indexToLSPLocation($0.location) }.sorted()
20002000
}
20012001

20022002
private func indexToLSPCallHierarchyItem(
@@ -2103,7 +2103,7 @@ extension SourceKitServer {
21032103
)
21042104
}
21052105
}
2106-
return calls
2106+
return calls.sorted(by: { $0.from.name < $1.from.name })
21072107
}
21082108

21092109
func outgoingCalls(_ req: CallHierarchyOutgoingCallsRequest) async throws -> [CallHierarchyOutgoingCall]? {
@@ -2133,7 +2133,7 @@ extension SourceKitServer {
21332133
fromRanges: [location.range]
21342134
)
21352135
}
2136-
return calls
2136+
return calls.sorted(by: { $0.to.name < $1.to.name })
21372137
}
21382138

21392139
private func indexToLSPTypeHierarchyItem(
@@ -2152,7 +2152,7 @@ extension SourceKitServer {
21522152
if conformances.isEmpty {
21532153
name = symbol.name
21542154
} else {
2155-
name = "\(symbol.name): \(conformances.map(\.symbol.name).joined(separator: ", "))"
2155+
name = "\(symbol.name): \(conformances.map(\.symbol.name).sorted().joined(separator: ", "))"
21562156
}
21572157
// Add the file name and line to the detail string
21582158
if let url = location.uri.fileURL,
@@ -2215,6 +2215,7 @@ extension SourceKitServer {
22152215
index: index
22162216
)
22172217
}
2218+
.sorted(by: { $0.name < $1.name })
22182219
}
22192220

22202221
/// Extracts our implementation-specific data about a type hierarchy
@@ -2273,7 +2274,7 @@ extension SourceKitServer {
22732274
index: index
22742275
)
22752276
}
2276-
return types
2277+
return types.sorted(by: { $0.name < $1.name })
22772278
}
22782279

22792280
func subtypes(_ req: TypeHierarchySubtypesRequest) async throws -> [TypeHierarchyItem]? {
@@ -2306,7 +2307,7 @@ extension SourceKitServer {
23062307
index: index
23072308
)
23082309
}
2309-
return types
2310+
return types.sorted { $0.name < $1.name }
23102311
}
23112312

23122313
func pollIndex(_ req: PollIndexRequest) async throws -> VoidResponse {

‎Sources/SourceKitLSP/TestDiscovery.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ extension SourceKitServer {
3939
return
4040
testSymbols
4141
.filter { $0.canBeTestDefinition }
42+
.sorted()
4243
.map(WorkspaceSymbolItem.init)
4344
}
4445

@@ -56,6 +57,7 @@ extension SourceKitServer {
5657
return
5758
testSymbols
5859
.filter { $0.canBeTestDefinition }
60+
.sorted()
5961
.map(WorkspaceSymbolItem.init)
6062
}
6163
}

‎Tests/SourceKitLSPTests/CallHierarchyTests.swift

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -121,14 +121,14 @@ final class CallHierarchyTests: XCTestCase {
121121
to: item("a()", .function, usr: aUsr, at: ws.positions["1️⃣"]),
122122
fromRanges: [Range(ws.positions["3️⃣"])]
123123
),
124-
CallHierarchyOutgoingCall(
125-
to: item("c()", .function, usr: cUsr, at: ws.positions["6️⃣"]),
126-
fromRanges: [Range(ws.positions["4️⃣"])]
127-
),
128124
CallHierarchyOutgoingCall(
129125
to: item("b(x:)", .function, usr: bUsr, at: ws.positions["2️⃣"]),
130126
fromRanges: [Range(ws.positions["5️⃣"])]
131127
),
128+
CallHierarchyOutgoingCall(
129+
to: item("c()", .function, usr: cUsr, at: ws.positions["6️⃣"]),
130+
fromRanges: [Range(ws.positions["4️⃣"])]
131+
),
132132
]
133133
)
134134
assertEqual(
@@ -138,14 +138,14 @@ final class CallHierarchyTests: XCTestCase {
138138
to: item("a()", .function, usr: aUsr, at: ws.positions["1️⃣"]),
139139
fromRanges: [Range(ws.positions["7️⃣"])]
140140
),
141-
CallHierarchyOutgoingCall(
142-
to: item("d()", .function, usr: dUsr, at: ws.positions["🔟"]),
143-
fromRanges: [Range(ws.positions["8️⃣"])]
144-
),
145141
CallHierarchyOutgoingCall(
146142
to: item("c()", .function, usr: cUsr, at: ws.positions["6️⃣"]),
147143
fromRanges: [Range(ws.positions["9️⃣"])]
148144
),
145+
CallHierarchyOutgoingCall(
146+
to: item("d()", .function, usr: dUsr, at: ws.positions["🔟"]),
147+
fromRanges: [Range(ws.positions["8️⃣"])]
148+
),
149149
]
150150
)
151151

‎Tests/SourceKitLSPTests/TypeHierarchyTests.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,8 @@ final class TypeHierarchyTests: XCTestCase {
6464
assertEqualIgnoringData(
6565
supertypes,
6666
[
67-
TypeHierarchyItem(name: "MyProtocol", kind: .interface, location: "1️⃣", in: ws),
6867
TypeHierarchyItem(name: "MyOtherProtocol", kind: .interface, location: "2️⃣", in: ws),
68+
TypeHierarchyItem(name: "MyProtocol", kind: .interface, location: "1️⃣", in: ws),
6969
]
7070
)
7171
}
@@ -107,8 +107,8 @@ final class TypeHierarchyTests: XCTestCase {
107107
subtypes,
108108
[
109109
TypeHierarchyItem(name: "MyClass", kind: .class, location: "2️⃣", in: ws),
110-
TypeHierarchyItem(name: "MyStruct", kind: .struct, location: "3️⃣", in: ws),
111110
TypeHierarchyItem(name: "MyEnum", kind: .enum, location: "4️⃣", in: ws),
111+
TypeHierarchyItem(name: "MyStruct", kind: .struct, location: "3️⃣", in: ws),
112112
]
113113
)
114114
}

‎Tests/SourceKitLSPTests/WorkspaceSymbolsTests.swift

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
13+
import LanguageServerProtocol
14+
import SKTestSupport
15+
import XCTest
16+
17+
class WorkspaceSymbolsTests: XCTestCase {
18+
func testWorkspaceSymbolsAcrossPackages() async throws {
19+
let ws = try await MultiFileTestWorkspace(
20+
files: [
21+
"packageA/Sources/PackageALib/PackageALib.swift": """
22+
public func 1️⃣afuncFromA() {}
23+
""",
24+
"packageA/Package.swift": """
25+
// swift-tools-version: 5.7
26+
27+
import PackageDescription
28+
29+
let package = Package(
30+
name: "PackageA",
31+
products: [
32+
.library(name: "PackageALib", targets: ["PackageALib"])
33+
],
34+
targets: [
35+
.target(name: "PackageALib"),
36+
]
37+
)
38+
""",
39+
"packageB/Sources/PackageBLib/PackageBLib.swift": """
40+
public func 2️⃣funcFromB() {}
41+
""",
42+
"packageB/Package.swift": """
43+
// swift-tools-version: 5.7
44+
45+
import PackageDescription
46+
47+
let package = Package(
48+
name: "PackageB",
49+
dependencies: [
50+
.package(path: "../packageA"),
51+
],
52+
targets: [
53+
.target(
54+
name: "PackageBLib",
55+
dependencies: [.product(name: "PackageALib", package: "PackageA")]
56+
),
57+
]
58+
)
59+
""",
60+
],
61+
workspaces: {
62+
return [WorkspaceFolder(uri: DocumentURI($0.appendingPathComponent("packageB")))]
63+
}
64+
)
65+
66+
try await SwiftPMTestWorkspace.build(at: ws.scratchDirectory.appendingPathComponent("packageB"))
67+
68+
_ = try await ws.testClient.send(PollIndexRequest())
69+
let response = try await ws.testClient.send(WorkspaceSymbolsRequest(query: "funcFrom"))
70+
71+
// Ideally, the item from the current package (PackageB) should be returned before the item from PackageA
72+
// https://github.com/apple/sourcekit-lsp/issues/1094
73+
XCTAssertEqual(
74+
response,
75+
[
76+
.symbolInformation(
77+
SymbolInformation(
78+
name: "afuncFromA()",
79+
kind: .function,
80+
location: Location(
81+
uri: try ws.uri(for: "PackageALib.swift"),
82+
range: Range(try ws.position(of: "1️⃣", in: "PackageALib.swift"))
83+
)
84+
)
85+
),
86+
.symbolInformation(
87+
SymbolInformation(
88+
name: "funcFromB()",
89+
kind: .function,
90+
location: Location(
91+
uri: try ws.uri(for: "PackageBLib.swift"),
92+
range: Range(try ws.position(of: "2️⃣", in: "PackageBLib.swift"))
93+
)
94+
)
95+
),
96+
]
97+
)
98+
}
99+
}

0 commit comments

Comments
 (0)