Skip to content

Remove #expect(throws: Never.self) and #require(throws: Never.self) as distinct overloads. #661

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 1 commit into from
Sep 4, 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
48 changes: 13 additions & 35 deletions Sources/Testing/Expectations/Expectation+Macro.swift
Original file line number Diff line number Diff line change
Expand Up @@ -158,26 +158,12 @@ public macro require<T>(
/// discarded.
///
/// If the thrown error need only equal another instance of [`Error`](https://developer.apple.com/documentation/swift/error),
/// use ``expect(throws:_:sourceLocation:performing:)-1xr34`` instead. If
/// `expression` should _never_ throw any error, use
/// ``expect(throws:_:sourceLocation:performing:)-5lzjz`` instead.
@freestanding(expression) public macro expect<E, R>(
throws errorType: E.Type,
_ comment: @autoclosure () -> Comment? = nil,
sourceLocation: SourceLocation = #_sourceLocation,
performing expression: () async throws -> R
) = #externalMacro(module: "TestingMacros", type: "ExpectMacro") where E: Error

/// Check that an expression never throws an error.
/// use ``expect(throws:_:sourceLocation:performing:)-1xr34`` instead.
///
/// - Parameters:
/// - comment: A comment describing the expectation.
/// - sourceLocation: The source location to which recorded expectations and
/// issues should be attributed.
/// - expression: The expression to be evaluated.
/// ## Expressions that should never throw
///
/// Use this overload of `#expect()` when the expression `expression` should
/// _never_ throw any error:
/// If the expression `expression` should _never_ throw any error, you can pass
/// [`Never.self`](https://developer.apple.com/documentation/swift/never):
///
/// ```swift
/// #expect(throws: Never.self) {
Expand All @@ -195,17 +181,12 @@ public macro require<T>(
/// fail when an error is thrown by `expression`, rather than to explicitly
/// check that an error is _not_ thrown by it, do not use this macro. Instead,
/// simply call the code in question and allow it to throw an error naturally.
///
/// If the thrown error need only be an instance of a particular type, use
/// ``expect(throws:_:sourceLocation:performing:)-79piu`` instead. If the thrown
/// error need only equal another instance of [`Error`](https://developer.apple.com/documentation/swift/error),
/// use ``expect(throws:_:sourceLocation:performing:)-1xr34`` instead.
@freestanding(expression) public macro expect<R>(
throws _: Never.Type,
@freestanding(expression) public macro expect<E, R>(
throws errorType: E.Type,
_ comment: @autoclosure () -> Comment? = nil,
sourceLocation: SourceLocation = #_sourceLocation,
performing expression: () async throws -> R
) = #externalMacro(module: "TestingMacros", type: "ExpectMacro")
) = #externalMacro(module: "TestingMacros", type: "ExpectMacro") where E: Error

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

// MARK: - Matching instances of equatable errors

Expand Down Expand Up @@ -294,9 +276,7 @@ public macro require<T>(
/// in the current task. Any value returned by `expression` is discarded.
///
/// If the thrown error need only be an instance of a particular type, use
/// ``expect(throws:_:sourceLocation:performing:)-79piu`` instead. If
/// `expression` should _never_ throw any error, use
/// ``expect(throws:_:sourceLocation:performing:)-5lzjz`` instead.
/// ``expect(throws:_:sourceLocation:performing:)-79piu`` instead.
@freestanding(expression) public macro expect<E, R>(
throws error: E,
_ comment: @autoclosure () -> Comment? = nil,
Expand Down Expand Up @@ -375,9 +355,7 @@ public macro require<T>(
/// If the thrown error need only be an instance of a particular type, use
/// ``expect(throws:_:sourceLocation:performing:)-79piu`` instead. If the thrown
/// error need only equal another instance of [`Error`](https://developer.apple.com/documentation/swift/error),
/// use ``expect(throws:_:sourceLocation:performing:)-1xr34`` instead. If an
/// error should _never_ be thrown, use
/// ``expect(throws:_:sourceLocation:performing:)-5lzjz`` instead.
/// use ``expect(throws:_:sourceLocation:performing:)-1xr34`` instead.
@freestanding(expression) public macro expect<R>(
_ comment: @autoclosure () -> Comment? = nil,
sourceLocation: SourceLocation = #_sourceLocation,
Expand Down
2 changes: 0 additions & 2 deletions Sources/Testing/Testing.docc/Expectations.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,9 @@ the test when the code doesn't satisfy a requirement, use
- ``expect(throws:_:sourceLocation:performing:)-79piu``
- ``expect(throws:_:sourceLocation:performing:)-1xr34``
- ``expect(_:sourceLocation:performing:throws:)``
- ``expect(throws:_:sourceLocation:performing:)-5lzjz``
- ``require(throws:_:sourceLocation:performing:)-76bjn``
- ``require(throws:_:sourceLocation:performing:)-7v83e``
- ``require(_:sourceLocation:performing:throws:)``
- ``require(throws:_:sourceLocation:performing:)-36uzc``

### Confirming that asynchronous events occur

Expand Down
20 changes: 20 additions & 0 deletions Sources/TestingMacros/ConditionMacro.swift
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,26 @@ public struct NonOptionalRequireMacro: RefinedConditionMacro {
}
}

/// A type describing the expansion of the `#require(throws:)` macro when it is
/// passed `Never.self`, which is redundant.
///
/// This type is otherwise exactly equivalent to ``RequireMacro``.
public struct RequireThrowsNeverMacro: RefinedConditionMacro {
public typealias Base = RequireMacro

public static func expansion(
of macro: some FreestandingMacroExpansionSyntax,
in context: some MacroExpansionContext
) throws -> ExprSyntax {
if let argument = macro.arguments.first {
context.diagnose(.requireThrowsNeverIsRedundant(argument.expression, in: macro))
}

// Perform the normal macro expansion for #require().
return try RequireMacro.expansion(of: macro, in: context)
}
}

// MARK: -

/// A syntax visitor that looks for uses of `#expect()` and `#require()` nested
Expand Down
20 changes: 20 additions & 0 deletions Sources/TestingMacros/Support/DiagnosticMessage.swift
Original file line number Diff line number Diff line change
Expand Up @@ -690,6 +690,26 @@ struct DiagnosticMessage: SwiftDiagnostics.DiagnosticMessage {
)
}

/// Create a diagnostic messages stating that `#require(throws: Never.self)`
/// is redundant.
///
/// - Parameters:
/// - expr: The error type expression.
///
/// - Returns: A diagnostic message.
static func requireThrowsNeverIsRedundant(_ expr: ExprSyntax, in macro: some FreestandingMacroExpansionSyntax) -> Self {
// We do not provide fix-its because we cannot see the leading "try" keyword
// so we can't provide a valid fix-it to remove the macro either. We can
// provide a fix-it to add "as Optional", but only providing that fix-it may
// confuse or mislead developers (and that's presumably usually the *wrong*
// fix-it to select anyway.)
Self(
syntax: Syntax(expr),
message: "Passing '\(expr.trimmed)' to \(_macroName(macro)) is redundant; invoke non-throwing test code directly instead",
severity: .warning
)
}

/// Create a diagnostic message stating that a condition macro nested inside
/// an exit test will not record any diagnostics.
///
Expand Down
1 change: 1 addition & 0 deletions Sources/TestingMacros/TestingMacrosMain.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ struct TestingMacrosMain: CompilerPlugin {
RequireMacro.self,
AmbiguousRequireMacro.self,
NonOptionalRequireMacro.self,
RequireThrowsNeverMacro.self,
ExitTestExpectMacro.self,
ExitTestRequireMacro.self,
TagMacro.self,
Expand Down
13 changes: 13 additions & 0 deletions Tests/TestingMacrosTests/ConditionMacroTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,19 @@ struct ConditionMacroTests {
#expect(diagnostic.message.contains("is redundant"))
}

@Test("#require(throws: Never.self) produces a diagnostic",
arguments: [
"#requireThrowsNever(throws: Never.self)",
]
)
func requireThrowsNeverProducesDiagnostic(input: String) throws {
let (_, diagnostics) = try parse(input)

let diagnostic = try #require(diagnostics.first)
#expect(diagnostic.diagMessage.severity == .warning)
#expect(diagnostic.message.contains("is redundant"))
}

#if !SWT_NO_EXIT_TESTS
@Test("Expectation inside an exit test diagnoses",
arguments: [
Expand Down
1 change: 1 addition & 0 deletions Tests/TestingMacrosTests/TestSupport/Parse.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ fileprivate let allMacros: [String: any Macro.Type] = [
"require": RequireMacro.self,
"requireAmbiguous": AmbiguousRequireMacro.self, // different name needed only for unit testing
"requireNonOptional": NonOptionalRequireMacro.self, // different name needed only for unit testing
"requireThrowsNever": RequireThrowsNeverMacro.self, // different name needed only for unit testing
"expectExitTest": ExitTestRequireMacro.self, // different name needed only for unit testing
"requireExitTest": ExitTestRequireMacro.self, // different name needed only for unit testing
"Suite": SuiteDeclarationMacro.self,
Expand Down