Skip to content

Commit 4fb999d

Browse files
committed
Fix try/await expressions in NoAssignmentInExpressions.
This rule only checked whether the _immediate_ parent of an expression like `x = y` was a `CodeBlockItem`. In cases like `try x = y`, the `try` wraps the entire expression and the `CodeBlockItem` would be the parent of that. So we look through any `TryExpr`s or `AwaitExpr`s we see when searching for the `CodeBlockItem`.
1 parent 420ca57 commit 4fb999d

File tree

2 files changed

+46
-2
lines changed

2 files changed

+46
-2
lines changed

Sources/SwiftFormatRules/NoAssignmentInExpressions.swift

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public final class NoAssignmentInExpressions: SyntaxFormatRule {
2727
public override func visit(_ node: InfixOperatorExprSyntax) -> ExprSyntax {
2828
// Diagnose any assignment that isn't directly a child of a `CodeBlockItem` (which would be the
2929
// case if it was its own statement).
30-
if isAssignmentExpression(node) && node.parent?.is(CodeBlockItemSyntax.self) == false {
30+
if isAssignmentExpression(node) && !isStandaloneAssignmentStatement(node) {
3131
diagnose(.moveAssignmentToOwnStatement, on: node)
3232
}
3333
return ExprSyntax(node)
@@ -59,7 +59,8 @@ public final class NoAssignmentInExpressions: SyntaxFormatRule {
5959
item: .expr(ExprSyntax(assignmentExpr)),
6060
semicolon: nil
6161
)
62-
.with(\.leadingTrivia,
62+
.with(
63+
\.leadingTrivia,
6364
(returnStmt.leadingTrivia) + (assignmentExpr.leadingTrivia))
6465
.with(\.trailingTrivia, []))
6566
newItems.append(
@@ -106,6 +107,30 @@ public final class NoAssignmentInExpressions: SyntaxFormatRule {
106107
return context.operatorTable.infixOperator(named: binaryOp.operatorToken.text)?.precedenceGroup
107108
== "AssignmentPrecedence"
108109
}
110+
111+
/// Returns a value indicating whether the given node is a standalone assignment statement.
112+
///
113+
/// This function considers try/await expressions and automatically walks up through them as
114+
/// needed. This is because `try f().x = y` should still be a standalone assignment for our
115+
/// purposes, even though a `TryExpr` will wrap the `InfixOperatorExpr` and thus would not be
116+
/// considered a standalone assignment if we only checked the infix expression for a
117+
/// `CodeBlockItem` parent.
118+
private func isStandaloneAssignmentStatement(_ node: InfixOperatorExprSyntax) -> Bool {
119+
var node = Syntax(node)
120+
while
121+
let parent = node.parent,
122+
parent.is(TryExprSyntax.self) || parent.is(AwaitExprSyntax.self)
123+
{
124+
node = parent
125+
}
126+
127+
guard let parent = node.parent else {
128+
// This shouldn't happen under normal circumstances (i.e., unless the expression is detached
129+
// from the rest of a tree). In that case, we may as well consider it to be "standalone".
130+
return true
131+
}
132+
return parent.is(CodeBlockItemSyntax.self)
133+
}
109134
}
110135

111136
extension Finding.Message {

Tests/SwiftFormatRulesTests/NoAssignmentInExpressionsTests.swift

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,4 +161,23 @@ final class NoAssignmentInExpressionsTests: LintOrFormatRuleTestCase {
161161
)
162162
XCTAssertDiagnosed(.moveAssignmentToOwnStatement, line: 2, column: 29)
163163
}
164+
165+
func testTryAndAwaitAssignmentExpressionsAreUnchanged() {
166+
XCTAssertFormatting(
167+
NoAssignmentInExpressions.self,
168+
input: """
169+
func foo() {
170+
try a.b = c
171+
await a.b = c
172+
}
173+
""",
174+
expected: """
175+
func foo() {
176+
try a.b = c
177+
await a.b = c
178+
}
179+
"""
180+
)
181+
XCTAssertNotDiagnosed(.moveAssignmentToOwnStatement)
182+
}
164183
}

0 commit comments

Comments
 (0)