Skip to content

Commit d4ffe93

Browse files
committed
Type-qualify accessors in call hierarchy
An accessor is a child of the corresponding variable, which is why we failed to compute the correct container for them. Look one `childOf` relation further for accessors to fix this. rdar://129452601
1 parent 77e66bd commit d4ffe93

File tree

2 files changed

+126
-63
lines changed

2 files changed

+126
-63
lines changed

Sources/SourceKitLSP/SourceKitLSPServer.swift

Lines changed: 80 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -1526,20 +1526,24 @@ extension SourceKitLSPServer {
15261526
)
15271527
}
15281528

1529-
/// Find all symbols in the workspace that include a string in their name.
1530-
/// - returns: An array of SymbolOccurrences that match the string.
1531-
func findWorkspaceSymbols(matching: String) throws -> [SymbolOccurrence] {
1529+
/// Handle a workspace/symbol request, returning the SymbolInformation.
1530+
/// - returns: An array with SymbolInformation for each matching symbol in the workspace.
1531+
func workspaceSymbols(_ req: WorkspaceSymbolsRequest) async throws -> [WorkspaceSymbolItem]? {
15321532
// Ignore short queries since they are:
15331533
// - noisy and slow, since they can match many symbols
15341534
// - normally unintentional, triggered when the user types slowly or if the editor doesn't
15351535
// debounce events while the user is typing
1536-
guard matching.count >= minWorkspaceSymbolPatternLength else {
1536+
guard req.query.count >= minWorkspaceSymbolPatternLength else {
15371537
return []
15381538
}
1539-
var symbolOccurrenceResults: [SymbolOccurrence] = []
1539+
var symbolsAndIndex: [(symbol: SymbolOccurrence, index: CheckedIndex)] = []
15401540
for workspace in workspaces {
1541-
workspace.index(checkedFor: .deletedFiles)?.forEachCanonicalSymbolOccurrence(
1542-
containing: matching,
1541+
guard let index = workspace.index(checkedFor: .deletedFiles) else {
1542+
continue
1543+
}
1544+
var symbolOccurrences: [SymbolOccurrence] = []
1545+
index.forEachCanonicalSymbolOccurrence(
1546+
containing: req.query,
15431547
anchorStart: false,
15441548
anchorEnd: false,
15451549
subsequence: true,
@@ -1551,19 +1555,36 @@ extension SourceKitLSPServer {
15511555
guard !symbol.location.isSystem && !symbol.roles.contains(.accessorOf) else {
15521556
return true
15531557
}
1554-
symbolOccurrenceResults.append(symbol)
1558+
symbolOccurrences.append(symbol)
15551559
return true
15561560
}
15571561
try Task.checkCancellation()
1562+
symbolsAndIndex += symbolOccurrences.map {
1563+
return ($0, index)
1564+
}
15581565
}
1559-
return symbolOccurrenceResults.sorted()
1560-
}
1566+
return symbolsAndIndex.sorted(by: { $0.symbol < $1.symbol }).map { symbolOccurrence, index in
1567+
let symbolPosition = Position(
1568+
line: symbolOccurrence.location.line - 1, // 1-based -> 0-based
1569+
// FIXME: we need to convert the utf8/utf16 column, which may require reading the file!
1570+
utf16index: symbolOccurrence.location.utf8Column - 1
1571+
)
15611572

1562-
/// Handle a workspace/symbol request, returning the SymbolInformation.
1563-
/// - returns: An array with SymbolInformation for each matching symbol in the workspace.
1564-
func workspaceSymbols(_ req: WorkspaceSymbolsRequest) async throws -> [WorkspaceSymbolItem]? {
1565-
let symbols = try findWorkspaceSymbols(matching: req.query).map(WorkspaceSymbolItem.init)
1566-
return symbols
1573+
let symbolLocation = Location(
1574+
uri: symbolOccurrence.location.documentUri,
1575+
range: Range(symbolPosition)
1576+
)
1577+
1578+
return WorkspaceSymbolItem.symbolInformation(
1579+
SymbolInformation(
1580+
name: symbolOccurrence.symbol.name,
1581+
kind: symbolOccurrence.symbol.kind.asLspSymbolKind(),
1582+
deprecated: nil,
1583+
location: symbolLocation,
1584+
containerName: index.containerName(of: symbolOccurrence)
1585+
)
1586+
)
1587+
}
15671588
}
15681589

15691590
/// Forwards a SymbolInfoRequest to the appropriate toolchain service for this document.
@@ -2083,7 +2104,7 @@ extension SourceKitLSPServer {
20832104
}
20842105
return self.indexToLSPCallHierarchyItem(
20852106
symbol: definition.symbol,
2086-
containerName: definition.containerName,
2107+
containerName: index.containerName(of: definition),
20872108
location: location
20882109
)
20892110
}.sorted(by: { Location(uri: $0.uri, range: $0.range) < Location(uri: $1.uri, range: $1.range) })
@@ -2159,6 +2180,12 @@ extension SourceKitLSPServer {
21592180
let definition = index.primaryDefinitionOrDeclarationOccurrence(ofUSR: caller.usr)
21602181
let definitionSymbolLocation = definition?.location
21612182
let definitionLocation = definitionSymbolLocation.flatMap(indexToLSPLocation2)
2183+
let containerName: String? =
2184+
if let definition {
2185+
index.containerName(of: definition)
2186+
} else {
2187+
nil
2188+
}
21622189

21632190
let locations = calls.compactMap { indexToLSPLocation2($0.location) }.sorted()
21642191
guard !locations.isEmpty else {
@@ -2168,7 +2195,7 @@ extension SourceKitLSPServer {
21682195
return CallHierarchyIncomingCall(
21692196
from: indexToLSPCallHierarchyItem2(
21702197
symbol: caller,
2171-
containerName: definition?.containerName,
2198+
containerName: containerName,
21722199
location: definitionLocation ?? locations.first!
21732200
),
21742201
fromRanges: locations.map(\.range)
@@ -2212,11 +2239,17 @@ extension SourceKitLSPServer {
22122239
let definition = index.primaryDefinitionOrDeclarationOccurrence(ofUSR: occurrence.symbol.usr)
22132240
let definitionSymbolLocation = definition?.location
22142241
let definitionLocation = definitionSymbolLocation.flatMap(indexToLSPLocation2)
2242+
let containerName: String? =
2243+
if let definition {
2244+
index.containerName(of: definition)
2245+
} else {
2246+
nil
2247+
}
22152248

22162249
return CallHierarchyOutgoingCall(
22172250
to: indexToLSPCallHierarchyItem2(
22182251
symbol: occurrence.symbol,
2219-
containerName: definition?.containerName,
2252+
containerName: containerName,
22202253
location: definitionLocation ?? location // Use occurrence location as fallback
22212254
),
22222255
fromRanges: [location.range]
@@ -2518,6 +2551,35 @@ fileprivate extension CheckedIndex {
25182551
}
25192552
return result
25202553
}
2554+
2555+
/// Get the name of the symbol that is a parent of this symbol, if one exists
2556+
func containerName(of symbol: SymbolOccurrence) -> String? {
2557+
// The container name of accessors is the container of the surrounding variable.
2558+
let accessorOf = symbol.relations.filter { $0.roles.contains(.accessorOf) }
2559+
if let primaryVariable = accessorOf.sorted().first {
2560+
if accessorOf.count > 1 {
2561+
logger.fault("Expected an occurrence to an accessor of at most one symbol, not multiple")
2562+
}
2563+
if let primaryVariable = primaryDefinitionOrDeclarationOccurrence(ofUSR: primaryVariable.symbol.usr) {
2564+
return containerName(of: primaryVariable)
2565+
}
2566+
}
2567+
2568+
let containers = symbol.relations.filter { $0.roles.contains(.childOf) }
2569+
if containers.count > 1 {
2570+
logger.fault("Expected an occurrence to a child of at most one symbol, not multiple")
2571+
}
2572+
return containers.filter {
2573+
switch $0.symbol.kind {
2574+
case .module, .namespace, .enum, .struct, .class, .protocol, .extension, .union:
2575+
return true
2576+
case .unknown, .namespaceAlias, .macro, .typealias, .function, .variable, .field, .enumConstant,
2577+
.instanceMethod, .classMethod, .staticMethod, .instanceProperty, .classProperty, .staticProperty, .constructor,
2578+
.destructor, .conversionFunction, .parameter, .using, .concept, .commentTag:
2579+
return false
2580+
}
2581+
}.sorted().first?.symbol.name
2582+
}
25212583
}
25222584

25232585
extension IndexSymbolKind {
@@ -2568,26 +2630,6 @@ extension IndexSymbolKind {
25682630
}
25692631
}
25702632

2571-
extension SymbolOccurrence {
2572-
/// Get the name of the symbol that is a parent of this symbol, if one exists
2573-
var containerName: String? {
2574-
let containers = relations.filter { $0.roles.contains(.childOf) }
2575-
if containers.count > 1 {
2576-
logger.fault("Expected an occurrence to a child of at most one symbol, not multiple")
2577-
}
2578-
return containers.filter {
2579-
switch $0.symbol.kind {
2580-
case .module, .namespace, .enum, .struct, .class, .protocol, .extension, .union:
2581-
return true
2582-
case .unknown, .namespaceAlias, .macro, .typealias, .function, .variable, .field, .enumConstant,
2583-
.instanceMethod, .classMethod, .staticMethod, .instanceProperty, .classProperty, .staticProperty, .constructor,
2584-
.destructor, .conversionFunction, .parameter, .using, .concept, .commentTag:
2585-
return false
2586-
}
2587-
}.sorted().first?.symbol.name
2588-
}
2589-
}
2590-
25912633
/// Simple struct for pending notifications/requests, including a cancellation handler.
25922634
/// For convenience the notifications/request handlers are type erased via wrapping.
25932635
fileprivate struct NotificationRequestOperation {
@@ -2633,31 +2675,6 @@ fileprivate func transitiveSubtypeClosure(ofUsrs usrs: [String], index: CheckedI
26332675
return result
26342676
}
26352677

2636-
extension WorkspaceSymbolItem {
2637-
init(_ symbolOccurrence: SymbolOccurrence) {
2638-
let symbolPosition = Position(
2639-
line: symbolOccurrence.location.line - 1, // 1-based -> 0-based
2640-
// FIXME: we need to convert the utf8/utf16 column, which may require reading the file!
2641-
utf16index: symbolOccurrence.location.utf8Column - 1
2642-
)
2643-
2644-
let symbolLocation = Location(
2645-
uri: symbolOccurrence.location.documentUri,
2646-
range: Range(symbolPosition)
2647-
)
2648-
2649-
self = .symbolInformation(
2650-
SymbolInformation(
2651-
name: symbolOccurrence.symbol.name,
2652-
kind: symbolOccurrence.symbol.kind.asLspSymbolKind(),
2653-
deprecated: nil,
2654-
location: symbolLocation,
2655-
containerName: symbolOccurrence.containerName
2656-
)
2657-
)
2658-
}
2659-
}
2660-
26612678
fileprivate extension RequestID {
26622679
/// Returns a numeric value for this request ID.
26632680
///

Tests/SourceKitLSPTests/CallHierarchyTests.swift

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -849,4 +849,50 @@ final class CallHierarchyTests: XCTestCase {
849849
]
850850
)
851851
}
852+
853+
func testIncomingCallHierarchyFromComputedMember() async throws {
854+
try await SkipUnless.indexOnlyHasContainedByRelationsToIndexedDecls()
855+
let project = try await IndexedSingleSwiftFileTestProject(
856+
"""
857+
struct Foo {
858+
func 1️⃣foo() {}
859+
860+
var testVar: Int {
861+
2️⃣get {
862+
let myVar = 3️⃣foo()
863+
return 2
864+
}
865+
}
866+
}
867+
"""
868+
)
869+
let prepare = try await project.testClient.send(
870+
CallHierarchyPrepareRequest(
871+
textDocument: TextDocumentIdentifier(project.fileURI),
872+
position: project.positions["1️⃣"]
873+
)
874+
)
875+
let initialItem = try XCTUnwrap(prepare?.only)
876+
let calls = try await project.testClient.send(CallHierarchyIncomingCallsRequest(item: initialItem))
877+
XCTAssertEqual(
878+
calls,
879+
[
880+
CallHierarchyIncomingCall(
881+
from: CallHierarchyItem(
882+
name: "Foo.getter:testVar",
883+
kind: .method,
884+
tags: nil,
885+
uri: project.fileURI,
886+
range: Range(project.positions["2️⃣"]),
887+
selectionRange: Range(project.positions["2️⃣"]),
888+
data: .dictionary([
889+
"usr": .string("s:4test3FooV0A3VarSivg"),
890+
"uri": .string(project.fileURI.stringValue),
891+
])
892+
),
893+
fromRanges: [Range(project.positions["3️⃣"])]
894+
)
895+
]
896+
)
897+
}
852898
}

0 commit comments

Comments
 (0)