Skip to content

Commit e783f14

Browse files
joehsiehjoehsieh
committed
Move the responsibility of sending requests to get diagnostics from SwiftLanguageServer to DiagnosticReportManager
1 parent 303ce16 commit e783f14

File tree

2 files changed

+83
-88
lines changed

2 files changed

+83
-88
lines changed

‎Sources/SourceKitLSP/Swift/DiagnosticReportManager.swift

Lines changed: 80 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -12,18 +12,18 @@
1212

1313
import LSPLogging
1414
import LanguageServerProtocol
15+
import SourceKitD
16+
import SwiftParserDiagnostics
1517

16-
protocol DiagnosticReportManagerDelegate: AnyObject {
17-
/// Notify the delegate that compute a report with a given snapshot
18-
func reportWithSnapshotReceived(_ snapshot: DocumentSnapshot, compilerArgs: [String]) async throws
19-
-> RelatedFullDocumentDiagnosticReport
18+
actor DiagnosticReportManager {
19+
private let sourcekitd: SourceKitD
20+
private let syntaxTreeManager: SyntaxTreeManager
21+
private let documentManager: DocumentManager
22+
private let clientHasDiagnosticsCodeDescriptionSupport: Bool
2023

21-
/// Notify the delegate that compute a report with a given snapshot for fallback
22-
func fallbackReportWithSnapshotReceived(_ snapshot: DocumentSnapshot) async throws
23-
-> RelatedFullDocumentDiagnosticReport
24-
}
24+
private nonisolated var keys: sourcekitd_keys { return sourcekitd.keys }
25+
private nonisolated var requests: sourcekitd_requests { return sourcekitd.requests }
2526

26-
actor DiagnosticReportManager {
2727
/// The cache that stores reports for snapshot ids
2828
///
2929
/// Conceptually, this is a dictionary. To prevent excessive memory usage we
@@ -40,19 +40,21 @@ actor DiagnosticReportManager {
4040
/// - Note: This has been chosen without scientific measurements.
4141
private let cacheSize = 5
4242

43-
/// Delegate to handle any DiagnosticReportManager events.
44-
private weak var delegate: DiagnosticReportManagerDelegate?
45-
46-
func setDelegate(_ delegate: DiagnosticReportManagerDelegate?) async {
47-
self.delegate = delegate
43+
init(
44+
sourcekitd: SourceKitD,
45+
syntaxTreeManager: SyntaxTreeManager,
46+
documentManager: DocumentManager,
47+
clientHasDiagnosticsCodeDescriptionSupport: Bool
48+
) {
49+
self.sourcekitd = sourcekitd
50+
self.syntaxTreeManager = syntaxTreeManager
51+
self.documentManager = documentManager
52+
self.clientHasDiagnosticsCodeDescriptionSupport = clientHasDiagnosticsCodeDescriptionSupport
4853
}
4954

5055
func diagnosticReport(for snapshot: DocumentSnapshot, buildSettings: SwiftCompileCommand?, useCache: Bool = false)
5156
async throws -> RelatedFullDocumentDiagnosticReport
5257
{
53-
guard let delegate = self.delegate else {
54-
throw ResponseError.unknown("DiagnosticReportManagerDelegate should not be nil")
55-
}
5658
if useCache, let report = report(for: snapshot.id) {
5759
return report
5860
}
@@ -64,26 +66,83 @@ actor DiagnosticReportManager {
6466
// sourcekitd won't be able to give us accurate semantic diagnostics.
6567
// Fall back to providing syntactic diagnostics from the built-in
6668
// swift-syntax. That's the best we can do for now.
67-
let report = try await delegate.fallbackReportWithSnapshotReceived(snapshot)
69+
let report = try await fallbackReport(with: snapshot)
6870
setReport(for: snapshot.id, report: report)
6971
return report
7072
}
7173

72-
let report = try await delegate.reportWithSnapshotReceived(snapshot, compilerArgs: buildSettings.compilerArgs)
74+
let report = try await report(with: snapshot, compilerArgs: buildSettings.compilerArgs)
7375
setReport(for: snapshot.id, report: report)
7476
return report
7577
}
78+
}
79+
80+
private extension DiagnosticReportManager {
81+
func report(with snapshot: DocumentSnapshot, compilerArgs: [String]) async throws
82+
-> LanguageServerProtocol.RelatedFullDocumentDiagnosticReport
83+
{
84+
try Task.checkCancellation()
85+
86+
let keys = self.keys
87+
88+
let skreq = SKDRequestDictionary(sourcekitd: self.sourcekitd)
89+
skreq[keys.request] = requests.diagnostics
90+
skreq[keys.sourcefile] = snapshot.uri.pseudoPath
91+
92+
// FIXME: SourceKit should probably cache this for us.
93+
skreq[keys.compilerargs] = compilerArgs
94+
95+
let dict = try await self.sourcekitd.send(skreq, fileContents: snapshot.text)
96+
97+
try Task.checkCancellation()
98+
guard (try? documentManager.latestSnapshot(snapshot.uri).id) == snapshot.id else {
99+
// Check that the document wasn't modified while we were getting diagnostics. This could happen because we are
100+
// calling `fullDocumentDiagnosticReport` from `publishDiagnosticsIfNeeded` outside of `messageHandlingQueue`
101+
// and thus a concurrent edit is possible while we are waiting for the sourcekitd request to return a result.
102+
throw ResponseError.unknown("Document was modified while loading document")
103+
}
104+
105+
let supportsCodeDescription = self.clientHasDiagnosticsCodeDescriptionSupport
106+
var diagnostics: [Diagnostic] = []
107+
dict[keys.diagnostics]?.forEach { _, diag in
108+
if let diag = Diagnostic(diag, in: snapshot, useEducationalNoteAsCode: supportsCodeDescription) {
109+
diagnostics.append(diag)
110+
}
111+
return true
112+
}
113+
114+
return RelatedFullDocumentDiagnosticReport(items: diagnostics)
115+
}
116+
117+
func fallbackReport(with snapshot: DocumentSnapshot) async throws
118+
-> LanguageServerProtocol.RelatedFullDocumentDiagnosticReport
119+
{
120+
// If we don't have build settings or we only have fallback build settings,
121+
// sourcekitd won't be able to give us accurate semantic diagnostics.
122+
// Fall back to providing syntactic diagnostics from the built-in
123+
// swift-syntax. That's the best we can do for now.
124+
let syntaxTree = await syntaxTreeManager.syntaxTree(for: snapshot)
125+
let swiftSyntaxDiagnostics = ParseDiagnosticsGenerator.diagnostics(for: syntaxTree)
126+
let diagnostics = swiftSyntaxDiagnostics.compactMap { (diag) -> Diagnostic? in
127+
if diag.diagnosticID == StaticTokenError.editorPlaceholder.diagnosticID {
128+
// Ignore errors about editor placeholders in the source file, similar to how sourcekitd ignores them.
129+
return nil
130+
}
131+
return Diagnostic(diag, in: snapshot)
132+
}
133+
return RelatedFullDocumentDiagnosticReport(items: diagnostics)
134+
}
76135

77136
/// The report for the given document snapshot.
78-
private func report(for snapshotID: DocumentSnapshot.ID) -> RelatedFullDocumentDiagnosticReport? {
137+
func report(for snapshotID: DocumentSnapshot.ID) -> RelatedFullDocumentDiagnosticReport? {
79138
return reportCache.first(where: { $0.snapshotID == snapshotID })?.report
80139
}
81140

82141
/// Set the report for the given document snapshot.
83142
///
84143
/// If we are already storing `cacheSize` many reports, the oldest one
85144
/// will get discarded.
86-
private func setReport(for snapshotID: DocumentSnapshot.ID, report: RelatedFullDocumentDiagnosticReport) {
145+
func setReport(for snapshotID: DocumentSnapshot.ID, report: RelatedFullDocumentDiagnosticReport) {
87146
reportCache.insert((snapshotID, report), at: 0)
88147

89148
// Remove any reports for old versions of this document.

‎Sources/SourceKitLSP/Swift/SwiftLanguageServer.swift

Lines changed: 3 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ public actor SwiftLanguageServer: ToolchainLanguageServer {
159159
self.state = .connected
160160
self.generatedInterfacesPath = options.generatedInterfacesPath.asURL
161161
try FileManager.default.createDirectory(at: generatedInterfacesPath, withIntermediateDirectories: true)
162-
self.diagnosticReportManager = DiagnosticReportManager()
162+
self.diagnosticReportManager = DiagnosticReportManager(sourcekitd: self.sourcekitd, syntaxTreeManager: syntaxTreeManager, documentManager: documentManager, clientHasDiagnosticsCodeDescriptionSupport: capabilityRegistry.clientHasDiagnosticsCodeDescriptionSupport)
163163
}
164164

165165
/// - Important: For testing only
@@ -199,8 +199,7 @@ public actor SwiftLanguageServer: ToolchainLanguageServer {
199199

200200
extension SwiftLanguageServer {
201201

202-
public func initialize(_ initialize: InitializeRequest) async throws -> InitializeResult {
203-
await diagnosticReportManager.setDelegate(self)
202+
public func initialize(_ initialize: InitializeRequest) throws -> InitializeResult {
204203
sourcekitd.addNotificationHandler(self)
205204

206205
return InitializeResult(
@@ -1054,21 +1053,6 @@ extension SwiftLanguageServer {
10541053
return Array(hints)
10551054
}
10561055

1057-
public func syntacticDiagnosticFromBuiltInSwiftSyntax(
1058-
for snapshot: DocumentSnapshot
1059-
) async throws -> RelatedFullDocumentDiagnosticReport {
1060-
let syntaxTree = await syntaxTreeManager.syntaxTree(for: snapshot)
1061-
let swiftSyntaxDiagnostics = ParseDiagnosticsGenerator.diagnostics(for: syntaxTree)
1062-
let diagnostics = swiftSyntaxDiagnostics.compactMap { (diag) -> Diagnostic? in
1063-
if diag.diagnosticID == StaticTokenError.editorPlaceholder.diagnosticID {
1064-
// Ignore errors about editor placeholders in the source file, similar to how sourcekitd ignores them.
1065-
return nil
1066-
}
1067-
return Diagnostic(diag, in: snapshot)
1068-
}
1069-
return RelatedFullDocumentDiagnosticReport(items: diagnostics)
1070-
}
1071-
10721056
public func documentDiagnostic(_ req: DocumentDiagnosticsRequest) async throws -> DocumentDiagnosticReport {
10731057
let snapshot = try documentManager.latestSnapshot(req.textDocument.uri)
10741058
let buildSettings = await self.buildSettings(for: req.textDocument.uri)
@@ -1297,52 +1281,4 @@ extension sourcekitd_uid_t {
12971281
return nil
12981282
}
12991283
}
1300-
}
1301-
1302-
extension SwiftLanguageServer: DiagnosticReportManagerDelegate {
1303-
func reportWithSnapshotReceived(_ snapshot: DocumentSnapshot, compilerArgs: [String]) async throws
1304-
-> LanguageServerProtocol.RelatedFullDocumentDiagnosticReport
1305-
{
1306-
try Task.checkCancellation()
1307-
1308-
let keys = self.keys
1309-
1310-
let skreq = SKDRequestDictionary(sourcekitd: self.sourcekitd)
1311-
skreq[keys.request] = requests.diagnostics
1312-
skreq[keys.sourcefile] = snapshot.uri.pseudoPath
1313-
1314-
// FIXME: SourceKit should probably cache this for us.
1315-
skreq[keys.compilerargs] = compilerArgs
1316-
1317-
let dict = try await self.sourcekitd.send(skreq, fileContents: snapshot.text)
1318-
1319-
try Task.checkCancellation()
1320-
guard (try? documentManager.latestSnapshot(snapshot.uri).id) == snapshot.id else {
1321-
// Check that the document wasn't modified while we were getting diagnostics. This could happen because we are
1322-
// calling `fullDocumentDiagnosticReport` from `publishDiagnosticsIfNeeded` outside of `messageHandlingQueue`
1323-
// and thus a concurrent edit is possible while we are waiting for the sourcekitd request to return a result.
1324-
throw ResponseError.unknown("Document was modified while loading document")
1325-
}
1326-
1327-
let supportsCodeDescription = capabilityRegistry.clientHasDiagnosticsCodeDescriptionSupport
1328-
var diagnostics: [Diagnostic] = []
1329-
dict[keys.diagnostics]?.forEach { _, diag in
1330-
if let diag = Diagnostic(diag, in: snapshot, useEducationalNoteAsCode: supportsCodeDescription) {
1331-
diagnostics.append(diag)
1332-
}
1333-
return true
1334-
}
1335-
1336-
return RelatedFullDocumentDiagnosticReport(items: diagnostics)
1337-
}
1338-
1339-
func fallbackReportWithSnapshotReceived(_ snapshot: DocumentSnapshot) async throws
1340-
-> LanguageServerProtocol.RelatedFullDocumentDiagnosticReport
1341-
{
1342-
// If we don't have build settings or we only have fallback build settings,
1343-
// sourcekitd won't be able to give us accurate semantic diagnostics.
1344-
// Fall back to providing syntactic diagnostics from the built-in
1345-
// swift-syntax. That's the best we can do for now.
1346-
return try await syntacticDiagnosticFromBuiltInSwiftSyntax(for: snapshot)
1347-
}
1348-
}
1284+
}

0 commit comments

Comments
 (0)