Add test cases that launch a SourceKitLSP server using a BSP server

Author: ahoppen

.gitignore

@@ -9,3 +9,4 @@ Package.resolved
 /*.sublime-workspace
 /.swiftpm
 .*.sw?
+__pycache__

Sources/BuildSystemIntegration/BuiltInBuildSystemAdapter.swift

@@ -48,7 +48,10 @@ private func createBuildSystem(
 ) async -> BuiltInBuildSystem? {
   switch buildSystemKind {
   case .buildServer(let projectRoot):
-    return await BuildServerBuildSystem(projectRoot: projectRoot, connectionToSourceKitLSP: connectionToSourceKitLSP)
+    return await LegacyBuildServerBuildSystem(
+      projectRoot: projectRoot,
+      connectionToSourceKitLSP: connectionToSourceKitLSP
+    )
   case .compilationDatabase(let projectRoot):
     return CompilationDatabaseBuildSystem(
       projectRoot: projectRoot,

Sources/BuildSystemIntegration/CMakeLists.txt

@@ -1,6 +1,5 @@
 
 add_library(BuildSystemIntegration STATIC
-  BuildServerBuildSystem.swift
   BuildSettingsLogger.swift
   BuildSystemManager.swift
   BuildSystemManagerDelegate.swift
@@ -14,6 +13,7 @@ add_library(BuildSystemIntegration STATIC
   FallbackBuildSettings.swift
   FileBuildSettings.swift
   Language+InferredFromFileExtension.swift
+  LegacyBuildServerBuildSystem.swift
   MainFilesProvider.swift
   PathPrefixMapping.swift
   PrefixMessageWithTaskEmoji.swift

Sources/BuildSystemIntegration/BuildServerBuildSystem.swift renamed to Sources/BuildSystemIntegration/LegacyBuildServerBuildSystem.swift

@@ -41,11 +41,15 @@ func executable(_ name: String) -> String {
   #endif
 }
 
-/// A `BuildSystem` based on communicating with a build server
+#if compiler(>=6.3)
+#warning("We have had a one year transition period to the pull based build server. Consider removing this build server")
+#endif
+
+/// A `BuildSystem` based on communicating with a build server using the old push-based settings model.
 ///
-/// Provides build settings from a build server launched based on a
-/// `buildServer.json` configuration file provided in the repo root.
-package actor BuildServerBuildSystem: MessageHandler {
+/// This build server should be phased out in favor of the pull-based settings model described in
+/// https://forums.swift.org/t/extending-functionality-of-build-server-protocol-with-sourcekit-lsp/74400
+package actor LegacyBuildServerBuildSystem: MessageHandler {
   package let projectRoot: AbsolutePath
   let serverConfig: BuildServerConfig
 
@@ -252,7 +256,7 @@ private func readResponseDataKey(data: LSPAny?, key: String) -> String? {
   return nil
 }
 
-extension BuildServerBuildSystem: BuiltInBuildSystem {
+extension LegacyBuildServerBuildSystem: BuiltInBuildSystem {
   static package func projectRoot(for workspaceFolder: AbsolutePath, options: SourceKitLSPOptions) -> AbsolutePath? {
     guard localFileSystem.isFile(workspaceFolder.appending(component: "buildServer.json")) else {
       return nil
@@ -267,7 +271,7 @@ extension BuildServerBuildSystem: BuiltInBuildSystem {
     return WorkspaceBuildTargetsResponse(targets: [
       BuildTarget(
         id: .dummy,
-        displayName: "Compilation database",
+        displayName: "BuildServer",
         baseDirectory: nil,
         tags: [.test],
         capabilities: BuildTargetCapabilities(),
@@ -279,10 +283,18 @@ extension BuildServerBuildSystem: BuiltInBuildSystem {
   }
 
   package func buildTargetSources(request: BuildTargetSourcesRequest) async throws -> BuildTargetSourcesResponse {
+    guard request.targets.contains(.dummy) else {
+      return BuildTargetSourcesResponse(items: [])
+    }
     // BuildServerBuildSystem does not support syntactic test discovery or background indexing.
     // (https://github.com/swiftlang/sourcekit-lsp/issues/1173).
     // TODO: (BSP migration) Forward this request to the BSP server
-    return BuildTargetSourcesResponse(items: [])
+    return BuildTargetSourcesResponse(items: [
+      SourcesItem(
+        target: .dummy,
+        sources: [SourceItem(uri: DocumentURI(self.projectRoot.asURL), kind: .directory, generated: false)]
+      )
+    ])
   }
 
   package func didChangeWatchedFiles(notification: OnWatchedFilesDidChangeNotification) {}
@@ -304,6 +316,7 @@ extension BuildServerBuildSystem: BuiltInBuildSystem {
     // which renders this code path dead.
     let uri = request.textDocument.uri
     if !urisRegisteredForChanges.contains(uri) {
+      urisRegisteredForChanges.insert(uri)
       let request = RegisterForChanges(uri: uri, action: .register)
       _ = self.buildServer?.send(request) { result in
         if let error = result.failure {

Sources/SKTestSupport/BuildServerTestProject.swift

@@ -0,0 +1,69 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+import ISDBTestSupport
+import XCTest
+
+/// The path to the INPUTS directory of shared test projects.
+private let skTestSupportInputsDirectory: URL = {
+  #if os(macOS)
+  var resources =
+    productsDirectory
+    .appendingPathComponent("SourceKitLSP_SKTestSupport.bundle")
+    .appendingPathComponent("Contents")
+    .appendingPathComponent("Resources")
+  if !FileManager.default.fileExists(atPath: resources.path) {
+    // Xcode and command-line swiftpm differ about the path.
+    resources.deleteLastPathComponent()
+    resources.deleteLastPathComponent()
+  }
+  #else
+  let resources = XCTestCase.productsDirectory
+    .appendingPathComponent("SourceKitLSP_SKTestSupport.resources")
+  #endif
+  guard FileManager.default.fileExists(atPath: resources.path) else {
+    fatalError("missing resources \(resources.path)")
+  }
+  return resources.appendingPathComponent("INPUTS", isDirectory: true).standardizedFileURL
+}()
+
+/// Creates a project that uses a BSP server to provide build settings.
+///
+/// The build server is implemented in Python on top of the code in `AbstractBuildServer.py`.
+package class BuildServerTestProject: MultiFileTestProject {
+  package init(files: [RelativeFileLocation: String], buildServer: String) async throws {
+    var files = files
+    files["buildServer.json"] = """
+      {
+        "name": "client name",
+        "version": "10",
+        "bspVersion": "2.0",
+        "languages": ["a", "b"],
+        "argv": ["server.py"]
+      }
+      """
+    files["server.py"] = """
+      import sys
+      from typing import Dict, List, Optional
+
+      sys.path.append("\(skTestSupportInputsDirectory.path)")
+
+      from AbstractBuildServer import AbstractBuildServer
+
+      \(buildServer)
+
+      BuildServer().run()
+      """
+    try await super.init(files: files)
+  }
+}

Sources/SKTestSupport/INPUTS/AbstractBuildServer.py

@@ -1,6 +1,6 @@
 import json
 import sys
-from typing import Optional
+from typing import Dict, List, Optional
 
 
 class RequestError(Exception):
@@ -39,14 +39,14 @@ def run(self):
             try:
                 result = self.handle_message(message)
                 if result:
-                    response_message: dict[str, object] = {
+                    response_message: Dict[str, object] = {
                         "jsonrpc": "2.0",
                         "id": message["id"],
                         "result": result,
                     }
                     self.send_raw_message(response_message)
             except RequestError as e:
-                error_response_message: dict[str, object] = {
+                error_response_message: Dict[str, object] = {
                     "jsonrpc": "2.0",
                     "id": message["id"],
                     "error": {
@@ -56,12 +56,12 @@ def run(self):
                 }
                 self.send_raw_message(error_response_message)
 
-    def handle_message(self, message: dict[str, object]) -> Optional[dict[str, object]]:
+    def handle_message(self, message: Dict[str, object]) -> Optional[Dict[str, object]]:
         """
         Dispatch handling of the given method, received from SourceKit-LSP to the message handling function.
         """
         method: str = str(message["method"])
-        params: dict[str, object] = message["params"]  # type: ignore
+        params: Dict[str, object] = message["params"]  # type: ignore
         if method == "build/exit":
             return self.exit(params)
         elif method == "build/initialize":
@@ -77,7 +77,7 @@ def handle_message(self, message: dict[str, object]) -> Optional[dict[str, objec
         if "id" in message:
             raise RequestError(code=-32601, message=f"Method not found: {method}")
 
-    def send_raw_message(self, message: dict[str, object]):
+    def send_raw_message(self, message: Dict[str, object]):
         """
         Send a raw message to SourceKit-LSP. The message needs to have all JSON-RPC wrapper fields.
 
@@ -89,24 +89,37 @@ def send_raw_message(self, message: dict[str, object]):
         )
         sys.stdout.flush()
 
-    def send_notification(self, method: str, params: dict[str, object]):
+    def send_notification(self, method: str, params: Dict[str, object]):
         """
         Send a notification with the given method and parameters to SourceKit-LSP.
         """
-        message: dict[str, object] = {
+        message: Dict[str, object] = {
             "jsonrpc": "2.0",
             "method": method,
             "params": params,
         }
         self.send_raw_message(message)
 
+    def send_sourcekit_options_changed(self, uri: str, options: List[str]):
+        """
+        Send a `build/sourceKitOptionsChanged` notification to SourceKit-LSP, informing it about new build settings
+        using the old push-based settings model.
+        """
+        self.send_notification(
+            "build/sourceKitOptionsChanged",
+            {
+                "uri": uri,
+                "updatedOptions": {"options": options},
+            },
+        )
+
     # Message handling functions.
     # Subclasses should override these to provide functionality.
 
-    def exit(self, notification: dict[str, object]) -> None:
+    def exit(self, notification: Dict[str, object]) -> None:
         pass
 
-    def initialize(self, request: dict[str, object]) -> dict[str, object]:
+    def initialize(self, request: Dict[str, object]) -> Dict[str, object]:
         return {
             "displayName": "test server",
             "version": "0.1",
@@ -119,11 +132,11 @@ def initialize(self, request: dict[str, object]) -> dict[str, object]:
             },
         }
 
-    def initialized(self, notification: dict[str, object]) -> None:
+    def initialized(self, notification: Dict[str, object]) -> None:
         pass
 
-    def register_for_changes(self, notification: dict[str, object]):
+    def register_for_changes(self, notification: Dict[str, object]):
         pass
 
-    def shutdown(self, notification: dict[str, object]) -> None:
+    def shutdown(self, notification: Dict[str, object]) -> None:
         pass

Sources/SKTestSupport/INPUTS/BuildServerBuildSystemTests.testBuildTargetsChanged/server.py

@@ -1,13 +1,14 @@
 from pathlib import Path
 import sys
+from typing import Dict
 
 sys.path.append(str(Path(__file__).parent.parent))
 
 from AbstractBuildServer import AbstractBuildServer
 
 
 class BuildServer(AbstractBuildServer):
-    def register_for_changes(self, notification: dict[str, object]):
+    def register_for_changes(self, notification: Dict[str, object]):
         if notification["action"] == "register":
             self.send_notification(
                 "buildTarget/didChange",

Sources/SKTestSupport/INPUTS/BuildServerBuildSystemTests.testFileRegistration/server.py

@@ -1,13 +1,14 @@
 from pathlib import Path
 import sys
+from typing import Dict
 
 sys.path.append(str(Path(__file__).parent.parent))
 
 from AbstractBuildServer import AbstractBuildServer
 
 
 class BuildServer(AbstractBuildServer):
-    def register_for_changes(self, notification: dict[str, object]):
+    def register_for_changes(self, notification: Dict[str, object]):
         if notification["action"] == "register":
             self.send_notification(
                 "build/sourceKitOptionsChanged",

Sources/SourceKitLSP/DetermineBuildSystem.swift

@@ -22,7 +22,7 @@ import struct TSCBasic.RelativePath
 fileprivate extension WorkspaceType {
   var buildSystemType: BuiltInBuildSystem.Type {
     switch self {
-    case .buildServer: return BuildServerBuildSystem.self
+    case .buildServer: return LegacyBuildServerBuildSystem.self
     case .compilationDatabase: return CompilationDatabaseBuildSystem.self
     case .swiftPM: return SwiftPMBuildSystem.self
     }