Skip to content

[6.0] Add test case that we don't reload the package if a .swift file get added in a folder that doesn't affect compilation #1512

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.

Sign up for GitHub

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 1 commit into from
Jun 26, 2024
Merged

[6.0] Add test case that we don't reload the package if a .swift file get added in a folder that doesn't affect compilation #1512

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
36 changes: 28 additions & 8 deletions Sources/SKSwiftPMWorkspace/SwiftPMBuildSystem.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,19 @@ fileprivate extension ConfiguredTarget {

fileprivate let preparationTaskID: AtomicUInt32 = AtomicUInt32(initialValue: 0)

public struct SwiftPMTestHooks: Sendable {
public var reloadPackageDidStart: (@Sendable () async -> Void)?
public var reloadPackageDidFinish: (@Sendable () async -> Void)?

public init(
reloadPackageDidStart: (@Sendable () async -> Void)? = nil,
reloadPackageDidFinish: (@Sendable () async -> Void)? = nil
) {
self.reloadPackageDidStart = reloadPackageDidStart
self.reloadPackageDidFinish = reloadPackageDidFinish
}
}

/// Swift Package Manager build system and workspace support.
///
/// This class implements the `BuildSystem` interface to provide the build settings for a Swift
Expand Down Expand Up @@ -124,6 +137,7 @@ public actor SwiftPMBuildSystem {
@_spi(Testing)
public var projectRoot: TSCAbsolutePath

private var buildDescription: SourceKitLSPAPI.BuildDescription?
private var modulesGraph: ModulesGraph
private let workspace: Workspace
@_spi(Testing) public let toolsBuildParameters: BuildParameters
Expand Down Expand Up @@ -168,6 +182,8 @@ public actor SwiftPMBuildSystem {
/// user's build.
private var isForIndexBuild: Bool { experimentalFeatures.contains(.backgroundIndexing) }

private let testHooks: SwiftPMTestHooks

/// Creates a build system using the Swift Package Manager, if this workspace is a package.
///
/// - Parameters:
Expand All @@ -182,7 +198,8 @@ public actor SwiftPMBuildSystem {
fileSystem: FileSystem = localFileSystem,
buildSetup: BuildSetup,
experimentalFeatures: Set<ExperimentalFeature>,
reloadPackageStatusCallback: @escaping (ReloadPackageStatus) async -> Void = { _ in }
reloadPackageStatusCallback: @escaping (ReloadPackageStatus) async -> Void = { _ in },
testHooks: SwiftPMTestHooks
) async throws {
self.workspacePath = workspacePath
self.buildSetup = buildSetup
Expand All @@ -193,6 +210,7 @@ public actor SwiftPMBuildSystem {

self.toolchain = toolchain
self.experimentalFeatures = experimentalFeatures
self.testHooks = testHooks

guard let packageRoot = findPackageDirectory(containing: workspacePath, fileSystem) else {
throw Error.noManifest(workspacePath: workspacePath)
Expand Down Expand Up @@ -287,7 +305,8 @@ public actor SwiftPMBuildSystem {
toolchainRegistry: ToolchainRegistry,
buildSetup: BuildSetup,
experimentalFeatures: Set<ExperimentalFeature>,
reloadPackageStatusCallback: @escaping (ReloadPackageStatus) async -> Void
reloadPackageStatusCallback: @escaping (ReloadPackageStatus) async -> Void,
testHooks: SwiftPMTestHooks
) async {
guard let fileURL = uri.fileURL else {
return nil
Expand All @@ -299,7 +318,8 @@ public actor SwiftPMBuildSystem {
fileSystem: localFileSystem,
buildSetup: buildSetup,
experimentalFeatures: experimentalFeatures,
reloadPackageStatusCallback: reloadPackageStatusCallback
reloadPackageStatusCallback: reloadPackageStatusCallback,
testHooks: testHooks
)
} catch Error.noManifest {
return nil
Expand All @@ -315,8 +335,10 @@ extension SwiftPMBuildSystem {
/// dependencies.
public func reloadPackage(forceResolvedVersions: Bool) async throws {
await reloadPackageStatusCallback(.start)
await testHooks.reloadPackageDidStart?()
defer {
Task {
await testHooks.reloadPackageDidFinish?()
await reloadPackageStatusCallback(.end)
}
}
Expand All @@ -336,6 +358,7 @@ extension SwiftPMBuildSystem {
observabilityScope: observabilitySystem.topScope
)
let buildDescription = BuildDescription(buildPlan: plan)
self.buildDescription = buildDescription

/// Make sure to execute any throwing statements before setting any
/// properties because otherwise we might end up in an inconsistent state
Expand Down Expand Up @@ -683,14 +706,11 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem {
}
switch event.type {
case .created, .deleted:
guard let path = try? AbsolutePath(validating: fileURL.path) else {
guard let buildDescription else {
return false
}

return self.workspace.fileAffectsSwiftOrClangBuildSettings(
filePath: path,
packageGraph: self.modulesGraph
)
return buildDescription.fileAffectsSwiftOrClangBuildSettings(fileURL)
case .changed:
return fileURL.lastPathComponent == "Package.swift" || fileURL.lastPathComponent == "Package.resolved"
default: // Unknown file change type
Expand Down
2 changes: 1 addition & 1 deletion Sources/SemanticIndex/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ add_library(SemanticIndex STATIC
CheckedIndex.swift
CompilerCommandLineOption.swift
IndexTaskDescription.swift
IndexTestHooks.swift
PreparationTaskDescription.swift
SemanticIndexManager.swift
TestHooks.swift
UpdateIndexStoreTaskDescription.swift
UpToDateTracker.swift
)
Expand Down
0 Sources/SemanticIndex/TestHooks.swift → Sources/SemanticIndex/IndexTestHooks.swift
View file
Open in desktop
File renamed without changes.
3 changes: 2 additions & 1 deletion Sources/SourceKitLSP/CreateBuildSystem.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ func createBuildSystem(
toolchainRegistry: toolchainRegistry,
buildSetup: options.buildSetup,
experimentalFeatures: options.experimentalFeatures,
reloadPackageStatusCallback: reloadPackageStatusCallback
reloadPackageStatusCallback: reloadPackageStatusCallback,
testHooks: options.swiftpmTestHooks
)
}

Expand Down
7 changes: 6 additions & 1 deletion Sources/SourceKitLSP/SourceKitLSPServer+Options.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import Foundation
import LanguageServerProtocol
import SKCore
import SKSupport
import SKSwiftPMWorkspace
import SemanticIndex

import struct TSCBasic.AbsolutePath
Expand Down Expand Up @@ -58,6 +59,8 @@ extension SourceKitLSPServer {

public var indexTestHooks: IndexTestHooks

public var swiftpmTestHooks: SwiftPMTestHooks

public init(
buildSetup: BuildSetup = .default,
clangdOptions: [String] = [],
Expand All @@ -68,7 +71,8 @@ extension SourceKitLSPServer {
swiftPublishDiagnosticsDebounceDuration: TimeInterval = 2, /* 2s */
workDoneProgressDebounceDuration: Duration = .seconds(0),
experimentalFeatures: Set<ExperimentalFeature> = [],
indexTestHooks: IndexTestHooks = IndexTestHooks()
indexTestHooks: IndexTestHooks = IndexTestHooks(),
swiftpmTestHooks: SwiftPMTestHooks = SwiftPMTestHooks()
) {
self.buildSetup = buildSetup
self.clangdOptions = clangdOptions
Expand All @@ -80,6 +84,7 @@ extension SourceKitLSPServer {
self.experimentalFeatures = experimentalFeatures
self.workDoneProgressDebounceDuration = workDoneProgressDebounceDuration
self.indexTestHooks = indexTestHooks
self.swiftpmTestHooks = swiftpmTestHooks
}
}
}
51 changes: 34 additions & 17 deletions Tests/SKSwiftPMWorkspaceTests/SwiftPMBuildSystemTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ final class SwiftPMBuildSystemTests: XCTestCase {
toolchainRegistry: tr,
fileSystem: fs,
buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
experimentalFeatures: []
experimentalFeatures: [],
testHooks: SwiftPMTestHooks()
)
)
}
Expand All @@ -81,7 +82,8 @@ final class SwiftPMBuildSystemTests: XCTestCase {
toolchainRegistry: tr,
fileSystem: fs,
buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
experimentalFeatures: []
experimentalFeatures: [],
testHooks: SwiftPMTestHooks()
)
await assertThrowsError(try await buildSystem.generateBuildGraph(allowFileSystemWrites: false))
}
Expand Down Expand Up @@ -111,7 +113,8 @@ final class SwiftPMBuildSystemTests: XCTestCase {
toolchainRegistry: ToolchainRegistry(toolchains: []),
fileSystem: fs,
buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
experimentalFeatures: []
experimentalFeatures: [],
testHooks: SwiftPMTestHooks()
)
)
}
Expand Down Expand Up @@ -142,7 +145,8 @@ final class SwiftPMBuildSystemTests: XCTestCase {
toolchainRegistry: tr,
fileSystem: fs,
buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
experimentalFeatures: []
experimentalFeatures: [],
testHooks: SwiftPMTestHooks()
)
try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)

Expand Down Expand Up @@ -207,7 +211,8 @@ final class SwiftPMBuildSystemTests: XCTestCase {
toolchainRegistry: tr,
fileSystem: localFileSystem,
buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
experimentalFeatures: []
experimentalFeatures: [],
testHooks: SwiftPMTestHooks()
)
try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)

Expand Down Expand Up @@ -270,7 +275,8 @@ final class SwiftPMBuildSystemTests: XCTestCase {
toolchainRegistry: tr,
fileSystem: fs,
buildSetup: config,
experimentalFeatures: []
experimentalFeatures: [],
testHooks: SwiftPMTestHooks()
)
try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)

Expand Down Expand Up @@ -312,7 +318,8 @@ final class SwiftPMBuildSystemTests: XCTestCase {
toolchainRegistry: tr,
fileSystem: fs,
buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
experimentalFeatures: []
experimentalFeatures: [],
testHooks: SwiftPMTestHooks()
)
try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)

Expand Down Expand Up @@ -349,7 +356,8 @@ final class SwiftPMBuildSystemTests: XCTestCase {
toolchainRegistry: tr,
fileSystem: fs,
buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
experimentalFeatures: []
experimentalFeatures: [],
testHooks: SwiftPMTestHooks()
)
try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)

Expand Down Expand Up @@ -398,7 +406,8 @@ final class SwiftPMBuildSystemTests: XCTestCase {
toolchainRegistry: tr,
fileSystem: fs,
buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
experimentalFeatures: []
experimentalFeatures: [],
testHooks: SwiftPMTestHooks()
)
try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)

Expand Down Expand Up @@ -463,7 +472,8 @@ final class SwiftPMBuildSystemTests: XCTestCase {
toolchainRegistry: tr,
fileSystem: fs,
buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
experimentalFeatures: []
experimentalFeatures: [],
testHooks: SwiftPMTestHooks()
)
try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)

Expand Down Expand Up @@ -507,7 +517,8 @@ final class SwiftPMBuildSystemTests: XCTestCase {
toolchainRegistry: tr,
fileSystem: fs,
buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
experimentalFeatures: []
experimentalFeatures: [],
testHooks: SwiftPMTestHooks()
)
try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)

Expand Down Expand Up @@ -588,7 +599,8 @@ final class SwiftPMBuildSystemTests: XCTestCase {
toolchainRegistry: ToolchainRegistry.forTesting,
fileSystem: fs,
buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
experimentalFeatures: []
experimentalFeatures: [],
testHooks: SwiftPMTestHooks()
)
try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)

Expand Down Expand Up @@ -640,7 +652,8 @@ final class SwiftPMBuildSystemTests: XCTestCase {
toolchainRegistry: tr,
fileSystem: fs,
buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
experimentalFeatures: []
experimentalFeatures: [],
testHooks: SwiftPMTestHooks()
)
try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)

Expand Down Expand Up @@ -708,7 +721,8 @@ final class SwiftPMBuildSystemTests: XCTestCase {
toolchainRegistry: ToolchainRegistry.forTesting,
fileSystem: fs,
buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
experimentalFeatures: []
experimentalFeatures: [],
testHooks: SwiftPMTestHooks()
)
try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)

Expand Down Expand Up @@ -748,7 +762,8 @@ final class SwiftPMBuildSystemTests: XCTestCase {
toolchainRegistry: tr,
fileSystem: fs,
buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
experimentalFeatures: []
experimentalFeatures: [],
testHooks: SwiftPMTestHooks()
)
try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)

Expand Down Expand Up @@ -789,7 +804,8 @@ final class SwiftPMBuildSystemTests: XCTestCase {
toolchainRegistry: tr,
fileSystem: fs,
buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
experimentalFeatures: []
experimentalFeatures: [],
testHooks: SwiftPMTestHooks()
)

assertEqual(await swiftpmBuildSystem.projectRoot, try resolveSymlinks(tempDir.appending(component: "pkg")))
Expand Down Expand Up @@ -824,7 +840,8 @@ final class SwiftPMBuildSystemTests: XCTestCase {
toolchainRegistry: tr,
fileSystem: fs,
buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
experimentalFeatures: []
experimentalFeatures: [],
testHooks: SwiftPMTestHooks()
)
try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)

Expand Down
24 changes: 24 additions & 0 deletions Tests/SourceKitLSPTests/BackgroundIndexingTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import LSPTestSupport
import LanguageServerProtocol
@_spi(Testing) import SKCore
import SKSupport
import SKTestSupport
import SemanticIndex
import SourceKitLSP
Expand Down Expand Up @@ -1210,6 +1211,29 @@ final class BackgroundIndexingTests: XCTestCase {
return hoverAfterPackageUpdate?.contents.markupContent?.value.contains("Do something v1.1.0") ?? false
}
}

func testAddingRandomSwiftFileDoesNotTriggerPackageReload() async throws {
let packageInitialized = AtomicBool(initialValue: false)

var serverOptions = SourceKitLSPServer.Options.testDefault
serverOptions.swiftpmTestHooks.reloadPackageDidStart = {
if packageInitialized.value {
XCTFail("Build graph should not get reloaded when random file gets added")
}
}
let project = try await SwiftPMTestProject(
files: ["Test.swift": ""],
serverOptions: serverOptions,
enableBackgroundIndexing: true
)
packageInitialized.value = true
project.testClient.send(
DidChangeWatchedFilesNotification(changes: [
FileEvent(uri: DocumentURI(project.scratchDirectory.appendingPathComponent("random.swift")), type: .created)
])
)
_ = try await project.testClient.send(PollIndexRequest())
}
}

extension HoverResponseContents {
Expand Down