Merge branch 'main' into 6.0/merge-main-2024-04-23

Author: ahoppen

Sources/Diagnose/DiagnoseCommand.swift

@@ -27,11 +27,11 @@ private var progressBar: PercentProgressAnimation? = nil
 
 /// A component of the diagnostic bundle that's collected in independent stages.
 fileprivate enum BundleComponent: String, CaseIterable, ExpressibleByArgument {
-  case crashReports
-  case logs
-  case swiftVersions
-  case sourcekitdCrashes
-  case swiftFrontendCrashes
+  case crashReports = "crash-reports"
+  case logs = "logs"
+  case swiftVersions = "swift-versions"
+  case sourcekitdCrashes = "sourcekitd-crashes"
+  case swiftFrontendCrashes = "swift-frontend-crashes"
 }
 
 public struct DiagnoseCommand: AsyncParsableCommand {

Sources/Diagnose/ReproducerBundle.swift

@@ -41,7 +41,7 @@ func makeReproducerBundle(for requestInfo: RequestInfo, toolchain: Toolchain, bu
   } else {
     let request = try requestInfo.request(for: URL(fileURLWithPath: "/input.swift"))
     try request.write(
-      to: bundlePath.appendingPathComponent("request.json"),
+      to: bundlePath.appendingPathComponent("request.yml"),
       atomically: true,
       encoding: .utf8
     )

Sources/SKCore/BuildServerBuildSystem.swift

@@ -316,6 +316,17 @@ extension BuildServerBuildSystem: BuildSystem {
 
     return .unhandled
   }
+
+  public func testFiles() async -> [DocumentURI] {
+    // BuildServerBuildSystem does not support syntactic test discovery
+    // (https://github.com/apple/sourcekit-lsp/issues/1173).
+    return []
+  }
+
+  public func addTestFilesDidChangeCallback(_ callback: @escaping () async -> Void) {
+    // BuildServerBuildSystem does not support syntactic test discovery
+    // (https://github.com/apple/sourcekit-lsp/issues/1173).
+  }
 }
 
 private func loadBuildServerConfig(path: AbsolutePath, fileSystem: FileSystem) throws -> BuildServerConfig {

Sources/SKCore/BuildSystem.swift

@@ -87,6 +87,18 @@ public protocol BuildSystem: AnyObject, Sendable {
   func filesDidChange(_ events: [FileEvent]) async
 
   func fileHandlingCapability(for uri: DocumentURI) async -> FileHandlingCapability
+
+  /// Returns the list of files that might contain test cases.
+  ///
+  /// The returned file list is an over-approximation. It might contain tests from non-test targets or files that don't
+  /// actually contain any tests. Keeping this list as minimal as possible helps reduce the amount of work that the
+  /// syntactic test indexer needs to perform.
+  func testFiles() async -> [DocumentURI]
+
+  /// Adds a callback that should be called when the value returned by `testFiles()` changes.
+  ///
+  /// The callback might also be called without an actual change to `testFiles`.
+  func addTestFilesDidChangeCallback(_ callback: @Sendable @escaping () async -> Void) async
 }
 
 public let buildTargetsNotSupported =

Sources/SKCore/BuildSystemManager.swift

@@ -176,6 +176,10 @@ extension BuildSystemManager {
       fallbackBuildSystem != nil ? .fallback : .unhandled
     )
   }
+
+  public func testFiles() async -> [DocumentURI] {
+    return await buildSystem?.testFiles() ?? []
+  }
 }
 
 extension BuildSystemManager: BuildSystemDelegate {

Sources/SKCore/CMakeLists.txt

@@ -7,6 +7,7 @@ add_library(SKCore STATIC
   BuildSystemManager.swift
   CompilationDatabase.swift
   CompilationDatabaseBuildSystem.swift
+  Debouncer.swift
   FallbackBuildSystem.swift
   FileBuildSettings.swift
   MainFilesProvider.swift

Sources/SKCore/CompilationDatabaseBuildSystem.swift

@@ -39,6 +39,9 @@ public actor CompilationDatabaseBuildSystem {
   /// Delegate to handle any build system events.
   public weak var delegate: BuildSystemDelegate? = nil
 
+  /// Callbacks that should be called if the list of possible test files has changed.
+  public var testFilesDidChangeCallbacks: [() async -> Void] = []
+
   public func setDelegate(_ delegate: BuildSystemDelegate?) async {
     self.delegate = delegate
   }
@@ -167,6 +170,9 @@ extension CompilationDatabaseBuildSystem: BuildSystem {
     if let delegate = self.delegate {
       await delegate.fileBuildSettingsChanged(self.watchedFiles)
     }
+    for testFilesDidChangeCallback in testFilesDidChangeCallbacks {
+      await testFilesDidChangeCallback()
+    }
   }
 
   public func filesDidChange(_ events: [FileEvent]) async {
@@ -185,4 +191,12 @@ extension CompilationDatabaseBuildSystem: BuildSystem {
       return .unhandled
     }
   }
+
+  public func testFiles() async -> [DocumentURI] {
+    return compdb?.allCommands.map { DocumentURI($0.url) } ?? []
+  }
+
+  public func addTestFilesDidChangeCallback(_ callback: @escaping () async -> Void) async {
+    testFilesDidChangeCallbacks.append(callback)
+  }
 }

Sources/SKCore/Debouncer.swift

@@ -0,0 +1,81 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2024 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
+//
+//===----------------------------------------------------------------------===//
+
+/// Debounces calls to a function/closure. If multiple calls to the closure are made, it allows aggregating the
+/// parameters.
+public actor Debouncer<Parameter> {
+  /// How long to wait for further `scheduleCall` calls before committing to actually calling `makeCall`.
+  private let debounceDuration: Duration
+
+  /// When `scheduleCall` is called while another `scheduleCall` was waiting to commit its call, combines the parameters
+  /// of those two calls.
+  ///
+  /// ### Example
+  ///
+  /// Two `scheduleCall` calls that are made within a time period shorter than `debounceDuration` like the following
+  /// ```swift
+  /// debouncer.scheduleCall(5)
+  /// debouncer.scheduleCall(10)
+  /// ```
+  /// will call `combineParameters(5, 10)`
+  private let combineParameters: (Parameter, Parameter) -> Parameter
+
+  /// After the debounce duration has elapsed, commit the call.
+  private let makeCall: (Parameter) async -> Void
+
+  /// In the time between the call to `scheduleCall` and the call actually being committed (ie. in the time that the
+  /// call can be debounced), the task that would commit the call (unless cancelled) and the parameter with which this
+  /// call should be made.
+  private var inProgressData: (Parameter, Task<Void, Never>)?
+
+  public init(
+    debounceDuration: Duration,
+    combineResults: @escaping (Parameter, Parameter) -> Parameter,
+    _ makeCall: @escaping (Parameter) async -> Void
+  ) {
+    self.debounceDuration = debounceDuration
+    self.combineParameters = combineResults
+    self.makeCall = makeCall
+  }
+
+  /// Schedule a debounced call. If `scheduleCall` is called within `debounceDuration`, the parameters of the two
+  /// `scheduleCall` calls will be combined using `combineParameters` and the new debounced call will be scheduled
+  /// `debounceDuration` after the second `scheduleCall` call.
+  public func scheduleCall(_ parameter: Parameter) {
+    var parameter = parameter
+    if let (inProgressParameter, inProgressTask) = inProgressData {
+      inProgressTask.cancel()
+      parameter = combineParameters(inProgressParameter, parameter)
+    }
+    let task = Task {
+      do {
+        try await Task.sleep(for: debounceDuration)
+        try Task.checkCancellation()
+      } catch {
+        return
+      }
+      inProgressData = nil
+      await makeCall(parameter)
+    }
+    inProgressData = (parameter, task)
+  }
+}
+
+extension Debouncer<Void> {
+  public init(debounceDuration: Duration, _ makeCall: @escaping () async -> Void) {
+    self.init(debounceDuration: debounceDuration, combineResults: { _, _ in }, makeCall)
+  }
+
+  public func scheduleCall() {
+    self.scheduleCall(())
+  }
+}

Sources/SKSupport/AsyncQueue.swift

@@ -16,6 +16,8 @@ import Foundation
 /// array.
 private protocol AnyTask: Sendable {
   func waitForCompletion() async
+
+  func cancel()
 }
 
 extension Task: AnyTask {
@@ -89,6 +91,16 @@ public final class AsyncQueue<TaskMetadata: DependencyTracker>: Sendable {
 
   public init() {}
 
+  public func cancelTasks(where filter: (TaskMetadata) -> Bool) {
+    pendingTasks.withLock { pendingTasks in
+      for task in pendingTasks {
+        if filter(task.metadata) {
+          task.task.cancel()
+        }
+      }
+    }
+  }
+
   /// Schedule a new closure to be executed on the queue.
   ///
   /// If this is a serial queue, all previously added tasks are guaranteed to

Sources/SKSwiftPMWorkspace/SwiftPMBuildSystem.swift

@@ -81,6 +81,9 @@ public actor SwiftPMBuildSystem {
     self.delegate = delegate
   }
 
+  /// Callbacks that should be called if the list of possible test files has changed.
+  public var testFilesDidChangeCallbacks: [() async -> Void] = []
+
   let workspacePath: TSCAbsolutePath
   /// The directory containing `Package.swift`.
   public var projectRoot: TSCAbsolutePath
@@ -99,6 +102,16 @@ public actor SwiftPMBuildSystem {
   /// This callback is informed when `reloadPackage` starts and ends executing.
   var reloadPackageStatusCallback: (ReloadPackageStatus) async -> Void
 
+  /// Debounces calls to `delegate.filesDependenciesUpdated`.
+  ///
+  /// This is to ensure we don't call `filesDependenciesUpdated` for the same file multiple time if the client does not
+  /// debounce `workspace/didChangeWatchedFiles` and sends a separate notification eg. for every file within a target as
+  /// it's being updated by a git checkout, which would cause other files within that target to receive a
+  /// `fileDependenciesUpdated` call once for every updated file within the target.
+  ///
+  /// Force-unwrapped optional because initializing it requires access to `self`.
+  var fileDependenciesUpdatedDebouncer: Debouncer<Set<DocumentURI>>! = nil
+
   /// Creates a build system using the Swift Package Manager, if this workspace is a package.
   ///
   /// - Parameters:
@@ -166,6 +179,19 @@ public actor SwiftPMBuildSystem {
     self.modulesGraph = try ModulesGraph(rootPackages: [], dependencies: [], binaryArtifacts: [:])
     self.reloadPackageStatusCallback = reloadPackageStatusCallback
 
+    // The debounce duration of 500ms was chosen arbitrarily without scientific research.
+    self.fileDependenciesUpdatedDebouncer = Debouncer(
+      debounceDuration: .milliseconds(500),
+      combineResults: { $0.union($1) }
+    ) {
+      [weak self] (filesWithUpdatedDependencies) in
+      guard let delegate = await self?.delegate else {
+        logger.fault("Not calling filesDependenciesUpdated because no delegate exists in SwiftPMBuildSystem")
+        return
+      }
+      await delegate.filesDependenciesUpdated(filesWithUpdatedDependencies)
+    }
+
     try await reloadPackage()
   }
 
@@ -267,6 +293,9 @@ extension SwiftPMBuildSystem {
     }
     await delegate.fileBuildSettingsChanged(self.watchedFiles)
     await delegate.fileHandlingCapabilityChanged()
+    for testFilesDidChangeCallback in testFilesDidChangeCallbacks {
+      await testFilesDidChangeCallback()
+    }
   }
 }
 
@@ -368,6 +397,34 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem {
         try await self.reloadPackage()
       }
     }
+
+    var filesWithUpdatedDependencies: Set<DocumentURI> = []
+    // If a Swift file within a target is updated, reload all the other files within the target since they might be
+    // referring to a function in the updated file.
+    for event in events {
+      guard let url = event.uri.fileURL,
+        url.pathExtension == "swift",
+        let absolutePath = try? AbsolutePath(validating: url.path),
+        let target = fileToTarget[absolutePath]
+      else {
+        continue
+      }
+      filesWithUpdatedDependencies.formUnion(target.sources.map { DocumentURI($0) })
+    }
+
+    // If a `.swiftmodule` file is updated, this means that we have performed a build / are
+    // performing a build and files that depend on this module have updated dependencies.
+    // We don't have access to the build graph from the SwiftPM API offered to SourceKit-LSP to figure out which files
+    // depend on the updated module, so assume that all files have updated dependencies.
+    // The file watching here is somewhat fragile as well because it assumes that the `.swiftmodule` files are being
+    // written to a directory within the workspace root. This is not necessarily true if the user specifies a build
+    // directory outside the source tree.
+    // All of this shouldn't be necessary once we have background preparation, in which case we know when preparation of
+    // a target has finished.
+    if events.contains(where: { $0.uri.fileURL?.pathExtension == "swiftmodule" }) {
+      filesWithUpdatedDependencies.formUnion(self.fileToTarget.keys.map { DocumentURI($0.asURL) })
+    }
+    await self.fileDependenciesUpdatedDebouncer.scheduleCall(filesWithUpdatedDependencies)
   }
 
   public func fileHandlingCapability(for uri: DocumentURI) -> FileHandlingCapability {
@@ -380,6 +437,15 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem {
       return .unhandled
     }
   }
+
+  public func testFiles() -> [DocumentURI] {
+    // We should only include source files from test targets (https://github.com/apple/sourcekit-lsp/issues/1174).
+    return fileToTarget.map { DocumentURI($0.key.asURL) }
+  }
+
+  public func addTestFilesDidChangeCallback(_ callback: @Sendable @escaping () async -> Void) async {
+    testFilesDidChangeCallbacks.append(callback)
+  }
 }
 
 extension SwiftPMBuildSystem {

Sources/SKTestSupport/FileManager+findFiles.swift

@@ -0,0 +1,27 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2024 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
+
+extension FileManager {
+  /// Returns the URLs of all files with the given file extension in the given directory (recursively).
+  public func findFiles(withExtension extensionName: String, in directory: URL) -> [URL] {
+    var result: [URL] = []
+    let enumerator = self.enumerator(at: directory, includingPropertiesForKeys: nil)
+    while let url = enumerator?.nextObject() as? URL {
+      if url.pathExtension == extensionName {
+        result.append(url)
+      }
+    }
+    return result
+  }
+}

Sources/SKTestSupport/IndexedSingleSwiftFileTestProject.swift

@@ -68,6 +68,25 @@ public struct IndexedSingleSwiftFileTestProject {
 
     if let sdk = TibsBuilder.defaultSDKPath {
       compilerArguments += ["-sdk", sdk]
+
+      // The following are needed so we can import XCTest
+      let sdkUrl = URL(fileURLWithPath: sdk)
+      let usrLibDir =
+        sdkUrl
+        .deletingLastPathComponent()
+        .deletingLastPathComponent()
+        .appendingPathComponent("usr")
+        .appendingPathComponent("lib")
+      let frameworksDir =
+        sdkUrl
+        .deletingLastPathComponent()
+        .deletingLastPathComponent()
+        .appendingPathComponent("Library")
+        .appendingPathComponent("Frameworks")
+      compilerArguments += [
+        "-I", usrLibDir.path,
+        "-F", frameworksDir.path,
+      ]
     }
 
     let compilationDatabase = JSONCompilationDatabase(

Sources/SKTestSupport/MultiFileTestProject.swift

@@ -142,4 +142,13 @@ public class MultiFileTestProject {
     }
     return DocumentPositions(markedText: fileData.markedText)[marker]
   }
+
+  public func range(from fromMarker: String, to toMarker: String, in fileName: String) throws -> Range<Position> {
+    return try position(of: fromMarker, in: fileName)..<position(of: toMarker, in: fileName)
+  }
+
+  public func location(from fromMarker: String, to toMarker: String, in fileName: String) throws -> Location {
+    let range = try self.range(from: fromMarker, to: toMarker, in: fileName)
+    return Location(uri: try self.uri(for: fileName), range: range)
+  }
 }