Skip to content

Commit 89c147c

Browse files
committed
Add more test coverage, ensure that try #require(throws: Never.self) only
throws if you try to cast the result to `any Error` (can't instantiate `Never`), add a way to suppress our diagnostics in our own unit tests.
1 parent b6d7f45 commit 89c147c

File tree

9 files changed

+123
-32
lines changed

9 files changed

+123
-32
lines changed

Sources/Testing/Expectations/Expectation+Macro.swift

+1-1
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ public macro require<T>(
235235
_ comment: @autoclosure () -> Comment? = nil,
236236
sourceLocation: SourceLocation = #_sourceLocation,
237237
performing expression: () async throws -> R
238-
) -> E = #externalMacro(module: "TestingMacros", type: "RequireMacro") where E: Error
238+
) -> E = #externalMacro(module: "TestingMacros", type: "RequireThrowsMacro") where E: Error
239239

240240
/// Check that an expression never throws an error, and throw an error if it
241241
/// does.

Sources/Testing/Expectations/ExpectationChecking+Macro.swift

+8-14
Original file line numberDiff line numberDiff line change
@@ -821,18 +821,11 @@ public func __checkCast<V, T>(
821821

822822
// MARK: - Matching errors by type
823823

824-
/// A placeholder type representing `Never` if a test attempts to instantiate it
825-
/// by calling `#expect(throws:)` and taking the result.
826-
///
827-
/// Errors of this type are never thrown; they act as placeholders in `Result`
828-
/// so that `#expect(throws: Never.self)` always produces `nil` (since `Never`
829-
/// cannot be instantiated.)
830-
private struct _CannotInstantiateNeverError: Error {}
831-
832824
/// Check that an expression always throws an error.
833825
///
834826
/// This overload is used for `#expect(throws:) { }` invocations that take error
835-
/// types.
827+
/// types. It is disfavored so that `#expect(throws: Never.self)` preferentially
828+
/// returns `Void`.
836829
///
837830
/// - Warning: This function is used to implement the `#expect()` and
838831
/// `#require()` macros. Do not call it directly.
@@ -869,7 +862,8 @@ public func __checkClosureCall<E>(
869862
/// Check that an expression always throws an error.
870863
///
871864
/// This overload is used for `await #expect(throws:) { }` invocations that take
872-
/// error types.
865+
/// error types. It is disfavored so that `#expect(throws: Never.self)`
866+
/// preferentially returns `Void`.
873867
///
874868
/// - Warning: This function is used to implement the `#expect()` and
875869
/// `#require()` macros. Do not call it directly.
@@ -923,7 +917,7 @@ public func __checkClosureCall(
923917
comments: @autoclosure () -> [Comment],
924918
isRequired: Bool,
925919
sourceLocation: SourceLocation
926-
) -> Result<Never?, any Error> {
920+
) -> Result<Void, any Error> {
927921
var success = true
928922
var mismatchExplanationValue: String? = nil
929923
do {
@@ -940,7 +934,7 @@ public func __checkClosureCall(
940934
comments: comments(),
941935
isRequired: isRequired,
942936
sourceLocation: sourceLocation
943-
).map { _ in nil }
937+
).map { _ in }
944938
}
945939

946940
/// Check that an expression never throws an error.
@@ -960,7 +954,7 @@ public func __checkClosureCall(
960954
isRequired: Bool,
961955
isolation: isolated (any Actor)? = #isolation,
962956
sourceLocation: SourceLocation
963-
) async -> Result<Never?, any Error> {
957+
) async -> Result<Void, any Error> {
964958
var success = true
965959
var mismatchExplanationValue: String? = nil
966960
do {
@@ -977,7 +971,7 @@ public func __checkClosureCall(
977971
comments: comments(),
978972
isRequired: isRequired,
979973
sourceLocation: sourceLocation
980-
).map { _ in nil }
974+
).map { _ in }
981975
}
982976

983977
// MARK: - Matching instances of equatable errors

Sources/Testing/Support/Additions/ResultAdditions.swift

+22-7
Original file line numberDiff line numberDiff line change
@@ -37,17 +37,32 @@ extension Result {
3737

3838
/// Handle this instance as if it were returned from a call to `#require()`.
3939
///
40-
/// If `#require()` is used with a `__check()` function that returns an
41-
/// optional value on success, that implies that the value cannot actually be
42-
/// `nil` on success and that it's safe to unwrap it. If the value really is
43-
/// `nil` (which would be a corner case), the testing library throws an error
44-
/// representing an issue of kind ``Issue/Kind-swift.enum/apiMisused``.
40+
/// This overload of `__require()` assumes that the result cannot actually be
41+
/// `nil` on success. The optionality is part of our ABI contract for the
42+
/// `__check()` function family so that we can support uninhabited types and
43+
/// "soft" failures.
4544
///
4645
/// - Warning: This function is used to implement the `#expect()` and
4746
/// `#require()` macros. Do not call it directly.
48-
public func __required<T>() throws -> T where Success == T? {
47+
@inlinable public func __required<T>() throws -> T where Success == T? {
48+
try get()!
49+
}
50+
51+
/// Handle this instance as if it were returned from a call to `#require()`.
52+
///
53+
/// This overload of `__require()` is used by `#require(throws:)`. It has
54+
/// special handling for a `nil` result so that `Never.self` (which can't be
55+
/// instantiated) can be used as the error type in the macro call.
56+
///
57+
/// If the value really is `nil` (i.e. we're dealing with `Never`), the
58+
/// testing library throws an error representing an issue of kind
59+
/// ``Issue/Kind-swift.enum/apiMisused``.
60+
///
61+
/// - Warning: This function is used to implement the `#expect()` and
62+
/// `#require()` macros. Do not call it directly.
63+
public func __required<T>() throws -> T where T: Error, Success == T? {
4964
guard let result = try get() else {
50-
throw APIMisuseError(description: "Could not unwrap 'nil' value of type Optional<\(T.self)>. Consider using #expect() instead of #require() here.")
65+
throw APIMisuseError(description: "Could not unwrap 'nil' value of type Optional<\(T.self)>. Consider using #expect(throws:) instead of #require(throws:) here.")
5166
}
5267
return result
5368
}

Sources/TestingMacros/ConditionMacro.swift

+29
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,35 @@ public struct NonOptionalRequireMacro: RefinedConditionMacro {
332332
}
333333
}
334334

335+
/// A type describing the expansion of the `#require(throws:)` macro.
336+
///
337+
/// This macro makes a best effort to check if the type argument is `Never.self`
338+
/// (as we only have the syntax tree here) and diagnoses it as redundant if so.
339+
/// See also ``RequireThrowsNeverMacro`` which is used when full type checking
340+
/// is contextually available.
341+
///
342+
/// This type is otherwise exactly equivalent to ``RequireMacro``.
343+
public struct RequireThrowsMacro: RefinedConditionMacro {
344+
public typealias Base = RequireMacro
345+
346+
public static func expansion(
347+
of macro: some FreestandingMacroExpansionSyntax,
348+
in context: some MacroExpansionContext
349+
) throws -> ExprSyntax {
350+
if let argument = macro.arguments.first {
351+
let argumentTokens: [String] = argument.expression.tokens(viewMode: .fixedUp).lazy
352+
.filter { $0.tokenKind != .period }
353+
.map(\.textWithoutBackticks)
354+
if argumentTokens == ["Swift", "Never", "self"] || argumentTokens == ["Never", "self"] {
355+
context.diagnose(.requireThrowsNeverIsRedundant(argument.expression, in: macro))
356+
}
357+
}
358+
359+
// Perform the normal macro expansion for #require().
360+
return try RequireMacro.expansion(of: macro, in: context)
361+
}
362+
}
363+
335364
/// A type describing the expansion of the `#require(throws:)` macro when it is
336365
/// passed `Never.self`, which is redundant.
337366
///

Sources/TestingMacros/Support/Additions/MacroExpansionContextAdditions.swift

+40-10
Original file line numberDiff line numberDiff line change
@@ -80,30 +80,60 @@ extension MacroExpansionContext {
8080
// MARK: -
8181

8282
extension MacroExpansionContext {
83+
/// Whether or not our generated warnings are suppressed in the current
84+
/// lexical context.
85+
///
86+
/// The value of this property is `true` if the current lexical context
87+
/// contains a node with the `@_semantics("testing.macros.nowarnings")`
88+
/// attribute applied to it.
89+
///
90+
/// - Warning: This functionality is not part of the public interface of the
91+
/// testing library. It may be modified or removed in a future update.
92+
var areWarningsSuppressed: Bool {
93+
for lexicalContext in self.lexicalContext {
94+
guard let lexicalContext = lexicalContext.asProtocol((any WithAttributesSyntax).self) else {
95+
continue
96+
}
97+
for attribute in lexicalContext.attributes {
98+
if case let .attribute(attribute) = attribute,
99+
attribute.attributeNameText == "_semantics",
100+
case let .string(argument) = attribute.arguments,
101+
argument.representedLiteralValue == "testing.macros.nowarnings" {
102+
return true
103+
}
104+
}
105+
}
106+
return false
107+
}
108+
83109
/// Emit a diagnostic message.
84110
///
85111
/// - Parameters:
86112
/// - message: The diagnostic message to emit. The `node` and `position`
87113
/// arguments to `Diagnostic.init()` are derived from the message's
88114
/// `syntax` property.
89115
func diagnose(_ message: DiagnosticMessage) {
90-
diagnose(
91-
Diagnostic(
92-
node: message.syntax,
93-
position: message.syntax.positionAfterSkippingLeadingTrivia,
94-
message: message,
95-
fixIts: message.fixIts
96-
)
97-
)
116+
diagnose(CollectionOfOne(message))
98117
}
99118

100119
/// Emit a sequence of diagnostic messages.
101120
///
102121
/// - Parameters:
103122
/// - messages: The diagnostic messages to emit.
104-
func diagnose(_ messages: some Sequence<DiagnosticMessage>) {
123+
func diagnose(_ messages: some Collection<DiagnosticMessage>) {
124+
lazy var areWarningsSuppressed = areWarningsSuppressed
105125
for message in messages {
106-
diagnose(message)
126+
if message.severity == .warning && areWarningsSuppressed {
127+
continue
128+
}
129+
diagnose(
130+
Diagnostic(
131+
node: message.syntax,
132+
position: message.syntax.positionAfterSkippingLeadingTrivia,
133+
message: message,
134+
fixIts: message.fixIts
135+
)
136+
)
107137
}
108138
}
109139

Sources/TestingMacros/TestingMacrosMain.swift

+1
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+
RequireThrowsMacro.self,
2728
RequireThrowsNeverMacro.self,
2829
ExitTestExpectMacro.self,
2930
ExitTestRequireMacro.self,

Tests/TestingMacrosTests/ConditionMacroTests.swift

+2
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,8 @@ struct ConditionMacroTests {
354354

355355
@Test("#require(throws: Never.self) produces a diagnostic",
356356
arguments: [
357+
"#requireThrows(throws: Swift.Never.self)",
358+
"#requireThrows(throws: Never.self)",
357359
"#requireThrowsNever(throws: Never.self)",
358360
]
359361
)

Tests/TestingMacrosTests/TestSupport/Parse.swift

+1
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+
"requireThrows": RequireThrowsMacro.self, // different name needed only for unit testing
2627
"requireThrowsNever": RequireThrowsNeverMacro.self, // different name needed only for unit testing
2728
"expectExitTest": ExitTestRequireMacro.self, // different name needed only for unit testing
2829
"requireExitTest": ExitTestRequireMacro.self, // different name needed only for unit testing

Tests/TestingTests/IssueTests.swift

+19
Original file line numberDiff line numberDiff line change
@@ -992,6 +992,25 @@ final class IssueTests: XCTestCase {
992992
await fulfillment(of: [errorCaught, apiMisused, expectationFailed], timeout: 0.0)
993993
}
994994

995+
@_semantics("testing.macros.nowarnings")
996+
func testErrorCheckingWithRequire_ResultValueIsNever_VariousSyntaxes() throws {
997+
// Basic expressions succeed and don't diagnose.
998+
#expect(throws: Never.self) {}
999+
try #require(throws: Never.self) {}
1000+
1001+
// Casting to specific types succeeds and doesn't diagnose.
1002+
let _: Void = try #require(throws: Never.self) {}
1003+
let _: Any = try #require(throws: Never.self) {}
1004+
1005+
// Casting to any Error throws an API misuse error because Never cannot be
1006+
// instantiated. NOTE: inner function needed for lexical context.
1007+
@_semantics("testing.macros.nowarnings")
1008+
func castToAnyError() throws {
1009+
let _: any Error = try #require(throws: Never.self) {}
1010+
}
1011+
#expect(throws: APIMisuseError.self, performing: castToAnyError)
1012+
}
1013+
9951014
func testFail() async throws {
9961015
var configuration = Configuration()
9971016
configuration.eventHandler = { event, _ in

0 commit comments

Comments
 (0)