From 98b1294ad966d94ba39692e3bdf86ea0e2c8ab53 Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Sat, 18 Jan 2025 08:47:49 -0800 Subject: [PATCH] Support opening documents within the same workspace with sourcekitd/clangd from different toolchains rdar://142909783 --- .../BuildSystemManager.swift | 1 + .../BuildTargetIdentifierExtensions.swift | 64 ++++++- .../CompilationDatabase.swift | 8 - .../JSONCompilationDatabaseBuildSystem.swift | 66 +++++-- .../SwiftPMBuildSystem.swift | 32 +--- .../SKTestSupport/MultiFileTestProject.swift | 8 +- .../TestSourceKitLSPClient.swift | 33 ++-- .../Clang/ClangLanguageService.swift | 4 +- .../DocumentationLanguageService.swift | 2 +- Sources/SourceKitLSP/LanguageService.swift | 5 +- Sources/SourceKitLSP/SourceKitLSPServer.swift | 15 +- .../Swift/SwiftLanguageService.swift | 8 +- Sources/ToolchainRegistry/Toolchain.swift | 6 +- .../ToolchainRegistry/ToolchainRegistry.swift | 13 +- .../CompilationDatabaseTests.swift | 4 +- .../CompilationDatabaseTests.swift | 177 ++++++++++++++++++ Tests/SourceKitLSPTests/WorkspaceTests.swift | 57 +++--- .../ToolchainRegistryTests.swift | 10 +- 18 files changed, 395 insertions(+), 118 deletions(-) diff --git a/Sources/BuildSystemIntegration/BuildSystemManager.swift b/Sources/BuildSystemIntegration/BuildSystemManager.swift index a1ee51ba..65c8a063 100644 --- a/Sources/BuildSystemIntegration/BuildSystemManager.swift +++ b/Sources/BuildSystemIntegration/BuildSystemManager.swift @@ -202,6 +202,7 @@ private extension BuildSystemSpec { ) { connectionToSourceKitLSP in try JSONCompilationDatabaseBuildSystem( configPath: configPath, + toolchainRegistry: toolchainRegistry, connectionToSourceKitLSP: connectionToSourceKitLSP ) } diff --git a/Sources/BuildSystemIntegration/BuildTargetIdentifierExtensions.swift b/Sources/BuildSystemIntegration/BuildTargetIdentifierExtensions.swift index a6ac0c4f..27e9c307 100644 --- a/Sources/BuildSystemIntegration/BuildTargetIdentifierExtensions.swift +++ b/Sources/BuildSystemIntegration/BuildTargetIdentifierExtensions.swift @@ -43,9 +43,14 @@ package enum BuildDestinationIdentifier { } } +// MARK: BuildTargetIdentifier for SwiftPM + extension BuildTargetIdentifier { /// - Important: *For testing only* - package init(target: String, destination: BuildDestinationIdentifier) throws { + package static func createSwiftPM( + target: String, + destination: BuildDestinationIdentifier + ) throws -> BuildTargetIdentifier { var components = URLComponents() components.scheme = "swiftpm" components.host = "target" @@ -67,12 +72,12 @@ extension BuildTargetIdentifier { throw FailedToConvertSwiftBuildTargetToUrlError(target: target, destination: destination.id) } - self.init(uri: URI(url)) + return BuildTargetIdentifier(uri: URI(url)) } - fileprivate static let forPackageManifest = BuildTargetIdentifier(uri: try! URI(string: "swiftpm://package-manifest")) + static let forPackageManifest = BuildTargetIdentifier(uri: try! URI(string: "swiftpm://package-manifest")) - fileprivate var targetProperties: (target: String, runDestination: String) { + var swiftpmTargetProperties: (target: String, runDestination: String) { get throws { struct InvalidTargetIdentifierError: Swift.Error, CustomStringConvertible { var target: BuildTargetIdentifier @@ -96,6 +101,57 @@ extension BuildTargetIdentifier { } } +// MARK: BuildTargetIdentifier for CompileCommands + +extension BuildTargetIdentifier { + /// - Important: *For testing only* + package static func createCompileCommands(compiler: String) throws -> BuildTargetIdentifier { + var components = URLComponents() + components.scheme = "compilecommands" + components.host = "target" + components.queryItems = [URLQueryItem(name: "compiler", value: compiler)] + + struct FailedToConvertSwiftBuildTargetToUrlError: Swift.Error, CustomStringConvertible { + var compiler: String + + var description: String { + return "Failed to generate URL for compiler: \(compiler)" + } + } + + guard let url = components.url else { + throw FailedToConvertSwiftBuildTargetToUrlError(compiler: compiler) + } + + return BuildTargetIdentifier(uri: URI(url)) + } + + var compileCommandsCompiler: String { + get throws { + struct InvalidTargetIdentifierError: Swift.Error, CustomStringConvertible { + var target: BuildTargetIdentifier + + var description: String { + return "Invalid target identifier \(target)" + } + } + guard let components = URLComponents(url: self.uri.arbitrarySchemeURL, resolvingAgainstBaseURL: false) else { + throw InvalidTargetIdentifierError(target: self) + } + guard components.scheme == "compilecommands", components.host == "target" else { + throw InvalidTargetIdentifierError(target: self) + } + let compiler = components.queryItems?.last(where: { $0.name == "compiler" })?.value + + guard let compiler else { + throw InvalidTargetIdentifierError(target: self) + } + + return compiler + } + } +} + #if compiler(>=6) extension BuildTargetIdentifier: CustomLogStringConvertible { package var description: String { diff --git a/Sources/BuildSystemIntegration/CompilationDatabase.swift b/Sources/BuildSystemIntegration/CompilationDatabase.swift index 8cf4133c..e50c28d3 100644 --- a/Sources/BuildSystemIntegration/CompilationDatabase.swift +++ b/Sources/BuildSystemIntegration/CompilationDatabase.swift @@ -11,7 +11,6 @@ //===----------------------------------------------------------------------===// #if compiler(>=6) -package import BuildServerProtocol package import Foundation package import LanguageServerProtocol import LanguageServerProtocolExtensions @@ -19,7 +18,6 @@ import SKLogging import SwiftExtensions import TSCExtensions #else -import BuildServerProtocol import Foundation import LanguageServerProtocol import LanguageServerProtocolExtensions @@ -164,12 +162,6 @@ package struct JSONCompilationDatabase: Equatable, Codable { return [] } - package var sourceItems: [SourceItem] { - return commands.map { - SourceItem(uri: $0.uri, kind: .file, generated: false) - } - } - private mutating func add(_ command: CompilationDatabaseCompileCommand) { let uri = command.uri pathToCommands[uri, default: []].append(commands.count) diff --git a/Sources/BuildSystemIntegration/JSONCompilationDatabaseBuildSystem.swift b/Sources/BuildSystemIntegration/JSONCompilationDatabaseBuildSystem.swift index 67878563..a111f76d 100644 --- a/Sources/BuildSystemIntegration/JSONCompilationDatabaseBuildSystem.swift +++ b/Sources/BuildSystemIntegration/JSONCompilationDatabaseBuildSystem.swift @@ -17,12 +17,27 @@ import SwiftExtensions package import BuildServerProtocol package import Foundation package import LanguageServerProtocol +package import ToolchainRegistry #else import BuildServerProtocol import Foundation import LanguageServerProtocol +import ToolchainRegistry #endif +fileprivate extension CompilationDatabaseCompileCommand { + /// The first entry in the command line identifies the compiler that should be used to compile the file and can thus + /// be used to infer the toolchain. + /// + /// Note that this compiler does not necessarily need to exist on disk. Eg. tools may just use `clang` as the compiler + /// without specifying a path. + /// + /// The absence of a compiler means we have an empty command line, which should never happen. + var compiler: String? { + return commandLine.first + } +} + /// A `BuildSystem` that provides compiler arguments from a `compile_commands.json` file. package actor JSONCompilationDatabaseBuildSystem: BuiltInBuildSystem { package static let dbName: String = "compile_commands.json" @@ -36,6 +51,8 @@ package actor JSONCompilationDatabaseBuildSystem: BuiltInBuildSystem { } } + private let toolchainRegistry: ToolchainRegistry + private let connectionToSourceKitLSP: any Connection package let configPath: URL @@ -73,34 +90,55 @@ package actor JSONCompilationDatabaseBuildSystem: BuiltInBuildSystem { package init( configPath: URL, + toolchainRegistry: ToolchainRegistry, connectionToSourceKitLSP: any Connection ) throws { self.compdb = try JSONCompilationDatabase(file: configPath) + self.toolchainRegistry = toolchainRegistry self.connectionToSourceKitLSP = connectionToSourceKitLSP - self.configPath = configPath } package func buildTargets(request: WorkspaceBuildTargetsRequest) async throws -> WorkspaceBuildTargetsResponse { - return WorkspaceBuildTargetsResponse(targets: [ - BuildTarget( - id: .dummy, + let compilers = Set(compdb.commands.compactMap(\.compiler)).sorted { $0 < $1 } + let targets = try await compilers.asyncMap { compiler in + let toolchainUri: URI? = + if let toolchainPath = await toolchainRegistry.toolchain(withCompiler: URL(fileURLWithPath: compiler))?.path { + URI(toolchainPath) + } else { + nil + } + return BuildTarget( + id: try BuildTargetIdentifier.createCompileCommands(compiler: compiler), displayName: nil, baseDirectory: nil, tags: [.test], capabilities: BuildTargetCapabilities(), // Be conservative with the languages that might be used in the target. SourceKit-LSP doesn't use this property. languageIds: [.c, .cpp, .objective_c, .objective_cpp, .swift], - dependencies: [] + dependencies: [], + dataKind: .sourceKit, + data: SourceKitBuildTarget(toolchain: toolchainUri).encodeToLSPAny() ) - ]) + } + return WorkspaceBuildTargetsResponse(targets: targets) } package func buildTargetSources(request: BuildTargetSourcesRequest) async throws -> BuildTargetSourcesResponse { - guard request.targets.contains(.dummy) else { - return BuildTargetSourcesResponse(items: []) + let items = request.targets.compactMap { (target) -> SourcesItem? in + guard let targetCompiler = orLog("Compiler for target", { try target.compileCommandsCompiler }) else { + return nil + } + let commandsWithRequestedCompilers = compdb.commands.lazy.filter { command in + return targetCompiler == command.compiler + } + let sources = commandsWithRequestedCompilers.map { + SourceItem(uri: $0.uri, kind: .file, generated: false) + } + return SourcesItem(target: target, sources: Array(sources)) } - return BuildTargetSourcesResponse(items: [SourcesItem(target: .dummy, sources: compdb.sourceItems)]) + + return BuildTargetSourcesResponse(items: items) } package func didChangeWatchedFiles(notification: OnWatchedFilesDidChangeNotification) { @@ -116,12 +154,16 @@ package actor JSONCompilationDatabaseBuildSystem: BuiltInBuildSystem { package func sourceKitOptions( request: TextDocumentSourceKitOptionsRequest ) async throws -> TextDocumentSourceKitOptionsResponse? { - guard let cmd = compdb[request.textDocument.uri].first else { + let targetCompiler = try request.target.compileCommandsCompiler + let command = compdb[request.textDocument.uri].filter { + $0.compiler == targetCompiler + }.first + guard let command else { return nil } return TextDocumentSourceKitOptionsResponse( - compilerArguments: Array(cmd.commandLine.dropFirst()), - workingDirectory: cmd.directory + compilerArguments: Array(command.commandLine.dropFirst()), + workingDirectory: command.directory ) } diff --git a/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift b/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift index 358ac637..db9fc815 100644 --- a/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift +++ b/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift @@ -75,32 +75,10 @@ extension BuildDestinationIdentifier { extension BuildTargetIdentifier { fileprivate init(_ buildTarget: any SwiftBuildTarget) throws { - try self.init(target: buildTarget.name, destination: BuildDestinationIdentifier(buildTarget.destination)) - } - - fileprivate static let forPackageManifest = BuildTargetIdentifier(uri: try! URI(string: "swiftpm://package-manifest")) - - fileprivate var targetProperties: (target: String, runDestination: String) { - get throws { - struct InvalidTargetIdentifierError: Swift.Error, CustomStringConvertible { - var target: BuildTargetIdentifier - - var description: String { - return "Invalid target identifier \(target)" - } - } - guard let components = URLComponents(url: self.uri.arbitrarySchemeURL, resolvingAgainstBaseURL: false) else { - throw InvalidTargetIdentifierError(target: self) - } - let target = components.queryItems?.last(where: { $0.name == "target" })?.value - let runDestination = components.queryItems?.last(where: { $0.name == "destination" })?.value - - guard let target, let runDestination else { - throw InvalidTargetIdentifierError(target: self) - } - - return (target, runDestination) - } + self = try Self.createSwiftPM( + target: buildTarget.name, + destination: BuildDestinationIdentifier(buildTarget.destination) + ) } } @@ -608,7 +586,7 @@ package actor SwiftPMBuildSystem: BuiltInBuildSystem { "--package-path", try projectRoot.filePath, "--scratch-path", self.swiftPMWorkspace.location.scratchDirectory.pathString, "--disable-index-store", - "--target", try target.targetProperties.target, + "--target", try target.swiftpmTargetProperties.target, ] if options.swiftPMOrDefault.disableSandbox ?? false { arguments += ["--disable-sandbox"] diff --git a/Sources/SKTestSupport/MultiFileTestProject.swift b/Sources/SKTestSupport/MultiFileTestProject.swift index 853670ce..05f145fc 100644 --- a/Sources/SKTestSupport/MultiFileTestProject.swift +++ b/Sources/SKTestSupport/MultiFileTestProject.swift @@ -10,18 +10,20 @@ // //===----------------------------------------------------------------------===// +import SwiftExtensions + #if compiler(>=6) package import Foundation package import LanguageServerProtocol package import SKOptions package import SourceKitLSP -import SwiftExtensions +package import ToolchainRegistry #else import Foundation import LanguageServerProtocol import SKOptions import SourceKitLSP -import SwiftExtensions +import ToolchainRegistry #endif /// The location of a test file within test workspace. @@ -137,6 +139,7 @@ package class MultiFileTestProject { initializationOptions: LSPAny? = nil, capabilities: ClientCapabilities = ClientCapabilities(), options: SourceKitLSPOptions = .testDefault(), + toolchainRegistry: ToolchainRegistry = .forTesting, hooks: Hooks = Hooks(), enableBackgroundIndexing: Bool = false, usePullDiagnostics: Bool = true, @@ -152,6 +155,7 @@ package class MultiFileTestProject { hooks: hooks, initializationOptions: initializationOptions, capabilities: capabilities, + toolchainRegistry: toolchainRegistry, usePullDiagnostics: usePullDiagnostics, enableBackgroundIndexing: enableBackgroundIndexing, workspaceFolders: workspaces(scratchDirectory), diff --git a/Sources/SKTestSupport/TestSourceKitLSPClient.swift b/Sources/SKTestSupport/TestSourceKitLSPClient.swift index 3b1ee898..5ab632fc 100644 --- a/Sources/SKTestSupport/TestSourceKitLSPClient.swift +++ b/Sources/SKTestSupport/TestSourceKitLSPClient.swift @@ -10,34 +10,26 @@ // //===----------------------------------------------------------------------===// +import Foundation +import InProcessClient +import LanguageServerProtocolExtensions +import LanguageServerProtocolJSONRPC +import SKUtilities +import SourceKitD +import SwiftExtensions +import SwiftSyntax +import XCTest + #if compiler(>=6) -import Foundation -import InProcessClient package import LanguageServerProtocol -import LanguageServerProtocolJSONRPC -import LanguageServerProtocolExtensions package import SKOptions -import SKUtilities -import SourceKitD package import SourceKitLSP -import SwiftExtensions -import SwiftSyntax -import ToolchainRegistry -import XCTest +package import ToolchainRegistry #else -import Foundation -import InProcessClient import LanguageServerProtocol -import LanguageServerProtocolJSONRPC -import LanguageServerProtocolExtensions import SKOptions -import SKUtilities -import SourceKitD import SourceKitLSP -import SwiftExtensions -import SwiftSyntax import ToolchainRegistry -import XCTest #endif extension SourceKitLSPOptions { @@ -150,6 +142,7 @@ package final class TestSourceKitLSPClient: MessageHandler, Sendable { initialize: Bool = true, initializationOptions: LSPAny? = nil, capabilities: ClientCapabilities = ClientCapabilities(), + toolchainRegistry: ToolchainRegistry = .forTesting, usePullDiagnostics: Bool = true, enableBackgroundIndexing: Bool = false, workspaceFolders: [WorkspaceFolder]? = nil, @@ -175,7 +168,7 @@ package final class TestSourceKitLSPClient: MessageHandler, Sendable { self.serverToClientConnection = serverToClientConnection server = SourceKitLSPServer( client: serverToClientConnection, - toolchainRegistry: ToolchainRegistry.forTesting, + toolchainRegistry: toolchainRegistry, options: options, hooks: hooks, onExit: { diff --git a/Sources/SourceKitLSP/Clang/ClangLanguageService.swift b/Sources/SourceKitLSP/Clang/ClangLanguageService.swift index 4271a3f8..6e0f0ca5 100644 --- a/Sources/SourceKitLSP/Clang/ClangLanguageService.swift +++ b/Sources/SourceKitLSP/Clang/ClangLanguageService.swift @@ -144,9 +144,9 @@ actor ClangLanguageService: LanguageService, MessageHandler { return ClangBuildSettings(settings, clangPath: clangPath) } - nonisolated func canHandle(workspace: Workspace) -> Bool { + nonisolated func canHandle(workspace: Workspace, toolchain: Toolchain) -> Bool { // We launch different clangd instance for each workspace because clangd doesn't have multi-root workspace support. - return workspace === self.workspace.value + return workspace === self.workspace.value && self.clangdPath == toolchain.clangd } func addStateChangeHandler(handler: @escaping (LanguageServerState, LanguageServerState) -> Void) { diff --git a/Sources/SourceKitLSP/Documentation/DocumentationLanguageService.swift b/Sources/SourceKitLSP/Documentation/DocumentationLanguageService.swift index 0803acf4..3de930cf 100644 --- a/Sources/SourceKitLSP/Documentation/DocumentationLanguageService.swift +++ b/Sources/SourceKitLSP/Documentation/DocumentationLanguageService.swift @@ -33,7 +33,7 @@ package actor DocumentationLanguageService: LanguageService, Sendable { workspace: Workspace ) async throws {} - package nonisolated func canHandle(workspace: Workspace) -> Bool { + package nonisolated func canHandle(workspace: Workspace, toolchain: Toolchain) -> Bool { return true } diff --git a/Sources/SourceKitLSP/LanguageService.swift b/Sources/SourceKitLSP/LanguageService.swift index 643c531f..30bb5773 100644 --- a/Sources/SourceKitLSP/LanguageService.swift +++ b/Sources/SourceKitLSP/LanguageService.swift @@ -113,10 +113,11 @@ package protocol LanguageService: AnyObject, Sendable { workspace: Workspace ) async throws - /// Returns `true` if this instance of the language server can handle opening documents in `workspace`. + /// Returns `true` if this instance of the language server can handle documents in `workspace` using the given + /// toolchain. /// /// If this returns `false`, a new language server will be started for `workspace`. - func canHandle(workspace: Workspace) -> Bool + func canHandle(workspace: Workspace, toolchain: Toolchain) -> Bool // MARK: - Lifetime diff --git a/Sources/SourceKitLSP/SourceKitLSPServer.swift b/Sources/SourceKitLSP/SourceKitLSPServer.swift index 05ae24aa..d0d3e5f9 100644 --- a/Sources/SourceKitLSP/SourceKitLSPServer.swift +++ b/Sources/SourceKitLSP/SourceKitLSPServer.swift @@ -433,14 +433,15 @@ package actor SourceKitLSPServer { } } - /// If a language service of type `serverType` that can handle `workspace` has - /// already been started, return it, otherwise return `nil`. + /// If a language service of type `serverType` that can handle `workspace` using the given toolchain has already been + /// started, return it, otherwise return `nil`. private func existingLanguageService( _ serverType: LanguageServerType, + toolchain: Toolchain, workspace: Workspace ) -> LanguageService? { for languageService in languageServices[serverType, default: []] { - if languageService.canHandle(workspace: workspace) { + if languageService.canHandle(workspace: workspace, toolchain: toolchain) { return languageService } } @@ -456,7 +457,7 @@ package actor SourceKitLSPServer { return nil } // Pick the first language service that can handle this workspace. - if let languageService = existingLanguageService(serverType, workspace: workspace) { + if let languageService = existingLanguageService(serverType, toolchain: toolchain, workspace: workspace) { return languageService } @@ -508,7 +509,11 @@ package actor SourceKitLSPServer { await service.clientInitialized(InitializedNotification()) - if let concurrentlyInitializedService = existingLanguageService(serverType, workspace: workspace) { + if let concurrentlyInitializedService = existingLanguageService( + serverType, + toolchain: toolchain, + workspace: workspace + ) { // Since we 'await' above, another call to languageService might have // happened concurrently, passed the `existingLanguageService` check at // the top and started initializing another language service. diff --git a/Sources/SourceKitLSP/Swift/SwiftLanguageService.swift b/Sources/SourceKitLSP/Swift/SwiftLanguageService.swift index 56a1cfb3..30f04897 100644 --- a/Sources/SourceKitLSP/Swift/SwiftLanguageService.swift +++ b/Sources/SourceKitLSP/Swift/SwiftLanguageService.swift @@ -118,6 +118,8 @@ package actor SwiftLanguageService: LanguageService, Sendable { /// The ``SourceKitLSPServer`` instance that created this `SwiftLanguageService`. private(set) weak var sourceKitLSPServer: SourceKitLSPServer? + private let sourcekitdPath: URL + let sourcekitd: SourceKitD /// Path to the swift-format executable if it exists in the toolchain. @@ -221,6 +223,7 @@ package actor SwiftLanguageService: LanguageService, Sendable { workspace: Workspace ) async throws { guard let sourcekitd = toolchain.sourcekitd else { return nil } + self.sourcekitdPath = sourcekitd self.sourceKitLSPServer = sourceKitLSPServer self.swiftFormat = toolchain.swiftFormat let pluginPaths: PluginPaths? @@ -321,9 +324,8 @@ package actor SwiftLanguageService: LanguageService, Sendable { ) } - package nonisolated func canHandle(workspace: Workspace) -> Bool { - // We have a single sourcekitd instance for all workspaces. - return true + package nonisolated func canHandle(workspace: Workspace, toolchain: Toolchain) -> Bool { + return self.sourcekitdPath == toolchain.sourcekitd } private func setState(_ newState: LanguageServerState) async { diff --git a/Sources/ToolchainRegistry/Toolchain.swift b/Sources/ToolchainRegistry/Toolchain.swift index 4a316e5e..f13ba5fd 100644 --- a/Sources/ToolchainRegistry/Toolchain.swift +++ b/Sources/ToolchainRegistry/Toolchain.swift @@ -146,7 +146,7 @@ public final class Toolchain: Sendable { package init( identifier: String, displayName: String, - path: URL? = nil, + path: URL, clang: URL? = nil, swift: URL? = nil, swiftc: URL? = nil, @@ -191,9 +191,7 @@ public final class Toolchain: Sendable { func isProperSuperset(of other: Toolchain) -> Bool { return self.isSuperset(of: other) && !other.isSuperset(of: self) } -} -extension Toolchain { /// Create a toolchain for the given path, if it contains at least one tool, otherwise return nil. /// /// This initializer looks for a toolchain using the following basic layout: @@ -218,7 +216,7 @@ extension Toolchain { // Properties that need to be initialized let identifier: String let displayName: String - let toolchainPath: URL? + let toolchainPath: URL var clang: URL? = nil var clangd: URL? = nil var swift: URL? = nil diff --git a/Sources/ToolchainRegistry/ToolchainRegistry.swift b/Sources/ToolchainRegistry/ToolchainRegistry.swift index d4429600..1bcc32e3 100644 --- a/Sources/ToolchainRegistry/ToolchainRegistry.swift +++ b/Sources/ToolchainRegistry/ToolchainRegistry.swift @@ -78,7 +78,7 @@ package final actor ToolchainRegistry { /// Create a toolchain registry with a pre-defined list of toolchains. /// /// For testing purposes only. - public init(toolchains: [Toolchain]) { + package init(toolchains: [Toolchain]) { self.init( toolchainsAndReasons: toolchains.map { ($0, .xcode) }, darwinToolchainOverride: nil @@ -263,6 +263,17 @@ package final actor ToolchainRegistry { return nil } + + /// If we have a toolchain in the toolchain registry that contains the compiler with the given URL, return it. + /// Otherwise, return `nil`. + package func toolchain(withCompiler compiler: URL) -> Toolchain? { + for toolchain in toolchains { + if [toolchain.clang, toolchain.swift, toolchain.swiftc].contains(compiler) { + return toolchain + } + } + return nil + } } /// Inspecting internal state for testing purposes. diff --git a/Tests/BuildSystemIntegrationTests/CompilationDatabaseTests.swift b/Tests/BuildSystemIntegrationTests/CompilationDatabaseTests.swift index 883a0480..c6db200a 100644 --- a/Tests/BuildSystemIntegrationTests/CompilationDatabaseTests.swift +++ b/Tests/BuildSystemIntegrationTests/CompilationDatabaseTests.swift @@ -17,6 +17,7 @@ import LanguageServerProtocolExtensions import SKTestSupport import SwiftExtensions import TSCExtensions +import ToolchainRegistry import XCTest import struct TSCBasic.RelativePath @@ -269,7 +270,7 @@ final class CompilationDatabaseTests: XCTestCase { let settings = try await buildSystem.sourceKitOptions( request: TextDocumentSourceKitOptionsRequest( textDocument: TextDocumentIdentifier(DocumentURI(URL(fileURLWithPath: "/a/a.swift"))), - target: BuildTargetIdentifier.dummy, + target: BuildTargetIdentifier.createCompileCommands(compiler: "swiftc"), language: .swift ) ) @@ -439,6 +440,7 @@ private func checkCompilationDatabaseBuildSystem( try compdb.write(to: configPath, atomically: true, encoding: .utf8) let buildSystem = try JSONCompilationDatabaseBuildSystem( configPath: configPath, + toolchainRegistry: .forTesting, connectionToSourceKitLSP: LocalConnection(receiverName: "Dummy SourceKit-LSP") ) try await block(XCTUnwrap(buildSystem)) diff --git a/Tests/SourceKitLSPTests/CompilationDatabaseTests.swift b/Tests/SourceKitLSPTests/CompilationDatabaseTests.swift index 11cb7b7c..a44bf282 100644 --- a/Tests/SourceKitLSPTests/CompilationDatabaseTests.swift +++ b/Tests/SourceKitLSPTests/CompilationDatabaseTests.swift @@ -15,6 +15,7 @@ import Foundation import LanguageServerProtocol import SKTestSupport import TSCBasic +import ToolchainRegistry import XCTest final class CompilationDatabaseTests: XCTestCase { @@ -90,4 +91,180 @@ final class CompilationDatabaseTests: XCTestCase { XCTAssert(didReceiveCorrectHighlight) } + + func testJSONCompilationDatabaseWithDifferentToolchainsForSwift() async throws { + let dummyToolchain = Toolchain( + identifier: "dummy", + displayName: "dummy", + path: URL(fileURLWithPath: "/dummy"), + clang: nil, + swift: URL(fileURLWithPath: "/dummy/usr/bin/swift"), + swiftc: URL(fileURLWithPath: "/dummy/usr/bin/swiftc"), + swiftFormat: nil, + clangd: nil, + sourcekitd: URL(fileURLWithPath: "/dummy/usr/lib/sourcekitd.framework/sourcekitd"), + libIndexStore: nil + ) + let toolchainRegistry = ToolchainRegistry(toolchains: [ + try await unwrap(ToolchainRegistry.forTesting.default), dummyToolchain, + ]) + + let project = try await MultiFileTestProject( + files: [ + "testFromDefaultToolchain.swift": """ + #warning("Test warning") + """, + "testFromDummyToolchain.swift": """ + #warning("Test warning") + """, + "compile_commands.json": """ + [ + { + "directory": "$TEST_DIR_BACKSLASH_ESCAPED", + "arguments": [ + "swiftc", + "$TEST_DIR_BACKSLASH_ESCAPED/testFromDefaultToolchain.swift", + \(defaultSDKArgs) + ], + "file": "testFromDefaultToolchain.swift", + "output": "$TEST_DIR_BACKSLASH_ESCAPED/testFromDefaultToolchain.swift.o" + }, + { + "directory": "$TEST_DIR_BACKSLASH_ESCAPED", + "arguments": [ + "/dummy/usr/bin/swiftc", + "$TEST_DIR_BACKSLASH_ESCAPED/testFromDummyToolchain.swift", + \(defaultSDKArgs) + ], + "file": "testFromDummyToolchain.swift", + "output": "$TEST_DIR_BACKSLASH_ESCAPED/testFromDummyToolchain.swift.o" + } + ] + """, + ], + toolchainRegistry: toolchainRegistry + ) + + // We should be able to provide semantic functionality for `testFromDefaultToolchain` because we open it using the + // default toolchain. + + let (forDefaultToolchainUri, _) = try project.openDocument("testFromDefaultToolchain.swift") + let diagnostics = try await project.testClient.send( + DocumentDiagnosticsRequest(textDocument: TextDocumentIdentifier(forDefaultToolchainUri)) + ) + XCTAssertEqual( + diagnostics.fullReport?.items.map(\.message), + ["Test warning"] + ) + + // But for `testFromDummyToolchain.swift`, we can't launch sourcekitd (because it doesn't exist, we just provided a + // dummy), so we should receive an error. The exact error here is not super relevant, the important part is that we + // apparently tried to launch a different sourcekitd. + let (forDummyToolchainUri, _) = try project.openDocument("testFromDummyToolchain.swift") + await assertThrowsError( + try await project.testClient.send( + DocumentDiagnosticsRequest(textDocument: TextDocumentIdentifier(forDummyToolchainUri)) + ) + ) { error in + guard let error = error as? ResponseError else { + XCTFail("Expected ResponseError, got \(error)") + return + } + XCTAssert(error.message.contains("No language service")) + } + } + + func testJSONCompilationDatabaseWithDifferentToolchainsForClang() async throws { + let dummyToolchain = Toolchain( + identifier: "dummy", + displayName: "dummy", + path: URL(fileURLWithPath: "/dummy"), + clang: URL(fileURLWithPath: "/dummy/usr/bin/clang"), + swift: nil, + swiftc: nil, + swiftFormat: nil, + clangd: URL(fileURLWithPath: "/dummy/usr/bin/clangd"), + sourcekitd: nil, + libIndexStore: nil + ) + let toolchainRegistry = ToolchainRegistry(toolchains: [ + try await unwrap(ToolchainRegistry.forTesting.default), dummyToolchain, + ]) + + let project = try await MultiFileTestProject( + files: [ + "testFromDefaultToolchain.c": """ + void 1️⃣main() {} + """, + "testFromDummyToolchain.c": """ + void 2️⃣main() {} + """, + "compile_commands.json": """ + [ + { + "directory": "$TEST_DIR_BACKSLASH_ESCAPED", + "arguments": [ + "clang", + "$TEST_DIR_BACKSLASH_ESCAPED/testFromDefaultToolchain.c" + ], + "file": "testFromDefaultToolchain.c", + "output": "$TEST_DIR_BACKSLASH_ESCAPED/testFromDefaultToolchain.o" + }, + { + "directory": "$TEST_DIR_BACKSLASH_ESCAPED", + "arguments": [ + "/dummy/usr/bin/clang", + "$TEST_DIR_BACKSLASH_ESCAPED/testFromDummyToolchain.c" + ], + "file": "testFromDummyToolchain.c", + "output": "$TEST_DIR_BACKSLASH_ESCAPED/testFromDummyToolchain.o" + } + ] + """, + ], + toolchainRegistry: toolchainRegistry + ) + + // We should be able to provide semantic functionality for `testFromDefaultToolchain` because we open it using the + // default toolchain. + + let (forDefaultToolchainUri, forDefaultToolchainPositions) = try project.openDocument("testFromDefaultToolchain.c") + let hover = try await project.testClient.send( + HoverRequest( + textDocument: TextDocumentIdentifier(forDefaultToolchainUri), + position: forDefaultToolchainPositions["1️⃣"] + ) + ) + let hoverContent = try XCTUnwrap(hover?.contents.markupContent?.value) + XCTAssert(hoverContent.contains("void main()")) + + // But for `testFromDummyToolchain.swift`, we can't launch sourcekitd (because it doesn't exist, we just provided a + // dummy), so we should receive an error. The exact error here is not super relevant, the important part is that we + // apparently tried to launch a different sourcekitd. + let (forDummyToolchainUri, forDummyToolchainPositions) = try project.openDocument("testFromDummyToolchain.c") + await assertThrowsError( + try await project.testClient.send( + HoverRequest( + textDocument: TextDocumentIdentifier(forDummyToolchainUri), + position: forDummyToolchainPositions["2️⃣"] + ) + ) + ) { error in + guard let error = error as? ResponseError else { + XCTFail("Expected ResponseError, got \(error)") + return + } + XCTAssert(error.message.contains("No language service")) + } + } } + +fileprivate let defaultSDKArgs: String = { + if let defaultSDKPath { + let escapedPath = defaultSDKPath.replacing(#"\"#, with: #"\\"#) + return """ + "-sdk", "\(escapedPath)" + """ + } + return "" +}() diff --git a/Tests/SourceKitLSPTests/WorkspaceTests.swift b/Tests/SourceKitLSPTests/WorkspaceTests.swift index f247987b..fd3e2bd9 100644 --- a/Tests/SourceKitLSPTests/WorkspaceTests.swift +++ b/Tests/SourceKitLSPTests/WorkspaceTests.swift @@ -984,12 +984,17 @@ final class WorkspaceTests: XCTestCase { "Sources/MyLib/Bar.swift": "", "build/compile_commands.json": """ [ - { - "directory": "$TEST_DIR_BACKSLASH_ESCAPED", - "command": "swiftc $TEST_DIR_BACKSLASH_ESCAPED/src/Foo.swift -DHAVE_SETTINGS \(defaultSDKArgs)", - "file": "src/Foo.swift", - "output": "$TEST_DIR_BACKSLASH_ESCAPED/build/Foo.swift.o" - } + { + "directory": "$TEST_DIR_BACKSLASH_ESCAPED", + "arguments": [ + "swiftc", + "$TEST_DIR_BACKSLASH_ESCAPED/src/Foo.swift", + \(defaultSDKArgs) + "-DHAVE_SETTINGS" + ], + "file": "src/Foo.swift", + "output": "$TEST_DIR_BACKSLASH_ESCAPED/build/Foo.swift.o" + } ] """, "Package.swift": """ @@ -1030,12 +1035,17 @@ final class WorkspaceTests: XCTestCase { "projA/Sources/MyLib/Bar.swift": "", "projA/build/compile_commands.json": """ [ - { - "directory": "$TEST_DIR_BACKSLASH_ESCAPED/projA", - "command": "swiftc $TEST_DIR_BACKSLASH_ESCAPED/projA/src/Foo.swift -DHAVE_SETTINGS \(defaultSDKArgs)", - "file": "src/Foo.swift", - "output": "$TEST_DIR_BACKSLASH_ESCAPED/projA/build/Foo.swift.o" - } + { + "directory": "$TEST_DIR_BACKSLASH_ESCAPED/projA", + "arguments": [ + "swiftc", + "$TEST_DIR_BACKSLASH_ESCAPED/projA/src/Foo.swift", + \(defaultSDKArgs) + "-DHAVE_SETTINGS" + ], + "file": "src/Foo.swift", + "output": "$TEST_DIR_BACKSLASH_ESCAPED/projA/build/Foo.swift.o" + } ] """, "projA/Package.swift": """ @@ -1077,12 +1087,17 @@ final class WorkspaceTests: XCTestCase { """, "otherbuild/compile_commands.json": """ [ - { - "directory": "$TEST_DIR_BACKSLASH_ESCAPED", - "command": "swiftc $TEST_DIR_BACKSLASH_ESCAPED/src/Foo.swift -DHAVE_SETTINGS \(defaultSDKArgs)", - "file": "src/Foo.swift", - "output": "$TEST_DIR_BACKSLASH_ESCAPED/otherbuild/Foo.swift.o" - } + { + "directory": "$TEST_DIR_BACKSLASH_ESCAPED", + "arguments": [ + "swiftc", + "$TEST_DIR_BACKSLASH_ESCAPED/src/Foo.swift", + \(defaultSDKArgs) + "-DHAVE_SETTINGS" + ], + "file": "src/Foo.swift", + "output": "$TEST_DIR_BACKSLASH_ESCAPED/otherbuild/Foo.swift.o" + } ] """, ]) @@ -1112,12 +1127,10 @@ final class WorkspaceTests: XCTestCase { fileprivate let defaultSDKArgs: String = { if let defaultSDKPath { - let escapedPath = defaultSDKPath + let escapedPath = defaultSDKPath.replacing(#"\"#, with: #"\\"#) return """ - -sdk "\(escapedPath)" + "-sdk", "\(escapedPath)", """ - .replacing(#"\"#, with: #"\\"#) - .replacing("\"", with: #"\""#) } return "" }() diff --git a/Tests/ToolchainRegistryTests/ToolchainRegistryTests.swift b/Tests/ToolchainRegistryTests/ToolchainRegistryTests.swift index 521eec25..929add58 100644 --- a/Tests/ToolchainRegistryTests/ToolchainRegistryTests.swift +++ b/Tests/ToolchainRegistryTests/ToolchainRegistryTests.swift @@ -23,15 +23,17 @@ import Android final class ToolchainRegistryTests: XCTestCase { func testDefaultSingleToolchain() async throws { - let tr = ToolchainRegistry(toolchains: [Toolchain(identifier: "a", displayName: "a", path: nil)]) + let tr = ToolchainRegistry(toolchains: [ + Toolchain(identifier: "a", displayName: "a", path: URL(fileURLWithPath: "/dummy")) + ]) await assertEqual(tr.default?.identifier, "a") } func testDefaultTwoToolchains() async throws { let tr = ToolchainRegistry( toolchains: [ - Toolchain(identifier: "a", displayName: "a", path: nil), - Toolchain(identifier: "b", displayName: "b", path: nil), + Toolchain(identifier: "a", displayName: "a", path: URL(fileURLWithPath: "/dummy")), + Toolchain(identifier: "b", displayName: "b", path: URL(fileURLWithPath: "/dummy")), ] ) await assertEqual(tr.default?.identifier, "a") @@ -517,7 +519,7 @@ final class ToolchainRegistryTests: XCTestCase { } func testDuplicateToolchainOnlyRegisteredOnce() async throws { - let toolchain = Toolchain(identifier: "a", displayName: "a", path: nil) + let toolchain = Toolchain(identifier: "a", displayName: "a", path: URL(fileURLWithPath: "/dummy")) let tr = ToolchainRegistry(toolchains: [toolchain, toolchain]) assertEqual(await tr.toolchains.count, 1) }