[scanner] Specialized scanner recovery for missing end curly brace

*TL;DR*

This improves scanner recovery for a missing `}` in certain situations,
reducing the risk of an in-body change causing a (temporary) outline
change (which in turn could result in the analyzer becoming unresponsive
for "no reason").

*Details*

The behavior of IntelliJ is that when typing `{` it only inserts a
matching end brace `}` when hitting enter.

Imagine you are typing an if: `if (1 + 1 == 2) {`, where you don't hit
enter quickly enough and you trigger a re-analysis at this point.

What happens then is that every method below where you are typing looks
to be local function declarations and thus the outline change. When the
outline change the analyzer has to do a lot of work: everything
(transitively) depending on the file has to be recompiled, and every
strongly connected component is compiled "in one go" where the analyzer
can't respond to queries. So if you have one or more large strongly
connected components depending on the file, or the file itself is part
of such a chain, you will (or at least might) experience that the
analyzer is slow to respond, and it will be extra puzzling because
logically you're just doing an in-body change.

For some code the user might not even naturally hit enter, e.g. `var foo
= {"I'm", "a", "set"};`.

The recovery in the scanner has always been that - upon reaching the end
of the file - it sees that we're missing a `}` and it inserts it at the
end. This CL instead tries to figure out a better place to insert it,
and if successful, will rerun the scanner, instructing it to insert it
at the better place and (hopefully) avoiding a subsequent outline
change.

It does this by looking at the indentation - which is new for recovery -
and under the assumption that the indentation was correct before, will
find the position where the start curly brace was inserted. Note that if
it finds a position it will always be between the start curly brace (the
one missing the end curly brace) and the end of file, and inserting the
missing curly end brace there can't really be "more wrong" than
inserting it at the end (if the new place is not correct it's just
"still wrong").

In the benchmark added we see how quickly we can get completion after
having typed `if (1+1==2) {`, then adding `\n ge\n}` and requesting
completion on the `ge` part, i.e. a simulation of typing

```
if (1+1==2) {
  ge
}
```

and asking for completion at the `ge`.

The change in this CL - on cycles of size 1024 - caused the time to
completion response to come in between ~5 times faster (going from ~10.2
to ~2.1 seconds) to ~18 times faster (going from ~10.3 seconds to ~0.56
seconds):

`CodeType.ImportExportCycle` goes from:

```
+------+-----------+------------+
| Size |  Initial  | Completion |
+------+-----------+------------+
|   16 |  2.019581 |    0.97504 |
|   32 |  3.028976 |   1.031008 |
|   64 |  4.422884 |   1.198383 |
|  128 |  7.612125 |   1.597091 |
|  256 | 12.860864 |   2.906553 |
|  512 | 24.391894 |   5.017093 |
| 1024 | 48.390993 |  10.243085 |
+------+-----------+------------+
```

to

```
+------+-----------+------------+
| Size |  Initial  | Completion |
+------+-----------+------------+
|   16 |  2.107213 |   0.661066 |
|   32 |  3.012952 |    0.70554 |
|   64 |  4.682508 |   0.731176 |
|  128 |  7.508434 |   0.745501 |
|  256 | 13.105477 |   0.852413 |
|  512 | 24.520184 |   1.278403 |
| 1024 | 48.804348 |    2.11903 |
+------+-----------+------------+
```

and `CodeType.ImportExportChain` goes from:

```
+------+-----------+------------+
| Size |  Initial  | Completion |
+------+-----------+------------+
|   16 |  2.059196 |   0.892082 |
|   32 |  3.080717 |    0.93232 |
|   64 |  4.647163 |   1.240303 |
|  128 |  7.377035 |   1.674859 |
|  256 | 12.939432 |   2.705483 |
|  512 | 24.529501 |    5.02689 |
| 1024 | 47.713553 |  10.385469 |
+------+-----------+------------+
```

to

```
+------+-----------+------------+
| Size |  Initial  | Completion |
+------+-----------+------------+
|   16 |  2.020809 |   0.709643 |
|   32 |  3.106856 |   0.648818 |
|   64 |  4.503067 |   0.593152 |
|  128 |   7.45692 |   0.622423 |
|  256 | 13.140592 |   0.606948 |
|  512 | 24.933216 |   0.612687 |
| 1024 | 50.167541 |   0.567544 |
+------+-----------+------------+
```

Change-Id: I8dbefe215162d00a209206ae3db83b2b17505853
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/415581
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Phil Quitslund <pquitslund@google.com>

Author: jensjoha

pkg/_fe_analyzer_shared/lib/src/scanner/abstract_scanner.dart

@@ -114,6 +114,10 @@ abstract class AbstractScanner implements Scanner {
   @override
   bool hasErrors = false;
 
+  Token? openBraceWithMissingEndForPossibleRecovery;
+
+  int? offsetForCurlyBracketRecoveryStart;
+
   /**
    * A pointer to the stream of comment tokens created by this scanner
    * before they are assigned to the [Token] precedingComments field
@@ -352,9 +356,136 @@ abstract class AbstractScanner implements Scanner {
     appendToken(new KeywordToken(keyword, tokenStart, comments));
   }
 
+  /// Find the indentation of the logical line of [token].
+  ///
+  /// By logical take this as an example:
+  ///
+  /// ```
+  ///   if (a &&
+  ///       b) {
+  ///   }
+  /// ```
+  ///
+  /// the indentation of `{` should logically be the same as the indentation of
+  /// `a` because it's the indentation of the `if`, even though the _line_ of a
+  /// has a different indentation of the _line_ of `{`.
+  int _spacesAtStartOfLogicalLineOf(Token token) {
+    if (lineStarts.isEmpty) return -1;
+
+    // If the previous token is a `)`, e.g. if this is the start curly brace in
+    // an if, we find the first token before the corresponding `(` (in this case
+    // the if) in an attempt to find "the right" token to get the indentation
+    // for - e.g. if the if is spread over several lines the given token itself
+    // will - if formatted by the formatter - be indented more than the "if".
+    if (token.isA(TokenType.OPEN_CURLY_BRACKET)) {
+      if (token.previous == null) return -1;
+      Token previous = token.previous!;
+      bool foundWanted = false;
+      if (previous.isA(TokenType.CLOSE_PAREN)) {
+        Token closeParen = token.previous!;
+        Token? candidate = closeParen.previous;
+        while (candidate != null && candidate.endGroup != closeParen) {
+          candidate = candidate.previous;
+        }
+        if (candidate?.endGroup == closeParen && candidate!.previous != null) {
+          token = candidate.previous!;
+          if (token.isA(Keyword.IF) ||
+              token.isA(Keyword.FOR) ||
+              token.isA(Keyword.WHILE) ||
+              token.isA(Keyword.SWITCH) ||
+              token.isA(Keyword.CATCH)) {
+            foundWanted = true;
+          }
+        }
+      } else if (previous.isA(Keyword.ELSE) ||
+          previous.isA(Keyword.TRY) ||
+          previous.isA(Keyword.FINALLY)) {
+        foundWanted = true;
+      } else if (previous.isA(TokenType.EQ) &&
+          (previous.previous?.isA(TokenType.IDENTIFIER) ?? false)) {
+        // `someIdentifier = {`
+        foundWanted = true;
+      } else if (previous.isA(Keyword.CONST) &&
+          (previous.previous?.isA(TokenType.EQ) ?? false) &&
+          (previous.previous?.previous?.isA(TokenType.IDENTIFIER) ?? false)) {
+        // `someIdentifier = const {`
+        foundWanted = true;
+      }
+      if (!foundWanted) return -1;
+    }
+
+    // Now find the line of [token].
+    final int offset = token.offset;
+    int low = 0, high = lineStarts.length - 1;
+    while (low < high) {
+      int mid = high - ((high - low) >> 1); // Get middle, rounding up.
+      int pivot = lineStarts[mid];
+      if (pivot <= offset) {
+        low = mid;
+      } else {
+        high = mid - 1;
+      }
+    }
+    final int lineIndex = low;
+    if (lineIndex == 0) {
+      // On first line.
+      return tokens.next?.charOffset ?? -1;
+    }
+
+    // Find the first token of the line.
+    int lineStartOfToken = lineStarts[lineIndex];
+    Token? candidate = token.previous;
+    while (candidate != null && candidate.offset >= lineStartOfToken) {
+      candidate = candidate.previous;
+    }
+    if (candidate != null) {
+      // candidate.next is the first token of the line.
+      return candidate.next!.offset - lineStartOfToken;
+    }
+
+    return -1;
+  }
+
+  /// If there was a single missing `}` when tokenizing, try to find the actual
+  /// `{` that is missing the `}`.
+  ///
+  /// This is done by checking all `{` tokens between the one (currently)
+  /// missing a `}` and the end of the token stream. For each we try to identify
+  /// the indentation of the `{` and the indentation of the endGroup (i.e. the
+  /// `}` it has been matched with). If the indentations mismatch we assume this
+  /// is a bad match. There might be more bad matches because of how the curly
+  /// braces are nested and we pick the last one, which should be the inner-most
+  /// one and thus the one introducing the error: That is going to be the one
+  /// that has "misaligned" the rest of the end-braces.
+  int? getOffsetForCurlyBracketRecoveryStart() {
+    if (openBraceWithMissingEndForPossibleRecovery == null) return null;
+    Token? next = openBraceWithMissingEndForPossibleRecovery!.next;
+    Token? lastMismatch;
+    while (next != null && !next.isEof) {
+      if (next.isA(TokenType.OPEN_CURLY_BRACKET)) {
+        int indentOfNext = _spacesAtStartOfLogicalLineOf(next);
+        if (indentOfNext >= 0 &&
+            indentOfNext != _spacesAtStartOfLogicalLineOf(next.endGroup!)) {
+          lastMismatch = next;
+        }
+      }
+      next = next.next;
+    }
+    if (lastMismatch != null) {
+      return lastMismatch.offset;
+    }
+    return null;
+  }
+
   void appendEofToken() {
     beginToken();
     discardOpenLt();
+    if (!groupingStack.isEmpty &&
+        groupingStack.head.isA(TokenType.OPEN_CURLY_BRACKET) &&
+        groupingStack.tail!.isEmpty) {
+      // We have a single `{` that's missing a `}`. Maybe the user is typing?
+      openBraceWithMissingEndForPossibleRecovery = groupingStack.head;
+    }
     while (!groupingStack.isEmpty) {
       unmatchedBeginGroup(groupingStack.head);
       groupingStack = groupingStack.tail!;
@@ -870,6 +1001,15 @@ abstract class AbstractScanner implements Scanner {
     }
 
     if (next == $CLOSE_CURLY_BRACKET) {
+      if (offsetForCurlyBracketRecoveryStart != null &&
+          !groupingStack.isEmpty &&
+          groupingStack.head.isA(TokenType.OPEN_CURLY_BRACKET) &&
+          groupingStack.head.offset == offsetForCurlyBracketRecoveryStart) {
+        // This instance of the scanner was instructed to recover this
+        // opening curly bracket.
+        unmatchedBeginGroup(groupingStack.head);
+        groupingStack = groupingStack.tail!;
+      }
       return appendEndGroup(
           TokenType.CLOSE_CURLY_BRACKET, OPEN_CURLY_BRACKET_TOKEN);
     }

pkg/_fe_analyzer_shared/lib/src/scanner/recover.dart

@@ -4,61 +4,6 @@
 
 library _fe_analyzer_shared.scanner.recover;
 
-import 'token.dart' show Token, TokenType;
-
-import 'token_impl.dart' show StringTokenImpl;
-
-import 'error_token.dart' show ErrorToken;
-
-/// Recover from errors in [tokens]. The original sources are provided as
-/// [bytes]. [lineStarts] are the beginning character offsets of lines, and
-/// must be updated if recovery is performed rewriting the original source
-/// code.
-Token scannerRecovery(List<int> bytes, Token tokens, List<int> lineStarts) {
-  // Sanity check that all error tokens are prepended.
-
-  // TODO(danrubel): Remove this in a while after the dust has settled.
-
-  // Skip over prepended error tokens
-  Token token = tokens;
-  while (token is ErrorToken) {
-    token = token.next!;
-  }
-
-  // Assert no error tokens in the remaining tokens
-  while (!token.isEof) {
-    if (token is ErrorToken) {
-      for (int count = 0; count < 3; ++count) {
-        Token previous = token.previous!;
-        if (previous.isEof) break;
-        token = previous;
-      }
-      StringBuffer msg = new StringBuffer(
-          "Internal error: All error tokens should have been prepended:");
-      for (int count = 0; count < 7; ++count) {
-        if (token.isEof) break;
-        msg.write(' ${token.runtimeType},');
-        token = token.next!;
-      }
-      throw msg.toString();
-    }
-    token = token.next!;
-  }
-
-  return tokens;
-}
-
-Token synthesizeToken(int charOffset, String value, TokenType type) {
-  return new StringTokenImpl.fromString(type, value, charOffset);
-}
-
-Token skipToEof(Token token) {
-  while (!token.isEof) {
-    token = token.next!;
-  }
-  return token;
-}
-
 String closeBraceFor(String openBrace) {
   return const {
     '(': ')',

pkg/_fe_analyzer_shared/lib/src/scanner/scanner.dart

@@ -4,12 +4,11 @@
 
 library _fe_analyzer_shared.scanner;
 
-import 'dart:convert' show Utf8Encoder, unicodeReplacementCharacterRune;
+import 'dart:convert' show unicodeReplacementCharacterRune;
 import 'dart:typed_data' show Uint8List;
 
 import 'abstract_scanner.dart'
     show LanguageVersionChanged, ScannerConfiguration;
-import 'recover.dart' show scannerRecovery;
 import 'string_scanner.dart' show StringScanner;
 import 'token.dart' show Token;
 import 'utf8_bytes_scanner.dart' show Utf8BytesScanner;
@@ -33,8 +32,6 @@ export 'utf8_bytes_scanner.dart' show Utf8BytesScanner;
 
 const int unicodeReplacementCharacter = unicodeReplacementCharacterRune;
 
-typedef Token Recover(List<int> bytes, Token tokens, List<int> lineStarts);
-
 abstract class Scanner {
   /// Returns true if an error occurred during [tokenize].
   bool get hasErrors;
@@ -61,12 +58,30 @@ ScannerResult scan(Uint8List bytes,
     bool includeComments = false,
     LanguageVersionChanged? languageVersionChanged,
     bool allowLazyStrings = true}) {
-  Scanner scanner = new Utf8BytesScanner(bytes,
+  Utf8BytesScanner scanner = new Utf8BytesScanner(bytes,
       configuration: configuration,
       includeComments: includeComments,
       languageVersionChanged: languageVersionChanged,
       allowLazyStrings: allowLazyStrings);
-  return _tokenizeAndRecover(scanner, bytes: bytes);
+  Token tokens = scanner.tokenize();
+  if (scanner.hasErrors) {
+    // If there was a single missing `}` and the scanner can identify a good
+    // candidate for better recovery, create a new scanner and instruct it to
+    // do that recovery.
+    int? offsetForCurlyBracketRecoveryStart =
+        scanner.getOffsetForCurlyBracketRecoveryStart();
+    if (offsetForCurlyBracketRecoveryStart != null) {
+      scanner = new Utf8BytesScanner(bytes,
+          configuration: configuration,
+          includeComments: includeComments,
+          languageVersionChanged: languageVersionChanged,
+          allowLazyStrings: allowLazyStrings);
+      scanner.offsetForCurlyBracketRecoveryStart =
+          offsetForCurlyBracketRecoveryStart;
+      tokens = scanner.tokenize();
+    }
+  }
+  return new ScannerResult(tokens, scanner.lineStarts, scanner.hasErrors);
 }
 
 /// Scan/tokenize the given [source].
@@ -78,15 +93,22 @@ ScannerResult scanString(String source,
       configuration: configuration,
       includeComments: includeComments,
       languageVersionChanged: languageVersionChanged);
-  return _tokenizeAndRecover(scanner, source: source);
-}
-
-ScannerResult _tokenizeAndRecover(Scanner scanner,
-    {List<int>? bytes, String? source}) {
   Token tokens = scanner.tokenize();
   if (scanner.hasErrors) {
-    if (bytes == null) bytes = const Utf8Encoder().convert(source!);
-    tokens = scannerRecovery(bytes, tokens, scanner.lineStarts);
+    // If there was a single missing `}` and the scanner can identify a good
+    // candidate for better recovery, create a new scanner and instruct it to
+    // do that recovery.
+    int? offsetForCurlyBracketRecoveryStart =
+        scanner.getOffsetForCurlyBracketRecoveryStart();
+    if (offsetForCurlyBracketRecoveryStart != null) {
+      scanner = new StringScanner(source,
+          configuration: configuration,
+          includeComments: includeComments,
+          languageVersionChanged: languageVersionChanged);
+      scanner.offsetForCurlyBracketRecoveryStart =
+          offsetForCurlyBracketRecoveryStart;
+      tokens = scanner.tokenize();
+    }
   }
   return new ScannerResult(tokens, scanner.lineStarts, scanner.hasErrors);
 }