Fix a crash and some formatting bugs in cascades. (#1410)

I did a test run of the new formatter on the Flutter repo and it hit
some crashes in cascades where a cascade section contains a method
chain like `foo..a.b`.

While investigating that, I realized that the existing code for
formatting cascades tries to reuse the regular method chain formatting
code entirely unnecessarily.

This fixes the crash, and the formatting, and is simpler for handling
cascades.

Author: munificent

lib/src/front_end/chain_builder.dart

@@ -72,8 +72,19 @@ class ChainBuilder {
     if (_root case CascadeExpression cascade) {
       _visitTarget(cascade.target);
 
+      // When [_root] is a cascade, the chain is the series of cascade sections.
       for (var section in cascade.cascadeSections) {
-        _unwrapCall(section);
+        var piece = _visitor.nodePiece(section);
+
+        var callType = switch (section) {
+          MethodInvocation(argumentList: var args)
+              when args.arguments.canSplit(args.rightParenthesis) =>
+            CallType.splittableCall,
+          MethodInvocation() => CallType.unsplittableCall,
+          _ => CallType.property,
+        };
+
+        _calls.add(ChainCall(piece, callType));
       }
     } else {
       _unwrapCall(_root);
@@ -160,9 +171,6 @@ class ChainBuilder {
   /// Given [expression], which is the expression for some call chain, traverses
   /// the selectors to fill in the list of [_calls].
   ///
-  /// If [_root] is a [CascadeSection], then this is called once for each
-  /// section in the cascade.
-  ///
   /// Otherwise, it's a method chain, and this recursively calls itself for the
   /// targets to unzip and flatten the nested selector expressions. Then it
   /// initializes [_target] with the innermost subexpression that isn't a part
@@ -176,10 +184,8 @@ class ChainBuilder {
   ///     .baz[0][1]
   ///     .bang()
   void _unwrapCall(Expression expression) {
-    var isCascade = _root is CascadeExpression;
-
     switch (expression) {
-      case Expression(looksLikeStaticCall: true) when !isCascade:
+      case Expression(looksLikeStaticCall: true):
         // Don't include things that look like static method or constructor
         // calls in the call chain because that tends to split up named
         // constructors from their class.
@@ -191,8 +197,8 @@ class ChainBuilder {
             expression.operator, expression.rightHandSide);
         _calls.add(ChainCall(piece, CallType.property));
 
-      case MethodInvocation(:var target) when isCascade || target != null:
-        if (target != null) _unwrapCall(target);
+      case MethodInvocation(:var target?):
+        _unwrapCall(target);
 
         var callPiece = _visitor.buildPiece((b) {
           b.token(expression.operator);
@@ -206,8 +212,8 @@ class ChainBuilder {
         _calls.add(ChainCall(callPiece,
             canSplit ? CallType.splittableCall : CallType.unsplittableCall));
 
-      case PropertyAccess(:var target):
-        if (target != null) _unwrapCall(target);
+      case PropertyAccess(:var target?):
+        _unwrapCall(target);
 
         var piece = _visitor.buildPiece((b) {
           b.token(expression.operator);
@@ -217,7 +223,7 @@ class ChainBuilder {
         _calls.add(ChainCall(piece, CallType.property));
 
       case PrefixedIdentifier(:var prefix):
-        if (!isCascade) _unwrapCall(prefix);
+        _unwrapCall(prefix);
 
         var piece = _visitor.buildPiece((b) {
           b.token(expression.period);
@@ -236,19 +242,6 @@ class ChainBuilder {
           });
         });
 
-      case IndexExpression() when isCascade && _calls.isEmpty:
-        // An index expression as the first cascade section should be part of
-        // the cascade chain and not part of the target, as in:
-        //
-        //     foo
-        //       ..[index]
-        //       ..another();
-        //
-        // For non-cascade method chains, we keep leave the index as part of
-        // the target since the method chain doesn't begin until the first `.`.
-        var piece = _visitor.createIndexExpression(null, expression);
-        _calls.add(ChainCall(piece, CallType.property));
-
       case IndexExpression():
         _unwrapPostfix(expression.target!, (target) {
           return _visitor.createIndexExpression(target, expression);
@@ -264,7 +257,7 @@ class ChainBuilder {
 
       default:
         // Otherwise, it isn't a selector so we've reached the target.
-        if (!isCascade) _visitTarget(expression);
+        _visitTarget(expression);
     }
   }
 

test/invocation/cascade.stmt

@@ -224,4 +224,26 @@ object?..[index]..method()..[index]=value;
 object
   ?..[index]
   ..method()
-  ..[index] = value;
+  ..[index] = value;
+>>> Property chain in cascade.
+object..first.second.third
+    ..fourth.fifth.sixth;
+<<<
+object
+  ..first.second.third
+  ..fourth.fifth.sixth;
+>>> Split property chain in cascade.
+object..firstProperty.secondProperty.thirdProperty
+    ..fourthProperty.fifthProperty.sixthProperty;
+<<<
+object
+  ..firstProperty
+    .secondProperty
+    .thirdProperty
+  ..fourthProperty
+    .fifthProperty
+    .sixthProperty;
+>>> Cascade chained property access setter.
+object..a.b = value;
+<<<
+object..a.b = value;

test/regression_tall/other/flutter.unit

@@ -0,0 +1,14 @@
+>>> Unitialized late variable on cascade containing method chains.
+main() {
+  htmlElement
+    ..style.width = '100%'
+    ..style.height = '100%'
+    ..classList.add(_kClassName);
+}
+<<<
+main() {
+  htmlElement
+    ..style.width = '100%'
+    ..style.height = '100%'
+    ..classList.add(_kClassName);
+}