Skip to content

[5.3][SourceKit/CodeFormat] Fix var/let declaration continuation lines appearing over-indented. #31945

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 42 additions & 4 deletions lib/IDE/Formatting.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -992,9 +992,14 @@ class ListAligner {
SourceLoc AlignLoc;
SourceLoc LastEndLoc;
bool HasOutdent = false;
bool BreakAlignment = false;

public:

/// Don't column-align if any element starts on the same line as IntroducerLoc
/// but ends on a later line.
bool BreakAlignmentIfSpanning = false;

/// Constructs a new \c ListAligner for a list bounded by separate opening and
/// closing tokens, e.g. tuples, array literals, parameter lists, etc.
///
Expand Down Expand Up @@ -1067,8 +1072,11 @@ class ListAligner {
assert(Range.isValid());
LastEndLoc = Range.End;

HasOutdent |= isOnSameLine(SM, IntroducerLoc, Range.Start) &&
OutdentChecker::hasOutdent(SM, Range, WalkableParent);
if (isOnSameLine(SM, IntroducerLoc, Range.Start)) {
HasOutdent |= OutdentChecker::hasOutdent(SM, Range, WalkableParent);
if (BreakAlignmentIfSpanning)
BreakAlignment |= !isOnSameLine(SM, IntroducerLoc, Range.End);
}

if (HasOutdent || !SM.isBeforeInBuffer(Range.Start, TargetLoc))
return;
Expand Down Expand Up @@ -1127,7 +1135,7 @@ class ListAligner {
}

bool ShouldIndent = shouldIndent(HasTrailingComma, TargetIsTrailing);
if (ShouldIndent && AlignLoc.isValid()) {
if (ShouldIndent && !BreakAlignment && AlignLoc.isValid()) {
setAlignmentIfNeeded(Override);
return IndentContext {AlignLoc, false, IndentContext::Exact};
}
Expand All @@ -1139,7 +1147,7 @@ class ListAligner {
/// This should be called before returning an \c IndentContext for a subrange
/// of the list.
void setAlignmentIfNeeded(ContextOverride &Override) {
if (HasOutdent || AlignLoc.isInvalid())
if (HasOutdent || BreakAlignment || AlignLoc.isInvalid())
return;
Override.setExact(SM, AlignLoc);
}
Expand Down Expand Up @@ -1675,6 +1683,36 @@ class FormatWalker : public ASTWalker {
SourceLoc ContextLoc = PBD->getStartLoc(), IntroducerLoc = PBD->getLoc();

ListAligner Aligner(SM, TargetLocation, ContextLoc, IntroducerLoc);

// Don't column align PBD entries if any entry spans from the same line as
// the IntroducerLoc (var/let) to another line. E.g.
//
// 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()
//
// getAThing()
// .andDoStuffWithIt()
// let foo = someItem
// .getValue() // so break column alignment in this case...
// doOtherThing()
//
// let foo = someItem.getValue(),
// bar = otherItem.getValue() // but not in this case.
//
// Using this rule, rather than handling single and multi-entry PBDs
// differently, ensures that the as-typed-out indentation matches the
// re-indented indentation for multi-entry PBDs.
Aligner.BreakAlignmentIfSpanning = true;

for (auto I: range(PBD->getNumPatternEntries())) {
SourceRange EntryRange = PBD->getEqualLoc(I);
VarDecl *SingleVar = nullptr;
Expand Down
2 changes: 1 addition & 1 deletion test/SourceKit/CodeFormat/indent-multiline-string.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,4 @@ this is line1,
// CHECK: key.sourcetext: "this is line1,"
// CHECK: key.sourcetext: " this is line2,"
// CHECK: key.sourcetext: "\"\"\""
// CHECK: key.sourcetext: " \"content\""
// CHECK: key.sourcetext: " \"content\""
2 changes: 1 addition & 1 deletion test/SourceKit/CodeFormat/rdar_32789463.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,5 @@ $

// CHECK: key.sourcetext: "struct $ {"
// CHECK: key.sourcetext: " let $: <#Type#>"
// CHECK: key.sourcetext: " = foo(\"foo \\($) bar\") {"
// CHECK: key.sourcetext: " = foo(\"foo \\($) bar\") {"
// CHECK: key.sourcetext: " $"
53 changes: 37 additions & 16 deletions test/swift-indent/basic.swift
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,9 @@ test(arg1: 1,
}

let x = [1, 2, 3]
.filter {$0 < $1}
.filter {$0 < $1}
.filter {$0 < $1}
.filter {$0 < $1}
.filter {$0 < $1}
.filter {$0 < $1}

bax(34949494949)
.foo(a: Int,
Expand Down Expand Up @@ -141,11 +141,11 @@ if #available(
// do the same.
//
let _ = []
.map {
f {
print()
} ?? 0
}
.map {
f {
print()
} ?? 0
}

basename
.foo(a: Int,
Expand Down Expand Up @@ -234,10 +234,10 @@ let arrayC = [2]
let arrayD = [3]

let array1 =
arrayA +
arrayB +
arrayC +
arrayD
arrayA +
arrayB +
arrayC +
arrayD

array1 =
arrayA +
Expand All @@ -254,9 +254,9 @@ arrayC +
arrayD

let array2 = arrayA +
arrayB +
arrayC +
arrayD
arrayB +
arrayC +
arrayD


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

var (d, e):
(Int, Int) = (1, 3),
(Int, Int) = (1, 3),
(f, g): (
Int,
Int
Expand Down Expand Up @@ -1015,3 +1015,24 @@ catch MyErr.a(let code, let message),
{
print("ahhh!")
}

// Pattern binding decls should only column-align if no element spans from the first line to beyond it.

public let x = 10,
y = 20

private var firstThing = 20,
secondThing = item
.filter {},
thirdThing = 42

public let myVar = itemWithALongName
.filter { $0 >= $1 && $0 - $1 < 50}

public let first = 45, second = itemWithALongName
.filter { $0 >= $1 && $0 - $1 < 50}

private var secondThing = item
.filter {},
firstThing = 20,
thirdThing = 56