Merge pull request #2092 from ahoppen/official-synchronization

Make `workspace/synchronize` a non-experimental request

Author: ahoppen

Contributor Documentation/LSP Extensions.md

@@ -664,33 +664,6 @@ export interface SourceKitOptionsResult {
 }
 ```
 
-## `workspace/_synchronize`
-
-New request from the client to the server to wait for SourceKit-LSP to handle all ongoing requests and, optionally, wait for background activity to finish.
-
-> [!IMPORTANT]
-> This request is experimental, guarded behind the `synchronize-request` experimental feature and may be modified or removed in future versions of SourceKit-LSP without notice. Do not rely on it.
-
-- params: `SynchronizeParams`
-- result: `void`
-
-```ts
-export interface SynchronizeParams {
-  /**
-   * Wait for the build server to have an up-to-date build graph by sending a `workspace/waitForBuildSystemUpdates` to 
-   * it.
-   */
-  buildServerUpdates?: bool
-
-  /**
-   * Wait for background indexing to finish and all index unit files to be loaded into indexstore-db.
-   * 
-   * Implies `buildServerUpdates = true`.
-   */
-  index?: bool
-}
-```
-
 ## `workspace/_outputPaths`
 
 New request from the client to the server to retrieve the output paths of a target (see the `buildTarget/outputPaths` BSP request).
@@ -789,6 +762,40 @@ export interface PeekDocumentsResult {
 }
 ```
 
+## `workspace/synchronize`
+
+Request from the client to the server to wait for SourceKit-LSP to handle all ongoing requests and, optionally, wait for background activity to finish.
+
+This method is intended to be used in automated environments which need to wait for background activity to finish before executing requests that rely on that background activity to finish. Examples of such cases are:
+ - Automated tests that need to wait for background indexing to finish and then checking the result of request results
+ - Automated tests that need to wait for requests like file changes to be handled and checking behavior after those have been processed
+ - Code analysis tools that want to use SourceKit-LSP to gather information about the project but can only do so after the index has been loaded
+
+Because this request waits for all other SourceKit-LSP requests to finish, it limits parallel request handling and is ill-suited for any kind of interactive environment. In those environments, it is preferable to quickly give the user a result based on the data that is available and (let the user) re-perform the action if the underlying index data has changed.
+
+- params: `SynchronizeParams`
+- result: `void`
+
+```ts
+export interface SynchronizeParams {
+  /**
+   * Wait for the build server to have an up-to-date build graph by sending a `workspace/waitForBuildSystemUpdates` to
+   * it.
+   *
+   * This is implied by `index = true`.
+   *
+   * This option is experimental, guarded behind the `synchronize-for-build-system-updates` experimental feature, and
+   * may be modified or removed in future versions of SourceKit-LSP without notice. Do not rely on it.
+   */
+  buildServerUpdates?: bool
+
+  /**
+   * Wait for background indexing to finish and all index unit files to be loaded into indexstore-db.
+   */
+  index?: bool
+}
+```
+
 ## `workspace/tests`
 
 New request that returns symbols for all the test classes and test methods within the current workspace.

Sources/LanguageServerProtocol/Requests/SynchronizeRequest.swift

@@ -10,20 +10,35 @@
 //
 //===----------------------------------------------------------------------===//
 
-/// Wait for SourceKit-LSP to handle all ongoing requests and, optionally, wait for background activity to finish.
+/// Request from the client to the server to wait for SourceKit-LSP to handle all ongoing requests and, optionally, wait
+/// for background activity to finish.
 ///
-/// **LSP Extension, For Testing**.
+/// This method is intended to be used in automated environments which need to wait for background activity to finish
+/// before executing requests that rely on that background activity to finish. Examples of such cases are:
+/// - Automated tests that need to wait for background indexing to finish and then checking the result of request
+///   results
+/// - Automated tests that need to wait for requests like file changes to be handled and checking behavior after those
+///   have been processed
+/// - Code analysis tools that want to use SourceKit-LSP to gather information about the project but can only do so
+///   after the index has been loaded
+///
+/// Because this request waits for all other SourceKit-LSP requests to finish, it limits parallel request handling and
+/// is ill-suited for any kind of interactive environment. In those environments, it is preferable to quickly give the
+/// user a result based on the data that is available and (let the user) re-perform the action if the underlying index
+/// data has changed.
 public struct SynchronizeRequest: RequestType {
-  public static let method: String = "workspace/_synchronize"
+  public static let method: String = "workspace/synchronize"
   public typealias Response = VoidResponse
 
   /// Wait for the build server to have an up-to-date build graph by sending a `workspace/waitForBuildSystemUpdates` to
   /// it.
+  /// This is implied by `index = true`.
+  ///
+  /// This option is experimental, guarded behind the `synchronize-for-build-system-updates` experimental feature, and
+  /// may be modified or removed in future versions of SourceKit-LSP without notice. Do not rely on it.
   public var buildServerUpdates: Bool?
 
   /// Wait for background indexing to finish and all index unit files to be loaded into indexstore-db.
-  ///
-  /// Implies `buildServerUpdates = true`.
   public var index: Bool?
 
   public init(buildServerUpdates: Bool? = nil, index: Bool? = nil) {

Sources/SKOptions/ExperimentalFeatures.swift

@@ -40,8 +40,8 @@ public enum ExperimentalFeature: String, Codable, Sendable, CaseIterable {
   /// - Note: Internal option
   case outputPathsRequest = "output-paths-request"
 
-  /// Enable the `workspace/_synchronize` request.
+  /// Enable the `buildServerUpdates` option in the `workspace/synchronize` request.
   ///
   /// - Note: Internal option, for testing only
-  case synchronizeRequest = "synchronize-request"
+  case synchronizeForBuildSystemUpdates = "synchronize-for-build-system-updates"
 }

Sources/SKTestSupport/TestSourceKitLSPClient.swift

@@ -36,7 +36,7 @@ extension SourceKitLSPOptions {
         servicePlugin: try pluginPaths.servicePlugin.filePath
       ),
       backgroundIndexing: backgroundIndexing,
-      experimentalFeatures: experimentalFeatures.union([.synchronizeRequest]),
+      experimentalFeatures: experimentalFeatures,
       swiftPublishDiagnosticsDebounceDuration: 0,
       workDoneProgressDebounceDuration: 0
     )

Sources/SourceKitLSP/SourceKitLSPServer.swift

@@ -2540,8 +2540,8 @@ extension SourceKitLSPServer {
   }
 
   func synchronize(_ req: SynchronizeRequest) async throws -> VoidResponse {
-    guard self.options.hasExperimentalFeature(.synchronizeRequest) else {
-      throw ResponseError.unknown("\(SynchronizeRequest.method) indexing is an experimental request")
+    if req.buildServerUpdates != nil, !self.options.hasExperimentalFeature(.synchronizeForBuildSystemUpdates) {
+      throw ResponseError.unknown("\(SynchronizeRequest.method).buildServerUpdates is an experimental request option")
     }
     for workspace in workspaces {
       await workspace.synchronize(req)

Tests/SourceKitLSPTests/BackgroundIndexingTests.swift

@@ -1633,10 +1633,7 @@ final class BackgroundIndexingTests: XCTestCase {
           ]
         )
         """,
-      options: SourceKitLSPOptions(
-        backgroundPreparationMode: .enabled,
-        experimentalFeatures: [.synchronizeRequest]
-      ),
+      options: SourceKitLSPOptions(backgroundPreparationMode: .enabled),
       enableBackgroundIndexing: true
     )