[parser] Recover written out binary operators

* Add mechanisms to try out a recovery and only perform it if is
  successful.
* Recover written out binary operators, e.g. "a xor b", "a or b" etc.
  This fixes #26810 and is also how these operators are written in e.g.
  Kotlin. This might be somewhat controversial though.
* Adds a mechanism to _replace_ a token by another token.
  This might be controversial.
  It is done because just inserting the operator causes the same rewrite
  to be attempted on the same token stream several times (at least with
  the way the token stream is parsed via the CFE) in turn causing it to
  be recovered *sometimes*. This is now avoided by actually replacing
  the token.

Change-Id: Icfa806045575d2aa2e5f35126708651b275bcf84
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/162003
Commit-Queue: Jens Johansen <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
Reviewed-by: Johnni Winther <[email protected]>

Author: jensjoha

pkg/_fe_analyzer_shared/lib/src/messages/codes_generated.dart

@@ -292,6 +292,36 @@ const MessageCode messageAwaitNotAsync = const MessageCode("AwaitNotAsync",
     analyzerCodes: <String>["AWAIT_IN_WRONG_CONTEXT"],
     message: r"""'await' can only be used in 'async' or 'async*' methods.""");
 
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const Template<
+    Message Function(
+        String string,
+        String
+            string2)> templateBinaryOperatorWrittenOut = const Template<
+        Message Function(String string, String string2)>(
+    messageTemplate:
+        r"""Binary operator '#string' is written as '#string2' instead of the written out word.""",
+    tipTemplate: r"""Try replacing '#string' with '#string2'.""",
+    withArguments: _withArgumentsBinaryOperatorWrittenOut);
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const Code<Message Function(String string, String string2)>
+    codeBinaryOperatorWrittenOut =
+    const Code<Message Function(String string, String string2)>(
+        "BinaryOperatorWrittenOut", templateBinaryOperatorWrittenOut,
+        index: 112);
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+Message _withArgumentsBinaryOperatorWrittenOut(String string, String string2) {
+  if (string.isEmpty) throw 'No string provided';
+  if (string2.isEmpty) throw 'No string provided';
+  return new Message(codeBinaryOperatorWrittenOut,
+      message:
+          """Binary operator '${string}' is written as '${string2}' instead of the written out word.""",
+      tip: """Try replacing '${string}' with '${string2}'.""",
+      arguments: {'string': string, 'string2': string2});
+}
+
 // DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
 const Template<
     Message Function(

pkg/_fe_analyzer_shared/lib/src/parser/parser_impl.dart

@@ -4615,13 +4615,22 @@ class Parser {
       token = typeArg.parseArguments(bangToken, this);
       assert(optional('(', token.next));
     }
+
+    return _parsePrecedenceExpressionLoop(
+        precedence, allowCascades, typeArg, token);
+  }
+
+  Token _parsePrecedenceExpressionLoop(int precedence, bool allowCascades,
+      TypeParamOrArgInfo typeArg, Token token) {
     Token next = token.next;
     TokenType type = next.type;
     int tokenLevel = _computePrecedence(next);
+    bool enteredLoop = false;
     for (int level = tokenLevel; level >= precedence; --level) {
       int lastBinaryExpressionLevel = -1;
       Token lastCascade;
       while (identical(tokenLevel, level)) {
+        enteredLoop = true;
         Token operator = next;
         if (identical(tokenLevel, CASCADE_PRECEDENCE)) {
           if (!allowCascades) {
@@ -4725,10 +4734,103 @@ class Parser {
         type = next.type;
         tokenLevel = _computePrecedence(next);
       }
+      if (_recoverAtPrecedenceLevel && !_currentlyRecovering) {
+        // Attempt recovery
+        if (_attemptPrecedenceLevelRecovery(
+            token, precedence, level, allowCascades, typeArg)) {
+          // Recovered - try again at same level with the replacement token.
+          level++;
+          next = token.next;
+          type = next.type;
+          tokenLevel = _computePrecedence(next);
+        }
+      }
+    }
+
+    if (!enteredLoop && _recoverAtPrecedenceLevel && !_currentlyRecovering) {
+      // Attempt recovery
+      if (_attemptPrecedenceLevelRecovery(
+          token, precedence, /*currentLevel = */ -1, allowCascades, typeArg)) {
+        return _parsePrecedenceExpressionLoop(
+            precedence, allowCascades, typeArg, token);
+      }
     }
     return token;
   }
 
+  /// Attempt a recovery where [token.next] is replaced.
+  bool _attemptPrecedenceLevelRecovery(Token token, int precedence,
+      int currentLevel, bool allowCascades, TypeParamOrArgInfo typeArg) {
+    // Attempt recovery.
+    assert(_token_recovery_replacements.containsKey(token.next.lexeme));
+    TokenType replacement = _token_recovery_replacements[token.next.lexeme];
+    if (currentLevel >= 0) {
+      // Check that the new precedence and currentLevel would have accepted this
+      // replacement here.
+      int newLevel = replacement.precedence;
+      // The loop it would normally have gone through is something like
+      // for (; ; --level) {
+      //   while (identical(tokenLevel, level)) {
+      //   }
+      // }
+      // So if the new tokens level <= the "old" (current) level, [level] (in
+      // the above code snippet) would get down to it and accept it.
+      // But if the new tokens level > the "old" (current) level, normally we
+      // would never get to it - so we shouldn't here either. As the loop starts
+      // by taking the first tokens tokenLevel as level, recursing below won't
+      // weed that out so we need to do it here.
+      if (newLevel > currentLevel) return false;
+    }
+
+    _currentlyRecovering = true;
+    _recoverAtPrecedenceLevel = false;
+    Listener originalListener = listener;
+    TokenStreamRewriter originalRewriter = cachedRewriter;
+    NullListener nullListener = listener = new NullListener();
+    UndoableTokenStreamRewriter undoableTokenStreamRewriter =
+        new UndoableTokenStreamRewriter();
+    cachedRewriter = undoableTokenStreamRewriter;
+    rewriter.replaceNextTokenWithSyntheticToken(token, replacement);
+    bool acceptRecovery = false;
+    Token afterExpression = _parsePrecedenceExpressionLoop(
+        precedence, allowCascades, typeArg, token);
+
+    if (!nullListener.hasErrors &&
+        isOneOfOrEof(afterExpression.next, const [';', ',', ')', '{', '}'])) {
+      // Seems good!
+      acceptRecovery = true;
+    }
+
+    // Undo all changes and reset.
+    _currentlyRecovering = false;
+    undoableTokenStreamRewriter.undo();
+    listener = originalListener;
+    cachedRewriter = originalRewriter;
+
+    if (acceptRecovery) {
+      // Report and redo recovery.
+      reportRecoverableError(
+          token.next,
+          codes.templateBinaryOperatorWrittenOut
+              .withArguments(token.next.lexeme, replacement.lexeme));
+      rewriter.replaceNextTokenWithSyntheticToken(token, replacement);
+      return true;
+    }
+    return false;
+  }
+
+  bool _recoverAtPrecedenceLevel = false;
+  bool _currentlyRecovering = false;
+  static const Map<String, TokenType> _token_recovery_replacements = const {
+    // E.g. in Kotlin these are written out, see.
+    // https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-int/.
+    "xor": TokenType.CARET,
+    "and": TokenType.AMPERSAND,
+    "or": TokenType.BAR,
+    "shl": TokenType.LT_LT,
+    "shr": TokenType.GT_GT,
+  };
+
   int _computePrecedence(Token token) {
     TokenType type = token.type;
     if (identical(type, TokenType.BANG)) {
@@ -4751,7 +4853,15 @@ class Parser {
       if (!isConditional) {
         return SELECTOR_PRECEDENCE;
       }
+    } else if (identical(type, TokenType.IDENTIFIER)) {
+      // An identifier at this point is not right. So some recovery is going to
+      // happen soon. The question is, if we can do a better recovery here.
+      if (!_currentlyRecovering &&
+          _token_recovery_replacements.containsKey(token.lexeme)) {
+        _recoverAtPrecedenceLevel = true;
+      }
     }
+
     return type.precedence;
   }
 

pkg/_fe_analyzer_shared/lib/src/parser/token_stream_rewriter.dart

@@ -9,6 +9,7 @@ import '../scanner/token.dart'
         BeginToken,
         CommentToken,
         Keyword,
+        ReplacementToken,
         SimpleToken,
         SyntheticBeginToken,
         SyntheticKeywordToken,
@@ -113,6 +114,27 @@ abstract class TokenStreamRewriter with _TokenStreamMixin {
     return current;
   }
 
+  /// Insert a new simple synthetic token of [newTokenType] after
+  /// [previousToken] instead of the token actually coming after it and return
+  /// the new token.
+  /// The old token will be linked from the new one though, so it's not totally
+  /// gone.
+  ReplacementToken replaceNextTokenWithSyntheticToken(
+      Token previousToken, TokenType newTokenType) {
+    assert(newTokenType is! Keyword,
+        'use an unwritten variation of insertSyntheticKeyword instead');
+
+    // [token] <--> [a] <--> [b]
+    ReplacementToken replacement =
+        new ReplacementToken(newTokenType, previousToken.next);
+    insertToken(previousToken, replacement);
+    // [token] <--> [replacement] <--> [a] <--> [b]
+    _setNext(replacement, replacement.next.next);
+    // [token] <--> [replacement] <--> [b]
+
+    return replacement;
+  }
+
   Token _setNext(Token setOn, Token nextToken);
   void _setEndGroup(BeginToken setOn, Token endGroup);
   void _setOffset(Token setOn, int offset);
@@ -248,6 +270,12 @@ class TokenStreamGhostWriter
   void _setPrevious(Token setOn, Token previous) {
     throw new UnimplementedError("_setPrevious");
   }
+
+  @override
+  ReplacementToken replaceNextTokenWithSyntheticToken(
+      Token previousToken, TokenType newTokenType) {
+    throw new UnimplementedError("replaceWithSyntheticToken");
+  }
 }
 
 mixin _TokenStreamMixin {

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

@@ -804,6 +804,32 @@ class SyntheticToken extends SimpleToken {
   Token copy() => new SyntheticToken(type, offset);
 }
 
+/// A token used to replace another token in the stream, while still keeping the
+/// old token around (in [replacedToken]). Automatically sets the offset and
+/// precedingComments from the data available on [replacedToken].
+class ReplacementToken extends SyntheticToken {
+  /// The token that this token replaces. This will normally be the token
+  /// representing what the user actually wrote.
+  final Token replacedToken;
+
+  ReplacementToken(TokenType type, this.replacedToken)
+      : super(type, replacedToken.offset) {
+    precedingComments = replacedToken.precedingComments;
+  }
+
+  @override
+  Token beforeSynthetic;
+
+  @override
+  bool get isSynthetic => true;
+
+  @override
+  int get length => 0;
+
+  @override
+  Token copy() => new ReplacementToken(type, replacedToken);
+}
+
 /**
  * A token that was scanned from the input. Each token knows which tokens
  * precede and follow it, acting as a link in a doubly linked list of tokens.

pkg/analyzer/lib/error/error.dart

@@ -597,6 +597,7 @@ const List<ErrorCode> errorCodeValues = [
   ParserErrorCode.ANNOTATION_ON_TYPE_ARGUMENT,
   ParserErrorCode.ANNOTATION_WITH_TYPE_ARGUMENTS,
   ParserErrorCode.ASYNC_KEYWORD_USED_AS_IDENTIFIER,
+  ParserErrorCode.BINARY_OPERATOR_WRITTEN_OUT,
   ParserErrorCode.BREAK_OUTSIDE_OF_LOOP,
   ParserErrorCode.CATCH_SYNTAX,
   ParserErrorCode.CATCH_SYNTAX_EXTRA_PARAMETERS,

pkg/analyzer/lib/src/dart/error/syntactic_errors.dart

@@ -72,6 +72,9 @@ class ParserErrorCode extends ErrorCode {
           "The keywords 'await' and 'yield' can't be used as "
               "identifiers in an asynchronous or generator function.");
 
+  static const ParserErrorCode BINARY_OPERATOR_WRITTEN_OUT =
+      _BINARY_OPERATOR_WRITTEN_OUT;
+
   static const ParserErrorCode BREAK_OUTSIDE_OF_LOOP = _BREAK_OUTSIDE_OF_LOOP;
 
   static const ParserErrorCode CATCH_SYNTAX = _CATCH_SYNTAX;

pkg/analyzer/lib/src/dart/error/syntactic_errors.g.dart

@@ -119,6 +119,7 @@ final fastaAnalyzerErrorCodes = <ErrorCode>[
   _EXTERNAL_LATE_FIELD,
   _ABSTRACT_EXTERNAL_FIELD,
   _ANNOTATION_ON_TYPE_ARGUMENT,
+  _BINARY_OPERATOR_WRITTEN_OUT,
 ];
 
 const ParserErrorCode _ABSTRACT_CLASS_MEMBER = ParserErrorCode(
@@ -148,6 +149,11 @@ const ParserErrorCode _ANNOTATION_WITH_TYPE_ARGUMENTS = ParserErrorCode(
     'ANNOTATION_WITH_TYPE_ARGUMENTS',
     r"An annotation (metadata) can't use type arguments.");
 
+const ParserErrorCode _BINARY_OPERATOR_WRITTEN_OUT = ParserErrorCode(
+    'BINARY_OPERATOR_WRITTEN_OUT',
+    r"Binary operator '#string' is written as '#string2' instead of the written out word.",
+    correction: "Try replacing '#string' with '#string2'.");
+
 const ParserErrorCode _BREAK_OUTSIDE_OF_LOOP = ParserErrorCode(
     'BREAK_OUTSIDE_OF_LOOP',
     r"A break statement can't be used outside of a loop or switch statement.",

pkg/analyzer/test/generated/parser_fasta_test.dart

@@ -1114,6 +1114,17 @@ class CollectionLiteralParserTest extends FastaParserTestCase {
 @reflectiveTest
 class ComplexParserTest_Fasta extends FastaParserTestCase
     with ComplexParserTestMixin {
+  void test_binary_operator_written_out_expression() {
+    BinaryExpression expression = parseExpression('x xor y', errors: [
+      expectedError(ParserErrorCode.BINARY_OPERATOR_WRITTEN_OUT, 2, 3),
+    ]);
+    SimpleIdentifier lhs = expression.leftOperand;
+    expect(lhs.name, 'x');
+    expect(expression.operator.lexeme, '^');
+    SimpleIdentifier rhs = expression.rightOperand;
+    expect(rhs.name, 'y');
+  }
+
   void test_conditionalExpression_precedence_nullableType_as2() {
     ExpressionStatement statement = parseStatement('x as bool? ? (x + y) : z;');
     ConditionalExpression expression = statement.expression;