diff --git a/Sources/SKCore/BuildServerBuildSystem.swift b/Sources/SKCore/BuildServerBuildSystem.swift index 663b15a63..c75a7cea0 100644 --- a/Sources/SKCore/BuildServerBuildSystem.swift +++ b/Sources/SKCore/BuildServerBuildSystem.swift @@ -316,6 +316,17 @@ extension BuildServerBuildSystem: BuildSystem { return .unhandled } + + public func testFiles() async -> [DocumentURI] { + // BuildServerBuildSystem does not support syntactic test discovery + // (https://github.com/apple/sourcekit-lsp/issues/1173). + return [] + } + + public func addTestFilesDidChangeCallback(_ callback: @escaping () async -> Void) { + // BuildServerBuildSystem does not support syntactic test discovery + // (https://github.com/apple/sourcekit-lsp/issues/1173). + } } private func loadBuildServerConfig(path: AbsolutePath, fileSystem: FileSystem) throws -> BuildServerConfig { diff --git a/Sources/SKCore/BuildSystem.swift b/Sources/SKCore/BuildSystem.swift index 85633cfdc..e9756023d 100644 --- a/Sources/SKCore/BuildSystem.swift +++ b/Sources/SKCore/BuildSystem.swift @@ -87,6 +87,18 @@ public protocol BuildSystem: AnyObject, Sendable { func filesDidChange(_ events: [FileEvent]) async func fileHandlingCapability(for uri: DocumentURI) async -> FileHandlingCapability + + /// Returns the list of files that might contain test cases. + /// + /// The returned file list is an over-approximation. It might contain tests from non-test targets or files that don't + /// actually contain any tests. Keeping this list as minimal as possible helps reduce the amount of work that the + /// syntactic test indexer needs to perform. + func testFiles() async -> [DocumentURI] + + /// Adds a callback that should be called when the value returned by `testFiles()` changes. + /// + /// The callback might also be called without an actual change to `testFiles`. + func addTestFilesDidChangeCallback(_ callback: @Sendable @escaping () async -> Void) async } public let buildTargetsNotSupported = diff --git a/Sources/SKCore/BuildSystemManager.swift b/Sources/SKCore/BuildSystemManager.swift index 8659c3f0e..a6a44a067 100644 --- a/Sources/SKCore/BuildSystemManager.swift +++ b/Sources/SKCore/BuildSystemManager.swift @@ -176,6 +176,10 @@ extension BuildSystemManager { fallbackBuildSystem != nil ? .fallback : .unhandled ) } + + public func testFiles() async -> [DocumentURI] { + return await buildSystem?.testFiles() ?? [] + } } extension BuildSystemManager: BuildSystemDelegate { diff --git a/Sources/SKCore/CompilationDatabaseBuildSystem.swift b/Sources/SKCore/CompilationDatabaseBuildSystem.swift index c9009af86..929110f21 100644 --- a/Sources/SKCore/CompilationDatabaseBuildSystem.swift +++ b/Sources/SKCore/CompilationDatabaseBuildSystem.swift @@ -39,6 +39,9 @@ public actor CompilationDatabaseBuildSystem { /// Delegate to handle any build system events. public weak var delegate: BuildSystemDelegate? = nil + /// Callbacks that should be called if the list of possible test files has changed. + public var testFilesDidChangeCallbacks: [() async -> Void] = [] + public func setDelegate(_ delegate: BuildSystemDelegate?) async { self.delegate = delegate } @@ -167,6 +170,9 @@ extension CompilationDatabaseBuildSystem: BuildSystem { if let delegate = self.delegate { await delegate.fileBuildSettingsChanged(self.watchedFiles) } + for testFilesDidChangeCallback in testFilesDidChangeCallbacks { + await testFilesDidChangeCallback() + } } public func filesDidChange(_ events: [FileEvent]) async { @@ -185,4 +191,12 @@ extension CompilationDatabaseBuildSystem: BuildSystem { return .unhandled } } + + public func testFiles() async -> [DocumentURI] { + return compdb?.allCommands.map { DocumentURI($0.url) } ?? [] + } + + public func addTestFilesDidChangeCallback(_ callback: @escaping () async -> Void) async { + testFilesDidChangeCallbacks.append(callback) + } } diff --git a/Sources/SKSupport/AsyncQueue.swift b/Sources/SKSupport/AsyncQueue.swift index 7db9d60ed..4829983a7 100644 --- a/Sources/SKSupport/AsyncQueue.swift +++ b/Sources/SKSupport/AsyncQueue.swift @@ -16,6 +16,8 @@ import Foundation /// array. private protocol AnyTask: Sendable { func waitForCompletion() async + + func cancel() } extension Task: AnyTask { @@ -89,6 +91,16 @@ public final class AsyncQueue: Sendable { public init() {} + public func cancelTasks(where filter: (TaskMetadata) -> Bool) { + pendingTasks.withLock { pendingTasks in + for task in pendingTasks { + if filter(task.metadata) { + task.task.cancel() + } + } + } + } + /// Schedule a new closure to be executed on the queue. /// /// If this is a serial queue, all previously added tasks are guaranteed to diff --git a/Sources/SKSwiftPMWorkspace/SwiftPMBuildSystem.swift b/Sources/SKSwiftPMWorkspace/SwiftPMBuildSystem.swift index d4981712b..bf2837e07 100644 --- a/Sources/SKSwiftPMWorkspace/SwiftPMBuildSystem.swift +++ b/Sources/SKSwiftPMWorkspace/SwiftPMBuildSystem.swift @@ -81,6 +81,9 @@ public actor SwiftPMBuildSystem { self.delegate = delegate } + /// Callbacks that should be called if the list of possible test files has changed. + public var testFilesDidChangeCallbacks: [() async -> Void] = [] + let workspacePath: TSCAbsolutePath /// The directory containing `Package.swift`. public var projectRoot: TSCAbsolutePath @@ -267,6 +270,9 @@ extension SwiftPMBuildSystem { } await delegate.fileBuildSettingsChanged(self.watchedFiles) await delegate.fileHandlingCapabilityChanged() + for testFilesDidChangeCallback in testFilesDidChangeCallbacks { + await testFilesDidChangeCallback() + } } } @@ -380,6 +386,15 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem { return .unhandled } } + + public func testFiles() -> [DocumentURI] { + // We should only include source files from test targets (https://github.com/apple/sourcekit-lsp/issues/1174). + return fileToTarget.map { DocumentURI($0.key.asURL) } + } + + public func addTestFilesDidChangeCallback(_ callback: @Sendable @escaping () async -> Void) async { + testFilesDidChangeCallbacks.append(callback) + } } extension SwiftPMBuildSystem { diff --git a/Sources/SKTestSupport/IndexedSingleSwiftFileTestProject.swift b/Sources/SKTestSupport/IndexedSingleSwiftFileTestProject.swift index 762233aea..cae4aef53 100644 --- a/Sources/SKTestSupport/IndexedSingleSwiftFileTestProject.swift +++ b/Sources/SKTestSupport/IndexedSingleSwiftFileTestProject.swift @@ -68,6 +68,25 @@ public struct IndexedSingleSwiftFileTestProject { if let sdk = TibsBuilder.defaultSDKPath { compilerArguments += ["-sdk", sdk] + + // The following are needed so we can import XCTest + let sdkUrl = URL(fileURLWithPath: sdk) + let usrLibDir = + sdkUrl + .deletingLastPathComponent() + .deletingLastPathComponent() + .appendingPathComponent("usr") + .appendingPathComponent("lib") + let frameworksDir = + sdkUrl + .deletingLastPathComponent() + .deletingLastPathComponent() + .appendingPathComponent("Library") + .appendingPathComponent("Frameworks") + compilerArguments += [ + "-I", usrLibDir.path, + "-F", frameworksDir.path, + ] } let compilationDatabase = JSONCompilationDatabase( diff --git a/Sources/SKTestSupport/MultiFileTestProject.swift b/Sources/SKTestSupport/MultiFileTestProject.swift index 3eec7f453..1484c4a64 100644 --- a/Sources/SKTestSupport/MultiFileTestProject.swift +++ b/Sources/SKTestSupport/MultiFileTestProject.swift @@ -142,4 +142,13 @@ public class MultiFileTestProject { } return DocumentPositions(markedText: fileData.markedText)[marker] } + + public func range(from fromMarker: String, to toMarker: String, in fileName: String) throws -> Range { + return try position(of: fromMarker, in: fileName).. Location { + let range = try self.range(from: fromMarker, to: toMarker, in: fileName) + return Location(uri: try self.uri(for: fileName), range: range) + } } diff --git a/Sources/SKTestSupport/SwiftPMTestProject.swift b/Sources/SKTestSupport/SwiftPMTestProject.swift index 16c64e6fb..958a9d1b2 100644 --- a/Sources/SKTestSupport/SwiftPMTestProject.swift +++ b/Sources/SKTestSupport/SwiftPMTestProject.swift @@ -40,6 +40,7 @@ public class SwiftPMTestProject: MultiFileTestProject { manifest: String = SwiftPMTestProject.defaultPackageManifest, workspaces: (URL) -> [WorkspaceFolder] = { [WorkspaceFolder(uri: DocumentURI($0))] }, build: Bool = false, + allowBuildFailure: Bool = false, usePullDiagnostics: Bool = true, testName: String = #function ) async throws { @@ -66,7 +67,11 @@ public class SwiftPMTestProject: MultiFileTestProject { ) if build { - try await Self.build(at: self.scratchDirectory) + if allowBuildFailure { + try? await Self.build(at: self.scratchDirectory) + } else { + try await Self.build(at: self.scratchDirectory) + } } // Wait for the indexstore-db to finish indexing _ = try await testClient.send(PollIndexRequest()) diff --git a/Sources/SKTestSupport/TestSourceKitLSPClient.swift b/Sources/SKTestSupport/TestSourceKitLSPClient.swift index 72fe9107d..48a4c67dd 100644 --- a/Sources/SKTestSupport/TestSourceKitLSPClient.swift +++ b/Sources/SKTestSupport/TestSourceKitLSPClient.swift @@ -362,7 +362,7 @@ public struct DocumentPositions { } } - init(markedText: String) { + public init(markedText: String) { let (markers, textWithoutMarker) = extractMarkers(markedText) self.init(markers: markers, textWithoutMarkers: textWithoutMarker) } diff --git a/Sources/SourceKitLSP/CMakeLists.txt b/Sources/SourceKitLSP/CMakeLists.txt index fc48972d8..0d83ba4ae 100644 --- a/Sources/SourceKitLSP/CMakeLists.txt +++ b/Sources/SourceKitLSP/CMakeLists.txt @@ -2,6 +2,7 @@ add_library(SourceKitLSP STATIC CapabilityRegistry.swift DocumentManager.swift + DocumentSnapshot+FromFileContents.swift IndexOutOfDateChecker.swift IndexStoreDB+MainFilesProvider.swift LanguageService.swift @@ -12,6 +13,7 @@ add_library(SourceKitLSP STATIC SourceKitLSPCommandMetadata.swift SourceKitLSPServer.swift SourceKitLSPServer+Options.swift + SymbolLocation+DocumentURI.swift TestDiscovery.swift WorkDoneProgressManager.swift Workspace.swift @@ -44,9 +46,10 @@ target_sources(SourceKitLSP PRIVATE Swift/SwiftLanguageService.swift Swift/SwiftTestingScanner.swift Swift/SymbolInfo.swift + Swift/SyntacticTestIndex.swift Swift/SyntaxHighlightingToken.swift - Swift/SyntaxHighlightingTokens.swift Swift/SyntaxHighlightingTokenParser.swift + Swift/SyntaxHighlightingTokens.swift Swift/SyntaxTreeManager.swift Swift/VariableTypeInfo.swift ) diff --git a/Sources/SourceKitLSP/DocumentSnapshot+FromFileContents.swift b/Sources/SourceKitLSP/DocumentSnapshot+FromFileContents.swift new file mode 100644 index 000000000..7a41f0aa8 --- /dev/null +++ b/Sources/SourceKitLSP/DocumentSnapshot+FromFileContents.swift @@ -0,0 +1,35 @@ +//===----------------------------------------------------------------------===// +// +// 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 LanguageServerProtocol +import SKSupport + +public extension DocumentSnapshot { + /// Creates a `DocumentSnapshot` with the file contents from disk. + /// + /// Throws an error if the file could not be read. + /// Returns `nil` if the `uri` is not a file URL. + init?(withContentsFromDisk uri: DocumentURI, language: Language) throws { + guard let url = uri.fileURL else { + return nil + } + try self.init(withContentsFromDisk: url, language: language) + } + + /// Creates a `DocumentSnapshot` with the file contents from disk. + /// + /// Throws an error if the file could not be read. + init(withContentsFromDisk url: URL, language: Language) throws { + let contents = try String(contentsOf: url) + self.init(uri: DocumentURI(url), language: language, version: 0, lineTable: LineTable(contents)) + } +} diff --git a/Sources/SourceKitLSP/IndexOutOfDateChecker.swift b/Sources/SourceKitLSP/IndexOutOfDateChecker.swift index 31537901e..48d32ee7d 100644 --- a/Sources/SourceKitLSP/IndexOutOfDateChecker.swift +++ b/Sources/SourceKitLSP/IndexOutOfDateChecker.swift @@ -13,13 +13,19 @@ import Foundation import IndexStoreDB import LSPLogging +import LanguageServerProtocol /// Helper class to check if symbols from the index are up-to-date or if the source file has been modified after it was -/// indexed. +/// indexed. Modifications include both changes to the file on disk as well as modifications to the file that have not +/// been saved to disk (ie. changes that only live in `DocumentManager`). /// /// The checker caches mod dates of source files. It should thus not be long lived. Its intended lifespan is the /// evaluation of a single request. struct IndexOutOfDateChecker { + /// The `DocumentManager` that holds the in-memory file contents. We consider the index out-of-date for all files that + /// have in-memory changes. + private let documentManager: DocumentManager + /// The last modification time of a file. Can also represent the fact that the file does not exist. private enum ModificationTime { case fileDoesNotExist @@ -37,12 +43,19 @@ struct IndexOutOfDateChecker { } } - /// File paths to modification times that have already been computed. - private var modTimeCache: [String: ModificationTime] = [:] + /// File URLs to modification times that have already been computed. + private var modTimeCache: [URL: ModificationTime] = [:] + + /// Caches whether a file URL has modifications in `documentManager` that haven't been saved to disk yet. + private var hasFileInMemoryModificationsCache: [URL: Bool] = [:] - private func modificationDateUncached(of path: String) throws -> ModificationTime { + init(documentManager: DocumentManager) { + self.documentManager = documentManager + } + + private func modificationDateUncached(of url: URL) throws -> ModificationTime { do { - let attributes = try FileManager.default.attributesOfItem(atPath: path) + let attributes = try FileManager.default.attributesOfItem(atPath: url.path) guard let modificationDate = attributes[FileAttributeKey.modificationDate] as? Date else { throw Error.fileAttributesDontHaveModificationDate } @@ -52,20 +65,46 @@ struct IndexOutOfDateChecker { } } - private mutating func modificationDate(of path: String) throws -> ModificationTime { - if let cached = modTimeCache[path] { + private mutating func modificationDate(of url: URL) throws -> ModificationTime { + if let cached = modTimeCache[url] { return cached } - let modTime = try modificationDateUncached(of: path) - modTimeCache[path] = modTime + let modTime = try modificationDateUncached(of: url) + modTimeCache[url] = modTime return modTime } + private func hasFileInMemoryModificationsUncached(at url: URL) -> Bool { + guard let document = try? documentManager.latestSnapshot(DocumentURI(url)) else { + return false + } + + guard let onDiskFileContents = try? String(contentsOf: url, encoding: .utf8) else { + // If we can't read the file on disk, it's an in-memory document + return true + } + return onDiskFileContents != document.lineTable.content + } + + /// Returns `true` if the file has modified in-memory state, ie. if the version stored in the `DocumentManager` is + /// different than the version on disk. + public mutating func fileHasInMemoryModifications(_ url: URL) -> Bool { + if let cached = hasFileInMemoryModificationsCache[url] { + return cached + } + let hasInMemoryModifications = hasFileInMemoryModificationsUncached(at: url) + hasFileInMemoryModificationsCache[url] = hasInMemoryModifications + return hasInMemoryModifications + } + /// Returns `true` if the source file for the given symbol location exists and has not been modified after it has been /// indexed. mutating func isUpToDate(_ symbolLocation: SymbolLocation) -> Bool { + if fileHasInMemoryModifications(URL(fileURLWithPath: symbolLocation.path)) { + return false + } do { - let sourceFileModificationDate = try modificationDate(of: symbolLocation.path) + let sourceFileModificationDate = try modificationDate(of: URL(fileURLWithPath: symbolLocation.path)) switch sourceFileModificationDate { case .fileDoesNotExist: return false @@ -81,8 +120,11 @@ 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: String, index: IndexStoreDB) -> Bool { - guard let lastUnitDate = index.dateOfLatestUnitFor(filePath: filePath) else { + mutating func indexHasUpToDateUnit(for filePath: URL, index: IndexStoreDB) -> Bool { + if fileHasInMemoryModifications(filePath) { + return false + } + guard let lastUnitDate = index.dateOfLatestUnitFor(filePath: filePath.path) else { return false } do { diff --git a/Sources/SourceKitLSP/Rename.swift b/Sources/SourceKitLSP/Rename.swift index 43c6be799..4cf6bcdb8 100644 --- a/Sources/SourceKitLSP/Rename.swift +++ b/Sources/SourceKitLSP/Rename.swift @@ -283,16 +283,6 @@ private extension LineTable { } } -private extension DocumentSnapshot { - init?(_ uri: DocumentURI, language: Language) throws { - guard let url = uri.fileURL else { - return nil - } - let contents = try String(contentsOf: url) - self.init(uri: DocumentURI(url), language: language, version: 0, lineTable: LineTable(contents)) - } -} - private extension RenameLocation.Usage { init(roles: SymbolRole) { if roles.contains(.definition) || roles.contains(.declaration) { @@ -495,7 +485,7 @@ extension DocumentManager { /// Returns the latest open snapshot of `uri` or, if no document with that URI is open, reads the file contents of /// that file from disk. fileprivate func latestSnapshotOrDisk(_ uri: DocumentURI, language: Language) -> DocumentSnapshot? { - return (try? self.latestSnapshot(uri)) ?? (try? DocumentSnapshot.init(uri, language: language)) + return (try? self.latestSnapshot(uri)) ?? (try? DocumentSnapshot(withContentsFromDisk: uri, language: language)) } } @@ -520,7 +510,7 @@ extension SourceKitLSPServer { guard let reference else { return nil } - let uri = DocumentURI(URL(fileURLWithPath: reference.location.path)) + let uri = reference.location.documentUri guard let snapshot = self.documentManager.latestSnapshotOrDisk(uri, language: .swift) else { return nil } @@ -588,7 +578,7 @@ extension SourceKitLSPServer { case .objc: .objective_c case .swift: .swift } - let definitionDocumentUri = DocumentURI(URL(fileURLWithPath: definitionOccurrence.location.path)) + let definitionDocumentUri = definitionOccurrence.location.documentUri guard let definitionLanguageService = await self.languageService( diff --git a/Sources/SourceKitLSP/SourceKitLSPServer.swift b/Sources/SourceKitLSP/SourceKitLSPServer.swift index 649a3b9be..211b8a632 100644 --- a/Sources/SourceKitLSP/SourceKitLSPServer.swift +++ b/Sources/SourceKitLSP/SourceKitLSPServer.swift @@ -198,6 +198,14 @@ fileprivate enum TaskMetadata: DependencyTracker { /// might be relying on this task to take effect. case globalConfigurationChange + /// A request that depends on the state of all documents. + /// + /// These requests wait for `documentUpdate` tasks for all documents to finish before being executed. + /// + /// Requests that only read the semantic index and are not affected by changes to the in-memory file contents should + /// `freestanding` requests. + case workspaceRequest + /// Changes the contents of the document with the given URI. /// /// Any other updates or requests to this document must wait for the @@ -218,16 +226,30 @@ fileprivate enum TaskMetadata: DependencyTracker { /// Whether this request needs to finish before `other` can start executing. func isDependency(of other: TaskMetadata) -> Bool { switch (self, other) { + // globalConfigurationChange case (.globalConfigurationChange, _): return true case (_, .globalConfigurationChange): return true + + // globalDocumentState + case (.workspaceRequest, .workspaceRequest): return false + case (.documentUpdate, .workspaceRequest): return true + case (.workspaceRequest, .documentUpdate): return true + case (.workspaceRequest, .documentRequest): return false + case (.documentRequest, .workspaceRequest): return false + + // documentUpdate case (.documentUpdate(let selfUri), .documentUpdate(let otherUri)): return selfUri == otherUri case (.documentUpdate(let selfUri), .documentRequest(let otherUri)): return selfUri == otherUri case (.documentRequest(let selfUri), .documentUpdate(let otherUri)): return selfUri == otherUri + + // documentRequest case (.documentRequest, .documentRequest): return false + + // freestanding case (.freestanding, _): return false case (_, .freestanding): @@ -248,7 +270,7 @@ fileprivate enum TaskMetadata: DependencyTracker { case let notification as DidChangeTextDocumentNotification: self = .documentUpdate(notification.textDocument.uri) case is DidChangeWatchedFilesNotification: - self = .freestanding + self = .globalConfigurationChange case is DidChangeWorkspaceFoldersNotification: self = .globalConfigurationChange case let notification as DidCloseNotebookDocumentNotification: @@ -372,7 +394,7 @@ fileprivate enum TaskMetadata: DependencyTracker { case is WorkspaceSymbolsRequest: self = .freestanding case is WorkspaceTestsRequest: - self = .freestanding + self = .workspaceRequest default: logger.error( """ @@ -1624,7 +1646,7 @@ extension SourceKitLSPServer { // (e.g. Package.swift doesn't have build settings but affects build // settings). Inform the build system about all file changes. for workspace in workspaces { - await workspace.buildSystemManager.filesDidChange(notification.changes) + await workspace.filesDidChange(notification.changes) } } @@ -1826,7 +1848,7 @@ extension SourceKitLSPServer { private func indexToLSPLocation(_ location: SymbolLocation) -> Location? { guard !location.path.isEmpty else { return nil } return Location( - uri: DocumentURI(URL(fileURLWithPath: location.path)), + uri: location.documentUri, range: Range( Position( // 1-based -> 0-based @@ -2584,7 +2606,7 @@ extension WorkspaceSymbolItem { ) let symbolLocation = Location( - uri: DocumentURI(URL(fileURLWithPath: symbolOccurrence.location.path)), + uri: symbolOccurrence.location.documentUri, range: Range(symbolPosition) ) diff --git a/Sources/SourceKitLSP/Swift/SwiftTestingScanner.swift b/Sources/SourceKitLSP/Swift/SwiftTestingScanner.swift index 3e65a74d1..84b9e0316 100644 --- a/Sources/SourceKitLSP/Swift/SwiftTestingScanner.swift +++ b/Sources/SourceKitLSP/Swift/SwiftTestingScanner.swift @@ -180,6 +180,13 @@ final class SyntacticSwiftTestingTestScanner: SyntaxVisitor { in snapshot: DocumentSnapshot, syntaxTreeManager: SyntaxTreeManager ) async -> [TestItem] { + guard snapshot.text.contains("Suite") || snapshot.text.contains("Test") else { + // If the file contains swift-testing tests, it must contain a `@Suite` or `@Test` attribute. + // Only check for the attribute name because the attribute may be module qualified and contain an arbitrary amount + // of whitespace. + // This is intended to filter out files that obviously do not contain tests. + return [] + } let syntaxTree = await syntaxTreeManager.syntaxTree(for: snapshot) let visitor = SyntacticSwiftTestingTestScanner(snapshot: snapshot, allTestsDisabled: false, parentTypeNames: []) visitor.walk(syntaxTree) diff --git a/Sources/SourceKitLSP/Swift/SyntacticTestIndex.swift b/Sources/SourceKitLSP/Swift/SyntacticTestIndex.swift new file mode 100644 index 000000000..8884e18ec --- /dev/null +++ b/Sources/SourceKitLSP/Swift/SyntacticTestIndex.swift @@ -0,0 +1,237 @@ +//===----------------------------------------------------------------------===// +// +// 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 LSPLogging +import LanguageServerProtocol +import SKSupport + +/// Task metadata for `SyntacticTestIndexer.indexingQueue` +fileprivate enum TaskMetadata: DependencyTracker, Equatable { + case read + case index(Set) + + /// Reads can be concurrent and files can be indexed concurrently. But we need to wait for all files to finish + /// indexing before reading the index. + func isDependency(of other: TaskMetadata) -> Bool { + switch (self, other) { + case (.read, .read): + // We allow concurrent reads + return false + case (.read, .index(_)): + // We allow index tasks scheduled after a read task to be be executed before the read. + // This effectively means that a `read` requires the index to be updated *at least* up to the state at which the + // read was scheduled. If more changes come in in the meantime, it is OK for the read to pick them up. This also + // ensures that reads aren't parallelization barriers. + return false + case (.index(_), .read): + // We require all index tasks scheduled before the read to be finished. + // This ensures that the index has been updated at least to the state of file at which the read was scheduled. + // Adding the dependency also elevates the index task's priorities. + return true + case (.index(let lhsUris), .index(let rhsUris)): + // Technically, we should be able to allow simultaneous indexing of the same file. But conceptually the code + // becomes simpler if we don't need to think racing indexing tasks for the same file and it shouldn't make a + // performance impact in practice because if a first task indexes a file, a subsequent index task for the same + // file will realize that the index is already up-to-date based on the file's mtime and early exit. + return !lhsUris.intersection(rhsUris).isEmpty + } + } +} + +/// Data from a syntactic scan of a source file for tests. +fileprivate struct IndexedTests { + /// The tests within the source file. + let tests: [TestItem] + + /// The modification date of the source file when it was scanned. A file won't get re-scanned if its modification date + /// is older or the same as this date. + let sourceFileModificationDate: Date +} + +/// Syntactically scans the file at the given URL for tests declared within it. +/// +/// Does not write the results to the index. +/// +/// The order of the returned tests is not defined. The results should be sorted before being returned to the editor. +fileprivate func testItems(in url: URL) async -> [TestItem] { + guard url.pathExtension == "swift" else { + return [] + } + let syntaxTreeManager = SyntaxTreeManager() + let snapshot = orLog("Getting document snapshot for swift-testing scanning") { + try DocumentSnapshot(withContentsFromDisk: url, language: .swift) + } + guard let snapshot else { + return [] + } + async let swiftTestingTests = SyntacticSwiftTestingTestScanner.findTestSymbols( + in: snapshot, + syntaxTreeManager: syntaxTreeManager + ) + async let xcTests = SyntacticSwiftXCTestScanner.findTestSymbols(in: snapshot, syntaxTreeManager: syntaxTreeManager) + return await swiftTestingTests + xcTests +} + +/// An in-memory syntactic index of test items within a workspace. +/// +/// The index does not get persisted to disk but instead gets rebuilt every time a workspace is opened (ie. usually when +/// sourcekit-lsp is launched). Building it takes only a few seconds, even for large projects. +actor SyntacticTestIndex { + /// The tests discovered by the index. + private var indexedTests: [DocumentURI: IndexedTests] = [:] + + /// Files that have been removed using `removeFileForIndex`. + /// + /// We need to keep track of these files because when the files get removed, there might be an in-progress indexing + /// operation running for that file. We need to ensure that this indexing operation doesn't add the removed file + /// back to `indexTests`. + private var removedFiles: Set = [] + + /// The queue on which the index is being updated and queried. + /// + /// Tracking dependencies between tasks within this queue allows us to start indexing tasks in parallel with low + /// priority and elevate their priority once a read task comes in, which has higher priority and depends on the + /// indexing tasks to finish. + private let indexingQueue = AsyncQueue() + + init() {} + + private func removeFilesFromIndex(_ removedFiles: Set) { + self.removedFiles.formUnion(removedFiles) + for removedFile in removedFiles { + self.indexedTests[removedFile] = nil + } + } + + /// Called when the list of files that may contain tests is updated. + /// + /// All files that are not in the new list of test files will be removed from the index. + func listOfTestFilesDidChange(_ testFiles: [DocumentURI]) { + let removedFiles = Set(self.indexedTests.keys).subtracting(testFiles) + removeFilesFromIndex(removedFiles) + + rescanFiles(testFiles) + } + + func filesDidChange(_ events: [FileEvent]) { + for fileEvent in events { + switch fileEvent.type { + case .created: + // We don't know if this is a potential test file. It would need to be added to the index via + // `listOfTestFilesDidChange` + break + case .changed: + rescanFiles([fileEvent.uri]) + case .deleted: + removeFilesFromIndex([fileEvent.uri]) + default: + logger.error("Ignoring unknown FileEvent type \(fileEvent.type.rawValue) in SyntacticTestIndex") + } + } + } + + /// Called when a list of files was updated. Re-scans those files + private func rescanFiles(_ uris: [DocumentURI]) { + // If we scan a file again, it might have been added after being removed before. Remove it from the list of removed + // files. + removedFiles.subtract(uris) + + // Divide the files into multiple batches. This is more efficient than spawning a new task for every file, mostly + // because it keeps the number of pending items in `indexingQueue` low and adding a new task to `indexingQueue` is + // in O(number of pending tasks), since we need to scan for dependency edges to add, which would make scanning files + // be O(number of files). + // Over-subscribe the processor count in case one batch finishes more quickly than another. + let batches = uris.partition(intoNumberOfBatches: ProcessInfo.processInfo.processorCount * 4) + for batch in batches { + self.indexingQueue.async(priority: .low, metadata: .index(Set(batch))) { + for uri in batch { + await self.rescanFileAssumingOnQueue(uri) + } + } + } + } + + /// Re-scans a single file. + /// + /// - Important: This method must be called in a task that is executing on `indexingQueue`. + private func rescanFileAssumingOnQueue(_ uri: DocumentURI) async { + guard let url = uri.fileURL else { + logger.log("Not indexing \(uri.forLogging) for swift-testing tests because it is not a file URL") + return + } + if Task.isCancelled { + return + } + guard !removedFiles.contains(uri) else { + return + } + guard + let fileModificationDate = try? FileManager.default.attributesOfItem(atPath: url.path)[.modificationDate] + as? Date + else { + logger.fault("Not indexing \(uri.forLogging) for tests because the modification date could not be determined") + return + } + if let indexModificationDate = self.indexedTests[uri]?.sourceFileModificationDate, + indexModificationDate >= fileModificationDate + { + // Index already up to date. + return + } + if Task.isCancelled { + return + } + let testItems = await testItems(in: url) + + guard !removedFiles.contains(uri) else { + // Check whether the file got removed while we were scanning it for tests. If so, don't add it back to + // `indexedTests`. + return + } + self.indexedTests[uri] = IndexedTests(tests: testItems, sourceFileModificationDate: fileModificationDate) + } + + /// Gets all the tests in the syntactic index. + /// + /// This waits for any pending document updates to be indexed before returning a result. + nonisolated func tests() async -> [TestItem] { + let readTask = indexingQueue.async(metadata: .read) { + return await self.indexedTests.values.flatMap { $0.tests } + } + return await readTask.value + } +} + +fileprivate extension Collection { + /// Partition the elements of the collection into `numberOfBatches` roughly equally sized batches. + /// + /// Elements are assigned to the batches round-robin. This ensures that elements that are close to each other in the + /// original collection end up in different batches. This is important because eg. test files will live close to each + /// other in the file system and test scanning wants to scan them in different batches so we don't end up with one + /// batch only containing source files and the other only containing test files. + func partition(intoNumberOfBatches numberOfBatches: Int) -> [[Element]] { + var batches: [[Element]] = Array( + repeating: { + var batch: [Element] = [] + batch.reserveCapacity(self.count / numberOfBatches) + return batch + }(), + count: numberOfBatches + ) + + for (index, element) in self.enumerated() { + batches[index % numberOfBatches].append(element) + } + return batches.filter { !$0.isEmpty } + } +} diff --git a/Sources/SourceKitLSP/SymbolLocation+DocumentURI.swift b/Sources/SourceKitLSP/SymbolLocation+DocumentURI.swift new file mode 100644 index 000000000..25dc797ac --- /dev/null +++ b/Sources/SourceKitLSP/SymbolLocation+DocumentURI.swift @@ -0,0 +1,20 @@ +//===----------------------------------------------------------------------===// +// +// 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 IndexStoreDB +import LanguageServerProtocol + +extension SymbolLocation { + var documentUri: DocumentURI { + return DocumentURI(URL(fileURLWithPath: self.path)) + } +} diff --git a/Sources/SourceKitLSP/TestDiscovery.swift b/Sources/SourceKitLSP/TestDiscovery.swift index ef4b234a9..938436254 100644 --- a/Sources/SourceKitLSP/TestDiscovery.swift +++ b/Sources/SourceKitLSP/TestDiscovery.swift @@ -109,7 +109,7 @@ extension SourceKitLSPServer { ) -> TestItem { let symbolPosition: Position if let snapshot = try? documentManager.latestSnapshot( - DocumentURI(URL(fileURLWithPath: testSymbolOccurrence.location.path)) + testSymbolOccurrence.location.documentUri ) { symbolPosition = snapshot.position(of: testSymbolOccurrence.location) } else { @@ -122,8 +122,7 @@ extension SourceKitLSPServer { ) } let id = (context + [testSymbolOccurrence.symbol.name]).joined(separator: "/") - let uri = DocumentURI(URL(fileURLWithPath: testSymbolOccurrence.location.path)) - let location = resolveLocation(uri, symbolPosition) + let location = resolveLocation(testSymbolOccurrence.location.documentUri, symbolPosition) let children = occurrencesByParent[testSymbolOccurrence.symbol.usr, default: []] @@ -147,16 +146,92 @@ extension SourceKitLSPServer { .map { testItem(for: $0, documentManager: documentManager, context: []) } } - func workspaceTests(_ req: WorkspaceTestsRequest) async throws -> [TestItem] { - // Gather all tests classes and test methods. - let testSymbolOccurrences = - workspaces - .flatMap { $0.index?.unitTests() ?? [] } - .filter { $0.canBeTestDefinition } - return testItems( - for: testSymbolOccurrences, + /// Return all the tests in the given workspace. + /// + /// This merges tests from the semantic index, the syntactic index and in-memory file states. + /// + /// The returned list of tests is not sorted. It should be sorted before being returned to the editor. + private func tests(in workspace: Workspace) async -> [TestItem] { + // Gather all tests classes and test methods. We include test from different sources: + // - For all files that have been not been modified since they were last indexed in the semantic index, include + // XCTests from the semantic index. + // - For all files that have been modified since the last semantic index but that don't have any in-memory + // modifications (ie. modifications that the user has made in the editor but not saved), include XCTests from + // the syntactic test index + // - For all files that don't have any in-memory modifications, include swift-testing tests from the syntactic test + // index. + // - All files that have in-memory modifications are syntactically scanned for tests here. + var outOfDateChecker = IndexOutOfDateChecker(documentManager: documentManager) + + let filesWithInMemoryState = documentManager.documents.keys.filter { uri in + guard let url = uri.fileURL else { + return true + } + return outOfDateChecker.fileHasInMemoryModifications(url) + } + + let testsFromFilesWithInMemoryState = await filesWithInMemoryState.concurrentMap { (uri) -> [TestItem] in + guard let languageService = workspace.documentService[uri] else { + return [] + } + return await orLog("Getting document tests for \(uri)") { + try await self.documentTests( + DocumentTestsRequest(textDocument: TextDocumentIdentifier(uri)), + workspace: workspace, + languageService: languageService + ) + } ?? [] + }.flatMap { $0 } + + let semanticTestSymbolOccurrences = + (workspace.index?.unitTests() ?? []) + .filter { return $0.canBeTestDefinition && outOfDateChecker.isUpToDate($0.location) } + + let testsFromSyntacticIndex = await workspace.syntacticTestIndex.tests() + let testsFromSemanticIndex = testItems( + for: semanticTestSymbolOccurrences, resolveLocation: { uri, position in Location(uri: uri, range: Range(position)) } ) + let filesWithTestsFromSemanticIndex = Set(testsFromSemanticIndex.map(\.location.uri)) + + let syntacticTestsToInclude = + testsFromSyntacticIndex + .filter { testItem in + if testItem.style == TestStyle.swiftTesting { + // Swift-testing tests aren't part of the semantic index. Always include them. + return true + } + if filesWithTestsFromSemanticIndex.contains(testItem.location.uri) { + // If we have an semantic tests from this file, then the semantic index is up-to-date for this file. We thus + // don't need to include results from the syntactic index. + return false + } + if filesWithInMemoryState.contains(testItem.location.uri) { + // If the file has been modified in the editor, the syntactic index (which indexes on-disk files) is no longer + // up-to-date. Include the tests from `testsFromFilesWithInMemoryState`. + return false + } + if let fileUrl = testItem.location.uri.fileURL, + let index = workspace.index, + outOfDateChecker.indexHasUpToDateUnit(for: fileUrl, index: index) + { + // We don't have a test for this file in the semantic index but an up-to-date unit file. This means that the + // index is up-to-date and has more knowledge that identifies a `TestItem` as not actually being a test, eg. + // because it starts with `test` but doesn't appear in a class inheriting from `XCTestCase`. + return false + } + return true + } + + // We don't need to sort the tests here because they will get + return testsFromSemanticIndex + syntacticTestsToInclude + testsFromFilesWithInMemoryState + } + + func workspaceTests(_ req: WorkspaceTestsRequest) async throws -> [TestItem] { + return await self.workspaces + .concurrentMap { await self.tests(in: $0) } + .flatMap { $0 } + .sorted { $0.location < $1.location } } /// Extracts a flat dictionary mapping test IDs to their locations from the given `testItems`. @@ -187,7 +262,7 @@ extension SourceKitLSPServer { syntacticTests.filter { $0.style == TestStyle.swiftTesting } } - var outOfDateChecker = IndexOutOfDateChecker() + var outOfDateChecker = IndexOutOfDateChecker(documentManager: documentManager) let testSymbols = index.unitTests(referencedByMainFiles: [mainFileUri.pseudoPath]) .filter { $0.canBeTestDefinition && outOfDateChecker.isUpToDate($0.location) } @@ -211,7 +286,7 @@ extension SourceKitLSPServer { } ) + syntacticSwiftTestingTests } - if let fileURL = mainFileUri.fileURL, outOfDateChecker.indexHasUpToDateUnit(for: fileURL.path, index: index) { + if let fileURL = mainFileUri.fileURL, outOfDateChecker.indexHasUpToDateUnit(for: fileURL, index: index) { // The semantic index is up-to-date and doesn't contain any tests. We don't need to do a syntactic fallback for // XCTest. We do still need to return swift-testing tests which don't have a semantic index. return syntacticSwiftTestingTests @@ -226,7 +301,7 @@ extension SourceKitLSPServer { /// /// The syntax visitor scans from class and extension declarations that could be `XCTestCase` classes or extensions /// thereof. It then calls into `findTestMethods` to find the actual test methods. -private final class SyntacticSwiftXCTestScanner: SyntaxVisitor { +final class SyntacticSwiftXCTestScanner: SyntaxVisitor { /// The document snapshot of the syntax tree that is being walked. private var snapshot: DocumentSnapshot @@ -242,6 +317,12 @@ private final class SyntacticSwiftXCTestScanner: SyntaxVisitor { in snapshot: DocumentSnapshot, syntaxTreeManager: SyntaxTreeManager ) async -> [TestItem] { + guard snapshot.text.contains("XCTestCase") || snapshot.text.contains("test") else { + // If the file contains tests that can be discovered syntactically, it needs to have a class inheriting from + // `XCTestCase` or a function starting with `test`. + // This is intended to filter out files that obviously do not contain tests. + return [] + } let syntaxTree = await syntaxTreeManager.syntaxTree(for: snapshot) let visitor = SyntacticSwiftXCTestScanner(snapshot: snapshot) visitor.walk(syntaxTree) diff --git a/Sources/SourceKitLSP/Workspace.swift b/Sources/SourceKitLSP/Workspace.swift index 97add89ce..9b5111f83 100644 --- a/Sources/SourceKitLSP/Workspace.swift +++ b/Sources/SourceKitLSP/Workspace.swift @@ -62,6 +62,9 @@ public final class Workspace { /// The source code index, if available. public var index: IndexStoreDB? = nil + /// The index that syntactically scans the workspace for tests. + let syntacticTestIndex = SyntacticTestIndex() + /// Documents open in the SourceKitLSPServer. This may include open documents from other workspaces. private let documentManager: DocumentManager @@ -91,6 +94,14 @@ public final class Workspace { await indexDelegate?.addMainFileChangedCallback { [weak self] in await self?.buildSystemManager.mainFilesChanged() } + await underlyingBuildSystem?.addTestFilesDidChangeCallback { [weak self] in + guard let self else { + return + } + await self.syntacticTestIndex.listOfTestFilesDidChange(self.buildSystemManager.testFiles()) + } + // Trigger an initial population of `syntacticTestIndex`. + await syntacticTestIndex.listOfTestFilesDidChange(buildSystemManager.testFiles()) } /// Creates a workspace for a given root `URL`, inferring the `ExternalWorkspace` if possible. @@ -205,6 +216,11 @@ public final class Workspace { indexDelegate: indexDelegate ) } + + public func filesDidChange(_ events: [FileEvent]) async { + await buildSystemManager.filesDidChange(events) + await syntacticTestIndex.filesDidChange(events) + } } /// Wrapper around a workspace that isn't being retained. diff --git a/Tests/SKCoreTests/BuildSystemManagerTests.swift b/Tests/SKCoreTests/BuildSystemManagerTests.swift index f0c71e0b1..54a20dddb 100644 --- a/Tests/SKCoreTests/BuildSystemManagerTests.swift +++ b/Tests/SKCoreTests/BuildSystemManagerTests.swift @@ -458,6 +458,12 @@ class ManualBuildSystem: BuildSystem { return .unhandled } } + + func testFiles() async -> [DocumentURI] { + return [] + } + + func addTestFilesDidChangeCallback(_ callback: @escaping () async -> Void) {} } /// A `BuildSystemDelegate` setup for testing. diff --git a/Tests/SourceKitLSPTests/BuildSystemTests.swift b/Tests/SourceKitLSPTests/BuildSystemTests.swift index b6e389858..4e5cd46f0 100644 --- a/Tests/SourceKitLSPTests/BuildSystemTests.swift +++ b/Tests/SourceKitLSPTests/BuildSystemTests.swift @@ -60,6 +60,12 @@ final class TestBuildSystem: BuildSystem { return .unhandled } } + + func testFiles() async -> [DocumentURI] { + return [] + } + + func addTestFilesDidChangeCallback(_ callback: @escaping () async -> Void) async {} } final class BuildSystemTests: XCTestCase { diff --git a/Tests/SourceKitLSPTests/DefinitionTests.swift b/Tests/SourceKitLSPTests/DefinitionTests.swift index a6aae84d1..caba17e48 100644 --- a/Tests/SourceKitLSPTests/DefinitionTests.swift +++ b/Tests/SourceKitLSPTests/DefinitionTests.swift @@ -46,7 +46,7 @@ class DefinitionTests: XCTestCase { func 1️⃣doThing() } - struct TestImpl: TestProtocol { + struct TestImpl: TestProtocol { func 2️⃣doThing() { } } @@ -297,13 +297,7 @@ class DefinitionTests: XCTestCase { XCTAssertEqual(locations.count, 1) let location = try XCTUnwrap(locations.first) - XCTAssertEqual( - location, - Location( - uri: try project.uri(for: "LibA.h"), - range: try project.position(of: "1️⃣", in: "LibA.h")..