Skip to content

Commit 7865c39

Browse files
authored
[6.0] Add isolation argument to functions taking non-sendable async closures. (#643)
**Explanation:** Fixes concurrency issues using `confirmation {}`, `withKnownIssue {}`, and `#expect(throws:) {}` with actor-isolated code. **Scope:** 6.0 branch (6.0.1) **Issue:** https://github.com/swiftlang/swift-integration-tests/issue/622 **Original PR:** https://github.com/swiftlang/swift-integration-tests/pull/624 **Risk:** Moderate—involves changing the signatures of a number of functions which will break ABI for them as well as potential source breakage if somebody explicitly refers to these functions by name. **Testing:** New unit test coverage. **Reviewer:** @briancroom
1 parent b118feb commit 7865c39

13 files changed

+81
-31
lines changed

Sources/Testing/Expectations/ExpectationChecking+Macro.swift

+11-4
Original file line numberDiff line numberDiff line change
@@ -835,10 +835,11 @@ public func __checkClosureCall<E>(
835835
/// `#require()` macros. Do not call it directly.
836836
public func __checkClosureCall<E>(
837837
throws errorType: E.Type,
838-
performing body: () async throws -> some Any,
838+
performing body: () async throws -> sending some Any,
839839
expression: __Expression,
840840
comments: @autoclosure () -> [Comment],
841841
isRequired: Bool,
842+
isolation: isolated (any Actor)? = #isolation,
842843
sourceLocation: SourceLocation
843844
) async -> Result<Void, any Error> where E: Error {
844845
if errorType == Never.self {
@@ -848,6 +849,7 @@ public func __checkClosureCall<E>(
848849
expression: expression,
849850
comments: comments(),
850851
isRequired: isRequired,
852+
isolation: isolation,
851853
sourceLocation: sourceLocation
852854
)
853855
} else {
@@ -858,6 +860,7 @@ public func __checkClosureCall<E>(
858860
expression: expression,
859861
comments: comments(),
860862
isRequired: isRequired,
863+
isolation: isolation,
861864
sourceLocation: sourceLocation
862865
)
863866
}
@@ -911,10 +914,11 @@ public func __checkClosureCall(
911914
/// `#require()` macros. Do not call it directly.
912915
public func __checkClosureCall(
913916
throws _: Never.Type,
914-
performing body: () async throws -> some Any,
917+
performing body: () async throws -> sending some Any,
915918
expression: __Expression,
916919
comments: @autoclosure () -> [Comment],
917920
isRequired: Bool,
921+
isolation: isolated (any Actor)? = #isolation,
918922
sourceLocation: SourceLocation
919923
) async -> Result<Void, any Error> {
920924
var success = true
@@ -973,10 +977,11 @@ public func __checkClosureCall<E>(
973977
/// `#require()` macros. Do not call it directly.
974978
public func __checkClosureCall<E>(
975979
throws error: E,
976-
performing body: () async throws -> some Any,
980+
performing body: () async throws -> sending some Any,
977981
expression: __Expression,
978982
comments: @autoclosure () -> [Comment],
979983
isRequired: Bool,
984+
isolation: isolated (any Actor)? = #isolation,
980985
sourceLocation: SourceLocation
981986
) async -> Result<Void, any Error> where E: Error & Equatable {
982987
await __checkClosureCall(
@@ -986,6 +991,7 @@ public func __checkClosureCall<E>(
986991
expression: expression,
987992
comments: comments(),
988993
isRequired: isRequired,
994+
isolation: isolation,
989995
sourceLocation: sourceLocation
990996
)
991997
}
@@ -1047,12 +1053,13 @@ public func __checkClosureCall<R>(
10471053
/// - Warning: This function is used to implement the `#expect()` and
10481054
/// `#require()` macros. Do not call it directly.
10491055
public func __checkClosureCall<R>(
1050-
performing body: () async throws -> R,
1056+
performing body: () async throws -> sending R,
10511057
throws errorMatcher: (any Error) async throws -> Bool,
10521058
mismatchExplanation: ((any Error) -> String)? = nil,
10531059
expression: __Expression,
10541060
comments: @autoclosure () -> [Comment],
10551061
isRequired: Bool,
1062+
isolation: isolated (any Actor)? = #isolation,
10561063
sourceLocation: SourceLocation
10571064
) async -> Result<Void, any Error> {
10581065
var errorMatches = false

Sources/Testing/Issues/Confirmation.swift

+9-4
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ extension Confirmation {
5555
/// `body` is invoked. The default value of this argument is `1`, indicating
5656
/// that the event should occur exactly once. Pass `0` if the event should
5757
/// _never_ occur when `body` is invoked.
58+
/// - isolation: The actor to which `body` is isolated, if any.
5859
/// - sourceLocation: The source location to which any recorded issues should
5960
/// be attributed.
6061
/// - body: The function to invoke.
@@ -94,12 +95,14 @@ extension Confirmation {
9495
public func confirmation<R>(
9596
_ comment: Comment? = nil,
9697
expectedCount: Int = 1,
98+
isolation: isolated (any Actor)? = #isolation,
9799
sourceLocation: SourceLocation = #_sourceLocation,
98-
_ body: (Confirmation) async throws -> R
100+
_ body: (Confirmation) async throws -> sending R
99101
) async rethrows -> R {
100102
try await confirmation(
101103
comment,
102104
expectedCount: expectedCount ... expectedCount,
105+
isolation: isolation,
103106
sourceLocation: sourceLocation,
104107
body
105108
)
@@ -114,6 +117,7 @@ public func confirmation<R>(
114117
/// function.
115118
/// - expectedCount: A range of integers indicating the number of times the
116119
/// expected event should occur when `body` is invoked.
120+
/// - isolation: The actor to which `body` is isolated, if any.
117121
/// - sourceLocation: The source location to which any recorded issues should
118122
/// be attributed.
119123
/// - body: The function to invoke.
@@ -156,13 +160,14 @@ public func confirmation<R>(
156160
/// preconditions have been met, and records an issue if they have not.
157161
///
158162
/// If an exact count is expected, use
159-
/// ``confirmation(_:expectedCount:sourceLocation:_:)-7kfko`` instead.
163+
/// ``confirmation(_:expectedCount:isolation:sourceLocation:_:)`` instead.
160164
@_spi(Experimental)
161165
public func confirmation<R>(
162166
_ comment: Comment? = nil,
163167
expectedCount: some Confirmation.ExpectedCount,
168+
isolation: isolated (any Actor)? = #isolation,
164169
sourceLocation: SourceLocation = #_sourceLocation,
165-
_ body: (Confirmation) async throws -> R
170+
_ body: (Confirmation) async throws -> sending R
166171
) async rethrows -> R {
167172
let confirmation = Confirmation()
168173
defer {
@@ -182,7 +187,7 @@ public func confirmation<R>(
182187
@_spi(Experimental)
183188
extension Confirmation {
184189
/// A protocol that describes a range expression that can be used with
185-
/// ``confirmation(_:expectedCount:sourceLocation:_:)-41gmd``.
190+
/// ``confirmation(_:expectedCount:isolation:sourceLocation:_:)-9rt6m``.
186191
///
187192
/// This protocol represents any expression that describes a range of
188193
/// confirmation counts. For example, the expression `1 ..< 10` automatically

Sources/Testing/Issues/Issue+Recording.swift

+2
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,7 @@ extension Issue {
196196
/// - sourceLocation: The source location to attribute any caught error to.
197197
/// - configuration: The test configuration to use when recording an issue.
198198
/// The default value is ``Configuration/current``.
199+
/// - isolation: The actor to which `body` is isolated, if any.
199200
/// - body: An asynchronous closure that might throw an error.
200201
///
201202
/// - Returns: The issue representing the caught error, if any error was
@@ -204,6 +205,7 @@ extension Issue {
204205
static func withErrorRecording(
205206
at sourceLocation: SourceLocation,
206207
configuration: Configuration? = nil,
208+
isolation: isolated (any Actor)? = #isolation,
207209
_ body: () async throws -> Void
208210
) async -> (any Error)? {
209211
// Ensure that we are capturing backtraces for errors before we start

Sources/Testing/Issues/Issue.swift

+4-4
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public struct Issue: Sendable {
3333
/// ``Confirmation/confirm(count:)`` should have been called.
3434
///
3535
/// This issue can occur when calling
36-
/// ``confirmation(_:expectedCount:sourceLocation:_:)`` when the
36+
/// ``confirmation(_:expectedCount:isolation:sourceLocation:_:)`` when the
3737
/// confirmation passed to these functions' `body` closures is confirmed too
3838
/// few or too many times.
3939
indirect case confirmationMiscounted(actual: Int, expected: Int)
@@ -48,9 +48,9 @@ public struct Issue: Sendable {
4848
/// ``Confirmation/confirm(count:)`` should have been called.
4949
///
5050
/// This issue can occur when calling
51-
/// ``confirmation(_:expectedCount:sourceLocation:_:)-41gmd`` when the
52-
/// confirmation passed to these functions' `body` closures is confirmed too
53-
/// few or too many times.
51+
/// ``confirmation(_:expectedCount:isolation:sourceLocation:_:)-9rt6m`` when
52+
/// the confirmation passed to these functions' `body` closures is confirmed
53+
/// too few or too many times.
5454
@_spi(Experimental)
5555
indirect case confirmationOutOfRange(actual: Int, expected: any Confirmation.ExpectedCount)
5656

Sources/Testing/Issues/KnownIssue.swift

+9-5
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ public typealias KnownIssueMatcher = @Sendable (_ issue: Issue) -> Bool
110110
/// Because all errors thrown by `body` are caught as known issues, this
111111
/// function is not throwing. If only some errors or issues are known to occur
112112
/// while others should continue to cause test failures, use
113-
/// ``withKnownIssue(_:isIntermittent:sourceLocation:_:when:matching:)-5vi5n``
113+
/// ``withKnownIssue(_:isIntermittent:sourceLocation:_:when:matching:)``
114114
/// instead.
115115
public func withKnownIssue(
116116
_ comment: Comment? = nil,
@@ -161,7 +161,7 @@ public func withKnownIssue(
161161
///
162162
/// It is not necessary to specify both `precondition` and `issueMatcher` if
163163
/// only one is relevant. If all errors and issues should be considered known
164-
/// issues, use ``withKnownIssue(_:isIntermittent:sourceLocation:_:)-95r6o``
164+
/// issues, use ``withKnownIssue(_:isIntermittent:sourceLocation:_:)``
165165
/// instead.
166166
///
167167
/// - Note: `issueMatcher` may be invoked more than once for the same issue.
@@ -200,6 +200,7 @@ public func withKnownIssue(
200200
/// - isIntermittent: Whether or not the known issue occurs intermittently. If
201201
/// this argument is `true` and the known issue does not occur, no secondary
202202
/// issue is recorded.
203+
/// - isolation: The actor to which `body` is isolated, if any.
203204
/// - sourceLocation: The source location to which any recorded issues should
204205
/// be attributed.
205206
/// - body: The function to invoke.
@@ -218,15 +219,16 @@ public func withKnownIssue(
218219
/// Because all errors thrown by `body` are caught as known issues, this
219220
/// function is not throwing. If only some errors or issues are known to occur
220221
/// while others should continue to cause test failures, use
221-
/// ``withKnownIssue(_:isIntermittent:sourceLocation:_:when:matching:)-47y3z``
222+
/// ``withKnownIssue(_:isIntermittent:isolation:sourceLocation:_:when:matching:)``
222223
/// instead.
223224
public func withKnownIssue(
224225
_ comment: Comment? = nil,
225226
isIntermittent: Bool = false,
227+
isolation: isolated (any Actor)? = #isolation,
226228
sourceLocation: SourceLocation = #_sourceLocation,
227229
_ body: () async throws -> Void
228230
) async {
229-
try? await withKnownIssue(comment, isIntermittent: isIntermittent, sourceLocation: sourceLocation, body, matching: { _ in true })
231+
try? await withKnownIssue(comment, isIntermittent: isIntermittent, isolation: isolation, sourceLocation: sourceLocation, body, matching: { _ in true })
230232
}
231233

232234
/// Invoke a function that has a known issue that is expected to occur during
@@ -237,6 +239,7 @@ public func withKnownIssue(
237239
/// - isIntermittent: Whether or not the known issue occurs intermittently. If
238240
/// this argument is `true` and the known issue does not occur, no secondary
239241
/// issue is recorded.
242+
/// - isolation: The actor to which `body` is isolated, if any.
240243
/// - sourceLocation: The source location to which any recorded issues should
241244
/// be attributed.
242245
/// - body: The function to invoke.
@@ -269,13 +272,14 @@ public func withKnownIssue(
269272
///
270273
/// It is not necessary to specify both `precondition` and `issueMatcher` if
271274
/// only one is relevant. If all errors and issues should be considered known
272-
/// issues, use ``withKnownIssue(_:isIntermittent:sourceLocation:_:)-3g6b7``
275+
/// issues, use ``withKnownIssue(_:isIntermittent:isolation:sourceLocation:_:when:matching:)``
273276
/// instead.
274277
///
275278
/// - Note: `issueMatcher` may be invoked more than once for the same issue.
276279
public func withKnownIssue(
277280
_ comment: Comment? = nil,
278281
isIntermittent: Bool = false,
282+
isolation: isolated (any Actor)? = #isolation,
279283
sourceLocation: SourceLocation = #_sourceLocation,
280284
_ body: () async throws -> Void,
281285
when precondition: () async -> Bool = { true },

Sources/Testing/Testing.docc/Expectations.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ the test when the code doesn't satisfy a requirement, use
7777
### Confirming that asynchronous events occur
7878

7979
- <doc:testing-asynchronous-code>
80-
- ``confirmation(_:expectedCount:sourceLocation:_:)``
80+
- ``confirmation(_:expectedCount:isolation:sourceLocation:_:)``
8181
- ``Confirmation``
8282

8383
### Retrieving information about checked expectations

Sources/Testing/Testing.docc/MigratingFromXCTest.md

+5-5
Original file line numberDiff line numberDiff line change
@@ -434,7 +434,7 @@ Some tests, especially those that test asynchronously-delivered events, cannot
434434
be readily converted to use Swift concurrency. The testing library offers
435435
functionality called _confirmations_ which can be used to implement these tests.
436436
Instances of ``Confirmation`` are created and used within the scope of the
437-
function ``confirmation(_:expectedCount:sourceLocation:_:)``.
437+
function ``confirmation(_:expectedCount:isolation:sourceLocation:_:)``.
438438

439439
Confirmations function similarly to the expectations API of XCTest, however, they don't
440440
block or suspend the caller while waiting for a condition to be fulfilled.
@@ -531,8 +531,8 @@ to tell XCTest and its infrastructure that the issue shouldn't cause the test
531531
to fail. The testing library has an equivalent function with synchronous and
532532
asynchronous variants:
533533

534-
- ``withKnownIssue(_:isIntermittent:sourceLocation:_:)-95r6o``
535-
- ``withKnownIssue(_:isIntermittent:sourceLocation:_:)-3g6b7``
534+
- ``withKnownIssue(_:isIntermittent:sourceLocation:_:)``
535+
- ``withKnownIssue(_:isIntermittent:isolation:sourceLocation:_:)``
536536

537537
This function can be used to annotate a section of a test as having a known
538538
issue:
@@ -627,8 +627,8 @@ Additional options can be specified when calling `XCTExpectFailure()`:
627627
The testing library includes overloads of `withKnownIssue()` that take
628628
additional arguments with similar behavior:
629629

630-
- ``withKnownIssue(_:isIntermittent:sourceLocation:_:when:matching:)-5vi5n``
631-
- ``withKnownIssue(_:isIntermittent:sourceLocation:_:when:matching:)-47y3z``
630+
- ``withKnownIssue(_:isIntermittent:sourceLocation:_:when:matching:)``
631+
- ``withKnownIssue(_:isIntermittent:isolation:sourceLocation:_:when:matching:)``
632632

633633
To conditionally enable known-issue matching or to match only certain kinds
634634
of issues:

Sources/Testing/Testing.docc/known-issues.md

+4-4
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,10 @@ at runtime not to mark the test as failing when issues occur.
2222

2323
### Recording known issues in tests
2424

25-
- ``withKnownIssue(_:isIntermittent:sourceLocation:_:)-95r6o``
26-
- ``withKnownIssue(_:isIntermittent:sourceLocation:_:)-3g6b7``
27-
- ``withKnownIssue(_:isIntermittent:sourceLocation:_:when:matching:)-5vi5n``
28-
- ``withKnownIssue(_:isIntermittent:sourceLocation:_:when:matching:)-47y3z``
25+
- ``withKnownIssue(_:isIntermittent:sourceLocation:_:)``
26+
- ``withKnownIssue(_:isIntermittent:isolation:sourceLocation:_:)``
27+
- ``withKnownIssue(_:isIntermittent:sourceLocation:_:when:matching:)``
28+
- ``withKnownIssue(_:isIntermittent:isolation:sourceLocation:_:when:matching:)``
2929
- ``KnownIssueMatcher``
3030

3131
### Describing a failure or warning

Sources/Testing/Testing.docc/testing-asynchronous-code.md

+3-3
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,9 @@ expected event happens.
3131

3232
### Confirm that an event happens
3333

34-
Call ``confirmation(_:expectedCount:sourceLocation:_:)`` in your asynchronous
35-
test function to create a `Confirmation` for the expected event. In the trailing
36-
closure parameter, call the code under test. Swift Testing passes a
34+
Call ``confirmation(_:expectedCount:isolation:sourceLocation:_:)`` in your
35+
asynchronous test function to create a `Confirmation` for the expected event. In
36+
the trailing closure parameter, call the code under test. Swift Testing passes a
3737
`Confirmation` as the parameter to the closure, which you call as a function in
3838
the event handler for the code under test when the event you're testing for
3939
occurs:

Sources/TestingMacros/ConditionMacro.swift

+16-1
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,20 @@ public import SwiftSyntaxMacros
3333
///
3434
/// The `__check()` function that implements expansions of these macros must
3535
/// take any developer-supplied arguments _before_ the ones inserted during
36-
/// macro expansion (starting with the `"expression"` argument.)
36+
/// macro expansion (starting with the `"expression"` argument.) The `isolation`
37+
/// argument (if present) and `sourceLocation` argument are placed at the end of
38+
/// the generated function call's argument list.
3739
public protocol ConditionMacro: ExpressionMacro, Sendable {
3840
/// Whether or not the macro's expansion may throw an error.
3941
static var isThrowing: Bool { get }
4042
}
4143

4244
// MARK: -
4345

46+
/// The token used as the label of the argument passed to `#expect()` and
47+
/// `#require()` and used for actor isolation.
48+
private var _isolationLabel: TokenSyntax { .identifier("isolation") }
49+
4450
/// The token used as the label of the source location argument passed to
4551
/// `#expect()` and `#require()`.
4652
private var _sourceLocationLabel: TokenSyntax { .identifier("sourceLocation") }
@@ -89,6 +95,9 @@ extension ConditionMacro {
8995
// never the first argument.)
9096
commentIndex = macroArguments.dropFirst().lastIndex { $0.label == nil }
9197
}
98+
let isolationArgumentIndex = macroArguments.lazy
99+
.compactMap(\.label)
100+
.firstIndex { $0.tokenKind == _isolationLabel.tokenKind }
92101
let sourceLocationArgumentIndex = macroArguments.lazy
93102
.compactMap(\.label)
94103
.firstIndex { $0.tokenKind == _sourceLocationLabel.tokenKind }
@@ -103,6 +112,7 @@ extension ConditionMacro {
103112
// arguments here.
104113
checkArguments += macroArguments.indices.lazy
105114
.filter { $0 != commentIndex }
115+
.filter { $0 != isolationArgumentIndex }
106116
.filter { $0 != sourceLocationArgumentIndex }
107117
.map { macroArguments[$0] }
108118

@@ -124,6 +134,7 @@ extension ConditionMacro {
124134
// "sourceLocation" arguments here.
125135
checkArguments += macroArguments.dropFirst().indices.lazy
126136
.filter { $0 != commentIndex }
137+
.filter { $0 != isolationArgumentIndex }
127138
.filter { $0 != sourceLocationArgumentIndex }
128139
.map { macroArguments[$0] }
129140

@@ -160,6 +171,10 @@ extension ConditionMacro {
160171

161172
checkArguments.append(Argument(label: "isRequired", expression: BooleanLiteralExprSyntax(isThrowing)))
162173

174+
if let isolationArgumentIndex {
175+
checkArguments.append(macroArguments[isolationArgumentIndex])
176+
}
177+
163178
if let sourceLocationArgumentIndex {
164179
checkArguments.append(macroArguments[sourceLocationArgumentIndex])
165180
} else {

Tests/TestingMacrosTests/ConditionMacroTests.swift

+4
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,8 @@ struct ConditionMacroTests {
8888
##"Testing.__checkPropertyAccess(a.self, getting: { $0???.isB }, expression: .__fromPropertyAccess(.__fromSyntaxNode("a"), .__fromSyntaxNode("isB")), comments: [], isRequired: false, sourceLocation: Testing.SourceLocation.__here()).__expected()"##,
8989
##"#expect(a?.b.isB)"##:
9090
##"Testing.__checkPropertyAccess(a?.b.self, getting: { $0?.isB }, expression: .__fromPropertyAccess(.__fromSyntaxNode("a?.b"), .__fromSyntaxNode("isB")), comments: [], isRequired: false, sourceLocation: Testing.SourceLocation.__here()).__expected()"##,
91+
##"#expect(isolation: somewhere) {}"##:
92+
##"Testing.__checkClosureCall(performing: {}, expression: .__fromSyntaxNode("{}"), comments: [], isRequired: false, isolation: somewhere, sourceLocation: Testing.SourceLocation.__here()).__expected()"##,
9193
]
9294
)
9395
func expectMacro(input: String, expectedOutput: String) throws {
@@ -164,6 +166,8 @@ struct ConditionMacroTests {
164166
##"Testing.__checkPropertyAccess(a.self, getting: { $0???.isB }, expression: .__fromPropertyAccess(.__fromSyntaxNode("a"), .__fromSyntaxNode("isB")), comments: [], isRequired: true, sourceLocation: Testing.SourceLocation.__here()).__required()"##,
165167
##"#require(a?.b.isB)"##:
166168
##"Testing.__checkPropertyAccess(a?.b.self, getting: { $0?.isB }, expression: .__fromPropertyAccess(.__fromSyntaxNode("a?.b"), .__fromSyntaxNode("isB")), comments: [], isRequired: true, sourceLocation: Testing.SourceLocation.__here()).__required()"##,
169+
##"#require(isolation: somewhere) {}"##:
170+
##"Testing.__checkClosureCall(performing: {}, expression: .__fromSyntaxNode("{}"), comments: [], isRequired: true, isolation: somewhere, sourceLocation: Testing.SourceLocation.__here()).__required()"##,
167171
]
168172
)
169173
func requireMacro(input: String, expectedOutput: String) throws {

0 commit comments

Comments
 (0)