Skip to content

Commit 9de9084

Browse files
authored
Remove #expect(throws: Never.self) and #require(throws: Never.self) as distinct overloads. (#661)
`#expect(throws: Never.self)` and `#require(throws: Never.self)` do not need to be specified as separate overloads. This PR removes them as distinct symbols: - The documentation for `#expect(throws: Never.self)` is merged into the documentation for `#expect(throws:)`. - The diagnostic emitted when using `#require(throws: Never.self)` is lowered from a deprecation warning to a macro-generated custom warning. This change has no other compile-time or runtime effects: code that passes `Never.self` to either macro will continue to compile, and macros cannot be referenced so there is no risk of source-level breakage like there might be from a function signature change. ### 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.
1 parent 5720222 commit 9de9084

File tree

7 files changed

+68
-37
lines changed

7 files changed

+68
-37
lines changed

Sources/Testing/Expectations/Expectation+Macro.swift

Lines changed: 13 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -158,26 +158,12 @@ public macro require<T>(
158158
/// discarded.
159159
///
160160
/// If the thrown error need only equal another instance of [`Error`](https://developer.apple.com/documentation/swift/error),
161-
/// use ``expect(throws:_:sourceLocation:performing:)-1xr34`` instead. If
162-
/// `expression` should _never_ throw any error, use
163-
/// ``expect(throws:_:sourceLocation:performing:)-5lzjz`` instead.
164-
@freestanding(expression) public macro expect<E, R>(
165-
throws errorType: E.Type,
166-
_ comment: @autoclosure () -> Comment? = nil,
167-
sourceLocation: SourceLocation = #_sourceLocation,
168-
performing expression: () async throws -> R
169-
) = #externalMacro(module: "TestingMacros", type: "ExpectMacro") where E: Error
170-
171-
/// Check that an expression never throws an error.
161+
/// use ``expect(throws:_:sourceLocation:performing:)-1xr34`` instead.
172162
///
173-
/// - Parameters:
174-
/// - comment: A comment describing the expectation.
175-
/// - sourceLocation: The source location to which recorded expectations and
176-
/// issues should be attributed.
177-
/// - expression: The expression to be evaluated.
163+
/// ## Expressions that should never throw
178164
///
179-
/// Use this overload of `#expect()` when the expression `expression` should
180-
/// _never_ throw any error:
165+
/// If the expression `expression` should _never_ throw any error, you can pass
166+
/// [`Never.self`](https://developer.apple.com/documentation/swift/never):
181167
///
182168
/// ```swift
183169
/// #expect(throws: Never.self) {
@@ -195,17 +181,12 @@ public macro require<T>(
195181
/// fail when an error is thrown by `expression`, rather than to explicitly
196182
/// check that an error is _not_ thrown by it, do not use this macro. Instead,
197183
/// simply call the code in question and allow it to throw an error naturally.
198-
///
199-
/// If the thrown error need only be an instance of a particular type, use
200-
/// ``expect(throws:_:sourceLocation:performing:)-79piu`` instead. If the thrown
201-
/// error need only equal another instance of [`Error`](https://developer.apple.com/documentation/swift/error),
202-
/// use ``expect(throws:_:sourceLocation:performing:)-1xr34`` instead.
203-
@freestanding(expression) public macro expect<R>(
204-
throws _: Never.Type,
184+
@freestanding(expression) public macro expect<E, R>(
185+
throws errorType: E.Type,
205186
_ comment: @autoclosure () -> Comment? = nil,
206187
sourceLocation: SourceLocation = #_sourceLocation,
207188
performing expression: () async throws -> R
208-
) = #externalMacro(module: "TestingMacros", type: "ExpectMacro")
189+
) = #externalMacro(module: "TestingMacros", type: "ExpectMacro") where E: Error
209190

210191
/// Check that an expression always throws an error of a given type, and throw
211192
/// an error if it does not.
@@ -260,13 +241,14 @@ public macro require<T>(
260241
///
261242
/// - Throws: An instance of ``ExpectationFailedError`` if `expression` throws
262243
/// any error. The error thrown by `expression` is not rethrown.
263-
@available(*, deprecated, message: "try #require(throws: Never.self) is redundant. Invoke non-throwing test code directly instead.")
264-
@freestanding(expression) public macro require<R>(
244+
@freestanding(expression)
245+
@_documentation(visibility: private)
246+
public macro require<R>(
265247
throws _: Never.Type,
266248
_ comment: @autoclosure () -> Comment? = nil,
267249
sourceLocation: SourceLocation = #_sourceLocation,
268250
performing expression: () async throws -> R
269-
) = #externalMacro(module: "TestingMacros", type: "RequireMacro")
251+
) = #externalMacro(module: "TestingMacros", type: "RequireThrowsNeverMacro")
270252

271253
// MARK: - Matching instances of equatable errors
272254

@@ -294,9 +276,7 @@ public macro require<T>(
294276
/// in the current task. Any value returned by `expression` is discarded.
295277
///
296278
/// If the thrown error need only be an instance of a particular type, use
297-
/// ``expect(throws:_:sourceLocation:performing:)-79piu`` instead. If
298-
/// `expression` should _never_ throw any error, use
299-
/// ``expect(throws:_:sourceLocation:performing:)-5lzjz`` instead.
279+
/// ``expect(throws:_:sourceLocation:performing:)-79piu`` instead.
300280
@freestanding(expression) public macro expect<E, R>(
301281
throws error: E,
302282
_ comment: @autoclosure () -> Comment? = nil,
@@ -375,9 +355,7 @@ public macro require<T>(
375355
/// If the thrown error need only be an instance of a particular type, use
376356
/// ``expect(throws:_:sourceLocation:performing:)-79piu`` instead. If the thrown
377357
/// error need only equal another instance of [`Error`](https://developer.apple.com/documentation/swift/error),
378-
/// use ``expect(throws:_:sourceLocation:performing:)-1xr34`` instead. If an
379-
/// error should _never_ be thrown, use
380-
/// ``expect(throws:_:sourceLocation:performing:)-5lzjz`` instead.
358+
/// use ``expect(throws:_:sourceLocation:performing:)-1xr34`` instead.
381359
@freestanding(expression) public macro expect<R>(
382360
_ comment: @autoclosure () -> Comment? = nil,
383361
sourceLocation: SourceLocation = #_sourceLocation,

Sources/Testing/Testing.docc/Expectations.md

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,9 @@ the test when the code doesn't satisfy a requirement, use
6868
- ``expect(throws:_:sourceLocation:performing:)-79piu``
6969
- ``expect(throws:_:sourceLocation:performing:)-1xr34``
7070
- ``expect(_:sourceLocation:performing:throws:)``
71-
- ``expect(throws:_:sourceLocation:performing:)-5lzjz``
7271
- ``require(throws:_:sourceLocation:performing:)-76bjn``
7372
- ``require(throws:_:sourceLocation:performing:)-7v83e``
7473
- ``require(_:sourceLocation:performing:throws:)``
75-
- ``require(throws:_:sourceLocation:performing:)-36uzc``
7674

7775
### Confirming that asynchronous events occur
7876

Sources/TestingMacros/ConditionMacro.swift

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,26 @@ public struct NonOptionalRequireMacro: RefinedConditionMacro {
328328
}
329329
}
330330

331+
/// A type describing the expansion of the `#require(throws:)` macro when it is
332+
/// passed `Never.self`, which is redundant.
333+
///
334+
/// This type is otherwise exactly equivalent to ``RequireMacro``.
335+
public struct RequireThrowsNeverMacro: RefinedConditionMacro {
336+
public typealias Base = RequireMacro
337+
338+
public static func expansion(
339+
of macro: some FreestandingMacroExpansionSyntax,
340+
in context: some MacroExpansionContext
341+
) throws -> ExprSyntax {
342+
if let argument = macro.arguments.first {
343+
context.diagnose(.requireThrowsNeverIsRedundant(argument.expression, in: macro))
344+
}
345+
346+
// Perform the normal macro expansion for #require().
347+
return try RequireMacro.expansion(of: macro, in: context)
348+
}
349+
}
350+
331351
// MARK: -
332352

333353
/// A syntax visitor that looks for uses of `#expect()` and `#require()` nested

Sources/TestingMacros/Support/DiagnosticMessage.swift

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -690,6 +690,26 @@ struct DiagnosticMessage: SwiftDiagnostics.DiagnosticMessage {
690690
)
691691
}
692692

693+
/// Create a diagnostic messages stating that `#require(throws: Never.self)`
694+
/// is redundant.
695+
///
696+
/// - Parameters:
697+
/// - expr: The error type expression.
698+
///
699+
/// - Returns: A diagnostic message.
700+
static func requireThrowsNeverIsRedundant(_ expr: ExprSyntax, in macro: some FreestandingMacroExpansionSyntax) -> Self {
701+
// We do not provide fix-its because we cannot see the leading "try" keyword
702+
// so we can't provide a valid fix-it to remove the macro either. We can
703+
// provide a fix-it to add "as Optional", but only providing that fix-it may
704+
// confuse or mislead developers (and that's presumably usually the *wrong*
705+
// fix-it to select anyway.)
706+
Self(
707+
syntax: Syntax(expr),
708+
message: "Passing '\(expr.trimmed)' to \(_macroName(macro)) is redundant; invoke non-throwing test code directly instead",
709+
severity: .warning
710+
)
711+
}
712+
693713
/// Create a diagnostic message stating that a condition macro nested inside
694714
/// an exit test will not record any diagnostics.
695715
///

Sources/TestingMacros/TestingMacrosMain.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ struct TestingMacrosMain: CompilerPlugin {
2424
RequireMacro.self,
2525
AmbiguousRequireMacro.self,
2626
NonOptionalRequireMacro.self,
27+
RequireThrowsNeverMacro.self,
2728
ExitTestExpectMacro.self,
2829
ExitTestRequireMacro.self,
2930
TagMacro.self,

Tests/TestingMacrosTests/ConditionMacroTests.swift

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,19 @@ struct ConditionMacroTests {
352352
#expect(diagnostic.message.contains("is redundant"))
353353
}
354354

355+
@Test("#require(throws: Never.self) produces a diagnostic",
356+
arguments: [
357+
"#requireThrowsNever(throws: Never.self)",
358+
]
359+
)
360+
func requireThrowsNeverProducesDiagnostic(input: String) throws {
361+
let (_, diagnostics) = try parse(input)
362+
363+
let diagnostic = try #require(diagnostics.first)
364+
#expect(diagnostic.diagMessage.severity == .warning)
365+
#expect(diagnostic.message.contains("is redundant"))
366+
}
367+
355368
#if !SWT_NO_EXIT_TESTS
356369
@Test("Expectation inside an exit test diagnoses",
357370
arguments: [

Tests/TestingMacrosTests/TestSupport/Parse.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ fileprivate let allMacros: [String: any Macro.Type] = [
2323
"require": RequireMacro.self,
2424
"requireAmbiguous": AmbiguousRequireMacro.self, // different name needed only for unit testing
2525
"requireNonOptional": NonOptionalRequireMacro.self, // different name needed only for unit testing
26+
"requireThrowsNever": RequireThrowsNeverMacro.self, // different name needed only for unit testing
2627
"expectExitTest": ExitTestRequireMacro.self, // different name needed only for unit testing
2728
"requireExitTest": ExitTestRequireMacro.self, // different name needed only for unit testing
2829
"Suite": SuiteDeclarationMacro.self,

0 commit comments

Comments
 (0)