Skip to content

Commit 05dd102

Browse files
committed
Wait for target to be prepared before returning diagnostics
Diagnostics are usually not helpful if the file’s target hasn’t been prepared yet. We should await the target’s preparation before returning diagnostics. rdar://128645617
1 parent 5d75e14 commit 05dd102

File tree

8 files changed

+131
-35
lines changed

8 files changed

+131
-35
lines changed

Package.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,6 +367,7 @@ let package = Package(
367367
name: "SourceKitLSPTests",
368368
dependencies: [
369369
"BuildServerProtocol",
370+
"CAtomics",
370371
"LSPLogging",
371372
"LSPTestSupport",
372373
"LanguageServerProtocol",

Sources/SKTestSupport/MultiFileTestProject.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ public class MultiFileTestProject {
8282
workspaces: (URL) async throws -> [WorkspaceFolder] = { [WorkspaceFolder(uri: DocumentURI($0))] },
8383
capabilities: ClientCapabilities = ClientCapabilities(),
8484
serverOptions: SourceKitLSPServer.Options = .testDefault,
85+
enableBackgroundIndexing: Bool = false,
8586
usePullDiagnostics: Bool = true,
8687
preInitialization: ((TestSourceKitLSPClient) -> Void)? = nil,
8788
cleanUp: (() -> Void)? = nil,
@@ -117,6 +118,7 @@ public class MultiFileTestProject {
117118
serverOptions: serverOptions,
118119
capabilities: capabilities,
119120
usePullDiagnostics: usePullDiagnostics,
121+
enableBackgroundIndexing: enableBackgroundIndexing,
120122
workspaceFolders: workspaces(scratchDirectory),
121123
preInitialization: preInitialization,
122124
cleanUp: { [scratchDirectory] in

Sources/SKTestSupport/SwiftPMTestProject.swift

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,10 @@ public class SwiftPMTestProject: MultiFileTestProject {
4444
allowBuildFailure: Bool = false,
4545
capabilities: ClientCapabilities = ClientCapabilities(),
4646
serverOptions: SourceKitLSPServer.Options = .testDefault,
47+
enableBackgroundIndexing: Bool = false,
48+
usePullDiagnostics: Bool = true,
4749
pollIndex: Bool = true,
4850
preInitialization: ((TestSourceKitLSPClient) -> Void)? = nil,
49-
usePullDiagnostics: Bool = true,
5051
cleanUp: (() -> Void)? = nil,
5152
testName: String = #function
5253
) async throws {
@@ -71,6 +72,7 @@ public class SwiftPMTestProject: MultiFileTestProject {
7172
workspaces: workspaces,
7273
capabilities: capabilities,
7374
serverOptions: serverOptions,
75+
enableBackgroundIndexing: enableBackgroundIndexing,
7476
usePullDiagnostics: usePullDiagnostics,
7577
preInitialization: preInitialization,
7678
cleanUp: cleanUp,

Sources/SKTestSupport/TestSourceKitLSPClient.swift

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,8 @@ public final class TestSourceKitLSPClient: MessageHandler {
8888
/// `true` by default
8989
/// - initializationOptions: Initialization options to pass to the SourceKit-LSP server.
9090
/// - capabilities: The test client's capabilities.
91-
/// - usePullDiagnostics: Whether to use push diagnostics or use push-based diagnostics
91+
/// - usePullDiagnostics: Whether to use push diagnostics or use push-based diagnostics.
92+
/// - enableBackgroundIndexing: Whether background indexing should be enabled in the project.
9293
/// - workspaceFolders: Workspace folders to open.
9394
/// - preInitialization: A closure that is called after the test client is created but before SourceKit-LSP is
9495
/// initialized. This can be used to eg. register request handlers.
@@ -102,6 +103,7 @@ public final class TestSourceKitLSPClient: MessageHandler {
102103
initializationOptions: LSPAny? = nil,
103104
capabilities: ClientCapabilities = ClientCapabilities(),
104105
usePullDiagnostics: Bool = true,
106+
enableBackgroundIndexing: Bool = false,
105107
workspaceFolders: [WorkspaceFolder]? = nil,
106108
preInitialization: ((TestSourceKitLSPClient) -> Void)? = nil,
107109
cleanUp: @Sendable @escaping () -> Void = {}
@@ -115,6 +117,7 @@ public final class TestSourceKitLSPClient: MessageHandler {
115117
if let moduleCache {
116118
serverOptions.buildSetup.flags.swiftCompilerFlags += ["-module-cache-path", moduleCache.path]
117119
}
120+
serverOptions.indexOptions.enableBackgroundIndexing = enableBackgroundIndexing
118121

119122
var notificationYielder: AsyncStream<any NotificationType>.Continuation!
120123
self.notifications = AsyncStream { continuation in
@@ -155,8 +158,8 @@ public final class TestSourceKitLSPClient: MessageHandler {
155158
XCTAssertEqual(request.registrations.only?.method, DocumentDiagnosticsRequest.method)
156159
return VoidResponse()
157160
}
158-
preInitialization?(self)
159161
}
162+
preInitialization?(self)
160163
if initialize {
161164
_ = try await self.send(
162165
InitializeRequest(
@@ -193,12 +196,22 @@ public final class TestSourceKitLSPClient: MessageHandler {
193196
/// Send the request to `server` and return the request result.
194197
public func send<R: RequestType>(_ request: R) async throws -> R.Response {
195198
return try await withCheckedThrowingContinuation { continuation in
196-
server.handle(request, id: .number(Int(nextRequestID.fetchAndIncrement()))) { result in
199+
self.send(request) { result in
197200
continuation.resume(with: result)
198201
}
199202
}
200203
}
201204

205+
/// Send the request to `server` and return the result via a completion handler.
206+
///
207+
/// This version of the `send` function should only be used if some action needs to be performed after the request is
208+
/// sent but before it returns a result.
209+
public func send<R: RequestType>(_ request: R, completionHandler: @escaping (LSPResult<R.Response>) -> Void) {
210+
server.handle(request, id: .number(Int(nextRequestID.fetchAndIncrement()))) { result in
211+
completionHandler(result)
212+
}
213+
}
214+
202215
/// Send the notification to `server`.
203216
public func send(_ notification: some NotificationType) {
204217
server.handle(notification)

Sources/SemanticIndex/SemanticIndexManager.swift

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -327,10 +327,7 @@ public final actor SemanticIndexManager {
327327
/// Schedule preparation of the target that contains the given URI, building all modules that the file depends on.
328328
///
329329
/// This is intended to be called when the user is interacting with the document at the given URI.
330-
public func schedulePreparationForEditorFunctionality(
331-
of uri: DocumentURI,
332-
priority: TaskPriority? = nil
333-
) {
330+
public func schedulePreparationForEditorFunctionality(of uri: DocumentURI, priority: TaskPriority? = nil) {
334331
if inProgressPrepareForEditorTask?.document == uri {
335332
// We are already preparing this document, so nothing to do. This is necessary to avoid the following scenario:
336333
// Determining the canonical configured target for a document takes 1s and we get a new document request for the
@@ -341,13 +338,7 @@ public final actor SemanticIndexManager {
341338
let id = UUID()
342339
let task = Task(priority: priority) {
343340
await withLoggingScope("preparation") {
344-
guard let target = await buildSystemManager.canonicalConfiguredTarget(for: uri) else {
345-
return
346-
}
347-
if Task.isCancelled {
348-
return
349-
}
350-
await self.prepare(targets: [target], priority: priority)
341+
await self.prepareFileForEditorFunctionality(uri)
351342
if inProgressPrepareForEditorTask?.id == id {
352343
inProgressPrepareForEditorTask = nil
353344
}
@@ -357,6 +348,20 @@ public final actor SemanticIndexManager {
357348
inProgressPrepareForEditorTask = (id, uri, task)
358349
}
359350

351+
/// Prepare the target that the given file is in, building all modules that the file depends on. Returns when
352+
/// preparation has finished.
353+
///
354+
/// If file's target is known to be up-to-date, this returns almost immediately.
355+
public func prepareFileForEditorFunctionality(_ uri: DocumentURI) async {
356+
guard let target = await buildSystemManager.canonicalConfiguredTarget(for: uri) else {
357+
return
358+
}
359+
if Task.isCancelled {
360+
return
361+
}
362+
await self.prepare(targets: [target], priority: nil)
363+
}
364+
360365
// MARK: - Helper functions
361366

362367
/// Prepare the given targets for indexing.

Sources/SourceKitLSP/Swift/SwiftLanguageService.swift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import LSPLogging
1717
import LanguageServerProtocol
1818
import SKCore
1919
import SKSupport
20+
import SemanticIndex
2021
import SourceKitD
2122
import SwiftParser
2223
import SwiftParserDiagnostics
@@ -123,6 +124,9 @@ public actor SwiftLanguageService: LanguageService, Sendable {
123124

124125
let syntaxTreeManager = SyntaxTreeManager()
125126

127+
/// The `semanticIndexManager` of the workspace this language service was created for.
128+
private let semanticIndexManager: SemanticIndexManager?
129+
126130
nonisolated var keys: sourcekitd_api_keys { return sourcekitd.keys }
127131
nonisolated var requests: sourcekitd_api_requests { return sourcekitd.requests }
128132
nonisolated var values: sourcekitd_api_values { return sourcekitd.values }
@@ -192,6 +196,7 @@ public actor SwiftLanguageService: LanguageService, Sendable {
192196
self.swiftFormat = toolchain.swiftFormat
193197
self.sourcekitd = try await DynamicallyLoadedSourceKitD.getOrCreate(dylibPath: sourcekitd)
194198
self.capabilityRegistry = workspace.capabilityRegistry
199+
self.semanticIndexManager = workspace.semanticIndexManager
195200
self.serverOptions = options
196201
self.documentManager = DocumentManager()
197202
self.state = .connected
@@ -875,6 +880,7 @@ extension SwiftLanguageService {
875880

876881
public func documentDiagnostic(_ req: DocumentDiagnosticsRequest) async throws -> DocumentDiagnosticReport {
877882
do {
883+
await semanticIndexManager?.prepareFileForEditorFunctionality(req.textDocument.uri)
878884
let snapshot = try documentManager.latestSnapshot(req.textDocument.uri)
879885
let buildSettings = await self.buildSettings(for: req.textDocument.uri)
880886
let diagnosticReport = try await self.diagnosticReportManager.diagnosticReport(

Tests/SourceKitLSPTests/BackgroundIndexingTests.swift

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,6 @@ import SemanticIndex
1818
import SourceKitLSP
1919
import XCTest
2020

21-
fileprivate let backgroundIndexingOptions = SourceKitLSPServer.Options(
22-
indexOptions: IndexOptions(enableBackgroundIndexing: true)
23-
)
24-
2521
final class BackgroundIndexingTests: XCTestCase {
2622
func testBackgroundIndexingOfSingleFile() async throws {
2723
let project = try await SwiftPMTestProject(
@@ -33,7 +29,7 @@ final class BackgroundIndexingTests: XCTestCase {
3329
}
3430
"""
3531
],
36-
serverOptions: backgroundIndexingOptions
32+
enableBackgroundIndexing: true
3733
)
3834

3935
let (uri, positions) = try project.openDocument("MyFile.swift")
@@ -76,7 +72,7 @@ final class BackgroundIndexingTests: XCTestCase {
7672
}
7773
""",
7874
],
79-
serverOptions: backgroundIndexingOptions
75+
enableBackgroundIndexing: true
8076
)
8177

8278
let (uri, positions) = try project.openDocument("MyFile.swift")
@@ -134,7 +130,7 @@ final class BackgroundIndexingTests: XCTestCase {
134130
]
135131
)
136132
""",
137-
serverOptions: backgroundIndexingOptions
133+
enableBackgroundIndexing: true
138134
)
139135

140136
let (uri, positions) = try project.openDocument("MyFile.swift")
@@ -166,7 +162,7 @@ final class BackgroundIndexingTests: XCTestCase {
166162
}
167163

168164
func testBackgroundIndexingHappensWithLowPriority() async throws {
169-
var serverOptions = backgroundIndexingOptions
165+
var serverOptions = SourceKitLSPServer.Options.testDefault
170166
serverOptions.indexTestHooks.preparationTaskDidFinish = { taskDescription in
171167
XCTAssert(Task.currentPriority == .low, "\(taskDescription) ran with priority \(Task.currentPriority)")
172168
}
@@ -199,6 +195,7 @@ final class BackgroundIndexingTests: XCTestCase {
199195
)
200196
""",
201197
serverOptions: serverOptions,
198+
enableBackgroundIndexing: true,
202199
pollIndex: false
203200
)
204201

@@ -249,7 +246,7 @@ final class BackgroundIndexingTests: XCTestCase {
249246
]
250247
)
251248
""",
252-
serverOptions: backgroundIndexingOptions
249+
enableBackgroundIndexing: true
253250
)
254251

255252
let dependencyUrl = try XCTUnwrap(
@@ -300,7 +297,7 @@ final class BackgroundIndexingTests: XCTestCase {
300297
}
301298
""",
302299
],
303-
serverOptions: backgroundIndexingOptions
300+
enableBackgroundIndexing: true
304301
)
305302

306303
let (uri, positions) = try project.openDocument("MyFile.c")
@@ -339,7 +336,7 @@ final class BackgroundIndexingTests: XCTestCase {
339336
let receivedReportProgressNotification = self.expectation(
340337
description: "Received work done progress saying indexing"
341338
)
342-
var serverOptions = backgroundIndexingOptions
339+
var serverOptions = SourceKitLSPServer.Options.testDefault
343340
serverOptions.indexTestHooks = IndexTestHooks(
344341
buildGraphGenerationDidFinish: {
345342
await self.fulfillment(of: [receivedBeginProgressNotification], timeout: defaultTimeout)
@@ -359,6 +356,7 @@ final class BackgroundIndexingTests: XCTestCase {
359356
],
360357
capabilities: ClientCapabilities(window: WindowClientCapabilities(workDoneProgress: true)),
361358
serverOptions: serverOptions,
359+
enableBackgroundIndexing: true,
362360
pollIndex: false,
363361
preInitialization: { testClient in
364362
testClient.handleMultipleRequests { (request: CreateWorkDoneProgressRequest) in
@@ -419,7 +417,7 @@ final class BackgroundIndexingTests: XCTestCase {
419417
""",
420418
"MyOtherFile.swift": "",
421419
],
422-
serverOptions: backgroundIndexingOptions
420+
enableBackgroundIndexing: true
423421
)
424422

425423
let (uri, positions) = try project.openDocument("MyFile.swift")
@@ -486,7 +484,7 @@ final class BackgroundIndexingTests: XCTestCase {
486484
#include "Header.h"
487485
""",
488486
],
489-
serverOptions: backgroundIndexingOptions
487+
enableBackgroundIndexing: true
490488
)
491489

492490
let (uri, positions) = try project.openDocument("Header.h", language: .c)
@@ -544,7 +542,7 @@ final class BackgroundIndexingTests: XCTestCase {
544542

545543
func testPrepareTargetAfterEditToDependency() async throws {
546544
try await SkipUnless.swiftpmStoresModulesInSubdirectory()
547-
var serverOptions = backgroundIndexingOptions
545+
var serverOptions = SourceKitLSPServer.Options.testDefault
548546
let expectedPreparationTracker = ExpectedIndexTaskTracker(expectedPreparations: [
549547
[
550548
ExpectedPreparation(targetID: "LibA", runDestinationID: "dummy"),
@@ -580,6 +578,7 @@ final class BackgroundIndexingTests: XCTestCase {
580578
)
581579
""",
582580
serverOptions: serverOptions,
581+
enableBackgroundIndexing: true,
583582
cleanUp: { expectedPreparationTracker.keepAlive() }
584583
)
585584

@@ -637,7 +636,7 @@ final class BackgroundIndexingTests: XCTestCase {
637636
let libDPreparedForEditing = self.expectation(description: "LibD prepared for editing")
638637

639638
try await SkipUnless.swiftpmStoresModulesInSubdirectory()
640-
var serverOptions = backgroundIndexingOptions
639+
var serverOptions = SourceKitLSPServer.Options.testDefault
641640
let expectedPreparationTracker = ExpectedIndexTaskTracker(expectedPreparations: [
642641
// Preparation of targets during the initial of the target
643642
[
@@ -689,6 +688,7 @@ final class BackgroundIndexingTests: XCTestCase {
689688
)
690689
""",
691690
serverOptions: serverOptions,
691+
enableBackgroundIndexing: true,
692692
cleanUp: { expectedPreparationTracker.keepAlive() }
693693
)
694694

@@ -718,7 +718,7 @@ final class BackgroundIndexingTests: XCTestCase {
718718
files: [
719719
"MyFile.swift": ""
720720
],
721-
serverOptions: backgroundIndexingOptions
721+
enableBackgroundIndexing: true
722722
)
723723
let targetPrepareNotification = try await project.testClient.nextNotification(ofType: LogMessageNotification.self)
724724
XCTAssert(
@@ -732,13 +732,13 @@ final class BackgroundIndexingTests: XCTestCase {
732732
)
733733
}
734734

735-
func testPreparationHappensInParallel() async throws {
735+
func testIndexingHappensInParallel() async throws {
736736
try await SkipUnless.swiftpmStoresModulesInSubdirectory()
737737

738738
let fileAIndexingStarted = self.expectation(description: "FileA indexing started")
739739
let fileBIndexingStarted = self.expectation(description: "FileB indexing started")
740740

741-
var serverOptions = backgroundIndexingOptions
741+
var serverOptions = SourceKitLSPServer.Options.testDefault
742742
let expectedIndexTaskTracker = ExpectedIndexTaskTracker(
743743
expectedIndexStoreUpdates: [
744744
[
@@ -771,6 +771,7 @@ final class BackgroundIndexingTests: XCTestCase {
771771
"FileB.swift": "",
772772
],
773773
serverOptions: serverOptions,
774+
enableBackgroundIndexing: true,
774775
cleanUp: { expectedIndexTaskTracker.keepAlive() }
775776
)
776777
}
@@ -801,10 +802,10 @@ final class BackgroundIndexingTests: XCTestCase {
801802
]
802803
)
803804
""",
804-
serverOptions: backgroundIndexingOptions
805+
enableBackgroundIndexing: true
805806
)
806807

807-
var otherClientOptions = backgroundIndexingOptions
808+
var otherClientOptions = SourceKitLSPServer.Options.testDefault
808809
otherClientOptions.indexTestHooks = IndexTestHooks(
809810
preparationTaskDidStart: { taskDescription in
810811
XCTFail("Did not expect any target preparation, got \(taskDescription.targetsToPrepare)")
@@ -815,6 +816,7 @@ final class BackgroundIndexingTests: XCTestCase {
815816
)
816817
let otherClient = try await TestSourceKitLSPClient(
817818
serverOptions: otherClientOptions,
819+
enableBackgroundIndexing: true,
818820
workspaceFolders: [
819821
WorkspaceFolder(uri: DocumentURI(project.scratchDirectory))
820822
]

0 commit comments

Comments
 (0)