From 5bae73fca88e3427fc0dff8704a11ed779fd042f Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Fri, 20 Sep 2024 17:11:07 -0700 Subject: [PATCH] =?UTF-8?q?Use=20fallback=20build=20settings=20if=20build?= =?UTF-8?q?=20system=20doesn=E2=80=99t=20provide=20build=20settings=20with?= =?UTF-8?q?in=20a=20timeout?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When we receive build settings after hitting the timeout, we call `fileBuildSettingsChanged` on the delegate, which should cause the document to get re-opened in sourcekitd and diagnostics to get refreshed. rdar://136332685 Fixes #1693 --- Documentation/Configuration File.md | 1 + .../BuildSystemManager.swift | 74 ++++++++---- .../BuildSystemTestHooks.swift | 17 ++- .../BuiltInBuildSystemAdapter.swift | 7 +- Sources/BuildSystemIntegration/CMakeLists.txt | 1 - Sources/SKOptions/SourceKitLSPOptions.swift | 7 ++ Sources/SKSupport/CMakeLists.txt | 1 + .../Language+InferredFromFileExtension.swift | 7 +- Sources/SKTestSupport/WrappedSemaphore.swift | 8 +- .../UpdateIndexStoreTaskDescription.swift | 7 +- .../Clang/ClangLanguageService.swift | 13 ++- Sources/SourceKitLSP/Rename.swift | 6 +- .../SourceKitLSP/Swift/CodeCompletion.swift | 2 +- Sources/SourceKitLSP/Swift/CursorInfo.swift | 7 +- .../SourceKitLSP/Swift/MacroExpansion.swift | 2 +- .../SourceKitLSP/Swift/OpenInterface.swift | 3 +- .../Swift/RefactoringResponse.swift | 3 +- .../Swift/RelatedIdentifiers.swift | 3 +- .../SourceKitLSP/Swift/SemanticTokens.swift | 4 +- .../Swift/SwiftLanguageService.swift | 22 ++-- Sources/SourceKitLSP/Swift/SymbolInfo.swift | 3 +- .../SourceKitLSP/Swift/VariableTypeInfo.swift | 3 +- Sources/SwiftExtensions/AsyncUtils.swift | 46 ++++++++ .../BuildSystemManagerTests.swift | 32 +++++- .../FallbackBuildSettingsTests.swift | 40 +++++++ .../SwiftPMBuildSystemTests.swift | 107 ++++++++++++++---- 26 files changed, 346 insertions(+), 80 deletions(-) rename Sources/{BuildSystemIntegration => SKSupport}/Language+InferredFromFileExtension.swift (87%) diff --git a/Documentation/Configuration File.md b/Documentation/Configuration File.md index ea03e6df..4f6b31f7 100644 --- a/Documentation/Configuration File.md +++ b/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. diff --git a/Sources/BuildSystemIntegration/BuildSystemManager.swift b/Sources/BuildSystemIntegration/BuildSystemManager.swift index 0c256c53..1dbdb27f 100644 --- a/Sources/BuildSystemIntegration/BuildSystemManager.swift +++ b/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) diff --git a/Sources/BuildSystemIntegration/BuildSystemTestHooks.swift b/Sources/BuildSystemIntegration/BuildSystemTestHooks.swift index b76343b3..aa59f033 100644 --- a/Sources/BuildSystemIntegration/BuildSystemTestHooks.swift +++ b/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 } } diff --git a/Sources/BuildSystemIntegration/BuiltInBuildSystemAdapter.swift b/Sources/BuildSystemIntegration/BuiltInBuildSystemAdapter.swift index 76eb5126..002c35ae 100644 --- a/Sources/BuildSystemIntegration/BuiltInBuildSystemAdapter.swift +++ b/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) -> Void ) async { let request = RequestAndReply(request, reply: reply) + await buildSystemTestHooks.handleRequest?(request.params) switch request { case let request as RequestAndReply: await request.reply { VoidResponse() } diff --git a/Sources/BuildSystemIntegration/CMakeLists.txt b/Sources/BuildSystemIntegration/CMakeLists.txt index c342933c..2b0a41ca 100644 --- a/Sources/BuildSystemIntegration/CMakeLists.txt +++ b/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 diff --git a/Sources/SKOptions/SourceKitLSPOptions.swift b/Sources/SKOptions/SourceKitLSPOptions.swift index 2c5d6452..58e0065b 100644 --- a/Sources/SKOptions/SourceKitLSPOptions.swift +++ b/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? diff --git a/Sources/SKSupport/CMakeLists.txt b/Sources/SKSupport/CMakeLists.txt index c93f5040..546c4c44 100644 --- a/Sources/SKSupport/CMakeLists.txt +++ b/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 diff --git a/Sources/BuildSystemIntegration/Language+InferredFromFileExtension.swift b/Sources/SKSupport/Language+InferredFromFileExtension.swift similarity index 87% rename from Sources/BuildSystemIntegration/Language+InferredFromFileExtension.swift rename to Sources/SKSupport/Language+InferredFromFileExtension.swift index 7e2c1a6a..160996eb 100644 --- a/Sources/BuildSystemIntegration/Language+InferredFromFileExtension.swift +++ b/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 diff --git a/Sources/SKTestSupport/WrappedSemaphore.swift b/Sources/SKTestSupport/WrappedSemaphore.swift index 428ea258..22e3e49c 100644 --- a/Sources/SKTestSupport/WrappedSemaphore.swift +++ b/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) } } } diff --git a/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift b/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift index e8515f3c..c27b6c62 100644 --- a/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift +++ b/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 diff --git a/Sources/SourceKitLSP/Clang/ClangLanguageService.swift b/Sources/SourceKitLSP/Clang/ClangLanguageService.swift index 8c0cb94a..b2929cba 100644 --- a/Sources/SourceKitLSP/Clang/ClangLanguageService.swift +++ b/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, diff --git a/Sources/SourceKitLSP/Rename.swift b/Sources/SourceKitLSP/Rename.swift index 80f8feaa..e1763888 100644 --- a/Sources/SourceKitLSP/Rename.swift +++ b/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, ]) diff --git a/Sources/SourceKitLSP/Swift/CodeCompletion.swift b/Sources/SourceKitLSP/Swift/CodeCompletion.swift index 84701e3d..4b0ff3ae 100644 --- a/Sources/SourceKitLSP/Swift/CodeCompletion.swift +++ b/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)) diff --git a/Sources/SourceKitLSP/Swift/CursorInfo.swift b/Sources/SourceKitLSP/Swift/CursorInfo.swift index 99271683..3d97488e 100644 --- a/Sources/SourceKitLSP/Swift/CursorInfo.swift +++ b/Sources/SourceKitLSP/Swift/CursorInfo.swift @@ -141,10 +141,12 @@ extension SwiftLanguageService { /// - Parameters: /// - url: Document URI in which to perform the request. Must be an open document. /// - range: The position range within the document to lookup the symbol at. - /// - completion: Completion block to asynchronously receive the CursorInfo, or error. + /// - fallbackSettingsAfterTimeout: Whether fallback build settings should be used for the cursor info request if no + /// build settings can be retrieved within a timeout. func cursorInfo( _ uri: DocumentURI, _ range: Range, + fallbackSettingsAfterTimeout: Bool, additionalParameters appendAdditionalParameters: ((SKDRequestDictionary) -> Void)? = nil ) async throws -> (cursorInfo: [CursorInfo], refactorActions: [SemanticRefactorCommand]) { let documentManager = try self.documentManager @@ -161,7 +163,8 @@ extension SwiftLanguageService { keys.length: offsetRange.upperBound != offsetRange.lowerBound ? offsetRange.count : nil, keys.sourceFile: snapshot.uri.sourcekitdSourceFile, keys.primaryFile: snapshot.uri.primaryFile?.pseudoPath, - keys.compilerArgs: await self.buildSettings(for: uri)?.compilerArgs as [SKDRequestValue]?, + keys.compilerArgs: await self.buildSettings(for: uri, fallbackAfterTimeout: fallbackSettingsAfterTimeout)? + .compilerArgs as [SKDRequestValue]?, ]) appendAdditionalParameters?(skreq) diff --git a/Sources/SourceKitLSP/Swift/MacroExpansion.swift b/Sources/SourceKitLSP/Swift/MacroExpansion.swift index 6ff5c9ca..1d65158f 100644 --- a/Sources/SourceKitLSP/Swift/MacroExpansion.swift +++ b/Sources/SourceKitLSP/Swift/MacroExpansion.swift @@ -86,7 +86,7 @@ actor MacroExpansionManager { } let snapshot = try await swiftLanguageService.latestSnapshot(for: uri) - let buildSettings = await swiftLanguageService.buildSettings(for: uri) + let buildSettings = await swiftLanguageService.buildSettings(for: uri, fallbackAfterTimeout: false) if let cacheEntry = cache.first(where: { $0.snapshotID == snapshot.id && $0.range == range && $0.buildSettings == buildSettings diff --git a/Sources/SourceKitLSP/Swift/OpenInterface.swift b/Sources/SourceKitLSP/Swift/OpenInterface.swift index 3069860e..80835619 100644 --- a/Sources/SourceKitLSP/Swift/OpenInterface.swift +++ b/Sources/SourceKitLSP/Swift/OpenInterface.swift @@ -100,7 +100,8 @@ extension SwiftLanguageService { keys.groupName: groupName, keys.name: interfaceURI.pseudoPath, keys.synthesizedExtension: 1, - keys.compilerArgs: await self.buildSettings(for: document)?.compilerArgs as [SKDRequestValue]?, + keys.compilerArgs: await self.buildSettings(for: document, fallbackAfterTimeout: false)?.compilerArgs + as [SKDRequestValue]?, ]) let dict = try await sendSourcekitdRequest(skreq, fileContents: nil) diff --git a/Sources/SourceKitLSP/Swift/RefactoringResponse.swift b/Sources/SourceKitLSP/Swift/RefactoringResponse.swift index f415c8fa..d6b7204d 100644 --- a/Sources/SourceKitLSP/Swift/RefactoringResponse.swift +++ b/Sources/SourceKitLSP/Swift/RefactoringResponse.swift @@ -126,7 +126,8 @@ extension SwiftLanguageService { keys.column: utf8Column + 1, keys.length: snapshot.utf8OffsetRange(of: refactorCommand.positionRange).count, keys.actionUID: self.sourcekitd.api.uid_get_from_cstr(refactorCommand.actionString)!, - keys.compilerArgs: await self.buildSettings(for: snapshot.uri)?.compilerArgs as [SKDRequestValue]?, + keys.compilerArgs: await self.buildSettings(for: snapshot.uri, fallbackAfterTimeout: true)?.compilerArgs + as [SKDRequestValue]?, ]) let dict = try await sendSourcekitdRequest(skreq, fileContents: snapshot.text) diff --git a/Sources/SourceKitLSP/Swift/RelatedIdentifiers.swift b/Sources/SourceKitLSP/Swift/RelatedIdentifiers.swift index d285fed4..8e3ec78b 100644 --- a/Sources/SourceKitLSP/Swift/RelatedIdentifiers.swift +++ b/Sources/SourceKitLSP/Swift/RelatedIdentifiers.swift @@ -73,7 +73,8 @@ extension SwiftLanguageService { keys.sourceFile: snapshot.uri.sourcekitdSourceFile, keys.primaryFile: snapshot.uri.primaryFile?.pseudoPath, keys.includeNonEditableBaseNames: includeNonEditableBaseNames ? 1 : 0, - keys.compilerArgs: await self.buildSettings(for: snapshot.uri)?.compilerArgs as [SKDRequestValue]?, + keys.compilerArgs: await self.buildSettings(for: snapshot.uri, fallbackAfterTimeout: true)?.compilerArgs + as [SKDRequestValue]?, ]) let dict = try await sendSourcekitdRequest(skreq, fileContents: snapshot.text) diff --git a/Sources/SourceKitLSP/Swift/SemanticTokens.swift b/Sources/SourceKitLSP/Swift/SemanticTokens.swift index 361d7695..d95c8b10 100644 --- a/Sources/SourceKitLSP/Swift/SemanticTokens.swift +++ b/Sources/SourceKitLSP/Swift/SemanticTokens.swift @@ -29,7 +29,9 @@ import SwiftSyntax extension SwiftLanguageService { /// Requests the semantic highlighting tokens for the given snapshot from sourcekitd. private func semanticHighlightingTokens(for snapshot: DocumentSnapshot) async throws -> SyntaxHighlightingTokens? { - guard let buildSettings = await self.buildSettings(for: snapshot.uri), !buildSettings.isFallback else { + guard let buildSettings = await self.buildSettings(for: snapshot.uri, fallbackAfterTimeout: false), + !buildSettings.isFallback + else { return nil } diff --git a/Sources/SourceKitLSP/Swift/SwiftLanguageService.swift b/Sources/SourceKitLSP/Swift/SwiftLanguageService.swift index e66e989e..f1fa3e74 100644 --- a/Sources/SourceKitLSP/Swift/SwiftLanguageService.swift +++ b/Sources/SourceKitLSP/Swift/SwiftLanguageService.swift @@ -259,7 +259,7 @@ package actor SwiftLanguageService: LanguageService, Sendable { } } - func buildSettings(for document: DocumentURI) async -> SwiftCompileCommand? { + func buildSettings(for document: DocumentURI, fallbackAfterTimeout: Bool) async -> SwiftCompileCommand? { let primaryDocument = document.primaryFile ?? document guard let sourceKitLSPServer else { @@ -271,7 +271,8 @@ package actor SwiftLanguageService: LanguageService, Sendable { } if let settings = await workspace.buildSystemManager.buildSettingsInferredFromMainFile( for: primaryDocument, - language: .swift + language: .swift, + fallbackAfterTimeout: fallbackAfterTimeout ) { return SwiftCompileCommand(settings) } else { @@ -428,7 +429,7 @@ extension SwiftLanguageService { try await self.sendSourcekitdRequest(closeReq, fileContents: nil) } - let buildSettings = await buildSettings(for: snapshot.uri) + let buildSettings = await buildSettings(for: snapshot.uri, fallbackAfterTimeout: true) let openReq = openDocumentSourcekitdRequest( snapshot: snapshot, compileCommand: buildSettings @@ -450,7 +451,7 @@ extension SwiftLanguageService { guard (try? documentManager.openDocuments.contains(uri)) ?? false else { return } - let newBuildSettings = await self.buildSettings(for: uri) + let newBuildSettings = await self.buildSettings(for: uri, fallbackAfterTimeout: false) if newBuildSettings != buildSettingsForOpenFiles[uri] { // Close and re-open the document internally to inform sourcekitd to update the compile command. At the moment // there's no better way to do this. @@ -516,7 +517,7 @@ extension SwiftLanguageService { cancelInFlightPublishDiagnosticsTask(for: notification.textDocument.uri) await diagnosticReportManager.removeItemsFromCache(with: notification.textDocument.uri) - let buildSettings = await self.buildSettings(for: snapshot.uri) + let buildSettings = await self.buildSettings(for: snapshot.uri, fallbackAfterTimeout: true) buildSettingsForOpenFiles[snapshot.uri] = buildSettings let req = openDocumentSourcekitdRequest(snapshot: snapshot, compileCommand: buildSettings) @@ -578,7 +579,7 @@ extension SwiftLanguageService { } do { let snapshot = try await self.latestSnapshot(for: document) - let buildSettings = await self.buildSettings(for: document) + let buildSettings = await self.buildSettings(for: document, fallbackAfterTimeout: false) let diagnosticReport = try await self.diagnosticReportManager.diagnosticReport( for: snapshot, buildSettings: buildSettings @@ -688,7 +689,8 @@ extension SwiftLanguageService { package func hover(_ req: HoverRequest) async throws -> HoverResponse? { let uri = req.textDocument.uri let position = req.position - let cursorInfoResults = try await cursorInfo(uri, position.. String? in if let documentation = cursorInfo.documentation { @@ -891,6 +893,7 @@ extension SwiftLanguageService { let cursorInfoResponse = try await cursorInfo( params.textDocument.uri, params.range, + fallbackSettingsAfterTimeout: true, additionalParameters: additionalCursorInfoParameters ) @@ -917,7 +920,7 @@ extension SwiftLanguageService { func retrieveQuickFixCodeActions(_ params: CodeActionRequest) async throws -> [CodeAction] { let snapshot = try await self.latestSnapshot(for: params.textDocument.uri) - let buildSettings = await self.buildSettings(for: params.textDocument.uri) + let buildSettings = await self.buildSettings(for: params.textDocument.uri, fallbackAfterTimeout: true) let diagnosticReport = try await self.diagnosticReportManager.diagnosticReport( for: snapshot, buildSettings: buildSettings @@ -1010,7 +1013,8 @@ extension SwiftLanguageService { req.textDocument.uri.primaryFile ?? req.textDocument.uri ) let snapshot = try await self.latestSnapshot(for: req.textDocument.uri) - let buildSettings = await self.buildSettings(for: req.textDocument.uri) + let buildSettings = await self.buildSettings(for: req.textDocument.uri, fallbackAfterTimeout: false) + try Task.checkCancellation() let diagnosticReport = try await self.diagnosticReportManager.diagnosticReport( for: snapshot, buildSettings: buildSettings diff --git a/Sources/SourceKitLSP/Swift/SymbolInfo.swift b/Sources/SourceKitLSP/Swift/SymbolInfo.swift index cc178cae..4b5320fe 100644 --- a/Sources/SourceKitLSP/Swift/SymbolInfo.swift +++ b/Sources/SourceKitLSP/Swift/SymbolInfo.swift @@ -21,6 +21,7 @@ extension SwiftLanguageService { let uri = req.textDocument.uri let snapshot = try documentManager.latestSnapshot(uri) let position = await self.adjustPositionToStartOfIdentifier(req.position, in: snapshot) - return try await cursorInfo(uri, position..( } } } + +/// Executes `body`. If it doesn't finish after `duration`, return `nil` and continue running body. When `body` returns +/// a value after the timeout, `resultReceivedAfterTimeout` is called. +/// +/// - Important: `body` will not be cancelled when the timeout is received. Use the other overload of `withTimeout` if +/// `body` should be cancelled after `timeout`. +package func withTimeout( + _ timeout: Duration, + body: @escaping @Sendable () async throws -> T?, + resultReceivedAfterTimeout: @escaping @Sendable () async -> Void +) async throws -> T? { + let didHitTimeout = AtomicBool(initialValue: false) + + let stream = AsyncThrowingStream { continuation in + Task { + try await Task.sleep(for: timeout) + didHitTimeout.value = true + continuation.yield(nil) + } + + Task { + do { + let result = try await body() + if didHitTimeout.value { + await resultReceivedAfterTimeout() + } + continuation.yield(result) + } catch { + continuation.yield(with: .failure(error)) + } + } + } + + for try await value in stream { + return value + } + // The only reason for the loop above to terminate is if the Task got cancelled or if the continuation finishes + // (which it never does). + if Task.isCancelled { + throw CancellationError() + } else { + preconditionFailure("Continuation never finishes") + } +} diff --git a/Tests/BuildSystemIntegrationTests/BuildSystemManagerTests.swift b/Tests/BuildSystemIntegrationTests/BuildSystemManagerTests.swift index f27dba78..28e1f7d4 100644 --- a/Tests/BuildSystemIntegrationTests/BuildSystemManagerTests.swift +++ b/Tests/BuildSystemIntegrationTests/BuildSystemManagerTests.swift @@ -118,7 +118,11 @@ final class BuildSystemManagerTests: XCTestCase { // Wait for the new build settings to settle before registering for change notifications await bsm.waitForUpToDateBuildGraph() await bsm.registerForChangeNotifications(for: a, language: .swift) - assertEqual(await bsm.buildSettingsInferredFromMainFile(for: a, language: .swift)?.compilerArguments, ["x"]) + assertEqual( + await bsm.buildSettingsInferredFromMainFile(for: a, language: .swift, fallbackAfterTimeout: false)? + .compilerArguments, + ["x"] + ) let changed = expectation(description: "changed settings") await del.setExpected([ @@ -166,7 +170,10 @@ final class BuildSystemManagerTests: XCTestCase { let del = await BSMDelegate(bsm) let fallbackSettings = fallbackBuildSettings(for: a, language: .swift, options: .init()) await bsm.registerForChangeNotifications(for: a, language: .swift) - assertEqual(await bsm.buildSettingsInferredFromMainFile(for: a, language: .swift), fallbackSettings) + assertEqual( + await bsm.buildSettingsInferredFromMainFile(for: a, language: .swift, fallbackAfterTimeout: false), + fallbackSettings + ) let changed = expectation(description: "changed settings") await del.setExpected([(a, .swift, FileBuildSettings(compilerArguments: ["non-fallback", "args"]), changed)]) @@ -212,7 +219,10 @@ final class BuildSystemManagerTests: XCTestCase { // Wait for the new build settings to settle before registering for change notifications await bsm.waitForUpToDateBuildGraph() await bsm.registerForChangeNotifications(for: h, language: .c) - assertEqual(await bsm.buildSettingsInferredFromMainFile(for: h, language: .c)?.compilerArguments, ["C++ 1"]) + assertEqual( + await bsm.buildSettingsInferredFromMainFile(for: h, language: .c, fallbackAfterTimeout: false)?.compilerArguments, + ["C++ 1"] + ) await mainFiles.updateMainFiles(for: h, to: [cpp2]) @@ -281,8 +291,14 @@ final class BuildSystemManagerTests: XCTestCase { let expectedArgsH1 = FileBuildSettings(compilerArguments: ["-xc++", cppArg, h1.pseudoPath]) let expectedArgsH2 = FileBuildSettings(compilerArguments: ["-xc++", cppArg, h2.pseudoPath]) - assertEqual(await bsm.buildSettingsInferredFromMainFile(for: h1, language: .c), expectedArgsH1) - assertEqual(await bsm.buildSettingsInferredFromMainFile(for: h2, language: .c), expectedArgsH2) + assertEqual( + await bsm.buildSettingsInferredFromMainFile(for: h1, language: .c, fallbackAfterTimeout: false), + expectedArgsH1 + ) + assertEqual( + await bsm.buildSettingsInferredFromMainFile(for: h2, language: .c, fallbackAfterTimeout: false), + expectedArgsH2 + ) let newCppArg = "New C++ Main File" let changed1 = expectation(description: "initial settings h1 via cpp") @@ -362,7 +378,11 @@ private actor BSMDelegate: BuildSystemManagerDelegate { self.expected.remove(at: expectedIndex) XCTAssertEqual(uri, expected.uri, file: expected.file, line: expected.line) - let settings = await bsm.buildSettingsInferredFromMainFile(for: uri, language: expected.language) + let settings = await bsm.buildSettingsInferredFromMainFile( + for: uri, + language: expected.language, + fallbackAfterTimeout: false + ) XCTAssertEqual(settings, expected.settings, file: expected.file, line: expected.line) expected.expectation.fulfill() } diff --git a/Tests/BuildSystemIntegrationTests/FallbackBuildSettingsTests.swift b/Tests/BuildSystemIntegrationTests/FallbackBuildSettingsTests.swift index 0698a8cf..a074d8e3 100644 --- a/Tests/BuildSystemIntegrationTests/FallbackBuildSettingsTests.swift +++ b/Tests/BuildSystemIntegrationTests/FallbackBuildSettingsTests.swift @@ -10,10 +10,12 @@ // //===----------------------------------------------------------------------===// +import BuildServerProtocol @_spi(Testing) import BuildSystemIntegration import LanguageServerProtocol import SKOptions import SKTestSupport +import SourceKitLSP import TSCBasic import XCTest @@ -160,4 +162,42 @@ final class FallbackBuildSystemTests: XCTestCase { XCTAssertNil(fallbackBuildSettings(for: source, language: Language(rawValue: "unknown"), options: .init())) } + + func testFallbackBuildSettingsWhileBuildSystemIsComputingBuildSettings() async throws { + let fallbackResultsReceived = WrappedSemaphore(name: "Fallback results received") + let project = try await SwiftPMTestProject( + files: [ + "Test.swift": """ + let x: 1️⃣String2️⃣ = 1 + """ + ], + testHooks: TestHooks( + buildSystemTestHooks: BuildSystemTestHooks( + handleRequest: { request in + if request is TextDocumentSourceKitOptionsRequest { + fallbackResultsReceived.waitOrXCTFail() + } + } + ) + ) + ) + + let (uri, positions) = try project.openDocument("Test.swift") + + let documentHighlight = try await project.testClient.send( + DocumentHighlightRequest(textDocument: TextDocumentIdentifier(uri), position: positions["1️⃣"]) + ) + XCTAssertEqual(documentHighlight, [DocumentHighlight(range: positions["1️⃣"]..