Make SKTestSupport build in Swift 6 mode

ahoppen · ahoppen · commit 2ddb923e467f · 2024-05-14T06:36:05.000-07:00
diff --git a/Sources/SKSupport/ThreadSafeBox.swift b/Sources/SKSupport/ThreadSafeBox.swift
@@ -47,6 +47,12 @@ public class ThreadSafeBox<T>: @unchecked Sendable {
     _value = initialValue
   }
 
+  public func withLock<Result>(_ body: (inout T) -> Result) -> Result {
+    return lock.withLock {
+      return body(&_value)
+    }
+  }
+
   /// If the value in the box is an optional, return it and reset it to `nil`
   /// in an atomic operation.
   public func takeValue<U>() -> T where U? == T {
diff --git a/Sources/SKTestSupport/SkipUnless.swift b/Sources/SKTestSupport/SkipUnless.swift
@@ -27,14 +27,16 @@ import enum TSCBasic.ProcessEnv
 // MARK: - Skip checks
 
 /// Namespace for functions that are used to skip unsupported tests.
-public enum SkipUnless {
+public actor SkipUnless {
   private enum FeatureCheckResult {
     case featureSupported
     case featureUnsupported(skipMessage: String)
   }
 
+  private static let shared = SkipUnless()
+
   /// For any feature that has already been evaluated, the result of whether or not it should be skipped.
-  private static var checkCache: [String: FeatureCheckResult] = [:]
+  private var checkCache: [String: FeatureCheckResult] = [:]
 
   /// Throw an `XCTSkip` if any of the following conditions hold
   ///  - The Swift version of the toolchain used for testing (`ToolchainRegistry.forTesting.default`) is older than
@@ -49,7 +51,7 @@ public enum SkipUnless {
   ///
   /// Independently of these checks, the tests are never skipped in Swift CI (identified by the presence of the `SWIFTCI_USE_LOCAL_DEPS` environment). Swift CI is assumed to always build its own toolchain, which is thus
   /// guaranteed to be up-to-date.
-  private static func skipUnlessSupportedByToolchain(
+  private func skipUnlessSupportedByToolchain(
     swiftVersion: SwiftVersion,
     featureName: String = #function,
     file: StaticString,
@@ -96,10 +98,10 @@ public enum SkipUnless {
   }
 
   public static func sourcekitdHasSemanticTokensRequest(
-    file: StaticString = #file,
+    file: StaticString = #filePath,
     line: UInt = #line
   ) async throws {
-    try await skipUnlessSupportedByToolchain(swiftVersion: SwiftVersion(5, 11), file: file, line: line) {
+    try await shared.skipUnlessSupportedByToolchain(swiftVersion: SwiftVersion(5, 11), file: file, line: line) {
       let testClient = try await TestSourceKitLSPClient()
       let uri = DocumentURI.for(.swift)
       testClient.openDocument("0.bitPattern", uri: uri)
@@ -127,10 +129,10 @@ public enum SkipUnless {
   }
 
   public static func sourcekitdSupportsRename(
-    file: StaticString = #file,
+    file: StaticString = #filePath,
     line: UInt = #line
   ) async throws {
-    try await skipUnlessSupportedByToolchain(swiftVersion: SwiftVersion(5, 11), file: file, line: line) {
+    try await shared.skipUnlessSupportedByToolchain(swiftVersion: SwiftVersion(5, 11), file: file, line: line) {
       let testClient = try await TestSourceKitLSPClient()
       let uri = DocumentURI.for(.swift)
       let positions = testClient.openDocument("func 1️⃣test() {}", uri: uri)
@@ -147,10 +149,10 @@ public enum SkipUnless {
 
   /// Whether clangd has support for the `workspace/indexedRename` request.
   public static func clangdSupportsIndexBasedRename(
-    file: StaticString = #file,
+    file: StaticString = #filePath,
     line: UInt = #line
   ) async throws {
-    try await skipUnlessSupportedByToolchain(swiftVersion: SwiftVersion(5, 11), file: file, line: line) {
+    try await shared.skipUnlessSupportedByToolchain(swiftVersion: SwiftVersion(5, 11), file: file, line: line) {
       let testClient = try await TestSourceKitLSPClient()
       let uri = DocumentURI.for(.c)
       let positions = testClient.openDocument("void 1️⃣test() {}", uri: uri)
@@ -177,10 +179,10 @@ public enum SkipUnless {
   /// toolchain’s SwiftPM stores the Swift modules on the top level but we synthesize compiler arguments expecting the
   /// modules to be in a `Modules` subdirectory.
   public static func swiftpmStoresModulesInSubdirectory(
-    file: StaticString = #file,
+    file: StaticString = #filePath,
     line: UInt = #line
   ) async throws {
-    try await skipUnlessSupportedByToolchain(swiftVersion: SwiftVersion(5, 11), file: file, line: line) {
+    try await shared.skipUnlessSupportedByToolchain(swiftVersion: SwiftVersion(5, 11), file: file, line: line) {
       let workspace = try await SwiftPMTestProject(
         files: ["test.swift": ""],
         build: true
@@ -195,21 +197,21 @@ public enum SkipUnless {
   }
 
   public static func toolchainContainsSwiftFormat(
-    file: StaticString = #file,
+    file: StaticString = #filePath,
     line: UInt = #line
   ) async throws {
-    try await skipUnlessSupportedByToolchain(swiftVersion: SwiftVersion(5, 11), file: file, line: line) {
+    try await shared.skipUnlessSupportedByToolchain(swiftVersion: SwiftVersion(5, 11), file: file, line: line) {
       return await ToolchainRegistry.forTesting.default?.swiftFormat != nil
     }
   }
 
   public static func sourcekitdReturnsRawDocumentationResponse(
-    file: StaticString = #file,
+    file: StaticString = #filePath,
     line: UInt = #line
   ) async throws {
     struct ExpectedMarkdownContentsError: Error {}
 
-    return try await skipUnlessSupportedByToolchain(swiftVersion: SwiftVersion(6, 0), file: file, line: line) {
+    return try await shared.skipUnlessSupportedByToolchain(swiftVersion: SwiftVersion(6, 0), file: file, line: line) {
       // The XML-based doc comment conversion did not preserve `Precondition`.
       let testClient = try await TestSourceKitLSPClient()
       let uri = DocumentURI.for(.swift)
@@ -235,10 +237,10 @@ public enum SkipUnless {
   /// Checks whether the index contains a fix that prevents it from adding relations to non-indexed locals
   /// (https://github.com/apple/swift/pull/72930).
   public static func indexOnlyHasContainedByRelationsToIndexedDecls(
-    file: StaticString = #file,
+    file: StaticString = #filePath,
     line: UInt = #line
   ) async throws {
-    return try await skipUnlessSupportedByToolchain(swiftVersion: SwiftVersion(6, 0), file: file, line: line) {
+    return try await shared.skipUnlessSupportedByToolchain(swiftVersion: SwiftVersion(6, 0), file: file, line: line) {
       let project = try await IndexedSingleSwiftFileTestProject(
         """
         func foo() {}
diff --git a/Sources/SKTestSupport/TestSourceKitLSPClient.swift b/Sources/SKTestSupport/TestSourceKitLSPClient.swift
@@ -10,6 +10,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+import CAtomics
 import Foundation
 import LSPTestSupport
 import LanguageServerProtocol
@@ -35,7 +36,8 @@ public final class TestSourceKitLSPClient: MessageHandler {
   public typealias RequestHandler<Request: RequestType> = (Request) -> Request.Response
 
   /// The ID that should be assigned to the next request sent to the `server`.
-  private var nextRequestID: Int = 0
+  /// `nonisolated(unsafe)` is fine because `nextRequestID` is atomic.
+  private nonisolated(unsafe) var nextRequestID = AtomicUInt32(initialValue: 0)
 
   /// If the server is not using the global module cache, the path of the local
   /// module cache.
@@ -66,12 +68,12 @@ public final class TestSourceKitLSPClient: MessageHandler {
   ///
   /// Conceptually, this is an array of `RequestHandler<any RequestType>` but
   /// since we can't express this in the Swift type system, we use `[Any]`.
-  private var requestHandlers: [Any] = []
+  private nonisolated(unsafe) var requestHandlers = ThreadSafeBox<[Any]>(initialValue: [])
 
   /// A closure that is called when the `TestSourceKitLSPClient` is destructed.
   ///
   /// This allows e.g. a `IndexedSingleSwiftFileTestProject` to delete its temporary files when they are no longer needed.
-  private let cleanUp: () -> Void
+  private let cleanUp: @Sendable () -> Void
 
   /// - Parameters:
   ///   - serverOptions: The equivalent of the command line options with which sourcekit-lsp should be started
@@ -94,7 +96,7 @@ public final class TestSourceKitLSPClient: MessageHandler {
     capabilities: ClientCapabilities = ClientCapabilities(),
     usePullDiagnostics: Bool = true,
     workspaceFolders: [WorkspaceFolder]? = nil,
-    cleanUp: @escaping () -> Void = {}
+    cleanUp: @Sendable @escaping () -> Void = {}
   ) async throws {
     if !useGlobalModuleCache {
       moduleCache = URL(fileURLWithPath: NSTemporaryDirectory()).appendingPathComponent(UUID().uuidString)
@@ -165,8 +167,7 @@ public final class TestSourceKitLSPClient: MessageHandler {
     // It's really unfortunate that there are no async deinits. If we had async
     // deinits, we could await the sending of a ShutdownRequest.
     let sema = DispatchSemaphore(value: 0)
-    nextRequestID += 1
-    server.handle(ShutdownRequest(), id: .number(nextRequestID)) { result in
+    server.handle(ShutdownRequest(), id: .number(Int(nextRequestID.fetchAndIncrement()))) { result in
       sema.signal()
     }
     sema.wait()
@@ -182,9 +183,8 @@ public final class TestSourceKitLSPClient: MessageHandler {
 
   /// Send the request to `server` and return the request result.
   public func send<R: RequestType>(_ request: R) async throws -> R.Response {
-    nextRequestID += 1
     return try await withCheckedThrowingContinuation { continuation in
-      server.handle(request, id: .number(self.nextRequestID)) { result in
+      server.handle(request, id: .number(Int(nextRequestID.fetchAndIncrement()))) { result in
         continuation.resume(with: result)
       }
     }
@@ -269,7 +269,7 @@ public final class TestSourceKitLSPClient: MessageHandler {
   /// If the next request that is sent to the client is of a different kind than
   /// the given handler, `TestSourceKitLSPClient` will emit an `XCTFail`.
   public func handleNextRequest<R: RequestType>(_ requestHandler: @escaping RequestHandler<R>) {
-    requestHandlers.append(requestHandler)
+    requestHandlers.value.append(requestHandler)
   }
 
   // MARK: - Conformance to MessageHandler
@@ -286,18 +286,20 @@ public final class TestSourceKitLSPClient: MessageHandler {
     id: LanguageServerProtocol.RequestID,
     reply: @escaping (LSPResult<Request.Response>) -> Void
   ) {
-    guard let requestHandler = requestHandlers.first else {
-      reply(.failure(.methodNotFound(Request.method)))
-      return
-    }
-    guard let requestHandler = requestHandler as? RequestHandler<Request> else {
-      print("\(RequestHandler<Request>.self)")
-      XCTFail("Received request of unexpected type \(Request.method)")
-      reply(.failure(.methodNotFound(Request.method)))
-      return
+    requestHandlers.withLock { requestHandlers in
+      guard let requestHandler = requestHandlers.first else {
+        reply(.failure(.methodNotFound(Request.method)))
+        return
+      }
+      guard let requestHandler = requestHandler as? RequestHandler<Request> else {
+        print("\(RequestHandler<Request>.self)")
+        XCTFail("Received request of unexpected type \(Request.method)")
+        reply(.failure(.methodNotFound(Request.method)))
+        return
+      }
+      reply(.success(requestHandler(params)))
+      requestHandlers.removeFirst()
     }
-    reply(.success(requestHandler(params)))
-    requestHandlers.removeFirst()
   }
 
   // MARK: - Convenience functions
@@ -391,8 +393,10 @@ public struct DocumentPositions {
 ///
 /// This allows us to set the ``TestSourceKitLSPClient`` as the message handler of
 /// `SourceKitLSPServer` without retaining it.
-private class WeakMessageHandler: MessageHandler {
-  private weak var handler: (any MessageHandler)?
+private final class WeakMessageHandler: MessageHandler, Sendable {
+  // `nonisolated(unsafe)` is fine because `handler` is never modified, only if the weak reference is deallocated, which
+  // is atomic.
+  private nonisolated(unsafe) weak var handler: (any MessageHandler)?
 
   init(_ handler: any MessageHandler) {
     self.handler = handler
@@ -405,7 +409,7 @@ private class WeakMessageHandler: MessageHandler {
   func handle<Request: RequestType>(
     _ params: Request,
     id: LanguageServerProtocol.RequestID,
-    reply: @escaping (LanguageServerProtocol.LSPResult<Request.Response>) -> Void
+    reply: @Sendable @escaping (LanguageServerProtocol.LSPResult<Request.Response>) -> Void
   ) {
     guard let handler = handler else {
       reply(.failure(.unknown("Handler has been deallocated")))
