Skip to content

Commit bcf900f

Browse files
committed
Split .bug() into two taking a URL or a number.
This PR changes the initializers for `.bug()` from ones taking an undecorated and unspecified "identifier" to ones taking either a URL (as a string) or a numeric ID ("bug number.") The existing interface is ambiguous to tools authors seeking to integrate with it. They must treat _every_ bug as potentially containing a URL with fallback paths if a bug's ID cannot be parsed as a URL. By splitting URLs and numbers up into two separate properties, tools authors can provide reliable, distinct interfaces for bugs known by number vs. those known by URL.
1 parent d50745c commit bcf900f

File tree

14 files changed

+193
-129
lines changed

14 files changed

+193
-129
lines changed

Sources/Testing/Testing.docc/AssociatingBugs.md

+8-4
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,13 @@ specific bugs with tests that reproduce them or verify they are fixed.
2525

2626
## Associate a bug with a test
2727

28-
To associate a bug with a test, use the ``Trait/bug(_:_:)-2u8j9`` or
29-
``Trait/bug(_:_:)-7mo2w`` function. The first argument to this function is the
30-
bug's _identifier_ in its bug-tracking system:
28+
To associate a bug with a test, use one of these functions:
29+
- ``Trait/bug(url:_:)``
30+
- ``Trait/bug(_:url:_:)-7f1e4``
31+
- ``Trait/bug(_:url:_:)-41twe``
32+
33+
The first argument to these functions is the bug's _identifier_ in its
34+
bug-tracking system:
3135

3236
```swift
3337
@Test("Food truck engine works", .bug("12345"), .bug(67890))
@@ -43,7 +47,7 @@ specified as a string, it must be parseable as an unsigned integer or as a URL.
4347
For more information on the formats recognized by the testing library, see
4448
<doc:BugIdentifiers>.
4549

46-
## Add comments to associated bugs
50+
## Add titles to associated bugs
4751

4852
A bug identifier may be insufficient to uniquely and clearly identify a bug
4953
associated with a test. Bug trackers universally provide a "title" field for

Sources/Testing/Testing.docc/BugIdentifiers.md

+25-30
Original file line numberDiff line numberDiff line change
@@ -14,37 +14,30 @@ How the testing library interprets bug identifiers provided by developers.
1414

1515
## Overview
1616

17-
As a convenience, the testing library assumes that bug identifiers with specific
18-
formats are associated with some common bug-tracking systems.
17+
The testing library supports two distinct ways to identify a bug:
18+
19+
1. A URL linking to more information about the bug; and
20+
2. A unique identifier in the bug's associated bug-tracking system.
1921

2022
- Note: "Bugs" as described in this document may also be referred to as
21-
"issues." To avoid confusion with the ``Issue`` type in the testing library,
22-
this document consistently refers to them as "bugs."
23-
24-
## Recognized formats
25-
26-
- If the bug identifier begins with `"rdar:"`, it is assumed to represent a bug
27-
filed with Apple's Radar system.
28-
- If the bug identifier can be parsed as a URL according to
29-
[RFC 3986](https://www.ietf.org/rfc/rfc3986.txt), it is assumed to represent
30-
an issue tracked at that URL.
31-
- If the bug identifier begins with `"FB"` and the rest of it can be parsed as
32-
an unsigned integer, it is assumed to represent a bug filed with the
33-
[Apple Feedback Assistant](https://feedbackassistant.apple.com).
34-
- If the bug identifier can be parsed as an unsigned integer, it is assumed to
35-
represent an issue with that numeric identifier in an unspecified bug-tracking
36-
system.
37-
- All other bug identifiers are considered invalid and will cause the compiler
38-
to generate an error at compile time.
23+
"issues." To avoid confusion with the ``Issue`` type in the testing library,
24+
this document consistently refers to them as "bugs."
3925

40-
<!--
41-
Possible additional formats we could recognize (which would require special
42-
handling to detect:
26+
A bug may have both an associated URL _and_ an associated unique identifier. It
27+
must have at least one or the other in order for the testing library to be able
28+
to interpret it correctly.
4329

44-
- If the bug identifier begins with `"#"` and can be parsed as a positive
45-
integer, it is assumed to represent a [GitHub](https://github.com) issue in
46-
the same repository as the test.
47-
-->
30+
To create an instance of ``Bug`` with a URL, use the ``Trait/bug(url:_:)``
31+
trait. At compile time, the testing library will validate that the given string
32+
can be parsed as a URL according to [RFC 3986](https://www.ietf.org/rfc/rfc3986.txt).
33+
34+
To create an instance of ``Bug`` with a bug's unique identifier, use the
35+
``Trait/bug(_:url:_:)-7f1e4`` trait. The testing library does not require that
36+
a bug's unique identifier match any particular format, but will interpret
37+
unique identifiers starting with `"FB"` as referring to bugs tracked with the
38+
[Apple Feedback Assistant](https://feedbackassistant.apple.com). For
39+
convenience, you can also directly pass an integer as a bug's identifier using
40+
``Trait/bug(_:url:_:)-41twe``.
4841

4942
## Examples
5043

@@ -53,10 +46,12 @@ handling to detect:
5346
| `.bug(12345)` | Yes | None |
5447
| `.bug("12345")` | Yes | None |
5548
| `.bug("Things don't work")` | **No** | None |
56-
| `.bug("rdar:12345")` | Yes | Apple Radar |
57-
| `.bug("https://github.com/apple/swift/pull/12345")` | Yes | [GitHub Issues for the Swift project](https://github.com/apple/swift/issues) |
58-
| `.bug("https://bugs.webkit.org/show_bug.cgi?id=12345")` | Yes | [WebKit Bugzilla](https://bugs.webkit.org/) |
49+
| `.bug(12345, "Things don't work")` | Yes | None |
50+
| `.bug(url: "rdar:12345")` | Yes | Apple Radar |
51+
| `.bug(url: "https://github.com/apple/swift/pull/12345")` | Yes | [GitHub Issues for the Swift project](https://github.com/apple/swift/issues) |
52+
| `.bug(url: "https://bugs.webkit.org/show_bug.cgi?id=12345")` | Yes | [WebKit Bugzilla](https://bugs.webkit.org/) |
5953
| `.bug("FB12345")` | Yes | Apple Feedback Assistant | <!-- SEE ALSO: rdar://104582015 -->
54+
| `.bug("12345", url: "rdar:12345")` | Yes | Apple Radar |
6055
<!--
6156
| `.bug("#12345")` | Yes | GitHub Issues for the current repository (if hosted there) |
6257
-->

Sources/Testing/Testing.docc/EnablingAndDisabling.md

+10-4
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,14 @@ func isCold() async throws { ... }
8383
```
8484

8585
If a test is disabled because of a problem for which there is a corresponding
86-
bug report, you can use the ``Trait/bug(_:_:)-2u8j9`` or
87-
``Trait/bug(_:_:)-7mo2w`` function to show the relationship:
86+
bug report, you can use one of these functions to show the relationship
87+
between the test and the bug report:
88+
89+
- ``Trait/bug(url:_:)``
90+
- ``Trait/bug(_:url:_:)-7f1e4``
91+
- ``Trait/bug(_:url:_:)-41twe``
92+
93+
For example, the following test cannot run due to bug number `"12345"`:
8894

8995
```swift
9096
@Test(
@@ -97,8 +103,8 @@ func isCold() async throws { ... }
97103
```
98104

99105
If a test has multiple conditions applied to it, they must _all_ pass for it to
100-
run. Otherwise, the test notes the first condition to fail as the reason the test
101-
is skipped.
106+
run. Otherwise, the test notes the first condition to fail as the reason the
107+
test is skipped.
102108

103109
## Handle complex conditions
104110

Sources/Testing/Testing.docc/Traits.md

+3-2
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,9 @@ HIDDEN: .serialized is experimental SPI pending feature review.
4646
- <doc:AssociatingBugs>
4747
- <doc:BugIdentifiers>
4848
- ``Tag()``
49-
- ``Trait/bug(_:_:)-2u8j9``
50-
- ``Trait/bug(_:_:)-7mo2w``
49+
- ``Trait/bug(url:_:)``
50+
- ``Trait/bug(_:url:_:)-7f1e4``
51+
- ``Trait/bug(_:url:_:)-41twe``
5152

5253
### Creating custom traits
5354

Sources/Testing/Testing.docc/Traits/Trait.md

+3-2
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,9 @@ See https://swift.org/CONTRIBUTORS.txt for Swift project authors
3131

3232
### Associating bugs
3333

34-
- ``Trait/bug(_:_:)-2u8j9``
35-
- ``Trait/bug(_:_:)-7mo2w``
34+
- ``Trait/bug(url:_:)``
35+
- ``Trait/bug(_:url:_:)-7f1e4``
36+
- ``Trait/bug(_:url:_:)-41twe``
3637

3738
### Adding information to tests
3839
- ``Trait/comments``

Sources/Testing/Traits/Bug.swift

+46-27
Original file line numberDiff line numberDiff line change
@@ -12,32 +12,37 @@
1212
///
1313
/// To add this trait to a test, use one of the following functions:
1414
///
15-
/// - ``Trait/bug(_:_:)-86mmm``
16-
/// - ``Trait/bug(_:_:)-3hsi5``
15+
/// - ``Trait/bug(url:_:)``
16+
/// - ``Trait/bug(_:url:_:)-7f1e4``
17+
/// - ``Trait/bug(_:url:_:)-41twe``
1718
public struct Bug {
18-
/// The identifier of this bug in the associated bug-tracking system.
19+
/// A unique identifier in this bug's associated bug-tracking system, if
20+
/// available.
1921
///
2022
/// For more information on how the testing library interprets bug
2123
/// identifiers, see <doc:BugIdentifiers>.
22-
public var identifier: String
24+
public var id: String?
2325

24-
/// An optional, user-specified comment describing this trait.
25-
public var comment: Comment?
26+
/// A URL linking to more information about the bug, if available.
27+
///
28+
/// The value of this property represents a URL conforming to
29+
/// [RFC 3986](https://www.ietf.org/rfc/rfc3986.txt).
30+
public var url: String?
31+
32+
/// The human-readable title of the bug, if specified by the test author.
33+
public var title: Comment?
2634
}
2735

28-
// MARK: - Equatable, Hashable, Comparable
36+
// MARK: - Equatable, Hashable
2937

30-
extension Bug: Equatable, Hashable, Comparable {
38+
extension Bug: Equatable, Hashable {
3139
public static func ==(lhs: Self, rhs: Self) -> Bool {
32-
lhs.identifier == rhs.identifier
40+
lhs.url == rhs.url && lhs.id == rhs.id
3341
}
3442

3543
public func hash(into hasher: inout Hasher) {
36-
hasher.combine(identifier)
37-
}
38-
39-
public static func <(lhs: Self, rhs: Self) -> Bool {
40-
lhs.identifier < rhs.identifier
44+
hasher.combine(url)
45+
hasher.combine(id)
4146
}
4247
}
4348

@@ -49,35 +54,49 @@ extension Bug: Codable {}
4954

5055
extension Bug: TestTrait, SuiteTrait {
5156
public var comments: [Comment] {
52-
Array(comment)
57+
Array(title)
5358
}
5459
}
5560

5661
extension Trait where Self == Bug {
5762
/// Construct a bug to track with a test.
5863
///
5964
/// - Parameters:
60-
/// - identifier: The identifier of this bug in the associated bug-tracking
61-
/// system. For more information on how this value is interpreted, see the
62-
/// documentation for ``Bug``.
63-
/// - comment: An optional, user-specified comment describing this trait.
65+
/// - url: A URL referring to this bug in the associated bug-tracking
66+
/// system.
67+
/// - title: Optionally, the human-readable title of the bug.
68+
///
69+
/// - Returns: An instance of ``Bug`` representing the specified bug.
70+
public static func bug(url: _const String, _ title: Comment? = nil) -> Self {
71+
Self(url: url, title: title)
72+
}
73+
74+
/// Construct a bug to track with a test.
75+
///
76+
/// - Parameters:
77+
/// - id: The unique identifier of this bug in its associated bug-tracking
78+
/// system.
79+
/// - url: A URL referring to this bug in the associated bug-tracking
80+
/// system.
81+
/// - title: Optionally, the human-readable title of the bug.
6482
///
6583
/// - Returns: An instance of ``Bug`` representing the specified bug.
66-
public static func bug(_ identifier: String, _ comment: Comment? = nil) -> Self {
67-
Self(identifier: identifier, comment: comment)
84+
public static func bug(_ id: some Numeric, url: _const String? = nil, _ title: Comment? = nil) -> Self {
85+
Self(id: String(describing: id), url: url, title: title)
6886
}
6987

7088
/// Construct a bug to track with a test.
7189
///
7290
/// - Parameters:
73-
/// - identifier: The identifier of this bug in the associated bug-tracking
74-
/// system. For more information on how this value is interpreted, see the
75-
/// documentation for ``Bug``.
76-
/// - comment: An optional, user-specified comment describing this trait.
91+
/// - id: The unique identifier of this bug in its associated bug-tracking
92+
/// system.
93+
/// - url: A URL referring to this bug in the associated bug-tracking
94+
/// system.
95+
/// - title: Optionally, the human-readable title of the bug.
7796
///
7897
/// - Returns: An instance of ``Bug`` representing the specified bug.
79-
public static func bug(_ identifier: some Numeric, _ comment: Comment? = nil) -> Self {
80-
Self(identifier: String(describing: identifier), comment: comment)
98+
public static func bug(_ id: _const String, url: _const String? = nil, _ title: Comment? = nil) -> Self {
99+
Self(id: id, url: url, title: title)
81100
}
82101
}
83102

Sources/Testing/Traits/Comment.swift

+4-2
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@
1818
/// or `@Suite` attribute. See <doc:AddingComments> for more details.
1919
///
2020
/// - Note: This type is not intended to reference bugs related to a test.
21-
/// Instead, use ``Trait/bug(_:_:)-2u8j9`` or ``Trait/bug(_:_:)-7mo2w``.
21+
/// Instead, use ``Trait/bug(url:_:)``, ``Trait/bug(_:url:_:)-7f1e4``, or
22+
/// ``Trait/bug(_:url:_:)-41twe``.
2223
public struct Comment: RawRepresentable, Sendable {
2324
/// The single comment string contained in this instance.
2425
///
@@ -121,7 +122,8 @@ extension Trait where Self == Comment {
121122
/// comment.
122123
///
123124
/// - Note: This function is not intended to reference bugs related to a test.
124-
/// Instead, use ``Trait/bug(_:_:)-2u8j9`` or ``Trait/bug(_:_:)-7mo2w``.
125+
/// Instead, use ``Trait/bug(url:_:)``, ``Trait/bug(_:url:_:)-7f1e4``,
126+
/// or ``Trait/bug(_:url:_:)-41twe``.
125127
public static func comment(_ comment: String) -> Self {
126128
Self(rawValue: comment, kind: .trait)
127129
}

Sources/TestingMacros/Support/DiagnosticMessage+Diagnosing.swift

+7-22
Original file line numberDiff line numberDiff line change
@@ -108,29 +108,14 @@ private func _diagnoseIssuesWithTagsTrait(_ traitExpr: FunctionCallExprSyntax, a
108108
/// - attribute: The `@Test` or `@Suite` attribute.
109109
/// - context: The macro context in which the expression is being parsed.
110110
private func _diagnoseIssuesWithBugTrait(_ traitExpr: FunctionCallExprSyntax, addedTo attribute: AttributeSyntax, in context: some MacroExpansionContext) {
111-
// If the first argument to the .bug() trait is unlabelled and a string
112-
// literal, check that it can be parsed as a URL or at least as an integer.
113-
guard let arg = traitExpr.arguments.first.map(Argument.init),
114-
arg.label == nil,
115-
let stringLiteralExpr = arg.expression.as(StringLiteralExprSyntax.self),
116-
let urlString = stringLiteralExpr.representedLiteralValue else {
117-
return
111+
// If there is an argument to the .bug() trait labelled "url:" and its value
112+
// is a string literal, check that it can be parsed the way we expect.
113+
let urlArgIndex = traitExpr.arguments.firstIndex { argument in
114+
argument.label?.textWithoutBackticks == "url"
118115
}
119-
120-
if UInt64(urlString) != nil {
121-
// The entire URL string can be parsed as an integer, so allow it. Although
122-
// the testing library prefers valid URLs here, some bug-tracking systems
123-
// might not provide URLs, or might provide excessively long URLs, so we
124-
// allow numeric identifiers as a fallback.
125-
return
126-
}
127-
128-
if urlString.count > 3 && urlString.starts(with: "FB") && UInt64(urlString.dropFirst(2)) != nil {
129-
// The string appears to be of the form "FBnnn...". Such strings are used by
130-
// Apple to indicate issues filed using Feedback Assistant. Although we
131-
// don't want to special-case every possible bug-tracking system out there,
132-
// Feedback Assistant is very important to Apple so we're making an
133-
// exception for it.
116+
guard let urlArgIndex,
117+
let stringLiteralExpr = traitExpr.arguments[urlArgIndex].expression.as(StringLiteralExprSyntax.self),
118+
let urlString = stringLiteralExpr.representedLiteralValue else {
134119
return
135120
}
136121

Sources/TestingMacros/Support/DiagnosticMessage.swift

+1-1
Original file line numberDiff line numberDiff line change
@@ -567,7 +567,7 @@ struct DiagnosticMessage: SwiftDiagnostics.DiagnosticMessage {
567567
return Self(
568568
syntax: Syntax(urlExpr),
569569
message: #"URL "\#(urlString)" is invalid and cannot be used with trait '\#(traitName.trimmed)' in attribute \#(_macroName(attribute))"#,
570-
severity: .error,
570+
severity: .warning,
571571
fixIts: [
572572
FixIt(
573573
message: MacroExpansionFixItMessage(#"Replace "\#(urlString)" with URL"#),
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
//
2+
// This source file is part of the Swift.org open source project
3+
//
4+
// Copyright (c) 2024 Apple Inc. and the Swift project authors
5+
// Licensed under Apache License v2.0 with Runtime Library Exception
6+
//
7+
// See https://swift.org/LICENSE.txt for license information
8+
// See https://swift.org/CONTRIBUTORS.txt for Swift project authors
9+
//
10+
11+
#if canImport(Foundation)
12+
@_spi(Experimental) @_spi(ForToolsIntegrationOnly) public import Testing
13+
public import Foundation
14+
15+
extension Trait where Self == Bug {
16+
/// Construct a bug to track with a test.
17+
///
18+
/// - Parameters:
19+
/// - url: A URL referring to this bug in the associated bug-tracking
20+
/// system.
21+
/// - title: Optionally, the human-readable title of the bug.
22+
///
23+
/// - Returns: An instance of ``Bug`` representing the specified bug.
24+
public static func bug(url: URL, _ title: Comment? = nil) -> Self {
25+
// The factory functions in Bug.swift take _const strings in order to help
26+
// enforce URL validation, so work around it by creating an instance with a
27+
// blank ID and replacing it.
28+
var result = Bug.bug(url: "")
29+
result.url = url.absoluteString
30+
return result
31+
}
32+
}
33+
#endif

0 commit comments

Comments
 (0)