Merge pull request #1501 from ahoppen/watch-package-resolved

Watch for changes to `Package.resolved`

Author: ahoppen

Sources/SKSwiftPMWorkspace/SwiftPMBuildSystem.swift

@@ -691,7 +691,7 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem {
         packageGraph: self.modulesGraph
       )
     case .changed:
-      return fileURL.lastPathComponent == "Package.swift"
+      return fileURL.lastPathComponent == "Package.swift" || fileURL.lastPathComponent == "Package.resolved"
     default:  // Unknown file change type
       return false
     }

Sources/SKTestSupport/RepeatUntilExpectedResult.swift

@@ -0,0 +1,38 @@
+//===----------------------------------------------------------------------===//
+//
+// 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 LSPTestSupport
+import XCTest
+
+/// Runs the body repeatedly once per second until it returns `true`, giving up after `timeout`.
+///
+/// This is useful to test some request that requires global state to be updated but will eventually converge on the
+/// correct result.
+///
+/// If `bodyHasOneSecondDelay` is true, it is assume that the body already has a one-second delay between iterations.
+public func repeatUntilExpectedResult(
+  _ body: () async throws -> Bool,
+  bodyHasOneSecondDelay: Bool = false,
+  timeout: TimeInterval = defaultTimeout,
+  file: StaticString = #filePath,
+  line: UInt = #line
+) async throws {
+  for _ in 0..<Int(timeout) {
+    if try await body() {
+      return
+    }
+    if !bodyHasOneSecondDelay {
+      try await Task.sleep(for: .seconds(1))
+    }
+  }
+  XCTFail("Failed to get expected result", file: file, line: line)
+}

Sources/SKTestSupport/SwiftPMDependencyProject.swift

@@ -86,15 +86,20 @@ public class SwiftPMDependencyProject {
     }
 
     try await runGitCommand(["init"], workingDirectory: packageDirectory)
+    try await tag(changedFiles: files.keys.map { $0.url(relativeTo: packageDirectory) }, version: "1.0.0")
+  }
+
+  public func tag(changedFiles: [URL], version: String) async throws {
     try await runGitCommand(
-      ["add"] + files.keys.map { $0.url(relativeTo: packageDirectory).path },
+      ["add"] + changedFiles.map(\.path),
       workingDirectory: packageDirectory
     )
     try await runGitCommand(
-      ["-c", "user.name=Dummy", "-c", "[email protected]", "commit", "-m", "Initial commit"],
+      ["-c", "user.name=Dummy", "-c", "[email protected]", "commit", "-m", "Version \(version)"],
       workingDirectory: packageDirectory
     )
-    try await runGitCommand(["tag", "1.0.0"], workingDirectory: packageDirectory)
+
+    try await runGitCommand(["tag", version], workingDirectory: self.packageDirectory)
   }
 
   deinit {

Sources/SourceKitLSP/SourceKitLSPServer.swift

@@ -1195,6 +1195,7 @@ extension SourceKitLSPServer {
       return FileSystemWatcher(globPattern: "**/*.\(fileExtension)", kind: [.create, .change, .delete])
     }
     watchers.append(FileSystemWatcher(globPattern: "**/Package.swift", kind: [.change]))
+    watchers.append(FileSystemWatcher(globPattern: "**/Package.resolved", kind: [.change]))
     watchers.append(FileSystemWatcher(globPattern: "**/compile_commands.json", kind: [.create, .change, .delete]))
     watchers.append(FileSystemWatcher(globPattern: "**/compile_flags.txt", kind: [.create, .change, .delete]))
     // Watch for changes to `.swiftmodule` files to detect updated modules during a build.

Tests/SourceKitLSPTests/BackgroundIndexingTests.swift

@@ -12,12 +12,14 @@
 
 import LSPTestSupport
 import LanguageServerProtocol
-import SKCore
+@_spi(Testing) import SKCore
 import SKTestSupport
 import SemanticIndex
 import SourceKitLSP
 import XCTest
 
+import class TSCBasic.Process
+
 final class BackgroundIndexingTests: XCTestCase {
   func testBackgroundIndexingOfSingleFile() async throws {
     let project = try await SwiftPMTestProject(
@@ -1107,4 +1109,126 @@ final class BackgroundIndexingTests: XCTestCase {
     )
     XCTAssertEqual(response, .locations([try project.location(from: "1️⃣", to: "1️⃣", in: "LibB.swift")]))
   }
+
+  func testUpdatePackageDependency() async throws {
+    try SkipUnless.longTestsEnabled()
+
+    let dependencyProject = try await SwiftPMDependencyProject(files: [
+      "Sources/MyDependency/Dependency.swift": """
+      /// Do something v1.0.0
+      public func doSomething() {}
+      """
+    ])
+    let dependencySwiftURL = dependencyProject.packageDirectory
+      .appendingPathComponent("Sources")
+      .appendingPathComponent("MyDependency")
+      .appendingPathComponent("Dependency.swift")
+    defer { dependencyProject.keepAlive() }
+
+    let project = try await SwiftPMTestProject(
+      files: [
+        "Test.swift": """
+        import MyDependency
+
+        func test() {
+          1️⃣doSomething()
+        }
+        """
+      ],
+      manifest: """
+        let package = Package(
+          name: "MyLibrary",
+          dependencies: [.package(url: "\(dependencyProject.packageDirectory)", from: "1.0.0")],
+          targets: [
+            .target(
+              name: "MyLibrary",
+              dependencies: [.product(name: "MyDependency", package: "MyDependency")]
+            )
+          ]
+        )
+        """,
+      enableBackgroundIndexing: true
+    )
+    let packageResolvedURL = project.scratchDirectory.appendingPathComponent("Package.resolved")
+
+    let originalPackageResolvedContents = try String(contentsOf: packageResolvedURL)
+
+    // First check our setup to see that we get the expected hover response before changing the dependency project.
+    let (uri, positions) = try project.openDocument("Test.swift")
+    let hoverBeforeUpdate = try await project.testClient.send(
+      HoverRequest(textDocument: TextDocumentIdentifier(uri), position: positions["1️⃣"])
+    )
+    XCTAssert(
+      hoverBeforeUpdate?.contents.markupContent?.value.contains("Do something v1.0.0") ?? false,
+      "Did not contain expected string: \(String(describing: hoverBeforeUpdate))"
+    )
+
+    // Just committing a new version of the dependency shouldn't change anything because we didn't update the package
+    // dependencies.
+    try """
+    /// Do something v1.1.0
+    public func doSomething() {}
+    """.write(to: dependencySwiftURL, atomically: true, encoding: .utf8)
+    try await dependencyProject.tag(changedFiles: [dependencySwiftURL], version: "1.1.0")
+
+    let hoverAfterNewVersionCommit = try await project.testClient.send(
+      HoverRequest(textDocument: TextDocumentIdentifier(uri), position: positions["1️⃣"])
+    )
+    XCTAssert(
+      hoverAfterNewVersionCommit?.contents.markupContent?.value.contains("Do something v1.0.0") ?? false,
+      "Did not contain expected string: \(String(describing: hoverBeforeUpdate))"
+    )
+
+    // 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)
+      ])
+    )
+    _ = try await project.testClient.send(PollIndexRequest())
+    XCTAssertEqual(try String(contentsOf: packageResolvedURL), originalPackageResolvedContents)
+
+    // Simulate a package update which goes as follows:
+    //  - The user runs `swift package update`
+    //  - This updates `Package.resolved`, which we watch
+    //  - We reload the package, which updates `Dependency.swift` in `.index-build/checkouts`, which we also watch.
+    try await Process.run(
+      arguments: [
+        unwrap(ToolchainRegistry.forTesting.default?.swift?.pathString),
+        "package", "update",
+        "--package-path", project.scratchDirectory.path,
+      ],
+      workingDirectory: nil
+    )
+    XCTAssertNotEqual(try String(contentsOf: packageResolvedURL), originalPackageResolvedContents)
+    project.testClient.send(
+      DidChangeWatchedFilesNotification(changes: [
+        FileEvent(uri: DocumentURI(project.scratchDirectory.appendingPathComponent("Package.resolved")), type: .changed)
+      ])
+    )
+    _ = try await project.testClient.send(PollIndexRequest())
+    project.testClient.send(
+      DidChangeWatchedFilesNotification(
+        changes: FileManager.default.findFiles(named: "Dependency.swift", in: project.scratchDirectory).map {
+          FileEvent(uri: DocumentURI($0), type: .changed)
+        }
+      )
+    )
+
+    try await repeatUntilExpectedResult {
+      let hoverAfterPackageUpdate = try await project.testClient.send(
+        HoverRequest(textDocument: TextDocumentIdentifier(uri), position: positions["1️⃣"])
+      )
+      return hoverAfterPackageUpdate?.contents.markupContent?.value.contains("Do something v1.1.0") ?? false
+    }
+  }
+}
+
+extension HoverResponseContents {
+  var markupContent: MarkupContent? {
+    switch self {
+    case .markupContent(let markupContent): return markupContent
+    default: return nil
+    }
+  }
 }

Tests/SourceKitLSPTests/BuildSystemTests.swift

@@ -187,15 +187,10 @@ final class BuildSystemTests: XCTestCase {
 
     await buildSystem.delegate?.fileBuildSettingsChanged([doc])
 
-    var receivedCorrectDiagnostic = false
-    for _ in 0..<Int(defaultTimeout) {
+    try await repeatUntilExpectedResult {
       let refreshedDiags = try await testClient.nextDiagnosticsNotification(timeout: .seconds(1))
-      if refreshedDiags.diagnostics.count == 0, try text == documentManager.latestSnapshot(doc).text {
-        receivedCorrectDiagnostic = true
-        break
-      }
+      return try text == documentManager.latestSnapshot(doc).text && refreshedDiags.diagnostics.count == 0
     }
-    XCTAssert(receivedCorrectDiagnostic)
   }
 
   func testSwiftDocumentUpdatedBuildSettings() async throws {

Tests/SourceKitLSPTests/MainFilesProviderTests.swift

@@ -201,16 +201,12 @@ final class MainFilesProviderTests: XCTestCase {
     // `clangd` may return diagnostics from the old build settings sometimes (I believe when it's still building the
     // preamble for shared.h when the new build settings come in). Check that it eventually returns the correct
     // diagnostics.
-    var receivedCorrectDiagnostic = false
-    for _ in 0..<Int(defaultTimeout) {
+    try await repeatUntilExpectedResult {
       let refreshedDiags = try await project.testClient.nextDiagnosticsNotification(timeout: .seconds(1))
-      if let diagnostic = refreshedDiags.diagnostics.only,
-        diagnostic.message == "Unused variable 'fromMyFancyLibrary'"
-      {
-        receivedCorrectDiagnostic = true
-        break
+      guard let diagnostic = refreshedDiags.diagnostics.only else {
+        return false
       }
+      return diagnostic.message == "Unused variable 'fromMyFancyLibrary'"
     }
-    XCTAssert(receivedCorrectDiagnostic)
   }
 }