From 63b9b3dbafd7ced4a0ca4478eeb97517fdf3fcb1 Mon Sep 17 00:00:00 2001 From: Ben Langmuir Date: Mon, 3 Dec 2018 22:30:18 -0800 Subject: [PATCH] [build-system] Rename BuildSettingsProvider->BuildSystem and fix method name I intend to fold more things than file-settings into here, so use a more generic name. Also drop an unnecessary word from the method name. --- .../SKCore/BuildSettingsProviderList.swift | 8 +++--- Sources/SKCore/BuildSystem.swift | 4 +-- .../CompilationDatabaseBuildSystem.swift | 4 +-- Sources/SKCore/ExternalWorkspace.swift | 2 +- .../FallbackBuildSettingsProvider.swift | 12 ++++----- .../SKSwiftPMWorkspace/SwiftPMWorkspace.swift | 25 +++++++++++-------- Sources/SourceKit/SourceKitServer.swift | 25 ++++++++++++------- Sources/SourceKit/Workspace.swift | 4 +-- .../clangd/ClangLanguageServer.swift | 12 ++++----- .../sourcekitd/SwiftLanguageServer.swift | 18 ++++++------- .../CompilationDatabaseTests.swift | 4 +-- .../SwiftPMWorkspaceTests.swift | 14 +++++------ 12 files changed, 71 insertions(+), 61 deletions(-) diff --git a/Sources/SKCore/BuildSettingsProviderList.swift b/Sources/SKCore/BuildSettingsProviderList.swift index 6503d9f6..7ac02608 100644 --- a/Sources/SKCore/BuildSettingsProviderList.swift +++ b/Sources/SKCore/BuildSettingsProviderList.swift @@ -13,18 +13,18 @@ import LanguageServerProtocol /// Provides build settings from a list of providers in priority order. -public final class BuildSettingsProviderList: BuildSettingsProvider { +public final class BuildSettingsProviderList: BuildSystem { /// The build settings providers to try (in order). - public var providers: [BuildSettingsProvider] = [ + public var providers: [BuildSystem] = [ FallbackBuildSettingsProvider() ] public init() {} - public func settings(for url: URL, language: Language) -> FileBuildSettings? { + public func settings(for url: URL, _ language: Language) -> FileBuildSettings? { for provider in providers { - if let settings = provider.settings(for: url, language: language) { + if let settings = provider.settings(for: url, language) { return settings } } diff --git a/Sources/SKCore/BuildSystem.swift b/Sources/SKCore/BuildSystem.swift index 11bda50d..8c67d6c0 100644 --- a/Sources/SKCore/BuildSystem.swift +++ b/Sources/SKCore/BuildSystem.swift @@ -13,10 +13,10 @@ import LanguageServerProtocol /// Provider of build settings. -public protocol BuildSettingsProvider { +public protocol BuildSystem { /// Returns the settings for the given url and language mode, if known. - func settings(for: URL, language: Language) -> FileBuildSettings? + func settings(for: URL, _ language: Language) -> FileBuildSettings? // TODO: notifications when settings change. } diff --git a/Sources/SKCore/CompilationDatabaseBuildSystem.swift b/Sources/SKCore/CompilationDatabaseBuildSystem.swift index 313f5ad2..44837711 100644 --- a/Sources/SKCore/CompilationDatabaseBuildSystem.swift +++ b/Sources/SKCore/CompilationDatabaseBuildSystem.swift @@ -17,7 +17,7 @@ import LanguageServerProtocol /// /// Provides build settings from a `CompilationDatabase` found by searching a project. For now, only /// one compilation database, located at the project root. -public final class CompilationDatabaseBuildSystem: BuildSettingsProvider { +public final class CompilationDatabaseBuildSystem: BuildSystem { /// The compilation database. var compdb: CompilationDatabase? = nil @@ -31,7 +31,7 @@ public final class CompilationDatabaseBuildSystem: BuildSettingsProvider { } } - public func settings(for url: URL, language: Language) -> FileBuildSettings? { + public func settings(for url: URL, _ language: Language) -> FileBuildSettings? { guard let db = database(for: url), let cmd = db[url].first else { return nil } return FileBuildSettings( diff --git a/Sources/SKCore/ExternalWorkspace.swift b/Sources/SKCore/ExternalWorkspace.swift index 86561b03..f0f63e68 100644 --- a/Sources/SKCore/ExternalWorkspace.swift +++ b/Sources/SKCore/ExternalWorkspace.swift @@ -19,7 +19,7 @@ import Basic public protocol ExternalWorkspace { /// The build system, providing access to compiler arguments. - var buildSystem: BuildSettingsProvider { get } + var buildSystem: BuildSystem { get } /// The path to the raw index store data, if any. var indexStorePath: AbsolutePath? { get } diff --git a/Sources/SKCore/FallbackBuildSettingsProvider.swift b/Sources/SKCore/FallbackBuildSettingsProvider.swift index 412afde4..f738cbe6 100644 --- a/Sources/SKCore/FallbackBuildSettingsProvider.swift +++ b/Sources/SKCore/FallbackBuildSettingsProvider.swift @@ -15,7 +15,7 @@ import Basic import enum Utility.Platform /// A simple build settings provider suitable as a fallback when accurate settings are unknown. -public final class FallbackBuildSettingsProvider: BuildSettingsProvider { +public final class FallbackBuildSettingsProvider: BuildSystem { lazy var sdkpath: AbsolutePath? = { if case .darwin? = Platform.currentPlatform { @@ -26,22 +26,22 @@ public final class FallbackBuildSettingsProvider: BuildSettingsProvider { return nil }() - public func settings(for url: URL, language: Language) -> FileBuildSettings? { + public func settings(for url: URL, _ language: Language) -> FileBuildSettings? { guard let path = try? AbsolutePath(validating: url.path) else { return nil } switch language { case .swift: - return settingsSwift(path: path) + return settingsSwift(path) case .c, .cpp, .objective_c, .objective_cpp: - return settingsClang(path: path, language: language) + return settingsClang(path, language) default: return nil } } - func settingsSwift(path: AbsolutePath) -> FileBuildSettings { + func settingsSwift(_ path: AbsolutePath) -> FileBuildSettings { var args: [String] = [] if let sdkpath = sdkpath { args += [ @@ -53,7 +53,7 @@ public final class FallbackBuildSettingsProvider: BuildSettingsProvider { return FileBuildSettings(preferredToolchain: nil, compilerArguments: args) } - func settingsClang(path: AbsolutePath, language: Language) -> FileBuildSettings { + func settingsClang(_ path: AbsolutePath, _ language: Language) -> FileBuildSettings { var args: [String] = [] if let sdkpath = sdkpath { args += [ diff --git a/Sources/SKSwiftPMWorkspace/SwiftPMWorkspace.swift b/Sources/SKSwiftPMWorkspace/SwiftPMWorkspace.swift index d9281a66..2809fa53 100644 --- a/Sources/SKSwiftPMWorkspace/SwiftPMWorkspace.swift +++ b/Sources/SKSwiftPMWorkspace/SwiftPMWorkspace.swift @@ -40,7 +40,7 @@ public final class SwiftPMWorkspace { var fileToTarget: [AbsolutePath: TargetBuildDescription] = [:] var sourceDirToTarget: [AbsolutePath: TargetBuildDescription] = [:] - /// Creates a `BuildSettingsProvider` using the Swift Package Manager, if this workspace is part of a package. + /// Creates a `BuildSystem` using the Swift Package Manager, if this workspace is part of a package. /// /// - returns: nil if `workspacePath` is not part of a package or there is an error. public convenience init?(url: LanguageServerProtocol.URL, toolchainRegistry: ToolchainRegistry) { @@ -52,7 +52,7 @@ public final class SwiftPMWorkspace { } } - /// Creates a `BuildSettingsProvider` using the Swift Package Manager, if this workspace is part of a package. + /// Creates a `BuildSystem` using the Swift Package Manager, if this workspace is part of a package. /// /// - returns: nil if `workspacePath` is not part of a package. /// - throws: If there is an error loading the package. @@ -162,9 +162,9 @@ public final class SwiftPMWorkspace { } } -extension SwiftPMWorkspace: ExternalWorkspace, BuildSettingsProvider { +extension SwiftPMWorkspace: ExternalWorkspace, BuildSystem { - public var buildSystem: BuildSettingsProvider { return self } + public var buildSystem: BuildSystem { return self } public var buildPath: AbsolutePath { return buildParameters.buildPath @@ -178,13 +178,16 @@ extension SwiftPMWorkspace: ExternalWorkspace, BuildSettingsProvider { return buildPath.appending(components: "index", "db") } - public func settings(for url: LanguageServerProtocol.URL, language: Language) -> FileBuildSettings? { + public func settings( + for url: LanguageServerProtocol.URL, + _ language: Language) -> FileBuildSettings? + { guard let path = try? AbsolutePath(validating: url.path) else { return nil } if let td = self.fileToTarget[path] { - return settings(for: path, language: language, targetDescription: td) + return settings(for: path, language, td) } if path.basename == "Package.swift" { @@ -192,7 +195,7 @@ extension SwiftPMWorkspace: ExternalWorkspace, BuildSettingsProvider { } if path.extension == "h" { - return settings(forHeader: path, language: language) + return settings(forHeader: path, language) } return nil @@ -205,8 +208,8 @@ extension SwiftPMWorkspace { public func settings( for path: AbsolutePath, - language: Language, - targetDescription td: TargetBuildDescription + _ language: Language, + _ td: TargetBuildDescription ) -> FileBuildSettings? { let buildPath = self.buildPath @@ -314,11 +317,11 @@ extension SwiftPMWorkspace { return nil } - public func settings(forHeader path: AbsolutePath, language: Language) -> FileBuildSettings? { + public func settings(forHeader path: AbsolutePath, _ language: Language) -> FileBuildSettings? { var dir = path.parentDirectory while !dir.isRoot { if let td = sourceDirToTarget[dir] { - return settings(for: path, language: language, targetDescription: td) + return settings(for: path, language, td) } dir = dir.parentDirectory } diff --git a/Sources/SourceKit/SourceKitServer.swift b/Sources/SourceKit/SourceKitServer.swift index af8edea7..b78f1d63 100644 --- a/Sources/SourceKit/SourceKitServer.swift +++ b/Sources/SourceKit/SourceKitServer.swift @@ -118,8 +118,8 @@ public final class SourceKitServer: LanguageServer { client.send(note.params) } - func toolchain(for url: URL, language: Language) -> Toolchain? { - if let id = workspace?.buildSettings.settings(for: url, language: language)?.preferredToolchain, + func toolchain(for url: URL, _ language: Language) -> Toolchain? { + if let id = workspace?.buildSettings.settings(for: url, language)?.preferredToolchain, let toolchain = toolchainRegistry.toolchain(identifier:id) { return toolchain @@ -150,7 +150,7 @@ public final class SourceKitServer: LanguageServer { return nil } - func languageService(for toolchain: Toolchain, language: Language) -> Connection? { + func languageService(for toolchain: Toolchain, _ language: Language) -> Connection? { let key = LanguageServiceKey(toolchain: toolchain.identifier, language: language) if let service = languageService[key] { return service @@ -158,7 +158,7 @@ public final class SourceKitServer: LanguageServer { // Start a new service. return orLog("failed to start language service", level: .error) { - guard let service = try SourceKit.languageService(for: toolchain, language: language, client: self) else { + guard let service = try SourceKit.languageService(for: toolchain, language, client: self) else { return nil } @@ -183,12 +183,14 @@ public final class SourceKitServer: LanguageServer { } } - func languageService(for url: URL, language: Language, workspace: Workspace) -> Connection? { + func languageService(for url: URL, _ language: Language, in workspace: Workspace) -> Connection? { if let service = workspace.documentService[url] { return service } - guard let toolchain = toolchain(for: url, language: language), let service = languageService(for: toolchain, language: language) else { + guard let toolchain = toolchain(for: url, language), + let service = languageService(for: toolchain, language) + else { return nil } @@ -274,7 +276,7 @@ extension SourceKitServer { func openDocument(_ note: Notification, workspace: Workspace) { workspace.documentManager.open(note) - if let service = languageService(for: note.params.textDocument.url, language: note.params.textDocument.language, workspace: workspace) { + if let service = languageService(for: note.params.textDocument.url, note.params.textDocument.language, in: workspace) { service.send(note.params) } } @@ -448,11 +450,16 @@ extension SourceKitServer { } } -/// Creates a new connection from `client` to a service for `language` if available, and launches the service. Does *not* send the initialization request. +/// Creates a new connection from `client` to a service for `language` if available, and launches +/// the service. Does *not* send the initialization request. /// /// - returns: The connection, if a suitable language service is available; otherwise nil. /// - throws: If there is a suitable service but it fails to launch, throws an error. -public func languageService(for toolchain: Toolchain, language: Language, client: MessageHandler) throws -> Connection? { +public func languageService( + for toolchain: Toolchain, + _ language: Language, + client: MessageHandler) throws -> Connection? +{ switch language { case .c, .cpp, .objective_c, .objective_cpp: diff --git a/Sources/SourceKit/Workspace.swift b/Sources/SourceKit/Workspace.swift index c0409c16..39e711fb 100644 --- a/Sources/SourceKit/Workspace.swift +++ b/Sources/SourceKit/Workspace.swift @@ -40,7 +40,7 @@ public final class Workspace { /// /// If `external` is not `nil`, this will typically include `external.buildSystem`. It may also /// provide settings for files outside the workspace using additional providers. - public let buildSettings: BuildSettingsProvider + public let buildSettings: BuildSystem /// The source code index, if available. public var index: IndexStoreDB? = nil @@ -55,7 +55,7 @@ public final class Workspace { rootPath: AbsolutePath?, clientCapabilities: ClientCapabilities, external: ExternalWorkspace?, - buildSettings: BuildSettingsProvider, + buildSettings: BuildSystem, index: IndexStoreDB?) { self.rootPath = rootPath diff --git a/Sources/SourceKit/clangd/ClangLanguageServer.swift b/Sources/SourceKit/clangd/ClangLanguageServer.swift index b16f05de..3da3e07a 100644 --- a/Sources/SourceKit/clangd/ClangLanguageServer.swift +++ b/Sources/SourceKit/clangd/ClangLanguageServer.swift @@ -22,13 +22,13 @@ final class ClangLanguageServerShim: LanguageServer { let clangd: Connection - let buildSettingsProvider: BuildSettingsProvider + let buildSystem: BuildSystem /// Creates a language server for the given client using the sourcekitd dylib at the specified path. - public init(client: Connection, clangd: Connection, buildSettingsProvider: BuildSettingsProvider) throws { + public init(client: Connection, clangd: Connection, buildSystem: BuildSystem) throws { self.clangd = clangd - self.buildSettingsProvider = buildSettingsProvider + self.buildSystem = buildSystem super.init(client: client) } @@ -65,7 +65,7 @@ final class ClangLanguageServerShim: LanguageServer { func openDocument(_ note: Notification) { let url = note.params.textDocument.url - let settings = buildSettingsProvider.settings(for: url, language: note.params.textDocument.language) + let settings = buildSystem.settings(for: url, note.params.textDocument.language) logAsync(level: settings == nil ? .warning : .debug) { _ in let settingsStr = settings == nil ? "nil" : settings!.compilerArguments.description @@ -82,7 +82,7 @@ final class ClangLanguageServerShim: LanguageServer { } } -func makeJSONRPCClangServer(client: MessageHandler, clangd: AbsolutePath, buildSettings: BuildSettingsProvider?) throws -> Connection { +func makeJSONRPCClangServer(client: MessageHandler, clangd: AbsolutePath, buildSettings: BuildSystem?) throws -> Connection { let clientToServer: Pipe = Pipe() let serverToClient: Pipe = Pipe() @@ -98,7 +98,7 @@ func makeJSONRPCClangServer(client: MessageHandler, clangd: AbsolutePath, buildS let shim = try ClangLanguageServerShim( client: connectionToClient, clangd: connection, - buildSettingsProvider: buildSettings ?? BuildSettingsProviderList() + buildSystem: buildSettings ?? BuildSettingsProviderList() ) connectionToShim.start(handler: shim) diff --git a/Sources/SourceKit/sourcekitd/SwiftLanguageServer.swift b/Sources/SourceKit/sourcekitd/SwiftLanguageServer.swift index 24355dfe..4b559eaf 100644 --- a/Sources/SourceKit/sourcekitd/SwiftLanguageServer.swift +++ b/Sources/SourceKit/sourcekitd/SwiftLanguageServer.swift @@ -21,7 +21,7 @@ public final class SwiftLanguageServer: LanguageServer { let sourcekitd: SwiftSourceKitFramework - let buildSettingsProvider: BuildSettingsProvider + let buildSystem: BuildSystem // FIXME: ideally we wouldn't need separate management from a parent server in the same process. var documentManager: DocumentManager @@ -34,10 +34,10 @@ public final class SwiftLanguageServer: LanguageServer { var values: sourcekitd_values { return sourcekitd.values } /// Creates a language server for the given client using the sourcekitd dylib at the specified path. - public init(client: Connection, sourcekitd: AbsolutePath, buildSettingsProvider: BuildSettingsProvider, onExit: @escaping () -> () = {}) throws { + public init(client: Connection, sourcekitd: AbsolutePath, buildSystem: BuildSystem, onExit: @escaping () -> () = {}) throws { self.sourcekitd = try SwiftSourceKitFramework(dylib: sourcekitd) - self.buildSettingsProvider = buildSettingsProvider + self.buildSystem = buildSystem self.documentManager = DocumentManager() self.onExit = onExit super.init(client: client) @@ -220,7 +220,7 @@ extension SwiftLanguageServer { req[keys.name] = note.params.textDocument.url.path req[keys.sourcetext] = snapshot.text - if let settings = buildSettingsProvider.settings(for: snapshot.document.url, language: snapshot.document.language) { + if let settings = buildSystem.settings(for: snapshot.document.url, snapshot.document.language) { req[keys.compilerargs] = settings.compilerArguments } @@ -308,7 +308,7 @@ extension SwiftLanguageServer { skreq[keys.sourcefile] = snapshot.document.url.path skreq[keys.sourcetext] = snapshot.text - if let settings = buildSettingsProvider.settings(for: snapshot.document.url, language: snapshot.document.language) { + if let settings = buildSystem.settings(for: snapshot.document.url, snapshot.document.language) { skreq[keys.compilerargs] = settings.compilerArguments } @@ -432,7 +432,7 @@ extension SwiftLanguageServer { skreq[keys.sourcefile] = snapshot.document.url.path // FIXME: should come from the internal document - if let settings = buildSettingsProvider.settings(for: snapshot.document.url, language: snapshot.document.language) { + if let settings = buildSystem.settings(for: snapshot.document.url, snapshot.document.language) { skreq[keys.compilerargs] = settings.compilerArguments } @@ -510,7 +510,7 @@ extension SwiftLanguageServer { skreq[keys.sourcefile] = snapshot.document.url.path // FIXME: should come from the internal document - if let settings = buildSettingsProvider.settings(for: snapshot.document.url, language: snapshot.document.language) { + if let settings = buildSystem.settings(for: snapshot.document.url, snapshot.document.language) { skreq[keys.compilerargs] = settings.compilerArguments } @@ -568,7 +568,7 @@ extension DocumentSnapshot { } } -func makeLocalSwiftServer(client: MessageHandler, sourcekitd: AbsolutePath, buildSettings: BuildSettingsProvider?) throws -> Connection { +func makeLocalSwiftServer(client: MessageHandler, sourcekitd: AbsolutePath, buildSettings: BuildSystem?) throws -> Connection { let connectionToSK = LocalConnection() let connectionToClient = LocalConnection() @@ -576,7 +576,7 @@ func makeLocalSwiftServer(client: MessageHandler, sourcekitd: AbsolutePath, buil let server = try SwiftLanguageServer( client: connectionToClient, sourcekitd: sourcekitd, - buildSettingsProvider: buildSettings ?? BuildSettingsProviderList() + buildSystem: buildSettings ?? BuildSettingsProviderList() ) connectionToSK.start(handler: server) diff --git a/Tests/SKCoreTests/CompilationDatabaseTests.swift b/Tests/SKCoreTests/CompilationDatabaseTests.swift index 03f37933..54774f09 100644 --- a/Tests/SKCoreTests/CompilationDatabaseTests.swift +++ b/Tests/SKCoreTests/CompilationDatabaseTests.swift @@ -197,10 +197,10 @@ final class CompilationDatabaseTests: XCTestCase { ] """) - let buildSystem: BuildSettingsProvider = CompilationDatabaseBuildSystem( + let buildSystem: BuildSystem = CompilationDatabaseBuildSystem( projectRoot: AbsolutePath("/a"), fileSystem: fs) - let settings = buildSystem.settings(for: URL(fileURLWithPath: "/a/a.swift"), language: .swift) + let settings = buildSystem.settings(for: URL(fileURLWithPath: "/a/a.swift"), .swift) XCTAssertNotNil(settings) XCTAssertEqual(settings?.workingDirectory, "/a") XCTAssertEqual(settings?.compilerArguments, ["-swift-version", "4", "/a/a.swift"]) diff --git a/Tests/SKSwiftPMWorkspaceTests/SwiftPMWorkspaceTests.swift b/Tests/SKSwiftPMWorkspaceTests/SwiftPMWorkspaceTests.swift index 1e8691e8..a0e001a9 100644 --- a/Tests/SKSwiftPMWorkspaceTests/SwiftPMWorkspaceTests.swift +++ b/Tests/SKSwiftPMWorkspaceTests/SwiftPMWorkspaceTests.swift @@ -42,7 +42,7 @@ final class SwiftPMWorkspaceTests: XCTestCase { XCTAssertEqual(ws.buildPath, packageRoot.appending(components: ".build", "debug")) XCTAssertNotNil(ws.indexStorePath) - let arguments = ws.settings(for: aswift.asURL, language: .swift)!.compilerArguments + let arguments = ws.settings(for: aswift.asURL, .swift)!.compilerArguments check( "-module-name", "lib", "-incremental", "-emit-dependencies", @@ -87,10 +87,10 @@ final class SwiftPMWorkspaceTests: XCTestCase { let aswift = packageRoot.appending(components: "Sources", "lib", "a.swift") let bswift = packageRoot.appending(components: "Sources", "lib", "b.swift") - let argumentsA = ws.settings(for: aswift.asURL, language: .swift)!.compilerArguments + let argumentsA = ws.settings(for: aswift.asURL, .swift)!.compilerArguments check(aswift.asString, arguments: argumentsA) check(bswift.asString, arguments: argumentsA) - let argumentsB = ws.settings(for: aswift.asURL, language: .swift)!.compilerArguments + let argumentsB = ws.settings(for: aswift.asURL, .swift)!.compilerArguments check(aswift.asString, arguments: argumentsB) check(bswift.asString, arguments: argumentsB) } @@ -124,12 +124,12 @@ final class SwiftPMWorkspaceTests: XCTestCase { let aswift = packageRoot.appending(components: "Sources", "libA", "a.swift") let bswift = packageRoot.appending(components: "Sources", "libB", "b.swift") - let arguments = ws.settings(for: aswift.asURL, language: .swift)!.compilerArguments + let arguments = ws.settings(for: aswift.asURL, .swift)!.compilerArguments check(aswift.asString, arguments: arguments) checkNot(bswift.asString, arguments: arguments) check("-I", packageRoot.appending(components: "Sources", "libC", "include").asString, arguments: arguments) - let argumentsB = ws.settings(for: bswift.asURL, language: .swift)!.compilerArguments + let argumentsB = ws.settings(for: bswift.asURL, .swift)!.compilerArguments check(bswift.asString, arguments: argumentsB) checkNot(aswift.asString, arguments: argumentsB) checkNot("-I", packageRoot.appending(components: "Sources", "libC", "include").asString, arguments: argumentsB) @@ -164,7 +164,7 @@ final class SwiftPMWorkspaceTests: XCTestCase { XCTAssertEqual(ws.buildPath, build) XCTAssertNotNil(ws.indexStorePath) - let arguments = ws.settings(for: acxx.asURL, language: .cpp)!.compilerArguments + let arguments = ws.settings(for: acxx.asURL, .cpp)!.compilerArguments check("-MD", "-MT", "dependencies", "-MF", build.appending(components: "lib.build", "a.cpp.d").asString, @@ -210,7 +210,7 @@ final class SwiftPMWorkspaceTests: XCTestCase { fileSystem: fs)! let aswift = packageRoot.appending(components: "Sources", "lib", "a.swift") - let arguments = ws.settings(for: aswift.asURL, language: .swift)!.compilerArguments + let arguments = ws.settings(for: aswift.asURL, .swift)!.compilerArguments check("-target", arguments: arguments) // Only one! #if os(macOS) check("-target", "x86_64-apple-macosx10.13", arguments: arguments)