Merge pull request #1687 from ahoppen/bsp-server-tests

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

@@ -0,0 +1,142 @@
+import json
+import sys
+from typing import Dict, List, Optional
+
+
+class RequestError(Exception):
+    """
+    An error that can be thrown from a request handling function in `AbstractBuildServer` to return an error response to
+    SourceKit-LSP.
+    """
+
+    code: int
+    message: str
+
+    def __init__(self, code: int, message: str):
+        self.code = code
+        self.message = message
+
+
+class AbstractBuildServer:
+    """
+    An abstract class to implement a BSP server in Python for SourceKit-LSP testing purposes.
+    """
+
+    def run(self):
+        """
+        Run the build server. This should be called from the top-level code of the build server's Python file.
+        """
+        while True:
+            line = sys.stdin.readline()
+            if len(line) == 0:
+                break
+
+            assert line.startswith("Content-Length:")
+            length = int(line[len("Content-Length:") :])
+            sys.stdin.readline()
+            message = json.loads(sys.stdin.read(length))
+
+            try:
+                result = self.handle_message(message)
+                if result:
+                    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] = {
+                    "jsonrpc": "2.0",
+                    "id": message["id"],
+                    "error": {
+                        "code": e.code,
+                        "message": e.message,
+                    },
+                }
+                self.send_raw_message(error_response_message)
+
+    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
+        if method == "build/exit":
+            return self.exit(params)
+        elif method == "build/initialize":
+            return self.initialize(params)
+        elif method == "build/initialized":
+            return self.initialized(params)
+        elif method == "build/shutdown":
+            return self.shutdown(params)
+        elif method == "textDocument/registerForChanges":
+            return self.register_for_changes(params)
+
+        # ignore other notifications
+        if "id" in message:
+            raise RequestError(code=-32601, message=f"Method not found: {method}")
+
+    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.
+
+        Subclasses should not call this directly
+        """
+        message_str = json.dumps(message)
+        sys.stdout.buffer.write(
+            f"Content-Length: {len(message_str)}\r\n\r\n{message_str}".encode("utf-8")
+        )
+        sys.stdout.flush()
+
+    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] = {
+            "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:
+        pass
+
+    def initialize(self, request: Dict[str, object]) -> Dict[str, object]:
+        return {
+            "displayName": "test server",
+            "version": "0.1",
+            "bspVersion": "2.0",
+            "rootUri": "blah",
+            "capabilities": {"languageIds": ["a", "b"]},
+            "data": {
+                "indexDatabasePath": "some/index/db/path",
+                "indexStorePath": "some/index/store/path",
+            },
+        }
+
+    def initialized(self, notification: Dict[str, object]) -> None:
+        pass
+
+    def register_for_changes(self, notification: Dict[str, object]):
+        pass
+
+    def shutdown(self, notification: Dict[str, object]) -> None:
+        pass