Simplify the configuration option for synchronous test isolation. (#747)

This PR replaces `Configuration.isMainActorIsolationEnforced` with a new
property `Configuration.defaultIsolationContext` which can be set to any
actor, not just the main actor. This property is then used as the
isolation context for synchronous test functions that are not otherwise
actor-isolated.

The typical use case for this property would be to set it to
`MainActor.shared` so that synchronous test functions run on the main
actor, but a testing tool might want to instead run them all on another
actor or serial executor for performance (or other) reasons.

This change isn't just speculative: it allows us to simplify the
macro-generated thunk for synchronous test functions so that it does not
need to emit the body of the thunk twice (once for the main actor, once
for non-isolated.) This change wasn't possible before `#isolation` was
introduced, but we'd already added
`Configuration.isMainActorIsolationEnforced` when that happened so the
timing didn't line up.

By rewriting this part of the macro expansion, we're able to toss a
bunch of support code that isn't needed anymore. Yay!

### Checklist:

- [x] Code and documentation should follow the style of the [Style
Guide](https://github.com/apple/swift-testing/blob/main/Documentation/StyleGuide.md).
- [x] If public symbols are renamed or modified, DocC references should
be updated.

Author: grynspan

Sources/Testing/Running/Configuration.swift

@@ -105,14 +105,13 @@ public struct Configuration: Sendable {
   /// By default, the value of this property allows for a single iteration.
   public var repetitionPolicy: RepetitionPolicy = .once
 
-  // MARK: - Main actor isolation
+  // MARK: - Isolation context for synchronous tests
 
-#if !SWT_NO_GLOBAL_ACTORS
-  /// Whether or not synchronous test functions need to run on the main actor.
+  /// The isolation context to use for synchronous test functions.
   ///
-  /// This property is available on platforms where UI testing is implemented.
-  public var isMainActorIsolationEnforced = false
-#endif
+  /// If the value of this property is `nil`, synchronous test functions run in
+  /// an unspecified isolation context.
+  public var defaultSynchronousIsolationContext: (any Actor)? = nil
 
   // MARK: - Time limits
 
@@ -233,3 +232,23 @@ public struct Configuration: Sendable {
   /// The test case filter to which test cases should be filtered when run.
   public var testCaseFilter: TestCaseFilter = { _, _ in true }
 }
+
+// MARK: - Deprecated
+
+extension Configuration {
+#if !SWT_NO_GLOBAL_ACTORS
+  @available(*, deprecated, message: "Set defaultSynchronousIsolationContext instead.")
+  public var isMainActorIsolationEnforced: Bool {
+    get {
+      defaultSynchronousIsolationContext === MainActor.shared
+    }
+    set {
+      if newValue {
+        defaultSynchronousIsolationContext = MainActor.shared
+      } else {
+        defaultSynchronousIsolationContext = nil
+      }
+    }
+  }
+#endif
+}

Sources/Testing/Test+Macro.swift

@@ -504,58 +504,13 @@ extension Test {
   value
 }
 
-#if !SWT_NO_GLOBAL_ACTORS
-/// Invoke a function isolated to the main actor if appropriate.
+/// The current default isolation context.
 ///
-/// - Parameters:
-///   - thenBody: The function to invoke, isolated to the main actor, if actor
-///     isolation is required.
-///   - elseBody: The function to invoke if actor isolation is not required.
-///
-/// - Returns: Whatever is returned by `thenBody` or `elseBody`.
-///
-/// - Throws: Whatever is thrown by `thenBody` or `elseBody`.
-///
-/// `thenBody` and `elseBody` should represent the same function with differing
-/// actor isolation. Which one is invoked depends on whether or not synchronous
-/// test functions need to run on the main actor.
-///
-/// - Warning: This function is used to implement the `@Test` macro. Do not call
-///   it directly.
-public func __ifMainActorIsolationEnforced<R>(
-  _ thenBody: @Sendable @MainActor () async throws -> R,
-  else elseBody: @Sendable () async throws -> R
-) async throws -> R where R: Sendable {
-  if Configuration.current?.isMainActorIsolationEnforced == true {
-    try await thenBody()
-  } else {
-    try await elseBody()
-  }
-}
-#else
-/// Invoke a function.
-///
-/// - Parameters:
-///   - body: The function to invoke.
-///
-/// - Returns: Whatever is returned by `body`.
-///
-/// - Throws: Whatever is thrown by `body`.
-///
-/// This function simply invokes `body`. Its signature matches that of the same
-/// function when `SWT_NO_GLOBAL_ACTORS` is not defined so that it can be used
-/// during expansion of the `@Test` macro without knowing the value of that
-/// compiler conditional on the target platform.
-///
-/// - Warning: This function is used to implement the `@Test` macro. Do not call
+/// - Warning: This property is used to implement the `@Test` macro. Do not call
 ///   it directly.
-@inlinable public func __ifMainActorIsolationEnforced<R>(
-  _: @Sendable () async throws -> R,
-  else body: @Sendable () async throws -> R
-) async throws -> R where R: Sendable {
-  try await body()
+public var __defaultSynchronousIsolationContext: (any Actor)? {
+  Configuration.current?.defaultSynchronousIsolationContext ?? #isolation
 }
-#endif
 
 /// Run a test function as an `XCTestCase`-compatible method.
 ///

Sources/TestingMacros/Support/Additions/FunctionDeclSyntaxAdditions.swift

@@ -26,6 +26,13 @@ extension FunctionDeclSyntax {
       .contains(.keyword(.mutating))
   }
 
+  /// Whether or not this function is a `nonisolated` function.
+  var isNonisolated: Bool {
+    modifiers.lazy
+      .map(\.name.tokenKind)
+      .contains(.keyword(.nonisolated))
+  }
+
   /// The name of this function including parentheses, parameter labels, and
   /// colons.
   var completeName: String {

Sources/TestingMacros/TestDeclarationMacro.swift

@@ -172,62 +172,6 @@ public struct TestDeclarationMacro: PeerMacro, Sendable {
     return FunctionParameterClauseSyntax(parameters: parameterList)
   }
 
-  /// Create a closure capture list used to capture the arguments to a function
-  /// when calling it from its corresponding thunk function.
-  ///
-  /// - Parameters:
-  ///   - parametersWithLabels: A sequence of tuples containing parameters to
-  ///     the original function and their corresponding identifiers as used by
-  ///     the thunk function.
-  ///
-  /// - Returns: A closure capture list syntax node representing the arguments
-  ///   to the thunk function.
-  ///
-  /// We need to construct a capture list when calling a synchronous test
-  /// function because of the call to `__ifMainActorIsolationEnforced(_:else:)`
-  /// that we insert. That function theoretically captures all arguments twice,
-  /// which is not allowed for arguments marked `borrowing` or `consuming`. The
-  /// capture list forces those arguments to be copied, side-stepping the issue.
-  ///
-  /// - Note: We do not support move-only types as arguments yet. Instances of
-  ///   move-only types cannot be used with generics, so they cannot be elements
-  ///   of a `Collection`.
-  private static func _createCaptureListExpr(
-    from parametersWithLabels: some Sequence<(DeclReferenceExprSyntax, FunctionParameterSyntax)>
-  ) -> ClosureCaptureClauseSyntax {
-    let specifierKeywordsNeedingCopy: [TokenKind] = [.keyword(.borrowing), .keyword(.consuming),]
-    let closureCaptures = parametersWithLabels.lazy.map { label, parameter in
-      var needsCopy = false
-      if let parameterType = parameter.type.as(AttributedTypeSyntax.self) {
-        needsCopy = parameterType.specifiers.contains { specifier in
-          guard case let .simpleTypeSpecifier(specifier) = specifier else {
-            return false
-          }
-          return specifierKeywordsNeedingCopy.contains(specifier.specifier.tokenKind)
-        }
-      }
-
-      if needsCopy {
-        return ClosureCaptureSyntax(
-          name: label.baseName,
-          equal: .equalToken(),
-          expression: CopyExprSyntax(
-            copyKeyword: .keyword(.copy).with(\.trailingTrivia, .space),
-            expression: label
-          )
-        )
-      } else {
-        return ClosureCaptureSyntax(expression: label)
-      }
-    }
-
-    return ClosureCaptureClauseSyntax {
-      for closureCapture in closureCaptures {
-        closureCapture
-      }
-    }
-  }
-
   /// The `static` keyword, if `typeName` is not `nil`.
   ///
   /// - Parameters:
@@ -269,7 +213,6 @@ public struct TestDeclarationMacro: PeerMacro, Sendable {
     // needed, so it's lazy.
     let forwardedParamsExpr = _createForwardedParamsExpr(from: parametersWithLabels)
     let thunkParamsExpr = _createThunkParamsExpr(from: parametersWithLabels)
-    lazy var captureListExpr = _createCaptureListExpr(from: parametersWithLabels)
 
     // How do we call a function if we don't know whether it's `async` or
     // `throws`? Yes, we know if the keywords are on the function, but it could
@@ -290,7 +233,7 @@ public struct TestDeclarationMacro: PeerMacro, Sendable {
     // If the function is noasync *and* main-actor-isolated, we'll call through
     // MainActor.run to invoke it. We do not have a general mechanism for
     // detecting isolation to other global actors.
-    lazy var isMainActorIsolated = !functionDecl.attributes(named: "MainActor", inModuleNamed: "Swift").isEmpty
+    lazy var isMainActorIsolated = !functionDecl.attributes(named: "MainActor", inModuleNamed: "_Concurrency").isEmpty
     var forwardCall: (ExprSyntax) -> ExprSyntax = {
       "try await Testing.__requiringTry(Testing.__requiringAwait(\($0)))"
     }
@@ -315,7 +258,7 @@ public struct TestDeclarationMacro: PeerMacro, Sendable {
       if functionDecl.isStaticOrClass {
         thunkBody = "_ = \(forwardCall("\(typeName).\(functionDecl.name.trimmed)\(forwardedParamsExpr)"))"
       } else {
-        let instanceName = context.makeUniqueName(thunking: functionDecl)
+        let instanceName = context.makeUniqueName("")
         let varOrLet = functionDecl.isMutating ? "var" : "let"
         thunkBody = """
         \(raw: varOrLet) \(raw: instanceName) = \(forwardInit("\(typeName)()"))
@@ -344,16 +287,45 @@ public struct TestDeclarationMacro: PeerMacro, Sendable {
       thunkBody = "_ = \(forwardCall("\(functionDecl.name.trimmed)\(forwardedParamsExpr)"))"
     }
 
-    // If this function is synchronous and is not explicitly isolated to the
-    // main actor, it may still need to run main-actor-isolated depending on the
-    // runtime configuration in the test process.
-    if functionDecl.signature.effectSpecifiers?.asyncSpecifier == nil && !isMainActorIsolated {
+    // If this function is synchronous, is not explicitly nonisolated, and is
+    // not explicitly isolated to some actor, it should run in the configured
+    // default isolation context. If the suite type is an actor, this will cause
+    // a hop off the actor followed by an immediate hop back on, but otherwise
+    // should be harmless. Note that we do not support specifying an `isolated`
+    // parameter on a test function at this time.
+    //
+    // We use a second, inner thunk function here instead of just adding the
+    // isolation parameter to the "real" thunk because adding it there prevents
+    // correct tuple desugaring of the "real" arguments to the thunk.
+    if functionDecl.signature.effectSpecifiers?.asyncSpecifier == nil && !isMainActorIsolated && !functionDecl.isNonisolated {
+      // Get a unique name for this secondary thunk. We don't need it to be
+      // uniqued against functionDecl because it's interior to the "real" thunk,
+      // so its name can't conflict with any other names visible in this scope.
+      let isolationThunkName = context.makeUniqueName("")
+
+      // Insert a (defaulted) isolated argument. If we emit a closure (or inner
+      // function) that captured the arguments to the "real" thunk, the compiler
+      // has trouble reasoning about the lifetime of arguments to that closure
+      // especially if those arguments are borrowed or consumed, which results
+      // in hard-to-avoid compile-time errors. Fortunately, forwarding the full
+      // argument list is straightforward.
+      let thunkParamsExprCopy = FunctionParameterClauseSyntax {
+        for thunkParam in thunkParamsExpr.parameters {
+          thunkParam
+        }
+        FunctionParameterSyntax(
+          modifiers: [DeclModifierSyntax(name: .keyword(.isolated))],
+          firstName: .wildcardToken(),
+          type: "isolated (any Actor)?" as TypeSyntax,
+          defaultValue: InitializerClauseSyntax(value: "Testing.__defaultSynchronousIsolationContext" as ExprSyntax)
+        )
+      }
+
       thunkBody = """
-      try await Testing.__ifMainActorIsolationEnforced { \(captureListExpr) in
-        \(thunkBody)
-      } else: { \(captureListExpr) in
+      @Sendable func \(isolationThunkName)\(thunkParamsExprCopy) async throws {
         \(thunkBody)
       }
+      try await \(isolationThunkName)\(forwardedParamsExpr)
       """
     }
 

Tests/TestingMacrosTests/TestDeclarationMacroTests.swift

@@ -325,8 +325,6 @@ struct TestDeclarationMacroTests {
       ("@Test @_unavailableFromAsync @MainActor func f() {}", nil, "MainActor.run"),
       ("@Test @available(*, noasync) func f() {}", nil, "__requiringTry"),
       ("@Test @_unavailableFromAsync func f() {}", nil, "__requiringTry"),
-      ("@Test(arguments: []) func f(i: borrowing Int) {}", nil, "copy"),
-      ("@Test(arguments: []) func f(_ i: borrowing Int) {}", nil, "copy"),
       ("@Test(arguments: []) func f(f: () -> String) {}", "(() -> String).self", nil),
       ("struct S {\n\t@Test func testF() {} }", nil, "__invokeXCTestCaseMethod"),
       ("struct S {\n\t@Test func testF() throws {} }", nil, "__invokeXCTestCaseMethod"),

Tests/TestingTests/MiscellaneousTests.swift

@@ -99,12 +99,28 @@ struct SendableTests: Sendable {
   @Test(.hidden, arguments: FixtureData.zeroUpTo100)
   func parameterizedBorrowingAsync(i: borrowing Int) async {}
 
+  @MainActor
+  @Test(.hidden, arguments: FixtureData.zeroUpTo100)
+  func parameterizedBorrowingMainActor(i: borrowing Int) {}
+
+  @available(*, noasync)
+  @Test(.hidden, arguments: FixtureData.zeroUpTo100)
+  func parameterizedBorrowingNoasync(i: borrowing Int) {}
+
   @Test(.hidden, arguments: FixtureData.zeroUpTo100)
   func parameterizedConsuming(i: consuming Int) {}
 
   @Test(.hidden, arguments: FixtureData.zeroUpTo100)
   func parameterizedConsumingAsync(i: consuming Int) async { }
 
+  @MainActor
+  @Test(.hidden, arguments: FixtureData.zeroUpTo100)
+  func parameterizedConsumingMainActor(i: consuming Int) {}
+
+  @available(*, noasync)
+  @Test(.hidden, arguments: FixtureData.zeroUpTo100)
+  func parameterizedConsumingNoasync(i: consuming Int) {}
+
   @Test(.hidden, arguments: FixtureData.stringReturningClosureArray)
   func parameterizedAcceptingFunction(f: @Sendable () -> String) {}
 }

Tests/TestingTests/RunnerTests.swift

@@ -807,7 +807,11 @@ final class RunnerTests: XCTestCase {
   @TaskLocal static var isMainActorIsolationEnforced = false
 
   @Suite(.hidden) struct MainActorIsolationTests {
-    @Test(.hidden) func mustRunOnMainActor() {
+    @Test(.hidden) func mightRunOnMainActor() {
+      XCTAssertEqual(Thread.isMainThread, isMainActorIsolationEnforced)
+    }
+
+    @Test(.hidden, arguments: 0 ..< 10) func mightRunOnMainActor(arg: Int) {
       XCTAssertEqual(Thread.isMainThread, isMainActorIsolationEnforced)
     }
 
@@ -822,8 +826,13 @@ final class RunnerTests: XCTestCase {
     @Test(.hidden) @MainActor func asyncButRunsOnMainActor() async {
       XCTAssertTrue(Thread.isMainThread)
     }
+
+    @Test(.hidden) nonisolated func runsNonisolated() {
+      XCTAssertFalse(Thread.isMainThread)
+    }
   }
 
+  @available(*, deprecated)
   func testSynchronousTestFunctionRunsOnMainActorWhenEnforced() async {
     var configuration = Configuration()
     configuration.isMainActorIsolationEnforced = true
@@ -836,6 +845,19 @@ final class RunnerTests: XCTestCase {
       await runTest(for: MainActorIsolationTests.self, configuration: configuration)
     }
   }
+
+  func testSynchronousTestFunctionRunsInDefaultIsolationContext() async {
+    var configuration = Configuration()
+    configuration.defaultSynchronousIsolationContext = MainActor.shared
+    await Self.$isMainActorIsolationEnforced.withValue(true) {
+      await runTest(for: MainActorIsolationTests.self, configuration: configuration)
+    }
+
+    configuration.defaultSynchronousIsolationContext = nil
+    await Self.$isMainActorIsolationEnforced.withValue(false) {
+      await runTest(for: MainActorIsolationTests.self, configuration: configuration)
+    }
+  }
 #endif
 
   @Suite(.hidden) struct DeprecatedVersionTests {