IssueHandlingTrait refinements: Rename to 'compactMapIssues' and special-case system issues (#1136)

Refinements to `IssueHandlingTrait` based on
[pitch](https://forums.swift.org/t/pitch-issue-handling-traits/80019)
feedback.

### Modifications:

- Rename the `transformIssues(_:)` function to `compactMapIssues(_:)` to
align better with `filterIssues(_:)`.
- Ignore issues for which the value of `kind` is `.system`.
- Prohibit returning an issue from the closure passed to
`compactMapIssues(_:)` whose `kind` is `.system`.
- Adjust documentation and tests.

### 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: stmontgomery

Sources/Testing/Testing.docc/Traits.md

@@ -51,7 +51,7 @@ types that customize the behavior of your tests.
 <!--
 ### Handling issues
 
-- ``Trait/transformIssues(_:)``
+- ``Trait/compactMapIssues(_:)``
 - ``Trait/filterIssues(_:)``
 -->
 

Sources/Testing/Traits/IssueHandlingTrait.swift

@@ -15,14 +15,14 @@
 /// modifying one or more of its properties, and returning the copy. You can
 /// observe recorded issues by returning them unmodified. Or you can suppress an
 /// issue by either filtering it using ``Trait/filterIssues(_:)`` or returning
-/// `nil` from the closure passed to ``Trait/transformIssues(_:)``.
+/// `nil` from the closure passed to ``Trait/compactMapIssues(_:)``.
 ///
 /// When an instance of this trait is applied to a suite, it is recursively
 /// inherited by all child suites and tests.
 ///
 /// To add this trait to a test, use one of the following functions:
 ///
-/// - ``Trait/transformIssues(_:)``
+/// - ``Trait/compactMapIssues(_:)``
 /// - ``Trait/filterIssues(_:)``
 @_spi(Experimental)
 public struct IssueHandlingTrait: TestTrait, SuiteTrait {
@@ -96,15 +96,26 @@ extension IssueHandlingTrait: TestScoping {
         return
       }
 
+      // Ignore system issues, as they are not expected to be caused by users.
+      if case .system = issue.kind {
+        oldConfiguration.eventHandler(event, context)
+        return
+      }
+
       // Use the original configuration's event handler when invoking the
-      // transformer to avoid infinite recursion if the transformer itself
+      // handler closure to avoid infinite recursion if the handler itself
       // records new issues. This means only issue handling traits whose scope
       // is outside this one will be allowed to handle such issues.
       let newIssue = Configuration.withCurrent(oldConfiguration) {
         handleIssue(issue)
       }
 
       if let newIssue {
+        // Prohibit assigning the issue's kind to system.
+        if case .system = newIssue.kind {
+          preconditionFailure("Issue returned by issue handling closure cannot have kind 'system': \(newIssue)")
+        }
+
         var event = event
         event.kind = .issueRecorded(newIssue)
         oldConfiguration.eventHandler(event, context)
@@ -120,31 +131,35 @@ extension Trait where Self == IssueHandlingTrait {
   /// Constructs an trait that transforms issues recorded by a test.
   ///
   /// - Parameters:
-  ///   - transformer: The closure called for each issue recorded by the test
+  ///   - transform: A closure called for each issue recorded by the test
   ///     this trait is applied to. It is passed a recorded issue, and returns
   ///     an optional issue to replace the passed-in one.
   ///
   /// - Returns: An instance of ``IssueHandlingTrait`` that transforms issues.
   ///
-  /// The `transformer` closure is called synchronously each time an issue is
+  /// The `transform` closure is called synchronously each time an issue is
   /// recorded by the test this trait is applied to. The closure is passed the
   /// recorded issue, and if it returns a non-`nil` value, that will be recorded
   /// instead of the original. Otherwise, if the closure returns `nil`, the
   /// issue is suppressed and will not be included in the results.
   ///
-  /// The `transformer` closure may be called more than once if the test records
+  /// The `transform` closure may be called more than once if the test records
   /// multiple issues. If more than one instance of this trait is applied to a
-  /// test (including via inheritance from a containing suite), the `transformer`
+  /// test (including via inheritance from a containing suite), the `transform`
   /// closure for each instance will be called in right-to-left, innermost-to-
   /// outermost order, unless `nil` is returned, which will skip invoking the
   /// remaining traits' closures.
   ///
-  /// Within `transformer`, you may access the current test or test case (if any)
+  /// Within `transform`, you may access the current test or test case (if any)
   /// using ``Test/current`` ``Test/Case/current``, respectively. You may also
   /// record new issues, although they will only be handled by issue handling
   /// traits which precede this trait or were inherited from a containing suite.
-  public static func transformIssues(_ transformer: @escaping @Sendable (Issue) -> Issue?) -> Self {
-    Self(handler: transformer)
+  ///
+  /// - Note: `transform` will never be passed an issue for which the value of
+  ///   ``Issue/kind`` is ``Issue/Kind/system``, and may not return such an
+  ///   issue.
+  public static func compactMapIssues(_ transform: @escaping @Sendable (Issue) -> Issue?) -> Self {
+    Self(handler: transform)
   }
 
   /// Constructs a trait that filters issues recorded by a test.
@@ -174,6 +189,9 @@ extension Trait where Self == IssueHandlingTrait {
   /// using ``Test/current`` ``Test/Case/current``, respectively. You may also
   /// record new issues, although they will only be handled by issue handling
   /// traits which precede this trait or were inherited from a containing suite.
+  ///
+  /// - Note: `isIncluded` will never be passed an issue for which the value of
+  ///   ``Issue/kind`` is ``Issue/Kind/system``.
   public static func filterIssues(_ isIncluded: @escaping @Sendable (Issue) -> Bool) -> Self {
     Self { issue in
       isIncluded(issue) ? issue : nil

Tests/TestingTests/Traits/IssueHandlingTraitTests.swift

@@ -8,7 +8,7 @@
 // See https://swift.org/CONTRIBUTORS.txt for Swift project authors
 //
 
-@_spi(Experimental) @_spi(ForToolsIntegrationOnly) import Testing
+@testable @_spi(Experimental) @_spi(ForToolsIntegrationOnly) import Testing
 
 @Suite("IssueHandlingTrait Tests")
 struct IssueHandlingTraitTests {
@@ -23,7 +23,7 @@ struct IssueHandlingTraitTests {
       #expect(issue.comments == ["Foo", "Bar"])
     }
 
-    let handler = IssueHandlingTrait.transformIssues { issue in
+    let handler = IssueHandlingTrait.compactMapIssues { issue in
       var issue = issue
       issue.comments.append("Bar")
       return issue
@@ -34,16 +34,16 @@ struct IssueHandlingTraitTests {
     }.run(configuration: configuration)
   }
 
-  @Test("Suppressing an issue by returning `nil` from the transform closure")
-  func suppressIssueUsingTransformer() async throws {
+  @Test("Suppressing an issue by returning `nil` from the closure passed to compactMapIssues()")
+  func suppressIssueUsingCompactMapIssues() async throws {
     var configuration = Configuration()
     configuration.eventHandler = { event, context in
       if case .issueRecorded = event.kind {
         Issue.record("Unexpected issue recorded event: \(event)")
       }
     }
 
-    let handler = IssueHandlingTrait.transformIssues { _ in
+    let handler = IssueHandlingTrait.compactMapIssues { _ in
       // Return nil to suppress the issue.
       nil
     }
@@ -81,10 +81,10 @@ struct IssueHandlingTraitTests {
 
     struct MyError: Error {}
 
-    try await confirmation("Transformer closure is called") { transformerCalled in
-      let transformer: @Sendable (Issue) -> Issue? = { issue in
+    try await confirmation("Issue handler closure is called") { issueHandlerCalled in
+      let transform: @Sendable (Issue) -> Issue? = { issue in
         defer {
-          transformerCalled()
+          issueHandlerCalled()
         }
 
         #expect(Test.Case.current == nil)
@@ -96,7 +96,7 @@ struct IssueHandlingTraitTests {
 
       let test = Test(
         .enabled(if: try { throw MyError() }()),
-        .transformIssues(transformer)
+        .compactMapIssues(transform)
       ) {}
 
       // Use a detached task to intentionally clear task local values for the
@@ -108,12 +108,12 @@ struct IssueHandlingTraitTests {
   }
 #endif
 
-  @Test("Accessing the current Test and Test.Case from a transformer closure")
+  @Test("Accessing the current Test and Test.Case from an issue handler closure")
   func currentTestAndCase() async throws {
-    await confirmation("Transformer closure is called") { transformerCalled in
-      let handler = IssueHandlingTrait.transformIssues { issue in
+    await confirmation("Issue handler closure is called") { issueHandlerCalled in
+      let handler = IssueHandlingTrait.compactMapIssues { issue in
         defer {
-          transformerCalled()
+          issueHandlerCalled()
         }
         #expect(Test.current?.name == "fixture()")
         #expect(Test.Case.current != nil)
@@ -140,12 +140,12 @@ struct IssueHandlingTraitTests {
       #expect(issue.comments == ["Foo", "Bar", "Baz"])
     }
 
-    let outerHandler = IssueHandlingTrait.transformIssues { issue in
+    let outerHandler = IssueHandlingTrait.compactMapIssues { issue in
       var issue = issue
       issue.comments.append("Baz")
       return issue
     }
-    let innerHandler = IssueHandlingTrait.transformIssues { issue in
+    let innerHandler = IssueHandlingTrait.compactMapIssues { issue in
       var issue = issue
       issue.comments.append("Bar")
       return issue
@@ -156,7 +156,7 @@ struct IssueHandlingTraitTests {
     }.run(configuration: configuration)
   }
 
-  @Test("Secondary issue recorded from a transformer closure")
+  @Test("Secondary issue recorded from an issue handler closure")
   func issueRecordedFromClosure() async throws {
     await confirmation("Original issue recorded") { originalIssueRecorded in
       await confirmation("Secondary issue recorded") { secondaryIssueRecorded in
@@ -175,14 +175,14 @@ struct IssueHandlingTraitTests {
           }
         }
 
-        let handler1 = IssueHandlingTrait.transformIssues { issue in
+        let handler1 = IssueHandlingTrait.compactMapIssues { issue in
           return issue
         }
-        let handler2 = IssueHandlingTrait.transformIssues { issue in
+        let handler2 = IssueHandlingTrait.compactMapIssues { issue in
           Issue.record("Something else")
           return issue
         }
-        let handler3 = IssueHandlingTrait.transformIssues { issue in
+        let handler3 = IssueHandlingTrait.compactMapIssues { issue in
           // The "Something else" issue should not be passed to this closure.
           #expect(issue.comments.contains("Foo"))
           return issue
@@ -194,4 +194,40 @@ struct IssueHandlingTraitTests {
       }
     }
   }
+
+  @Test("System issues are not passed to issue handler closures")
+  func ignoresSystemIssues() async throws {
+    var configuration = Configuration()
+    configuration.eventHandler = { event, context in
+      if case let .issueRecorded(issue) = event.kind, case .unconditional = issue.kind {
+        issue.record()
+      }
+    }
+
+    let handler = IssueHandlingTrait.compactMapIssues { issue in
+      if case .system = issue.kind {
+        Issue.record("Unexpectedly received a system issue")
+      }
+      return nil
+    }
+
+    await Test(handler) {
+      Issue(kind: .system).record()
+    }.run(configuration: configuration)
+  }
+
+#if !SWT_NO_EXIT_TESTS
+  @Test("Disallow assigning kind to .system")
+  func disallowAssigningSystemKind() async throws {
+    await #expect(processExitsWith: .failure) {
+      await Test(.compactMapIssues { issue in
+        var issue = issue
+        issue.kind = .system
+        return issue
+      }) {
+        Issue.record("A non-system issue")
+      }.run()
+    }
+  }
+#endif
 }