diff --git a/Sources/SKCore/BuildServerBuildSystem.swift b/Sources/SKCore/BuildServerBuildSystem.swift index f45dd93fa..361e1a981 100644 --- a/Sources/SKCore/BuildServerBuildSystem.swift +++ b/Sources/SKCore/BuildServerBuildSystem.swift @@ -263,7 +263,11 @@ extension BuildServerBuildSystem: BuildSystem { /// /// Returns `nil` if no build settings have been received from the build /// server yet or if no build settings are available for this file. - public func buildSettings(for document: DocumentURI, language: Language) async -> FileBuildSettings? { + public func buildSettings( + for document: DocumentURI, + in target: ConfiguredTarget, + language: Language + ) async -> FileBuildSettings? { return buildSettings[document] } @@ -271,6 +275,10 @@ extension BuildServerBuildSystem: BuildSystem { return nil } + public func configuredTargets(for document: DocumentURI) async -> [ConfiguredTarget] { + return [ConfiguredTarget(targetID: "dummy", runDestinationID: "dummy")] + } + public func registerForChangeNotifications(for uri: DocumentURI) { let request = RegisterForChanges(uri: uri, action: .register) _ = self.buildServer?.send(request) { result in diff --git a/Sources/SKCore/BuildSystem.swift b/Sources/SKCore/BuildSystem.swift index ee48db020..03b2bdad9 100644 --- a/Sources/SKCore/BuildSystem.swift +++ b/Sources/SKCore/BuildSystem.swift @@ -43,6 +43,27 @@ public struct SourceFileInfo: Sendable { } } +/// A target / run destination combination. For example, a configured target can represent building the target +/// `MyLibrary` for iOS. +public struct ConfiguredTarget: Hashable, Sendable { + /// An opaque string that represents the target. + /// + /// The target's ID should be generated by the build system that handles the target and only interpreted by that + /// build system. + public let targetID: String + + /// An opaque string that represents the run destination. + /// + /// The run destination's ID should be generated by the build system that handles the target and only interpreted by + /// that build system. + public let runDestinationID: String + + public init(targetID: String, runDestinationID: String) { + self.targetID = targetID + self.runDestinationID = runDestinationID + } +} + /// Provider of FileBuildSettings and other build-related information. /// /// The primary role of the build system is to answer queries for @@ -53,7 +74,6 @@ public struct SourceFileInfo: Sendable { /// For example, a SwiftPMWorkspace provides compiler arguments for the files /// contained in a SwiftPM package root directory. public protocol BuildSystem: AnyObject, Sendable { - /// The root of the project that this build system manages. For example, for SwiftPM packages, this is the folder /// containing Package.swift. For compilation databases it is the root folder based on which the compilation database /// was found. @@ -85,7 +105,14 @@ public protocol BuildSystem: AnyObject, Sendable { /// /// Returns `nil` if the build system can't provide build settings for this /// file or if it hasn't computed build settings for the file yet. - func buildSettings(for document: DocumentURI, language: Language) async throws -> FileBuildSettings? + func buildSettings( + for document: DocumentURI, + in target: ConfiguredTarget, + language: Language + ) async throws -> FileBuildSettings? + + /// Return the list of targets and run destinations that the given document can be built for. + func configuredTargets(for document: DocumentURI) async -> [ConfiguredTarget] /// If the build system has knowledge about the language that this document should be compiled in, return it. /// diff --git a/Sources/SKCore/BuildSystemManager.swift b/Sources/SKCore/BuildSystemManager.swift index a7100f9ac..bc83e7830 100644 --- a/Sources/SKCore/BuildSystemManager.swift +++ b/Sources/SKCore/BuildSystemManager.swift @@ -122,22 +122,53 @@ extension BuildSystemManager { } } + /// Returns the `ConfiguredTarget` that should be used for semantic functionality of the given document. + public func canonicalConfiguredTarget(for document: DocumentURI) async -> ConfiguredTarget? { + // Sort the configured targets to deterministically pick the same `ConfiguredTarget` every time. + // We could allow the user to specify a preference of one target over another. For now this is not necessary because + // no build system currently returns multiple targets for a source file. + return await buildSystem?.configuredTargets(for: document) + .sorted { ($0.targetID, $0.runDestinationID) < ($1.targetID, $1.runDestinationID) } + .first + } + + /// Returns the build settings for `document` from `buildSystem`. + /// + /// Implementation detail of `buildSettings(for:language:)`. + private func buildSettingsFromPrimaryBuildSystem( + for document: DocumentURI, + language: Language + ) async throws -> FileBuildSettings? { + guard let buildSystem else { + return nil + } + guard let target = await canonicalConfiguredTarget(for: document) else { + logger.error("Failed to get target for \(document.forLogging)") + return nil + } + // FIXME: (async) We should only wait `fallbackSettingsTimeout` for build + // settings and return fallback afterwards. I am not sure yet, how best to + // implement that with Swift concurrency. + // For now, this should be fine because all build systems return + // very quickly from `settings(for:language:)`. + guard let settings = try await buildSystem.buildSettings(for: document, in: target, language: language) else { + return nil + } + return settings + } + private func buildSettings( for document: DocumentURI, language: Language ) async -> FileBuildSettings? { do { - // FIXME: (async) We should only wait `fallbackSettingsTimeout` for build - // settings and return fallback afterwards. I am not sure yet, how best to - // implement that with Swift concurrency. - // For now, this should be fine because all build systems return - // very quickly from `settings(for:language:)`. - if let settings = try await buildSystem?.buildSettings(for: document, language: language) { - return settings + if let buildSettings = try await buildSettingsFromPrimaryBuildSystem(for: document, language: language) { + return buildSettings } } catch { logger.error("Getting build settings failed: \(error.forLogging)") } + guard var settings = fallbackBuildSystem?.buildSettings(for: document, language: language) else { return nil } diff --git a/Sources/SKCore/CompilationDatabaseBuildSystem.swift b/Sources/SKCore/CompilationDatabaseBuildSystem.swift index 031b1d10c..b8eaf03b9 100644 --- a/Sources/SKCore/CompilationDatabaseBuildSystem.swift +++ b/Sources/SKCore/CompilationDatabaseBuildSystem.swift @@ -93,14 +93,17 @@ public actor CompilationDatabaseBuildSystem { } extension CompilationDatabaseBuildSystem: BuildSystem { - public var indexDatabasePath: AbsolutePath? { indexStorePath?.parentDirectory.appending(component: "IndexDatabase") } public var indexPrefixMappings: [PathPrefixMapping] { return [] } - public func buildSettings(for document: DocumentURI, language: Language) async -> FileBuildSettings? { + public func buildSettings( + for document: DocumentURI, + in buildTarget: ConfiguredTarget, + language: Language + ) async -> FileBuildSettings? { guard let url = document.fileURL else { // We can't determine build settings for non-file URIs. return nil @@ -118,6 +121,10 @@ extension CompilationDatabaseBuildSystem: BuildSystem { return nil } + public func configuredTargets(for document: DocumentURI) async -> [ConfiguredTarget] { + return [ConfiguredTarget(targetID: "dummy", runDestinationID: "dummy")] + } + public func registerForChangeNotifications(for uri: DocumentURI) async { self.watchedFiles.insert(uri) } diff --git a/Sources/SKSupport/Process+WaitUntilExitWithCancellation.swift b/Sources/SKSupport/Process+WaitUntilExitWithCancellation.swift index aa6f1c13f..b265161e6 100644 --- a/Sources/SKSupport/Process+WaitUntilExitWithCancellation.swift +++ b/Sources/SKSupport/Process+WaitUntilExitWithCancellation.swift @@ -15,9 +15,10 @@ import Foundation import class TSCBasic.Process import struct TSCBasic.ProcessResult -public extension Process { +extension Process { /// Wait for the process to exit. If the task gets cancelled, during this time, send a `SIGINT` to the process. - func waitUntilExitSendingSigIntOnTaskCancellation() async throws -> ProcessResult { + @discardableResult + public func waitUntilExitSendingSigIntOnTaskCancellation() async throws -> ProcessResult { return try await withTaskCancellationHandler { try await waitUntilExit() } onCancel: { diff --git a/Sources/SKSwiftPMWorkspace/SwiftPMBuildSystem.swift b/Sources/SKSwiftPMWorkspace/SwiftPMBuildSystem.swift index bd33e1596..6d6f6a334 100644 --- a/Sources/SKSwiftPMWorkspace/SwiftPMBuildSystem.swift +++ b/Sources/SKSwiftPMWorkspace/SwiftPMBuildSystem.swift @@ -28,10 +28,14 @@ import Workspace import struct Basics.AbsolutePath import struct Basics.TSCAbsolutePath import struct Foundation.URL +import struct TSCBasic.AbsolutePath import protocol TSCBasic.FileSystem +import class TSCBasic.Process import var TSCBasic.localFileSystem import func TSCBasic.resolveSymlinks +typealias AbsolutePath = Basics.AbsolutePath + #if canImport(SPMBuildCore) import SPMBuildCore #endif @@ -91,9 +95,11 @@ public actor SwiftPMBuildSystem { let workspace: Workspace public let buildParameters: BuildParameters let fileSystem: FileSystem + private let toolchainRegistry: ToolchainRegistry var fileToTarget: [AbsolutePath: SwiftBuildTarget] = [:] var sourceDirToTarget: [AbsolutePath: SwiftBuildTarget] = [:] + var targets: [SwiftBuildTarget] = [] /// The URIs for which the delegate has registered for change notifications, /// mapped to the language the delegate specified when registering for change notifications. @@ -129,6 +135,7 @@ public actor SwiftPMBuildSystem { ) async throws { self.workspacePath = workspacePath self.fileSystem = fileSystem + self.toolchainRegistry = toolchainRegistry guard let packageRoot = findPackageDirectory(containing: workspacePath, fileSystem) else { throw Error.noManifest(workspacePath: workspacePath) @@ -259,6 +266,8 @@ extension SwiftPMBuildSystem { /// with only some properties modified. self.modulesGraph = modulesGraph + self.targets = try buildDescription.allTargetsInTopologicalOrder(in: modulesGraph) + self.fileToTarget = [AbsolutePath: SwiftBuildTarget]( modulesGraph.allTargets.flatMap { target in return target.sources.paths.compactMap { @@ -314,36 +323,42 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem { public var indexPrefixMappings: [PathPrefixMapping] { return [] } - public func buildSettings(for uri: DocumentURI, language: Language) throws -> FileBuildSettings? { - // SwiftPMBuildSystem doesn't respect the langue specified by the editor. - return try buildSettings(for: uri) - } - - private func buildSettings(for uri: DocumentURI) throws -> FileBuildSettings? { - guard let url = uri.fileURL else { + public func buildSettings( + for uri: DocumentURI, + in configuredTarget: ConfiguredTarget, + language: Language + ) throws -> FileBuildSettings? { + guard let url = uri.fileURL, let path = try? AbsolutePath(validating: url.path) else { // We can't determine build settings for non-file URIs. return nil } - guard let path = try? AbsolutePath(validating: url.path) else { - return nil - } - if let buildTarget = try buildTarget(for: path) { - return FileBuildSettings( - compilerArguments: try buildTarget.compileArguments(for: path.asURL), - workingDirectory: workspacePath.pathString - ) + if configuredTarget.targetID == "" { + return try settings(forPackageManifest: path) } - if path.basename == "Package.swift" { - return try settings(forPackageManifest: path) + let buildTargets = self.targets.filter({ $0.name == configuredTarget.targetID }) + if buildTargets.count > 1 { + logger.error("Found multiple targets with name \(configuredTarget.targetID). Picking the first one") + } + guard let buildTarget = buildTargets.first else { + if buildTargets.isEmpty { + logger.error("Did not find target with name \(configuredTarget.targetID)") + } + return nil } - if path.extension == "h" { - return try settings(forHeader: path) + if url.pathExtension == "h", let substituteFile = buildTarget.sources.first { + return FileBuildSettings( + compilerArguments: try buildTarget.compileArguments(for: substituteFile), + workingDirectory: workspacePath.pathString + ).patching(newFile: try resolveSymlinks(path).pathString, originalFile: substituteFile.absoluteString) } - return nil + return FileBuildSettings( + compilerArguments: try buildTarget.compileArguments(for: url), + workingDirectory: workspacePath.pathString + ) } public func defaultLanguage(for document: DocumentURI) async -> Language? { @@ -351,6 +366,29 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem { return nil } + public func configuredTargets(for uri: DocumentURI) -> [ConfiguredTarget] { + guard let url = uri.fileURL, let path = try? AbsolutePath(validating: url.path) else { + // We can't determine targets for non-file URIs. + return [] + } + + if let target = try? buildTarget(for: path) { + return [ConfiguredTarget(targetID: target.name, runDestinationID: "dummy")] + } + + if path.basename == "Package.swift" { + // We use an empty target name to represent the package manifest since an empty target name is not valid for any + // user-defined target. + return [ConfiguredTarget(targetID: "", runDestinationID: "dummy")] + } + + if url.pathExtension == "h", let target = try? target(forHeader: path) { + return [target] + } + + return [] + } + public func registerForChangeNotifications(for uri: DocumentURI) async { self.watchedFiles.insert(uri) } @@ -437,10 +475,10 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem { } public func fileHandlingCapability(for uri: DocumentURI) -> FileHandlingCapability { - if (try? buildSettings(for: uri)) != nil { - return .handled + if configuredTargets(for: uri).isEmpty { + return .unhandled } - return .unhandled + return .handled } public func sourceFiles() -> [SourceFileInfo] { @@ -485,25 +523,13 @@ extension SwiftPMBuildSystem { return canonicalPath == path ? nil : impl(canonicalPath) } - /// Retrieve settings for a given header file. - /// - /// This finds the target the header belongs to based on its location in the file system, retrieves the build settings - /// for any file within that target and generates compiler arguments by replacing that picked file with the header - /// file. - /// This is safe because all files within one target have the same build settings except for reference to the file - /// itself, which we are replacing. - private func settings(forHeader path: AbsolutePath) throws -> FileBuildSettings? { - func impl(_ path: AbsolutePath) throws -> FileBuildSettings? { + /// This finds the target the header belongs to based on its location in the file system. + private func target(forHeader path: AbsolutePath) throws -> ConfiguredTarget? { + func impl(_ path: AbsolutePath) throws -> ConfiguredTarget? { var dir = path.parentDirectory while !dir.isRoot { if let buildTarget = sourceDirToTarget[dir] { - if let sourceFile = buildTarget.sources.first { - return FileBuildSettings( - compilerArguments: try buildTarget.compileArguments(for: sourceFile), - workingDirectory: workspacePath.pathString - ).patching(newFile: path.pathString, originalFile: sourceFile.absoluteString) - } - return nil + return ConfiguredTarget(targetID: buildTarget.name, runDestinationID: "dummy") } dir = dir.parentDirectory } diff --git a/Sources/SourceKitLSP/SourceKitLSPServer.swift b/Sources/SourceKitLSP/SourceKitLSPServer.swift index 8ad25dac0..f925eb9bb 100644 --- a/Sources/SourceKitLSP/SourceKitLSPServer.swift +++ b/Sources/SourceKitLSP/SourceKitLSPServer.swift @@ -2471,7 +2471,11 @@ fileprivate extension CheckedIndex { /// If the USR has an ambiguous definition, the most important role of this function is to deterministically return /// the same result every time. func primaryDefinitionOrDeclarationOccurrence(ofUSR usr: String) -> SymbolOccurrence? { - return definitionOrDeclarationOccurrences(ofUSR: usr).sorted().first + let result = definitionOrDeclarationOccurrences(ofUSR: usr).sorted().first + if result == nil { + logger.error("Failed to find definition of \(usr) in index") + } + return result } } diff --git a/Tests/SKCoreTests/BuildSystemManagerTests.swift b/Tests/SKCoreTests/BuildSystemManagerTests.swift index 8dd89ccf7..09a201cd6 100644 --- a/Tests/SKCoreTests/BuildSystemManagerTests.swift +++ b/Tests/SKCoreTests/BuildSystemManagerTests.swift @@ -445,7 +445,7 @@ class ManualBuildSystem: BuildSystem { self.delegate = delegate } - func buildSettings(for uri: DocumentURI, language: Language) -> FileBuildSettings? { + func buildSettings(for uri: DocumentURI, in buildTarget: ConfiguredTarget, language: Language) -> FileBuildSettings? { return map[uri] } @@ -453,6 +453,10 @@ class ManualBuildSystem: BuildSystem { return nil } + public func configuredTargets(for document: DocumentURI) async -> [ConfiguredTarget] { + return [ConfiguredTarget(targetID: "dummy", runDestinationID: "dummy")] + } + func registerForChangeNotifications(for uri: DocumentURI) async { } diff --git a/Tests/SKCoreTests/CompilationDatabaseTests.swift b/Tests/SKCoreTests/CompilationDatabaseTests.swift index 3498ba425..934c1aaa7 100644 --- a/Tests/SKCoreTests/CompilationDatabaseTests.swift +++ b/Tests/SKCoreTests/CompilationDatabaseTests.swift @@ -290,6 +290,7 @@ final class CompilationDatabaseTests: XCTestCase { ) { buildSystem in let settings = await buildSystem.buildSettings( for: DocumentURI(URL(fileURLWithPath: "/a/a.swift")), + in: ConfiguredTarget(targetID: "dummy", runDestinationID: "dummy"), language: .swift ) XCTAssertNotNil(settings) diff --git a/Tests/SKSwiftPMWorkspaceTests/SwiftPMBuildSystemTests.swift b/Tests/SKSwiftPMWorkspaceTests/SwiftPMBuildSystemTests.swift index 7aed19472..900915f7e 100644 --- a/Tests/SKSwiftPMWorkspaceTests/SwiftPMBuildSystemTests.swift +++ b/Tests/SKSwiftPMWorkspaceTests/SwiftPMBuildSystemTests.swift @@ -27,8 +27,16 @@ import struct PackageModel.BuildFlags import SPMBuildCore #endif -final class SwiftPMWorkspaceTests: XCTestCase { +fileprivate extension SwiftPMBuildSystem { + func buildSettings(for uri: DocumentURI, language: Language) throws -> FileBuildSettings? { + guard let target = self.configuredTargets(for: uri).only else { + return nil + } + return try buildSettings(for: uri, in: target, language: language) + } +} +final class SwiftPMBuildSystemTests: XCTestCase { func testNoPackage() async throws { let fs = InMemoryFileSystem() try await withTestScratchDir { tempDir in diff --git a/Tests/SourceKitLSPTests/BuildSystemTests.swift b/Tests/SourceKitLSPTests/BuildSystemTests.swift index 7e43505ff..252abea30 100644 --- a/Tests/SourceKitLSPTests/BuildSystemTests.swift +++ b/Tests/SourceKitLSPTests/BuildSystemTests.swift @@ -39,7 +39,11 @@ final class TestBuildSystem: BuildSystem { /// Files currently being watched by our delegate. var watchedFiles: Set = [] - func buildSettings(for document: DocumentURI, language: Language) async throws -> FileBuildSettings? { + func buildSettings( + for document: DocumentURI, + in buildTarget: ConfiguredTarget, + language: Language + ) async throws -> FileBuildSettings? { return buildSettingsByFile[document] } @@ -47,6 +51,10 @@ final class TestBuildSystem: BuildSystem { return nil } + public func configuredTargets(for document: DocumentURI) async -> [ConfiguredTarget] { + return [ConfiguredTarget(targetID: "dummy", runDestinationID: "dummy")] + } + func registerForChangeNotifications(for uri: DocumentURI) async { watchedFiles.insert(uri) }