Skip to content

Commit b5fa554

Browse files
authored
Include code comments before expectations which are preceded by try/await in recorded issues (#1092)
This fixes an issue where code comments placed before an expectation like `#expect()` which has effect introducer keywords like `try` or `await` are ignored, and ensures they are included in the recorded issue if the expectation fails. Consider this example of two failing expectations: ```swift // Uh oh! #expect(try x() == 2) // Uh oh! try #expect(x() == 2) ``` Prior to this PR, if `x()` returned a value other than `2`, there would be two issues recorded, but the second one would not have the comment `“Uh oh!”` because from the macro’s perspective, that code comment was on the `try` keyword and it could only see trivia associated with `#expect()`. Now, with the recent swift-syntax fix from swiftlang/swift-syntax#3037, the `try` keyword and its associated trivia can be included and this bug can be fixed. We recently adopted a new-enough swift-syntax in #1069, so the only fix needed is to adopt `lexicalContext` for this new purpose in our macro. ### 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.
1 parent cacb295 commit b5fa554

File tree

3 files changed

+79
-2
lines changed

3 files changed

+79
-2
lines changed

Sources/TestingMacros/ConditionMacro.swift

+10-1
Original file line numberDiff line numberDiff line change
@@ -157,8 +157,17 @@ extension ConditionMacro {
157157
expandedFunctionName = conditionArgument.expandedFunctionName
158158
}
159159

160-
// Capture any comments as well (either in source or as a macro argument.)
160+
// Capture any comments as well -- either in source, preceding the
161+
// expression macro or one of its lexical context nodes, or as an argument
162+
// to the macro.
161163
let commentsArrayExpr = ArrayExprSyntax {
164+
// Lexical context is ordered innermost-to-outermost, so reverse it to
165+
// maintain the expected order.
166+
for lexicalSyntaxNode in context.lexicalContext.trailingEffectExpressions.reversed() {
167+
for commentTraitExpr in createCommentTraitExprs(for: lexicalSyntaxNode) {
168+
ArrayElementSyntax(expression: commentTraitExpr)
169+
}
170+
}
162171
for commentTraitExpr in createCommentTraitExprs(for: macro) {
163172
ArrayElementSyntax(expression: commentTraitExpr)
164173
}

Sources/TestingMacros/Support/EffectfulExpressionHandling.swift

+16-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import SwiftSyntax
1212
import SwiftSyntaxBuilder
1313
import SwiftSyntaxMacros
1414

15-
// MARK: - Finding effect keywords
15+
// MARK: - Finding effect keywords and expressions
1616

1717
/// A syntax visitor class that looks for effectful keywords in a given
1818
/// expression.
@@ -69,6 +69,21 @@ func findEffectKeywords(in node: some SyntaxProtocol, context: some MacroExpansi
6969
return effectFinder.effectKeywords
7070
}
7171

72+
extension BidirectionalCollection<Syntax> {
73+
/// The suffix of syntax nodes in this collection which are effectful
74+
/// expressions, such as those for `try` or `await`.
75+
var trailingEffectExpressions: some Collection<Syntax> {
76+
reversed()
77+
.prefix { node in
78+
// This could be simplified if/when swift-syntax introduces a protocol
79+
// which all effectful expression syntax node types conform to.
80+
// See https://github.com/swiftlang/swift-syntax/issues/3040
81+
node.is(TryExprSyntax.self) || node.is(AwaitExprSyntax.self) || node.is(UnsafeExprSyntax.self)
82+
}
83+
.reversed()
84+
}
85+
}
86+
7287
// MARK: - Inserting effect keywords/thunks
7388

7489
/// Make a function call expression to an effectful thunk function provided by

Tests/TestingMacrosTests/ConditionMacroTests.swift

+53
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,59 @@ struct ConditionMacroTests {
240240
// Capture me
241241
Testing.__checkValue(try x(), expression: .__fromSyntaxNode("try x()"), comments: [.__line("// Capture me")], isRequired: false, sourceLocation: Testing.SourceLocation.__here()).__expected()
242242
""",
243+
244+
"""
245+
// Capture me
246+
try #expect(x)
247+
""":
248+
"""
249+
// Capture me
250+
try Testing.__checkValue(x, expression: .__fromSyntaxNode("x"), comments: [.__line("// Capture me")], isRequired: false, sourceLocation: Testing.SourceLocation.__here()).__expected()
251+
""",
252+
253+
"""
254+
// Capture me
255+
await #expect(x)
256+
""":
257+
"""
258+
// Capture me
259+
await Testing.__checkValue(x, expression: .__fromSyntaxNode("x"), comments: [.__line("// Capture me")], isRequired: false, sourceLocation: Testing.SourceLocation.__here()).__expected()
260+
""",
261+
262+
"""
263+
// Ignore me
264+
265+
// Comment for try
266+
try
267+
// Comment for await
268+
await
269+
// Comment for expect
270+
#expect(x)
271+
""":
272+
"""
273+
// Comment for try
274+
try
275+
// Comment for await
276+
await
277+
// Comment for expect
278+
Testing.__checkValue(x, expression: .__fromSyntaxNode("x"), comments: [.__line("// Comment for try"), .__line("// Comment for await"), .__line("// Comment for expect")], isRequired: false, sourceLocation: Testing.SourceLocation.__here()).__expected()
279+
""",
280+
281+
"""
282+
// Ignore me
283+
func example() {
284+
// Capture me
285+
#expect(x())
286+
}
287+
""":
288+
"""
289+
func example() {
290+
// Capture me
291+
Testing.__checkFunctionCall((), calling: { _ in
292+
x()
293+
}, expression: .__fromFunctionCall(nil, "x"), comments: [.__line("// Capture me")], isRequired: false, sourceLocation: Testing.SourceLocation.__here()).__expected()
294+
}
295+
""",
243296
]
244297
)
245298
func commentCapture(input: String, expectedOutput: String) throws {

0 commit comments

Comments
 (0)