Skip to content

Commit 12a4763

Browse files
committed
Improve wrapping of parenthesized expressions.
Since paren exprs are just represented as one-element tuples, they were getting the same kind of wrapping as tuples, with breaks inside the parens. This doesn't look great, because these are different kinds of expressions. Now, we keep the parens inseparable from the tokens that they are adjacent to, which lets them wrap more traditionally, and we increase the continuation indentation inside them. This required some changes to the way we stack indentation for variable initialization and assignment to make multiple parenthesized expressions line up nicely. For example, whereas we used to do this: ```swift x = foo + bar + baz ``` Now we do ```swift x = foo + bar + baz ``` ...so that if those terms are paren exprs and they wrap inside, they still shift in and out nicely and line up well: ```swift x = (foo + foo2) + (bar + bar2) + (baz + baz2) ``` We find the loss of the original extra level of indentation to be acceptable because the subsequent lines still start with a spaced operator or some other token (like a prefix dot) that indicates that the line is unambiguously a continuation line.
1 parent 4837fec commit 12a4763

14 files changed

+416
-128
lines changed

Sources/SwiftFormatPrettyPrint/PrettyPrint.swift

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,6 @@ public class PrettyPrinter {
292292
// scope. Additionally, continuation open breaks must indent when the break fires.
293293
let continuationBreakWillFire = openKind == .continuation
294294
&& (isAtStartOfLine || length > spaceRemaining || mustBreak)
295-
&& !isBreakingSupressed
296295
let contributesContinuationIndent = currentLineIsContinuation || continuationBreakWillFire
297296

298297
activeOpenBreaks.append(

Sources/SwiftFormatPrettyPrint/TokenStreamCreator.swift

Lines changed: 112 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -601,13 +601,26 @@ private final class TokenStreamCreator: SyntaxVisitor {
601601
}
602602

603603
func visit(_ node: TupleExprSyntax) -> SyntaxVisitorContinueKind {
604-
after(node.leftParen, tokens: .break(.open, size: 0), .open)
605-
before(node.rightParen, tokens: .close, .break(.close, size: 0))
606-
607-
insertTokens(.break(.same), betweenElementsOf: node.elementList)
608-
609-
for element in node.elementList {
610-
arrangeAsTupleExprElement(element)
604+
// We'll do nothing if it's a zero-element tuple, because we just want to keep the empty `()`
605+
// together.
606+
let elementCount = node.elementList.count
607+
608+
if elementCount == 1 {
609+
// A tuple with one element is a parenthesized expression; add a group around it to keep it
610+
// together when possible, but breaks are handled elsewhere (see calls to
611+
// `stackedIndentationBehavior`).
612+
after(node.leftParen, tokens: .open)
613+
before(node.rightParen, tokens: .close)
614+
} else if elementCount > 1 {
615+
// Tuples with more than one element are "true" tuples, and should indent as block structures.
616+
after(node.leftParen, tokens: .break(.open, size: 0), .open)
617+
before(node.rightParen, tokens: .close, .break(.close, size: 0))
618+
619+
insertTokens(.break(.same), betweenElementsOf: node.elementList)
620+
621+
for element in node.elementList {
622+
arrangeAsTupleExprElement(element)
623+
}
611624
}
612625

613626
return .visitChildren
@@ -1296,35 +1309,47 @@ private final class TokenStreamCreator: SyntaxVisitor {
12961309

12971310
if shouldRequireWhitespace(around: binOp) {
12981311
if isAssigningOperator(binOp) {
1299-
var beforeTokens: [Token] = [.break(.open(kind: .continuation))]
1300-
var afterTokens: [Token] = [.break(.close(mustBreak: false), size: 0)]
1312+
var beforeTokens: [Token]
1313+
1314+
// If the rhs starts with a parenthesized expression, stack indentation around it.
1315+
// Otherwise, use regular continuation breaks.
1316+
if let (unindentingNode, _) = stackedIndentationBehavior(after: binOp, rhs: rhs) {
1317+
beforeTokens = [.break(.open(kind: .continuation))]
1318+
after(unindentingNode.lastToken, tokens: [.break(.close(mustBreak: false), size: 0)])
1319+
} else {
1320+
beforeTokens = [.break(.continue)]
1321+
}
13011322

13021323
// When the RHS is a simple expression, even if is requires multiple lines, we don't add a
13031324
// group so that as much of the expression as possible can stay on the same line as the
13041325
// operator token.
13051326
if isCompoundExpression(rhs) {
13061327
beforeTokens.append(.open)
1307-
afterTokens.append(.close)
1328+
after(rhs.lastToken, tokens: .close)
13081329
}
1330+
13091331
after(binOp.lastToken, tokens: beforeTokens)
1310-
after(rhs.lastToken, tokens: afterTokens)
1311-
} else if shouldStackIndentation(after: binOp) {
1312-
// For certain operators like `&&` and `||`, we don't want to treat all continue breaks
1313-
// the same. If we did, then all operators would line up at the same alignment regardless
1314-
// of whether they were, for example, `&&` or something between a pair of `&&`. To make
1315-
// long conditionals format more cleanly, we use open-continuation/close pairs around such
1316-
// operators and their right-hand sides so that the continuation breaks inside those
1317-
// scopes "stack", instead of receiving the usual single-level "continuation line or not"
1318-
// behavior.
1332+
} else if let (unindentingNode, shouldReset) =
1333+
stackedIndentationBehavior(after: binOp, rhs: rhs)
1334+
{
1335+
// For parenthesized expressions and for unparenthesized usages of `&&` and `||`, we don't
1336+
// want to treat all continue breaks the same. If we did, then all operators would line up
1337+
// at the same alignment regardless of whether they were, for example, `&&` or something
1338+
// between a pair of `&&`. To make long expressions/conditionals format more cleanly, we
1339+
// use open-continuation/close pairs around such operators and their right-hand sides so
1340+
// that the continuation breaks inside those scopes "stack", instead of receiving the
1341+
// usual single-level "continuation line or not" behavior.
13191342
let openBreakTokens: [Token] = [.break(.open(kind: .continuation)), .open]
13201343
if wrapsBeforeOperator {
13211344
before(binOp.firstToken, tokens: openBreakTokens)
13221345
} else {
13231346
after(binOp.lastToken, tokens: openBreakTokens)
13241347
}
1325-
after(
1326-
rhs.lastToken,
1327-
tokens: [.break(.reset, size: 0), .break(.close(mustBreak: false), size: 0), .close])
1348+
1349+
let closeBreakTokens: [Token] =
1350+
(shouldReset ? [.break(.reset, size: 0)] : [])
1351+
+ [.break(.close(mustBreak: false), size: 0), .close]
1352+
after(unindentingNode.lastToken, tokens: closeBreakTokens)
13281353
} else {
13291354
if wrapsBeforeOperator {
13301355
before(binOp.firstToken, tokens: .break(.continue))
@@ -1422,14 +1447,19 @@ private final class TokenStreamCreator: SyntaxVisitor {
14221447
closeAfterToken = typeAnnotation.lastToken
14231448
}
14241449
if let initializer = node.initializer {
1425-
after(initializer.equal, tokens: .break(.open(kind: .continuation)))
1426-
closesNeeded += 1
1450+
let expr = initializer.value
1451+
1452+
if let (unindentingNode, _) = stackedIndentationBehavior(rhs: expr) {
1453+
after(initializer.equal, tokens: .break(.open(kind: .continuation)))
1454+
after(unindentingNode.lastToken, tokens: .break(.close(mustBreak: false), size: 0))
1455+
} else {
1456+
after(initializer.equal, tokens: .break(.continue))
1457+
}
14271458
closeAfterToken = initializer.lastToken
14281459

14291460
// When the RHS is a simple expression, even if is requires multiple lines, we don't add a
14301461
// group so that as much of the expression as possible can stay on the same line as the
14311462
// operator token.
1432-
let expr = initializer.value
14331463
if isCompoundExpression(expr) {
14341464
before(expr.firstToken, tokens: .open)
14351465
after(expr.lastToken, tokens: .close)
@@ -2474,13 +2504,16 @@ private final class TokenStreamCreator: SyntaxVisitor {
24742504
/// that are known to wrap an expressions, e.g. try expressions, are handled by checking the
24752505
/// expression that they contain.
24762506
private func isCompoundExpression(_ expr: ExprSyntax) -> Bool {
2477-
if let sequenceExpr = expr as? SequenceExprSyntax {
2507+
switch expr {
2508+
case let sequenceExpr as SequenceExprSyntax:
24782509
return sequenceExpr.elements.count > 1
2479-
}
2480-
if let tryExpr = expr as? TryExprSyntax {
2510+
case let tryExpr as TryExprSyntax:
24812511
return isCompoundExpression(tryExpr.expression)
2512+
case let tupleExpr as TupleExprSyntax where tupleExpr.elementList.count == 1:
2513+
return isCompoundExpression(tupleExpr.elementList.first!.expression)
2514+
default:
2515+
return false
24822516
}
2483-
return false
24842517
}
24852518

24862519
/// Returns whether the given operator behaves as an assignment, to assign a right-hand-side to a
@@ -2503,24 +2536,65 @@ private final class TokenStreamCreator: SyntaxVisitor {
25032536
return false
25042537
}
25052538

2506-
/// Returns a value indicating whether indentation should be stacked within subexpressions to the
2507-
/// right of the given operator.
2539+
/// Walks the expression and returns the leftmost subexpression if it is parenthesized (which
2540+
/// might be the expression itself).
2541+
///
2542+
/// - Parameter expr: The expression whose parenthesized leftmost subexpression should be
2543+
/// returned.
2544+
/// - Returns: The parenthesized leftmost subexpression, or nil if the leftmost subexpression was
2545+
/// not parenthesized.
2546+
private func parenthesizedLeftmostExpr(of expr: ExprSyntax) -> TupleExprSyntax? {
2547+
switch expr {
2548+
case let tupleExpr as TupleExprSyntax where tupleExpr.elementList.count == 1:
2549+
return tupleExpr
2550+
case let sequenceExpr as SequenceExprSyntax:
2551+
return parenthesizedLeftmostExpr(of: sequenceExpr.elements.first!)
2552+
case let ternaryExpr as TernaryExprSyntax:
2553+
return parenthesizedLeftmostExpr(of: ternaryExpr.conditionExpression)
2554+
default:
2555+
return nil
2556+
}
2557+
}
2558+
2559+
/// Determines if indentation should be stacked around a subexpression to the right of the given
2560+
/// operator, and, if so, returns the node after which indentation stacking should be closed and
2561+
/// whether or not the continuation state should be reset as well.
25082562
///
2509-
/// Operators that are good candidates for this behavior are ones that have low precedence and
2510-
/// may occur in chains, such as logical AND (`&&`) and OR (`||`) in conditional statements, where
2511-
/// the extra level of indentation helps to improve readability with the operators inside those
2512-
/// conditions.
2513-
private func shouldStackIndentation(after operatorExpr: ExprSyntax) -> Bool {
2563+
/// Stacking is applied around parenthesized expressions, but also for low-precedence operators
2564+
/// that frequently occur in long chains, such as logical AND (`&&`) and OR (`||`) in conditional
2565+
/// statements. In this case, the extra level of indentation helps to improve readability with the
2566+
/// operators inside those conditions even when parentheses are not used.
2567+
private func stackedIndentationBehavior(
2568+
after operatorExpr: ExprSyntax? = nil,
2569+
rhs: ExprSyntax
2570+
) -> (unindentingNode: ExprSyntax, shouldReset: Bool)? {
2571+
// Check for logical operators first, and if it's that kind of operator, stack indentation
2572+
// around the entire right-hand-side. We have to do this check before checking the RHS for
2573+
// parentheses because if the user writes something like `... && (foo) > bar || ...`, we don't
2574+
// want the indentation stacking that starts before the `&&` to stop after the closing
2575+
// parenthesis in `(foo)`.
2576+
//
2577+
// We also want to reset after undoing the stacked indentation so that we have a visual
2578+
// indication that the subexpression has ended.
25142579
if let binaryOperator = operatorExpr as? BinaryOperatorExprSyntax {
25152580
let operatorText = binaryOperator.operatorToken.text
25162581
if let precedence = operatorContext.infixOperator(named: operatorText)?.precedenceGroup,
25172582
precedence === operatorContext.precedenceGroup(named: .logicalConjunction)
25182583
|| precedence === operatorContext.precedenceGroup(named: .logicalDisjunction)
25192584
{
2520-
return true
2585+
return (unindentingNode: rhs, shouldReset: true)
25212586
}
25222587
}
2523-
return false
2588+
2589+
// If the right-hand-side of the operator is or starts with a parenthesized expression, stack
2590+
// indentation around the operator and those parentheses. We don't need to reset here because
2591+
// the parentheses are sufficient to provide a visual indication of the nesting relationship.
2592+
if let parenthesizedExpr = parenthesizedLeftmostExpr(of: rhs) {
2593+
return (unindentingNode: parenthesizedExpr, shouldReset: false)
2594+
}
2595+
2596+
// Otherwise, don't stack--use regular continuation breaks instead.
2597+
return nil
25242598
}
25252599

25262600
/// Returns a value indicating whether whitespace should be required around the given operator.

Tests/SwiftFormatPrettyPrintTests/AssignmentExprTests.swift

Lines changed: 47 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,13 @@ public class AssignmentExprTests: PrettyPrintTestCase {
3232
"""
3333
someVeryLongVariableName =
3434
anotherPrettyLongVariableName
35-
&& someOtherOperand
35+
&& someOtherOperand
3636
shortName =
3737
wxyz + aaaaaa + bbbbbb
38-
+ cccccc
38+
+ cccccc
3939
longerName =
4040
wxyz + aaaaaa + bbbbbb
41-
+ cccccc || zzzzzzz
41+
+ cccccc || zzzzzzz
4242
4343
"""
4444

@@ -60,27 +60,27 @@ public class AssignmentExprTests: PrettyPrintTestCase {
6060
"""
6161
result =
6262
firstOp + secondOp
63-
+ someOpFetchingFunc(
64-
foo, bar: bar, baz: baz)
63+
+ someOpFetchingFunc(
64+
foo, bar: bar, baz: baz)
6565
result = someOpFetchingFunc(
6666
foo, bar: bar, baz: baz)
6767
result += someOpFetchingFunc(
6868
foo, bar: bar, baz: baz)
6969
result =
7070
someOpFetchingFunc(
7171
foo, bar: bar, baz: baz)
72-
+ someOtherOperand
73-
+ andAThirdOneForReasons
72+
+ someOtherOperand
73+
+ andAThirdOneForReasons
7474
result =
7575
firstOp + secondOp + thirdOp
76-
+ someOpFetchingFunc(
77-
foo, bar, baz) + nextOp
78-
+ lastOp
76+
+ someOpFetchingFunc(
77+
foo, bar, baz) + nextOp
78+
+ lastOp
7979
result +=
8080
firstOp + secondOp + thirdOp
81-
+ someOpFetchingFunc(
82-
foo, bar, baz) + nextOp
83-
+ lastOp
81+
+ someOpFetchingFunc(
82+
foo, bar, baz) + nextOp
83+
+ lastOp
8484
8585
"""
8686

@@ -93,11 +93,11 @@ public class AssignmentExprTests: PrettyPrintTestCase {
9393
"""
9494
result =
9595
firstOp + secondOp
96-
+ someOpFetchingFunc(
97-
foo,
98-
bar: bar,
99-
baz: baz
100-
)
96+
+ someOpFetchingFunc(
97+
foo,
98+
bar: bar,
99+
baz: baz
100+
)
101101
result = someOpFetchingFunc(
102102
foo,
103103
bar: bar,
@@ -114,21 +114,21 @@ public class AssignmentExprTests: PrettyPrintTestCase {
114114
bar: bar,
115115
baz: baz
116116
) + someOtherOperand
117-
+ andAThirdOneForReasons
117+
+ andAThirdOneForReasons
118118
result =
119119
firstOp + secondOp + thirdOp
120-
+ someOpFetchingFunc(
121-
foo,
122-
bar,
123-
baz
124-
) + nextOp + lastOp
120+
+ someOpFetchingFunc(
121+
foo,
122+
bar,
123+
baz
124+
) + nextOp + lastOp
125125
result +=
126126
firstOp + secondOp + thirdOp
127-
+ someOpFetchingFunc(
128-
foo,
129-
bar,
130-
baz
131-
) + nextOp + lastOp
127+
+ someOpFetchingFunc(
128+
foo,
129+
bar,
130+
baz
131+
) + nextOp + lastOp
132132
133133
"""
134134
config.lineBreakBeforeEachArgument = true
@@ -149,20 +149,20 @@ public class AssignmentExprTests: PrettyPrintTestCase {
149149
"""
150150
let result =
151151
firstOp + secondOp
152-
+ someOpFetchingFunc(
153-
foo, bar: bar, baz: baz)
152+
+ someOpFetchingFunc(
153+
foo, bar: bar, baz: baz)
154154
let result = someOpFetchingFunc(
155155
foo, bar: bar, baz: baz)
156156
let result =
157157
someOpFetchingFunc(
158158
foo, bar: bar, baz: baz)
159-
+ someOtherOperand
160-
+ andAThirdOneForReasons
159+
+ someOtherOperand
160+
+ andAThirdOneForReasons
161161
let result =
162162
firstOp + secondOp + thirdOp
163-
+ someOpFetchingFunc(
164-
foo, bar, baz) + nextOp
165-
+ lastOp
163+
+ someOpFetchingFunc(
164+
foo, bar, baz) + nextOp
165+
+ lastOp
166166
167167
"""
168168

@@ -175,11 +175,11 @@ public class AssignmentExprTests: PrettyPrintTestCase {
175175
"""
176176
let result =
177177
firstOp + secondOp
178-
+ someOpFetchingFunc(
179-
foo,
180-
bar: bar,
181-
baz: baz
182-
)
178+
+ someOpFetchingFunc(
179+
foo,
180+
bar: bar,
181+
baz: baz
182+
)
183183
let result = someOpFetchingFunc(
184184
foo,
185185
bar: bar,
@@ -191,14 +191,14 @@ public class AssignmentExprTests: PrettyPrintTestCase {
191191
bar: bar,
192192
baz: baz
193193
) + someOtherOperand
194-
+ andAThirdOneForReasons
194+
+ andAThirdOneForReasons
195195
let result =
196196
firstOp + secondOp + thirdOp
197-
+ someOpFetchingFunc(
198-
foo,
199-
bar,
200-
baz
201-
) + nextOp + lastOp
197+
+ someOpFetchingFunc(
198+
foo,
199+
bar,
200+
baz
201+
) + nextOp + lastOp
202202
203203
"""
204204
config.lineBreakBeforeEachArgument = true

0 commit comments

Comments
 (0)