Use fallback build settings if build system doesn’t provide build settings within a timeout

Documentation/Configuration File.md

@@ -31,6 +31,7 @@ The structure of the file is currently not guaranteed to be stable. Options may
   - `cxxCompilerFlags: string[]`: Extra arguments passed to the compiler for C++ files
   - `swiftCompilerFlags: string[]`: Extra arguments passed to the compiler for Swift files
   - `sdk: string`: The SDK to use for fallback arguments. Default is to infer the SDK using `xcrun`.
+- `buildSettingsTimeout: int`: Number of milliseconds to wait for build settings from the build system before using fallback build settings.
 - `clangdOptions: string[]`: Extra command line arguments passed to `clangd` when launching it
 - `index`: Dictionary with the following keys, defining options related to indexing
     - `indexStorePath: string`: Directory in which a separate compilation stores the index store. By default, inferred from the build system.

Sources/BuildSystemIntegration/BuildSystemManager.swift

@@ -139,6 +139,7 @@ private extension BuildSystemKind {
   private static func createBuiltInBuildSystemAdapter(
     projectRoot: AbsolutePath,
     messagesToSourceKitLSPHandler: any MessageHandler,
+    buildSystemTestHooks: BuildSystemTestHooks,
     _ createBuildSystem: @Sendable (_ connectionToSourceKitLSP: any Connection) async throws -> BuiltInBuildSystem?
   ) async -> BuildSystemAdapter? {
     let connectionToSourceKitLSP = LocalConnection(
@@ -156,7 +157,8 @@ private extension BuildSystemKind {
     logger.log("Created \(type(of: buildSystem), privacy: .public) at \(projectRoot.pathString)")
     let buildSystemAdapter = BuiltInBuildSystemAdapter(
       underlyingBuildSystem: buildSystem,
-      connectionToSourceKitLSP: connectionToSourceKitLSP
+      connectionToSourceKitLSP: connectionToSourceKitLSP,
+      buildSystemTestHooks: buildSystemTestHooks
     )
     let connectionToBuildSystem = LocalConnection(
       receiverName: "\(type(of: buildSystem)) for \(projectRoot.asURL.lastPathComponent)"
@@ -190,7 +192,8 @@ private extension BuildSystemKind {
     case .compilationDatabase(projectRoot: let projectRoot):
       return await Self.createBuiltInBuildSystemAdapter(
         projectRoot: projectRoot,
-        messagesToSourceKitLSPHandler: messagesToSourceKitLSPHandler
+        messagesToSourceKitLSPHandler: messagesToSourceKitLSPHandler,
+        buildSystemTestHooks: testHooks
       ) { connectionToSourceKitLSP in
         CompilationDatabaseBuildSystem(
           projectRoot: projectRoot,
@@ -203,7 +206,8 @@ private extension BuildSystemKind {
     case .swiftPM(projectRoot: let projectRoot):
       return await Self.createBuiltInBuildSystemAdapter(
         projectRoot: projectRoot,
-        messagesToSourceKitLSPHandler: messagesToSourceKitLSPHandler
+        messagesToSourceKitLSPHandler: messagesToSourceKitLSPHandler,
+        buildSystemTestHooks: testHooks
       ) { connectionToSourceKitLSP in
         try await SwiftPMBuildSystem(
           projectRoot: projectRoot,
@@ -216,7 +220,8 @@ private extension BuildSystemKind {
     case .testBuildSystem(projectRoot: let projectRoot):
       return await Self.createBuiltInBuildSystemAdapter(
         projectRoot: projectRoot,
-        messagesToSourceKitLSPHandler: messagesToSourceKitLSPHandler
+        messagesToSourceKitLSPHandler: messagesToSourceKitLSPHandler,
+        buildSystemTestHooks: testHooks
       ) { connectionToSourceKitLSP in
         TestBuildSystem(projectRoot: projectRoot, connectionToSourceKitLSP: connectionToSourceKitLSP)
       }
@@ -411,7 +416,8 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
         )
         let adapter = BuiltInBuildSystemAdapter(
           underlyingBuildSystem: legacyBuildServer,
-          connectionToSourceKitLSP: legacyBuildServer.connectionToSourceKitLSP
+          connectionToSourceKitLSP: legacyBuildServer.connectionToSourceKitLSP,
+          buildSystemTestHooks: buildSystemTestHooks
         )
         let connectionToBuildSystem = LocalConnection(receiverName: "Legacy BSP server")
         connectionToBuildSystem.start(handler: adapter)
@@ -672,7 +678,12 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
   /// Returns the target's module name as parsed from the `BuildTargetIdentifier`'s compiler arguments.
   package func moduleName(for document: DocumentURI, in target: BuildTargetIdentifier) async -> String? {
     guard let language = await self.defaultLanguage(for: document, in: target),
-      let buildSettings = await buildSettings(for: document, in: target, language: language)
+      let buildSettings = await buildSettings(
+        for: document,
+        in: target,
+        language: language,
+        fallbackAfterTimeout: false
+      )
     else {
       return nil
     }
@@ -715,11 +726,6 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
       language: language
     )
 
-    // TODO: We should only wait `fallbackSettingsTimeout` for build settings
-    // and return fallback afterwards.
-    // For now, this should be fine because all build systems return
-    // very quickly from `settings(for:language:)`.
-    // https://github.com/apple/sourcekit-lsp/issues/1181
     let response = try await cachedSourceKitOptions.get(request, isolation: self) { request in
       try await buildSystemAdapter.send(request)
     }
@@ -740,19 +746,30 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
   /// Only call this method if it is known that `document` is a main file. Prefer `buildSettingsInferredFromMainFile`
   /// otherwise. If `document` is a header file, this will most likely return fallback settings because header files
   /// don't have build settings by themselves.
+  ///
+  /// If `fallbackAfterTimeout` is true fallback build settings will be returned if no build settings can be found in
+  /// `SourceKitLSPOptions.buildSettingsTimeoutOrDefault`.
   package func buildSettings(
     for document: DocumentURI,
     in target: BuildTargetIdentifier?,
-    language: Language
+    language: Language,
+    fallbackAfterTimeout: Bool
   ) async -> FileBuildSettings? {
-    do {
-      if let target,
-        let buildSettings = try await buildSettingsFromBuildSystem(for: document, in: target, language: language)
-      {
-        return buildSettings
+    if let target {
+      let buildSettingsFromBuildSystem = await orLog("Getting build settings") {
+        if fallbackAfterTimeout {
+          try await withTimeout(options.buildSettingsTimeoutOrDefault) {
+            return try await self.buildSettingsFromBuildSystem(for: document, in: target, language: language)
+          } resultReceivedAfterTimeout: {
+            await self.delegate?.fileBuildSettingsChanged([document])
+          }
+        } else {
+          try await self.buildSettingsFromBuildSystem(for: document, in: target, language: language)
+        }
+      }
+      if let buildSettingsFromBuildSystem {
+        return buildSettingsFromBuildSystem
       }
-    } catch {
-      logger.error("Getting build settings failed: \(error.forLogging)")
     }
 
     guard
@@ -780,14 +797,27 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
   /// by the header file.
   package func buildSettingsInferredFromMainFile(
     for document: DocumentURI,
-    language: Language
+    language: Language,
+    fallbackAfterTimeout: Bool
   ) async -> FileBuildSettings? {
     func mainFileAndSettings(
       basedOn document: DocumentURI
     ) async -> (mainFile: DocumentURI, settings: FileBuildSettings)? {
       let mainFile = await self.mainFile(for: document, language: language)
-      let target = await canonicalTarget(for: mainFile)
-      guard let settings = await buildSettings(for: mainFile, in: target, language: language) else {
+      let settings = await orLog("Getting build settings") {
+        let target = try await withTimeout(options.buildSettingsTimeoutOrDefault) {
+          await self.canonicalTarget(for: mainFile)
+        } resultReceivedAfterTimeout: {
+          await self.delegate?.fileBuildSettingsChanged([document])
+        }
+        return await self.buildSettings(
+          for: mainFile,
+          in: target,
+          language: language,
+          fallbackAfterTimeout: fallbackAfterTimeout
+        )
+      }
+      guard let settings else {
         return nil
       }
       return (mainFile, settings)

Sources/BuildSystemIntegration/BuildSystemTestHooks.swift

@@ -10,10 +10,25 @@
 //
 //===----------------------------------------------------------------------===//
 
+#if compiler(>=6)
+package import LanguageServerProtocol
+#else
+import LanguageServerProtocol
+#endif
+
 package struct BuildSystemTestHooks: Sendable {
   package var swiftPMTestHooks: SwiftPMTestHooks
 
-  package init(swiftPMTestHooks: SwiftPMTestHooks = SwiftPMTestHooks()) {
+  /// A hook that will be executed before a request is handled by a `BuiltInBuildSystem`.
+  ///
+  /// This allows requests to be artificially delayed.
+  package var handleRequest: (@Sendable (any RequestType) async -> Void)?
+
+  package init(
+    swiftPMTestHooks: SwiftPMTestHooks = SwiftPMTestHooks(),
+    handleRequest: (@Sendable (any RequestType) async -> Void)? = nil
+  ) {
     self.swiftPMTestHooks = swiftPMTestHooks
+    self.handleRequest = handleRequest
   }
 }

Sources/BuildSystemIntegration/BuiltInBuildSystemAdapter.swift

@@ -59,6 +59,8 @@ actor BuiltInBuildSystemAdapter: QueueBasedMessageHandler {
   /// The connection with which messages are sent to `BuildSystemManager`.
   private let connectionToSourceKitLSP: LocalConnection
 
+  private let buildSystemTestHooks: BuildSystemTestHooks
+
   /// If the underlying build system is a `TestBuildSystem`, return it. Otherwise, `nil`
   ///
   /// - Important: For testing purposes only.
@@ -70,10 +72,12 @@ actor BuiltInBuildSystemAdapter: QueueBasedMessageHandler {
   /// from the build system to SourceKit-LSP.
   init(
     underlyingBuildSystem: BuiltInBuildSystem,
-    connectionToSourceKitLSP: LocalConnection
+    connectionToSourceKitLSP: LocalConnection,
+    buildSystemTestHooks: BuildSystemTestHooks
   ) {
     self.underlyingBuildSystem = underlyingBuildSystem
     self.connectionToSourceKitLSP = connectionToSourceKitLSP
+    self.buildSystemTestHooks = buildSystemTestHooks
   }
 
   deinit {
@@ -116,6 +120,7 @@ actor BuiltInBuildSystemAdapter: QueueBasedMessageHandler {
     reply: @Sendable @escaping (LSPResult<Request.Response>) -> Void
   ) async {
     let request = RequestAndReply(request, reply: reply)
+    await buildSystemTestHooks.handleRequest?(request.params)
     switch request {
     case let request as RequestAndReply<BuildShutdownRequest>:
       await request.reply { VoidResponse() }

Sources/BuildSystemIntegration/CMakeLists.txt

@@ -14,7 +14,6 @@ add_library(BuildSystemIntegration STATIC
   ExternalBuildSystemAdapter.swift
   FallbackBuildSettings.swift
   FileBuildSettings.swift
-  Language+InferredFromFileExtension.swift
   LegacyBuildServerBuildSystem.swift
   MainFilesProvider.swift
   PathPrefixMapping.swift

Sources/SKOptions/SourceKitLSPOptions.swift

@@ -250,6 +250,13 @@ public struct SourceKitLSPOptions: Sendable, Codable, Equatable {
     set { fallbackBuildSystem = newValue }
   }
 
+  /// Number of milliseconds to wait for build settings from the build system before using fallback build settings.
+  public var buildSettingsTimeout: Int?
+  public var buildSettingsTimeoutOrDefault: Duration {
+    // The default timeout of 500ms was chosen arbitrarily without any measurements.
+    get { .milliseconds(buildSettingsTimeout ?? 500) }
+  }
+
   public var clangdOptions: [String]?
 
   private var index: IndexOptions?

Sources/SKSupport/CMakeLists.txt

@@ -8,6 +8,7 @@ add_library(SKSupport STATIC
   DocumentURI+CustomLogStringConvertible.swift
   DocumentURI+symlinkTarget.swift
   FileSystem.swift
+  Language+InferredFromFileExtension.swift
   LineTable.swift
   LocalConnection.swift
   Process+Run.swift

Sources/BuildSystemIntegration/Language+InferredFromFileExtension.swift → Sources/SKSupport/Language+InferredFromFileExtension.swift

@@ -10,11 +10,16 @@
 //
 //===----------------------------------------------------------------------===//
 
+#if compiler(>=6)
+import Foundation
+package import LanguageServerProtocol
+#else
 import Foundation
 import LanguageServerProtocol
+#endif
 
 extension Language {
-  init?(inferredFromFileExtension uri: DocumentURI) {
+  package init?(inferredFromFileExtension uri: DocumentURI) {
     // URL.pathExtension is only set for file URLs but we want to also infer a file extension for non-file URLs like
     // untitled:file.cpp
     let pathExtension = uri.fileURL?.pathExtension ?? (uri.pseudoPath as NSString).pathExtension

Sources/SKTestSupport/WrappedSemaphore.swift

@@ -56,12 +56,16 @@ package struct WrappedSemaphore: Sendable {
   }
 
   /// Wait for a signal and emit an XCTFail if the semaphore is not signaled within `timeout`.
-  package func waitOrXCTFail(timeout: DispatchTime = DispatchTime.now() + .seconds(Int(defaultTimeout))) {
+  package func waitOrXCTFail(
+    timeout: DispatchTime = DispatchTime.now() + .seconds(Int(defaultTimeout)),
+    file: StaticString = #filePath,
+    line: UInt = #line
+  ) {
     switch self.wait(timeout: timeout) {
     case .success:
       break
     case .timedOut:
-      XCTFail("\(name) timed out")
+      XCTFail("\(name) timed out", file: file, line: line)
     }
   }
 }

Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift

@@ -254,7 +254,12 @@ package struct UpdateIndexStoreTaskDescription: IndexTaskDescription {
       logger.error("Not indexing \(file.forLogging) because its language could not be determined")
       return
     }
-    let buildSettings = await buildSystemManager.buildSettings(for: file.mainFile, in: target, language: language)
+    let buildSettings = await buildSystemManager.buildSettings(
+      for: file.mainFile,
+      in: target,
+      language: language,
+      fallbackAfterTimeout: false
+    )
     guard let buildSettings else {
       logger.error("Not indexing \(file.forLogging) because it has no compiler arguments")
       return

Sources/SourceKitLSP/Clang/ClangLanguageService.swift

@@ -129,14 +129,15 @@ actor ClangLanguageService: LanguageService, MessageHandler {
     try startClangdProcess()
   }
 
-  private func buildSettings(for document: DocumentURI) async -> ClangBuildSettings? {
+  private func buildSettings(for document: DocumentURI, fallbackAfterTimeout: Bool) async -> ClangBuildSettings? {
     guard let workspace = workspace.value, let language = openDocuments[document] else {
       return nil
     }
     guard
       let settings = await workspace.buildSystemManager.buildSettingsInferredFromMainFile(
         for: document,
-        language: language
+        language: language,
+        fallbackAfterTimeout: fallbackAfterTimeout
       )
     else {
       return nil
@@ -340,7 +341,7 @@ actor ClangLanguageService: LanguageService, MessageHandler {
 
 extension ClangLanguageService {
 
-  /// Intercept clangd's `PublishDiagnosticsNotification` to withold it if we're using fallback
+  /// Intercept clangd's `PublishDiagnosticsNotification` to withhold it if we're using fallback
   /// build settings.
   func publishDiagnostics(_ notification: PublishDiagnosticsNotification) async {
     // Technically, the publish diagnostics notification could still originate
@@ -354,7 +355,9 @@ extension ClangLanguageService {
     // short and we expect clangd to send us new diagnostics with the updated
     // non-fallback settings very shortly after, which will override the
     // incorrect result, making it very temporary.
-    let buildSettings = await self.buildSettings(for: notification.uri)
+    // TODO: We want to know the build settings that are currently transmitted to clangd, not whichever ones we would
+    // get next. (https://github.com/swiftlang/sourcekit-lsp/issues/1761)
+    let buildSettings = await self.buildSettings(for: notification.uri, fallbackAfterTimeout: true)
     guard let sourceKitLSPServer else {
       logger.fault("Cannot publish diagnostics because SourceKitLSPServer has been destroyed")
       return
@@ -452,7 +455,7 @@ extension ClangLanguageService {
       logger.error("Received updated build settings for non-file URI '\(uri.forLogging)'. Ignoring the update.")
       return
     }
-    let clangBuildSettings = await self.buildSettings(for: uri)
+    let clangBuildSettings = await self.buildSettings(for: uri, fallbackAfterTimeout: false)
 
     // The compile command changed, send over the new one.
     if let compileCommand = clangBuildSettings?.compileCommand,

Sources/SourceKitLSP/Rename.swift

@@ -364,7 +364,8 @@ extension SwiftLanguageService {
     let req = sourcekitd.dictionary([
       keys.request: sourcekitd.requests.nameTranslation,
       keys.sourceFile: snapshot.uri.pseudoPath,
-      keys.compilerArgs: await self.buildSettings(for: snapshot.uri)?.compilerArgs as [SKDRequestValue]?,
+      keys.compilerArgs: await self.buildSettings(for: snapshot.uri, fallbackAfterTimeout: false)?.compilerArgs
+        as [SKDRequestValue]?,
       keys.offset: snapshot.utf8Offset(of: snapshot.position(of: symbolLocation)),
       keys.nameKind: sourcekitd.values.nameSwift,
       keys.baseName: name.baseName,
@@ -415,7 +416,8 @@ extension SwiftLanguageService {
     let req = sourcekitd.dictionary([
       keys.request: sourcekitd.requests.nameTranslation,
       keys.sourceFile: snapshot.uri.pseudoPath,
-      keys.compilerArgs: await self.buildSettings(for: snapshot.uri)?.compilerArgs as [SKDRequestValue]?,
+      keys.compilerArgs: await self.buildSettings(for: snapshot.uri, fallbackAfterTimeout: false)?.compilerArgs
+        as [SKDRequestValue]?,
       keys.offset: snapshot.utf8Offset(of: snapshot.position(of: symbolLocation)),
       keys.nameKind: sourcekitd.values.nameObjc,
     ])

Sources/SourceKitLSP/Swift/CodeCompletion.swift

@@ -34,7 +34,7 @@ extension SwiftLanguageService {
 
     let clientSupportsSnippets =
       capabilityRegistry.clientCapabilities.textDocument?.completion?.completionItem?.snippetSupport ?? false
-    let buildSettings = await buildSettings(for: snapshot.uri)
+    let buildSettings = await buildSettings(for: snapshot.uri, fallbackAfterTimeout: false)
 
     let inferredIndentationWidth = BasicFormat.inferIndentation(of: await syntaxTreeManager.syntaxTree(for: snapshot))