Avoid needless indentation on chained if-else elements. (#822)
* Avoid needless indentation on chained if-else elements.
Fix #813.
* Use collection for!
I also went ahead and pub upgraded, which brought in some lints, which
I fixed.
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 65250ae..e31a726 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,7 @@
* Better indentation of function expressions inside trailing comma argument
lists. (Thanks a14@!)
+* Avoid needless indentation on chained if-else elements (#813).
# 1.2.7
diff --git a/lib/src/call_chain_visitor.dart b/lib/src/call_chain_visitor.dart
index 936beee..7546e22 100644
--- a/lib/src/call_chain_visitor.dart
+++ b/lib/src/call_chain_visitor.dart
@@ -108,7 +108,9 @@
// .property
// .method()[1][2];
var call = expression;
- while (call is IndexExpression) call = (call as IndexExpression).target;
+ while (call is IndexExpression) {
+ call = (call as IndexExpression).target;
+ }
if (SourceVisitor.looksLikeStaticCall(call)) {
// Don't include things that look like static method or constructor
@@ -180,7 +182,9 @@
}
if (blockCalls != null) {
- for (var blockCall in blockCalls) calls.remove(blockCall);
+ for (var blockCall in blockCalls) {
+ calls.remove(blockCall);
+ }
}
if (hangingCall != null) {
diff --git a/lib/src/line_splitting/solve_state.dart b/lib/src/line_splitting/solve_state.dart
index 0ae7a70..734b1e0 100644
--- a/lib/src/line_splitting/solve_state.dart
+++ b/lib/src/line_splitting/solve_state.dart
@@ -420,7 +420,9 @@
});
// Add the costs for the spans containing splits.
- for (var span in splitSpans) cost += span.cost;
+ for (var span in splitSpans) {
+ cost += span.cost;
+ }
// Finish the last line.
endLine(_splitter.chunks.length);
diff --git a/lib/src/source_visitor.dart b/lib/src/source_visitor.dart
index 96f60bc..47230c6 100644
--- a/lib/src/source_visitor.dart
+++ b/lib/src/source_visitor.dart
@@ -1517,15 +1517,14 @@
}
void visitIfElement(IfElement node) {
- // Wrap the whole thing in a single rule. If a split happens inside the
- // condition or the then clause, we want the then and else clauses to split.
- builder.startRule();
-
- token(node.ifKeyword);
- space();
- token(node.leftParenthesis);
- visit(node.condition);
- token(node.rightParenthesis);
+ // Treat a chain of if-else elements as a single unit so that we don't
+ // unnecessarily indent each subsequent section of the chain.
+ var ifElements = [
+ for (CollectionElement thisNode = node;
+ thisNode is IfElement;
+ thisNode = (thisNode as IfElement).elseElement)
+ thisNode as IfElement
+ ];
// If the body of the then or else branch is a spread of a collection
// literal, then we want to format those collections more like blocks than
@@ -1551,66 +1550,81 @@
//
// To do that, if we see that either clause is a spread collection, we
// create a single rule and force both collections to use it.
- var thenSpreadBracket = _findSpreadCollectionBracket(node.thenElement);
- var elseSpreadBracket = _findSpreadCollectionBracket(node.elseElement);
-
- if (thenSpreadBracket != null || elseSpreadBracket != null) {
- var spreadRule = Rule();
- if (thenSpreadBracket != null) {
- beforeBlock(thenSpreadBracket, spreadRule, null);
- }
-
- if (elseSpreadBracket != null) {
- beforeBlock(elseSpreadBracket, spreadRule, null);
+ var spreadRule = Rule();
+ var spreadBrackets = <CollectionElement, Token>{};
+ for (var element in ifElements) {
+ var spreadBracket = _findSpreadCollectionBracket(element.thenElement);
+ if (spreadBracket != null) {
+ spreadBrackets[element] = spreadBracket;
+ beforeBlock(spreadBracket, spreadRule, null);
}
}
- builder.nestExpression(indent: 2, now: true);
-
- // Treat a spread of a collection literal like a block in an if statement
- // and don't split after the "else".
- if (thenSpreadBracket != null) {
- space();
- } else {
- split();
-
- // If the then clause is a non-spread collection or lambda, make sure the
- // body is indented.
- builder.startBlockArgumentNesting();
+ var elseSpreadBracket =
+ _findSpreadCollectionBracket(ifElements.last.elseElement);
+ if (elseSpreadBracket != null) {
+ spreadBrackets[ifElements.last.elseElement] = elseSpreadBracket;
+ beforeBlock(elseSpreadBracket, spreadRule, null);
}
- visit(node.thenElement);
-
- if (thenSpreadBracket == null) builder.endBlockArgumentNesting();
- builder.unnest();
-
- if (node.elseElement != null) {
- if (thenSpreadBracket != null) {
- space();
- } else {
- split();
- }
-
- token(node.elseKeyword);
-
+ visitChild(CollectionElement element, CollectionElement child) {
builder.nestExpression(indent: 2, now: true);
- if (elseSpreadBracket != null) {
+ // Treat a spread of a collection literal like a block in an if statement
+ // and don't split after the "else".
+ var isSpread = spreadBrackets.containsKey(element);
+ if (isSpread) {
space();
} else {
split();
- // If the else clause is a non-spread collection or lambda, make sure
- // the body is indented.
+ // If the then clause is a non-spread collection or lambda, make sure the
+ // body is indented.
builder.startBlockArgumentNesting();
}
- visit(node.elseElement);
+ visit(child);
- if (elseSpreadBracket == null) builder.endBlockArgumentNesting();
+ if (!isSpread) builder.endBlockArgumentNesting();
builder.unnest();
}
+ // Wrap the whole thing in a single rule. If a split happens inside the
+ // condition or the then clause, we want the then and else clauses to split.
+ builder.startRule();
+
+ for (var element in ifElements) {
+ // The condition.
+ token(element.ifKeyword);
+ space();
+ token(element.leftParenthesis);
+ visit(element.condition);
+ token(element.rightParenthesis);
+
+ visitChild(element, element.thenElement);
+
+ // 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]
+ // or the final else element handled after the loop.
+ if (element.elseElement != null) {
+ if (spreadBrackets.containsKey(element)) {
+ space();
+ } else {
+ split();
+ }
+
+ token(node.elseKeyword);
+
+ // If there is another if element in the chain, put a space between
+ // it and this "else".
+ if (element != ifElements.last) space();
+ }
+ }
+
+ // Handle the final trailing else if there is one.
+ var lastElse = ifElements.last.elseElement;
+ if (lastElse != null) visitChild(lastElse, lastElse);
+
builder.endRule();
}
diff --git a/pubspec.lock b/pubspec.lock
index 3b394d1..374e069 100644
--- a/pubspec.lock
+++ b/pubspec.lock
@@ -14,14 +14,14 @@
name: args
url: "https://pub.dartlang.org"
source: hosted
- version: "1.5.1"
+ version: "1.5.2"
async:
dependency: transitive
description:
name: async
url: "https://pub.dartlang.org"
source: hosted
- version: "2.1.0"
+ version: "2.2.0"
boolean_selector:
dependency: transitive
description:
@@ -105,14 +105,14 @@
name: http
url: "https://pub.dartlang.org"
source: hosted
- version: "0.12.0+1"
+ version: "0.12.0+2"
http_multi_server:
dependency: transitive
description:
name: http_multi_server
url: "https://pub.dartlang.org"
source: hosted
- version: "2.0.5"
+ version: "2.1.0"
http_parser:
dependency: transitive
description:
@@ -140,7 +140,7 @@
name: json_rpc_2
url: "https://pub.dartlang.org"
source: hosted
- version: "2.0.9"
+ version: "2.1.0"
kernel:
dependency: transitive
description:
@@ -168,7 +168,7 @@
name: mime
url: "https://pub.dartlang.org"
source: hosted
- version: "0.9.6+2"
+ version: "0.9.6+3"
multi_server_socket:
dependency: transitive
description:
@@ -210,7 +210,7 @@
name: pedantic
url: "https://pub.dartlang.org"
source: hosted
- version: "1.5.0"
+ version: "1.7.0"
pool:
dependency: transitive
description:
@@ -231,7 +231,7 @@
name: shelf
url: "https://pub.dartlang.org"
source: hosted
- version: "0.7.4"
+ version: "0.7.5"
shelf_packages_handler:
dependency: transitive
description:
@@ -252,7 +252,7 @@
name: shelf_web_socket
url: "https://pub.dartlang.org"
source: hosted
- version: "0.2.2+5"
+ version: "0.2.3"
source_map_stack_trace:
dependency: transitive
description:
@@ -287,7 +287,7 @@
name: stream_channel
url: "https://pub.dartlang.org"
source: hosted
- version: "1.7.0"
+ version: "2.0.0"
string_scanner:
dependency: transitive
description:
@@ -308,21 +308,21 @@
name: test
url: "https://pub.dartlang.org"
source: hosted
- version: "1.6.0"
+ version: "1.6.4"
test_api:
dependency: transitive
description:
name: test_api
url: "https://pub.dartlang.org"
source: hosted
- version: "0.2.3"
+ version: "0.2.6"
test_core:
dependency: transitive
description:
name: test_core
url: "https://pub.dartlang.org"
source: hosted
- version: "0.2.2"
+ version: "0.2.6"
test_descriptor:
dependency: "direct dev"
description:
@@ -350,7 +350,7 @@
name: vm_service_client
url: "https://pub.dartlang.org"
source: hosted
- version: "0.2.6"
+ version: "0.2.6+2"
watcher:
dependency: transitive
description:
@@ -364,7 +364,7 @@
name: web_socket_channel
url: "https://pub.dartlang.org"
source: hosted
- version: "1.0.12"
+ version: "1.0.13"
yaml:
dependency: "direct dev"
description:
@@ -373,4 +373,4 @@
source: hosted
version: "2.1.15"
sdks:
- dart: ">=2.1.1-dev.0.0 <3.0.0"
+ dart: ">=2.3.0 <3.0.0"
diff --git a/pubspec.yaml b/pubspec.yaml
index b1d58e5..1cfc78c 100644
--- a/pubspec.yaml
+++ b/pubspec.yaml
@@ -8,7 +8,7 @@
homepage: https://github.com/dart-lang/dart_style
environment:
- sdk: '>=2.1.0 <3.0.0'
+ sdk: '>=2.3.0 <3.0.0'
dependencies:
analyzer: '>=0.36.3 <0.37.0'
diff --git a/test/splitting/list_collection_if.stmt b/test/splitting/list_collection_if.stmt
index 7f272a7..36e44c2 100644
--- a/test/splitting/list_collection_if.stmt
+++ b/test/splitting/list_collection_if.stmt
@@ -214,4 +214,103 @@
if (veryLongCondition +
thatNeedsToSplit)
thing
+];
+>>> chained if-else
+var list = [if (condition1) thing1 else if (condition2) thing2];
+<<<
+var list = [
+ if (condition1)
+ thing1
+ else if (condition2)
+ thing2
+];
+>>> chained if-else with else at end
+var list = [if (condition1) thing1 else if (condition2) thing2 else thing3];
+<<<
+var list = [
+ if (condition1)
+ thing1
+ else if (condition2)
+ thing2
+ else
+ thing3
+];
+>>> long chained if-else
+var list = [if (condition1) thing1 else if (condition2) thing2 else if (condition3) thing3];
+<<<
+var list = [
+ if (condition1)
+ thing1
+ else if (condition2)
+ thing2
+ else if (condition3)
+ thing3
+];
+>>> long chained if-else with else at end
+var list = [if (condition1) thing1 else if (condition2) thing2 else if (condition3) thing3 else thing4];
+<<<
+var list = [
+ if (condition1)
+ thing1
+ else if (condition2)
+ thing2
+ else if (condition3)
+ thing3
+ else
+ thing4
+];
+>>> chained if-else with spread and unspread collections
+var list = [
+ if (condition1) ...[
+ spreadList
+ ] else if (condition2)
+ [notSpread]
+ else if (condition3)
+ thing1
+ else ...{
+ spreadSet
+ }
+];
+<<<
+var list = [
+ if (condition1) ...[
+ spreadList
+ ] else if (condition2)
+ [notSpread]
+ else if (condition3)
+ thing1
+ else ...{
+ spreadSet
+ }
+];
+>>> nested chained if-else
+var list = [
+ if (condition1)
+ if (condition2) a else b
+ else if (condition4)
+ if (condition5) c
+ else if (condition6) d else e
+ else if (condition7)
+ if (condition8) ...[f]
+ else if (condition9) [g] else ...{h}
+];
+<<<
+var list = [
+ if (condition1)
+ if (condition2) a else b
+ else if (condition4)
+ if (condition5)
+ c
+ else if (condition6)
+ d
+ else
+ e
+ else if (condition7)
+ if (condition8) ...[
+ f
+ ] else if (condition9)
+ [g]
+ else ...{
+ h
+ }
];
\ No newline at end of file
