diff --git a/Documentation/Configuration File.md b/Documentation/Configuration File.md index 48b84eb30..ae96a1768 100644 --- a/Documentation/Configuration File.md +++ b/Documentation/Configuration File.md @@ -43,7 +43,7 @@ The structure of the file is currently not guaranteed to be stable. Options may - `logLevel: "debug"|"info"|"default"|"error"|"fault"`: The level from which one onwards log messages should be written. - `privacyLevel: "public"|"private"|"sensitive"`: Whether potentially sensitive information should be redacted. Default is `public`, which redacts potentially sensitive information. - `inputMirrorDirectory: string`: Write all input received by SourceKit-LSP on stdin to a file in this directory. Useful to record and replay an entire SourceKit-LSP session. -- `defaultWorkspaceType: "buildserver"|"compdb"|"swiftpm"`: Overrides workspace type selection logic. +- `defaultWorkspaceType: "swiftPM"|"compilationDatabase"|"buildServer"`: Overrides workspace type selection logic. - `generatedFilesPath: string`: Directory in which generated interfaces and macro expansions should be stored. - `backgroundIndexing: bool`: Explicitly enable or disable background indexing. - `backgroundPreparationMode: "build"|"noLazy"|"enabled"`: Determines how background indexing should prepare a target. Possible values are: diff --git a/Sources/BuildSystemIntegration/DetermineBuildSystem.swift b/Sources/BuildSystemIntegration/DetermineBuildSystem.swift index fabf33783..d2dae51b2 100644 --- a/Sources/BuildSystemIntegration/DetermineBuildSystem.swift +++ b/Sources/BuildSystemIntegration/DetermineBuildSystem.swift @@ -31,11 +31,13 @@ import struct TSCBasic.AbsolutePath #endif /// Determine which build system should be started to handle the given workspace folder and at which folder that build -/// system's project root is (see `BuiltInBuildSystem.projectRoot(for:options:)`). +/// system's project root is (see `BuiltInBuildSystem.projectRoot(for:options:)`). `onlyConsiderRoot` controls whether +/// paths outside the root should be considered (eg. configuration files in the user's home directory). /// /// Returns `nil` if no build system can handle this workspace folder. package func determineBuildSystem( forWorkspaceFolder workspaceFolder: DocumentURI, + onlyConsiderRoot: Bool, options: SourceKitLSPOptions ) -> BuildSystemSpec? { var buildSystemPreference: [WorkspaceType] = [ diff --git a/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift b/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift index 50a8e963a..d5d2ac6ad 100644 --- a/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift +++ b/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift @@ -230,17 +230,12 @@ package actor SwiftPMBuildSystem: BuiltInBuildSystem { guard var path = orLog("Getting realpath for project root", { try path.realpath }) else { return nil } - while true { - let packagePath = path.appendingPathComponent("Package.swift") - if (try? String(contentsOf: packagePath, encoding: .utf8))?.contains("PackageDescription") ?? false { - return path - } - if (try? AbsolutePath(validating: path.filePath))?.isRoot ?? true { - break - } - path.deleteLastPathComponent() + let packagePath = path.appendingPathComponent("Package.swift") + if (try? String(contentsOf: packagePath, encoding: .utf8))?.contains("PackageDescription") ?? false { + return path } + return nil } diff --git a/Sources/SKOptions/SourceKitLSPOptions.swift b/Sources/SKOptions/SourceKitLSPOptions.swift index 124452586..aa171d52f 100644 --- a/Sources/SKOptions/SourceKitLSPOptions.swift +++ b/Sources/SKOptions/SourceKitLSPOptions.swift @@ -280,7 +280,7 @@ public struct SourceKitLSPOptions: Sendable, Codable, Equatable { set { logging = newValue } } - /// Default workspace type (buildserver|compdb|swiftpm). Overrides workspace type selection logic. + /// Default workspace type (swiftPM|compilationDatabase|buildServer). Overrides workspace type selection logic. public var defaultWorkspaceType: WorkspaceType? public var generatedFilesPath: String? @@ -444,6 +444,17 @@ public struct SourceKitLSPOptions: Sendable, Codable, Equatable { ) } + public static func merging(base: SourceKitLSPOptions, workspaceFolder: DocumentURI) -> SourceKitLSPOptions { + return SourceKitLSPOptions.merging( + base: base, + override: SourceKitLSPOptions( + path: workspaceFolder.fileURL? + .appendingPathComponent(".sourcekit-lsp") + .appendingPathComponent("config.json") + ) + ) + } + public var generatedFilesAbsolutePath: URL { if let generatedFilesPath { return URL(fileURLWithPath: generatedFilesPath) diff --git a/Sources/SourceKitLSP/SourceKitLSPServer.swift b/Sources/SourceKitLSP/SourceKitLSPServer.swift index 323d6fb12..c251271f7 100644 --- a/Sources/SourceKitLSP/SourceKitLSPServer.swift +++ b/Sources/SourceKitLSP/SourceKitLSPServer.swift @@ -209,29 +209,40 @@ package actor SourceKitLSPServer { guard var url = uri.fileURL?.deletingLastPathComponent() else { return nil } - let projectRoots = await self.workspacesAndIsImplicit.filter { !$0.isImplicit }.asyncCompactMap { + + // Roots of opened workspaces - only consider explicit here (all implicit must necessarily be subdirectories of + // the explicit workspace roots) + let workspaceRoots = workspacesAndIsImplicit.filter { !$0.isImplicit }.compactMap { $0.workspace.rootUri?.fileURL } + + // We want to skip creating another workspace if the project roots are the same. This could happen if an existing + // workspace hasn't reloaded after a new file was added to it (and thus that build system needs to be reloaded). + let projectRoots = await workspacesAndIsImplicit.asyncCompactMap { await $0.workspace.buildSystemManager.projectRoot } - let rootURLs = workspacesAndIsImplicit.filter { !$0.isImplicit }.compactMap { $0.workspace.rootUri?.fileURL } - while url.pathComponents.count > 1 && rootURLs.contains(where: { $0.isPrefix(of: url) }) { + + while url.pathComponents.count > 1 && workspaceRoots.contains(where: { $0.isPrefix(of: url) }) { defer { url.deleteLastPathComponent() } - // Ignore workspaces that have the same project root as an existing workspace. - // This might happen if there is an existing SwiftPM workspace that hasn't been reloaded after a new file - // was added to it and thus currently doesn't know that it can handle that file. In that case, we shouldn't open - // a new workspace for the same root. Instead, the existing workspace's build system needs to be reloaded. + let uri = DocumentURI(url) - guard let buildSystemSpec = determineBuildSystem(forWorkspaceFolder: uri, options: self.options) else { + let options = SourceKitLSPOptions.merging(base: self.options, workspaceFolder: uri) + + // Some build systems consider paths outside of the folder (eg. BSP has settings in the home directory). If we + // allowed those paths, then the very first folder that the file is in would always be its own build system - so + // skip them in that case. + guard let buildSystemSpec = determineBuildSystem(forWorkspaceFolder: uri, onlyConsiderRoot: true, options: options) else { continue } guard !projectRoots.contains(buildSystemSpec.projectRoot) else { continue } + + // No existing workspace matches this root - create one. guard let workspace = await orLog( - "Creating workspace", - { try await createWorkspace(workspaceFolder: uri, buildSystemSpec: buildSystemSpec) } + "Creating implicit workspace", + { try await createWorkspace(workspaceFolder: uri, options: options, buildSystemSpec: buildSystemSpec) } ) else { continue @@ -818,9 +829,11 @@ extension SourceKitLSPServer { /// Creates a workspace at the given `uri`. /// - /// If the build system that was determined for the workspace does not satisfy `condition`, `nil` is returned. + /// A workspace does not necessarily have any buildsystem attached to it, in which case `buildSystemSpec` may be + /// `nil` - consider eg. a top level workspace folder with multiple SwiftPM projects inside it. private func createWorkspace( workspaceFolder: DocumentURI, + options: SourceKitLSPOptions, buildSystemSpec: BuildSystemSpec? ) async throws -> Workspace { guard let capabilityRegistry = capabilityRegistry else { @@ -828,16 +841,8 @@ extension SourceKitLSPServer { logger.log("Cannot open workspace before server is initialized") throw NoCapabilityRegistryError() } - let testHooks = self.testHooks - let options = SourceKitLSPOptions.merging( - base: self.options, - override: SourceKitLSPOptions( - path: workspaceFolder.fileURL? - .appendingPathComponent(".sourcekit-lsp") - .appendingPathComponent("config.json") - ) - ) - logger.log("Creating workspace at \(workspaceFolder.forLogging) with options: \(options.forLogging)") + + logger.log("Creating workspace at \(workspaceFolder.forLogging)") logger.logFullObjectInMultipleLogMessages(header: "Options for workspace", options.loggingProxy) let workspace = await Workspace( @@ -848,7 +853,7 @@ extension SourceKitLSPServer { buildSystemSpec: buildSystemSpec, toolchainRegistry: self.toolchainRegistry, options: options, - testHooks: testHooks, + testHooks: self.testHooks, indexTaskScheduler: indexTaskScheduler ) return workspace @@ -857,9 +862,12 @@ extension SourceKitLSPServer { /// Determines the build system for the given workspace folder and creates a `Workspace` that uses this inferred build /// system. private func createWorkspaceWithInferredBuildSystem(workspaceFolder: DocumentURI) async throws -> Workspace { + let options = SourceKitLSPOptions.merging(base: self.options, workspaceFolder: workspaceFolder) + let buildSystemSpec = determineBuildSystem(forWorkspaceFolder: workspaceFolder, onlyConsiderRoot: false, options: options) return try await self.createWorkspace( workspaceFolder: workspaceFolder, - buildSystemSpec: determineBuildSystem(forWorkspaceFolder: workspaceFolder, options: self.options) + options: options, + buildSystemSpec: buildSystemSpec ) } diff --git a/Tests/BuildSystemIntegrationTests/SwiftPMBuildSystemTests.swift b/Tests/BuildSystemIntegrationTests/SwiftPMBuildSystemTests.swift index 3944dbbaf..47d41244c 100644 --- a/Tests/BuildSystemIntegrationTests/SwiftPMBuildSystemTests.swift +++ b/Tests/BuildSystemIntegrationTests/SwiftPMBuildSystemTests.swift @@ -976,9 +976,9 @@ final class SwiftPMBuildSystemTests: XCTestCase { .appendingPathComponent("pkg") .appendingPathComponent("Sources") .appendingPathComponent("lib") - let projectRoot = SwiftPMBuildSystem.projectRoot(for: workspaceRoot, options: .testDefault()) - assertEqual(projectRoot, tempDir.appendingPathComponent("pkg", isDirectory: true)) + let projectRoot = SwiftPMBuildSystem.projectRoot(for: workspaceRoot, options: .testDefault()) + assertNil(projectRoot) } } diff --git a/Tests/SourceKitLSPTests/WorkspaceTests.swift b/Tests/SourceKitLSPTests/WorkspaceTests.swift index 84a1187bc..9a4f0be9a 100644 --- a/Tests/SourceKitLSPTests/WorkspaceTests.swift +++ b/Tests/SourceKitLSPTests/WorkspaceTests.swift @@ -10,6 +10,7 @@ // //===----------------------------------------------------------------------===// +import BuildSystemIntegration import Foundation import LanguageServerProtocol import SKLogging @@ -967,4 +968,116 @@ final class WorkspaceTests: XCTestCase { ["Cannot convert value of type 'Int' to specified type 'String'"] ) } + + func testWorkspaceOptionsOverrideBuildSystem() async throws { + let swiftc = try await unwrap(ToolchainRegistry.forTesting.default?.swiftc) + let sdkArgs = + if let defaultSDKPath { + """ + -sdk '\(defaultSDKPath)' + """ + } else { + "" + } + + let project = try await MultiFileTestProject(files: [ + ".sourcekit-lsp/config.json": """ + { + "defaultWorkspaceType": "compilationDatabase" + } + """, + "src/Foo.swift": """ + #if HAVE_SETTINGS + #error("Have settings") + #endif + """, + "Sources/MyLib/Bar.swift": "", + "build/compile_commands.json": """ + [ + { + "directory": "$TEST_DIR", + "command": "\(swiftc.filePath) $TEST_DIR/src/Foo.swift -DHAVE_SETTINGS \(sdkArgs)", + "file": "src/Foo.swift", + "output": "$TEST_DIR/build/Foo.swift.o" + }, + ] + """, + "Package.swift": """ + // swift-tools-version: 5.7 + + import PackageDescription + + let package = Package( + name: "MyLib", + targets: [ + .target(name: "MyLib"), + ] + ) + """, + ]) + let (uri, _) = try project.openDocument("Foo.swift") + let diagnostics = try await project.testClient.send( + DocumentDiagnosticsRequest(textDocument: TextDocumentIdentifier(uri)) + ) + XCTAssertEqual( + diagnostics.fullReport?.items.map(\.message), + ["Have settings"] + ) + } + + func testImplicitWorkspaceOptionsOverrideBuildSystem() async throws { + let swiftc = try await unwrap(ToolchainRegistry.forTesting.default?.swiftc) + let sdkArgs = + if let defaultSDKPath { + """ + -sdk '\(defaultSDKPath)' + """ + } else { + "" + } + + let project = try await MultiFileTestProject(files: [ + "projA/.sourcekit-lsp/config.json": """ + { + "defaultWorkspaceType": "compilationDatabase" + } + """, + "projA/src/Foo.swift": """ + #if HAVE_SETTINGS + #error("Have settings") + #endif + """, + "projA/Sources/MyLib/Bar.swift": "", + "projA/build/compile_commands.json": """ + [ + { + "directory": "$TEST_DIR/projA", + "command": "\(swiftc.filePath) $TEST_DIR/projA/src/Foo.swift -DHAVE_SETTINGS \(sdkArgs)", + "file": "src/Foo.swift", + "output": "$TEST_DIR/projA/build/Foo.swift.o" + }, + ] + """, + "projA/Package.swift": """ + // swift-tools-version: 5.7 + + import PackageDescription + + let package = Package( + name: "MyLib", + targets: [ + .target(name: "MyLib"), + ] + ) + """, + ]) + let (uri, _) = try project.openDocument("Foo.swift") + let diagnostics = try await project.testClient.send( + DocumentDiagnosticsRequest(textDocument: TextDocumentIdentifier(uri)) + ) + XCTAssertEqual( + diagnostics.fullReport?.items.map(\.message), + ["Have settings"] + ) + } }