diff --git a/Sources/LanguageServerProtocol/Requests/DocumentTestsRequest.swift b/Sources/LanguageServerProtocol/Requests/DocumentTestsRequest.swift index 9445a42d..c79dffac 100644 --- a/Sources/LanguageServerProtocol/Requests/DocumentTestsRequest.swift +++ b/Sources/LanguageServerProtocol/Requests/DocumentTestsRequest.swift @@ -15,7 +15,7 @@ /// **(LSP Extension)** public struct DocumentTestsRequest: TextDocumentRequest, Hashable { public static let method: String = "textDocument/tests" - public typealias Response = [WorkspaceSymbolItem]? + public typealias Response = [TestItem] public var textDocument: TextDocumentIdentifier diff --git a/Sources/LanguageServerProtocol/SupportTypes/TestItem.swift b/Sources/LanguageServerProtocol/SupportTypes/TestItem.swift index 67befa80..b91ccba9 100644 --- a/Sources/LanguageServerProtocol/SupportTypes/TestItem.swift +++ b/Sources/LanguageServerProtocol/SupportTypes/TestItem.swift @@ -35,13 +35,16 @@ public struct TestItem: ResponseType, Equatable { public let description: String? /// A string that should be used when comparing this item with other items. + /// /// When `nil` the `label` is used. public let sortText: String? /// The location of the test item in the source code. public let location: Location - /// The children of this test item. For a test suite, this may contain the individual test cases or nested suites. + /// The children of this test item. + /// + /// For a test suite, this may contain the individual test cases or nested suites. public let children: [TestItem] /// Tags associated with this test item. diff --git a/Sources/SourceKitLSP/LanguageService.swift b/Sources/SourceKitLSP/LanguageService.swift index 85d72043..da124411 100644 --- a/Sources/SourceKitLSP/LanguageService.swift +++ b/Sources/SourceKitLSP/LanguageService.swift @@ -198,7 +198,7 @@ public protocol LanguageService: AnyObject { /// Perform a syntactic scan of the file at the given URI for test cases and test classes. /// /// This is used as a fallback to show the test cases in a file if the index for a given file is not up-to-date. - func syntacticDocumentTests(for uri: DocumentURI) async throws -> [WorkspaceSymbolItem]? + func syntacticDocumentTests(for uri: DocumentURI) async throws -> [TestItem] /// Crash the language server. Should be used for crash recovery testing only. func _crash() async diff --git a/Sources/SourceKitLSP/SourceKitLSPServer.swift b/Sources/SourceKitLSP/SourceKitLSPServer.swift index ac21d14b..af3f4f93 100644 --- a/Sources/SourceKitLSP/SourceKitLSPServer.swift +++ b/Sources/SourceKitLSP/SourceKitLSPServer.swift @@ -1318,7 +1318,7 @@ extension SourceKitLSPServer { inlayHintProvider: inlayHintOptions, experimental: .dictionary([ "workspace/tests": .dictionary(["version": .int(2)]), - "textDocument/tests": .dictionary(["version": .int(1)]), + "textDocument/tests": .dictionary(["version": .int(2)]), ]) ) } diff --git a/Sources/SourceKitLSP/TestDiscovery.swift b/Sources/SourceKitLSP/TestDiscovery.swift index b6532663..77c35a08 100644 --- a/Sources/SourceKitLSP/TestDiscovery.swift +++ b/Sources/SourceKitLSP/TestDiscovery.swift @@ -33,13 +33,44 @@ fileprivate extension SymbolOccurrence { } } -extension SourceKitLSPServer { - func workspaceTests(_ req: WorkspaceTestsRequest) async throws -> [TestItem] { - // Gather all tests classes and test methods. - let testSymbolOccurrences = workspaces.flatMap { (workspace) -> [SymbolOccurrence] in - return workspace.index?.unitTests() ?? [] +/// Find the innermost range of a document symbol that contains the given position. +private func findInnermostSymbolRange( + containing position: Position, + documentSymbols documentSymbolsResponse: DocumentSymbolResponse +) -> Range? { + guard case .documentSymbols(let documentSymbols) = documentSymbolsResponse else { + // Both `ClangLanguageService` and `SwiftLanguageService` return `documentSymbols` so we don't need to handle the + // .symbolInformation case. + logger.fault( + """ + Expected documentSymbols response from language service to resolve test ranges but got \ + \(documentSymbolsResponse.forLogging) + """ + ) + return nil + } + for documentSymbol in documentSymbols where documentSymbol.range.contains(position) { + if let children = documentSymbol.children, + let rangeOfChild = findInnermostSymbolRange(containing: position, documentSymbols: .documentSymbols(children)) + { + // If a child contains the position, prefer that because it's more specific. + return rangeOfChild } + return documentSymbol.range + } + return nil +} +extension SourceKitLSPServer { + /// Converts a flat list of test symbol occurrences to a hierarchical `TestItem` array, inferring the hierarchical + /// structure from `childOf` relations between the symbol occurrences. + /// + /// `resolvePositions` resolves the position of a test to a `Location` that is effectively a range. This allows us to + /// provide ranges for the test cases in source code instead of only the test's location that we get from the index. + private func testItems( + for testSymbolOccurrences: [SymbolOccurrence], + resolveLocation: (DocumentURI, Position) -> Location + ) -> [TestItem] { // Arrange tests by the USR they are contained in. This allows us to emit test methods as children of test classes. // `occurrencesByParent[nil]` are the root test symbols that aren't a child of another test symbol. var occurrencesByParent: [String?: [SymbolOccurrence]] = [:] @@ -66,25 +97,41 @@ extension SourceKitLSPServer { /// `context` is used to build the test's ID. It is an array containing the names of all parent symbols. These will /// be joined with the test symbol's name using `/` to form the test ID. The test ID can be used to run an /// individual test. - func testItem(for testSymbolOccurrence: SymbolOccurrence, context: [String]) -> TestItem { - let symbolPosition = Position( - line: testSymbolOccurrence.location.line - 1, // 1-based -> 0-based - // FIXME: we need to convert the utf8/utf16 column, which may require reading the file! - utf16index: testSymbolOccurrence.location.utf8Column - 1 - ) + func testItem( + for testSymbolOccurrence: SymbolOccurrence, + documentManager: DocumentManager, + context: [String] + ) -> TestItem { + let symbolPosition: Position + if let snapshot = try? documentManager.latestSnapshot( + DocumentURI(URL(fileURLWithPath: testSymbolOccurrence.location.path)) + ), + let position = snapshot.position(of: testSymbolOccurrence.location) + { + symbolPosition = position + } else { + // Technically, we always need to convert UTF-8 columns to UTF-16 columns, which requires reading the file. + // In practice, they are almost always the same. + // We chose to avoid hitting the file system even if it means that we might report an incorrect column. + symbolPosition = Position( + line: testSymbolOccurrence.location.line - 1, // 1-based -> 0-based + utf16index: testSymbolOccurrence.location.utf8Column - 1 + ) + } + let id = (context + [testSymbolOccurrence.symbol.name]).joined(separator: "/") + let uri = DocumentURI(URL(fileURLWithPath: testSymbolOccurrence.location.path)) + let location = resolveLocation(uri, symbolPosition) - let symbolLocation = Location( - uri: DocumentURI(URL(fileURLWithPath: testSymbolOccurrence.location.path)), - range: Range(symbolPosition) - ) let children = occurrencesByParent[testSymbolOccurrence.symbol.usr, default: []] .sorted() - .map { testItem(for: $0, context: context + [testSymbolOccurrence.symbol.name]) } + .map { + testItem(for: $0, documentManager: documentManager, context: context + [testSymbolOccurrence.symbol.name]) + } return TestItem( - id: (context + [testSymbolOccurrence.symbol.name]).joined(separator: "/"), + id: id, label: testSymbolOccurrence.symbol.name, - location: symbolLocation, + location: location, children: children, tags: [] ) @@ -92,19 +139,42 @@ extension SourceKitLSPServer { return occurrencesByParent[nil, default: []] .sorted() - .map { testItem(for: $0, context: []) } + .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, + resolveLocation: { uri, position in Location(uri: uri, range: Range(position)) } + ) + } + + /// Extracts a flat dictionary mapping test IDs to their locations from the given `testItems`. + private func testLocations(from testItems: [TestItem]) -> [String: Location] { + var result: [String: Location] = [:] + for testItem in testItems { + result[testItem.id] = testItem.location + result.merge(testLocations(from: testItem.children)) { old, new in new } + } + return result } func documentTests( _ req: DocumentTestsRequest, workspace: Workspace, languageService: LanguageService - ) async throws -> [WorkspaceSymbolItem]? { + ) async throws -> [TestItem] { let snapshot = try self.documentManager.latestSnapshot(req.textDocument.uri) let mainFileUri = await workspace.buildSystemManager.mainFile( for: req.textDocument.uri, language: snapshot.language ) + if let index = workspace.index { var outOfDateChecker = IndexOutOfDateChecker() let testSymbols = @@ -112,7 +182,21 @@ extension SourceKitLSPServer { .filter { $0.canBeTestDefinition && outOfDateChecker.isUpToDate($0.location) } if !testSymbols.isEmpty { - return testSymbols.sorted().map(WorkspaceSymbolItem.init) + let documentSymbols = await orLog("Getting document symbols for test ranges") { + try await languageService.documentSymbol(DocumentSymbolRequest(textDocument: req.textDocument)) + } + + return testItems( + for: testSymbols, + resolveLocation: { uri, position in + if uri == snapshot.uri, let documentSymbols, + let range = findInnermostSymbolRange(containing: position, documentSymbols: documentSymbols) + { + return Location(uri: uri, range: range) + } + return Location(uri: uri, range: Range(position)) + } + ) } if outOfDateChecker.indexHasUpToDateUnit(for: mainFileUri.pseudoPath, index: index) { // The index is up-to-date and doesn't contain any tests. We don't need to do a syntactic fallback. @@ -133,7 +217,7 @@ private final class SyntacticSwiftXCTestScanner: SyntaxVisitor { private var snapshot: DocumentSnapshot /// The workspace symbols representing the found `XCTestCase` subclasses and test methods. - private var result: [WorkspaceSymbolItem] = [] + private var result: [TestItem] = [] /// Names of classes that are known to not inherit from `XCTestCase` and can thus be ruled out to be test classes. private static let knownNonXCTestSubclasses = ["NSObject"] @@ -146,15 +230,15 @@ private final class SyntacticSwiftXCTestScanner: SyntaxVisitor { public static func findTestSymbols( in snapshot: DocumentSnapshot, syntaxTreeManager: SyntaxTreeManager - ) async -> [WorkspaceSymbolItem] { + ) async -> [TestItem] { let syntaxTree = await syntaxTreeManager.syntaxTree(for: snapshot) let visitor = SyntacticSwiftXCTestScanner(snapshot: snapshot) visitor.walk(syntaxTree) return visitor.result } - private func findTestMethods(in members: MemberBlockItemListSyntax, containerName: String) -> [WorkspaceSymbolItem] { - return members.compactMap { (member) -> WorkspaceSymbolItem? in + private func findTestMethods(in members: MemberBlockItemListSyntax, containerName: String) -> [TestItem] { + return members.compactMap { (member) -> TestItem? in guard let function = member.decl.as(FunctionDeclSyntax.self) else { return nil } @@ -166,22 +250,26 @@ private final class SyntacticSwiftXCTestScanner: SyntaxVisitor { // Test methods can't be static. return nil } - guard function.signature.returnClause == nil else { - // Test methods can't have a return type. + guard function.signature.returnClause == nil, function.signature.parameterClause.parameters.isEmpty else { + // Test methods can't have a return type or have parameters. // Technically we are also filtering out functions that have an explicit `Void` return type here but such // declarations are probably less common than helper functions that start with `test` and have a return type. return nil } - guard let position = snapshot.position(of: function.name.positionAfterSkippingLeadingTrivia) else { + guard + let range = snapshot.range( + of: function.positionAfterSkippingLeadingTrivia.. [WorkspaceSymbolItem]? { + public func syntacticDocumentTests(for uri: DocumentURI) async throws -> [TestItem] { let snapshot = try documentManager.latestSnapshot(uri) return await SyntacticSwiftXCTestScanner.findTestSymbols(in: snapshot, syntaxTreeManager: syntaxTreeManager) } } extension ClangLanguageService { - public func syntacticDocumentTests(for uri: DocumentURI) async -> [WorkspaceSymbolItem]? { - return nil + public func syntacticDocumentTests(for uri: DocumentURI) async -> [TestItem] { + return [] } } diff --git a/Tests/SourceKitLSPTests/TestDiscoveryTests.swift b/Tests/SourceKitLSPTests/TestDiscoveryTests.swift index cd9f79ed..f2bd3373 100644 --- a/Tests/SourceKitLSPTests/TestDiscoveryTests.swift +++ b/Tests/SourceKitLSPTests/TestDiscoveryTests.swift @@ -80,11 +80,11 @@ final class TestDiscoveryTests: XCTestCase { "Tests/MyLibraryTests/MyTests.swift": """ import XCTest - class 1️⃣MyTests: XCTestCase { - func 2️⃣testMyLibrary() {} + 1️⃣class MyTests: XCTestCase { + 2️⃣func testMyLibrary() {}3️⃣ func unrelatedFunc() {} var testVariable: Int = 0 - } + }4️⃣ """, "Tests/MyLibraryTests/MoreTests.swift": """ import XCTest @@ -112,27 +112,21 @@ final class TestDiscoveryTests: XCTestCase { XCTAssertEqual( tests, [ - WorkspaceSymbolItem.symbolInformation( - SymbolInformation( - name: "MyTests", - kind: .class, - location: Location( - uri: uri, - range: Range(positions["1️⃣"]) + TestItem( + id: "MyTests", + label: "MyTests", + location: Location(uri: uri, range: positions["1️⃣"]..