|
6 | 6 | * Status: **Active Review (Jun 24 - July 8, 2025)**
|
7 | 7 | * Implementation: [swiftlang/swift-testing#1080](https://github.com/swiftlang/swift-testing/pull/1080),
|
8 | 8 | [swiftlang/swift-testing#1121](https://github.com/swiftlang/swift-testing/pull/1121),
|
9 |
| - [swiftlang/swift-testing#1136](https://github.com/swiftlang/swift-testing/pull/1136) |
| 9 | + [swiftlang/swift-testing#1136](https://github.com/swiftlang/swift-testing/pull/1136), |
| 10 | + [swiftlang/swift-testing#1198](https://github.com/swiftlang/swift-testing/pull/1198) |
10 | 11 | * Review: ([pitch](https://forums.swift.org/t/pitch-issue-handling-traits/80019)) ([review](https://forums.swift.org/t/st-0011-issue-handling-traits/80644))
|
11 | 12 |
|
12 | 13 | ## Introduction
|
@@ -303,13 +304,15 @@ Issue handling traits are applied to a test by a user, and are only intended for
|
303 | 304 | handling issues recorded by tests written by the user. If an issue is recorded
|
304 | 305 | by the testing library itself or the underlying system, not due to a failure
|
305 | 306 | within the tests being run, such an issue will not be passed to an issue
|
306 |
| -handling trait. |
| 307 | +handling trait. Similarly, an issue handling trait should not return an issue |
| 308 | +which represents a problem they could not have caused in their test. |
307 | 309 |
|
308 | 310 | Concretely, this policy means that issues for which the value of the `kind`
|
309 | 311 | property is `.system` will not be passed to the closure of an issue handling
|
310 |
| -trait. Similarly, it is not supported for a closure passed to |
| 312 | +trait. Also, it is not supported for a closure passed to |
311 | 313 | `compactMapIssues(_:)` to return an issue for which the value of `kind` is
|
312 |
| -`.system`. |
| 314 | +either `.system` or `.apiMisused` (unless the passed-in issue had that kind, |
| 315 | +which should only be possible for `.apiMisused`). |
313 | 316 |
|
314 | 317 | ## Detailed design
|
315 | 318 |
|
@@ -507,15 +510,29 @@ the handler encounters an error.
|
507 | 510 |
|
508 | 511 | The closure parameter of `compactMapIssues(_:)` currently has one parameter of
|
509 | 512 | type `Issue` and returns an optional `Issue?` to support returning `nil` in
|
510 |
| -order to suppress an issue. This closure could instead have a `Void` return type |
511 |
| -and its parameter could be `inout`, and this would mean that use cases that |
512 |
| -involve modifying an issue would not need to first copy the issue to a variable |
513 |
| -(`var`) before modifying it. |
514 |
| - |
515 |
| -However, in order to _suppress_ an issue, the parameter would also need to |
516 |
| -become optional (`inout Issue?`) and this would mean that all usages would first |
517 |
| -need to be unwrapped. This feels non-ergonomic, and would differ from the |
518 |
| -standard library's typical pattern for `compactMap` functions. |
| 513 | +order to suppress an issue. If an issue handler wants to modify an issue, it |
| 514 | +first needs to copy it to a mutable variable (`var`), mutate it, then return the |
| 515 | +modified copy. These copy and return steps require extra lines of code within |
| 516 | +the closure, and they could be eliminated if the parameter was declared `inout`. |
| 517 | + |
| 518 | +The most straightforward way to achieve this would be for the closure to instead |
| 519 | +have a `Void` return type and for its parameter to become `inout`. However, in |
| 520 | +order to _suppress_ an issue, the parameter would also need to become optional |
| 521 | +(`inout Issue?`) and this would mean that all usages would first need to be |
| 522 | +unwrapped. This feels non-ergonomic, and would differ from the standard |
| 523 | +library's typical pattern for `compactMap` functions. |
| 524 | + |
| 525 | +Another way to achieve this ([suggested](https://forums.swift.org/t/st-0011-issue-handling-traits/80644/3) |
| 526 | +by [@Val](https://forums.swift.org/u/Val) during proposal review) could be to |
| 527 | +declare the return type of the closure `Void?` and the parameter type |
| 528 | +`inout Issue` (non-optional). This alternative would not require unwrapping the |
| 529 | +issue first and would still permit suppressing issues by returning `nil`. It |
| 530 | +could also make one of the alternative names (such as `transformIssues` |
| 531 | +discussed below) more fitting. However, this is a novel API pattern which isn't |
| 532 | +widely used in Swift, and may be confusing to users. There were also concerns |
| 533 | +raised by other reviewers that the language's implicit return for `Void` may not |
| 534 | +be intentionally applied to `Optional<Void>` and that this mechanism could break |
| 535 | +in the future. |
519 | 536 |
|
520 | 537 | ### Alternate names for the static trait functions
|
521 | 538 |
|
|
0 commit comments