Skip to content

Implement a syntactic test discovery fallback for XCTests written in Swift #1148

New issue

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

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

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Sources/SourceKitLSP/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
add_library(SourceKitLSP STATIC
CapabilityRegistry.swift
DocumentManager.swift
IndexOutOfDateChecker.swift
IndexStoreDB+MainFilesProvider.swift
LanguageService.swift
Rename.swift
Expand Down
101 changes: 101 additions & 0 deletions Sources/SourceKitLSP/IndexOutOfDateChecker.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
//===----------------------------------------------------------------------===//
//
// 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 Foundation
import IndexStoreDB
import LSPLogging

/// 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.
///
/// 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 last modification time of a file. Can also represent the fact that the file does not exist.
private enum ModificationTime {
case fileDoesNotExist
case date(Date)
}

private enum Error: Swift.Error, CustomStringConvertible {
case fileAttributesDontHaveModificationDate

var description: String {
switch self {
case .fileAttributesDontHaveModificationDate:
return "File attributes don't contain a modification date"
}
}
}

/// File paths to modification times that have already been computed.
private var modTimeCache: [String: ModificationTime] = [:]

private func modificationDateUncached(of path: String) throws -> ModificationTime {
do {
let attributes = try FileManager.default.attributesOfItem(atPath: path)
guard let modificationDate = attributes[FileAttributeKey.modificationDate] as? Date else {
throw Error.fileAttributesDontHaveModificationDate
}
return .date(modificationDate)
} catch let error as NSError where error.domain == NSCocoaErrorDomain && error.code == NSFileReadNoSuchFileError {
return .fileDoesNotExist
}
}

private mutating func modificationDate(of path: String) throws -> ModificationTime {
if let cached = modTimeCache[path] {
return cached
}
let modTime = try modificationDateUncached(of: path)
modTimeCache[path] = modTime
return modTime
}

/// 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 {
do {
let sourceFileModificationDate = try modificationDate(of: symbolLocation.path)
switch sourceFileModificationDate {
case .fileDoesNotExist:
return false
case .date(let sourceFileModificationDate):
return sourceFileModificationDate <= symbolLocation.timestamp
}
} catch {
logger.fault("Unable to determine if SymbolLocation is up-to-date: \(error.forLogging)")
return true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only error at the moment is no mtime in attributes. Seems like we should just assume it isn't up to date in that case? I doubt it really matters though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thinking was that if sourcekit-lsp is running on some platform that for some reason doesn’t have modification time or if Foundation doesn’t know how to extract it (not sure if those exist), it’s better to assume all files are up-to-date than to completely ignore index results. But as you said, I don’t think it really makes a difference.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough, that's fine with me 👍

}
}

/// 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 {
return false
}
do {
let sourceModificationDate = try modificationDate(of: filePath)
switch sourceModificationDate {
case .fileDoesNotExist:
return false
case .date(let sourceModificationDate):
return sourceModificationDate <= lastUnitDate
}
} catch {
logger.fault("Unable to determine if source file has up-to-date unit: \(error.forLogging)")
return true
}
}
}
5 changes: 5 additions & 0 deletions Sources/SourceKitLSP/LanguageService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,11 @@ public protocol LanguageService: AnyObject {

func executeCommand(_ req: ExecuteCommandRequest) async throws -> LSPAny?

/// 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]?

/// Crash the language server. Should be used for crash recovery testing only.
func _crash() async
}
143 changes: 137 additions & 6 deletions Sources/SourceKitLSP/TestDiscovery.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@
//===----------------------------------------------------------------------===//

import IndexStoreDB
import LSPLogging
import LanguageServerProtocol
import SwiftSyntax

fileprivate extension SymbolOccurrence {
/// Assuming that this is a symbol occurrence returned by the index, return whether it can constitute the definition
Expand Down Expand Up @@ -53,11 +55,140 @@ extension SourceKitLSPServer {
for: req.textDocument.uri,
language: snapshot.language
)
let testSymbols = workspace.index?.unitTests(referencedByMainFiles: [mainFileUri.pseudoPath]) ?? []
return
testSymbols
.filter { $0.canBeTestDefinition }
.sorted()
.map(WorkspaceSymbolItem.init)
if let index = workspace.index {
var outOfDateChecker = IndexOutOfDateChecker()
let testSymbols =
index.unitTests(referencedByMainFiles: [mainFileUri.pseudoPath])
.filter { $0.canBeTestDefinition && outOfDateChecker.isUpToDate($0.location) }

if !testSymbols.isEmpty {
return testSymbols.sorted().map(WorkspaceSymbolItem.init)
}
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.
return []
}
}
// We don't have any up-to-date index entries for this file. Syntactically look for tests.
return try await languageService.syntacticDocumentTests(for: req.textDocument.uri)
}
}

/// Scans a source file for `XCTestCase` classes and test methods.
///
/// 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 {
/// The document snapshot of the syntax tree that is being walked.
private var snapshot: DocumentSnapshot

/// The workspace symbols representing the found `XCTestCase` subclasses and test methods.
private var result: [WorkspaceSymbolItem] = []

/// 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"]

private init(snapshot: DocumentSnapshot) {
self.snapshot = snapshot
super.init(viewMode: .fixedUp)
}

public static func findTestSymbols(
in snapshot: DocumentSnapshot,
syntaxTreeManager: SyntaxTreeManager
) async -> [WorkspaceSymbolItem] {
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
guard let function = member.decl.as(FunctionDeclSyntax.self) else {
return nil
}
guard function.name.text.starts(with: "test") else {
return nil
}
guard function.modifiers.map(\.name.tokenKind).allSatisfy({ $0 != .keyword(.static) && $0 != .keyword(.class) })
else {
// Test methods can't be static.
return nil
}
guard function.signature.returnClause == nil else {
// Test methods can't have a return type.
// 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 {
logger.fault(
"Failed to convert offset \(function.name.positionAfterSkippingLeadingTrivia.utf8Offset) to UTF-16-based position"
)
return nil
}
let symbolInformation = SymbolInformation(
name: function.name.text,
kind: .method,
location: Location(uri: snapshot.uri, range: Range(position)),
containerName: containerName
)
return WorkspaceSymbolItem.symbolInformation(symbolInformation)
}
}

override func visit(_ node: ClassDeclSyntax) -> SyntaxVisitorContinueKind {
guard let inheritedTypes = node.inheritanceClause?.inheritedTypes, let superclass = inheritedTypes.first else {
// The class has no superclass and thus can't inherit from XCTestCase.
// Continue scanning its children in case it has a nested subclass that inherits from XCTestCase.
return .visitChildren
}
if let superclassIdentifier = superclass.type.as(IdentifierTypeSyntax.self),
Self.knownNonXCTestSubclasses.contains(superclassIdentifier.name.text)
{
// We know that the class can't be an subclass of `XCTestCase` so don't visit it.
// We can't explicitly check for the `XCTestCase` superclass because the class might inherit from a class that in
// turn inherits from `XCTestCase`. Resolving that inheritance hierarchy would be semantic.
return .visitChildren
}
let testMethods = findTestMethods(in: node.memberBlock.members, containerName: node.name.text)
guard !testMethods.isEmpty else {
// Don't report a test class if it doesn't contain any test methods.
return .visitChildren
}
guard let position = snapshot.position(of: node.name.positionAfterSkippingLeadingTrivia) else {
logger.fault(
"Failed to convert offset \(node.name.positionAfterSkippingLeadingTrivia.utf8Offset) to UTF-16-based position"
)
return .visitChildren
}
let testClassSymbolInformation = SymbolInformation(
name: node.name.text,
kind: .class,
location: Location(uri: snapshot.uri, range: Range(position)),
containerName: nil
)
result.append(.symbolInformation(testClassSymbolInformation))
result += testMethods
return .visitChildren
}

override func visit(_ node: ExtensionDeclSyntax) -> SyntaxVisitorContinueKind {
result += findTestMethods(in: node.memberBlock.members, containerName: node.extendedType.trimmedDescription)
return .visitChildren
}
}

extension SwiftLanguageService {
public func syntacticDocumentTests(for uri: DocumentURI) async throws -> [WorkspaceSymbolItem]? {
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
}
}
Loading