Format conditional expressions. (#1292)

Format conditional expressions.

Author: munificent

lib/src/front_end/ast_node_visitor.dart

@@ -8,6 +8,7 @@ import 'package:analyzer/source/line_info.dart';
 import '../dart_formatter.dart';
 import '../piece/do_while.dart';
 import '../piece/if.dart';
+import '../piece/infix.dart';
 import '../piece/piece.dart';
 import '../piece/variable.dart';
 import '../source_code.dart';
@@ -210,7 +211,32 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>
 
   @override
   void visitConditionalExpression(ConditionalExpression node) {
-    throw UnimplementedError();
+    visit(node.condition);
+    var condition = writer.pop();
+    writer.split();
+
+    token(node.question);
+    writer.space();
+    visit(node.thenExpression);
+    var thenBranch = writer.pop();
+    writer.split();
+
+    token(node.colon);
+    writer.space();
+    visit(node.elseExpression);
+    var elseBranch = writer.pop();
+
+    var piece = InfixPiece([condition, thenBranch, elseBranch]);
+
+    // If conditional expressions are directly nested, force them all to split,
+    // both parents and children.
+    if (node.parent is ConditionalExpression ||
+        node.thenExpression is ConditionalExpression ||
+        node.elseExpression is ConditionalExpression) {
+      piece.pin(State.split);
+    }
+
+    writer.push(piece);
   }
 
   @override

lib/src/piece/assign.dart

@@ -34,7 +34,7 @@ import 'piece.dart';
 ///
 /// ```
 /// var name =
-///   longValueExpression;
+///     longValueExpression;
 /// ```
 class AssignPiece extends Piece {
   /// Split inside the value but not at the `=`.
@@ -59,6 +59,30 @@ class AssignPiece extends Piece {
   AssignPiece(this.target, this.value, {required bool isValueDelimited})
       : _isValueDelimited = isValueDelimited;
 
+  // TODO(tall): The old formatter allows the first operand of a split
+  // conditional expression to be on the same line as the `=`, as in:
+  //
+  // ```
+  // var value = condition
+  //     ? thenValue
+  //     : elseValue;
+  // ```
+  //
+  // It's not clear if this behavior is deliberate or not. It does look OK,
+  // though. We could do the same thing here. If we do, I think it's worth
+  // considering allowing the same thing for infix expressions too:
+  //
+  // ```
+  // var value = operand +
+  //    operand +
+  //    operand;
+  // ```
+  //
+  // For now, we not implement this special case behavior. Once more of the
+  // language is implemented in the new back end and we can run the formatter
+  // on a large corpus of code, we can try it out and see if the special case
+  // behavior is worth it.
+
   @override
   List<State> get additionalStates =>
       [if (_isValueDelimited) _insideValue, _atEquals];

test/expression/condition.stmt

@@ -0,0 +1,51 @@
+40 columns                              |
+>>> Unsplit.
+condition  ?  thenBranch  :  elseBranch  ;
+<<<
+condition ? thenBranch : elseBranch;
+>>> Split because condition splits.
+veryLongElement != otherReallyLongElement ? longArgument : arg;
+<<<
+veryLongElement !=
+        otherReallyLongElement
+    ? longArgument
+    : arg;
+>>> Split because of split in then branch.
+condition ? veryLongExpression +
+    otherLongExpression : elseExpr;
+<<<
+condition
+    ? veryLongExpression +
+        otherLongExpression
+    : elseExpr;
+>>> Split because of split in else branch.
+condition ? thenExpression
+    : veryLongExpression +
+        otherLongExpression;
+<<<
+condition
+    ? thenExpression
+    : veryLongExpression +
+        otherLongExpression;
+>>> Force split all conditionals when nested.
+var kind = a ? b ? c : d : e;
+<<<
+var kind =
+    a
+        ? b
+            ? c
+            : d
+        : e;
+>>>
+var kind = a ? b : c ? d : e;
+<<<
+var kind =
+    a
+        ? b
+        : c
+            ? d
+            : e;
+>>> Don't force split conditionals when indirectly nested.
+var kind = a ? b : (c ? d : e);
+<<<
+var kind = a ? b : (c ? d : e);

test/expression/condition_comment.stmt

@@ -0,0 +1,62 @@
+40 columns                              |
+>>> Inline block comment before `?`.
+cond /* c */ ? 1 : 2;
+<<<
+cond /* c */ ? 1 : 2;
+>>> Inline block comment after `?`.
+cond ? /* c */ 1 : 2;
+<<<
+cond ? /* c */ 1 : 2;
+>>> Inline block comment before `:`.
+cond ? 1 /* c */ : 2;
+<<<
+cond ? 1 /* c */ : 2;
+>>> Inline block comment after `:`.
+cond ? 1 : /* c */ 2;
+<<<
+cond ? 1 : /* c */ 2;
+>>> Inline block comment after else.
+cond ? 1 : 2 /* c */;
+<<<
+cond ? 1 : 2 /* c */;
+>>> Line comment before `?`.
+cond // c
+? 1 : 2;
+<<<
+cond // c
+    ? 1
+    : 2;
+>>> Line comment after `?`.
+cond ? // c
+1 : 2;
+<<<
+### Looks weird, but users generally won't put comments here.
+cond
+    ? // c
+    1
+    : 2;
+>>> Line comment before `:`.
+cond ? 1 // c
+: 2;
+<<<
+cond
+    ? 1 // c
+    : 2;
+>>> Line comment after `:`.
+cond ? 1 : // c
+2;
+<<<
+### Looks weird, but users generally won't put comments here.
+cond
+    ? 1
+    : // c
+    2;
+>>> Line comment after else.
+cond ? 1 : 2 // c
+;
+<<<
+### Looks weird, but users generally won't put comments here.
+cond
+    ? 1
+    : 2 // c
+    ;

test/statement/variable.stmt

@@ -193,4 +193,14 @@ var variableName = ([
 var variableName = (notDelimited + expression);
 <<<
 var variableName =
-    (notDelimited + expression);
+    (notDelimited + expression);
+>>> Split all variables if an initializer has a split internally.
+var a = 1, b = [element, element, element, element];
+<<<
+var a = 1,
+    b = [
+      element,
+      element,
+      element,
+      element,
+    ];