Skip to content

Commit 3c43044

Browse files
committed
Suppress the complex expansion of expectations when we see effects in the lexical context.
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.
1 parent b97b576 commit 3c43044

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)