diff --git a/Sources/BuildServerIntegration/SwiftPMBuildServer.swift b/Sources/BuildServerIntegration/SwiftPMBuildServer.swift index 4f9737a23..58a65c15b 100644 --- a/Sources/BuildServerIntegration/SwiftPMBuildServer.swift +++ b/Sources/BuildServerIntegration/SwiftPMBuildServer.swift @@ -137,8 +137,12 @@ package actor SwiftPMBuildServer: BuiltInBuildServer { private var targetDependencies: [BuildTargetIdentifier: Set] = [:] - static package func searchForConfig(in path: URL, options: SourceKitLSPOptions) -> BuildServerSpec? { + /// Regular expression that matches version-specific package manifest file names such as Package@swift-6.1.swift + private static var versionSpecificPackageManifestNameRegex: Regex<(Substring, Substring, Substring?, Substring?)> { + #/^Package@swift-(\d+)(?:\.(\d+))?(?:\.(\d+))?.swift$/# + } + static package func searchForConfig(in path: URL, options: SourceKitLSPOptions) -> BuildServerSpec? { let packagePath = path.appendingPathComponent("Package.swift") if (try? String(contentsOf: packagePath, encoding: .utf8))?.contains("PackageDescription") ?? false { return BuildServerSpec(kind: .swiftPM, projectRoot: path, configPath: packagePath) @@ -543,12 +547,11 @@ package actor SwiftPMBuildServer: BuiltInBuildServer { // (https://github.com/swiftlang/sourcekit-lsp/issues/1267) for target in request.targets { if target == .forPackageManifest { - let packageManifestName = #/^Package@swift-(\d+)(?:\.(\d+))?(?:\.(\d+))?.swift$/# let versionSpecificManifests = try? FileManager.default.contentsOfDirectory( at: projectRoot, includingPropertiesForKeys: nil ).compactMap { (url) -> SourceItem? in - guard (try? packageManifestName.wholeMatch(in: url.lastPathComponent)) != nil else { + guard (try? Self.versionSpecificPackageManifestNameRegex.wholeMatch(in: url.lastPathComponent)) != nil else { return nil } return SourceItem( @@ -806,11 +809,28 @@ package actor SwiftPMBuildServer: BuiltInBuildServer { } } + private func isPackageManifestOrPackageResolved(_ url: URL) -> Bool { + guard url.lastPathComponent.contains("Package") else { + // Fast check to early exit for files that don't like a package manifest or Package.resolved + return false + } + guard + url.lastPathComponent == "Package.resolved" || url.lastPathComponent == "Package.swift" + || (try? Self.versionSpecificPackageManifestNameRegex.wholeMatch(in: url.lastPathComponent)) != nil + else { + return false + } + return url.deletingLastPathComponent() == self.projectRoot + } + /// An event is relevant if it modifies a file that matches one of the file rules used by the SwiftPM workspace. private func fileEventShouldTriggerPackageReload(event: FileEvent) -> Bool { guard let fileURL = event.uri.fileURL else { return false } + if isPackageManifestOrPackageResolved(fileURL) { + return true + } switch event.type { case .created, .deleted: guard let buildDescription else { @@ -819,7 +839,8 @@ package actor SwiftPMBuildServer: BuiltInBuildServer { return buildDescription.fileAffectsSwiftOrClangBuildSettings(fileURL) case .changed: - return fileURL.lastPathComponent == "Package.swift" || fileURL.lastPathComponent == "Package.resolved" + // Only modified package manifests should trigger a package reload and that's handled above. + return false default: // Unknown file change type return false } diff --git a/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift b/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift index 9538222d1..ddabd4921 100644 --- a/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift +++ b/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift @@ -1325,7 +1325,7 @@ final class BackgroundIndexingTests: XCTestCase { // Updating Package.swift causes a package reload but should not cause dependencies to be updated. project.testClient.send( DidChangeWatchedFilesNotification(changes: [ - FileEvent(uri: DocumentURI(project.scratchDirectory.appendingPathComponent("Package.resolved")), type: .changed) + FileEvent(uri: DocumentURI(project.scratchDirectory.appendingPathComponent("Package.swift")), type: .changed) ]) ) try await project.testClient.send(SynchronizeRequest(index: true)) @@ -1360,14 +1360,19 @@ final class BackgroundIndexingTests: XCTestCase { ]) ) try await project.testClient.send(SynchronizeRequest(index: true)) + let dependencyCheckoutFile = try XCTUnwrap( + FileManager.default.findFiles( + named: "Dependency.swift", + in: project.scratchDirectory + .appendingPathComponent(".build") + .appendingPathComponent("index-build") + .appendingPathComponent("checkouts") + ).only + ) + // Check that modifying Package.resolved actually modified the dependency checkout inside the package + assertContains(try String(contentsOf: dependencyCheckoutFile, encoding: .utf8), "Do something v1.1.0") project.testClient.send( - DidChangeWatchedFilesNotification( - changes: FileManager.default.findFiles( - named: "Dependency.swift", - in: project.scratchDirectory.appendingPathComponent(".build").appendingPathComponent("index-build") - .appendingPathComponent("checkouts") - ).map { FileEvent(uri: DocumentURI($0), type: .changed) } - ) + DidChangeWatchedFilesNotification(changes: [FileEvent(uri: DocumentURI(dependencyCheckoutFile), type: .changed)]) ) try await repeatUntilExpectedResult { diff --git a/Tests/SourceKitLSPTests/SwiftPMIntegrationTests.swift b/Tests/SourceKitLSPTests/SwiftPMIntegrationTests.swift index 680d3ea70..2404829ef 100644 --- a/Tests/SourceKitLSPTests/SwiftPMIntegrationTests.swift +++ b/Tests/SourceKitLSPTests/SwiftPMIntegrationTests.swift @@ -561,4 +561,156 @@ final class SwiftPMIntegrationTests: XCTestCase { return location.range.lowerBound == Position(line: 3, utf16index: 4) } } + + func testChangePackageManifestFile() async throws { + let project = try await SwiftPMTestProject( + files: [ + "Lib.swift": """ + #if MY_FLAG + #error("MY_FLAG set") + #else + #error("MY_FLAG not set") + #endif + """ + ], + manifest: """ + // swift-tools-version: 5.7 + import PackageDescription + let package = Package( + name: "MyLibrary", + targets: [.target(name: "MyLibrary")] + ) + """ + ) + + let (uri, _) = try project.openDocument("Lib.swift") + let initialDiagnostics = try await project.testClient.send( + DocumentDiagnosticsRequest(textDocument: TextDocumentIdentifier(uri)) + ) + XCTAssertEqual(initialDiagnostics.fullReport?.items.map(\.message), ["MY_FLAG not set"]) + + try await project.changeFileOnDisk( + "Package.swift", + newMarkedContents: """ + // swift-tools-version: 5.7 + import PackageDescription + let package = Package( + name: "MyLibrary", + targets: [.target(name: "MyLibrary", swiftSettings: [.define("MY_FLAG")])] + ) + """ + ) + try await repeatUntilExpectedResult { + let diagnosticsAfterUpdate = try await project.testClient.send( + DocumentDiagnosticsRequest(textDocument: TextDocumentIdentifier(uri)) + ) + return diagnosticsAfterUpdate.fullReport?.items.map(\.message) == ["MY_FLAG set"] + } + } + + func testChangeVersionSpecificPackageManifestFile() async throws { + let project = try await SwiftPMTestProject( + files: [ + "Lib.swift": """ + #if MY_FLAG + #error("MY_FLAG set") + #elseif MY_OTHER_FLAG + #error("MY_OTHER_FLAG set") + #else + #error("no flag set") + #endif + """, + "/Package@swift-6.1.swift": """ + // swift-tools-version: 6.1 + import PackageDescription + let package = Package( + name: "MyLibrary", + targets: [.target(name: "MyLibrary", swiftSettings: [.define("MY_FLAG")])] + ) + """, + ], + manifest: """ + // swift-tools-version: 5.7 + import PackageDescription + let package = Package( + name: "MyLibrary", + targets: [.target(name: "MyLibrary")] + ) + """ + ) + + let (uri, _) = try project.openDocument("Lib.swift") + let initialDiagnostics = try await project.testClient.send( + DocumentDiagnosticsRequest(textDocument: TextDocumentIdentifier(uri)) + ) + XCTAssertEqual(initialDiagnostics.fullReport?.items.map(\.message), ["MY_FLAG set"]) + + try await project.changeFileOnDisk( + "Package@swift-6.1.swift", + newMarkedContents: """ + // swift-tools-version: 6.1 + import PackageDescription + let package = Package( + name: "MyLibrary", + targets: [.target(name: "MyLibrary", swiftSettings: [.define("MY_OTHER_FLAG")])] + ) + """ + ) + try await repeatUntilExpectedResult { + let diagnosticsAfterUpdate = try await project.testClient.send( + DocumentDiagnosticsRequest(textDocument: TextDocumentIdentifier(uri)) + ) + return diagnosticsAfterUpdate.fullReport?.items.map(\.message) == ["MY_OTHER_FLAG set"] + } + } + + func testAddVersionSpecificPackageManifestFile() async throws { + let project = try await SwiftPMTestProject( + files: [ + "Lib.swift": """ + #if MY_FLAG + #error("MY_FLAG set") + #else + #error("MY_FLAG not set") + #endif + """ + ], + manifest: """ + // swift-tools-version: 5.7 + import PackageDescription + let package = Package( + name: "MyLibrary", + targets: [.target(name: "MyLibrary")] + ) + """ + ) + + let (uri, _) = try project.openDocument("Lib.swift") + let initialDiagnostics = try await project.testClient.send( + DocumentDiagnosticsRequest(textDocument: TextDocumentIdentifier(uri)) + ) + XCTAssertEqual(initialDiagnostics.fullReport?.items.map(\.message), ["MY_FLAG not set"]) + + let versionSpecificManifestUrl = project.scratchDirectory.appending(component: "Package@swift-6.1.swift") + try await """ + // swift-tools-version: 6.1 + import PackageDescription + let package = Package( + name: "MyLibrary", + targets: [.target(name: "MyLibrary", swiftSettings: [.define("MY_FLAG")])] + ) + """.writeWithRetry(to: versionSpecificManifestUrl) + + project.testClient.send( + DidChangeWatchedFilesNotification(changes: [ + FileEvent(uri: DocumentURI(versionSpecificManifestUrl), type: .created) + ]) + ) + try await repeatUntilExpectedResult { + let diagnosticsAfterUpdate = try await project.testClient.send( + DocumentDiagnosticsRequest(textDocument: TextDocumentIdentifier(uri)) + ) + return diagnosticsAfterUpdate.fullReport?.items.map(\.message) == ["MY_FLAG set"] + } + } }