Skip to content

Commit 1a6b818

Browse files
matthewbastienmatthewbastien
committed
address review comments
1 parent 433bcac commit 1a6b818

File tree

4 files changed

+94
-57
lines changed

4 files changed

+94
-57
lines changed

‎Sources/SourceKitLSP/CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ target_sources(SourceKitLSP PRIVATE
5252
Swift/GeneratedInterfaceManager.swift
5353
Swift/MacroExpansion.swift
5454
Swift/MacroExpansionReferenceDocumentURLData.swift
55+
Swift/OnDiskDocumentManager.swift
5556
Swift/OpenInterface.swift
5657
Swift/RefactoringEdit.swift
5758
Swift/RefactoringResponse.swift
@@ -65,7 +66,6 @@ target_sources(SourceKitLSP PRIVATE
6566
Swift/SwiftCommand.swift
6667
Swift/SwiftLanguageService.swift
6768
Swift/SwiftTestingScanner.swift
68-
Swift/SymbolGraphCache.swift
6969
Swift/SymbolInfo.swift
7070
Swift/SyntacticSwiftXCTestScanner.swift
7171
Swift/SyntacticTestIndex.swift

‎Sources/SourceKitLSP/Documentation/DoccDocumentationHandler.swift

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -58,25 +58,26 @@ extension DocumentationLanguageService {
5858
guard let index = workspace.index(checkedFor: .deletedFiles) else {
5959
throw ResponseError.requestFailed(doccDocumentationError: .indexNotAvailable)
6060
}
61-
let symbolGraphCache = SymbolGraphCache(sourceKitLSPServer: sourceKitLSPServer)
62-
guard let symbolLink = DocCSymbolLink(linkString: symbolName),
63-
let symbolOccurrence = try await index.primaryDefinitionOrDeclarationOccurrence(
64-
ofDocCSymbolLink: symbolLink,
65-
fetchSymbolGraph: symbolGraphCache.fetchSymbolGraph(at:)
61+
return try await sourceKitLSPServer.withOnDiskDocumentManager { onDiskDocumentManager in
62+
guard let symbolLink = DocCSymbolLink(linkString: symbolName),
63+
let symbolOccurrence = try await index.primaryDefinitionOrDeclarationOccurrence(
64+
ofDocCSymbolLink: symbolLink,
65+
fetchSymbolGraph: onDiskDocumentManager.fetchSymbolGraph(at:)
66+
)
67+
else {
68+
throw ResponseError.requestFailed(doccDocumentationError: .symbolNotFound(symbolName))
69+
}
70+
guard let symbolGraph = try await onDiskDocumentManager.fetchSymbolGraph(at: symbolOccurrence.location) else {
71+
throw ResponseError.internalError("Unable to retrieve symbol graph for \(symbolOccurrence.symbol.name)")
72+
}
73+
return try await documentationManager.renderDocCDocumentation(
74+
symbolUSR: symbolOccurrence.symbol.usr,
75+
symbolGraph: symbolGraph,
76+
markupFile: snapshot.text,
77+
moduleName: moduleName,
78+
catalogURL: catalogURL
6679
)
67-
else {
68-
throw ResponseError.requestFailed(doccDocumentationError: .symbolNotFound(symbolName))
6980
}
70-
guard let symbolGraph = try await symbolGraphCache.fetchSymbolGraph(at: symbolOccurrence.location) else {
71-
throw ResponseError.internalError("Unable to retrieve symbol graph for \(symbolOccurrence.symbol.name)")
72-
}
73-
return try await documentationManager.renderDocCDocumentation(
74-
symbolUSR: symbolOccurrence.symbol.usr,
75-
symbolGraph: symbolGraph,
76-
markupFile: snapshot.text,
77-
moduleName: moduleName,
78-
catalogURL: catalogURL
79-
)
8081
}
8182
// This is a page representing the module itself.
8283
// Create a dummy symbol graph and tell SwiftDocC to convert the module name.

‎Sources/SourceKitLSP/Swift/DoccDocumentation.swift

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -69,15 +69,16 @@ extension SwiftLanguageService {
6969
throw ResponseError.internalError("Unable to retrieve symbol graph for the document")
7070
}
7171
// Locate the documentation extension and include it in the request if one exists
72-
let markupExtensionFile = await orLog("Finding markup extension file for symbol \(symbolUSR)") {
73-
let symbolGraphCache = SymbolGraphCache(sourceKitLSPServer: sourceKitLSPServer)
74-
return try await findMarkupExtensionFile(
75-
workspace: workspace,
76-
documentationManager: documentationManager,
77-
catalogURL: catalogURL,
78-
for: symbolUSR,
79-
fetchSymbolGraph: symbolGraphCache.fetchSymbolGraph(at:)
80-
)
72+
let markupExtensionFile = await sourceKitLSPServer.withOnDiskDocumentManager { onDiskDocumentManager in
73+
await orLog("Finding markup extension file for symbol \(symbolUSR)") {
74+
try await findMarkupExtensionFile(
75+
workspace: workspace,
76+
documentationManager: documentationManager,
77+
catalogURL: catalogURL,
78+
for: symbolUSR,
79+
fetchSymbolGraph: onDiskDocumentManager.fetchSymbolGraph(at:)
80+
)
81+
}
8182
}
8283
return try await documentationManager.renderDocCDocumentation(
8384
symbolUSR: symbolUSR,

‎Sources/SourceKitLSP/Swift/SymbolGraphCache.swift renamed to ‎Sources/SourceKitLSP/Swift/OnDiskDocumentManager.swift

Lines changed: 65 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,12 @@ import SKLogging
1818
import SKUtilities
1919
import SwiftExtensions
2020

21-
/// A cache of symbol graphs and their associated snapshots opened in sourcekitd. Any opened documents will be
22-
/// closed when the cache is de-initialized.
21+
/// Caches document snapshots from disk opened in sourcekitd.
2322
///
2423
/// Used by `textDocument/doccDocumentation` requests to retrieve symbol graphs for files that are not currently
2524
/// open in the editor. This allows for retrieving multiple symbol graphs from the same file without having
2625
/// to re-open and parse the syntax tree every time.
27-
actor SymbolGraphCache: Sendable {
26+
actor OnDiskDocumentManager: Sendable {
2827
private weak var sourceKitLSPServer: SourceKitLSPServer?
2928
private var openSnapshots: [DocumentURI: (snapshot: DocumentSnapshot, patchedCompileCommand: SwiftCompileCommand?)]
3029

@@ -33,20 +32,14 @@ actor SymbolGraphCache: Sendable {
3332
self.openSnapshots = [:]
3433
}
3534

36-
/// Open a unique dummy document in sourcekitd that has the contents of the file on disk for uri, but an arbitrary
37-
/// URI which doesn't exist on disk. Return the symbol graph from sourcekitd.
35+
/// Retrieve the symbol graph at a particular ``SymbolLocation``.
3836
///
39-
/// The document will be retained until ``DocCSymbolGraphCache`` is de-initialized. This will avoid parsing the same
40-
/// document multiple times if more than one symbol needs to be looked up.
37+
/// A unique dummy document will be opened in sourcekitd to fulfill the request.
4138
///
42-
/// - Parameter symbolLocation: The location of a symbol to find the symbol graph for.
43-
/// - Returns: The symbol graph for this location, if any.
39+
/// - Parameter symbolLocation: The location of the symbol to fetch the symbol graph for.
4440
func fetchSymbolGraph(at symbolLocation: SymbolLocation) async throws -> String? {
41+
let (snapshot, patchedCompileCommand) = try await openDocumentSnapshot(at: symbolLocation)
4542
let swiftLanguageService = try await swiftLanguageService(for: symbolLocation.documentUri)
46-
let (snapshot, patchedCompileCommand) = try await swiftLanguageService.openSnapshotFromDiskOpenedInSourcekitd(
47-
uri: symbolLocation.documentUri,
48-
fallbackSettingsAfterTimeout: false
49-
)
5043
return try await swiftLanguageService.cursorInfo(
5144
snapshot,
5245
compileCommand: patchedCompileCommand,
@@ -55,6 +48,52 @@ actor SymbolGraphCache: Sendable {
5548
).symbolGraph
5649
}
5750

51+
/// Open a unique dummy document in sourcekitd that has the contents of the file on disk for uri, but an arbitrary
52+
/// URI which doesn't exist on disk.
53+
///
54+
/// The document will be retained until ``closeAllDocuments()`` is called. This will avoid parsing the same
55+
/// document multiple times if more than one symbol needs to be looked up.
56+
///
57+
/// - Parameter symbolLocation: The location of the document that will be opened.
58+
func openDocumentSnapshot(
59+
at symbolLocation: SymbolLocation
60+
) async throws -> (snapshot: DocumentSnapshot, patchedCompileCommand: SwiftCompileCommand?) {
61+
try await openDocumentSnapshot(for: symbolLocation.documentUri)
62+
}
63+
64+
/// Open a unique dummy document in sourcekitd that has the contents of the file on disk for uri, but an arbitrary
65+
/// URI which doesn't exist on disk.
66+
///
67+
/// The document will be retained until ``closeAllDocuments()`` is called. This will avoid parsing the same
68+
/// document multiple times if more than one symbol needs to be looked up.
69+
///
70+
/// - Parameter uri: The ``DocumentURI`` that will be opened.
71+
func openDocumentSnapshot(
72+
for uri: DocumentURI
73+
) async throws -> (snapshot: DocumentSnapshot, patchedCompileCommand: SwiftCompileCommand?) {
74+
guard let cachedSnapshot = openSnapshots[uri] else {
75+
let languageService = try await swiftLanguageService(for: uri)
76+
let snapshot = try await languageService.openSnapshotFromDiskOpenedInSourcekitd(
77+
uri: uri,
78+
fallbackSettingsAfterTimeout: false
79+
)
80+
openSnapshots[uri] = snapshot
81+
return snapshot
82+
}
83+
return cachedSnapshot
84+
}
85+
86+
/// Closes all document snapshots that were opened by this ``OnDiskDocumentManager``.
87+
func closeAllDocuments() async {
88+
for (snapshot, _) in openSnapshots.values {
89+
await orLog("Close helper document \(snapshot.uri.forLogging)") {
90+
let languageService = try await swiftLanguageService(for: snapshot.uri)
91+
await languageService.closeSnapshotFromDiskOpenedInSourcekitd(snapshot: snapshot)
92+
}
93+
}
94+
openSnapshots = [:]
95+
}
96+
5897
private func swiftLanguageService(for uri: DocumentURI) async throws -> SwiftLanguageService {
5998
guard let sourceKitLSPServer else {
6099
throw ResponseError.internalError("SourceKit-LSP is shutting down")
@@ -67,24 +106,20 @@ actor SymbolGraphCache: Sendable {
67106
}
68107
return swiftLanguageService
69108
}
109+
}
70110

71-
deinit {
72-
guard let sourceKitLSPServer else {
73-
return
74-
}
75-
76-
let documentsToClose = openSnapshots.values
77-
Task {
78-
for (snapshot, _) in documentsToClose {
79-
guard let workspace = await sourceKitLSPServer.workspaceForDocument(uri: snapshot.uri),
80-
let languageService = await sourceKitLSPServer.languageService(for: snapshot.uri, .swift, in: workspace),
81-
let swiftLanguageService = languageService as? SwiftLanguageService
82-
else {
83-
logger.log("Unable to find SwiftLanguageService to close helper document \(snapshot.uri.forLogging)")
84-
return
85-
}
86-
await swiftLanguageService.closeSnapshotFromDiskOpenedInSourcekitd(snapshot: snapshot)
87-
}
111+
extension SourceKitLSPServer {
112+
nonisolated(nonsending) func withOnDiskDocumentManager<T>(
113+
_ body: (OnDiskDocumentManager) async throws -> T
114+
) async rethrows -> T {
115+
let onDiskDocumentManager = OnDiskDocumentManager(sourceKitLSPServer: self)
116+
do {
117+
let result = try await body(onDiskDocumentManager)
118+
await onDiskDocumentManager.closeAllDocuments()
119+
return result
120+
} catch {
121+
await onDiskDocumentManager.closeAllDocuments()
122+
throw error
88123
}
89124
}
90125
}

0 commit comments

Comments
 (0)