Merge pull request #904 from dart-lang/split-nested-control-flow
For split control flow elements that contain other ones.
diff --git a/CHANGELOG.md b/CHANGELOG.md
index ebb360b..9265c5e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,7 +3,8 @@
* Add `--fix-single-cascade-statements`.
* 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).
# 1.3.3
diff --git a/lib/src/source_visitor.dart b/lib/src/source_visitor.dart
index ffd7a0f..caeffe9 100644
--- a/lib/src/source_visitor.dart
+++ b/lib/src/source_visitor.dart
@@ -117,6 +117,9 @@
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;
@@ -1561,6 +1564,11 @@
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();
}
@@ -1867,7 +1875,6 @@
beforeBlock(elseSpreadBracket, spreadRule, null);
}
- @override
void visitChild(CollectionElement element, CollectionElement child) {
builder.nestExpression(indent: 2, now: true);
@@ -1894,6 +1901,7 @@
// 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);
@@ -1903,6 +1911,9 @@
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]
@@ -1924,8 +1935,17 @@
// 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();
}
diff --git a/test/regression/0800/0869.stmt b/test/regression/0800/0869.stmt
new file mode 100644
index 0000000..bd474dc
--- /dev/null
+++ b/test/regression/0800/0869.stmt
@@ -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,
+];
\ No newline at end of file
diff --git a/test/splitting/list_collection_for.stmt b/test/splitting/list_collection_for.stmt
index b569861..6d84800 100644
--- a/test/splitting/list_collection_for.stmt
+++ b/test/splitting/list_collection_for.stmt
@@ -72,15 +72,27 @@
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];
@@ -89,6 +101,30 @@
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];
<<<
diff --git a/test/splitting/list_collection_if.stmt b/test/splitting/list_collection_if.stmt
index 00d9cb3..467d2f8 100644
--- a/test/splitting/list_collection_if.stmt
+++ b/test/splitting/list_collection_if.stmt
@@ -190,15 +190,27 @@
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) fairlyLongThingHere
+ if (c)
+ if (d) thing
+];
+>>> split if child is for
+var list = [
+ if (a) for (var b in c) thing
+];
+<<<
+var list = [
+ 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];
@@ -207,6 +219,32 @@
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];
<<<
diff --git a/test/splitting/map_collection_for.stmt b/test/splitting/map_collection_for.stmt
index eed802e..ed3448f 100644
--- a/test/splitting/map_collection_for.stmt
+++ b/test/splitting/map_collection_for.stmt
@@ -72,15 +72,18 @@
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};
diff --git a/test/splitting/map_collection_if.stmt b/test/splitting/map_collection_if.stmt
index ef4758a..3b01be5 100644
--- a/test/splitting/map_collection_if.stmt
+++ b/test/splitting/map_collection_if.stmt
@@ -168,15 +168,18 @@
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) fairlyLongThingHere: 1
+ if (c)
+ if (d) thing: 1
+};
+>>> split collection before body
+var map = {if (c) fairlyLongThingHere: 1};
+<<<
+var map = {
+ if (c) fairlyLongThingHere: 1
};
>>> just split outer if
var map = {if (condition) if (another) longThingThatIsLong: 1};
diff --git a/test/splitting/set_collection_for.stmt b/test/splitting/set_collection_for.stmt
index 4d2a399..007b4fc 100644
--- a/test/splitting/set_collection_for.stmt
+++ b/test/splitting/set_collection_for.stmt
@@ -72,15 +72,18 @@
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};
diff --git a/test/splitting/set_collection_if.stmt b/test/splitting/set_collection_if.stmt
index 86375ad..75f722c 100644
--- a/test/splitting/set_collection_if.stmt
+++ b/test/splitting/set_collection_if.stmt
@@ -168,15 +168,18 @@
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) fairlyLongThingHere
+ if (c)
+ if (d) thing
+};
+>>> split collection before body
+var set = {if (c) longThingHereThatIsLong};
+<<<
+var set = {
+ if (c) longThingHereThatIsLong
};
>>> just split outer if
var set = {if (condition) if (another) longThingHereThatIsLong};
