diff --git a/.gitignore b/.gitignore index 32c80ecff..2000e2996 100644 --- a/.gitignore +++ b/.gitignore @@ -3,6 +3,7 @@ default.profraw Package.resolved /.build /.index-build +/.linux-build /Packages /*.xcodeproj /*.sublime-project diff --git a/Documentation/Files_To_Reindex.md b/Documentation/Files_To_Reindex.md new file mode 100644 index 000000000..98c76984e --- /dev/null +++ b/Documentation/Files_To_Reindex.md @@ -0,0 +1,35 @@ +# Which files to re-index when a file is modified + +## `.swift` + +Obviously affects the file itself, which we do re-index. + +If an internal declaration was changed, all files within the module might be also affected. If a public declaration was changed, all modules that depend on it might be affected. The effect can only really be in three ways: +1. It might change overload resolution in another file, which is fairly unlikely +2. A new declaration is introduced in this file that is already referenced by another file +3. A declaration is removed in this file that was referenced from other files. In those cases the other files now have an invalid reference. + +We decided to not re-index any files other than the file itself because naively re-indexing all files that might depend on the modified file requires too much processing power that will likely have no or very little effect on the index – we are trading accuracy for CPU time here. +We mark the targets of the changed file as well as any dependent targets as out-of-date. The assumption is that most likely the user will go back to any of the affected files shortly afterwards and modify them again. When that happens, the affected file will get re-indexed and bring the index back up to date. + +Alternatives would be: +- We could we check the file’s interface hash and re-index other files based on whether it has changed. But at that point we are somewhat implementing a build system. And even if a new public method was introduced it’s very likely that the user hasn’t actually used it anywhere yet, which means that re-indexing all dependent modules would still be doing unnecessary work. +- To cover case (2) from above, we could re-index only dependencies that previously indexed with errors. This is an alternative that hasn’t been fully explored. + +## `.h` + +All files that include the header (including via other headers) might be affected by the change, similar to how all `.swift` files that import a module might be affected. Similar to modules, we choose to not re-index all files that include the header because of the same considerations mentioned above. + +To re-index the header, we pick one main file that includes the header and re-index that, which will effectively update the index for the header. For existing headers, we know which files import a header from the existing index. For new header files, we assume that it hasn’t been included in any file yet and thus don't index it. If the user wrote an include to the new header before creating the header itself, we don't know about that include from the existing index. But it’s likely that the user will modify the file that includes the new header file shortly after, which will index the header and establish the header to main file connection. + +## `.c` / `.cpp` / `.m` + +This is the easy case since only the file itself is affected. + +## Compiler settings (`compile_commands.json` / `Package.swift`) + +Ideally, we would like to consider a file as changed when its compile commands have changed, if they changed in a meaningful way (ie. in a way that would also trigger re-compilation in an incremental build). Non-meaningful changes would be: +- If compiler arguments that aren't order dependent are shuffled around. We could have a really quick check for compiler arguments equality by comparing them unordered. Any real compiler argument change will most likely do more than rearranging the arguments. +- The addition of a new Swift file to a target is equivalent to that file being modified and shouldn’t trigger a re-index of the entire target. + +At the moment, unit files don’t include information about the compiler arguments with which they were created, so it’s impossible to know whether the compiler arguments have changed when a project is opened. Thus, for now, we don’t re-index any files on compiler settings changing. diff --git a/Editors/README.md b/Editors/README.md index 77589b469..5fece4076 100644 --- a/Editors/README.md +++ b/Editors/README.md @@ -22,58 +22,7 @@ The [Swift for Visual Studio Code extension](https://marketplace.visualstudio.co ## Sublime Text -Before using SourceKit-LSP with Sublime Text, you will need to install the LSP package from Package Control. To configure SourceKit-LSP, open the LSP package's settings. The following snippet should be enough to get started with Swift. - -You will need the path to the `sourcekit-lsp` executable for the "command" section. - -```json -{ - "clients": - { - "SourceKit-LSP": - { - "enabled": true, - "command": [ - "" - ], - "env": { - // To override the toolchain, uncomment the following: - // "SOURCEKIT_TOOLCHAIN_PATH": "", - }, - "languages": [ - { - "scopes": ["source.swift"], - "syntaxes": [ - "Packages/Swift/Syntaxes/Swift.tmLanguage", - "Packages/Decent Swift Syntax/Swift.sublime-syntax", - ], - "languageId": "swift" - }, - { - "scopes": ["source.c"], - "syntaxes": ["Packages/C++/C.sublime-syntax"], - "languageId": "c" - }, - { - "scopes": ["source.c++"], - "syntaxes": ["Packages/C++/C++.sublime-syntax"], - "languageId": "cpp" - }, - { - "scopes": ["source.objc"], - "syntaxes": ["Packages/Objective-C/Objective-C.sublime-syntax"], - "languageId": "objective-c" - }, - { - "scopes": ["source.objc++"], - "syntaxes": ["Packages/Objective-C/Objective-C++.sublime-syntax"], - "languageId": "objective-cpp" - }, - ] - } - } -} -``` +Before using SourceKit-LSP with Sublime Text, you will need to install the [LSP](https://packagecontrol.io/packages/LSP), [LSP-SourceKit](https://github.com/sublimelsp/LSP-SourceKit) and [Swift-Next](https://github.com/Swift-Next/Swift-Next) packages from Package Control. Then toggle the server on by typing in command palette `LSP: Enable Language Server Globally` or `LSP: Enable Language Server in Project`. ## Emacs diff --git a/Package.swift b/Package.swift index b5ef7c762..e3d62c047 100644 --- a/Package.swift +++ b/Package.swift @@ -182,6 +182,13 @@ let package = Package( exclude: ["CMakeLists.txt"] ), + .testTarget( + name: "SemanticIndexTests", + dependencies: [ + "SemanticIndex" + ] + ), + // MARK: SKCore // Data structures and algorithms useful across the project, but not necessarily // suitable for use in other packages. diff --git a/Sources/LSPLogging/NonDarwinLogging.swift b/Sources/LSPLogging/NonDarwinLogging.swift index b838a29db..94ab0d1ce 100644 --- a/Sources/LSPLogging/NonDarwinLogging.swift +++ b/Sources/LSPLogging/NonDarwinLogging.swift @@ -195,6 +195,17 @@ public struct NonDarwinLogInterpolation: StringInterpolationProtocol, Sendable { append(description: message.description, redactedDescription: message.redactedDescription, privacy: privacy) } + public mutating func appendInterpolation( + _ message: (some CustomLogStringConvertibleWrapper & Sendable)?, + privacy: NonDarwinLogPrivacy = .private + ) { + if let message { + self.appendInterpolation(message, privacy: privacy) + } else { + self.appendLiteral("") + } + } + public mutating func appendInterpolation(_ type: Any.Type, privacy: NonDarwinLogPrivacy = .public) { append(description: String(reflecting: type), redactedDescription: "", privacy: privacy) } diff --git a/Sources/LSPTestSupport/LocalConnection.swift b/Sources/LSPTestSupport/LocalConnection.swift new file mode 100644 index 000000000..190e7f133 --- /dev/null +++ b/Sources/LSPTestSupport/LocalConnection.swift @@ -0,0 +1,133 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2014 - 2020 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +import Dispatch +import LSPLogging +import LanguageServerProtocol + +/// A connection between two message handlers in the same process. +/// +/// You must call `start(handler:)` before sending any messages, and must call `close()` when finished to avoid a memory leak. +/// +/// ``` +/// let client: MessageHandler = ... +/// let server: MessageHandler = ... +/// let conn = LocalConnection() +/// conn.start(handler: server) +/// conn.send(...) // handled by server +/// conn.close() +/// ``` +/// +/// - Note: Unchecked sendable conformance because shared state is guarded by `queue`. +public final class LocalConnection: Connection, @unchecked Sendable { + enum State { + case ready, started, closed + } + + /// A name of the endpoint for this connection, used for logging, e.g. `clangd`. + private let name: String + + /// The queue guarding `_nextRequestID`. + let queue: DispatchQueue = DispatchQueue(label: "local-connection-queue") + + var _nextRequestID: Int = 0 + + var state: State = .ready + + var handler: MessageHandler? = nil + + public init(name: String) { + self.name = name + } + + deinit { + if state != .closed { + close() + } + } + + public func start(handler: MessageHandler) { + precondition(state == .ready) + state = .started + self.handler = handler + } + + public func close() { + precondition(state != .closed) + handler = nil + state = .closed + } + + func nextRequestID() -> RequestID { + return queue.sync { + _nextRequestID += 1 + return .number(_nextRequestID) + } + } + + public func send(_ notification: Notification) { + logger.info( + """ + Sending notification to \(self.name, privacy: .public) + \(notification.forLogging) + """ + ) + self.handler?.handle(notification) + } + + public func send( + _ request: Request, + reply: @Sendable @escaping (LSPResult) -> Void + ) -> RequestID { + let id = nextRequestID() + + logger.info( + """ + Sending request to \(self.name, privacy: .public) (id: \(id, privacy: .public)): + \(request.forLogging) + """ + ) + + guard let handler = self.handler else { + logger.info( + """ + Replying to request \(id, privacy: .public) with .serverCancelled because no handler is specified in \(self.name, privacy: .public) + """ + ) + reply(.failure(.serverCancelled)) + return id + } + + precondition(self.state == .started) + handler.handle(request, id: id) { result in + switch result { + case .success(let response): + logger.info( + """ + Received reply for request \(id, privacy: .public) from \(self.name, privacy: .public) + \(response.forLogging) + """ + ) + case .failure(let error): + logger.error( + """ + Received error for request \(id, privacy: .public) from \(self.name, privacy: .public) + \(error.forLogging) + """ + ) + } + reply(result) + } + + return id + } +} diff --git a/Sources/LSPTestSupport/TestJSONRPCConnection.swift b/Sources/LSPTestSupport/TestJSONRPCConnection.swift index 4bec75ba2..9400822cf 100644 --- a/Sources/LSPTestSupport/TestJSONRPCConnection.swift +++ b/Sources/LSPTestSupport/TestJSONRPCConnection.swift @@ -17,7 +17,7 @@ import XCTest import class Foundation.Pipe -public final class TestJSONRPCConnection { +public final class TestJSONRPCConnection: Sendable { public let clientToServer: Pipe = Pipe() public let serverToClient: Pipe = Pipe() @@ -76,9 +76,9 @@ public final class TestJSONRPCConnection { public struct TestLocalConnection { public let client: TestClient - public let clientConnection: LocalConnection = LocalConnection() + public let clientConnection: LocalConnection = LocalConnection(name: "Test") public let server: TestServer - public let serverConnection: LocalConnection = LocalConnection() + public let serverConnection: LocalConnection = LocalConnection(name: "Test") public init(allowUnexpectedNotification: Bool = true) { client = TestClient(connectionToServer: serverConnection, allowUnexpectedNotification: allowUnexpectedNotification) @@ -151,7 +151,7 @@ public actor TestClient: MessageHandler { /// Send a request to the LSP server and (asynchronously) receive a reply. public nonisolated func send( _ request: Request, - reply: @escaping (LSPResult) -> Void + reply: @Sendable @escaping (LSPResult) -> Void ) -> RequestID { return connectionToServer.send(request, reply: reply) } diff --git a/Sources/LanguageServerProtocol/Connection.swift b/Sources/LanguageServerProtocol/Connection.swift index e33a40f9a..22db5eff1 100644 --- a/Sources/LanguageServerProtocol/Connection.swift +++ b/Sources/LanguageServerProtocol/Connection.swift @@ -10,8 +10,6 @@ // //===----------------------------------------------------------------------===// -import Dispatch - /// An abstract connection, allow messages to be sent to a (potentially remote) `MessageHandler`. public protocol Connection: AnyObject, Sendable { @@ -47,83 +45,3 @@ public protocol MessageHandler: AnyObject, Sendable { reply: @Sendable @escaping (LSPResult) -> Void ) } - -/// A connection between two message handlers in the same process. -/// -/// You must call `start(handler:)` before sending any messages, and must call `close()` when finished to avoid a memory leak. -/// -/// ``` -/// let client: MessageHandler = ... -/// let server: MessageHandler = ... -/// let conn = LocalConnection() -/// conn.start(handler: server) -/// conn.send(...) // handled by server -/// conn.close() -/// ``` -/// -/// - Note: Unchecked sendable conformance because shared state is guarded by `queue`. -public final class LocalConnection: Connection, @unchecked Sendable { - - enum State { - case ready, started, closed - } - - /// The queue guarding `_nextRequestID`. - let queue: DispatchQueue = DispatchQueue(label: "local-connection-queue") - - var _nextRequestID: Int = 0 - - var state: State = .ready - - var handler: MessageHandler? = nil - - public init() {} - - deinit { - if state != .closed { - close() - } - } - - public func start(handler: MessageHandler) { - precondition(state == .ready) - state = .started - self.handler = handler - } - - public func close() { - precondition(state != .closed) - handler = nil - state = .closed - } - - func nextRequestID() -> RequestID { - return queue.sync { - _nextRequestID += 1 - return .number(_nextRequestID) - } - } - - public func send(_ notification: Notification) where Notification: NotificationType { - self.handler?.handle(notification) - } - - public func send( - _ request: Request, - reply: @Sendable @escaping (LSPResult) -> Void - ) -> RequestID { - let id = nextRequestID() - - guard let handler = self.handler else { - reply(.failure(.serverCancelled)) - return id - } - - precondition(self.state == .started) - handler.handle(request, id: id) { result in - reply(result) - } - - return id - } -} diff --git a/Sources/SKCore/BuildServerBuildSystem.swift b/Sources/SKCore/BuildServerBuildSystem.swift index 01561d721..615ad3bd2 100644 --- a/Sources/SKCore/BuildServerBuildSystem.swift +++ b/Sources/SKCore/BuildServerBuildSystem.swift @@ -285,6 +285,10 @@ extension BuildServerBuildSystem: BuildSystem { return nil } + public func targets(dependingOn targets: [ConfiguredTarget]) -> [ConfiguredTarget]? { + return nil + } + public func prepare(targets: [ConfiguredTarget]) async throws { throw PrepareNotSupportedError() } diff --git a/Sources/SKCore/BuildSystem.swift b/Sources/SKCore/BuildSystem.swift index f6880a00b..e436cabc4 100644 --- a/Sources/SKCore/BuildSystem.swift +++ b/Sources/SKCore/BuildSystem.swift @@ -11,6 +11,7 @@ //===----------------------------------------------------------------------===// import BuildServerProtocol +import LSPLogging import LanguageServerProtocol import struct TSCBasic.AbsolutePath @@ -50,7 +51,7 @@ public struct SourceFileInfo: Sendable { /// A target / run destination combination. For example, a configured target can represent building the target /// `MyLibrary` for iOS. -public struct ConfiguredTarget: Hashable, Sendable { +public struct ConfiguredTarget: Hashable, Sendable, CustomLogStringConvertible { /// An opaque string that represents the target. /// /// The target's ID should be generated by the build system that handles the target and only interpreted by that @@ -67,6 +68,14 @@ public struct ConfiguredTarget: Hashable, Sendable { self.targetID = targetID self.runDestinationID = runDestinationID } + + public var description: String { + "\(targetID)-\(runDestinationID)" + } + + public var redactedDescription: String { + "\(targetID.hashForLogging)-\(runDestinationID.hashForLogging)" + } } /// An error build systems can throw from `prepare` if they don't support preparation of targets. @@ -138,6 +147,15 @@ public protocol BuildSystem: AnyObject, Sendable { /// `nil` if the build system doesn't support topological sorting of targets. func topologicalSort(of targets: [ConfiguredTarget]) async -> [ConfiguredTarget]? + /// Returns the list of targets that might depend on the given target and that need to be re-prepared when a file in + /// `target` is modified. + /// + /// The returned list can be an over-approximation, in which case the indexer will perform more work than strictly + /// necessary by scheduling re-preparation of a target where it isn't necessary. + /// + /// Returning `nil` indicates that all targets should be considered depending on the given target. + func targets(dependingOn targets: [ConfiguredTarget]) async -> [ConfiguredTarget]? + /// Prepare the given targets for indexing and semantic functionality. This should build all swift modules of target /// dependencies. func prepare(targets: [ConfiguredTarget]) async throws @@ -167,6 +185,8 @@ public protocol BuildSystem: AnyObject, Sendable { func fileHandlingCapability(for uri: DocumentURI) async -> FileHandlingCapability /// Returns the list of source files in the project. + /// + /// Header files should not be considered as source files because they cannot be compiled. func sourceFiles() async -> [SourceFileInfo] /// Adds a callback that should be called when the value returned by `sourceFiles()` changes. diff --git a/Sources/SKCore/BuildSystemManager.swift b/Sources/SKCore/BuildSystemManager.swift index 33d755c57..f2897bfa3 100644 --- a/Sources/SKCore/BuildSystemManager.swift +++ b/Sources/SKCore/BuildSystemManager.swift @@ -118,7 +118,7 @@ extension BuildSystemManager { } switch document.fileURL?.pathExtension { case "c": return .c - case "cpp", "cc", "cxx": return .cpp + case "cpp", "cc", "cxx", "hpp": return .cpp case "m": return .objective_c case "mm", "h": return .objective_cpp case "swift": return .swift @@ -126,12 +126,17 @@ extension BuildSystemManager { } } + /// Returns all the `ConfiguredTarget`s that the document is part of. + public func configuredTargets(for document: DocumentURI) async -> [ConfiguredTarget] { + return await buildSystem?.configuredTargets(for: document) ?? [] + } + /// Returns the `ConfiguredTarget` that should be used for semantic functionality of the given document. public func canonicalConfiguredTarget(for document: DocumentURI) async -> ConfiguredTarget? { // Sort the configured targets to deterministically pick the same `ConfiguredTarget` every time. // We could allow the user to specify a preference of one target over another. For now this is not necessary because // no build system currently returns multiple targets for a source file. - return await buildSystem?.configuredTargets(for: document) + return await configuredTargets(for: document) .sorted { ($0.targetID, $0.runDestinationID) < ($1.targetID, $1.runDestinationID) } .first } @@ -141,13 +146,10 @@ extension BuildSystemManager { /// Implementation detail of `buildSettings(for:language:)`. private func buildSettingsFromPrimaryBuildSystem( for document: DocumentURI, + in target: ConfiguredTarget?, language: Language ) async throws -> FileBuildSettings? { - guard let buildSystem else { - return nil - } - guard let target = await canonicalConfiguredTarget(for: document) else { - logger.error("Failed to get target for \(document.forLogging)") + guard let buildSystem, let target else { return nil } // FIXME: (async) We should only wait `fallbackSettingsTimeout` for build @@ -155,18 +157,25 @@ extension BuildSystemManager { // implement that with Swift concurrency. // For now, this should be fine because all build systems return // very quickly from `settings(for:language:)`. - guard let settings = try await buildSystem.buildSettings(for: document, in: target, language: language) else { - return nil - } - return settings + return try await buildSystem.buildSettings(for: document, in: target, language: language) } - private func buildSettings( + /// Returns the build settings for the given file in the given target. + /// + /// Only call this method if it is known that `document` is a main file. Prefer `buildSettingsInferredFromMainFile` + /// otherwise. If `document` is a header file, this will most likely return fallback settings because header files + /// don't have build settings by themselves. + public func buildSettings( for document: DocumentURI, + in target: ConfiguredTarget?, language: Language ) async -> FileBuildSettings? { do { - if let buildSettings = try await buildSettingsFromPrimaryBuildSystem(for: document, language: language) { + if let buildSettings = try await buildSettingsFromPrimaryBuildSystem( + for: document, + in: target, + language: language + ) { return buildSettings } } catch { @@ -194,11 +203,11 @@ extension BuildSystemManager { /// references to that C file in the build settings by the header file. public func buildSettingsInferredFromMainFile( for document: DocumentURI, - language: Language, - logBuildSettings: Bool = true + language: Language ) async -> FileBuildSettings? { let mainFile = await mainFile(for: document, language: language) - guard var settings = await buildSettings(for: mainFile, language: language) else { + let target = await canonicalConfiguredTarget(for: mainFile) + guard var settings = await buildSettings(for: mainFile, in: target, language: language) else { return nil } if mainFile != document { @@ -206,9 +215,7 @@ extension BuildSystemManager { // to reference `document` instead of `mainFile`. settings = settings.patching(newFile: document.pseudoPath, originalFile: mainFile.pseudoPath) } - if logBuildSettings { - await BuildSettingsLogger.shared.log(settings: settings, for: document) - } + await BuildSettingsLogger.shared.log(settings: settings, for: document) return settings } @@ -220,6 +227,10 @@ extension BuildSystemManager { return await buildSystem?.topologicalSort(of: targets) } + public func targets(dependingOn targets: [ConfiguredTarget]) async -> [ConfiguredTarget]? { + return await buildSystem?.targets(dependingOn: targets) + } + public func prepare(targets: [ConfiguredTarget]) async throws { try await buildSystem?.prepare(targets: targets) } diff --git a/Sources/SKCore/CompilationDatabaseBuildSystem.swift b/Sources/SKCore/CompilationDatabaseBuildSystem.swift index eab796d52..853765c54 100644 --- a/Sources/SKCore/CompilationDatabaseBuildSystem.swift +++ b/Sources/SKCore/CompilationDatabaseBuildSystem.swift @@ -135,6 +135,10 @@ extension CompilationDatabaseBuildSystem: BuildSystem { return nil } + public func targets(dependingOn targets: [ConfiguredTarget]) -> [ConfiguredTarget]? { + return nil + } + public func registerForChangeNotifications(for uri: DocumentURI) async { self.watchedFiles.insert(uri) } diff --git a/Sources/SKCore/TaskScheduler.swift b/Sources/SKCore/TaskScheduler.swift index 58cf052fb..23be76912 100644 --- a/Sources/SKCore/TaskScheduler.swift +++ b/Sources/SKCore/TaskScheduler.swift @@ -20,8 +20,6 @@ public enum TaskDependencyAction { case cancelAndRescheduleDependency(TaskDescription) } -private let taskSchedulerSubsystem = "org.swift.sourcekit-lsp.task-scheduler" - public protocol TaskDescriptionProtocol: Identifiable, Sendable, CustomLogStringConvertible { /// Execute the task. /// @@ -89,7 +87,7 @@ public enum TaskExecutionState { case finished } -fileprivate actor QueuedTask { +public actor QueuedTask { /// Result of `executionTask` / the tasks in `executionTaskCreatedContinuation`. /// See doc comment on `executionTask`. enum ExecutionTaskFinishStatus { @@ -149,14 +147,38 @@ fileprivate actor QueuedTask { /// Gets reset every time `executionTask` finishes. nonisolated(unsafe) private var cancelledToBeRescheduled: AtomicBool = .init(initialValue: false) + private nonisolated(unsafe) var _isExecuting: AtomicBool = .init(initialValue: false) + + /// Whether the task is currently executing or still queued to be executed later. + public nonisolated var isExecuting: Bool { + return _isExecuting.value + } + + /// Wait for the task to finish. + /// + /// If the tasks that waits for this queued task to finished is cancelled, the QueuedTask will still continue + /// executing. + public func waitToFinish() async { + return await resultTask.value + } + + /// Wait for the task to finish. + /// + /// If the tasks that waits for this queued task to finished is cancelled, the QueuedTask will also be cancelled. + /// This assumes that the caller of this method has unique control over the task and is the only one interested in its + /// value. + public func waitToFinishPropagatingCancellation() async { + return await resultTask.valuePropagatingCancellation + } + /// A callback that will be called when the task starts executing, is cancelled to be rescheduled, or when it finishes /// execution. - private let executionStateChangedCallback: (@Sendable (TaskExecutionState) async -> Void)? + private let executionStateChangedCallback: (@Sendable (QueuedTask, TaskExecutionState) async -> Void)? init( priority: TaskPriority? = nil, description: TaskDescription, - executionStateChangedCallback: (@Sendable (TaskExecutionState) async -> Void)? + executionStateChangedCallback: (@Sendable (QueuedTask, TaskExecutionState) async -> Void)? ) async { self._priority = .init(initialValue: priority?.rawValue ?? Task.currentPriority.rawValue) self.description = description @@ -216,19 +238,21 @@ fileprivate actor QueuedTask { } executionTask = task executionTaskCreatedContinuation.yield(task) - await executionStateChangedCallback?(.executing) + _isExecuting.value = true + await executionStateChangedCallback?(self, .executing) return await task.value } /// Implementation detail of `execute` that is called after `self.description.execute()` finishes. private func finalizeExecution() async -> ExecutionTaskFinishStatus { self.executionTask = nil + _isExecuting.value = false if Task.isCancelled && self.cancelledToBeRescheduled.value { - await executionStateChangedCallback?(.cancelledToBeRescheduled) + await executionStateChangedCallback?(self, .cancelledToBeRescheduled) self.cancelledToBeRescheduled.value = false return ExecutionTaskFinishStatus.cancelledToBeRescheduled } else { - await executionStateChangedCallback?(.finished) + await executionStateChangedCallback?(self, .finished) return ExecutionTaskFinishStatus.terminated } } @@ -259,11 +283,6 @@ fileprivate actor QueuedTask { /// a new task that depends on it. Otherwise a no-op. nonisolated func elevatePriority(to targetPriority: TaskPriority) { if priority < targetPriority { - withLoggingSubsystemAndScope(subsystem: taskSchedulerSubsystem, scope: nil) { - logger.debug( - "Elevating priority of \(self.description.forLogging) from \(self.priority.rawValue) to \(targetPriority.rawValue)" - ) - } Task(priority: targetPriority) { await self.resultTask.value } @@ -334,11 +353,10 @@ public actor TaskScheduler { public func schedule( priority: TaskPriority? = nil, _ taskDescription: TaskDescription, - @_inheritActorContext executionStateChangedCallback: (@Sendable (TaskExecutionState) async -> Void)? = nil - ) async -> Task { - withLoggingSubsystemAndScope(subsystem: taskSchedulerSubsystem, scope: nil) { - logger.debug("Scheduling \(taskDescription.forLogging)") - } + @_inheritActorContext executionStateChangedCallback: ( + @Sendable (QueuedTask, TaskExecutionState) async -> Void + )? = nil + ) async -> QueuedTask { let queuedTask = await QueuedTask( priority: priority, description: taskDescription, @@ -351,7 +369,7 @@ public actor TaskScheduler { // queued task. await self.poke() } - return queuedTask.resultTask + return queuedTask } /// Trigger all queued tasks to update their priority. @@ -397,17 +415,13 @@ public actor TaskScheduler { case .cancelAndRescheduleDependency(let taskDescription): guard let dependency = self.currentlyExecutingTasks.first(where: { $0.description.id == taskDescription.id }) else { - withLoggingSubsystemAndScope(subsystem: taskSchedulerSubsystem, scope: nil) { - logger.fault( - "Cannot find task to wait for \(taskDescription.forLogging) in list of currently executing tasks" - ) - } + logger.fault( + "Cannot find task to wait for \(taskDescription.forLogging) in list of currently executing tasks" + ) return nil } if !taskDescription.isIdempotent { - withLoggingSubsystemAndScope(subsystem: taskSchedulerSubsystem, scope: nil) { - logger.fault("Cannot reschedule task '\(taskDescription.forLogging)' since it is not idempotent") - } + logger.fault("Cannot reschedule task '\(taskDescription.forLogging)' since it is not idempotent") return dependency } if dependency.priority > task.priority { @@ -418,11 +432,9 @@ public actor TaskScheduler { case .waitAndElevatePriorityOfDependency(let taskDescription): guard let dependency = self.currentlyExecutingTasks.first(where: { $0.description.id == taskDescription.id }) else { - withLoggingSubsystemAndScope(subsystem: taskSchedulerSubsystem, scope: nil) { - logger.fault( - "Cannot find task to wait for '\(taskDescription.forLogging)' in list of currently executing tasks" - ) - } + logger.fault( + "Cannot find task to wait for '\(taskDescription.forLogging)' in list of currently executing tasks" + ) return nil } return dependency @@ -440,11 +452,9 @@ public actor TaskScheduler { switch taskDependency { case .cancelAndRescheduleDependency(let taskDescription): guard let task = self.currentlyExecutingTasks.first(where: { $0.description.id == taskDescription.id }) else { - withLoggingSubsystemAndScope(subsystem: taskSchedulerSubsystem, scope: nil) { - logger.fault( - "Cannot find task to reschedule \(taskDescription.forLogging) in list of currently executing tasks" - ) - } + logger.fault( + "Cannot find task to reschedule \(taskDescription.forLogging) in list of currently executing tasks" + ) return nil } return task @@ -455,9 +465,6 @@ public actor TaskScheduler { if !rescheduleTasks.isEmpty { Task.detached(priority: task.priority) { for task in rescheduleTasks { - withLoggingSubsystemAndScope(subsystem: taskSchedulerSubsystem, scope: nil) { - logger.debug("Suspending \(task.description.forLogging)") - } await task.cancelToBeRescheduled() } } @@ -468,25 +475,12 @@ public actor TaskScheduler { return } - withLoggingSubsystemAndScope(subsystem: taskSchedulerSubsystem, scope: nil) { - logger.debug("Executing \(task.description.forLogging) with priority \(task.priority.rawValue)") - } currentlyExecutingTasks.append(task) pendingTasks.removeAll(where: { $0 === task }) Task.detached(priority: task.priority) { - withLoggingSubsystemAndScope(subsystem: taskSchedulerSubsystem, scope: nil) { - logger.debug( - "Execution of \(task.description.forLogging) started with priority \(Task.currentPriority.rawValue)" - ) - } // Await the task's return in a task so that this poker can continue checking if there are more execution // slots that can be filled with queued tasks. let finishStatus = await task.execute() - withLoggingSubsystemAndScope(subsystem: taskSchedulerSubsystem, scope: nil) { - logger.debug( - "Execution of \(task.description.forLogging) finished with priority \(Task.currentPriority.rawValue)" - ) - } await self.finalizeTaskExecution(task: task, finishStatus: finishStatus) } } diff --git a/Sources/SKSupport/CMakeLists.txt b/Sources/SKSupport/CMakeLists.txt index 8c5e1685a..0171ac617 100644 --- a/Sources/SKSupport/CMakeLists.txt +++ b/Sources/SKSupport/CMakeLists.txt @@ -15,6 +15,7 @@ add_library(SKSupport STATIC Process+WaitUntilExitWithCancellation.swift Random.swift Result.swift + Sequence+AsyncMap.swift SwitchableProcessResultExitStatus.swift ThreadSafeBox.swift WorkspaceType.swift diff --git a/Sources/SourceKitLSP/Sequence+AsyncMap.swift b/Sources/SKSupport/Sequence+AsyncMap.swift similarity index 74% rename from Sources/SourceKitLSP/Sequence+AsyncMap.swift rename to Sources/SKSupport/Sequence+AsyncMap.swift index 7ec4e100e..97d6817db 100644 --- a/Sources/SourceKitLSP/Sequence+AsyncMap.swift +++ b/Sources/SKSupport/Sequence+AsyncMap.swift @@ -12,7 +12,7 @@ extension Sequence { /// Just like `Sequence.map` but allows an `async` transform function. - func asyncMap( + public func asyncMap( @_inheritActorContext _ transform: @Sendable (Element) async throws -> T ) async rethrows -> [T] { var result: [T] = [] @@ -26,7 +26,7 @@ extension Sequence { } /// Just like `Sequence.compactMap` but allows an `async` transform function. - func asyncCompactMap( + public func asyncCompactMap( @_inheritActorContext _ transform: @Sendable (Element) async throws -> T? ) async rethrows -> [T] { var result: [T] = [] @@ -39,4 +39,19 @@ extension Sequence { return result } + + /// Just like `Sequence.map` but allows an `async` transform function. + public func asyncFilter( + @_inheritActorContext _ predicate: @Sendable (Element) async throws -> Bool + ) async rethrows -> [Element] { + var result: [Element] = [] + + for element in self { + if try await predicate(element) { + result.append(element) + } + } + + return result + } } diff --git a/Sources/SKSupport/SwitchableProcessResultExitStatus.swift b/Sources/SKSupport/SwitchableProcessResultExitStatus.swift index 8e6f85733..3b0637897 100644 --- a/Sources/SKSupport/SwitchableProcessResultExitStatus.swift +++ b/Sources/SKSupport/SwitchableProcessResultExitStatus.swift @@ -10,7 +10,6 @@ // //===----------------------------------------------------------------------===// -// We need to import all of TSCBasic because otherwise we can't refer to Process.ExitStatus (rdar://127577691) import struct TSCBasic.ProcessResult /// Same as `ProcessResult.ExitStatus` in tools-support-core but has the same cases on all platforms and is thus easier diff --git a/Sources/SKSwiftPMWorkspace/SwiftPMBuildSystem.swift b/Sources/SKSwiftPMWorkspace/SwiftPMBuildSystem.swift index 69497b842..6b311cb96 100644 --- a/Sources/SKSwiftPMWorkspace/SwiftPMBuildSystem.swift +++ b/Sources/SKSwiftPMWorkspace/SwiftPMBuildSystem.swift @@ -63,6 +63,14 @@ private func getDefaultToolchain(_ toolchainRegistry: ToolchainRegistry) async - return await toolchainRegistry.default } +fileprivate extension ConfiguredTarget { + init(_ buildTarget: any SwiftBuildTarget) { + self.init(targetID: buildTarget.name, runDestinationID: "dummy") + } + + static let forPackageManifest = ConfiguredTarget(targetID: "", runDestinationID: "") +} + /// Swift Package Manager build system and workspace support. /// /// This class implements the `BuildSystem` interface to provide the build settings for a Swift @@ -101,10 +109,10 @@ public actor SwiftPMBuildSystem { var fileToTarget: [AbsolutePath: SwiftBuildTarget] = [:] var sourceDirToTarget: [AbsolutePath: SwiftBuildTarget] = [:] - /// Maps target ids (aka. `ConfiguredTarget.targetID`) to their SwiftPM build target as well as an index in their - /// topological sorting. Targets with lower index are more low level, ie. targets with higher indices depend on - /// targets with lower indices. - var targets: [String: (index: Int, buildTarget: SwiftBuildTarget)] = [:] + /// Maps configured targets ids to their SwiftPM build target as well as an index in their topological sorting. + /// + /// Targets with lower index are more low level, ie. targets with higher indices depend on targets with lower indices. + var targets: [ConfiguredTarget: (index: Int, buildTarget: SwiftBuildTarget)] = [:] /// The URIs for which the delegate has registered for change notifications, /// mapped to the language the delegate specified when registering for change notifications. @@ -128,12 +136,9 @@ public actor SwiftPMBuildSystem { logger.log(level: diagnostic.severity.asLogLevel, "SwiftPM log: \(diagnostic.description)") }) - /// Whether the SwiftPMBuildSystem may modify `Package.resolved` or not. - /// - /// This is `false` if the `SwiftPMBuildSystem` is pointed at a `.index-build` directory that's independent of the - /// user's build. In this case `SwiftPMBuildSystem` is allowed to clone repositories even if no `Package.resolved` - /// exists. - private let forceResolvedVersions: Bool + /// Whether the `SwiftPMBuildSystem` is pointed at a `.index-build` directory that's independent of the + /// user's build. + private let isForIndexBuild: Bool /// Creates a build system using the Swift Package Manager, if this workspace is a package. /// @@ -148,13 +153,13 @@ public actor SwiftPMBuildSystem { toolchainRegistry: ToolchainRegistry, fileSystem: FileSystem = localFileSystem, buildSetup: BuildSetup, - forceResolvedVersions: Bool, + isForIndexBuild: Bool, reloadPackageStatusCallback: @escaping (ReloadPackageStatus) async -> Void = { _ in } ) async throws { self.workspacePath = workspacePath self.fileSystem = fileSystem self.toolchainRegistry = toolchainRegistry - self.forceResolvedVersions = forceResolvedVersions + self.isForIndexBuild = isForIndexBuild guard let packageRoot = findPackageDirectory(containing: workspacePath, fileSystem) else { throw Error.noManifest(workspacePath: workspacePath) @@ -233,7 +238,7 @@ public actor SwiftPMBuildSystem { url: URL, toolchainRegistry: ToolchainRegistry, buildSetup: BuildSetup, - forceResolvedVersions: Bool, + isForIndexBuild: Bool, reloadPackageStatusCallback: @escaping (ReloadPackageStatus) async -> Void ) async { do { @@ -242,7 +247,7 @@ public actor SwiftPMBuildSystem { toolchainRegistry: toolchainRegistry, fileSystem: localFileSystem, buildSetup: buildSetup, - forceResolvedVersions: forceResolvedVersions, + isForIndexBuild: isForIndexBuild, reloadPackageStatusCallback: reloadPackageStatusCallback ) } catch Error.noManifest { @@ -271,7 +276,7 @@ extension SwiftPMBuildSystem { let modulesGraph = try self.workspace.loadPackageGraph( rootInput: PackageGraphRootInput(packages: [AbsolutePath(projectRoot)]), - forceResolvedVersions: forceResolvedVersions, + forceResolvedVersions: !isForIndexBuild, availableLibraries: self.buildParameters.toolchain.providedLibraries, observabilityScope: observabilitySystem.topScope ) @@ -292,7 +297,7 @@ extension SwiftPMBuildSystem { self.targets = Dictionary( try buildDescription.allTargetsInTopologicalOrder(in: modulesGraph).enumerated().map { (index, target) in - return (key: target.name, (index, target)) + return (key: ConfiguredTarget(target), (index, target)) }, uniquingKeysWith: { first, second in logger.fault("Found two targets with the same name \(first.buildTarget.name)") @@ -365,16 +370,25 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem { return nil } - if configuredTarget.targetID == "" { + if configuredTarget == .forPackageManifest { return try settings(forPackageManifest: path) } - guard let buildTarget = self.targets[configuredTarget.targetID]?.buildTarget else { + guard let buildTarget = self.targets[configuredTarget]?.buildTarget else { logger.error("Did not find target with name \(configuredTarget.targetID)") return nil } - if url.pathExtension == "h", let substituteFile = buildTarget.sources.first { + if !buildTarget.sources.contains(url), + let substituteFile = buildTarget.sources.sorted(by: { $0.path < $1.path }).first + { + // If `url` is not part of the target's source, it's most likely a header file. Fake compiler arguments for it + // from a substitute file within the target. + // Even if the file is not a header, this should give reasonable results: Say, there was a new `.cpp` file in a + // target and for some reason the `SwiftPMBuildSystem` doesn’t know about it. Then we would infer the target based + // on the file's location on disk and generate compiler arguments for it by picking a source file in that target, + // getting its compiler arguments and then patching up the compiler arguments by replacing the substitute file + // with the `.cpp` file. return FileBuildSettings( compilerArguments: try buildTarget.compileArguments(for: substituteFile), workingDirectory: workspacePath.pathString @@ -387,7 +401,7 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem { ) } - public func defaultLanguage(for document: DocumentURI) async -> Language? { + public func defaultLanguage(for document: DocumentURI) -> Language? { // TODO (indexing): Query The SwiftPM build system for the document's language. // https://github.com/apple/sourcekit-lsp/issues/1267 return nil @@ -400,16 +414,16 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem { } if let target = try? buildTarget(for: path) { - return [ConfiguredTarget(targetID: target.name, runDestinationID: "dummy")] + return [ConfiguredTarget(target)] } if path.basename == "Package.swift" { // We use an empty target name to represent the package manifest since an empty target name is not valid for any // user-defined target. - return [ConfiguredTarget(targetID: "", runDestinationID: "dummy")] + return [ConfiguredTarget.forPackageManifest] } - if url.pathExtension == "h", let target = try? target(forHeader: path) { + if let target = try? inferredTarget(for: path) { return [target] } @@ -418,18 +432,41 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem { public func topologicalSort(of targets: [ConfiguredTarget]) -> [ConfiguredTarget]? { return targets.sorted { (lhs: ConfiguredTarget, rhs: ConfiguredTarget) -> Bool in - let lhsIndex = self.targets[lhs.targetID]?.index ?? self.targets.count - let rhsIndex = self.targets[lhs.targetID]?.index ?? self.targets.count + let lhsIndex = self.targets[lhs]?.index ?? self.targets.count + let rhsIndex = self.targets[lhs]?.index ?? self.targets.count return lhsIndex < rhsIndex } } + public func targets(dependingOn targets: [ConfiguredTarget]) -> [ConfiguredTarget]? { + let targetIndices = targets.compactMap { self.targets[$0]?.index } + let minimumTargetIndex: Int? + if targetIndices.count == targets.count { + minimumTargetIndex = targetIndices.min() + } else { + // One of the targets didn't have an entry in self.targets. We don't know what might depend on it. + minimumTargetIndex = nil + } + + // Files that occur before the target in the topological sorting don't depend on it. + // Ideally, we should consult the dependency graph here for more accurate dependency analysis instead of relying on + // a flattened list (https://github.com/apple/sourcekit-lsp/issues/1312). + return self.targets.compactMap { (configuredTarget, value) -> ConfiguredTarget? in + if let minimumTargetIndex, value.index <= minimumTargetIndex { + return nil + } + return configuredTarget + } + } + public func prepare(targets: [ConfiguredTarget]) async throws { // TODO (indexing): Support preparation of multiple targets at once. // https://github.com/apple/sourcekit-lsp/issues/1262 for target in targets { try await prepare(singleTarget: target) } + let filesInPreparedTargets = targets.flatMap { self.targets[$0]?.buildTarget.sources ?? [] } + await fileDependenciesUpdatedDebouncer.scheduleCall(Set(filesInPreparedTargets.map(DocumentURI.init))) } private func prepare(singleTarget target: ConfiguredTarget) async throws { @@ -561,9 +598,9 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem { // The file watching here is somewhat fragile as well because it assumes that the `.swiftmodule` files are being // written to a directory within the workspace root. This is not necessarily true if the user specifies a build // directory outside the source tree. - // All of this shouldn't be necessary once we have background preparation, in which case we know when preparation of - // a target has finished. - if events.contains(where: { $0.uri.fileURL?.pathExtension == "swiftmodule" }) { + // If we have background indexing enabled, this is not necessary because we call `fileDependenciesUpdated` when + // preparation of a target finishes. + if !isForIndexBuild, events.contains(where: { $0.uri.fileURL?.pathExtension == "swiftmodule" }) { filesWithUpdatedDependencies.formUnion(self.fileToTarget.keys.map { DocumentURI($0.asURL) }) } await self.fileDependenciesUpdatedDebouncer.scheduleCall(filesWithUpdatedDependencies) @@ -615,13 +652,15 @@ extension SwiftPMBuildSystem { return canonicalPath == path ? nil : impl(canonicalPath) } - /// This finds the target the header belongs to based on its location in the file system. - private func target(forHeader path: AbsolutePath) throws -> ConfiguredTarget? { + /// This finds the target a file belongs to based on its location in the file system. + /// + /// This is primarily intended to find the target a header belongs to. + private func inferredTarget(for path: AbsolutePath) throws -> ConfiguredTarget? { func impl(_ path: AbsolutePath) throws -> ConfiguredTarget? { var dir = path.parentDirectory while !dir.isRoot { if let buildTarget = sourceDirToTarget[dir] { - return ConfiguredTarget(targetID: buildTarget.name, runDestinationID: "dummy") + return ConfiguredTarget(buildTarget) } dir = dir.parentDirectory } diff --git a/Sources/SKTestSupport/DefaultSDKPath.swift b/Sources/SKTestSupport/DefaultSDKPath.swift new file mode 100644 index 000000000..eb64d55ab --- /dev/null +++ b/Sources/SKTestSupport/DefaultSDKPath.swift @@ -0,0 +1,37 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2014 - 2024 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +import class TSCBasic.Process + +#if !os(macOS) +import Foundation +#endif + +private func xcrunMacOSSDKPath() -> String? { + guard var path = try? Process.checkNonZeroExit(arguments: ["/usr/bin/xcrun", "--show-sdk-path", "--sdk", "macosx"]) + else { + return nil + } + if path.last == "\n" { + path = String(path.dropLast()) + } + return path +} + +/// The default sdk path to use. +public let defaultSDKPath: String? = { + #if os(macOS) + return xcrunMacOSSDKPath() + #else + return ProcessInfo.processInfo.environment["SDKROOT"] + #endif +}() diff --git a/Sources/SKTestSupport/IndexedSingleSwiftFileTestProject.swift b/Sources/SKTestSupport/IndexedSingleSwiftFileTestProject.swift index cae4aef53..9a2d3b833 100644 --- a/Sources/SKTestSupport/IndexedSingleSwiftFileTestProject.swift +++ b/Sources/SKTestSupport/IndexedSingleSwiftFileTestProject.swift @@ -11,7 +11,6 @@ //===----------------------------------------------------------------------===// import Foundation -import ISDBTibs import LanguageServerProtocol @_spi(Testing) import SKCore import SourceKitLSP @@ -66,7 +65,7 @@ public struct IndexedSingleSwiftFileTestProject { compilerArguments.append("-index-ignore-system-modules") } - if let sdk = TibsBuilder.defaultSDKPath { + if let sdk = defaultSDKPath { compilerArguments += ["-sdk", sdk] // The following are needed so we can import XCTest diff --git a/Sources/SKTestSupport/MultiFileTestProject.swift b/Sources/SKTestSupport/MultiFileTestProject.swift index ea300f497..e16d8846a 100644 --- a/Sources/SKTestSupport/MultiFileTestProject.swift +++ b/Sources/SKTestSupport/MultiFileTestProject.swift @@ -84,6 +84,7 @@ public class MultiFileTestProject { serverOptions: SourceKitLSPServer.Options = .testDefault, usePullDiagnostics: Bool = true, preInitialization: ((TestSourceKitLSPClient) -> Void)? = nil, + cleanUp: (() -> Void)? = nil, testName: String = #function ) async throws { scratchDirectory = try testScratchDir(testName: testName) @@ -122,6 +123,7 @@ public class MultiFileTestProject { if cleanScratchDirectories { try? FileManager.default.removeItem(at: scratchDirectory) } + cleanUp?() } ) } diff --git a/Sources/SKTestSupport/SkipUnless.swift b/Sources/SKTestSupport/SkipUnless.swift index b8b5b9410..803b8a507 100644 --- a/Sources/SKTestSupport/SkipUnless.swift +++ b/Sources/SKTestSupport/SkipUnless.swift @@ -27,14 +27,16 @@ import enum TSCBasic.ProcessEnv // MARK: - Skip checks /// Namespace for functions that are used to skip unsupported tests. -public enum SkipUnless { +public actor SkipUnless { private enum FeatureCheckResult { case featureSupported case featureUnsupported(skipMessage: String) } + private static let shared = SkipUnless() + /// For any feature that has already been evaluated, the result of whether or not it should be skipped. - private static var checkCache: [String: FeatureCheckResult] = [:] + private var checkCache: [String: FeatureCheckResult] = [:] /// Throw an `XCTSkip` if any of the following conditions hold /// - The Swift version of the toolchain used for testing (`ToolchainRegistry.forTesting.default`) is older than @@ -49,7 +51,7 @@ public enum SkipUnless { /// /// Independently of these checks, the tests are never skipped in Swift CI (identified by the presence of the `SWIFTCI_USE_LOCAL_DEPS` environment). Swift CI is assumed to always build its own toolchain, which is thus /// guaranteed to be up-to-date. - private static func skipUnlessSupportedByToolchain( + private func skipUnlessSupportedByToolchain( swiftVersion: SwiftVersion, featureName: String = #function, file: StaticString, @@ -96,10 +98,10 @@ public enum SkipUnless { } public static func sourcekitdHasSemanticTokensRequest( - file: StaticString = #file, + file: StaticString = #filePath, line: UInt = #line ) async throws { - try await skipUnlessSupportedByToolchain(swiftVersion: SwiftVersion(5, 11), file: file, line: line) { + try await shared.skipUnlessSupportedByToolchain(swiftVersion: SwiftVersion(5, 11), file: file, line: line) { let testClient = try await TestSourceKitLSPClient() let uri = DocumentURI.for(.swift) testClient.openDocument("0.bitPattern", uri: uri) @@ -127,10 +129,10 @@ public enum SkipUnless { } public static func sourcekitdSupportsRename( - file: StaticString = #file, + file: StaticString = #filePath, line: UInt = #line ) async throws { - try await skipUnlessSupportedByToolchain(swiftVersion: SwiftVersion(5, 11), file: file, line: line) { + try await shared.skipUnlessSupportedByToolchain(swiftVersion: SwiftVersion(5, 11), file: file, line: line) { let testClient = try await TestSourceKitLSPClient() let uri = DocumentURI.for(.swift) let positions = testClient.openDocument("func 1️⃣test() {}", uri: uri) @@ -147,10 +149,10 @@ public enum SkipUnless { /// Whether clangd has support for the `workspace/indexedRename` request. public static func clangdSupportsIndexBasedRename( - file: StaticString = #file, + file: StaticString = #filePath, line: UInt = #line ) async throws { - try await skipUnlessSupportedByToolchain(swiftVersion: SwiftVersion(5, 11), file: file, line: line) { + try await shared.skipUnlessSupportedByToolchain(swiftVersion: SwiftVersion(5, 11), file: file, line: line) { let testClient = try await TestSourceKitLSPClient() let uri = DocumentURI.for(.c) let positions = testClient.openDocument("void 1️⃣test() {}", uri: uri) @@ -177,10 +179,10 @@ public enum SkipUnless { /// toolchain’s SwiftPM stores the Swift modules on the top level but we synthesize compiler arguments expecting the /// modules to be in a `Modules` subdirectory. public static func swiftpmStoresModulesInSubdirectory( - file: StaticString = #file, + file: StaticString = #filePath, line: UInt = #line ) async throws { - try await skipUnlessSupportedByToolchain(swiftVersion: SwiftVersion(5, 11), file: file, line: line) { + try await shared.skipUnlessSupportedByToolchain(swiftVersion: SwiftVersion(5, 11), file: file, line: line) { let workspace = try await SwiftPMTestProject( files: ["test.swift": ""], build: true @@ -195,21 +197,21 @@ public enum SkipUnless { } public static func toolchainContainsSwiftFormat( - file: StaticString = #file, + file: StaticString = #filePath, line: UInt = #line ) async throws { - try await skipUnlessSupportedByToolchain(swiftVersion: SwiftVersion(5, 11), file: file, line: line) { + try await shared.skipUnlessSupportedByToolchain(swiftVersion: SwiftVersion(5, 11), file: file, line: line) { return await ToolchainRegistry.forTesting.default?.swiftFormat != nil } } public static func sourcekitdReturnsRawDocumentationResponse( - file: StaticString = #file, + file: StaticString = #filePath, line: UInt = #line ) async throws { struct ExpectedMarkdownContentsError: Error {} - return try await skipUnlessSupportedByToolchain(swiftVersion: SwiftVersion(6, 0), file: file, line: line) { + return try await shared.skipUnlessSupportedByToolchain(swiftVersion: SwiftVersion(6, 0), file: file, line: line) { // The XML-based doc comment conversion did not preserve `Precondition`. let testClient = try await TestSourceKitLSPClient() let uri = DocumentURI.for(.swift) @@ -235,10 +237,10 @@ public enum SkipUnless { /// Checks whether the index contains a fix that prevents it from adding relations to non-indexed locals /// (https://github.com/apple/swift/pull/72930). public static func indexOnlyHasContainedByRelationsToIndexedDecls( - file: StaticString = #file, + file: StaticString = #filePath, line: UInt = #line ) async throws { - return try await skipUnlessSupportedByToolchain(swiftVersion: SwiftVersion(6, 0), file: file, line: line) { + return try await shared.skipUnlessSupportedByToolchain(swiftVersion: SwiftVersion(6, 0), file: file, line: line) { let project = try await IndexedSingleSwiftFileTestProject( """ func foo() {} diff --git a/Sources/SKTestSupport/SwiftPMTestProject.swift b/Sources/SKTestSupport/SwiftPMTestProject.swift index abd4a20d2..28944fb2a 100644 --- a/Sources/SKTestSupport/SwiftPMTestProject.swift +++ b/Sources/SKTestSupport/SwiftPMTestProject.swift @@ -47,6 +47,7 @@ public class SwiftPMTestProject: MultiFileTestProject { pollIndex: Bool = true, preInitialization: ((TestSourceKitLSPClient) -> Void)? = nil, usePullDiagnostics: Bool = true, + cleanUp: (() -> Void)? = nil, testName: String = #function ) async throws { var filesByPath: [RelativeFileLocation: String] = [:] @@ -72,6 +73,7 @@ public class SwiftPMTestProject: MultiFileTestProject { serverOptions: serverOptions, usePullDiagnostics: usePullDiagnostics, preInitialization: preInitialization, + cleanUp: cleanUp, testName: testName ) diff --git a/Sources/SKTestSupport/TestSourceKitLSPClient.swift b/Sources/SKTestSupport/TestSourceKitLSPClient.swift index f3854c039..b3f348194 100644 --- a/Sources/SKTestSupport/TestSourceKitLSPClient.swift +++ b/Sources/SKTestSupport/TestSourceKitLSPClient.swift @@ -10,6 +10,7 @@ // //===----------------------------------------------------------------------===// +import CAtomics import Foundation import LSPTestSupport import LanguageServerProtocol @@ -22,7 +23,7 @@ import XCTest extension SourceKitLSPServer.Options { /// The default SourceKitLSPServer options for testing. - public static var testDefault = Self(swiftPublishDiagnosticsDebounceDuration: 0) + public static let testDefault = Self(swiftPublishDiagnosticsDebounceDuration: 0) } /// A mock SourceKit-LSP client (aka. a mock editor) that behaves like an editor @@ -35,7 +36,8 @@ public final class TestSourceKitLSPClient: MessageHandler { public typealias RequestHandler = (Request) -> Request.Response /// The ID that should be assigned to the next request sent to the `server`. - private var nextRequestID: Int = 0 + /// `nonisolated(unsafe)` is fine because `nextRequestID` is atomic. + private nonisolated(unsafe) var nextRequestID = AtomicUInt32(initialValue: 0) /// If the server is not using the global module cache, the path of the local /// module cache. @@ -66,12 +68,12 @@ public final class TestSourceKitLSPClient: MessageHandler { /// /// Conceptually, this is an array of `RequestHandler` but /// since we can't express this in the Swift type system, we use `[Any]`. - private var requestHandlers: [Any] = [] + private nonisolated(unsafe) var requestHandlers = ThreadSafeBox<[Any]>(initialValue: []) /// A closure that is called when the `TestSourceKitLSPClient` is destructed. /// /// This allows e.g. a `IndexedSingleSwiftFileTestProject` to delete its temporary files when they are no longer needed. - private let cleanUp: () -> Void + private let cleanUp: @Sendable () -> Void /// - Parameters: /// - serverOptions: The equivalent of the command line options with which sourcekit-lsp should be started @@ -97,7 +99,7 @@ public final class TestSourceKitLSPClient: MessageHandler { usePullDiagnostics: Bool = true, workspaceFolders: [WorkspaceFolder]? = nil, preInitialization: ((TestSourceKitLSPClient) -> Void)? = nil, - cleanUp: @escaping () -> Void = {} + cleanUp: @Sendable @escaping () -> Void = {} ) async throws { if !useGlobalModuleCache { moduleCache = URL(fileURLWithPath: NSTemporaryDirectory()).appendingPathComponent(UUID().uuidString) @@ -115,7 +117,7 @@ public final class TestSourceKitLSPClient: MessageHandler { } self.notificationYielder = notificationYielder - let serverToClientConnection = LocalConnection() + let serverToClientConnection = LocalConnection(name: "client") self.serverToClientConnection = serverToClientConnection server = SourceKitLSPServer( client: serverToClientConnection, @@ -169,8 +171,7 @@ public final class TestSourceKitLSPClient: MessageHandler { // It's really unfortunate that there are no async deinits. If we had async // deinits, we could await the sending of a ShutdownRequest. let sema = DispatchSemaphore(value: 0) - nextRequestID += 1 - server.handle(ShutdownRequest(), id: .number(nextRequestID)) { result in + server.handle(ShutdownRequest(), id: .number(Int(nextRequestID.fetchAndIncrement()))) { result in sema.signal() } sema.wait() @@ -186,9 +187,8 @@ public final class TestSourceKitLSPClient: MessageHandler { /// Send the request to `server` and return the request result. public func send(_ request: R) async throws -> R.Response { - nextRequestID += 1 return try await withCheckedThrowingContinuation { continuation in - server.handle(request, id: .number(self.nextRequestID)) { result in + server.handle(request, id: .number(Int(nextRequestID.fetchAndIncrement()))) { result in continuation.resume(with: result) } } @@ -273,7 +273,7 @@ public final class TestSourceKitLSPClient: MessageHandler { /// If the next request that is sent to the client is of a different kind than /// the given handler, `TestSourceKitLSPClient` will emit an `XCTFail`. public func handleNextRequest(_ requestHandler: @escaping RequestHandler) { - requestHandlers.append(requestHandler) + requestHandlers.value.append(requestHandler) } // MARK: - Conformance to MessageHandler @@ -290,19 +290,21 @@ public final class TestSourceKitLSPClient: MessageHandler { id: LanguageServerProtocol.RequestID, reply: @escaping (LSPResult) -> Void ) { - let requestHandlerAndIndex = requestHandlers.enumerated().compactMap { - (index, handler) -> (RequestHandler, Int)? in - guard let handler = handler as? RequestHandler else { - return nil + requestHandlers.withLock { requestHandlers in + let requestHandlerAndIndex = requestHandlers.enumerated().compactMap { + (index, handler) -> (RequestHandler, Int)? in + guard let handler = handler as? RequestHandler else { + return nil + } + return (handler, index) + }.first + guard let (requestHandler, index) = requestHandlerAndIndex else { + reply(.failure(.methodNotFound(Request.method))) + return } - return (handler, index) - }.first - guard let (requestHandler, index) = requestHandlerAndIndex else { - reply(.failure(.methodNotFound(Request.method))) - return + reply(.success(requestHandler(params))) + requestHandlers.remove(at: index) } - reply(.success(requestHandler(params))) - requestHandlers.remove(at: index) } // MARK: - Convenience functions @@ -396,8 +398,10 @@ public struct DocumentPositions { /// /// This allows us to set the ``TestSourceKitLSPClient`` as the message handler of /// `SourceKitLSPServer` without retaining it. -private class WeakMessageHandler: MessageHandler { - private weak var handler: (any MessageHandler)? +private final class WeakMessageHandler: MessageHandler, Sendable { + // `nonisolated(unsafe)` is fine because `handler` is never modified, only if the weak reference is deallocated, which + // is atomic. + private nonisolated(unsafe) weak var handler: (any MessageHandler)? init(_ handler: any MessageHandler) { self.handler = handler @@ -410,7 +414,7 @@ private class WeakMessageHandler: MessageHandler { func handle( _ params: Request, id: LanguageServerProtocol.RequestID, - reply: @escaping (LanguageServerProtocol.LSPResult) -> Void + reply: @Sendable @escaping (LanguageServerProtocol.LSPResult) -> Void ) { guard let handler = handler else { reply(.failure(.unknown("Handler has been deallocated"))) diff --git a/Sources/SKTestSupport/Utils.swift b/Sources/SKTestSupport/Utils.swift index cebdc730d..f55ed78ae 100644 --- a/Sources/SKTestSupport/Utils.swift +++ b/Sources/SKTestSupport/Utils.swift @@ -25,8 +25,12 @@ extension Language { init?(fileExtension: String) { switch fileExtension { + case "c": self = .c + case "cpp": self = .cpp case "m": self = .objective_c - default: self.init(rawValue: fileExtension) + case "mm": self = .objective_cpp + case "swift": self = .swift + default: return nil } } } @@ -70,7 +74,7 @@ public func testScratchDir(testName: String = #function) throws -> URL { /// The temporary directory will be deleted at the end of `directory` unless the /// `SOURCEKITLSP_KEEP_TEST_SCRATCH_DIR` environment variable is set. public func withTestScratchDir( - _ body: (AbsolutePath) async throws -> T, + @_inheritActorContext _ body: @Sendable (AbsolutePath) async throws -> T, testName: String = #function ) async throws -> T { let scratchDirectory = try testScratchDir(testName: testName) diff --git a/Sources/SKTestSupport/WrappedSemaphore.swift b/Sources/SKTestSupport/WrappedSemaphore.swift index ee2036557..cdbf3be6b 100644 --- a/Sources/SKTestSupport/WrappedSemaphore.swift +++ b/Sources/SKTestSupport/WrappedSemaphore.swift @@ -17,8 +17,8 @@ import Dispatch /// /// This should only be used for tests that test priority escalation and thus cannot await a `Task` (which would cause /// priority elevations). -public struct WrappedSemaphore { - let semaphore = DispatchSemaphore(value: 0) +public struct WrappedSemaphore: Sendable { + private let semaphore = DispatchSemaphore(value: 0) public init() {} diff --git a/Sources/SemanticIndex/CMakeLists.txt b/Sources/SemanticIndex/CMakeLists.txt index 197bfde6e..b087edaeb 100644 --- a/Sources/SemanticIndex/CMakeLists.txt +++ b/Sources/SemanticIndex/CMakeLists.txt @@ -1,9 +1,12 @@ add_library(SemanticIndex STATIC CheckedIndex.swift + CompilerCommandLineOption.swift + IndexStatusManager.swift IndexTaskDescription.swift PreparationTaskDescription.swift SemanticIndexManager.swift + TestHooks.swift UpdateIndexStoreTaskDescription.swift ) set_target_properties(SemanticIndex PROPERTIES diff --git a/Sources/SemanticIndex/CheckedIndex.swift b/Sources/SemanticIndex/CheckedIndex.swift index d6a6da1ae..c8953ae07 100644 --- a/Sources/SemanticIndex/CheckedIndex.swift +++ b/Sources/SemanticIndex/CheckedIndex.swift @@ -104,10 +104,6 @@ public final class CheckedIndex { } } - public func symbolProvider(for sourceFilePath: String) -> SymbolProviderKind? { - return index.symbolProvider(for: sourceFilePath) - } - public func symbols(inFilePath path: String) -> [Symbol] { guard self.hasUpToDateUnit(for: URL(fileURLWithPath: path, isDirectory: false)) else { return [] @@ -120,6 +116,19 @@ public final class CheckedIndex { return index.unitTests(referencedByMainFiles: mainFilePaths).filter { checker.isUpToDate($0.location) } } + /// Returns all the files that (transitively) include the header file at the given path. + /// + /// If `crossLanguage` is set to `true`, Swift files that import a header through a module will also be reported. + public func mainFilesContainingFile(uri: DocumentURI, crossLanguage: Bool = false) -> [DocumentURI] { + return index.mainFilesContainingFile(path: uri.pseudoPath, crossLanguage: crossLanguage).compactMap { + let url = URL(fileURLWithPath: $0) + guard checker.indexHasUpToDateUnit(for: url, mainFile: nil, index: self.index) else { + return nil + } + return DocumentURI(url) + } + } + /// Returns all unit test symbols in the index. public func unitTests() -> [SymbolOccurrence] { return index.unitTests().filter { checker.isUpToDate($0.location) } @@ -128,8 +137,12 @@ public final class CheckedIndex { /// Return `true` if a unit file has been indexed for the given file path after its last modification date. /// /// This means that at least a single build configuration of this file has been indexed since its last modification. - public func hasUpToDateUnit(for url: URL) -> Bool { - return checker.indexHasUpToDateUnit(for: url, index: index) + /// + /// If `mainFile` is passed, then `url` is a header file that won't have a unit associated with it. `mainFile` is + /// assumed to be a file that imports `url`. To check that `url` has an up-to-date unit, check that the latest unit + /// for `mainFile` is newer than the mtime of the header file at `url`. + public func hasUpToDateUnit(for url: URL, mainFile: URL? = nil) -> Bool { + return checker.indexHasUpToDateUnit(for: url, mainFile: mainFile, index: index) } /// Returns true if the file at the given URL has a different content in the document manager than on-disk. This is @@ -139,11 +152,6 @@ public final class CheckedIndex { public func fileHasInMemoryModifications(_ url: URL) -> Bool { return checker.fileHasInMemoryModifications(url) } - - /// Wait for IndexStoreDB to be updated based on new unit files written to disk. - public func pollForUnitChangesAndWait() { - self.index.pollForUnitChangesAndWait() - } } /// A wrapper around `IndexStoreDB` that allows the retrieval of a `CheckedIndex` with a specified check level or the @@ -167,6 +175,15 @@ public struct UncheckedIndex: Sendable { public func checked(for checkLevel: IndexCheckLevel) -> CheckedIndex { return CheckedIndex(index: underlyingIndexStoreDB, checkLevel: checkLevel) } + + public func symbolProvider(for sourceFilePath: String) -> SymbolProviderKind? { + return underlyingIndexStoreDB.symbolProvider(for: sourceFilePath) + } + + /// Wait for IndexStoreDB to be updated based on new unit files written to disk. + public func pollForUnitChangesAndWait() { + self.underlyingIndexStoreDB.pollForUnitChangesAndWait() + } } /// Helper class to check if symbols from the index are up-to-date or if the source file has been modified after it was @@ -241,7 +258,11 @@ private struct IndexOutOfDateChecker { /// Return `true` if a unit file has been indexed for the given file path after its last modification date. /// /// This means that at least a single build configuration of this file has been indexed since its last modification. - mutating func indexHasUpToDateUnit(for filePath: URL, index: IndexStoreDB) -> Bool { + /// + /// If `mainFile` is passed, then `filePath` is a header file that won't have a unit associated with it. `mainFile` is + /// assumed to be a file that imports `url`. To check that `url` has an up-to-date unit, check that the latest unit + /// for `mainFile` is newer than the mtime of the header file at `url`. + mutating func indexHasUpToDateUnit(for filePath: URL, mainFile: URL?, index: IndexStoreDB) -> Bool { switch checkLevel { case .inMemoryModifiedFiles(let documentManager): if fileHasInMemoryModifications(filePath, documentManager: documentManager) { @@ -252,7 +273,7 @@ private struct IndexOutOfDateChecker { // If there are no in-memory modifications check if there are on-disk modifications. fallthrough case .modifiedFiles: - guard let lastUnitDate = index.dateOfLatestUnitFor(filePath: filePath.path) else { + guard let lastUnitDate = index.dateOfLatestUnitFor(filePath: (mainFile ?? filePath).path) else { return false } do { diff --git a/Sources/SemanticIndex/CompilerCommandLineOption.swift b/Sources/SemanticIndex/CompilerCommandLineOption.swift new file mode 100644 index 000000000..2eb26503a --- /dev/null +++ b/Sources/SemanticIndex/CompilerCommandLineOption.swift @@ -0,0 +1,128 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2014 - 2024 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +@_spi(Testing) public struct CompilerCommandLineOption { + /// Return value of `matches(argument:)`. + public enum Match { + /// The `CompilerCommandLineOption` matched the command line argument. The next element in the command line is a + /// separate argument and should not be removed. + case removeOption + + /// The `CompilerCommandLineOption` matched the command line argument. The next element in the command line is an + /// argument to this option and should be removed as well. + case removeOptionAndNextArgument + } + + public enum DashSpelling { + case singleDash + case doubleDash + } + + public enum ArgumentStyles { + /// A command line option where arguments can be passed without a space such as `-MT/file.txt`. + case noSpace + /// A command line option where the argument is passed, separated by a space (eg. `--serialize-diagnostics /file.txt`) + case separatedBySpace + /// A command line option where the argument is passed after a `=`, eg. `-fbuild-session-file=`. + case separatedByEqualSign + } + + /// The name of the option, without any preceeding `-` or `--`. + private let name: String + + /// Whether the option can be spelled with one or two dashes. + private let dashSpellings: [DashSpelling] + + /// The ways that arguments can specified after the option. Empty if the option is a flag that doesn't take any + /// argument. + private let argumentStyles: [ArgumentStyles] + + public static func flag(_ name: String, _ dashSpellings: [DashSpelling]) -> CompilerCommandLineOption { + precondition(!dashSpellings.isEmpty) + return CompilerCommandLineOption(name: name, dashSpellings: dashSpellings, argumentStyles: []) + } + + public static func option( + _ name: String, + _ dashSpellings: [DashSpelling], + _ argumentStyles: [ArgumentStyles] + ) -> CompilerCommandLineOption { + precondition(!dashSpellings.isEmpty) + precondition(!argumentStyles.isEmpty) + return CompilerCommandLineOption(name: name, dashSpellings: dashSpellings, argumentStyles: argumentStyles) + } + + public func matches(argument: String) -> Match? { + let argumentName: Substring + if argument.hasPrefix("--") { + if dashSpellings.contains(.doubleDash) { + argumentName = argument.dropFirst(2) + } else { + return nil + } + } else if argument.hasPrefix("-") { + if dashSpellings.contains(.singleDash) { + argumentName = argument.dropFirst(1) + } else { + return nil + } + } else { + return nil + } + guard argumentName.hasPrefix(self.name) else { + // Fast path in case the argument doesn't match. + return nil + } + + // Examples: + // - self.name: "emit-module", argument: "-emit-module", then textAfterArgumentName: "" + // - self.name: "o", argument: "-o", then textAfterArgumentName: "" + // - self.name: "o", argument: "-output-file-map", then textAfterArgumentName: "utput-file-map" + // - self.name: "MT", argument: "-MT/path/to/depfile", then textAfterArgumentName: "/path/to/depfile" + // - self.name: "fbuild-session-file", argument: "-fbuild-session-file=/path/to/file", then textAfterArgumentName: "=/path/to/file" + let textAfterArgumentName: Substring = argumentName.dropFirst(self.name.count) + + if argumentStyles.isEmpty { + if textAfterArgumentName.isEmpty { + return .removeOption + } + // The command line option is a flag but there is text remaining after the argument name. Thus the flag didn't + // match. Eg. self.name: "o" and argument: "-output-file-map" + return nil + } + + for argumentStyle in argumentStyles { + switch argumentStyle { + case .noSpace where !textAfterArgumentName.isEmpty: + return .removeOption + case .separatedBySpace where textAfterArgumentName.isEmpty: + return .removeOptionAndNextArgument + case .separatedByEqualSign where textAfterArgumentName.hasPrefix("="): + return .removeOption + default: + break + } + } + return nil + } +} + +extension Array { + func firstMatch(for argument: String) -> CompilerCommandLineOption.Match? { + for optionToRemove in self { + if let match = optionToRemove.matches(argument: argument) { + return match + } + } + return nil + } +} diff --git a/Sources/SemanticIndex/IndexStatusManager.swift b/Sources/SemanticIndex/IndexStatusManager.swift new file mode 100644 index 000000000..016243bf8 --- /dev/null +++ b/Sources/SemanticIndex/IndexStatusManager.swift @@ -0,0 +1,70 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2014 - 2024 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +import Foundation +import SKCore + +/// Keeps track of whether an item (a target or file to index) is up-to-date. +actor IndexUpToDateStatusManager { + private enum Status { + /// The item is up-to-date. + case upToDate + + /// The target or file has been marked out-of-date at the given date. + /// + /// Keeping track of the date is necessary so that we don't mark a target as up-to-date if we have the following + /// ordering of events: + /// - Preparation started + /// - Target marked out of date + /// - Preparation finished + case outOfDate(Date) + } + + private var status: [Item: Status] = [:] + + /// Mark the target or file as up-to-date from a preparation/update-indexstore operation started at + /// `updateOperationStartDate`. + /// + /// See comment on `Status.outOfDate` why `updateOperationStartDate` needs to be passed. + func markUpToDate(_ items: [Item], updateOperationStartDate: Date) { + for item in items { + switch status[item] { + case .upToDate: + break + case .outOfDate(let markedOutOfDate): + if markedOutOfDate < updateOperationStartDate { + status[item] = .upToDate + } + case nil: + status[item] = .upToDate + } + } + } + + func markOutOfDate(_ items: some Collection) { + let date = Date() + for item in items { + status[item] = .outOfDate(date) + } + } + + func markAllOutOfDate() { + markOutOfDate(status.keys) + } + + func isUpToDate(_ item: Item) -> Bool { + if case .upToDate = status[item] { + return true + } + return false + } +} diff --git a/Sources/SemanticIndex/PreparationTaskDescription.swift b/Sources/SemanticIndex/PreparationTaskDescription.swift index 7765c63e3..049ce896c 100644 --- a/Sources/SemanticIndex/PreparationTaskDescription.swift +++ b/Sources/SemanticIndex/PreparationTaskDescription.swift @@ -30,11 +30,16 @@ public struct PreparationTaskDescription: IndexTaskDescription { public let id = preparationIDForLogging.fetchAndIncrement() /// The targets that should be prepared. - private let targetsToPrepare: [ConfiguredTarget] + public let targetsToPrepare: [ConfiguredTarget] /// The build system manager that is used to get the toolchain and build settings for the files to index. private let buildSystemManager: BuildSystemManager + private let preparationUpToDateStatus: IndexUpToDateStatusManager + + /// Test hooks that should be called when the preparation task finishes. + private let testHooks: IndexTestHooks + /// The task is idempotent because preparing the same target twice produces the same result as preparing it once. public var isIdempotent: Bool { true } @@ -50,10 +55,14 @@ public struct PreparationTaskDescription: IndexTaskDescription { init( targetsToPrepare: [ConfiguredTarget], - buildSystemManager: BuildSystemManager + buildSystemManager: BuildSystemManager, + preparationUpToDateStatus: IndexUpToDateStatusManager, + testHooks: IndexTestHooks ) { self.targetsToPrepare = targetsToPrepare self.buildSystemManager = buildSystemManager + self.preparationUpToDateStatus = preparationUpToDateStatus + self.testHooks = testHooks } public func execute() async { @@ -61,10 +70,15 @@ public struct PreparationTaskDescription: IndexTaskDescription { // See comment in `withLoggingScope`. // The last 2 digits should be sufficient to differentiate between multiple concurrently running preparation operations await withLoggingScope("preparation-\(id % 100)") { - let startDate = Date() - let targetsToPrepare = targetsToPrepare.sorted(by: { + let targetsToPrepare = await targetsToPrepare.asyncFilter { + await !preparationUpToDateStatus.isUpToDate($0) + }.sorted(by: { ($0.targetID, $0.runDestinationID) < ($1.targetID, $1.runDestinationID) }) + if targetsToPrepare.isEmpty { + return + } + let targetsToPrepareDescription = targetsToPrepare .map { "\($0.targetID)-\($0.runDestinationID)" } @@ -72,6 +86,7 @@ public struct PreparationTaskDescription: IndexTaskDescription { logger.log( "Starting preparation with priority \(Task.currentPriority.rawValue, privacy: .public): \(targetsToPrepareDescription)" ) + let startDate = Date() do { try await buildSystemManager.prepare(targets: targetsToPrepare) } catch { @@ -79,6 +94,10 @@ public struct PreparationTaskDescription: IndexTaskDescription { "Preparation failed: \(error.forLogging)" ) } + await testHooks.preparationTaskDidFinish?(self) + if !Task.isCancelled { + await preparationUpToDateStatus.markUpToDate(targetsToPrepare, updateOperationStartDate: startDate) + } logger.log( "Finished preparation in \(Date().timeIntervalSince(startDate) * 1000, privacy: .public)ms: \(targetsToPrepareDescription)" ) diff --git a/Sources/SemanticIndex/SemanticIndexManager.swift b/Sources/SemanticIndex/SemanticIndexManager.swift index c60a12a3e..6ce61adfc 100644 --- a/Sources/SemanticIndex/SemanticIndexManager.swift +++ b/Sources/SemanticIndex/SemanticIndexManager.swift @@ -15,46 +15,77 @@ import LSPLogging import LanguageServerProtocol import SKCore -/// Describes the state of indexing for a single source file -private enum FileIndexStatus { - /// The index is up-to-date. - case upToDate - /// The file is not up to date and we have scheduled a task to index it but that index operation hasn't been started - /// yet. - case scheduled(Task) - /// We are currently actively indexing this file, ie. we are running a subprocess that indexes the file. - case executing(Task) - - var description: String { - switch self { - case .upToDate: - return "upToDate" - case .scheduled: - return "scheduled" - case .executing: - return "executing" - } +/// A wrapper around `QueuedTask` that only allows equality comparison and inspection whether the `QueuedTask` is +/// currently executing. +/// +/// This way we can store `QueuedTask` in the `inProgress` dictionaries while guaranteeing that whoever created the +/// queued task still has exclusive ownership of the task and can thus control the task's cancellation. +private struct OpaqueQueuedIndexTask: Equatable { + private let task: QueuedTask + + var isExecuting: Bool { + task.isExecuting + } + + init(_ task: QueuedTask) { + self.task = task + } + + static func == (lhs: OpaqueQueuedIndexTask, rhs: OpaqueQueuedIndexTask) -> Bool { + return lhs.task === rhs.task } } +private enum InProgressIndexStore { + /// We are waiting for preparation of the file's target to finish before we can index it. + /// + /// `preparationTaskID` identifies the preparation task so that we can transition a file's index state to + /// `updatingIndexStore` when its preparation task has finished. + /// + /// `indexTask` is a task that finishes after both preparation and index store update are done. Whoever owns the index + /// task is still the sole owner of it and responsible for its cancellation. + case waitingForPreparation(preparationTaskID: UUID, indexTask: Task) + + /// The file's target has been prepared and we are updating the file's index store. + /// + /// `updateIndexStoreTask` is the task that updates the index store itself. + /// + /// `indexTask` is a task that finishes after both preparation and index store update are done. Whoever owns the index + /// task is still the sole owner of it and responsible for its cancellation. + case updatingIndexStore(updateIndexStoreTask: OpaqueQueuedIndexTask, indexTask: Task) +} + /// Schedules index tasks and keeps track of the index status of files. public final actor SemanticIndexManager { /// The underlying index. This is used to check if the index of a file is already up-to-date, in which case it doesn't /// need to be indexed again. - private let index: CheckedIndex + private let index: UncheckedIndex /// The build system manager that is used to get compiler arguments for a file. private let buildSystemManager: BuildSystemManager - /// The index status of the source files that the `SemanticIndexManager` knows about. - /// - /// Files that have never been indexed are not in this dictionary. - private var indexStatus: [DocumentURI: FileIndexStatus] = [:] + private let testHooks: IndexTestHooks /// The task to generate the build graph (resolving package dependencies, generating the build description, /// ...). `nil` if no build graph is currently being generated. private var generateBuildGraphTask: Task? + private let preparationUpToDateStatus = IndexUpToDateStatusManager() + + private let indexStoreUpToDateStatus = IndexUpToDateStatusManager() + + /// The preparation tasks that have been started and are either scheduled in the task scheduler or currently + /// executing. + /// + /// After a preparation task finishes, it is removed from this dictionary. + private var inProgressPreparationTasks: [ConfiguredTarget: OpaqueQueuedIndexTask] = [:] + + /// The files that are currently being index, either waiting for their target to be prepared, waiting for the index + /// store update task to be scheduled in the task scheduler or which currently have an index store update running. + /// + /// After the file is indexed, it is removed from this dictionary. + private var inProgressIndexTasks: [DocumentURI: InProgressIndexStore] = [:] + /// The `TaskScheduler` that manages the scheduling of index tasks. This is shared among all `SemanticIndexManager`s /// in the process, to ensure that we don't schedule more index operations than processor cores from multiple /// workspaces. @@ -77,51 +108,76 @@ public final actor SemanticIndexManager { /// The files that still need to be indexed. /// - /// See `FileIndexStatus` for the distinction between `scheduled` and `executing`. - public var inProgressIndexTasks: (scheduled: [DocumentURI], executing: [DocumentURI]) { - let scheduled = indexStatus.compactMap { (uri: DocumentURI, status: FileIndexStatus) in - if case .scheduled = status { - return uri + /// Scheduled tasks are files that are waiting for their target to be prepared or whose index store update task is + /// waiting to be scheduled by the task scheduler. + /// + /// `executing` are the files that currently have an active index store update task running. + public var inProgressIndexFiles: (scheduled: [DocumentURI], executing: [DocumentURI]) { + var scheduled: [DocumentURI] = [] + var executing: [DocumentURI] = [] + for (uri, status) in inProgressIndexTasks { + let isExecuting: Bool + switch status { + case .waitingForPreparation: + isExecuting = false + case .updatingIndexStore(updateIndexStoreTask: let updateIndexStoreTask, indexTask: _): + isExecuting = updateIndexStoreTask.isExecuting } - return nil - } - let inProgress = indexStatus.compactMap { (uri: DocumentURI, status: FileIndexStatus) in - if case .executing = status { - return uri + + if isExecuting { + executing.append(uri) + } else { + scheduled.append(uri) } - return nil } - return (scheduled, inProgress) + + return (scheduled, executing) } public init( index: UncheckedIndex, buildSystemManager: BuildSystemManager, + testHooks: IndexTestHooks, indexTaskScheduler: TaskScheduler, indexTasksWereScheduled: @escaping @Sendable (Int) -> Void, indexTaskDidFinish: @escaping @Sendable () -> Void ) { - self.index = index.checked(for: .modifiedFiles) + self.index = index self.buildSystemManager = buildSystemManager + self.testHooks = testHooks self.indexTaskScheduler = indexTaskScheduler self.indexTasksWereScheduled = indexTasksWereScheduled self.indexTaskDidFinish = indexTaskDidFinish } - /// Schedules a task to index all files in `files` that don't already have an up-to-date index. + /// Schedules a task to index `files`. Files that are known to be up-to-date based on `indexStatus` will + /// not be re-indexed. The method will re-index files even if they have a unit with a timestamp that matches the + /// source file's mtime. This allows re-indexing eg. after compiler arguments or dependencies have changed. + /// /// Returns immediately after scheduling that task. /// /// Indexing is being performed with a low priority. - public func scheduleBackgroundIndex(files: some Collection) async { - await self.index(files: files, priority: .low) + private func scheduleBackgroundIndex(files: some Collection) async { + _ = await self.scheduleIndexing(of: files, priority: .low) } /// Regenerate the build graph (also resolving package dependencies) and then index all the source files known to the - /// build system. + /// build system that don't currently have a unit with a timestamp that matches the mtime of the file. + /// + /// This method is intended to initially update the index of a project after it is opened. public func scheduleBuildGraphGenerationAndBackgroundIndexAllFiles() async { generateBuildGraphTask = Task(priority: .low) { await orLog("Generating build graph") { try await self.buildSystemManager.generateBuildGraph() } - await scheduleBackgroundIndex(files: await self.buildSystemManager.sourceFiles().map(\.uri)) + let index = index.checked(for: .modifiedFiles) + let filesToIndex = await self.buildSystemManager.sourceFiles().lazy.map(\.uri) + .filter { uri in + guard let url = uri.fileURL else { + // The URI is not a file, so there's nothing we can index. + return false + } + return !index.hasUpToDateUnit(for: url) + } + await scheduleBackgroundIndex(files: filesToIndex) generateBuildGraphTask = nil } } @@ -134,14 +190,14 @@ public final actor SemanticIndexManager { await generateBuildGraphTask?.value await withTaskGroup(of: Void.self) { taskGroup in - for (_, status) in indexStatus { + for (_, status) in inProgressIndexTasks { switch status { - case .scheduled(let task), .executing(let task): + case .waitingForPreparation(preparationTaskID: _, indexTask: let indexTask), + .updatingIndexStore(updateIndexStoreTask: _, indexTask: let indexTask): taskGroup.addTask { - await task.value + await indexTask.value } - case .upToDate: - break + } } await taskGroup.waitForAll() @@ -165,96 +221,195 @@ public final actor SemanticIndexManager { // Create a new index task for the files that aren't up-to-date. The newly scheduled index tasks will // - Wait for the existing index operations to finish if they have the same number of files. // - Reschedule the background index task in favor of an index task with fewer source files. - await self.index(files: uris, priority: nil).value + await self.scheduleIndexing(of: uris, priority: nil).value index.pollForUnitChangesAndWait() logger.debug("Done waiting for up-to-date index") } + public func filesDidChange(_ events: [FileEvent]) async { + // We only re-index the files that were changed and don't re-index any of their dependencies. See the + // `Documentation/Files_To_Reindex.md` file. + let changedFiles = events.map(\.uri) + await indexStoreUpToDateStatus.markOutOfDate(changedFiles) + + // Note that configured targets are the right abstraction layer here (instead of a non-configured target) because a + // build system might have targets that include different source files. Hence a source file might be in target T + // configured for macOS but not in target T configured for iOS. + let targets = await changedFiles.asyncMap { await buildSystemManager.configuredTargets(for: $0) }.flatMap { $0 } + if let dependentTargets = await buildSystemManager.targets(dependingOn: targets) { + await preparationUpToDateStatus.markOutOfDate(dependentTargets) + } else { + // We couldn't determine which targets depend on the modified targets. Be conservative and assume all of them do. + await indexStoreUpToDateStatus.markOutOfDate(changedFiles) + + await preparationUpToDateStatus.markAllOutOfDate() + // `markAllOutOfDate` only marks targets out-of-date that have been indexed before. Also mark all targets with + // in-progress preparation out of date. So we don't get into the following situation, which would result in an + // incorrect up-to-date status of a target + // - Target preparation starts for the first time + // - Files changed + // - Target preparation finishes. + await preparationUpToDateStatus.markOutOfDate(inProgressPreparationTasks.keys) + } + + await scheduleBackgroundIndex(files: changedFiles) + } + + /// Returns the files that should be indexed to get up-to-date index information for the given files. + /// + /// If `files` contains a header file, this will return a `FileToIndex` that re-indexes a main file which includes the + /// header file to update the header file's index. + private func filesToIndex( + toCover files: some Collection + ) async -> [FileToIndex] { + let sourceFiles = Set(await buildSystemManager.sourceFiles().map(\.uri)) + let filesToReIndex = await files.asyncCompactMap { (uri) -> FileToIndex? in + if sourceFiles.contains(uri) { + // If this is a source file, just index it. + return .indexableFile(uri) + } + // Otherwise, see if it is a header file. If so, index a main file that that imports it to update header file's + // index. + // Deterministically pick a main file. This ensures that we always pick the same main file for a header. This way, + // if we request the same header to be indexed twice, we'll pick the same unit file the second time around, + // realize that its timestamp is later than the modification date of the header and we don't need to re-index. + let mainFile = index.checked(for: .deletedFiles) + .mainFilesContainingFile(uri: uri, crossLanguage: false) + .sorted(by: { $0.stringValue < $1.stringValue }).first + guard let mainFile else { + return nil + } + return .headerFile(header: uri, mainFile: mainFile) + } + return filesToReIndex + } + + /// Schedule preparation of the target that contains the given URI, building all modules that the file depends on. + /// + /// This is intended to be called when the user is interacting with the document at the given URI. + public func schedulePreparation(of uri: DocumentURI, priority: TaskPriority? = nil) { + Task(priority: priority) { + await withLoggingScope("preparation") { + guard let target = await buildSystemManager.canonicalConfiguredTarget(for: uri) else { + return + } + await self.prepare(targets: [target], priority: priority) + } + } + } + // MARK: - Helper functions /// Prepare the given targets for indexing private func prepare(targets: [ConfiguredTarget], priority: TaskPriority?) async { + // Perform a quick initial check whether the target is up-to-date, in which case we don't need to schedule a + // preparation operation at all. + // We will check the up-to-date status again in `PreparationTaskDescription.execute`. This ensures that if we + // schedule two preparations of the same target in quick succession, only the first one actually performs a prepare + // and the second one will be a no-op once it runs. + let targetsToPrepare = await targets.asyncFilter { + await !preparationUpToDateStatus.isUpToDate($0) + } + + guard !targetsToPrepare.isEmpty else { + return + } + let taskDescription = AnyIndexTaskDescription( PreparationTaskDescription( - targetsToPrepare: targets, - buildSystemManager: self.buildSystemManager + targetsToPrepare: targetsToPrepare, + buildSystemManager: self.buildSystemManager, + preparationUpToDateStatus: preparationUpToDateStatus, + testHooks: testHooks ) ) - await self.indexTaskScheduler.schedule(priority: priority, taskDescription).value - self.indexTaskDidFinish() + let preparationTask = await indexTaskScheduler.schedule(priority: priority, taskDescription) { task, newState in + guard case .finished = newState else { + return + } + for target in targetsToPrepare { + if self.inProgressPreparationTasks[target] == OpaqueQueuedIndexTask(task) { + self.inProgressPreparationTasks[target] = nil + } + } + self.indexTaskDidFinish() + } + for target in targetsToPrepare { + inProgressPreparationTasks[target] = OpaqueQueuedIndexTask(preparationTask) + } + return await preparationTask.waitToFinishPropagatingCancellation() } /// Update the index store for the given files, assuming that their targets have already been prepared. - private func updateIndexStore(for files: [DocumentURI], priority: TaskPriority?) async { + private func updateIndexStore( + for filesAndTargets: [FileAndTarget], + preparationTaskID: UUID, + priority: TaskPriority? + ) async { let taskDescription = AnyIndexTaskDescription( UpdateIndexStoreTaskDescription( - filesToIndex: Set(files), + filesToIndex: filesAndTargets, buildSystemManager: self.buildSystemManager, - index: self.index.unchecked + index: index, + indexStoreUpToDateStatus: indexStoreUpToDateStatus, + testHooks: testHooks ) ) - let updateIndexStoreTask = await self.indexTaskScheduler.schedule(priority: priority, taskDescription) { newState in - switch newState { - case .executing: - for file in files { - if case .scheduled(let task) = self.indexStatus[file] { - self.indexStatus[file] = .executing(task) - } else { - logger.fault( - """ - Index status of \(file) is in an unexpected state \ - '\(self.indexStatus[file]?.description ?? "", privacy: .public)' when update index store task \ - started executing - """ - ) - } - } - case .cancelledToBeRescheduled: - for file in files { - if case .executing(let task) = self.indexStatus[file] { - self.indexStatus[file] = .scheduled(task) - } else { - logger.fault( - """ - Index status of \(file) is in an unexpected state \ - '\(self.indexStatus[file]?.description ?? "", privacy: .public)' when update index store task \ - is cancelled to be rescheduled. - """ - ) - } - } - case .finished: - for file in files { - self.indexStatus[file] = .upToDate + let updateIndexTask = await indexTaskScheduler.schedule(priority: priority, taskDescription) { task, newState in + guard case .finished = newState else { + return + } + for fileAndTarget in filesAndTargets { + if case .updatingIndexStore(OpaqueQueuedIndexTask(task), _) = self.inProgressIndexTasks[ + fileAndTarget.file.sourceFile + ] { + self.inProgressIndexTasks[fileAndTarget.file.sourceFile] = nil } - self.indexTaskDidFinish() + } + self.indexTaskDidFinish() + } + for fileAndTarget in filesAndTargets { + if case .waitingForPreparation(preparationTaskID, let indexTask) = inProgressIndexTasks[ + fileAndTarget.file.sourceFile + ] { + inProgressIndexTasks[fileAndTarget.file.sourceFile] = .updatingIndexStore( + updateIndexStoreTask: OpaqueQueuedIndexTask(updateIndexTask), + indexTask: indexTask + ) } } - await updateIndexStoreTask.value + return await updateIndexTask.waitToFinishPropagatingCancellation() } - /// Index the given set of files at the given priority. + /// Index the given set of files at the given priority, preparing their targets beforehand, if needed. /// /// The returned task finishes when all files are indexed. - @discardableResult - private func index(files: some Collection, priority: TaskPriority?) async -> Task { - let outOfDateFiles = files.filter { - if case .upToDate = indexStatus[$0] { - return false - } - return true + private func scheduleIndexing( + of files: some Collection, + priority: TaskPriority? + ) async -> Task { + // Perform a quick initial check to whether the files is up-to-date, in which case we don't need to schedule a + // prepare and index operation at all. + // We will check the up-to-date status again in `IndexTaskDescription.execute`. This ensures that if we schedule + // schedule two indexing jobs for the same file in quick succession, only the first one actually updates the index + // store and the second one will be a no-op once it runs. + let outOfDateFiles = await filesToIndex(toCover: files).asyncFilter { + return await !indexStoreUpToDateStatus.isUpToDate($0.sourceFile) } - .sorted(by: { $0.stringValue < $1.stringValue }) // sort files to get deterministic indexing order + // sort files to get deterministic indexing order + .sorted(by: { $0.sourceFile.stringValue < $1.sourceFile.stringValue }) // Sort the targets in topological order so that low-level targets get built before high-level targets, allowing us // to index the low-level targets ASAP. - var filesByTarget: [ConfiguredTarget: [DocumentURI]] = [:] - for file in outOfDateFiles { - guard let target = await buildSystemManager.canonicalConfiguredTarget(for: file) else { - logger.error("Not indexing \(file.forLogging) because the target could not be determined") + var filesByTarget: [ConfiguredTarget: [FileToIndex]] = [:] + for fileToIndex in outOfDateFiles { + guard let target = await buildSystemManager.canonicalConfiguredTarget(for: fileToIndex.mainFile) else { + logger.error( + "Not indexing \(fileToIndex.forLogging) because the target could not be determined" + ) continue } - filesByTarget[target, default: []].append(file) + filesByTarget[target, default: []].append(fileToIndex) } var sortedTargets: [ConfiguredTarget] = @@ -281,6 +436,7 @@ public final actor SemanticIndexManager { // processor count, so we can get parallelism during preparation. // https://github.com/apple/sourcekit-lsp/issues/1262 for targetsBatch in sortedTargets.partition(intoBatchesOfSize: 1) { + let preparationTaskID = UUID() let indexTask = Task(priority: priority) { // First prepare the targets. await prepare(targets: targetsBatch, priority: priority) @@ -293,7 +449,11 @@ public final actor SemanticIndexManager { // https://github.com/apple/sourcekit-lsp/issues/1268 for fileBatch in filesByTarget[target]!.partition(intoBatchesOfSize: 1) { taskGroup.addTask { - await self.updateIndexStore(for: fileBatch, priority: priority) + await self.updateIndexStore( + for: fileBatch.map { FileAndTarget(file: $0, target: target) }, + preparationTaskID: preparationTaskID, + priority: priority + ) } } } @@ -308,7 +468,10 @@ public final actor SemanticIndexManager { // setting it to `.scheduled` because we don't have an `await` call between the creation of `indexTask` and // this loop, so we still have exclusive access to the `SemanticIndexManager` actor and hence `updateIndexStore` // can't execute until we have set all index statuses to `.scheduled`. - indexStatus[file] = .scheduled(indexTask) + inProgressIndexTasks[file.sourceFile] = .waitingForPreparation( + preparationTaskID: preparationTaskID, + indexTask: indexTask + ) } indexTasksWereScheduled(filesToIndex.count) } diff --git a/Sources/SemanticIndex/TestHooks.swift b/Sources/SemanticIndex/TestHooks.swift new file mode 100644 index 000000000..c3de7326c --- /dev/null +++ b/Sources/SemanticIndex/TestHooks.swift @@ -0,0 +1,27 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2014 - 2024 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +/// Callbacks that allow inspection of internal state modifications during testing. +public struct IndexTestHooks: Sendable { + public var preparationTaskDidFinish: (@Sendable (PreparationTaskDescription) async -> Void)? + + /// A callback that is called when an index task finishes. + public var updateIndexStoreTaskDidFinish: (@Sendable (UpdateIndexStoreTaskDescription) async -> Void)? + + public init( + preparationTaskDidFinish: (@Sendable (PreparationTaskDescription) async -> Void)? = nil, + updateIndexStoreTaskDidFinish: (@Sendable (UpdateIndexStoreTaskDescription) async -> Void)? = nil + ) { + self.preparationTaskDidFinish = preparationTaskDidFinish + self.updateIndexStoreTaskDidFinish = updateIndexStoreTaskDidFinish + } +} diff --git a/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift b/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift index cd8b48ea6..3863a27ec 100644 --- a/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift +++ b/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift @@ -22,6 +22,60 @@ import class TSCBasic.Process private nonisolated(unsafe) var updateIndexStoreIDForLogging = AtomicUInt32(initialValue: 1) +enum FileToIndex: CustomLogStringConvertible { + /// A non-header file + case indexableFile(DocumentURI) + + /// A header file where `mainFile` should be indexed to update the index of `header`. + case headerFile(header: DocumentURI, mainFile: DocumentURI) + + /// The file whose index store should be updated. + /// + /// This file might be a header file that doesn't have build settings associated with it. For the actual compiler + /// invocation that updates the index store, the `mainFile` should be used. + var sourceFile: DocumentURI { + switch self { + case .indexableFile(let uri): return uri + case .headerFile(header: let header, mainFile: _): return header + } + } + + /// The file that should be used for compiler invocations that update the index. + /// + /// If the `sourceFile` is a header file, this will be a main file that includes the header. Otherwise, it will be the + /// same as `sourceFile`. + var mainFile: DocumentURI { + switch self { + case .indexableFile(let uri): return uri + case .headerFile(header: _, mainFile: let mainFile): return mainFile + } + } + + var description: String { + switch self { + case .indexableFile(let uri): + return uri.description + case .headerFile(header: let header, mainFile: let mainFile): + return "\(header.description) using main file \(mainFile.description)" + } + } + + var redactedDescription: String { + switch self { + case .indexableFile(let uri): + return uri.redactedDescription + case .headerFile(header: let header, mainFile: let mainFile): + return "\(header.redactedDescription) using main file \(mainFile.redactedDescription)" + } + } +} + +/// A file to index and the target in which the file should be indexed. +struct FileAndTarget { + let file: FileToIndex + let target: ConfiguredTarget +} + /// Describes a task to index a set of source files. /// /// This task description can be scheduled in a `TaskScheduler`. @@ -30,15 +84,20 @@ public struct UpdateIndexStoreTaskDescription: IndexTaskDescription { public let id = updateIndexStoreIDForLogging.fetchAndIncrement() /// The files that should be indexed. - private let filesToIndex: Set + private let filesToIndex: [FileAndTarget] /// The build system manager that is used to get the toolchain and build settings for the files to index. private let buildSystemManager: BuildSystemManager + private let indexStoreUpToDateStatus: IndexUpToDateStatusManager + /// A reference to the underlying index store. Used to check if the index is already up-to-date for a file, in which /// case we don't need to index it again. private let index: UncheckedIndex + /// Test hooks that should be called when the index task finishes. + private let testHooks: IndexTestHooks + /// The task is idempotent because indexing the same file twice produces the same result as indexing it once. public var isIdempotent: Bool { true } @@ -53,13 +112,17 @@ public struct UpdateIndexStoreTaskDescription: IndexTaskDescription { } init( - filesToIndex: Set, + filesToIndex: [FileAndTarget], buildSystemManager: BuildSystemManager, - index: UncheckedIndex + index: UncheckedIndex, + indexStoreUpToDateStatus: IndexUpToDateStatusManager, + testHooks: IndexTestHooks ) { self.filesToIndex = filesToIndex self.buildSystemManager = buildSystemManager self.index = index + self.indexStoreUpToDateStatus = indexStoreUpToDateStatus + self.testHooks = testHooks } public func execute() async { @@ -72,18 +135,21 @@ public struct UpdateIndexStoreTaskDescription: IndexTaskDescription { ) { let startDate = Date() - let filesToIndexDescription = filesToIndex.map { $0.fileURL?.lastPathComponent ?? $0.stringValue } - .joined(separator: ", ") + let filesToIndexDescription = filesToIndex.map { + $0.file.sourceFile.fileURL?.lastPathComponent ?? $0.file.sourceFile.stringValue + } + .joined(separator: ", ") logger.log( "Starting updating index store with priority \(Task.currentPriority.rawValue, privacy: .public): \(filesToIndexDescription)" ) - let filesToIndex = filesToIndex.sorted(by: { $0.stringValue < $1.stringValue }) + let filesToIndex = filesToIndex.sorted(by: { $0.file.sourceFile.stringValue < $1.file.sourceFile.stringValue }) // TODO (indexing): Once swiftc supports it, we should group files by target and index files within the same // target together in one swiftc invocation. // https://github.com/apple/sourcekit-lsp/issues/1268 for file in filesToIndex { - await updateIndexStoreForSingleFile(file) + await updateIndexStore(forSingleFile: file.file, in: file.target) } + await testHooks.updateIndexStoreTaskDidFinish?(self) logger.log( "Finished updating index store in \(Date().timeIntervalSince(startDate) * 1000, privacy: .public)ms: \(filesToIndexDescription)" ) @@ -93,8 +159,11 @@ public struct UpdateIndexStoreTaskDescription: IndexTaskDescription { public func dependencies( to currentlyExecutingTasks: [UpdateIndexStoreTaskDescription] ) -> [TaskDependencyAction] { + let selfMainFiles = Set(filesToIndex.map(\.file.mainFile)) return currentlyExecutingTasks.compactMap { (other) -> TaskDependencyAction? in - guard !other.filesToIndex.intersection(filesToIndex).isEmpty else { + guard + !other.filesToIndex.lazy.map(\.file.mainFile).contains(where: { selfMainFiles.contains($0) }) + else { // Disjoint sets of files can be indexed concurrently. return nil } @@ -110,63 +179,70 @@ public struct UpdateIndexStoreTaskDescription: IndexTaskDescription { } } - private func updateIndexStoreForSingleFile(_ uri: DocumentURI) async { - guard let url = uri.fileURL else { + private func updateIndexStore(forSingleFile file: FileToIndex, in target: ConfiguredTarget) async { + guard await !indexStoreUpToDateStatus.isUpToDate(file.sourceFile) else { + // If we know that the file is up-to-date without having ot hit the index, do that because it's fastest. + return + } + guard let sourceFileUrl = file.sourceFile.fileURL else { // The URI is not a file, so there's nothing we can index. return } - guard !index.checked(for: .modifiedFiles).hasUpToDateUnit(for: url) else { + guard !index.checked(for: .modifiedFiles).hasUpToDateUnit(for: sourceFileUrl, mainFile: file.mainFile.fileURL) + else { + logger.debug("Not indexing \(file.forLogging) because index has an up-to-date unit") // We consider a file's index up-to-date if we have any up-to-date unit. Changing build settings does not // invalidate the up-to-date status of the index. return } - guard let language = await buildSystemManager.defaultLanguage(for: uri) else { - logger.error("Not indexing \(uri.forLogging) because its language could not be determined") - return + if file.mainFile != file.sourceFile { + logger.log("Updating index store of \(file.forLogging) using main file \(file.mainFile.forLogging)") } - let buildSettings = await buildSystemManager.buildSettingsInferredFromMainFile( - for: uri, - language: language, - logBuildSettings: false - ) - guard let buildSettings else { - logger.error("Not indexing \(uri.forLogging) because it has no compiler arguments") + guard let language = await buildSystemManager.defaultLanguage(for: file.mainFile) else { + logger.error("Not indexing \(file.forLogging) because its language could not be determined") return } - guard !buildSettings.isFallback else { - // Only index with real build settings. Indexing with fallback arguments could result in worse results than not - // indexing at all: If a file has been indexed with real build settings before, had a tiny modification made but - // we don't have any real build settings when it should get re-indexed. Then it's better to have the stale index - // from correct compiler arguments than no index at all. - logger.error("Not updating index store for \(uri.forLogging) because it has fallback compiler arguments") + let buildSettings = await buildSystemManager.buildSettings(for: file.mainFile, in: target, language: language) + guard let buildSettings else { + logger.error("Not indexing \(file.forLogging) because it has no compiler arguments") return } - guard let toolchain = await buildSystemManager.toolchain(for: uri, language) else { + guard let toolchain = await buildSystemManager.toolchain(for: file.mainFile, language) else { logger.error( - "Not updating index store for \(uri.forLogging) because no toolchain could be determined for the document" + "Not updating index store for \(file.forLogging) because no toolchain could be determined for the document" ) return } + let startDate = Date() switch language { case .swift: do { - try await updateIndexStore(forSwiftFile: uri, buildSettings: buildSettings, toolchain: toolchain) + try await updateIndexStore( + forSwiftFile: file.mainFile, + buildSettings: buildSettings, + toolchain: toolchain + ) } catch { - logger.error("Updating index store for \(uri) failed: \(error.forLogging)") - BuildSettingsLogger.log(settings: buildSettings, for: uri) + logger.error("Updating index store for \(file.forLogging) failed: \(error.forLogging)") + BuildSettingsLogger.log(settings: buildSettings, for: file.mainFile) } case .c, .cpp, .objective_c, .objective_cpp: do { - try await updateIndexStore(forClangFile: uri, buildSettings: buildSettings, toolchain: toolchain) + try await updateIndexStore( + forClangFile: file.mainFile, + buildSettings: buildSettings, + toolchain: toolchain + ) } catch { - logger.error("Updating index store for \(uri) failed: \(error.forLogging)") - BuildSettingsLogger.log(settings: buildSettings, for: uri) + logger.error("Updating index store for \(file) failed: \(error.forLogging)") + BuildSettingsLogger.log(settings: buildSettings, for: file.mainFile) } default: logger.error( - "Not updating index store for \(uri) because it is a language that is not supported by background indexing" + "Not updating index store for \(file) because it is a language that is not supported by background indexing" ) } + await indexStoreUpToDateStatus.markUpToDate([file.sourceFile], updateOperationStartDate: startDate) } private func updateIndexStore( @@ -272,37 +348,38 @@ private func adjustSwiftCompilerArgumentsForIndexStoreUpdate( _ compilerArguments: [String], fileToIndex: DocumentURI ) -> [String] { - let removeFlags: Set = [ - "-c", - "-disable-cmo", - "-emit-dependencies", - "-emit-module-interface", - "-emit-module", - "-emit-module", - "-emit-objc-header", - "-incremental", - "-no-color-diagnostics", - "-parseable-output", - "-save-temps", - "-serialize-diagnostics", - "-use-frontend-parseable-output", - "-validate-clang-modules-once", - "-whole-module-optimization", - ] + let optionsToRemove: [CompilerCommandLineOption] = [ + .flag("c", [.singleDash]), + .flag("disable-cmo", [.singleDash]), + .flag("emit-dependencies", [.singleDash]), + .flag("emit-module-interface", [.singleDash]), + .flag("emit-module", [.singleDash]), + .flag("emit-objc-header", [.singleDash]), + .flag("incremental", [.singleDash]), + .flag("no-color-diagnostics", [.singleDash]), + .flag("parseable-output", [.singleDash]), + .flag("save-temps", [.singleDash]), + .flag("serialize-diagnostics", [.singleDash]), + .flag("use-frontend-parseable-output", [.singleDash]), + .flag("validate-clang-modules-once", [.singleDash]), + .flag("whole-module-optimization", [.singleDash]), - let removeArguments: Set = [ - "-clang-build-session-file", - "-emit-module-interface-path", - "-emit-module-path", - "-emit-objc-header-path", - "-emit-package-module-interface-path", - "-emit-private-module-interface-path", - "-num-threads", - "-o", - "-output-file-map", + .option("clang-build-session-file", [.singleDash], [.separatedBySpace]), + .option("emit-module-interface-path", [.singleDash], [.separatedBySpace]), + .option("emit-module-path", [.singleDash], [.separatedBySpace]), + .option("emit-objc-header-path", [.singleDash], [.separatedBySpace]), + .option("emit-package-module-interface-path", [.singleDash], [.separatedBySpace]), + .option("emit-private-module-interface-path", [.singleDash], [.separatedBySpace]), + .option("num-threads", [.singleDash], [.separatedBySpace]), + // Technically, `-o` and the output file don't need to be separated by a space. Eg. `swiftc -oa file.swift` is + // valid and will write to an output file named `a`. + // We can't support that because the only way to know that `-output-file-map` is a different flag and not an option + // to write to an output file named `utput-file-map` is to know all compiler arguments of `swiftc`, which we don't. + .option("o", [.singleDash], [.separatedBySpace]), + .option("output-file-map", [.singleDash], [.separatedBySpace, .separatedByEqualSign]), ] - let removeFrontendFlags: Set = [ + let removeFrontendFlags = [ "-experimental-skip-non-inlinable-function-bodies", "-experimental-skip-all-function-bodies", ] @@ -311,12 +388,14 @@ private func adjustSwiftCompilerArgumentsForIndexStoreUpdate( result.reserveCapacity(compilerArguments.count) var iterator = compilerArguments.makeIterator() while let argument = iterator.next() { - if removeFlags.contains(argument) { + switch optionsToRemove.firstMatch(for: argument) { + case .removeOption: continue - } - if removeArguments.contains(argument) { + case .removeOptionAndNextArgument: _ = iterator.next() continue + case nil: + break } if argument == "-Xfrontend" { if let nextArgument = iterator.next() { @@ -347,43 +426,46 @@ private func adjustClangCompilerArgumentsForIndexStoreUpdate( _ compilerArguments: [String], fileToIndex: DocumentURI ) -> [String] { - let removeFlags: Set = [ + let optionsToRemove: [CompilerCommandLineOption] = [ // Disable writing of a depfile - "-M", - "-MD", - "-MMD", - "-MG", - "-MM", - "-MV", + .flag("M", [.singleDash]), + .flag("MD", [.singleDash]), + .flag("MMD", [.singleDash]), + .flag("MG", [.singleDash]), + .flag("MM", [.singleDash]), + .flag("MV", [.singleDash]), // Don't create phony targets - "-MP", - // Don't writ out compilation databases - "-MJ", - // Continue in the presence of errors during indexing - "-fmodules-validate-once-per-build-session", + .flag("MP", [.singleDash]), + // Don't write out compilation databases + .flag("MJ", [.singleDash]), // Don't compile - "-c", - ] + .flag("c", [.singleDash]), + + .flag("fmodules-validate-once-per-build-session", [.singleDash]), - let removeArguments: Set = [ // Disable writing of a depfile - "-MT", - "-MF", - "-MQ", + .option("MT", [.singleDash], [.noSpace, .separatedBySpace]), + .option("MF", [.singleDash], [.noSpace, .separatedBySpace]), + .option("MQ", [.singleDash], [.noSpace, .separatedBySpace]), + // Don't write serialized diagnostic files - "--serialize-diagnostics", + .option("serialize-diagnostics", [.singleDash, .doubleDash], [.separatedBySpace]), + + .option("fbuild-session-file", [.singleDash], [.separatedByEqualSign]), ] var result: [String] = [] result.reserveCapacity(compilerArguments.count) var iterator = compilerArguments.makeIterator() while let argument = iterator.next() { - if removeFlags.contains(argument) || argument.starts(with: "-fbuild-session-file=") { + switch optionsToRemove.firstMatch(for: argument) { + case .removeOption: continue - } - if removeArguments.contains(argument) { + case .removeOptionAndNextArgument: _ = iterator.next() continue + case nil: + break } result.append(argument) } @@ -392,3 +474,21 @@ private func adjustClangCompilerArgumentsForIndexStoreUpdate( ) return result } + +fileprivate extension Sequence { + /// Returns `true` if this sequence contains an element that is equal to an element in `otherSequence` when + /// considering two elements as equal if they satisfy `predicate`. + func hasIntersection( + with otherSequence: some Sequence, + where predicate: (Element, Element) -> Bool + ) -> Bool { + for outer in self { + for inner in otherSequence { + if predicate(outer, inner) { + return true + } + } + } + return false + } +} diff --git a/Sources/SourceKitLSP/CMakeLists.txt b/Sources/SourceKitLSP/CMakeLists.txt index 45684cec9..0a7c402d6 100644 --- a/Sources/SourceKitLSP/CMakeLists.txt +++ b/Sources/SourceKitLSP/CMakeLists.txt @@ -8,7 +8,6 @@ add_library(SourceKitLSP STATIC LanguageService.swift Rename.swift ResponseError+Init.swift - Sequence+AsyncMap.swift SourceKitIndexDelegate.swift SourceKitLSPCommandMetadata.swift SourceKitLSPServer.swift diff --git a/Sources/SourceKitLSP/Clang/ClangLanguageService.swift b/Sources/SourceKitLSP/Clang/ClangLanguageService.swift index 6e9a63d0e..5c1822c21 100644 --- a/Sources/SourceKitLSP/Clang/ClangLanguageService.swift +++ b/Sources/SourceKitLSP/Clang/ClangLanguageService.swift @@ -499,8 +499,6 @@ extension ClangLanguageService { return } let clangBuildSettings = await self.buildSettings(for: uri) - // FIXME: (logging) Log only the `-something` flags with paths redacted if private mode is enabled - logger.info("settings for \(uri.forLogging): \(clangBuildSettings?.compilerArgs.description ?? "nil")") // The compile command changed, send over the new one. // FIXME: what should we do if we no longer have valid build settings? diff --git a/Sources/SourceKitLSP/IndexProgressManager.swift b/Sources/SourceKitLSP/IndexProgressManager.swift index fdc745fa3..fb758af73 100644 --- a/Sources/SourceKitLSP/IndexProgressManager.swift +++ b/Sources/SourceKitLSP/IndexProgressManager.swift @@ -73,7 +73,7 @@ actor IndexProgressManager { var scheduled: [DocumentURI] = [] var executing: [DocumentURI] = [] for indexManager in await sourceKitLSPServer.workspaces.compactMap({ $0.semanticIndexManager }) { - let inProgress = await indexManager.inProgressIndexTasks + let inProgress = await indexManager.inProgressIndexFiles scheduled += inProgress.scheduled executing += inProgress.executing } diff --git a/Sources/SourceKitLSP/Rename.swift b/Sources/SourceKitLSP/Rename.swift index 29a8f723c..7d4a7369c 100644 --- a/Sources/SourceKitLSP/Rename.swift +++ b/Sources/SourceKitLSP/Rename.swift @@ -500,7 +500,7 @@ extension SourceKitLSPServer { ) async -> (swiftLanguageService: SwiftLanguageService, snapshot: DocumentSnapshot, location: SymbolLocation)? { var reference: SymbolOccurrence? = nil index.forEachSymbolOccurrence(byUSR: usr, roles: renameRoles) { - if index.symbolProvider(for: $0.location.path) == .swift { + if index.unchecked.symbolProvider(for: $0.location.path) == .swift { reference = $0 // We have found a reference from Swift. Stop iteration. return false @@ -631,7 +631,7 @@ extension SourceKitLSPServer { // If we terminate early by returning `false` from the closure, `forEachSymbolOccurrence` returns `true`, // indicating that we have found a reference from clang. let hasReferenceFromClang = !index.forEachSymbolOccurrence(byUSR: usr, roles: renameRoles) { - return index.symbolProvider(for: $0.location.path) != .clang + return index.unchecked.symbolProvider(for: $0.location.path) != .clang } let clangName: String? if hasReferenceFromClang { @@ -745,7 +745,7 @@ extension SourceKitLSPServer { return cachedValue } let serverType = LanguageServerType( - symbolProvider: index.checked(for: .deletedFiles).symbolProvider(for: url.path) + symbolProvider: index.symbolProvider(for: url.path) ) languageServerTypesCache[url] = serverType return serverType diff --git a/Sources/SourceKitLSP/SourceKitLSPServer+Options.swift b/Sources/SourceKitLSP/SourceKitLSPServer+Options.swift index b7005c989..402013d20 100644 --- a/Sources/SourceKitLSP/SourceKitLSPServer+Options.swift +++ b/Sources/SourceKitLSP/SourceKitLSPServer+Options.swift @@ -14,6 +14,7 @@ import Foundation import LanguageServerProtocol import SKCore import SKSupport +import SemanticIndex import struct TSCBasic.AbsolutePath import struct TSCBasic.RelativePath @@ -22,7 +23,6 @@ extension SourceKitLSPServer { /// Configuration options for the SourceKitServer. public struct Options: Sendable { - /// Additional compiler flags (e.g. `-Xswiftc` for SwiftPM projects) and other build-related /// configuration. public var buildSetup: BuildSetup @@ -49,10 +49,7 @@ extension SourceKitLSPServer { /// notification when running unit tests. public var swiftPublishDiagnosticsDebounceDuration: TimeInterval - /// A callback that is called when an index task finishes. - /// - /// Intended for testing purposes. - public var indexTaskDidFinish: (@Sendable () -> Void)? + public var indexTestHooks: IndexTestHooks public init( buildSetup: BuildSetup = .default, @@ -61,7 +58,8 @@ extension SourceKitLSPServer { indexOptions: IndexOptions = .init(), completionOptions: SKCompletionOptions = .init(), generatedInterfacesPath: AbsolutePath = defaultDirectoryForGeneratedInterfaces, - swiftPublishDiagnosticsDebounceDuration: TimeInterval = 2 /* 2s */ + swiftPublishDiagnosticsDebounceDuration: TimeInterval = 2, /* 2s */ + indexTestHooks: IndexTestHooks = IndexTestHooks() ) { self.buildSetup = buildSetup self.clangdOptions = clangdOptions @@ -70,6 +68,7 @@ extension SourceKitLSPServer { self.completionOptions = completionOptions self.generatedInterfacesPath = generatedInterfacesPath self.swiftPublishDiagnosticsDebounceDuration = swiftPublishDiagnosticsDebounceDuration + self.indexTestHooks = indexTestHooks } } } diff --git a/Sources/SourceKitLSP/SourceKitLSPServer.swift b/Sources/SourceKitLSP/SourceKitLSPServer.swift index 6c82f148d..f5c679c57 100644 --- a/Sources/SourceKitLSP/SourceKitLSPServer.swift +++ b/Sources/SourceKitLSP/SourceKitLSPServer.swift @@ -699,7 +699,6 @@ public actor SourceKitLSPServer { /// Send the given notification to the editor. public func sendNotificationToClient(_ notification: some NotificationType) { - logger.log("Sending notification: \(notification.forLogging)") client.send(notification) } @@ -847,6 +846,7 @@ public actor SourceKitLSPServer { guard let toolchain = await workspace.buildSystemManager.toolchain(for: uri, language), let service = await languageService(for: toolchain, language, in: workspace) else { + logger.error("Failed to create language service for \(uri)") return nil } @@ -993,6 +993,16 @@ extension SourceKitLSPServer: MessageHandler { logger.log("Received request \(id): \(params.forLogging)") + if let textDocumentRequest = params as? any TextDocumentRequest { + // When we are requesting information from a document, poke preparation of its target. We don't want to wait for + // the preparation to finish because that would cause too big a delay. + // In practice, while the user is working on a file, we'll get a text document request for it on a regular basis, + // which prepares the files. For files that are open but aren't being worked on (eg. a different tab), we don't + // get requests, ensuring that we don't unnecessarily prepare them. + let workspace = await self.workspaceForDocument(uri: textDocumentRequest.textDocument.uri) + await workspace?.semanticIndexManager?.schedulePreparation(of: textDocumentRequest.textDocument.uri) + } + switch request { case let request as RequestAndReply: await request.reply { try await initialize(request.params) } @@ -1200,7 +1210,6 @@ extension SourceKitLSPServer { logger.log("Cannot open workspace before server is initialized") return nil } - let indexTaskDidFinishCallback = options.indexTaskDidFinish var options = self.options options.buildSetup = self.options.buildSetup.merging(buildSetup(for: workspaceFolder)) return try? await Workspace( @@ -1226,7 +1235,6 @@ extension SourceKitLSPServer { }, indexTaskDidFinish: { [weak self] in self?.indexProgressManager.indexStatusDidChange() - indexTaskDidFinishCallback?() } ) } @@ -1275,7 +1283,6 @@ extension SourceKitLSPServer { if self.workspaces.isEmpty { logger.error("no workspace found") - let indexTaskDidFinishCallback = self.options.indexTaskDidFinish let workspace = await Workspace( documentManager: self.documentManager, rootUri: req.rootURI, @@ -1291,7 +1298,6 @@ extension SourceKitLSPServer { }, indexTaskDidFinish: { [weak self] in self?.indexProgressManager.indexStatusDidChange() - indexTaskDidFinishCallback?() } ) @@ -1537,6 +1543,7 @@ extension SourceKitLSPServer { ) return } + await workspace.semanticIndexManager?.schedulePreparation(of: uri) await openDocument(notification, workspace: workspace) } @@ -1592,6 +1599,7 @@ extension SourceKitLSPServer { ) return } + await workspace.semanticIndexManager?.schedulePreparation(of: uri) // If the document is ready, we can handle the change right now. documentManager.edit(notification) @@ -2486,7 +2494,7 @@ extension SourceKitLSPServer { func pollIndex(_ req: PollIndexRequest) async throws -> VoidResponse { for workspace in workspaces { await workspace.semanticIndexManager?.waitForUpToDateIndex() - workspace.index(checkedFor: .deletedFiles)?.pollForUnitChangesAndWait() + workspace.uncheckedIndex?.pollForUnitChangesAndWait() } return VoidResponse() } diff --git a/Sources/SourceKitLSP/Swift/CodeActions/PackageManifestEdits.swift b/Sources/SourceKitLSP/Swift/CodeActions/PackageManifestEdits.swift index 439513e2b..b029b3b38 100644 --- a/Sources/SourceKitLSP/Swift/CodeActions/PackageManifestEdits.swift +++ b/Sources/SourceKitLSP/Swift/CodeActions/PackageManifestEdits.swift @@ -24,7 +24,47 @@ struct PackageManifestEdits: SyntaxCodeActionProvider { return [] } - return addTestTargetActions(call: call, in: scope) + addProductActions(call: call, in: scope) + return addTargetActions(call: call, in: scope) + addTestTargetActions(call: call, in: scope) + + addProductActions(call: call, in: scope) + } + + /// Produce code actions to add new targets of various kinds. + static func addTargetActions( + call: FunctionCallExprSyntax, + in scope: SyntaxCodeActionScope + ) -> [CodeAction] { + do { + var actions: [CodeAction] = [] + let variants: [(TargetDescription.TargetType, String)] = [ + (.regular, "library"), + (.executable, "executable"), + (.macro, "macro"), + ] + + for (type, name) in variants { + let target = try TargetDescription( + name: "NewTarget", + type: type + ) + + let edits = try AddTarget.addTarget( + target, + to: scope.file + ) + + actions.append( + CodeAction( + title: "Add \(name) target", + kind: .refactor, + edit: edits.asWorkspaceEdit(snapshot: scope.snapshot) + ) + ) + } + + return actions + } catch { + return [] + } } /// Produce code actions to add test target(s) if we are currently on @@ -77,7 +117,7 @@ struct PackageManifestEdits: SyntaxCodeActionProvider { } /// A list of target kinds that allow the creation of tests. - static let targetsThatAllowTests: Set = [ + static let targetsThatAllowTests: [String] = [ "executableTarget", "macro", "target", @@ -123,7 +163,7 @@ struct PackageManifestEdits: SyntaxCodeActionProvider { } /// A list of target kinds that allow the creation of tests. - static let targetsThatAllowProducts: Set = [ + static let targetsThatAllowProducts: [String] = [ "executableTarget", "target", ] diff --git a/Sources/SourceKitLSP/Swift/FoldingRange.swift b/Sources/SourceKitLSP/Swift/FoldingRange.swift index 7ca633d32..cfe99be5b 100644 --- a/Sources/SourceKitLSP/Swift/FoldingRange.swift +++ b/Sources/SourceKitLSP/Swift/FoldingRange.swift @@ -164,6 +164,17 @@ fileprivate final class FoldingRangeFinder: SyntaxAnyVisitor { return .visitChildren } + override func visit(_ node: IfConfigClauseSyntax) -> SyntaxVisitorContinueKind { + guard let closePound = node.lastToken(viewMode: .sourceAccurate)?.nextToken(viewMode: .sourceAccurate) else { + return .visitChildren + } + + return self.addFoldingRange( + start: node.poundKeyword.positionAfterSkippingLeadingTrivia, + end: closePound.positionAfterSkippingLeadingTrivia + ) + } + override func visit(_ node: SubscriptCallExprSyntax) -> SyntaxVisitorContinueKind { return self.addFoldingRange( start: node.leftSquare.endPositionBeforeTrailingTrivia, diff --git a/Sources/SourceKitLSP/Workspace.swift b/Sources/SourceKitLSP/Workspace.swift index cd7a083f4..858ece66e 100644 --- a/Sources/SourceKitLSP/Workspace.swift +++ b/Sources/SourceKitLSP/Workspace.swift @@ -63,7 +63,11 @@ public final class Workspace: Sendable { /// The source code index, if available. /// /// Usually a checked index (retrieved using `index(checkedFor:)`) should be used instead of the unchecked index. - private let uncheckedIndex: ThreadSafeBox + private let _uncheckedIndex: ThreadSafeBox + + public var uncheckedIndex: UncheckedIndex? { + return _uncheckedIndex.value + } /// The index that syntactically scans the workspace for tests. let syntacticTestIndex = SyntacticTestIndex() @@ -97,7 +101,7 @@ public final class Workspace: Sendable { self.buildSetup = options.buildSetup self.rootUri = rootUri self.capabilityRegistry = capabilityRegistry - self.uncheckedIndex = ThreadSafeBox(initialValue: uncheckedIndex) + self._uncheckedIndex = ThreadSafeBox(initialValue: uncheckedIndex) self.buildSystemManager = await BuildSystemManager( buildSystem: underlyingBuildSystem, fallbackBuildSystem: FallbackBuildSystem(buildSetup: buildSetup), @@ -108,6 +112,7 @@ public final class Workspace: Sendable { self.semanticIndexManager = SemanticIndexManager( index: uncheckedIndex, buildSystemManager: buildSystemManager, + testHooks: options.indexTestHooks, indexTaskScheduler: indexTaskScheduler, indexTasksWereScheduled: indexTasksWereScheduled, indexTaskDidFinish: indexTaskDidFinish @@ -154,17 +159,17 @@ public final class Workspace: Sendable { if let rootUrl = rootUri.fileURL, let rootPath = try? AbsolutePath(validating: rootUrl.path) { var options = options - var forceResolvedVersions = true + var isForIndexBuild = false if options.indexOptions.enableBackgroundIndexing, options.buildSetup.path == nil { options.buildSetup.path = rootPath.appending(component: ".index-build") - forceResolvedVersions = false + isForIndexBuild = true } func createSwiftPMBuildSystem(rootUrl: URL) async -> SwiftPMBuildSystem? { return await SwiftPMBuildSystem( url: rootUrl, toolchainRegistry: toolchainRegistry, buildSetup: options.buildSetup, - forceResolvedVersions: forceResolvedVersions, + isForIndexBuild: isForIndexBuild, reloadPackageStatusCallback: reloadPackageStatusCallback ) } @@ -261,7 +266,7 @@ public final class Workspace: Sendable { /// Returns a `CheckedIndex` that verifies that all the returned entries are up-to-date with the given /// `IndexCheckLevel`. func index(checkedFor checkLevel: IndexCheckLevel) -> CheckedIndex? { - return uncheckedIndex.value?.checked(for: checkLevel) + return _uncheckedIndex.value?.checked(for: checkLevel) } /// Write the index to disk. @@ -269,12 +274,13 @@ public final class Workspace: Sendable { /// After this method is called, the workspace will no longer have an index associated with it. It should only be /// called when SourceKit-LSP shuts down. func closeIndex() { - uncheckedIndex.value = nil + _uncheckedIndex.value = nil } public func filesDidChange(_ events: [FileEvent]) async { await buildSystemManager.filesDidChange(events) await syntacticTestIndex.filesDidChange(events) + await semanticIndexManager?.filesDidChange(events) } } diff --git a/Tests/DiagnoseTests/DiagnoseTests.swift b/Tests/DiagnoseTests/DiagnoseTests.swift index f4502a40f..c1daf1d9c 100644 --- a/Tests/DiagnoseTests/DiagnoseTests.swift +++ b/Tests/DiagnoseTests/DiagnoseTests.swift @@ -19,32 +19,31 @@ import SKTestSupport import SourceKitD import XCTest -import class ISDBTibs.TibsBuilder import struct TSCBasic.AbsolutePath -final class DiagnoseTests: XCTestCase { - /// If a default SDK is present on the test machine, return the `-sdk` argument that can be placed in the request - /// YAML. Otherwise, return an empty string. - private var sdkArg: String { - if let sdk = TibsBuilder.defaultSDKPath { - return """ - "-sdk", "\(sdk)", - """ - } else { - return "" - } +/// If a default SDK is present on the test machine, return the `-sdk` argument that can be placed in the request +/// YAML. Otherwise, return an empty string. +private let sdkArg: String = { + if let sdk = defaultSDKPath { + return """ + "-sdk", "\(sdk)", + """ + } else { + return "" } - - /// If a default SDK is present on the test machine, return the `-sdk` argument that can be placed in the request - /// YAML. Otherwise, return an empty string. - private var sdkArgs: [String] { - if let sdk = TibsBuilder.defaultSDKPath { - return ["-sdk", "\(sdk)"] - } else { - return [] - } +}() + +/// If a default SDK is present on the test machine, return the `-sdk` argument that can be placed in the request +/// YAML. Otherwise, return an empty string. +private let sdkArgs: [String] = { + if let sdk = defaultSDKPath { + return ["-sdk", "\(sdk)"] + } else { + return [] } +}() +final class DiagnoseTests: XCTestCase { func testRemoveCodeItemsAndMembers() async throws { // We consider the test case reproducing if cursor info returns the two ambiguous results including their doc // comments. @@ -232,25 +231,25 @@ final class DiagnoseTests: XCTestCase { private func assertReduceSourceKitD( _ markedFileContents: String, request: String, - reproducerPredicate: @escaping (String) -> Bool, + reproducerPredicate: @Sendable @escaping (String) -> Bool, expectedReducedFileContents: String, - file: StaticString = #file, + file: StaticString = #filePath, line: UInt = #line ) async throws { let (markers, fileContents) = extractMarkers(markedFileContents) let toolchain = try await unwrap(ToolchainRegistry.forTesting.default) logger.debug("Using \(toolchain.path?.pathString ?? "") to reduce source file") - let requestExecutor = try InProcessSourceKitRequestExecutor( - toolchain: toolchain, - reproducerPredicate: NSPredicate(block: { (requestResponse, _) -> Bool in - reproducerPredicate(requestResponse as! String) - }) - ) let markerOffset = try XCTUnwrap(markers["1️⃣"], "Failed to find position marker 1️⃣ in file contents") try await withTestScratchDir { scratchDir in + let requestExecutor = try InProcessSourceKitRequestExecutor( + toolchain: toolchain, + reproducerPredicate: NSPredicate(block: { (requestResponse, _) -> Bool in + reproducerPredicate(requestResponse as! String) + }) + ) let testFilePath = scratchDir.appending(component: "test.swift").pathString try fileContents.write(toFile: testFilePath, atomically: false, encoding: .utf8) diff --git a/Tests/LanguageServerProtocolJSONRPCTests/CodingTests.swift b/Tests/LanguageServerProtocolJSONRPCTests/CodingTests.swift index a8d9f5a99..fbd9d656d 100644 --- a/Tests/LanguageServerProtocolJSONRPCTests/CodingTests.swift +++ b/Tests/LanguageServerProtocolJSONRPCTests/CodingTests.swift @@ -283,9 +283,8 @@ final class CodingTests: XCTestCase { return $0 == .string("unknown") ? nil : InitializeResult.self } - let info = defaultCodingInfo.merging([CodingUserInfoKey.responseTypeCallbackKey: responseTypeCallback]) { - (_, new) in new - } + var info = defaultCodingInfo as [CodingUserInfoKey: Any] + info[CodingUserInfoKey.responseTypeCallbackKey] = responseTypeCallback checkMessageDecodingError( MessageDecodingError.invalidRequest( @@ -367,7 +366,7 @@ final class CodingTests: XCTestCase { } } -let defaultCodingInfo: [CodingUserInfoKey: Any] = [CodingUserInfoKey.messageRegistryKey: MessageRegistry.lspProtocol] +let defaultCodingInfo = [CodingUserInfoKey.messageRegistryKey: MessageRegistry.lspProtocol] private func checkMessageCoding( _ value: Request, @@ -418,7 +417,7 @@ private func checkMessageCoding( return $0 == .string("unknown") ? nil : Response.self } - var codingInfo = defaultCodingInfo + var codingInfo = defaultCodingInfo as [CodingUserInfoKey: Any] codingInfo[.responseTypeCallbackKey] = callback checkCoding(JSONRPCMessage.response(value, id: id), json: json, userInfo: codingInfo, file: file, line: line) { diff --git a/Tests/LanguageServerProtocolJSONRPCTests/ConnectionPerfTests.swift b/Tests/LanguageServerProtocolJSONRPCTests/ConnectionPerfTests.swift index cb01c9686..8ba626dbb 100644 --- a/Tests/LanguageServerProtocolJSONRPCTests/ConnectionPerfTests.swift +++ b/Tests/LanguageServerProtocolJSONRPCTests/ConnectionPerfTests.swift @@ -32,11 +32,11 @@ class ConnectionPerfTests: PerfTestCase { let expectation = self.expectation(description: "response received") self.startMeasuring() _ = client.send(EchoRequest(string: "hello!")) { _ in - self.stopMeasuring() expectation.fulfill() } - waitForExpectations(timeout: defaultTimeout) + wait(for: [expectation], timeout: defaultTimeout) + self.stopMeasuring() } } diff --git a/Tests/LanguageServerProtocolJSONRPCTests/ConnectionTests.swift b/Tests/LanguageServerProtocolJSONRPCTests/ConnectionTests.swift index 0f8662da3..3f4838974 100644 --- a/Tests/LanguageServerProtocolJSONRPCTests/ConnectionTests.swift +++ b/Tests/LanguageServerProtocolJSONRPCTests/ConnectionTests.swift @@ -37,7 +37,7 @@ class ConnectionTests: XCTestCase { XCTAssertEqual("a/b", dec.string) } - func testEcho() { + func testEcho() async throws { let client = connection.client let expectation = self.expectation(description: "response received") @@ -48,7 +48,7 @@ class ConnectionTests: XCTestCase { expectation.fulfill() } - waitForExpectations(timeout: defaultTimeout) + try await fulfillmentOfOrThrow([expectation]) } func testMessageBuffer() async throws { @@ -95,7 +95,7 @@ class ConnectionTests: XCTestCase { XCTAssert(connection.serverToClientConnection.requestBufferIsEmpty) } - func testEchoError() { + func testEchoError() async throws { let client = connection.client let expectation = self.expectation(description: "response received 1") let expectation2 = self.expectation(description: "response received 2") @@ -114,7 +114,7 @@ class ConnectionTests: XCTestCase { expectation2.fulfill() } - waitForExpectations(timeout: defaultTimeout) + try await fulfillmentOfOrThrow([expectation, expectation2]) } func testEchoNote() async throws { @@ -131,7 +131,7 @@ class ConnectionTests: XCTestCase { try await fulfillmentOfOrThrow([expectation]) } - func testUnknownRequest() { + func testUnknownRequest() async throws { let client = connection.client let expectation = self.expectation(description: "response received") @@ -145,10 +145,10 @@ class ConnectionTests: XCTestCase { expectation.fulfill() } - waitForExpectations(timeout: defaultTimeout) + try await fulfillmentOfOrThrow([expectation]) } - func testUnknownNotification() { + func testUnknownNotification() async throws { let client = connection.client let expectation = self.expectation(description: "note received") @@ -167,10 +167,10 @@ class ConnectionTests: XCTestCase { expectation.fulfill() } - waitForExpectations(timeout: defaultTimeout) + try await fulfillmentOfOrThrow([expectation]) } - func testUnexpectedResponse() { + func testUnexpectedResponse() async throws { let client = connection.client let expectation = self.expectation(description: "response received") @@ -186,10 +186,10 @@ class ConnectionTests: XCTestCase { expectation.fulfill() } - waitForExpectations(timeout: defaultTimeout) + try await fulfillmentOfOrThrow([expectation]) } - func testSendAfterClose() { + func testSendAfterClose() async throws { let client = connection.client let expectation = self.expectation(description: "note received") @@ -206,7 +206,7 @@ class ConnectionTests: XCTestCase { connection.clientToServerConnection.close() connection.clientToServerConnection.close() - waitForExpectations(timeout: defaultTimeout) + try await fulfillmentOfOrThrow([expectation]) } func testSendBeforeClose() async throws { @@ -229,7 +229,7 @@ class ConnectionTests: XCTestCase { /// DispatchIO can make its callback at any time, so this test is to try to /// provoke a race between those things and ensure the closeHandler is called /// exactly once. - func testCloseRace() { + func testCloseRace() async throws { for _ in 0...100 { let to = Pipe() let from = Pipe() @@ -274,9 +274,8 @@ class ConnectionTests: XCTestCase { #endif conn.close() - withExtendedLifetime(conn) { - waitForExpectations(timeout: defaultTimeout) - } + try await fulfillmentOfOrThrow([expectation]) + withExtendedLifetime(conn) {} } } diff --git a/Tests/LanguageServerProtocolTests/ConnectionTests.swift b/Tests/LanguageServerProtocolTests/ConnectionTests.swift index d26e14770..73e5c447a 100644 --- a/Tests/LanguageServerProtocolTests/ConnectionTests.swift +++ b/Tests/LanguageServerProtocolTests/ConnectionTests.swift @@ -26,7 +26,7 @@ class ConnectionTests: XCTestCase { connection.close() } - func testEcho() { + func testEcho() async throws { let client = connection.client let expectation = self.expectation(description: "response received") @@ -37,10 +37,10 @@ class ConnectionTests: XCTestCase { expectation.fulfill() } - waitForExpectations(timeout: defaultTimeout) + try await fulfillmentOfOrThrow([expectation]) } - func testEchoError() { + func testEchoError() async throws { let client = connection.client let expectation = self.expectation(description: "response received 1") let expectation2 = self.expectation(description: "response received 2") @@ -57,7 +57,7 @@ class ConnectionTests: XCTestCase { expectation2.fulfill() } - waitForExpectations(timeout: defaultTimeout) + try await fulfillmentOfOrThrow([expectation, expectation2]) } func testEchoNote() async throws { diff --git a/Tests/SKCoreTests/BuildServerBuildSystemTests.swift b/Tests/SKCoreTests/BuildServerBuildSystemTests.swift index b1e8870bf..26d1d19a3 100644 --- a/Tests/SKCoreTests/BuildServerBuildSystemTests.swift +++ b/Tests/SKCoreTests/BuildServerBuildSystemTests.swift @@ -20,34 +20,57 @@ import SKTestSupport import TSCBasic import XCTest -final class BuildServerBuildSystemTests: XCTestCase { - /// The path to the INPUTS directory of shared test projects. - private static var skTestSupportInputsDirectory: URL = { - #if os(macOS) - // FIXME: Use Bundle.module.resourceURL once the fix for SR-12912 is released. - - var resources = XCTestCase.productsDirectory - .appendingPathComponent("SourceKitLSP_SKTestSupport.bundle") - .appendingPathComponent("Contents") - .appendingPathComponent("Resources") - if !FileManager.default.fileExists(atPath: resources.path) { - // Xcode and command-line swiftpm differ about the path. - resources.deleteLastPathComponent() - resources.deleteLastPathComponent() - } - #else - let resources = XCTestCase.productsDirectory - .appendingPathComponent("SourceKitLSP_SKTestSupport.resources") - #endif - guard FileManager.default.fileExists(atPath: resources.path) else { - fatalError("missing resources \(resources.path)") - } - return resources.appendingPathComponent("INPUTS", isDirectory: true).standardizedFileURL - }() +/// The bundle of the currently executing test. +private let testBundle: Bundle = { + #if os(macOS) + if let bundle = Bundle.allBundles.first(where: { $0.bundlePath.hasSuffix(".xctest") }) { + return bundle + } + fatalError("couldn't find the test bundle") + #else + return Bundle.main + #endif +}() + +/// The path to the built products directory. +private let productsDirectory: URL = { + #if os(macOS) + return testBundle.bundleURL.deletingLastPathComponent() + #else + return testBundle.bundleURL + #endif +}() + +/// The path to the INPUTS directory of shared test projects. +private let skTestSupportInputsDirectory: URL = { + #if os(macOS) + // FIXME: Use Bundle.module.resourceURL once the fix for SR-12912 is released. + + var resources = + productsDirectory + .appendingPathComponent("SourceKitLSP_SKTestSupport.bundle") + .appendingPathComponent("Contents") + .appendingPathComponent("Resources") + if !FileManager.default.fileExists(atPath: resources.path) { + // Xcode and command-line swiftpm differ about the path. + resources.deleteLastPathComponent() + resources.deleteLastPathComponent() + } + #else + let resources = XCTestCase.productsDirectory + .appendingPathComponent("SourceKitLSP_SKTestSupport.resources") + #endif + guard FileManager.default.fileExists(atPath: resources.path) else { + fatalError("missing resources \(resources.path)") + } + return resources.appendingPathComponent("INPUTS", isDirectory: true).standardizedFileURL +}() +final class BuildServerBuildSystemTests: XCTestCase { private var root: AbsolutePath { try! AbsolutePath( - validating: Self.skTestSupportInputsDirectory + validating: + skTestSupportInputsDirectory .appendingPathComponent(testDirectoryName, isDirectory: true).path ) } diff --git a/Tests/SKCoreTests/BuildSystemManagerTests.swift b/Tests/SKCoreTests/BuildSystemManagerTests.swift index bb15ad142..0caeef9ac 100644 --- a/Tests/SKCoreTests/BuildSystemManagerTests.swift +++ b/Tests/SKCoreTests/BuildSystemManagerTests.swift @@ -88,6 +88,7 @@ final class BuildSystemManagerTests: XCTestCase { await assertEqual(bsm._cachedMainFile(for: d), nil) } + @MainActor func testSettingsMainFile() async throws { let a = try DocumentURI(string: "bsm:a.swift") let mainFiles = ManualMainFilesProvider([a: [a]]) @@ -112,6 +113,7 @@ final class BuildSystemManagerTests: XCTestCase { try await fulfillmentOfOrThrow([changed]) } + @MainActor func testSettingsMainFileInitialNil() async throws { let a = try DocumentURI(string: "bsm:a.swift") let mainFiles = ManualMainFilesProvider([a: [a]]) @@ -134,6 +136,7 @@ final class BuildSystemManagerTests: XCTestCase { try await fulfillmentOfOrThrow([changed]) } + @MainActor func testSettingsMainFileWithFallback() async throws { let a = try DocumentURI(string: "bsm:a.swift") let mainFiles = ManualMainFilesProvider([a: [a]]) @@ -164,6 +167,7 @@ final class BuildSystemManagerTests: XCTestCase { try await fulfillmentOfOrThrow([revert]) } + @MainActor func testSettingsMainFileInitialIntersect() async throws { let a = try DocumentURI(string: "bsm:a.swift") let b = try DocumentURI(string: "bsm:b.swift") @@ -205,6 +209,7 @@ final class BuildSystemManagerTests: XCTestCase { try await fulfillmentOfOrThrow([changedBothA, changedBothB]) } + @MainActor func testSettingsMainFileUnchanged() async throws { let a = try DocumentURI(string: "bsm:a.swift") let b = try DocumentURI(string: "bsm:b.swift") @@ -236,6 +241,7 @@ final class BuildSystemManagerTests: XCTestCase { try await fulfillmentOfOrThrow([changed]) } + @MainActor func testSettingsHeaderChangeMainFile() async throws { let h = try DocumentURI(string: "bsm:header.h") let cpp1 = try DocumentURI(string: "bsm:main.cpp") @@ -292,6 +298,7 @@ final class BuildSystemManagerTests: XCTestCase { try await fulfillmentOfOrThrow([changed4]) } + @MainActor func testSettingsOneMainTwoHeader() async throws { let h1 = try DocumentURI(string: "bsm:header1.h") let h2 = try DocumentURI(string: "bsm:header2.h") @@ -340,6 +347,7 @@ final class BuildSystemManagerTests: XCTestCase { try await fulfillmentOfOrThrow([changed1, changed2]) } + @MainActor func testSettingsChangedAfterUnregister() async throws { let a = try DocumentURI(string: "bsm:a.swift") let b = try DocumentURI(string: "bsm:b.swift") @@ -384,6 +392,7 @@ final class BuildSystemManagerTests: XCTestCase { try await fulfillmentOfOrThrow([changedB]) } + @MainActor func testDependenciesUpdated() async throws { let a = try DocumentURI(string: "bsm:a.swift") let mainFiles = ManualMainFilesProvider([a: [a]]) @@ -434,6 +443,7 @@ private final actor ManualMainFilesProvider: MainFilesProvider { } /// A simple `BuildSystem` that wraps a dictionary, for testing. +@MainActor class ManualBuildSystem: BuildSystem { var projectRoot = try! AbsolutePath(validating: "/") @@ -467,6 +477,10 @@ class ManualBuildSystem: BuildSystem { return nil } + public func targets(dependingOn targets: [ConfiguredTarget]) -> [ConfiguredTarget]? { + return nil + } + func registerForChangeNotifications(for uri: DocumentURI) async { } diff --git a/Tests/SKCoreTests/TaskSchedulerTests.swift b/Tests/SKCoreTests/TaskSchedulerTests.swift index 9ca7a1bd0..90f07fe6c 100644 --- a/Tests/SKCoreTests/TaskSchedulerTests.swift +++ b/Tests/SKCoreTests/TaskSchedulerTests.swift @@ -348,7 +348,9 @@ fileprivate extension TaskScheduler { body, dependencies: dependencies ) - return await self.schedule(priority: priority, taskDescription) + return Task(priority: priority) { + await self.schedule(priority: priority, taskDescription).waitToFinishPropagatingCancellation() + } } } diff --git a/Tests/SKSwiftPMWorkspaceTests/SwiftPMBuildSystemTests.swift b/Tests/SKSwiftPMWorkspaceTests/SwiftPMBuildSystemTests.swift index 2ad593636..69cc4b0f2 100644 --- a/Tests/SKSwiftPMWorkspaceTests/SwiftPMBuildSystemTests.swift +++ b/Tests/SKSwiftPMWorkspaceTests/SwiftPMBuildSystemTests.swift @@ -24,7 +24,7 @@ import XCTest import struct PackageModel.BuildFlags #if canImport(SPMBuildCore) -import SPMBuildCore +@preconcurrency import SPMBuildCore #endif fileprivate extension SwiftPMBuildSystem { @@ -54,7 +54,7 @@ final class SwiftPMBuildSystemTests: XCTestCase { toolchainRegistry: tr, fileSystem: fs, buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup, - forceResolvedVersions: true + isForIndexBuild: false ) ) } @@ -82,7 +82,7 @@ final class SwiftPMBuildSystemTests: XCTestCase { toolchainRegistry: tr, fileSystem: fs, buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup, - forceResolvedVersions: true + isForIndexBuild: false ) ) } @@ -110,7 +110,7 @@ final class SwiftPMBuildSystemTests: XCTestCase { toolchainRegistry: ToolchainRegistry(toolchains: []), fileSystem: fs, buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup, - forceResolvedVersions: true + isForIndexBuild: false ) ) } @@ -138,7 +138,7 @@ final class SwiftPMBuildSystemTests: XCTestCase { toolchainRegistry: tr, fileSystem: fs, buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup, - forceResolvedVersions: true + isForIndexBuild: false ) let aswift = packageRoot.appending(components: "Sources", "lib", "a.swift") @@ -207,7 +207,7 @@ final class SwiftPMBuildSystemTests: XCTestCase { toolchainRegistry: tr, fileSystem: fs, buildSetup: config, - forceResolvedVersions: true + isForIndexBuild: false ) let aswift = packageRoot.appending(components: "Sources", "lib", "a.swift") @@ -245,7 +245,7 @@ final class SwiftPMBuildSystemTests: XCTestCase { toolchainRegistry: tr, fileSystem: fs, buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup, - forceResolvedVersions: true + isForIndexBuild: false ) let source = try resolveSymlinks(packageRoot.appending(component: "Package.swift")) @@ -279,7 +279,7 @@ final class SwiftPMBuildSystemTests: XCTestCase { toolchainRegistry: tr, fileSystem: fs, buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup, - forceResolvedVersions: true + isForIndexBuild: false ) let aswift = packageRoot.appending(components: "Sources", "lib", "a.swift") @@ -325,7 +325,7 @@ final class SwiftPMBuildSystemTests: XCTestCase { toolchainRegistry: tr, fileSystem: fs, buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup, - forceResolvedVersions: true + isForIndexBuild: false ) let aswift = packageRoot.appending(components: "Sources", "libA", "a.swift") @@ -388,7 +388,7 @@ final class SwiftPMBuildSystemTests: XCTestCase { toolchainRegistry: tr, fileSystem: fs, buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup, - forceResolvedVersions: true + isForIndexBuild: false ) let aswift = packageRoot.appending(components: "Sources", "libA", "a.swift") @@ -429,7 +429,7 @@ final class SwiftPMBuildSystemTests: XCTestCase { toolchainRegistry: tr, fileSystem: fs, buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup, - forceResolvedVersions: true + isForIndexBuild: false ) let acxx = packageRoot.appending(components: "Sources", "lib", "a.cpp") @@ -509,7 +509,7 @@ final class SwiftPMBuildSystemTests: XCTestCase { toolchainRegistry: ToolchainRegistry.forTesting, fileSystem: fs, buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup, - forceResolvedVersions: true + isForIndexBuild: false ) let aswift = packageRoot.appending(components: "Sources", "lib", "a.swift") @@ -557,7 +557,7 @@ final class SwiftPMBuildSystemTests: XCTestCase { toolchainRegistry: tr, fileSystem: fs, buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup, - forceResolvedVersions: true + isForIndexBuild: false ) let aswift1 = packageRoot.appending(components: "Sources", "lib", "a.swift") @@ -622,7 +622,7 @@ final class SwiftPMBuildSystemTests: XCTestCase { toolchainRegistry: ToolchainRegistry.forTesting, fileSystem: fs, buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup, - forceResolvedVersions: true + isForIndexBuild: false ) for file in [acpp, ah] { @@ -663,7 +663,7 @@ final class SwiftPMBuildSystemTests: XCTestCase { toolchainRegistry: tr, fileSystem: fs, buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup, - forceResolvedVersions: true + isForIndexBuild: false ) let aswift = packageRoot.appending(components: "Sources", "lib", "a.swift") @@ -700,7 +700,7 @@ final class SwiftPMBuildSystemTests: XCTestCase { toolchainRegistry: tr, fileSystem: fs, buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup, - forceResolvedVersions: true + isForIndexBuild: false ) assertEqual(await swiftpmBuildSystem.projectRoot, try resolveSymlinks(tempDir.appending(component: "pkg"))) @@ -737,7 +737,7 @@ final class SwiftPMBuildSystemTests: XCTestCase { toolchainRegistry: tr, fileSystem: fs, buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup, - forceResolvedVersions: true + isForIndexBuild: false ) let aswift = packageRoot.appending(components: "Plugins", "MyPlugin", "a.swift") diff --git a/Tests/SemanticIndexTests/CompilerCommandLineOptionMatchingTests.swift b/Tests/SemanticIndexTests/CompilerCommandLineOptionMatchingTests.swift new file mode 100644 index 000000000..9dacdb6a1 --- /dev/null +++ b/Tests/SemanticIndexTests/CompilerCommandLineOptionMatchingTests.swift @@ -0,0 +1,56 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2014 - 2019 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +@_spi(Testing) import SemanticIndex +import XCTest + +final class CompilerCommandLineOptionMatchingTests: XCTestCase { + func testFlags() { + assertOption(.flag("a", [.singleDash]), "-a", .removeOption) + assertOption(.flag("a", [.doubleDash]), "--a", .removeOption) + assertOption(.flag("a", [.singleDash, .doubleDash]), "-a", .removeOption) + assertOption(.flag("a", [.singleDash, .doubleDash]), "--a", .removeOption) + assertOption(.flag("a", [.singleDash]), "-another", nil) + assertOption(.flag("a", [.singleDash]), "--a", nil) + assertOption(.flag("a", [.doubleDash]), "-a", nil) + } + + func testOptions() { + assertOption(.option("a", [.singleDash], [.noSpace]), "-a/file.txt", .removeOption) + assertOption(.option("a", [.singleDash], [.noSpace]), "-another", .removeOption) + assertOption(.option("a", [.singleDash], [.separatedByEqualSign]), "-a=/file.txt", .removeOption) + assertOption(.option("a", [.singleDash], [.separatedByEqualSign]), "-a/file.txt", nil) + assertOption(.option("a", [.singleDash], [.separatedBySpace]), "-a", .removeOptionAndNextArgument) + assertOption(.option("a", [.singleDash], [.separatedBySpace]), "-another", nil) + assertOption(.option("a", [.singleDash], [.separatedBySpace]), "-a=/file.txt", nil) + assertOption(.option("a", [.singleDash], [.noSpace, .separatedBySpace]), "-a/file.txt", .removeOption) + assertOption(.option("a", [.singleDash], [.noSpace, .separatedBySpace]), "-a=/file.txt", .removeOption) + assertOption(.option("a", [.singleDash], [.noSpace, .separatedBySpace]), "-a", .removeOptionAndNextArgument) + assertOption(.option("a", [.singleDash], [.separatedByEqualSign, .separatedBySpace]), "-a/file.txt", nil) + assertOption(.option("a", [.singleDash], [.separatedByEqualSign, .separatedBySpace]), "-a=file.txt", .removeOption) + assertOption( + .option("a", [.singleDash], [.separatedByEqualSign, .separatedBySpace]), + "-a", + .removeOptionAndNextArgument + ) + } +} + +fileprivate func assertOption( + _ option: CompilerCommandLineOption, + _ argument: String, + _ expected: CompilerCommandLineOption.Match?, + file: StaticString = #filePath, + line: UInt = #line +) { + XCTAssertEqual(option.matches(argument: argument), expected, file: file, line: line) +} diff --git a/Tests/SourceKitDTests/CrashRecoveryTests.swift b/Tests/SourceKitDTests/CrashRecoveryTests.swift index 9ef320318..f69c2968a 100644 --- a/Tests/SourceKitDTests/CrashRecoveryTests.swift +++ b/Tests/SourceKitDTests/CrashRecoveryTests.swift @@ -279,18 +279,20 @@ final class CrashRecoveryTests: XCTestCase { let clangdRestartedFirstTime = self.expectation(description: "clangd restarted for the first time") let clangdRestartedSecondTime = self.expectation(description: "clangd restarted for the second time") - var clangdHasRestartedFirstTime = false + let clangdHasRestartedFirstTime = ThreadSafeBox(initialValue: false) await clangdServer.addStateChangeHandler { (oldState, newState) in switch newState { case .connectionInterrupted: clangdCrashed.fulfill() case .connected: - if !clangdHasRestartedFirstTime { - clangdRestartedFirstTime.fulfill() - clangdHasRestartedFirstTime = true - } else { - clangdRestartedSecondTime.fulfill() + clangdHasRestartedFirstTime.withLock { clangdHasRestartedFirstTime in + if !clangdHasRestartedFirstTime { + clangdRestartedFirstTime.fulfill() + clangdHasRestartedFirstTime = true + } else { + clangdRestartedSecondTime.fulfill() + } } default: break diff --git a/Tests/SourceKitDTests/SourceKitDRegistryTests.swift b/Tests/SourceKitDTests/SourceKitDRegistryTests.swift index d4680d751..8b43fcff2 100644 --- a/Tests/SourceKitDTests/SourceKitDRegistryTests.swift +++ b/Tests/SourceKitDTests/SourceKitDRegistryTests.swift @@ -10,6 +10,7 @@ // //===----------------------------------------------------------------------===// +import CAtomics import LSPTestSupport import SourceKitD import TSCBasic @@ -40,7 +41,7 @@ final class SourceKitDRegistryTests: XCTestCase { let registry = SourceKitDRegistry() @inline(never) - func scope(registry: SourceKitDRegistry) async throws -> Int { + func scope(registry: SourceKitDRegistry) async throws -> UInt32 { let a = await FakeSourceKitD.getOrCreate(try AbsolutePath(validating: "/a"), in: registry) await assertTrue(a === FakeSourceKitD.getOrCreate(try AbsolutePath(validating: "/a"), in: registry)) @@ -58,10 +59,10 @@ final class SourceKitDRegistryTests: XCTestCase { } } -private var nextToken = 0 +private nonisolated(unsafe) var nextToken = AtomicUInt32(initialValue: 0) final class FakeSourceKitD: SourceKitD { - let token: Int + let token: UInt32 var api: sourcekitd_api_functions_t { fatalError() } var keys: sourcekitd_api_keys { fatalError() } var requests: sourcekitd_api_requests { fatalError() } @@ -69,8 +70,7 @@ final class FakeSourceKitD: SourceKitD { func addNotificationHandler(_ handler: SKDNotificationHandler) { fatalError() } func removeNotificationHandler(_ handler: SKDNotificationHandler) { fatalError() } private init() { - token = nextToken - nextToken += 1 + token = nextToken.fetchAndIncrement() } static func getOrCreate(_ path: AbsolutePath, in registry: SourceKitDRegistry) async -> SourceKitD { diff --git a/Tests/SourceKitDTests/SourceKitDTests.swift b/Tests/SourceKitDTests/SourceKitDTests.swift index 8a2ebe18f..6a4dcb10f 100644 --- a/Tests/SourceKitDTests/SourceKitDTests.swift +++ b/Tests/SourceKitDTests/SourceKitDTests.swift @@ -32,7 +32,7 @@ final class SourceKitDTests: XCTestCase { let keys = sourcekitd.keys let path = DocumentURI.for(.swift).pseudoPath - let isExpectedNotification = { (response: SKDResponse) -> Bool in + let isExpectedNotification = { @Sendable (response: SKDResponse) -> Bool in if let notification: sourcekitd_api_uid_t = response.value?[keys.notification], let name: String = response.value?[keys.name] { @@ -95,10 +95,10 @@ final class SourceKitDTests: XCTestCase { } } -private class ClosureNotificationHandler: SKDNotificationHandler { - let f: (SKDResponse) -> Void +private final class ClosureNotificationHandler: SKDNotificationHandler { + let f: @Sendable (SKDResponse) -> Void - init(_ f: @escaping (SKDResponse) -> Void) { + init(_ f: @Sendable @escaping (SKDResponse) -> Void) { self.f = f } diff --git a/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift b/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift index 3e1bbf702..8efb8b041 100644 --- a/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift +++ b/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift @@ -12,7 +12,9 @@ import LSPTestSupport import LanguageServerProtocol +import SKCore import SKTestSupport +import SemanticIndex import SourceKitLSP import XCTest @@ -20,6 +22,47 @@ fileprivate let backgroundIndexingOptions = SourceKitLSPServer.Options( indexOptions: IndexOptions(enableBackgroundIndexing: true) ) +fileprivate actor ExpectedPreparationTracker { + /// The targets we expect to be prepared. For targets within the same set, we don't care about the exact order. + private var expectedPreparations: [Set] + + /// Implicitly-unwrapped optional so we can reference `self` when creating `IndexTestHooks`. + /// `nonisolated(unsafe)` is fine because this is not modified after `testHooks` is created. + nonisolated(unsafe) var testHooks: IndexTestHooks! + + init(expectedPreparations: [Set]) { + self.expectedPreparations = expectedPreparations + self.testHooks = IndexTestHooks(preparationTaskDidFinish: { [weak self] in + await self?.preparationTaskDidFinish(taskDescription: $0) + }) + } + + func preparationTaskDidFinish(taskDescription: PreparationTaskDescription) -> Void { + guard let expectedTargetsToPrepare = expectedPreparations.first else { + XCTFail("Didn't expect a preparation but received \(taskDescription)") + return + } + guard Set(taskDescription.targetsToPrepare).isSubset(of: expectedTargetsToPrepare) else { + XCTFail("Received unexpected preparation of \(taskDescription)") + return + } + let remainingExpectedTargetsToPrepare = expectedTargetsToPrepare.subtracting(taskDescription.targetsToPrepare) + if remainingExpectedTargetsToPrepare.isEmpty { + expectedPreparations.remove(at: 0) + } else { + expectedPreparations[0] = remainingExpectedTargetsToPrepare + } + } + + deinit { + let expectedPreparations = self.expectedPreparations + XCTAssert( + expectedPreparations.isEmpty, + "ExpectedPreparationTracker destroyed with unfulfilled expected preparations: \(expectedPreparations)." + ) + } +} + final class BackgroundIndexingTests: XCTestCase { func testBackgroundIndexingOfSingleFile() async throws { let project = try await SwiftPMTestProject( @@ -165,11 +208,11 @@ final class BackgroundIndexingTests: XCTestCase { func testBackgroundIndexingHappensWithLowPriority() async throws { var serverOptions = backgroundIndexingOptions - serverOptions.indexTaskDidFinish = { - XCTAssert( - Task.currentPriority == .low, - "An index task ran with priority \(Task.currentPriority)" - ) + serverOptions.indexTestHooks.preparationTaskDidFinish = { taskDescription in + XCTAssert(Task.currentPriority == .low, "\(taskDescription) ran with priority \(Task.currentPriority)") + } + serverOptions.indexTestHooks.updateIndexStoreTaskDidFinish = { taskDescription in + XCTAssert(Task.currentPriority == .low, "\(taskDescription) ran with priority \(Task.currentPriority)") } let project = try await SwiftPMTestProject( files: [ @@ -202,9 +245,10 @@ final class BackgroundIndexingTests: XCTestCase { // Wait for indexing to finish without elevating the priority let semaphore = WrappedSemaphore() + let testClient = project.testClient Task(priority: .low) { await assertNoThrow { - try await project.testClient.send(PollIndexRequest()) + try await testClient.send(PollIndexRequest()) } semaphore.signal() } @@ -373,4 +417,223 @@ final class BackgroundIndexingTests: XCTestCase { withExtendedLifetime(project) {} } + + func testBackgroundIndexingReindexesWhenSwiftFileIsModified() async throws { + let project = try await SwiftPMTestProject( + files: [ + "MyFile.swift": """ + func 1️⃣foo() {} + """, + "MyOtherFile.swift": "", + ], + serverOptions: backgroundIndexingOptions + ) + + let (uri, positions) = try project.openDocument("MyFile.swift") + let prepare = try await project.testClient.send( + CallHierarchyPrepareRequest(textDocument: TextDocumentIdentifier(uri), position: positions["1️⃣"]) + ) + + let callsBeforeEdit = try await project.testClient.send( + CallHierarchyIncomingCallsRequest(item: try XCTUnwrap(prepare?.only)) + ) + XCTAssertEqual(callsBeforeEdit, []) + + let otherFileMarkedContents = """ + func 2️⃣bar() { + 3️⃣foo() + } + """ + + let otherFileUri = try project.uri(for: "MyOtherFile.swift") + let otherFileUrl = try XCTUnwrap(otherFileUri.fileURL) + let otherFilePositions = DocumentPositions(markedText: otherFileMarkedContents) + + try extractMarkers(otherFileMarkedContents).textWithoutMarkers.write( + to: otherFileUrl, + atomically: true, + encoding: .utf8 + ) + + project.testClient.send(DidChangeWatchedFilesNotification(changes: [FileEvent(uri: otherFileUri, type: .changed)])) + _ = try await project.testClient.send(PollIndexRequest()) + + let callsAfterEdit = try await project.testClient.send( + CallHierarchyIncomingCallsRequest(item: try XCTUnwrap(prepare?.only)) + ) + XCTAssertEqual( + callsAfterEdit, + [ + CallHierarchyIncomingCall( + from: CallHierarchyItem( + name: "bar()", + kind: .function, + tags: nil, + uri: otherFileUri, + range: Range(otherFilePositions["2️⃣"]), + selectionRange: Range(otherFilePositions["2️⃣"]), + data: .dictionary([ + "usr": .string("s:9MyLibrary3baryyF"), + "uri": .string(otherFileUri.stringValue), + ]) + ), + fromRanges: [Range(otherFilePositions["3️⃣"])] + ) + ] + ) + } + + func testBackgroundIndexingReindexesHeader() async throws { + let project = try await SwiftPMTestProject( + files: [ + "MyLibrary/include/Header.h": """ + void 1️⃣someFunc(); + """, + "MyFile.c": """ + #include "Header.h" + """, + ], + serverOptions: backgroundIndexingOptions + ) + + let (uri, positions) = try project.openDocument("Header.h", language: .c) + let prepare = try await project.testClient.send( + CallHierarchyPrepareRequest(textDocument: TextDocumentIdentifier(uri), position: positions["1️⃣"]) + ) + + let callsBeforeEdit = try await project.testClient.send( + CallHierarchyIncomingCallsRequest(item: try XCTUnwrap(prepare?.only)) + ) + XCTAssertEqual(callsBeforeEdit, []) + + let headerNewMarkedContents = """ + void someFunc(); + + void 2️⃣test() { + 3️⃣someFunc(); + }; + """ + let newPositions = DocumentPositions(markedText: headerNewMarkedContents) + + try extractMarkers(headerNewMarkedContents).textWithoutMarkers.write( + to: try XCTUnwrap(uri.fileURL), + atomically: true, + encoding: .utf8 + ) + + project.testClient.send(DidChangeWatchedFilesNotification(changes: [FileEvent(uri: uri, type: .changed)])) + _ = try await project.testClient.send(PollIndexRequest()) + + let callsAfterEdit = try await project.testClient.send( + CallHierarchyIncomingCallsRequest(item: try XCTUnwrap(prepare?.only)) + ) + XCTAssertEqual( + callsAfterEdit, + [ + CallHierarchyIncomingCall( + from: CallHierarchyItem( + name: "test", + kind: .function, + tags: nil, + uri: uri, + range: Range(newPositions["2️⃣"]), + selectionRange: Range(newPositions["2️⃣"]), + data: .dictionary([ + "usr": .string("c:@F@test"), + "uri": .string(uri.stringValue), + ]) + ), + fromRanges: [Range(newPositions["3️⃣"])] + ) + ] + ) + } + + func testPrepareTarget() async throws { + try await SkipUnless.swiftpmStoresModulesInSubdirectory() + var serverOptions = backgroundIndexingOptions + let expectedPreparationTracker = ExpectedPreparationTracker(expectedPreparations: [ + [ + ConfiguredTarget(targetID: "LibA", runDestinationID: "dummy"), + ConfiguredTarget(targetID: "LibB", runDestinationID: "dummy"), + ], + [ + ConfiguredTarget(targetID: "LibB", runDestinationID: "dummy") + ], + ]) + serverOptions.indexTestHooks = expectedPreparationTracker.testHooks + + let project = try await SwiftPMTestProject( + files: [ + "LibA/MyFile.swift": "", + "LibB/MyOtherFile.swift": """ + import LibA + func bar() { + 1️⃣foo2️⃣() + } + """, + ], + manifest: """ + // swift-tools-version: 5.7 + + import PackageDescription + + let package = Package( + name: "MyLibrary", + targets: [ + .target(name: "LibA"), + .target(name: "LibB", dependencies: ["LibA"]), + ] + ) + """, + serverOptions: serverOptions, + cleanUp: { /* Keep expectedPreparationTracker alive */ _ = expectedPreparationTracker } + ) + + let (uri, _) = try project.openDocument("MyOtherFile.swift") + let initialDiagnostics = try await project.testClient.send( + DocumentDiagnosticsRequest(textDocument: TextDocumentIdentifier(uri)) + ) + guard case .full(let initialDiagnostics) = initialDiagnostics else { + XCTFail("Expected full diagnostics") + return + } + XCTAssertNotEqual(initialDiagnostics.items, []) + + try "public func foo() {}".write( + to: try XCTUnwrap(project.uri(for: "MyFile.swift").fileURL), + atomically: true, + encoding: .utf8 + ) + + project.testClient.send( + DidChangeWatchedFilesNotification(changes: [FileEvent(uri: try project.uri(for: "MyFile.swift"), type: .changed)]) + ) + + // Send a document request for `uri` to trigger re-preparation of its target. We don't actually care about the + // response for this request. Instead, we wait until SourceKit-LSP sends us a `DiagnosticsRefreshRequest`, + // indicating that the target of `uri` has been prepared. + _ = try await project.testClient.send( + DocumentDiagnosticsRequest(textDocument: TextDocumentIdentifier(uri)) + ) + + let receivedEmptyDiagnostics = self.expectation(description: "Received diagnostic refresh request") + project.testClient.handleNextRequest { (_: DiagnosticsRefreshRequest) in + Task { + let updatedDiagnostics = try await project.testClient.send( + DocumentDiagnosticsRequest(textDocument: TextDocumentIdentifier(uri)) + ) + guard case .full(let updatedDiagnostics) = updatedDiagnostics else { + XCTFail("Expected full diagnostics") + return + } + if updatedDiagnostics.items.isEmpty { + receivedEmptyDiagnostics.fulfill() + } + } + return VoidResponse() + } + + try await fulfillmentOfOrThrow([receivedEmptyDiagnostics]) + } } diff --git a/Tests/SourceKitLSPTests/BuildSystemTests.swift b/Tests/SourceKitLSPTests/BuildSystemTests.swift index c3eb343c8..49314ccac 100644 --- a/Tests/SourceKitLSPTests/BuildSystemTests.swift +++ b/Tests/SourceKitLSPTests/BuildSystemTests.swift @@ -21,11 +21,11 @@ import XCTest /// Build system to be used for testing BuildSystem and BuildSystemDelegate functionality with SourceKitLSPServer /// and other components. -final class TestBuildSystem: BuildSystem { - var projectRoot: AbsolutePath = try! AbsolutePath(validating: "/") - var indexStorePath: AbsolutePath? = nil - var indexDatabasePath: AbsolutePath? = nil - var indexPrefixMappings: [PathPrefixMapping] = [] +actor TestBuildSystem: BuildSystem { + let projectRoot: AbsolutePath = try! AbsolutePath(validating: "/") + let indexStorePath: AbsolutePath? = nil + let indexDatabasePath: AbsolutePath? = nil + let indexPrefixMappings: [PathPrefixMapping] = [] weak var delegate: BuildSystemDelegate? @@ -34,10 +34,14 @@ final class TestBuildSystem: BuildSystem { } /// Build settings by file. - var buildSettingsByFile: [DocumentURI: FileBuildSettings] = [:] + private var buildSettingsByFile: [DocumentURI: FileBuildSettings] = [:] /// Files currently being watched by our delegate. - var watchedFiles: Set = [] + private var watchedFiles: Set = [] + + func setBuildSettings(for uri: DocumentURI, to buildSettings: FileBuildSettings) { + buildSettingsByFile[uri] = buildSettings + } func buildSettings( for document: DocumentURI, @@ -65,6 +69,10 @@ final class TestBuildSystem: BuildSystem { return nil } + public func targets(dependingOn targets: [ConfiguredTarget]) -> [ConfiguredTarget]? { + return nil + } + func registerForChangeNotifications(for uri: DocumentURI) async { watchedFiles.insert(uri) } @@ -160,7 +168,7 @@ final class BuildSystemTests: XCTestCase { } """ - buildSystem.buildSettingsByFile[doc] = FileBuildSettings(compilerArguments: args) + await buildSystem.setBuildSettings(for: doc, to: FileBuildSettings(compilerArguments: args)) let documentManager = await self.testClient.server._documentManager @@ -173,7 +181,7 @@ final class BuildSystemTests: XCTestCase { // Modify the build settings and inform the delegate. // This should trigger a new publish diagnostics and we should no longer have errors. let newSettings = FileBuildSettings(compilerArguments: args + ["-DFOO"]) - buildSystem.buildSettingsByFile[doc] = newSettings + await buildSystem.setBuildSettings(for: doc, to: newSettings) await buildSystem.delegate?.fileBuildSettingsChanged([doc]) @@ -194,7 +202,7 @@ final class BuildSystemTests: XCTestCase { .buildSettings(for: doc, language: .swift)! .compilerArguments - buildSystem.buildSettingsByFile[doc] = FileBuildSettings(compilerArguments: args) + await buildSystem.setBuildSettings(for: doc, to: FileBuildSettings(compilerArguments: args)) let text = """ #if FOO @@ -214,7 +222,7 @@ final class BuildSystemTests: XCTestCase { // Modify the build settings and inform the delegate. // This should trigger a new publish diagnostics and we should no longer have errors. let newSettings = FileBuildSettings(compilerArguments: args + ["-DFOO"]) - buildSystem.buildSettingsByFile[doc] = newSettings + await buildSystem.setBuildSettings(for: doc, to: newSettings) await buildSystem.delegate?.fileBuildSettingsChanged([doc]) @@ -248,7 +256,7 @@ final class BuildSystemTests: XCTestCase { // Modify the build settings and inform the delegate. // This should trigger a new publish diagnostics and we should see a diagnostic. let newSettings = FileBuildSettings(compilerArguments: args) - buildSystem.buildSettingsByFile[doc] = newSettings + await buildSystem.setBuildSettings(for: doc, to: newSettings) await buildSystem.delegate?.fileBuildSettingsChanged([doc]) @@ -283,7 +291,7 @@ final class BuildSystemTests: XCTestCase { XCTAssertEqual(text, try documentManager.latestSnapshot(doc).text) // Swap from fallback settings to primary build system settings. - buildSystem.buildSettingsByFile[doc] = primarySettings + await buildSystem.setBuildSettings(for: doc, to: primarySettings) await buildSystem.delegate?.fileBuildSettingsChanged([doc]) diff --git a/Tests/SourceKitLSPTests/CodeActionTests.swift b/Tests/SourceKitLSPTests/CodeActionTests.swift index 8a340a21e..f70883b9c 100644 --- a/Tests/SourceKitLSPTests/CodeActionTests.swift +++ b/Tests/SourceKitLSPTests/CodeActionTests.swift @@ -20,7 +20,7 @@ private typealias CodeActionCapabilities = TextDocumentClientCapabilities.CodeAc private typealias CodeActionLiteralSupport = CodeActionCapabilities.CodeActionLiteralSupport private typealias CodeActionKindCapabilities = CodeActionLiteralSupport.CodeActionKind -private var clientCapabilitiesWithCodeActionSupport: ClientCapabilities = { +private let clientCapabilitiesWithCodeActionSupport: ClientCapabilities = { var documentCapabilities = TextDocumentClientCapabilities() var codeActionCapabilities = CodeActionCapabilities() let codeActionKinds = CodeActionKindCapabilities(valueSet: [.refactor, .quickFix]) @@ -609,6 +609,12 @@ final class CodeActionTests: XCTestCase { } XCTAssertNotNil(addTestAction) + XCTAssertTrue( + codeActions.contains { action in + action.title == "Add library target" + } + ) + guard let addTestChanges = addTestAction?.edit?.documentChanges else { XCTFail("Didn't have changes in the 'Add test target (Swift Testing)' action") return diff --git a/Tests/SourceKitLSPTests/CrossLanguageRenameTests.swift b/Tests/SourceKitLSPTests/CrossLanguageRenameTests.swift index 4588f4fc9..4a3bba952 100644 --- a/Tests/SourceKitLSPTests/CrossLanguageRenameTests.swift +++ b/Tests/SourceKitLSPTests/CrossLanguageRenameTests.swift @@ -89,6 +89,7 @@ final class CrossLanguageRenameTests: XCTestCase { } """, ], + headerFileLanguage: .c, newName: "dFunc", expectedPrepareRenamePlaceholder: "cFunc", expected: [ diff --git a/Tests/SourceKitLSPTests/FoldingRangeTests.swift b/Tests/SourceKitLSPTests/FoldingRangeTests.swift index e24f683e1..dad8ea8f8 100644 --- a/Tests/SourceKitLSPTests/FoldingRangeTests.swift +++ b/Tests/SourceKitLSPTests/FoldingRangeTests.swift @@ -357,4 +357,52 @@ final class FoldingRangeTests: XCTestCase { expectedRanges: [FoldingRangeSpec(from: "1️⃣", to: "2️⃣")] ) } + + func testFoldArgumentsForConditionalIfCompileDirectives() async throws { + try await assertFoldingRanges( + markedSource: """ + 1️⃣#if DEBUG + let foo = "x" + 2️⃣#endif + """, + expectedRanges: [ + FoldingRangeSpec(from: "1️⃣", to: "2️⃣") + ] + ) + } + + func testFoldArgumentsForConditionalElseIfCompileDirectives() async throws { + try await assertFoldingRanges( + markedSource: """ + 1️⃣#if DEBUG + let foo = "x" + 2️⃣#elseif TEST + let foo = "y" + 3️⃣#else + let foo = "z" + 4️⃣#endif + """, + expectedRanges: [ + FoldingRangeSpec(from: "1️⃣", to: "2️⃣"), + FoldingRangeSpec(from: "2️⃣", to: "3️⃣"), + FoldingRangeSpec(from: "3️⃣", to: "4️⃣"), + ] + ) + } + + func testFoldArgumentsForConditionalElseCompileDirectives() async throws { + try await assertFoldingRanges( + markedSource: """ + 1️⃣#if DEBUG + let foo = "x" + 2️⃣#else + let foo = "y" + 3️⃣#endif + """, + expectedRanges: [ + FoldingRangeSpec(from: "1️⃣", to: "2️⃣"), + FoldingRangeSpec(from: "2️⃣", to: "3️⃣"), + ] + ) + } } diff --git a/Tests/SourceKitLSPTests/LocalSwiftTests.swift b/Tests/SourceKitLSPTests/LocalSwiftTests.swift index 85d22d99c..bbde96fbf 100644 --- a/Tests/SourceKitLSPTests/LocalSwiftTests.swift +++ b/Tests/SourceKitLSPTests/LocalSwiftTests.swift @@ -1352,7 +1352,9 @@ final class LocalSwiftTests: XCTestCase { let uri = DocumentURI(url) let reusedNodeCallback = self.expectation(description: "reused node callback called") - var reusedNodes: [Syntax] = [] + // nonisolated(unsafe) is fine because the variable will only be read after all writes from reusedNodeCallback are + // done. + nonisolated(unsafe) var reusedNodes: [Syntax] = [] let swiftLanguageService = await testClient.server._languageService(for: uri, .swift, in: testClient.server.workspaceForDocument(uri: uri)!) as! SwiftLanguageService