Merge pull request #1294 from ahoppen/build-tests-in-swift-6-mode

Make tests build in Swift 6 mode

Author: ahoppen

.gitignore

@@ -3,6 +3,7 @@ default.profraw
 Package.resolved
 /.build
 /.index-build
+/.linux-build
 /Packages
 /*.xcodeproj
 /*.sublime-project

Sources/LSPTestSupport/TestJSONRPCConnection.swift

@@ -17,7 +17,7 @@ import XCTest
 
 import class Foundation.Pipe
 
-public final class TestJSONRPCConnection {
+public final class TestJSONRPCConnection: Sendable {
   public let clientToServer: Pipe = Pipe()
   public let serverToClient: Pipe = Pipe()
 
@@ -151,7 +151,7 @@ public actor TestClient: MessageHandler {
   /// Send a request to the LSP server and (asynchronously) receive a reply.
   public nonisolated func send<Request: RequestType>(
     _ request: Request,
-    reply: @escaping (LSPResult<Request.Response>) -> Void
+    reply: @Sendable @escaping (LSPResult<Request.Response>) -> Void
   ) -> RequestID {
     return connectionToServer.send(request, reply: reply)
   }

Sources/SKTestSupport/DefaultSDKPath.swift

@@ -0,0 +1,37 @@
+//===----------------------------------------------------------------------===//
+//
+// 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 class TSCBasic.Process
+
+#if !os(macOS)
+import Foundation
+#endif
+
+private func xcrunMacOSSDKPath() -> String? {
+  guard var path = try? Process.checkNonZeroExit(arguments: ["/usr/bin/xcrun", "--show-sdk-path", "--sdk", "macosx"])
+  else {
+    return nil
+  }
+  if path.last == "\n" {
+    path = String(path.dropLast())
+  }
+  return path
+}
+
+/// The default sdk path to use.
+public let defaultSDKPath: String? = {
+  #if os(macOS)
+  return xcrunMacOSSDKPath()
+  #else
+  return ProcessInfo.processInfo.environment["SDKROOT"]
+  #endif
+}()

Sources/SKTestSupport/IndexedSingleSwiftFileTestProject.swift

@@ -11,7 +11,6 @@
 //===----------------------------------------------------------------------===//
 
 import Foundation
-import ISDBTibs
 import LanguageServerProtocol
 @_spi(Testing) import SKCore
 import SourceKitLSP
@@ -66,7 +65,7 @@ public struct IndexedSingleSwiftFileTestProject {
       compilerArguments.append("-index-ignore-system-modules")
     }
 
-    if let sdk = TibsBuilder.defaultSDKPath {
+    if let sdk = defaultSDKPath {
       compilerArguments += ["-sdk", sdk]
 
       // The following are needed so we can import XCTest

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() {}

Sources/SKTestSupport/TestSourceKitLSPClient.swift

@@ -10,6 +10,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+import CAtomics
 import Foundation
 import LSPTestSupport
 import LanguageServerProtocol
@@ -22,7 +23,7 @@ import XCTest
 
 extension SourceKitLSPServer.Options {
   /// The default SourceKitLSPServer options for testing.
-  public static var testDefault = Self(swiftPublishDiagnosticsDebounceDuration: 0)
+  public static let testDefault = Self(swiftPublishDiagnosticsDebounceDuration: 0)
 }
 
 /// A mock SourceKit-LSP client (aka. a mock editor) that behaves like an editor
@@ -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
@@ -97,7 +99,7 @@ public final class TestSourceKitLSPClient: MessageHandler {
     usePullDiagnostics: Bool = true,
     workspaceFolders: [WorkspaceFolder]? = nil,
     preInitialization: ((TestSourceKitLSPClient) -> Void)? = nil,
-    cleanUp: @escaping () -> Void = {}
+    cleanUp: @Sendable @escaping () -> Void = {}
   ) async throws {
     if !useGlobalModuleCache {
       moduleCache = URL(fileURLWithPath: NSTemporaryDirectory()).appendingPathComponent(UUID().uuidString)
@@ -169,8 +171,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()
@@ -186,9 +187,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)
       }
     }
@@ -273,7 +273,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
@@ -290,19 +290,21 @@ public final class TestSourceKitLSPClient: MessageHandler {
     id: LanguageServerProtocol.RequestID,
     reply: @escaping (LSPResult<Request.Response>) -> Void
   ) {
-    let requestHandlerAndIndex = requestHandlers.enumerated().compactMap {
-      (index, handler) -> (RequestHandler<Request>, Int)? in
-      guard let handler = handler as? RequestHandler<Request> else {
-        return nil
+    requestHandlers.withLock { requestHandlers in
+      let requestHandlerAndIndex = requestHandlers.enumerated().compactMap {
+        (index, handler) -> (RequestHandler<Request>, Int)? in
+        guard let handler = handler as? RequestHandler<Request> else {
+          return nil
+        }
+        return (handler, index)
+      }.first
+      guard let (requestHandler, index) = requestHandlerAndIndex else {
+        reply(.failure(.methodNotFound(Request.method)))
+        return
       }
-      return (handler, index)
-    }.first
-    guard let (requestHandler, index) = requestHandlerAndIndex else {
-      reply(.failure(.methodNotFound(Request.method)))
-      return
+      reply(.success(requestHandler(params)))
+      requestHandlers.remove(at: index)
     }
-    reply(.success(requestHandler(params)))
-    requestHandlers.remove(at: index)
   }
 
   // MARK: - Convenience functions
@@ -396,8 +398,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
@@ -410,7 +414,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")))

Sources/SKTestSupport/Utils.swift

@@ -70,7 +70,7 @@ public func testScratchDir(testName: String = #function) throws -> URL {
 /// The temporary directory will be deleted at the end of `directory` unless the
 /// `SOURCEKITLSP_KEEP_TEST_SCRATCH_DIR` environment variable is set.
 public func withTestScratchDir<T>(
-  _ body: (AbsolutePath) async throws -> T,
+  @_inheritActorContext _ body: @Sendable (AbsolutePath) async throws -> T,
   testName: String = #function
 ) async throws -> T {
   let scratchDirectory = try testScratchDir(testName: testName)

Sources/SKTestSupport/WrappedSemaphore.swift

@@ -17,8 +17,8 @@ import Dispatch
 ///
 /// This should only be used for tests that test priority escalation and thus cannot await a `Task` (which would cause
 /// priority elevations).
-public struct WrappedSemaphore {
-  let semaphore = DispatchSemaphore(value: 0)
+public struct WrappedSemaphore: Sendable {
+  private let semaphore = DispatchSemaphore(value: 0)
 
   public init() {}