Skip to content

Make tests build in Swift 6 mode #1294

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
May 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ default.profraw
Package.resolved
/.build
/.index-build
/.linux-build
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could just add the following if you want to use this pattern:

/.*-build

/Packages
/*.xcodeproj
/*.sublime-project
Expand Down
4 changes: 2 additions & 2 deletions Sources/LSPTestSupport/TestJSONRPCConnection.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down Expand Up @@ -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)
}
Expand Down
37 changes: 37 additions & 0 deletions Sources/SKTestSupport/DefaultSDKPath.swift
Original file line number Diff line number Diff line change
@@ -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
}()
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
//===----------------------------------------------------------------------===//

import Foundation
import ISDBTibs
import LanguageServerProtocol
@_spi(Testing) import SKCore
import SourceKitLSP
Expand Down Expand Up @@ -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
Expand Down
36 changes: 19 additions & 17 deletions Sources/SKTestSupport/SkipUnless.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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
Expand All @@ -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)
Expand All @@ -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() {}
Expand Down
52 changes: 28 additions & 24 deletions Sources/SKTestSupport/TestSourceKitLSPClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
//
//===----------------------------------------------------------------------===//

import CAtomics
import Foundation
import LSPTestSupport
import LanguageServerProtocol
Expand All @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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()
Expand All @@ -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)
}
}
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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")))
Expand Down
2 changes: 1 addition & 1 deletion Sources/SKTestSupport/Utils.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions Sources/SKTestSupport/WrappedSemaphore.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}

Expand Down
Loading