Skip to content

Commit a665d82

Browse files
committed
Generalize BuildSystem.testFiles to return all source files in a project
1 parent ac6bf26 commit a665d82

File tree

8 files changed

+57
-26
lines changed

8 files changed

+57
-26
lines changed

Sources/SKCore/BuildServerBuildSystem.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -317,14 +317,14 @@ extension BuildServerBuildSystem: BuildSystem {
317317
return .unhandled
318318
}
319319

320-
public func testFiles() async -> [DocumentURI] {
321-
// BuildServerBuildSystem does not support syntactic test discovery
320+
public func sourceFiles() async -> [SourceFileInfo] {
321+
// BuildServerBuildSystem does not support syntactic test discovery or background indexing.
322322
// (https://github.com/apple/sourcekit-lsp/issues/1173).
323323
return []
324324
}
325325

326-
public func addTestFilesDidChangeCallback(_ callback: @escaping () async -> Void) {
327-
// BuildServerBuildSystem does not support syntactic test discovery
326+
public func addSourceFilesDidChangeCallback(_ callback: @escaping () async -> Void) {
327+
// BuildServerBuildSystem does not support syntactic test discovery or background indexing.
328328
// (https://github.com/apple/sourcekit-lsp/issues/1173).
329329
}
330330
}

Sources/SKCore/BuildSystem.swift

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,22 @@ public enum FileHandlingCapability: Comparable, Sendable {
2727
case handled
2828
}
2929

30+
public struct SourceFileInfo: Sendable {
31+
/// The URI of the source file.
32+
public let uri: DocumentURI
33+
34+
/// Whether the file might contain test cases. This property is an over-approximation. It might be true for files
35+
/// from non-test targets or files that don't actually contain any tests. Keeping this list of files with
36+
/// `mayContainTets` minimal as possible helps reduce the amount of work that the syntactic test indexer needs to
37+
/// perform.
38+
public let mayContainTests: Bool
39+
40+
public init(uri: DocumentURI, mayContainTests: Bool) {
41+
self.uri = uri
42+
self.mayContainTests = mayContainTests
43+
}
44+
}
45+
3046
/// Provider of FileBuildSettings and other build-related information.
3147
///
3248
/// The primary role of the build system is to answer queries for
@@ -88,17 +104,13 @@ public protocol BuildSystem: AnyObject, Sendable {
88104

89105
func fileHandlingCapability(for uri: DocumentURI) async -> FileHandlingCapability
90106

91-
/// Returns the list of files that might contain test cases.
92-
///
93-
/// The returned file list is an over-approximation. It might contain tests from non-test targets or files that don't
94-
/// actually contain any tests. Keeping this list as minimal as possible helps reduce the amount of work that the
95-
/// syntactic test indexer needs to perform.
96-
func testFiles() async -> [DocumentURI]
107+
/// Returns the list of source files in the project.
108+
func sourceFiles() async -> [SourceFileInfo]
97109

98-
/// Adds a callback that should be called when the value returned by `testFiles()` changes.
110+
/// Adds a callback that should be called when the value returned by `sourceFiles()` changes.
99111
///
100-
/// The callback might also be called without an actual change to `testFiles`.
101-
func addTestFilesDidChangeCallback(_ callback: @Sendable @escaping () async -> Void) async
112+
/// The callback might also be called without an actual change to `sourceFiles`.
113+
func addSourceFilesDidChangeCallback(_ callback: @Sendable @escaping () async -> Void) async
102114
}
103115

104116
public let buildTargetsNotSupported =

Sources/SKCore/BuildSystemManager.swift

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,8 +191,17 @@ extension BuildSystemManager {
191191
)
192192
}
193193

194+
public func sourceFiles() async -> [SourceFileInfo] {
195+
return await buildSystem?.sourceFiles() ?? []
196+
}
197+
194198
public func testFiles() async -> [DocumentURI] {
195-
return await buildSystem?.testFiles() ?? []
199+
return await sourceFiles().compactMap { (info: SourceFileInfo) -> DocumentURI? in
200+
guard info.mayContainTests else {
201+
return nil
202+
}
203+
return info.uri
204+
}
196205
}
197206
}
198207

Sources/SKCore/CompilationDatabaseBuildSystem.swift

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -192,11 +192,16 @@ extension CompilationDatabaseBuildSystem: BuildSystem {
192192
}
193193
}
194194

195-
public func testFiles() async -> [DocumentURI] {
196-
return compdb?.allCommands.map { DocumentURI($0.url) } ?? []
195+
public func sourceFiles() async -> [SourceFileInfo] {
196+
guard let compdb else {
197+
return []
198+
}
199+
return compdb.allCommands.map {
200+
SourceFileInfo(uri: DocumentURI($0.url), mayContainTests: true)
201+
}
197202
}
198203

199-
public func addTestFilesDidChangeCallback(_ callback: @escaping () async -> Void) async {
204+
public func addSourceFilesDidChangeCallback(_ callback: @escaping () async -> Void) async {
200205
testFilesDidChangeCallbacks.append(callback)
201206
}
202207
}

Sources/SKSwiftPMWorkspace/SwiftPMBuildSystem.swift

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -437,17 +437,22 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem {
437437
}
438438
}
439439

440-
public func testFiles() -> [DocumentURI] {
441-
return fileToTarget.compactMap { (path, target) -> DocumentURI? in
440+
public func sourceFiles() -> [SourceFileInfo] {
441+
return fileToTarget.compactMap { (path, target) -> SourceFileInfo? in
442442
guard target.isPartOfRootPackage else {
443443
// Don't consider files from package dependencies as possible test files.
444444
return nil
445445
}
446-
return DocumentURI(path.asURL)
446+
// We should only set mayContainTests to `true` for files from test targets
447+
// (https://github.com/apple/sourcekit-lsp/issues/1174).
448+
return SourceFileInfo(
449+
uri: DocumentURI(path.asURL),
450+
mayContainTests: true
451+
)
447452
}
448453
}
449454

450-
public func addTestFilesDidChangeCallback(_ callback: @Sendable @escaping () async -> Void) async {
455+
public func addSourceFilesDidChangeCallback(_ callback: @Sendable @escaping () async -> Void) async {
451456
testFilesDidChangeCallbacks.append(callback)
452457
}
453458
}

Sources/SourceKitLSP/Workspace.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ public final class Workspace {
9797
await indexDelegate?.addMainFileChangedCallback { [weak self] in
9898
await self?.buildSystemManager.mainFilesChanged()
9999
}
100-
await underlyingBuildSystem?.addTestFilesDidChangeCallback { [weak self] in
100+
await underlyingBuildSystem?.addSourceFilesDidChangeCallback { [weak self] in
101101
guard let self else {
102102
return
103103
}

Tests/SKCoreTests/BuildSystemManagerTests.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -469,11 +469,11 @@ class ManualBuildSystem: BuildSystem {
469469
}
470470
}
471471

472-
func testFiles() async -> [DocumentURI] {
472+
func sourceFiles() async -> [SourceFileInfo] {
473473
return []
474474
}
475475

476-
func addTestFilesDidChangeCallback(_ callback: @escaping () async -> Void) {}
476+
func addSourceFilesDidChangeCallback(_ callback: @escaping () async -> Void) {}
477477
}
478478

479479
/// A `BuildSystemDelegate` setup for testing.

Tests/SourceKitLSPTests/BuildSystemTests.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,11 +61,11 @@ final class TestBuildSystem: BuildSystem {
6161
}
6262
}
6363

64-
func testFiles() async -> [DocumentURI] {
64+
func sourceFiles() async -> [SourceFileInfo] {
6565
return []
6666
}
6767

68-
func addTestFilesDidChangeCallback(_ callback: @escaping () async -> Void) async {}
68+
func addSourceFilesDidChangeCallback(_ callback: @escaping () async -> Void) async {}
6969
}
7070

7171
final class BuildSystemTests: XCTestCase {

0 commit comments

Comments
 (0)