Skip to content

Commit 7d75e3a

Browse files
committed
Add parentheses when needed to convert let _ = expr to expr != nil.
Parens are required if `expr` is a `try` expression, a ternary expression, or an infix operator expression whose operator has lower precedence than `!=`.
1 parent bb70bc2 commit 7d75e3a

File tree

2 files changed

+103
-1
lines changed

2 files changed

+103
-1
lines changed

Sources/SwiftFormat/Rules/UseExplicitNilCheckInConditions.swift

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ public final class UseExplicitNilCheckInConditions: SyntaxFormatRule {
4747
operatorExpr.trailingTrivia = [.spaces(1)]
4848

4949
var inequalExpr = InfixOperatorExprSyntax(
50-
leftOperand: value,
50+
leftOperand: addingParenthesesIfNecessary(to: value),
5151
operator: operatorExpr,
5252
rightOperand: NilLiteralExprSyntax())
5353
inequalExpr.leadingTrivia = node.leadingTrivia
@@ -69,6 +69,54 @@ public final class UseExplicitNilCheckInConditions: SyntaxFormatRule {
6969
}
7070
return exprPattern.expression.is(DiscardAssignmentExprSyntax.self)
7171
}
72+
73+
/// Adds parentheses around the given expression if necessary to ensure that it will be parsed
74+
/// correctly when followed by `!= nil`.
75+
///
76+
/// Specifically, if `expr` is a `try` expression, ternary expression, or an infix operator with
77+
/// the same or lower precedence, we wrap it.
78+
private func addingParenthesesIfNecessary(to expr: ExprSyntax) -> ExprSyntax {
79+
func addingParentheses(to expr: ExprSyntax) -> ExprSyntax {
80+
var expr = expr
81+
let leadingTrivia = expr.leadingTrivia
82+
let trailingTrivia = expr.trailingTrivia
83+
expr.leadingTrivia = []
84+
expr.trailingTrivia = []
85+
86+
var tupleExpr = TupleExprSyntax(elements: [LabeledExprSyntax(expression: expr)])
87+
tupleExpr.leadingTrivia = leadingTrivia
88+
tupleExpr.trailingTrivia = trailingTrivia
89+
return ExprSyntax(tupleExpr)
90+
}
91+
92+
switch Syntax(expr).as(SyntaxEnum.self) {
93+
case .tryExpr, .ternaryExpr:
94+
return addingParentheses(to: expr)
95+
96+
case .infixOperatorExpr:
97+
// There's no public API in SwiftSyntax to get the relationship between two precedence groups.
98+
// Until that exists, here's a workaround I'm only mildly ashamed of: we reparse
99+
// "\(expr) != nil" and then fold it. If the top-level node is anything but an
100+
// `InfixOperatorExpr` whose operator is `!=` and whose RHS is `nil`, then it parsed
101+
// incorrectly and we need to add parentheses around `expr`.
102+
//
103+
// Note that we could also cover the `tryExpr` and `ternaryExpr` cases above with this, but
104+
// this reparsing trick is going to be slower so we should avoid it whenever we can.
105+
let reparsedExpr = "\(expr) != nil" as ExprSyntax
106+
if
107+
let infixExpr = reparsedExpr.as(InfixOperatorExprSyntax.self),
108+
let binOp = infixExpr.operator.as(BinaryOperatorExprSyntax.self),
109+
binOp.operator.text == "!=",
110+
infixExpr.rightOperand.is(NilLiteralExprSyntax.self)
111+
{
112+
return expr
113+
}
114+
return addingParentheses(to: expr)
115+
116+
default:
117+
return expr
118+
}
119+
}
72120
}
73121

74122
extension Finding.Message {

Tests/SwiftFormatTests/Rules/UseExplicitNilCheckInConditionsTests.swift

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,4 +95,58 @@ final class UseExplicitNilCheckInConditionsTests: LintOrFormatRuleTestCase {
9595
]
9696
)
9797
}
98+
99+
func testAddNecessaryParenthesesAroundTryExpr() {
100+
assertFormatting(
101+
UseExplicitNilCheckInConditions.self,
102+
input: """
103+
if 1️⃣let _ = try? x {}
104+
if 2️⃣let _ = try x {}
105+
""",
106+
expected: """
107+
if (try? x) != nil {}
108+
if (try x) != nil {}
109+
""",
110+
findings: [
111+
FindingSpec("1️⃣", message: "compare this value using `!= nil` instead of binding and discarding it"),
112+
FindingSpec("2️⃣", message: "compare this value using `!= nil` instead of binding and discarding it"),
113+
]
114+
)
115+
}
116+
117+
func testAddNecessaryParenthesesAroundTernaryExpr() {
118+
assertFormatting(
119+
UseExplicitNilCheckInConditions.self,
120+
input: """
121+
if 1️⃣let _ = x ? y : z {}
122+
""",
123+
expected: """
124+
if (x ? y : z) != nil {}
125+
""",
126+
findings: [
127+
FindingSpec("1️⃣", message: "compare this value using `!= nil` instead of binding and discarding it"),
128+
]
129+
)
130+
}
131+
132+
func testAddNecessaryParenthesesAroundSameOrLowerPrecedenceOperator() {
133+
// The use of `&&` and `==` are semantically meaningless here because they don't return
134+
// optionals. We just need them to stand in for any potential custom operator with lower or same
135+
// precedence, respectively.
136+
assertFormatting(
137+
UseExplicitNilCheckInConditions.self,
138+
input: """
139+
if 1️⃣let _ = x && y {}
140+
if 2️⃣let _ = x == y {}
141+
""",
142+
expected: """
143+
if (x && y) != nil {}
144+
if (x == y) != nil {}
145+
""",
146+
findings: [
147+
FindingSpec("1️⃣", message: "compare this value using `!= nil` instead of binding and discarding it"),
148+
FindingSpec("2️⃣", message: "compare this value using `!= nil` instead of binding and discarding it"),
149+
]
150+
)
151+
}
98152
}

0 commit comments

Comments
 (0)