Skip to content

Commit

Permalink
Merge branch 'master' into blank-line-after-script
Browse files Browse the repository at this point in the history
# Conflicts:
#	CHANGELOG.md
  • Loading branch information
munificent committed Mar 19, 2020
2 parents e5ad7d8 + 8cd12c3 commit 79784cc
Show file tree
Hide file tree
Showing 9 changed files with 149 additions and 32 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
* Correctly handle `var` in `--fix-function-typedefs` (#826).
* Preserve leading indentation in fixed doc comments (#821).
* Add `--verbose` to hide advanced options in `--help` output
* Add `--verbose` to hide advanced options in `--help` output.
* Split outer nested control flow elements (#869).
* Always place a blank line after script tags (#782).

# 1.3.3
Expand Down
24 changes: 22 additions & 2 deletions lib/src/source_visitor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,9 @@ class SourceVisitor extends ThrowingAstVisitor {
return false;
}

static bool _isControlFlowElement(AstNode node) =>
node is IfElement || node is ForElement;

/// The builder for the block that is currently being visited.
ChunkBuilder builder;

Expand Down Expand Up @@ -1561,6 +1564,11 @@ class SourceVisitor extends ThrowingAstVisitor {

if (!isSpreadBody) builder.endBlockArgumentNesting();
builder.unnest();

// If a control flow element is nested inside another, force the outer one
// to split.
if (_isControlFlowElement(node.body)) builder.forceRules();

builder.endRule();
}

Expand Down Expand Up @@ -1867,7 +1875,6 @@ class SourceVisitor extends ThrowingAstVisitor {
beforeBlock(elseSpreadBracket, spreadRule, null);
}

@override
void visitChild(CollectionElement element, CollectionElement child) {
builder.nestExpression(indent: 2, now: true);

Expand All @@ -1894,6 +1901,7 @@ class SourceVisitor extends ThrowingAstVisitor {
// condition or the then clause, we want the then and else clauses to split.
builder.startRule();

var hasInnerControlFlow = false;
for (var element in ifElements) {
// The condition.
token(element.ifKeyword);
Expand All @@ -1903,6 +1911,9 @@ class SourceVisitor extends ThrowingAstVisitor {
token(element.rightParenthesis);

visitChild(element, element.thenElement);
if (_isControlFlowElement(element.thenElement)) {
hasInnerControlFlow = true;
}

// Handle this element's "else" keyword and prepare to write the element,
// but don't write it. It will either be the next element in [ifElements]
Expand All @@ -1924,8 +1935,17 @@ class SourceVisitor extends ThrowingAstVisitor {

// Handle the final trailing else if there is one.
var lastElse = ifElements.last.elseElement;
if (lastElse != null) visitChild(lastElse, lastElse);
if (lastElse != null) {
visitChild(lastElse, lastElse);

if (_isControlFlowElement(lastElse)) {
hasInnerControlFlow = true;
}
}

// If a control flow element is nested inside another, force the outer one
// to split.
if (hasInnerControlFlow) builder.forceRules();
builder.endRule();
}

Expand Down
9 changes: 9 additions & 0 deletions test/regression/0800/0869.stmt
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
>>>
var failed = [
for (var e in _restResultEvents) if (!e.isSuccess) e.message,
];
<<<
var failed = [
for (var e in _restResultEvents)
if (!e.isSuccess) e.message,
];
46 changes: 41 additions & 5 deletions test/splitting/list_collection_for.stmt
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,27 @@ var list = [
body;
}
];
>>> nested for doesn't split if it fits
>>> split if child is for
var l = [for (a in b) for (c in d) t];
<<<
var l = [for (a in b) for (c in d) t];
>>> split collection before nested for
var list = [for (a in b) for (c in d) longThing];
var l = [
for (a in b)
for (c in d) t
];
>>> split if child is if
var list = [
for (var a in b) if (c) d
];
<<<
var list = [
for (a in b) for (c in d) longThing
for (var a in b)
if (c) d
];
>>> split collection before body
var list = [for (a in b) longThingThatIsLong];
<<<
var list = [
for (a in b) longThingThatIsLong
];
>>> just split outer for
var list = [for (a in b) for (c in d) longThingThatIsLong];
Expand All @@ -89,6 +101,30 @@ var list = [
for (a in b)
for (c in d) longThingThatIsLong
];
>>> nested list inside for element
var list = [for (var a in b) [c]];
<<<
var list = [
for (var a in b) [c]
];
>>> nested spread list inside for element
var list = [for (var a in b) ...[c]];
<<<
var list = [
for (var a in b) ...[c]
];
>>> nested if inside list
var l = [for (var a in b) [if (c) d]];
<<<
var l = [
for (var a in b) [if (c) d]
];
>>> nested for inside list
var l = [for (a in b) [for (c in d) e]];
<<<
var l = [
for (a in b) [for (c in d) e]
];
>>> split in for-in type
var list = [for (LongGenericTypeName<TypeArg, AnotherTypeArgument> a in b) body];
<<<
Expand Down
48 changes: 43 additions & 5 deletions test/splitting/list_collection_if.stmt
Original file line number Diff line number Diff line change
Expand Up @@ -190,15 +190,27 @@ var list = [
body;
}
];
>>> nested if doesn't split if it fits
>>> split if child is if
var list = [if (c) if (d) thing];
<<<
var list = [if (c) if (d) thing];
>>> split collection before nested if
var list = [if (c) if (d) fairlyLongThingHere];
var list = [
if (c)
if (d) thing
];
>>> split if child is for
var list = [
if (a) for (var b in c) thing
];
<<<
var list = [
if (c) if (d) fairlyLongThingHere
if (a)
for (var b in c) thing
];
>>> split collection before body
var list = [if (c) longThingHereThatIsLong];
<<<
var list = [
if (c) longThingHereThatIsLong
];
>>> just split outer if
var list = [if (condition) if (another) longThingHereThatIsLong];
Expand All @@ -207,6 +219,32 @@ var list = [
if (condition)
if (another) longThingHereThatIsLong
];
>>> nested list inside if element
var list = [if (a) [b]];
<<<
var list = [
if (a) [b]
];
>>> nested spread list inside if element
var list = [if (a) ...[b]];
<<<
var list = [
if (a) ...[b]
];
>>> nested if inside list
var list = [if (a) [if (b) c]];
<<<
var list = [
if (a) [if (b) c]
];
>>> nested for inside list
var l = [
if (a) [for (var b in c) d]
];
<<<
var l = [
if (a) [for (var b in c) d]
];
>>> split inside condition
var list = [if (veryLongCondition + thatNeedsToSplit) thing];
<<<
Expand Down
13 changes: 8 additions & 5 deletions test/splitting/map_collection_for.stmt
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,18 @@ var map = {
body;
}
};
>>> nested for doesn't split if it fits
>>> nested for splits outer
({for (a in b) for (c in d) 1: 1});
<<<
({for (a in b) for (c in d) 1: 1});
>>> split collection before nested for
var map = {for (a in b) for (c in d) 1: longThing};
({
for (a in b)
for (c in d) 1: 1
});
>>> split collection before body
var map = {for (a in b) longThingThatIsLong};
<<<
var map = {
for (a in b) for (c in d) 1: longThing
for (a in b) longThingThatIsLong
};
>>> just split outer for
var map = {for (a in b) for (c in d) 1: longThingThatIsLong};
Expand Down
13 changes: 8 additions & 5 deletions test/splitting/map_collection_if.stmt
Original file line number Diff line number Diff line change
Expand Up @@ -168,15 +168,18 @@ var map = {
body;
}
};
>>> nested if doesn't split if it fits
>>> nested if splits outer
var map = {if (c) if (d) thing: 1};
<<<
var map = {if (c) if (d) thing: 1};
>>> split collection before nested if
var map = {if (c) if (d) fairlyLongThingHere: 1};
var map = {
if (c)
if (d) thing: 1
};
>>> split collection before body
var map = {if (c) fairlyLongThingHere: 1};
<<<
var map = {
if (c) if (d) fairlyLongThingHere: 1
if (c) fairlyLongThingHere: 1
};
>>> just split outer if
var map = {if (condition) if (another) longThingThatIsLong: 1};
Expand Down
13 changes: 8 additions & 5 deletions test/splitting/set_collection_for.stmt
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,18 @@ var set = {
body;
}
};
>>> nested for doesn't split if it fits
>>> nested for splits outer for
var s = {for (a in b) for (c in d) t};
<<<
var s = {for (a in b) for (c in d) t};
>>> split collection before nested for
var set = {for (a in b) for (c in d) longThing};
var s = {
for (a in b)
for (c in d) t
};
>>> split collection before body
var set = {for (a in b) longThingThatIsLong};
<<<
var set = {
for (a in b) for (c in d) longThing
for (a in b) longThingThatIsLong
};
>>> just split outer for
var set = {for (a in b) for (c in d) longThingThatIsLong};
Expand Down
13 changes: 8 additions & 5 deletions test/splitting/set_collection_if.stmt
Original file line number Diff line number Diff line change
Expand Up @@ -168,15 +168,18 @@ var set = {
body;
}
};
>>> nested if doesn't split if it fits
>>> nested if splits outer
var set = {if (c) if (d) thing};
<<<
var set = {if (c) if (d) thing};
>>> split collection before nested if
var set = {if (c) if (d) fairlyLongThingHere};
var set = {
if (c)
if (d) thing
};
>>> split collection before body
var set = {if (c) longThingHereThatIsLong};
<<<
var set = {
if (c) if (d) fairlyLongThingHere
if (c) longThingHereThatIsLong
};
>>> just split outer if
var set = {if (condition) if (another) longThingHereThatIsLong};
Expand Down

0 comments on commit 79784cc

Please sign in to comment.