Skip to content

Commit eeeffd4

Browse files
authored
Suppress the complex expansion of expectations when we see effects in the lexical context. (#1161)
This PR changes the behaviour of the testing library for expressions such as: ```swift try #expect(a == b) ``` Currently, we don't expand that expression correctly because we can't tell where the `try` keyword should be applied. It sometimes expands and sometimes doesn't. This PR detects the presence of those keywords (with a recent-enough toolchain) and, if found, disables the fancy expansion in favour of a simpler one that is less likely to fail to compile. More thorough support for effectful expressions in expectations is tracked by #840 which involves fully refactoring the implementation of the `#expect()` macro. See also #162 for some more context. ### 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 b97b576 commit eeeffd4

File tree

2 files changed

+87
-40
lines changed

2 files changed

+87
-40
lines changed

Sources/TestingMacros/Support/ConditionArgumentParsing.swift

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -517,10 +517,8 @@ private func _parseCondition(from expr: ExprSyntax, for macro: some Freestanding
517517
/// - Returns: An instance of ``Condition`` describing `expr`.
518518
func parseCondition(from expr: ExprSyntax, for macro: some FreestandingMacroExpansionSyntax, in context: some MacroExpansionContext) -> Condition {
519519
// If the condition involves the `unsafe`, `try`, or `await` keywords, assume
520-
// we cannot expand it. This check cannot handle expressions like
521-
// `try #expect(a.b(c))` where `b()` is throwing because the `try` keyword is
522-
// outside the macro expansion. SEE: rdar://109470248
523-
let effectKeywordsToApply = findEffectKeywords(in: expr, context: context)
520+
// we cannot expand it.
521+
let effectKeywordsToApply = findEffectKeywords(in: expr).union(findEffectKeywords(in: context))
524522
guard effectKeywordsToApply.intersection([.unsafe, .try, .await]).isEmpty else {
525523
return Condition(expression: expr)
526524
}

Sources/TestingMacros/Support/EffectfulExpressionHandling.swift

Lines changed: 85 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -14,61 +14,105 @@ import SwiftSyntaxMacros
1414

1515
// MARK: - Finding effect keywords and expressions
1616

17+
/// Get the effect keyword corresponding to a given syntax node, if any.
18+
///
19+
/// - Parameters:
20+
/// - expr: The syntax node that may represent an effectful expression.
21+
///
22+
/// - Returns: The effect keyword corresponding to `expr`, if any.
23+
private func _effectKeyword(for expr: ExprSyntax) -> Keyword? {
24+
switch expr.kind {
25+
case .tryExpr:
26+
return .try
27+
case .awaitExpr:
28+
return .await
29+
case .consumeExpr:
30+
return .consume
31+
case .borrowExpr:
32+
return .borrow
33+
case .unsafeExpr:
34+
return .unsafe
35+
default:
36+
return nil
37+
}
38+
}
39+
40+
/// Determine how to descend further into a syntax node tree from a given node.
41+
///
42+
/// - Parameters:
43+
/// - node: The syntax node currently being walked.
44+
///
45+
/// - Returns: Whether or not to descend into `node` and visit its children.
46+
private func _continueKind(for node: Syntax) -> SyntaxVisitorContinueKind {
47+
switch node.kind {
48+
case .tryExpr, .awaitExpr, .consumeExpr, .borrowExpr, .unsafeExpr:
49+
// If this node represents an effectful expression, look inside it for
50+
// additional such expressions.
51+
return .visitChildren
52+
case .closureExpr, .functionDecl:
53+
// Do not delve into closures or function declarations.
54+
return .skipChildren
55+
case .variableDecl:
56+
// Delve into variable declarations.
57+
return .visitChildren
58+
default:
59+
// Do not delve into declarations other than variables.
60+
if node.isProtocol((any DeclSyntaxProtocol).self) {
61+
return .skipChildren
62+
}
63+
}
64+
65+
// Recurse into everything else.
66+
return .visitChildren
67+
}
68+
1769
/// A syntax visitor class that looks for effectful keywords in a given
1870
/// expression.
1971
private final class _EffectFinder: SyntaxAnyVisitor {
2072
/// The effect keywords discovered so far.
2173
var effectKeywords: Set<Keyword> = []
2274

2375
override func visitAny(_ node: Syntax) -> SyntaxVisitorContinueKind {
24-
switch node.kind {
25-
case .tryExpr:
26-
effectKeywords.insert(.try)
27-
case .awaitExpr:
28-
effectKeywords.insert(.await)
29-
case .consumeExpr:
30-
effectKeywords.insert(.consume)
31-
case .borrowExpr:
32-
effectKeywords.insert(.borrow)
33-
case .unsafeExpr:
34-
effectKeywords.insert(.unsafe)
35-
case .closureExpr, .functionDecl:
36-
// Do not delve into closures or function declarations.
37-
return .skipChildren
38-
case .variableDecl:
39-
// Delve into variable declarations.
40-
return .visitChildren
41-
default:
42-
// Do not delve into declarations other than variables.
43-
if node.isProtocol((any DeclSyntaxProtocol).self) {
44-
return .skipChildren
45-
}
76+
if let expr = node.as(ExprSyntax.self), let keyword = _effectKeyword(for: expr) {
77+
effectKeywords.insert(keyword)
4678
}
4779

48-
// Recurse into everything else.
49-
return .visitChildren
80+
return _continueKind(for: node)
5081
}
5182
}
5283

5384
/// Find effectful keywords in a syntax node.
5485
///
5586
/// - Parameters:
5687
/// - node: The node to inspect.
57-
/// - context: The macro context in which the expression is being parsed.
5888
///
5989
/// - Returns: A set of effectful keywords such as `await` that are present in
6090
/// `node`.
6191
///
6292
/// This function does not descend into function declarations or closure
6393
/// expressions because they represent distinct lexical contexts and their
6494
/// effects are uninteresting in the context of `node` unless they are called.
65-
func findEffectKeywords(in node: some SyntaxProtocol, context: some MacroExpansionContext) -> Set<Keyword> {
66-
// TODO: gather any effects from the lexical context once swift-syntax-#3037 and related PRs land
95+
func findEffectKeywords(in node: some SyntaxProtocol) -> Set<Keyword> {
6796
let effectFinder = _EffectFinder(viewMode: .sourceAccurate)
6897
effectFinder.walk(node)
6998
return effectFinder.effectKeywords
7099
}
71100

101+
/// Find effectful keywords in a macro's lexical context.
102+
///
103+
/// - Parameters:
104+
/// - context: The macro context in which the expression is being parsed.
105+
///
106+
/// - Returns: A set of effectful keywords such as `await` that are present in
107+
/// `context` and would apply to an expression macro during its expansion.
108+
func findEffectKeywords(in context: some MacroExpansionContext) -> Set<Keyword> {
109+
let result = context.lexicalContext.reversed().lazy
110+
.prefix { _continueKind(for: $0) == .visitChildren }
111+
.compactMap { $0.as(ExprSyntax.self) }
112+
.compactMap(_effectKeyword(for:))
113+
return Set(result)
114+
}
115+
72116
extension BidirectionalCollection<Syntax> {
73117
/// The suffix of syntax nodes in this collection which are effectful
74118
/// expressions, such as those for `try` or `await`.
@@ -128,10 +172,13 @@ private func _makeCallToEffectfulThunk(_ thunkName: TokenSyntax, passing expr: s
128172
/// - Parameters:
129173
/// - effectfulKeywords: The effectful keywords to apply.
130174
/// - expr: The expression to apply the keywords and thunk functions to.
175+
/// - insertThunkCalls: Whether or not to also insert calls to thunks to
176+
/// ensure the inserted keywords do not generate warnings. If you aren't
177+
/// sure whether thunk calls are needed, pass `true`.
131178
///
132179
/// - Returns: A copy of `expr` if no changes are needed, or an expression that
133180
/// adds the keywords in `effectfulKeywords` to `expr`.
134-
func applyEffectfulKeywords(_ effectfulKeywords: Set<Keyword>, to expr: some ExprSyntaxProtocol) -> ExprSyntax {
181+
func applyEffectfulKeywords(_ effectfulKeywords: Set<Keyword>, to expr: some ExprSyntaxProtocol, insertThunkCalls: Bool = true) -> ExprSyntax {
135182
let originalExpr = expr
136183
var expr = ExprSyntax(expr.trimmed)
137184

@@ -141,14 +188,16 @@ func applyEffectfulKeywords(_ effectfulKeywords: Set<Keyword>, to expr: some Exp
141188
let needUnsafe = isUnsafeKeywordSupported && effectfulKeywords.contains(.unsafe) && !expr.is(UnsafeExprSyntax.self)
142189

143190
// First, add thunk function calls.
144-
if needAwait {
145-
expr = _makeCallToEffectfulThunk(.identifier("__requiringAwait"), passing: expr)
146-
}
147-
if needTry {
148-
expr = _makeCallToEffectfulThunk(.identifier("__requiringTry"), passing: expr)
149-
}
150-
if needUnsafe {
151-
expr = _makeCallToEffectfulThunk(.identifier("__requiringUnsafe"), passing: expr)
191+
if insertThunkCalls {
192+
if needAwait {
193+
expr = _makeCallToEffectfulThunk(.identifier("__requiringAwait"), passing: expr)
194+
}
195+
if needTry {
196+
expr = _makeCallToEffectfulThunk(.identifier("__requiringTry"), passing: expr)
197+
}
198+
if needUnsafe {
199+
expr = _makeCallToEffectfulThunk(.identifier("__requiringUnsafe"), passing: expr)
200+
}
152201
}
153202

154203
// Then add keyword expressions. (We do this separately so we end up writing

0 commit comments

Comments
 (0)