Skip to content

Commit e5ec034

Browse files
committed
Require Sequence conformance too
1 parent 64e2607 commit e5ec034

File tree

6 files changed

+52
-119
lines changed

6 files changed

+52
-119
lines changed

Documentation/Proposals/NNNN-ranged-confirmations.md

+18-20
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ A new overload of `confirmation()` is added:
110110
/// ``confirmation(_:expectedCount:isolation:sourceLocation:_:)`` instead.
111111
public func confirmation<R>(
112112
_ comment: Comment? = nil,
113-
expectedCount: some RangeExpression<Int> & Sendable,
113+
expectedCount: some RangeExpression<Int> & Sequence<Int> Sendable,
114114
isolation: isolated (any Actor)? = #isolation,
115115
sourceLocation: SourceLocation = #_sourceLocation,
116116
_ body: (Confirmation) async throws -> sending R
@@ -121,22 +121,22 @@ public func confirmation<R>(
121121

122122
Certain types of range, specifically [`PartialRangeUpTo`](https://developer.apple.com/documentation/swift/partialrangeupto)
123123
and [`PartialRangeThrough`](https://developer.apple.com/documentation/swift/partialrangethrough),
124-
are valid when used with this new interface, but may have surprising behavior
125-
because they implicitly include `0`. If a test author writes `...10`, do they
126-
mean "zero to ten" or "one to ten"? The programmatic meaning is the former, but
127-
some test authors might mean the latter. If an event does not occur, a test
128-
using `confirmation()` and this `expectedCount` value would pass when the test
129-
author meant for it to fail.
130-
131-
Swift Testing will attempt to detect these ambiguous uses of `...n` and `..<n`
132-
expressions and diagnose them, recommending that test authors explicitly write
133-
`0` or `1` as a lower bound.
134-
135-
### Unbounded ranges
136-
137-
Finally, an overload is added that takes an "unbounded range" (which is not
138-
technically a range at all, but… a closure?) This overload is marked unavailable
139-
because an unbounded range is effectively useless for testing:
124+
may have surprising behavior when used with this new interface because they
125+
implicitly include `0`. If a test author writes `...10`, do they mean "zero to
126+
ten" or "one to ten"? The programmatic meaning is the former, but some test
127+
authors might mean the latter. If an event does not occur, a test using
128+
`confirmation()` and this `expectedCount` value would pass when the test author
129+
meant for it to fail.
130+
131+
The unbounded range (`...`) type `UnboundedRange` is effectively useless when
132+
used with this interface and any use of it here is almost certainly a programmer
133+
error.
134+
135+
`PartialRangeUpTo` and `PartialRangeThrough` conform to `RangeExpression`, but
136+
not to `Sequence`, so they will be rejected at compile time. `UnboundedRange` is
137+
a non-nominal type and will not match either. We will provide unavailable
138+
overloads of `confirmation()` for these types with messages that explain why
139+
they are unavailable, e.g.:
140140

141141
```swift
142142
@available(*, unavailable, message: "Unbounded range '...' has no effect when used with a confirmation.")
@@ -146,9 +146,7 @@ public func confirmation<R>(
146146
isolation: isolated (any Actor)? = #isolation,
147147
sourceLocation: SourceLocation = #_sourceLocation,
148148
_ body: (Confirmation) async throws -> R
149-
) async rethrows -> R {
150-
fatalError("Unsupported")
151-
}
149+
) async rethrows -> R
152150
```
153151

154152
## Source compatibility

Sources/Testing/Issues/Confirmation.swift

+33-1
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ public func confirmation<R>(
163163
/// ``confirmation(_:expectedCount:isolation:sourceLocation:_:)`` instead.
164164
public func confirmation<R>(
165165
_ comment: Comment? = nil,
166-
expectedCount: some RangeExpression<Int> & Sendable,
166+
expectedCount: some RangeExpression<Int> & Sequence<Int> & Sendable,
167167
isolation: isolated (any Actor)? = #isolation,
168168
sourceLocation: SourceLocation = #_sourceLocation,
169169
_ body: (Confirmation) async throws -> sending R
@@ -199,3 +199,35 @@ public func confirmation<R>(
199199
) async rethrows -> R {
200200
fatalError("Unsupported")
201201
}
202+
203+
/// An overload of ``confirmation(_:expectedCount:isolation:sourceLocation:_:)-6bkl6``
204+
/// that handles the unbounded range operator (`...`).
205+
///
206+
/// This overload is necessary because the lower bound of `PartialRangeThrough`
207+
/// is ambiguous: does it start at `0` or `1`? Test authors should specify a
208+
@available(*, unavailable, message: "Range expression '...n' is ambiguous without an explicit lower bound")
209+
public func confirmation<R>(
210+
_ comment: Comment? = nil,
211+
expectedCount: PartialRangeThrough<Int>,
212+
isolation: isolated (any Actor)? = #isolation,
213+
sourceLocation: SourceLocation = #_sourceLocation,
214+
_ body: (Confirmation) async throws -> R
215+
) async rethrows -> R {
216+
fatalError("Unsupported")
217+
}
218+
219+
/// An overload of ``confirmation(_:expectedCount:isolation:sourceLocation:_:)-6bkl6``
220+
/// that handles the unbounded range operator (`...`).
221+
///
222+
/// This overload is necessary because the lower bound of `PartialRangeUpTo` is
223+
/// ambiguous: does it start at `0` or `1`? Test authors should specify a
224+
@available(*, unavailable, message: "Range expression '..<n' is ambiguous without an explicit lower bound")
225+
public func confirmation<R>(
226+
_ comment: Comment? = nil,
227+
expectedCount: PartialRangeUpTo<Int>,
228+
isolation: isolated (any Actor)? = #isolation,
229+
sourceLocation: SourceLocation = #_sourceLocation,
230+
_ body: (Confirmation) async throws -> R
231+
) async rethrows -> R {
232+
fatalError("Unsupported")
233+
}

Sources/TestingMacros/Support/DiagnosticMessage.swift

-26
Original file line numberDiff line numberDiff line change
@@ -710,32 +710,6 @@ struct DiagnosticMessage: SwiftDiagnostics.DiagnosticMessage {
710710
)
711711
}
712712

713-
/// Create a diagnostic messages stating that a range expression constructed
714-
/// with a prefix operator (`...n` or `..<n`) is ambiguous.
715-
///
716-
/// - Parameters:
717-
/// - expr: The range expression.
718-
///
719-
/// - Returns: A diagnostic message.
720-
static func prefixRangeOperatorIsAmbiguous(_ expr: PrefixOperatorExprSyntax) -> Self {
721-
var exprCopy = expr
722-
exprCopy.operator.trailingTrivia = .space
723-
let lowerBoundPlaceholderExpr = EditorPlaceholderExprSyntax(type: "Int")
724-
let replacementExpr: ExprSyntax = "\(lowerBoundPlaceholderExpr) \(exprCopy)"
725-
726-
return Self(
727-
syntax: Syntax(expr),
728-
message: "Range expression '\(expr.trimmed)' is ambiguous without an explicit lower bound",
729-
severity: .warning,
730-
fixIts: [
731-
FixIt(
732-
message: MacroExpansionFixItMessage("Add lower bound"),
733-
changes: [.replace(oldNode: Syntax(expr), newNode: Syntax(replacementExpr)),]
734-
),
735-
]
736-
)
737-
}
738-
739713
/// Create a diagnostic message stating that a condition macro nested inside
740714
/// an exit test will not record any diagnostics.
741715
///

Sources/TestingMacros/TestDeclarationMacro.swift

-59
Original file line numberDiff line numberDiff line change
@@ -35,54 +35,6 @@ public struct TestDeclarationMacro: PeerMacro, Sendable {
3535
.disabled
3636
}
3737

38-
/// A syntax visitor that looks for uses of `...n` and `..<n` as arguments to
39-
/// `confirmation()`.
40-
private final class _AmbiguousConfirmationFinder: SyntaxVisitor {
41-
/// The set of discovered ambiguous `expectedCount` expressions.
42-
var ambiguousArgumentExprs = [PrefixOperatorExprSyntax]()
43-
44-
/// Check if a given expression is ambiguous as the `expectedCount` argument
45-
/// to `confirmation()`.
46-
///
47-
/// - Arguments:
48-
/// - argumentExpr: An expression.
49-
///
50-
/// - Returns: `argumentExpr` or a subexpression it contains if it is
51-
/// ambiguous, or `nil` if no ambiguity was found.
52-
private func _ambiguousArgumentExpr(_ argumentExpr: ExprSyntax) -> PrefixOperatorExprSyntax? {
53-
if let argumentExpr = removeParentheses(from: argumentExpr) {
54-
return _ambiguousArgumentExpr(argumentExpr)
55-
}
56-
57-
guard let argumentExpr = argumentExpr.as(PrefixOperatorExprSyntax.self) else {
58-
return nil
59-
}
60-
61-
let operatorTokenKind = argumentExpr.operator.tokenKind
62-
if operatorTokenKind == .prefixOperator("...") || operatorTokenKind == .prefixOperator("..<") {
63-
return argumentExpr
64-
}
65-
return nil
66-
}
67-
68-
override func visit(_ node: FunctionCallExprSyntax) -> SyntaxVisitorContinueKind {
69-
let calledExpr = node.calledExpression.tokens(viewMode: .fixedUp).map(\.textWithoutBackticks).joined()
70-
guard calledExpr == "confirmation" || calledExpr == "Testing.confirmation" else {
71-
return .visitChildren
72-
}
73-
74-
let expectedCountArgument = node.arguments.first { $0.label?.tokenKind == .identifier("expectedCount") }
75-
guard let argumentExpr = expectedCountArgument?.expression else {
76-
return .visitChildren
77-
}
78-
79-
if let argumentExpr = _ambiguousArgumentExpr(argumentExpr) {
80-
ambiguousArgumentExprs.append(argumentExpr)
81-
}
82-
return .visitChildren
83-
}
84-
}
85-
8638
/// Diagnose issues with a `@Test` declaration.
8739
///
8840
/// - Parameters:
@@ -166,17 +118,6 @@ public struct TestDeclarationMacro: PeerMacro, Sendable {
166118
}
167119
}
168120

169-
// Look for calls to confirmation() with a ..<n or ...n expected count
170-
// argument: we think these may be ambiguous, so we'd recommend explicitly
171-
// specifying the lower bound.
172-
if let body = function.body {
173-
let ambiguousConfirmationFinder = _AmbiguousConfirmationFinder(viewMode: .sourceAccurate)
174-
ambiguousConfirmationFinder.walk(body)
175-
for ambiguousArgumentExpr in ambiguousConfirmationFinder.ambiguousArgumentExprs {
176-
diagnostics.append(.prefixRangeOperatorIsAmbiguous(ambiguousArgumentExpr))
177-
}
178-
}
179-
180121
return !diagnostics.lazy.map(\.severity).contains(.error)
181122
}
182123

Tests/TestingMacrosTests/TestDeclarationMacroTests.swift

-8
Original file line numberDiff line numberDiff line change
@@ -256,14 +256,6 @@ struct TestDeclarationMacroTests {
256256
// .serialized on a non-parameterized test function
257257
"@Test(.serialized) func f() {}":
258258
"Trait '.serialized' has no effect when used with a non-parameterized test function",
259-
260-
// Confirmation with ambiguous lower bound
261-
"@Test func f() { confirmation(expectedCount: ...n) }":
262-
"Range expression '...n' is ambiguous without an explicit lower bound",
263-
"@Test func f() { confirmation(expectedCount: ..<n) }":
264-
"Range expression '..<n' is ambiguous without an explicit lower bound",
265-
"@Test func f() { confirmation(expectedCount: (...n)) }":
266-
"Range expression '...n' is ambiguous without an explicit lower bound",
267259
]
268260
)
269261
func apiMisuseWarnings(input: String, expectedMessage: String) throws {

Tests/TestingTests/ConfirmationTests.swift

+1-5
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,6 @@ struct UnsuccessfulConfirmationTests {
115115
1 ... 2 as any ExpectedCount,
116116
1 ..< 2,
117117
1 ..< 3,
118-
..<2,
119-
...2,
120118
999...,
121119
])
122120
func confirmedOutOfRange(_ range: any ExpectedCount) async {
@@ -131,9 +129,7 @@ struct UnsuccessfulConfirmationTests {
131129
/// Needed since we don't have generic test functions, so we need a concrete
132130
/// argument type for `confirmedOutOfRange(_:)`, but we can't write
133131
/// `any RangeExpression<Int> & Sendable`. ([96960993](rdar://96960993))
134-
protocol ExpectedCount: RangeExpression, Sendable where Bound == Int {}
132+
protocol ExpectedCount: RangeExpression, Sequence, Sendable where Bound == Int, Element == Int {}
135133
extension ClosedRange<Int>: ExpectedCount {}
136134
extension PartialRangeFrom<Int>: ExpectedCount {}
137-
extension PartialRangeThrough<Int>: ExpectedCount {}
138-
extension PartialRangeUpTo<Int>: ExpectedCount {}
139135
extension Range<Int>: ExpectedCount {}

0 commit comments

Comments
 (0)