Skip to content

Commit 65a35bc

Browse files
author
Nathan Hawes
committed
[SourceKit/CodeFormat] Don't column-align PatternBindingDecl entries in certain cases.
Don't column align PBD entries if any entry spans from the same line as the var/let to another line. E.g. ``` // Previous behavior: let foo = someItem .getValue(), // Column-alignment looks ok here, but... bar = otherItem .getValue() getAThing() .andDoStuffWithIt() let foo = someItem .getValue() // looks over-indented here, which is more common. getOtherThing() .andDoStuffWithIt() // New behavior getAThing() .andDoStuffWithIt() let foo = someItem .getValue() // No column alignment in this case... doOtherThing() let foo = someItem .getValue(), // Or in this case (unfortunate, but less common)... bar = otherItem .getValue() let foo = someItem.getValue(), bar = otherItem.getValue() // but still column-aligned in this case. ``` Resolves rdar://problem/63309288
1 parent b981b7e commit 65a35bc

File tree

4 files changed

+81
-22
lines changed

4 files changed

+81
-22
lines changed

lib/IDE/Formatting.cpp

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -992,9 +992,14 @@ class ListAligner {
992992
SourceLoc AlignLoc;
993993
SourceLoc LastEndLoc;
994994
bool HasOutdent = false;
995+
bool BreakAlignment = false;
995996

996997
public:
997998

999+
/// Don't column-align if any element starts on the same line as IntroducerLoc
1000+
/// but ends on a later line.
1001+
bool BreakAlignmentIfSpanning = false;
1002+
9981003
/// Constructs a new \c ListAligner for a list bounded by separate opening and
9991004
/// closing tokens, e.g. tuples, array literals, parameter lists, etc.
10001005
///
@@ -1067,8 +1072,11 @@ class ListAligner {
10671072
assert(Range.isValid());
10681073
LastEndLoc = Range.End;
10691074

1070-
HasOutdent |= isOnSameLine(SM, IntroducerLoc, Range.Start) &&
1071-
OutdentChecker::hasOutdent(SM, Range, WalkableParent);
1075+
if (isOnSameLine(SM, IntroducerLoc, Range.Start)) {
1076+
HasOutdent |= OutdentChecker::hasOutdent(SM, Range, WalkableParent);
1077+
if (BreakAlignmentIfSpanning)
1078+
BreakAlignment |= !isOnSameLine(SM, IntroducerLoc, Range.End);
1079+
}
10721080

10731081
if (HasOutdent || !SM.isBeforeInBuffer(Range.Start, TargetLoc))
10741082
return;
@@ -1127,7 +1135,7 @@ class ListAligner {
11271135
}
11281136

11291137
bool ShouldIndent = shouldIndent(HasTrailingComma, TargetIsTrailing);
1130-
if (ShouldIndent && AlignLoc.isValid()) {
1138+
if (ShouldIndent && !BreakAlignment && AlignLoc.isValid()) {
11311139
setAlignmentIfNeeded(Override);
11321140
return IndentContext {AlignLoc, false, IndentContext::Exact};
11331141
}
@@ -1139,7 +1147,7 @@ class ListAligner {
11391147
/// This should be called before returning an \c IndentContext for a subrange
11401148
/// of the list.
11411149
void setAlignmentIfNeeded(ContextOverride &Override) {
1142-
if (HasOutdent || AlignLoc.isInvalid())
1150+
if (HasOutdent || BreakAlignment || AlignLoc.isInvalid())
11431151
return;
11441152
Override.setExact(SM, AlignLoc);
11451153
}
@@ -1675,6 +1683,36 @@ class FormatWalker : public ASTWalker {
16751683
SourceLoc ContextLoc = PBD->getStartLoc(), IntroducerLoc = PBD->getLoc();
16761684

16771685
ListAligner Aligner(SM, TargetLocation, ContextLoc, IntroducerLoc);
1686+
1687+
// Don't column align PBD entries if any entry spans from the same line as
1688+
// the IntroducerLoc (var/let) to another line. E.g.
1689+
//
1690+
// let foo = someItem
1691+
// .getValue(), // Column-alignment looks ok here, but...
1692+
// bar = otherItem
1693+
// .getValue()
1694+
//
1695+
// getAThing()
1696+
// .andDoStuffWithIt()
1697+
// let foo = someItem
1698+
// .getValue() // looks over-indented here, which is more common...
1699+
// getOtherThing()
1700+
// .andDoStuffWithIt()
1701+
//
1702+
// getAThing()
1703+
// .andDoStuffWithIt()
1704+
// let foo = someItem
1705+
// .getValue() // so break column alignment in this case...
1706+
// doOtherThing()
1707+
//
1708+
// let foo = someItem.getValue(),
1709+
// bar = otherItem.getValue() // but not in this case.
1710+
//
1711+
// Using this rule, rather than handling single and multi-entry PBDs
1712+
// differently, ensures that the as-typed-out indentation matches the
1713+
// re-indented indentation for multi-entry PBDs.
1714+
Aligner.BreakAlignmentIfSpanning = true;
1715+
16781716
for (auto I: range(PBD->getNumPatternEntries())) {
16791717
SourceRange EntryRange = PBD->getEqualLoc(I);
16801718
VarDecl *SingleVar = nullptr;

test/SourceKit/CodeFormat/indent-multiline-string.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,4 +17,4 @@ this is line1,
1717
// CHECK: key.sourcetext: "this is line1,"
1818
// CHECK: key.sourcetext: " this is line2,"
1919
// CHECK: key.sourcetext: "\"\"\""
20-
// CHECK: key.sourcetext: " \"content\""
20+
// CHECK: key.sourcetext: " \"content\""

test/SourceKit/CodeFormat/rdar_32789463.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,5 +12,5 @@ $
1212
1313
// CHECK: key.sourcetext: "struct $ {"
1414
// CHECK: key.sourcetext: " let $: <#Type#>"
15-
// CHECK: key.sourcetext: " = foo(\"foo \\($) bar\") {"
15+
// CHECK: key.sourcetext: " = foo(\"foo \\($) bar\") {"
1616
// CHECK: key.sourcetext: " $"

test/swift-indent/basic.swift

Lines changed: 37 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,9 @@ test(arg1: 1,
6868
}
6969

7070
let x = [1, 2, 3]
71-
.filter {$0 < $1}
72-
.filter {$0 < $1}
73-
.filter {$0 < $1}
71+
.filter {$0 < $1}
72+
.filter {$0 < $1}
73+
.filter {$0 < $1}
7474

7575
bax(34949494949)
7676
.foo(a: Int,
@@ -141,11 +141,11 @@ if #available(
141141
// do the same.
142142
//
143143
let _ = []
144-
.map {
145-
f {
146-
print()
147-
} ?? 0
148-
}
144+
.map {
145+
f {
146+
print()
147+
} ?? 0
148+
}
149149

150150
basename
151151
.foo(a: Int,
@@ -234,10 +234,10 @@ let arrayC = [2]
234234
let arrayD = [3]
235235

236236
let array1 =
237-
arrayA +
238-
arrayB +
239-
arrayC +
240-
arrayD
237+
arrayA +
238+
arrayB +
239+
arrayC +
240+
arrayD
241241

242242
array1 =
243243
arrayA +
@@ -254,9 +254,9 @@ arrayC +
254254
arrayD
255255

256256
let array2 = arrayA +
257-
arrayB +
258-
arrayC +
259-
arrayD
257+
arrayB +
258+
arrayC +
259+
arrayD
260260

261261

262262
// Comments should not break exact alignment, and leading comments should be aligned, rather than the label.
@@ -867,7 +867,7 @@ func foo(
867867
) {}
868868

869869
var (d, e):
870-
(Int, Int) = (1, 3),
870+
(Int, Int) = (1, 3),
871871
(f, g): (
872872
Int,
873873
Int
@@ -1015,3 +1015,24 @@ catch MyErr.a(let code, let message),
10151015
{
10161016
print("ahhh!")
10171017
}
1018+
1019+
// Pattern binding decls should only column-align if no element spans from the first line to beyond it.
1020+
1021+
public let x = 10,
1022+
y = 20
1023+
1024+
private var firstThing = 20,
1025+
secondThing = item
1026+
.filter {},
1027+
thirdThing = 42
1028+
1029+
public let myVar = itemWithALongName
1030+
.filter { $0 >= $1 && $0 - $1 < 50}
1031+
1032+
public let first = 45, second = itemWithALongName
1033+
.filter { $0 >= $1 && $0 - $1 < 50}
1034+
1035+
private var secondThing = item
1036+
.filter {},
1037+
firstThing = 20,
1038+
thirdThing = 56

0 commit comments

Comments
 (0)