diff --git a/Sources/Testing/Expectations/Expectation+Macro.swift b/Sources/Testing/Expectations/Expectation+Macro.swift index 1df9acc3b..695dc7411 100644 --- a/Sources/Testing/Expectations/Expectation+Macro.swift +++ b/Sources/Testing/Expectations/Expectation+Macro.swift @@ -158,26 +158,12 @@ public macro require( /// 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( - 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) { @@ -195,17 +181,12 @@ public macro require( /// 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( - throws _: Never.Type, +@freestanding(expression) public macro expect( + 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. @@ -260,13 +241,14 @@ public macro require( /// /// - 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( +@freestanding(expression) +@_documentation(visibility: private) +public macro require( 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 @@ -294,9 +276,7 @@ public macro require( /// 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( throws error: E, _ comment: @autoclosure () -> Comment? = nil, @@ -375,9 +355,7 @@ public macro require( /// 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( _ comment: @autoclosure () -> Comment? = nil, sourceLocation: SourceLocation = #_sourceLocation, diff --git a/Sources/Testing/Testing.docc/Expectations.md b/Sources/Testing/Testing.docc/Expectations.md index 92185876a..083e09c64 100644 --- a/Sources/Testing/Testing.docc/Expectations.md +++ b/Sources/Testing/Testing.docc/Expectations.md @@ -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 diff --git a/Sources/TestingMacros/ConditionMacro.swift b/Sources/TestingMacros/ConditionMacro.swift index e6f4d1cdf..adb940950 100644 --- a/Sources/TestingMacros/ConditionMacro.swift +++ b/Sources/TestingMacros/ConditionMacro.swift @@ -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 diff --git a/Sources/TestingMacros/Support/DiagnosticMessage.swift b/Sources/TestingMacros/Support/DiagnosticMessage.swift index 8fa2b5519..7d8a83c20 100644 --- a/Sources/TestingMacros/Support/DiagnosticMessage.swift +++ b/Sources/TestingMacros/Support/DiagnosticMessage.swift @@ -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. /// diff --git a/Sources/TestingMacros/TestingMacrosMain.swift b/Sources/TestingMacros/TestingMacrosMain.swift index 8603a2031..ebc62d660 100644 --- a/Sources/TestingMacros/TestingMacrosMain.swift +++ b/Sources/TestingMacros/TestingMacrosMain.swift @@ -24,6 +24,7 @@ struct TestingMacrosMain: CompilerPlugin { RequireMacro.self, AmbiguousRequireMacro.self, NonOptionalRequireMacro.self, + RequireThrowsNeverMacro.self, ExitTestExpectMacro.self, ExitTestRequireMacro.self, TagMacro.self, diff --git a/Tests/TestingMacrosTests/ConditionMacroTests.swift b/Tests/TestingMacrosTests/ConditionMacroTests.swift index 070483a78..e29a4a33d 100644 --- a/Tests/TestingMacrosTests/ConditionMacroTests.swift +++ b/Tests/TestingMacrosTests/ConditionMacroTests.swift @@ -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: [ diff --git a/Tests/TestingMacrosTests/TestSupport/Parse.swift b/Tests/TestingMacrosTests/TestSupport/Parse.swift index 4fcfb22c3..fcb0215bc 100644 --- a/Tests/TestingMacrosTests/TestSupport/Parse.swift +++ b/Tests/TestingMacrosTests/TestSupport/Parse.swift @@ -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,