Skip to content

Remove custom decoding function from SourceKitLSPOptions #1646

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 17 additions & 17 deletions Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift
Original file line number Diff line number Diff line change
Expand Up @@ -229,11 +229,11 @@ package actor SwiftPMBuildSystem {
let destinationSDK = try SwiftSDK.deriveTargetSwiftSDK(
hostSwiftSDK: hostSDK,
hostTriple: hostSwiftPMToolchain.targetTriple,
customCompileTriple: options.swiftPM.triple.map { try Triple($0) },
swiftSDKSelector: options.swiftPM.swiftSDK,
customCompileTriple: options.swiftPMOrDefault.triple.map { try Triple($0) },
swiftSDKSelector: options.swiftPMOrDefault.swiftSDK,
store: SwiftSDKBundleStore(
swiftSDKsDirectory: fileSystem.getSharedSwiftSDKsDirectory(
explicitDirectory: options.swiftPM.swiftSDKsDirectory.map { try AbsolutePath(validating: $0) }
explicitDirectory: options.swiftPMOrDefault.swiftSDKsDirectory.map { try AbsolutePath(validating: $0) }
),
fileSystem: fileSystem,
observabilityScope: observabilitySystem.topScope,
Expand All @@ -251,7 +251,7 @@ package actor SwiftPMBuildSystem {
)
if options.backgroundIndexingOrDefault {
location.scratchDirectory = AbsolutePath(packageRoot.appending(component: ".index-build"))
} else if let scratchDirectory = options.swiftPM.scratchPath,
} else if let scratchDirectory = options.swiftPMOrDefault.scratchPath,
let scratchDirectoryPath = try? AbsolutePath(validating: scratchDirectory)
{
location.scratchDirectory = scratchDirectoryPath
Expand All @@ -267,25 +267,25 @@ package actor SwiftPMBuildSystem {
customHostToolchain: hostSwiftPMToolchain,
customManifestLoader: ManifestLoader(
toolchain: hostSwiftPMToolchain,
isManifestSandboxEnabled: !(options.swiftPM.disableSandbox ?? false),
isManifestSandboxEnabled: !(options.swiftPMOrDefault.disableSandbox ?? false),
cacheDir: location.sharedManifestsCacheDirectory,
importRestrictions: configuration.manifestImportRestrictions
)
)

let buildConfiguration: PackageModel.BuildConfiguration
switch options.swiftPM.configuration {
switch options.swiftPMOrDefault.configuration {
case .debug, nil:
buildConfiguration = .debug
case .release:
buildConfiguration = .release
}

let buildFlags = BuildFlags(
cCompilerFlags: options.swiftPM.cCompilerFlags ?? [],
cxxCompilerFlags: options.swiftPM.cxxCompilerFlags ?? [],
swiftCompilerFlags: options.swiftPM.swiftCompilerFlags ?? [],
linkerFlags: options.swiftPM.linkerFlags ?? []
cCompilerFlags: options.swiftPMOrDefault.cCompilerFlags ?? [],
cxxCompilerFlags: options.swiftPMOrDefault.cxxCompilerFlags ?? [],
swiftCompilerFlags: options.swiftPMOrDefault.swiftCompilerFlags ?? [],
linkerFlags: options.swiftPMOrDefault.linkerFlags ?? []
)

self.toolsBuildParameters = try BuildParameters(
Expand Down Expand Up @@ -381,7 +381,7 @@ extension SwiftPMBuildSystem {
destinationBuildParameters: destinationBuildParameters,
toolsBuildParameters: toolsBuildParameters,
graph: modulesGraph,
disableSandbox: options.swiftPM.disableSandbox ?? false,
disableSandbox: options.swiftPMOrDefault.disableSandbox ?? false,
fileSystem: fileSystem,
observabilityScope: observabilitySystem.topScope
)
Expand Down Expand Up @@ -607,16 +607,16 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuildSystem {
"--disable-index-store",
"--target", target.targetID,
]
if options.swiftPM.disableSandbox ?? false {
if options.swiftPMOrDefault.disableSandbox ?? false {
arguments += ["--disable-sandbox"]
}
if let configuration = options.swiftPM.configuration {
if let configuration = options.swiftPMOrDefault.configuration {
arguments += ["-c", configuration.rawValue]
}
arguments += options.swiftPM.cCompilerFlags?.flatMap { ["-Xcc", $0] } ?? []
arguments += options.swiftPM.cxxCompilerFlags?.flatMap { ["-Xcxx", $0] } ?? []
arguments += options.swiftPM.swiftCompilerFlags?.flatMap { ["-Xswiftc", $0] } ?? []
arguments += options.swiftPM.linkerFlags?.flatMap { ["-Xlinker", $0] } ?? []
arguments += options.swiftPMOrDefault.cCompilerFlags?.flatMap { ["-Xcc", $0] } ?? []
arguments += options.swiftPMOrDefault.cxxCompilerFlags?.flatMap { ["-Xcxx", $0] } ?? []
arguments += options.swiftPMOrDefault.swiftCompilerFlags?.flatMap { ["-Xswiftc", $0] } ?? []
arguments += options.swiftPMOrDefault.linkerFlags?.flatMap { ["-Xlinker", $0] } ?? []
switch options.backgroundPreparationModeOrDefault {
case .build: break
case .noLazy: arguments += ["--experimental-prepare-for-indexing", "--experimental-prepare-for-indexing-no-lazy"]
Expand Down
88 changes: 41 additions & 47 deletions Sources/SKOptions/SourceKitLSPOptions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ import struct TSCBasic.AbsolutePath
/// Options that can be used to modify SourceKit-LSP's behavior.
///
/// See `ConfigurationFile.md` for a description of the configuration file's behavior.
public struct SourceKitLSPOptions: Sendable, Codable, CustomLogStringConvertible {
public struct SwiftPMOptions: Sendable, Codable, CustomLogStringConvertible {
public struct SourceKitLSPOptions: Sendable, Codable, Equatable, CustomLogStringConvertible {
public struct SwiftPMOptions: Sendable, Codable, Equatable, CustomLogStringConvertible {
/// Build configuration (debug|release).
///
/// Equivalent to SwiftPM's `--configuration` option.
Expand Down Expand Up @@ -104,7 +104,7 @@ public struct SourceKitLSPOptions: Sendable, Codable, CustomLogStringConvertible
}
}

public struct CompilationDatabaseOptions: Sendable, Codable, CustomLogStringConvertible {
public struct CompilationDatabaseOptions: Sendable, Codable, Equatable, CustomLogStringConvertible {
/// Additional paths to search for a compilation database, relative to a workspace root.
public var searchPaths: [String]?

Expand All @@ -128,7 +128,7 @@ public struct SourceKitLSPOptions: Sendable, Codable, CustomLogStringConvertible
}
}

public struct FallbackBuildSystemOptions: Sendable, Codable, CustomLogStringConvertible {
public struct FallbackBuildSystemOptions: Sendable, Codable, Equatable, CustomLogStringConvertible {
public var cCompilerFlags: [String]?
public var cxxCompilerFlags: [String]?
public var swiftCompilerFlags: [String]?
Expand Down Expand Up @@ -163,7 +163,7 @@ public struct SourceKitLSPOptions: Sendable, Codable, CustomLogStringConvertible
}
}

public struct IndexOptions: Sendable, Codable, CustomLogStringConvertible {
public struct IndexOptions: Sendable, Codable, Equatable, CustomLogStringConvertible {
public var indexStorePath: String?
public var indexDatabasePath: String?
public var indexPrefixMap: [String: String]?
Expand Down Expand Up @@ -216,7 +216,7 @@ public struct SourceKitLSPOptions: Sendable, Codable, CustomLogStringConvertible
}
}

public struct LoggingOptions: Sendable, Codable, CustomLogStringConvertible {
public struct LoggingOptions: Sendable, Codable, Equatable, CustomLogStringConvertible {
/// The level from which one onwards log messages should be written.
public var level: String?
/// Whether potentially sensitive information should be redacted.
Expand Down Expand Up @@ -259,12 +259,37 @@ public struct SourceKitLSPOptions: Sendable, Codable, CustomLogStringConvertible
case enabled
}

public var swiftPM: SwiftPMOptions
public var compilationDatabase: CompilationDatabaseOptions
public var fallbackBuildSystem: FallbackBuildSystemOptions
private var swiftPM: SwiftPMOptions?
public var swiftPMOrDefault: SwiftPMOptions {
get { swiftPM ?? .init() }
set { swiftPM = newValue }
}

private var compilationDatabase: CompilationDatabaseOptions?
public var compilationDatabaseOrDefault: CompilationDatabaseOptions {
get { compilationDatabase ?? .init() }
set { compilationDatabase = newValue }
}

private var fallbackBuildSystem: FallbackBuildSystemOptions?
public var fallbackBuildSystemOrDefault: FallbackBuildSystemOptions {
get { fallbackBuildSystem ?? .init() }
set { fallbackBuildSystem = newValue }
}

public var clangdOptions: [String]?
public var index: IndexOptions
public var logging: LoggingOptions

private var index: IndexOptions?
public var indexOrDefault: IndexOptions {
get { index ?? .init() }
set { index = newValue }
}

private var logging: LoggingOptions?
public var loggingOrDefault: LoggingOptions {
get { logging ?? .init() }
set { logging = newValue }
}

/// Default workspace type (buildserver|compdb|swiftpm). Overrides workspace type selection logic.
public var defaultWorkspaceType: WorkspaceType?
Expand Down Expand Up @@ -406,18 +431,18 @@ public struct SourceKitLSPOptions: Sendable, Codable, CustomLogStringConvertible

public static func merging(base: SourceKitLSPOptions, override: SourceKitLSPOptions?) -> SourceKitLSPOptions {
return SourceKitLSPOptions(
swiftPM: SwiftPMOptions.merging(base: base.swiftPM, override: override?.swiftPM),
swiftPM: SwiftPMOptions.merging(base: base.swiftPMOrDefault, override: override?.swiftPM),
fallbackBuildSystem: FallbackBuildSystemOptions.merging(
base: base.fallbackBuildSystem,
base: base.fallbackBuildSystemOrDefault,
override: override?.fallbackBuildSystem
),
compilationDatabase: CompilationDatabaseOptions.merging(
base: base.compilationDatabase,
base: base.compilationDatabaseOrDefault,
override: override?.compilationDatabase
),
clangdOptions: override?.clangdOptions ?? base.clangdOptions,
index: IndexOptions.merging(base: base.index, override: override?.index),
logging: LoggingOptions.merging(base: base.logging, override: override?.logging),
index: IndexOptions.merging(base: base.indexOrDefault, override: override?.index),
logging: LoggingOptions.merging(base: base.loggingOrDefault, override: override?.logging),
defaultWorkspaceType: override?.defaultWorkspaceType ?? base.defaultWorkspaceType,
generatedFilesPath: override?.generatedFilesPath ?? base.generatedFilesPath,
backgroundIndexing: override?.backgroundIndexing ?? base.backgroundIndexing,
Expand Down Expand Up @@ -447,37 +472,6 @@ public struct SourceKitLSPOptions: Sendable, Codable, CustomLogStringConvertible
return experimentalFeatures.contains(feature)
}

public init(from decoder: any Decoder) throws {
let container = try decoder.container(keyedBy: CodingKeys.self)

self.swiftPM = try container.decodeIfPresent(SwiftPMOptions.self, forKey: CodingKeys.swiftPM) ?? .init()
self.compilationDatabase =
try container.decodeIfPresent(CompilationDatabaseOptions.self, forKey: CodingKeys.compilationDatabase) ?? .init()
self.fallbackBuildSystem =
try container.decodeIfPresent(FallbackBuildSystemOptions.self, forKey: CodingKeys.fallbackBuildSystem) ?? .init()
self.clangdOptions = try container.decodeIfPresent([String].self, forKey: CodingKeys.clangdOptions)
self.index = try container.decodeIfPresent(IndexOptions.self, forKey: CodingKeys.index) ?? .init()
self.logging = try container.decodeIfPresent(LoggingOptions.self, forKey: .logging) ?? .init()
self.defaultWorkspaceType = try container.decodeIfPresent(
WorkspaceType.self,
forKey: CodingKeys.defaultWorkspaceType
)
self.generatedFilesPath = try container.decodeIfPresent(String.self, forKey: CodingKeys.generatedFilesPath)
self.backgroundIndexing = try container.decodeIfPresent(Bool.self, forKey: CodingKeys.backgroundIndexing)
self.experimentalFeatures = try container.decodeIfPresent(
Set<ExperimentalFeature>.self,
forKey: CodingKeys.experimentalFeatures
)
self.swiftPublishDiagnosticsDebounceDuration = try container.decodeIfPresent(
Double.self,
forKey: CodingKeys.swiftPublishDiagnosticsDebounceDuration
)
self.workDoneProgressDebounceDuration = try container.decodeIfPresent(
Double.self,
forKey: CodingKeys.workDoneProgressDebounceDuration
)
}

public var description: String {
recursiveDescription(of: self)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,10 @@ package struct IndexedSingleSwiftFileTestProject {

// Create the test client
var options = SourceKitLSPOptions.testDefault()
options.index = SourceKitLSPOptions.IndexOptions(indexStorePath: indexURL.path, indexDatabasePath: indexDBURL.path)
options.indexOrDefault = SourceKitLSPOptions.IndexOptions(
indexStorePath: indexURL.path,
indexDatabasePath: indexDBURL.path
)
self.testClient = try await TestSourceKitLSPClient(
options: options,
workspaceFolders: [
Expand Down
4 changes: 2 additions & 2 deletions Sources/SKTestSupport/TestSourceKitLSPClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,8 @@ package final class TestSourceKitLSPClient: MessageHandler, Sendable {
) async throws {
var options = options
if let globalModuleCache {
options.swiftPM.swiftCompilerFlags =
(options.swiftPM.swiftCompilerFlags ?? []) + ["-module-cache-path", globalModuleCache.path]
options.swiftPMOrDefault.swiftCompilerFlags =
(options.swiftPMOrDefault.swiftCompilerFlags ?? []) + ["-module-cache-path", globalModuleCache.path]
}
options.backgroundIndexing = enableBackgroundIndexing

Expand Down
4 changes: 3 additions & 1 deletion Sources/SourceKitLSP/CreateBuildSystem.swift
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,9 @@ func createBuildSystem(
func createCompilationDatabaseBuildSystem(rootPath: AbsolutePath) -> CompilationDatabaseBuildSystem? {
return CompilationDatabaseBuildSystem(
projectRoot: rootPath,
searchPaths: (options.compilationDatabase.searchPaths ?? []).compactMap { try? RelativePath(validating: $0) }
searchPaths: (options.compilationDatabaseOrDefault.searchPaths ?? []).compactMap {
try? RelativePath(validating: $0)
}
)
}

Expand Down
3 changes: 2 additions & 1 deletion Sources/SourceKitLSP/SourceKitLSPServer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,8 @@ package actor SourceKitLSPServer {

self.client = client
let processorCount = ProcessInfo.processInfo.processorCount
let lowPriorityCores = options.index.maxCoresPercentageToUseForBackgroundIndexingOrDefault * Double(processorCount)
let lowPriorityCores =
options.indexOrDefault.maxCoresPercentageToUseForBackgroundIndexingOrDefault * Double(processorCount)
self.indexTaskScheduler = TaskScheduler(maxConcurrentTasksByPriority: [
(TaskPriority.medium, processorCount),
(TaskPriority.low, max(Int(lowPriorityCores), 1)),
Expand Down
6 changes: 3 additions & 3 deletions Sources/SourceKitLSP/Workspace.swift
Original file line number Diff line number Diff line change
Expand Up @@ -107,15 +107,15 @@ package final class Workspace: Sendable {
self._uncheckedIndex = ThreadSafeBox(initialValue: uncheckedIndex)
self.buildSystemManager = await BuildSystemManager(
buildSystem: underlyingBuildSystem,
fallbackBuildSystem: FallbackBuildSystem(options: options.fallbackBuildSystem),
fallbackBuildSystem: FallbackBuildSystem(options: options.fallbackBuildSystemOrDefault),
mainFilesProvider: uncheckedIndex,
toolchainRegistry: toolchainRegistry
)
if options.backgroundIndexingOrDefault, let uncheckedIndex, await buildSystemManager.supportsPreparation {
self.semanticIndexManager = SemanticIndexManager(
index: uncheckedIndex,
buildSystemManager: buildSystemManager,
updateIndexStoreTimeout: options.index.updateIndexStoreTimeoutOrDefault,
updateIndexStoreTimeout: options.indexOrDefault.updateIndexStoreTimeoutOrDefault,
testHooks: testHooks.indexTestHooks,
indexTaskScheduler: indexTaskScheduler,
logMessageToIndexLog: logMessageToIndexLog,
Expand Down Expand Up @@ -163,7 +163,7 @@ package final class Workspace: Sendable {
var index: IndexStoreDB? = nil
var indexDelegate: SourceKitIndexDelegate? = nil

let indexOptions = options.index
let indexOptions = options.indexOrDefault
if let storePath = await firstNonNil(
AbsolutePath(validatingOrNil: indexOptions.indexStorePath),
await buildSystem?.indexStorePath
Expand Down
6 changes: 4 additions & 2 deletions Sources/sourcekit-lsp/SourceKitLSP.swift
Original file line number Diff line number Diff line change
Expand Up @@ -277,10 +277,12 @@ struct SourceKitLSP: AsyncParsableCommand {
}

let globalConfigurationOptions = globalConfigurationOptions
if let logLevelStr = globalConfigurationOptions.logging.level, let logLevel = NonDarwinLogLevel(logLevelStr) {
if let logLevelStr = globalConfigurationOptions.loggingOrDefault.level,
let logLevel = NonDarwinLogLevel(logLevelStr)
{
LogConfig.logLevel.value = logLevel
}
if let privacyLevelStr = globalConfigurationOptions.logging.privacyLevel,
if let privacyLevelStr = globalConfigurationOptions.loggingOrDefault.privacyLevel,
let privacyLevel = NonDarwinLogPrivacy(privacyLevelStr)
{
LogConfig.privacyLevel.value = privacyLevel
Expand Down
4 changes: 2 additions & 2 deletions Tests/SourceKitLSPTests/BackgroundIndexingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -965,7 +965,7 @@ final class BackgroundIndexingTests: XCTestCase {

func testUseBuildFlagsDuringPreparation() async throws {
var options = SourceKitLSPOptions.testDefault()
options.swiftPM.swiftCompilerFlags = ["-D", "MY_FLAG"]
options.swiftPMOrDefault.swiftCompilerFlags = ["-D", "MY_FLAG"]
let project = try await SwiftPMTestProject(
files: [
"Lib/Lib.swift": """
Expand Down Expand Up @@ -1390,7 +1390,7 @@ final class BackgroundIndexingTests: XCTestCase {

var options = SourceKitLSPOptions.testDefault()
options.backgroundPreparationMode = SourceKitLSPOptions.BackgroundPreparationMode.enabled.rawValue
options.index.updateIndexStoreTimeout = 1 /* second */
options.indexOrDefault.updateIndexStoreTimeout = 1 /* second */

let dateStarted = Date()
_ = try await SwiftPMTestProject(
Expand Down
8 changes: 8 additions & 0 deletions Tests/SourceKitLSPTests/LifecycleTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
//===----------------------------------------------------------------------===//

import LanguageServerProtocol
import SKOptions
import SKTestSupport
import XCTest

Expand Down Expand Up @@ -39,6 +40,13 @@ final class LifecycleTests: XCTestCase {
XCTAssertNotNil(initResult.capabilities.completionProvider)
}

func testEmptySourceKitLSPOptionsCanBeDecoded() {
// Check that none of the keys in `SourceKitLSPOptions` are required.
assertNoThrow {
try JSONDecoder().decode(SourceKitLSPOptions.self, from: "{}") == SourceKitLSPOptions()
}
}

func testCancellation() async throws {
let testClient = try await TestSourceKitLSPClient()
let uri = DocumentURI(for: .swift)
Expand Down