Warn when passing a non-optional value to try #require(T?). (#620)

This PR adds an overload of `try #require()` that warns the developer if
they pass a non-optional, non-`Bool` value For example, this code:

```swift
let x = 0
let y = try #require(x)
```

Will produce the diagnostic:

> ⚠️ '#require(\_:\_:)' is redundant because 'x' never equals 'nil'

### 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.

Author: grynspan

Sources/Testing/Expectations/Expectation+Macro.swift

@@ -101,6 +101,34 @@ public macro require(
   sourceLocation: SourceLocation = #_sourceLocation
 ) -> Bool = #externalMacro(module: "TestingMacros", type: "AmbiguousRequireMacro")
 
+/// Unwrap an optional value or, if it is `nil`, fail and throw an error.
+///
+/// - Parameters:
+///   - optionalValue: The optional value to be unwrapped.
+///   - comment: A comment describing the expectation.
+///   - sourceLocation: The source location to which recorded expectations and
+///     issues should be attributed.
+///
+/// - Returns: The unwrapped value of `optionalValue`.
+///
+/// - Throws: An instance of ``ExpectationFailedError`` if `optionalValue` is
+///   `nil`.
+///
+/// If `optionalValue` is `nil`, an ``Issue`` is recorded for the test that is
+/// running in the current task and an instance of ``ExpectationFailedError`` is
+/// thrown.
+///
+/// This overload of ``require(_:_:sourceLocation:)-6w9oo`` is used when a
+/// non-optional, non-`Bool` value is passed to `#require()`. It emits a warning
+/// diagnostic indicating that the expectation is redundant.
+@freestanding(expression)
+@_documentation(visibility: private)
+public macro require<T>(
+  _ optionalValue: T,
+  _ comment: @autoclosure () -> Comment? = nil,
+  sourceLocation: SourceLocation = #_sourceLocation
+) -> T = #externalMacro(module: "TestingMacros", type: "NonOptionalRequireMacro")
+
 // MARK: - Matching errors by type
 
 /// Check that an expression always throws an error of a given type.

Sources/TestingMacros/ConditionMacro.swift

@@ -293,6 +293,26 @@ public struct AmbiguousRequireMacro: RefinedConditionMacro {
   }
 }
 
+/// A type describing the expansion of the `#require()` macro when it is passed
+/// a non-optional, non-`Bool` value.
+///
+/// This type is otherwise exactly equivalent to ``RequireMacro``.
+public struct NonOptionalRequireMacro: 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(.nonOptionalRequireIsRedundant(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

Sources/TestingMacros/Support/DiagnosticMessage.swift

@@ -670,6 +670,26 @@ struct DiagnosticMessage: SwiftDiagnostics.DiagnosticMessage {
     )
   }
 
+  /// Create a diagnostic messages stating that the expression passed to
+  /// `#require()` is not optional and the macro is redundant.
+  ///
+  /// - Parameters:
+  ///   - expr: The non-optional expression.
+  ///
+  /// - Returns: A diagnostic message.
+  static func nonOptionalRequireIsRedundant(_ 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: "\(_macroName(macro)) is redundant because '\(expr.trimmed)' never equals 'nil'",
+      severity: .warning
+    )
+  }
+
   /// Create a diagnostic message stating that a condition macro nested inside
   /// an exit test will not record any diagnostics.
   ///

Sources/TestingMacros/TestingMacrosMain.swift

@@ -23,6 +23,7 @@ struct TestingMacrosMain: CompilerPlugin {
       ExpectMacro.self,
       RequireMacro.self,
       AmbiguousRequireMacro.self,
+      NonOptionalRequireMacro.self,
       ExitTestExpectMacro.self,
       ExitTestRequireMacro.self,
       TagMacro.self,

Tests/TestingMacrosTests/ConditionMacroTests.swift

@@ -331,6 +331,19 @@ struct ConditionMacroTests {
     #expect(diagnostics.isEmpty)
   }
 
+  @Test("#require(non-optional value) produces a diagnostic",
+    arguments: [
+      "#requireNonOptional(expression)",
+    ]
+  )
+  func requireNonOptionalProducesDiagnostic(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: [

Tests/TestingMacrosTests/TestSupport/Parse.swift

@@ -22,6 +22,7 @@ fileprivate let allMacros: [String: any Macro.Type] = [
   "expect": ExpectMacro.self,
   "require": RequireMacro.self,
   "requireAmbiguous": AmbiguousRequireMacro.self, // different name needed only for unit testing
+  "requireNonOptional": NonOptionalRequireMacro.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,

Tests/TestingTests/BacktraceTests.swift

@@ -34,8 +34,8 @@ struct BacktraceTests {
   }
 
   @Test("Backtrace.current() is populated")
-  func currentBacktrace() throws {
-    let backtrace = try #require(Backtrace.current())
+  func currentBacktrace() {
+    let backtrace = Backtrace.current()
     #expect(!backtrace.addresses.isEmpty)
   }
 

Tests/TestingTests/IssueTests.swift

@@ -113,7 +113,7 @@ final class IssueTests: XCTestCase {
 
     await Test {
       let x: String? = nil
-      _ = try #require(x ?? "hello")
+      _ = try #require(x ?? ("hello" as String?))
     }.run(configuration: configuration)
   }
 

Tests/TestingTests/MiscellaneousTests.swift

@@ -369,9 +369,8 @@ struct MiscellaneousTests {
       #expect(firstParameter.index == 0)
       #expect(firstParameter.firstName == "i")
       #expect(firstParameter.secondName == nil)
-      let firstParameterTypeInfo = try #require(firstParameter.typeInfo)
-      #expect(firstParameterTypeInfo.fullyQualifiedName == "Swift.Int")
-      #expect(firstParameterTypeInfo.unqualifiedName == "Int")
+      #expect(firstParameter.typeInfo.fullyQualifiedName == "Swift.Int")
+      #expect(firstParameter.typeInfo.unqualifiedName == "Int")
     } catch {}
 
     do {
@@ -386,9 +385,8 @@ struct MiscellaneousTests {
       #expect(secondParameter.index == 1)
       #expect(secondParameter.firstName == "j")
       #expect(secondParameter.secondName == "k")
-      let secondParameterTypeInfo = try #require(secondParameter.typeInfo)
-      #expect(secondParameterTypeInfo.fullyQualifiedName == "Swift.String")
-      #expect(secondParameterTypeInfo.unqualifiedName == "String")
+      #expect(secondParameter.typeInfo.fullyQualifiedName == "Swift.String")
+      #expect(secondParameter.typeInfo.unqualifiedName == "String")
     } catch {}
   }