improve fasta closing brace recovery

pkg/front_end/lib/src/fasta/scanner/array_based_scanner.dart

Index: pkg/front_end/lib/src/fasta/scanner/array_based_scanner.dart
diff --git a/pkg/front_end/lib/src/fasta/scanner/array_based_scanner.dart b/pkg/front_end/lib/src/fasta/scanner/array_based_scanner.dart
index 43159a580a7d1846560494432beb8c246720ea9f..3d8cb58070e77ceca48f14b6a1ef58e283acb93f 100644
--- a/pkg/front_end/lib/src/fasta/scanner/array_based_scanner.dart
+++ b/pkg/front_end/lib/src/fasta/scanner/array_based_scanner.dart
@@ -139,16 +139,17 @@ abstract class ArrayBasedScanner extends AbstractScanner {
   /**
    * Appends a token that begins an end group, represented by [type].
    * It handles the group end tokens '}', ')' and ']'. The tokens '>' and
-   * '>>' are handled separately bo [appendGt] and [appendGtGt].
+   * '>>' are handled separately by [appendGt] and [appendGtGt].
    */
   int appendEndGroup(TokenType type, int openKind) {
     assert(!identical(openKind, LT_TOKEN)); // openKind is < for > and >>
-    discardBeginGroupUntil(openKind);
-    appendPrecedenceToken(type);
-    Token close = tail;
-    if (groupingStack.isEmpty) {
+    if (!discardBeginGroupUntil(openKind)) {
+      // No begin group found. Just continue.
+      appendPrecedenceToken(type);

[Paul Berry, 2017/07/20 16:01:02]

Not a blocking issue for this CL, but would it perhaps be better for error
recovery to just skip the unmatched end token?  That way the token stream passed
to the parser would always have matched openers/closers regardless of user
input.

[danrubel, 2017/07/21 14:23:50]

I have this (maybe unfounded) fear that analysis, and more specifically code
completion, might do really strange things if the token stream does not contain
all of the original text.

For example, given the following where "^" is the cursor location...

String foo = 'hello';
main() { foo))))).^ }

... then I fear that code completion would offer code completions at the cursor
location when I'm not sure that it should.

That said, dropping the unmatched end tokens would make parsing easier and is
definitely something to discuss.

[Paul Berry, 2017/07/21 17:13:45]

That's a good point.  I hadn't thought of that.
I'm currently leaning toward the idea we discussed this morning (insert a
synthetic opener so that balanced openers/closers are always delivered to the
parser).  But of course that's an issue for another CL, and I'm open to changing
my opinion based on what you find out :)

       return advance();
     }
+    appendPrecedenceToken(type);
+    Token close = tail;
     BeginToken begin = groupingStack.head;
     if (!identical(begin.kind, openKind)) {
       assert(begin.kind == STRING_INTERPOLATION_TOKEN &&
@@ -166,21 +167,52 @@ abstract class ArrayBasedScanner extends AbstractScanner {
   }
 
   /**
-   * Discards begin group tokens until a match with [openKind] is found.
-   * This recovers nicely from from a situation like "{[}".
+   * If a begin group token matches [openKind],
+   * then discard begin group tokens up to that match and return `true`,
+   * otherwise return `false`.
+   * This recovers nicely from from situations like "{[}" and "{foo());}",
+   * but not "foo(() {bar());});
    */
-  void discardBeginGroupUntil(int openKind) {
-    while (!groupingStack.isEmpty) {
+  bool discardBeginGroupUntil(int openKind) {
+    Link<BeginToken> originalStack = groupingStack;
+
+    bool first = true;
+    do {
       // Don't report unmatched errors for <; it is also the less-than operator.
       discardOpenLt();
-      if (groupingStack.isEmpty) return;
+      if (groupingStack.isEmpty) break; // recover
       BeginToken begin = groupingStack.head;
-      if (openKind == begin.kind) return;
-      if (openKind == OPEN_CURLY_BRACKET_TOKEN &&
-          begin.kind == STRING_INTERPOLATION_TOKEN) return;
-      unmatchedBeginGroup(begin);
+      if (openKind == begin.kind ||
+          (openKind == OPEN_CURLY_BRACKET_TOKEN &&
+              begin.kind == STRING_INTERPOLATION_TOKEN)) {
+        if (first) {
+          // If the expected opener has been found on the first pass
+          // then no recovery necessary.
+          return true;
+        }
+        break; // recover
+      }
+      first = false;
       groupingStack = groupingStack.tail;
+    } while (!groupingStack.isEmpty);
+
+    // If the stack does not have any opener of the given type,
+    // then return without discarding anything.
+    // This recovers nicely from from situations like "{foo());}".
+    if (groupingStack.isEmpty) {
+      groupingStack = originalStack;
+      return false;
+    }
+
+    // Insert synthetic closers and report errors for any unbalanced openers.
+    // This recovers nicely from from situations like "{[}".
+    while (originalStack != groupingStack) {

[Paul Berry, 2017/07/20 16:01:02]

Link.operator== does a deep equality comparison, which is unnecessary here. 
Use:

while (!identical(originalStack, groupingStack) {

[danrubel, 2017/07/21 14:23:51]

Good catch! Fixed.

+      // Don't report unmatched errors for <; it is also the less-than operator.
+      if (!identical(groupingStack.head.kind, LT_TOKEN))
+        unmatchedBeginGroup(originalStack.head);
+      originalStack = originalStack.tail;
     }
+    return true;
   }
 
   /**