Remove peeking from parseTypedef and parseClassOrNamedMixinApplication.

pkg/front_end/lib/src/fasta/parser/parser.dart

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 library fasta.parser.parser;
 
 import '../fasta_codes.dart'
     show
         FastaCode,
         FastaMessage,
@@ skipping 162 matching lines @@
 ///
 /// An assertion can legally occur as a statement. However, assertions are also
 /// experimentally allowed in initializers. For improved error recovery, we
 /// also attempt to parse asserts as expressions.
 enum Assert {
   Expression,
   Initializer,
   Statement,
 }
 
+enum TypeContinuation {

[Paul Berry, 2017/06/24 14:16:26]

It would be nice to have a comment explaining what this is for.  Here is the
best I've thought of (feel free to use this text or adapt it as you see fit). 
Nit: also consider renaming this to "TypeContext".

/// Indicates to [Parser.parseType] the context in which a type is to
/// be parsed; this determines the circumstances in which the attempt
/// to parse a type should be aborted (because the construct is
/// actually not a type).
enum TypeContinuation {
  /// The construct to be parsed is at the beginning of a typedef; if
  /// an identifier followed by an equals sign is seen, then type
  /// parsing should be aborted.
  Typedef,
}

[ahe, 2017/06/26 10:14:00]

I want to reserve that word for you :-)
Done.

+  Typedef,
+}
+
 /// An event generating parser of Dart programs. This parser expects all tokens
 /// in a linked list (aka a token stream).
 ///
 /// The class [Scanner] is used to generate a token stream. See the file
 /// [scanner.dart](../scanner.dart).
 ///
 /// Subclasses of the class [Listener] are used to listen to events.
 ///
 /// Most methods of this class belong in one of three major categories: parse
 /// methods, peek methods, and skip methods. Parse methods all have the prefix
@@ skipping 415 matching lines @@
 
   Token parseScript(Token token) {
     listener.handleScript(token);
     return token.next;
   }
 
   Token parseTypedef(Token token) {
     Token typedefKeyword = token;
     listener.beginFunctionTypeAlias(token);
     Token equals;
-    if (optional('=', peekAfterNominalType(token.next))) {
+    Token afterType = parseType(token.next,
+        isOptional: true, continuation: TypeContinuation.Typedef);
+    if (afterType == null) {
       token = parseIdentifier(token.next, IdentifierContext.typedefDeclaration);
       token = parseTypeVariablesOpt(token);
       equals = token;
       token = expect('=', token);
       token = parseType(token);
     } else {
-      token = parseType(token.next, isOptional: true);
+      token = afterType;
       token = parseIdentifier(token, IdentifierContext.typedefDeclaration);
       token = parseTypeVariablesOpt(token);
       token = parseFormalParameters(token, MemberKind.FunctionTypeAlias);
     }
     listener.endFunctionTypeAlias(typedefKeyword, equals, token);
     return expect(';', token);
   }
 
   Token parseMixinApplication(Token token) {
     listener.beginMixinApplication(token);
@@ skipping 325 matching lines @@
         count++;
       }
     }
     Token endBrace = token;
     token = expect('}', token);
     listener.endEnum(enumKeyword, endBrace, count);
     return token;
   }
 
   Token parseClassOrNamedMixinApplication(Token token) {
+    listener.beginClassOrNamedMixinApplication(token);
     Token begin = token;
-    Token abstractKeyword;
+    if (optional('abstract', token)) {
+      token = parseModifier(token);
+      listener.handleModifiers(1);
+    } else {
+      listener.handleModifiers(0);
+    }
     Token classKeyword = token;
-    if (optional('abstract', token)) {
-      abstractKeyword = token;
-      token = token.next;
-      classKeyword = token;
-    }
-    assert(optional('class', classKeyword));
-    int modifierCount = 0;
-    if (abstractKeyword != null) {
-      parseModifier(abstractKeyword);
-      modifierCount++;
-    }
-    listener.handleModifiers(modifierCount);
-    bool isMixinApplication = optional('=', peekAfterNominalType(token));
-    Token name = token.next;
-
-    if (isMixinApplication) {
-      token = parseIdentifier(name, IdentifierContext.namedMixinDeclaration);
+    token = expect("class", token);
+    Token name = token;
+    token =
+        parseIdentifier(name, IdentifierContext.classOrNamedMixinDeclaration);
+    token = parseTypeVariablesOpt(token);
+    if (optional('=', token)) {
       listener.beginNamedMixinApplication(begin, name);
-    } else {
-      token = parseIdentifier(name, IdentifierContext.classDeclaration);
-      listener.beginClassDeclaration(begin, name);
-    }
-
-    token = parseTypeVariablesOpt(token);
-
-    if (optional('=', token)) {
       Token equals = token;
       token = token.next;
       return parseNamedMixinApplication(
           token, begin, classKeyword, name, equals);
     } else {
+      listener.beginClassDeclaration(begin, name);
       return parseClass(token, begin, classKeyword, name);
     }
   }
 
   Token parseNamedMixinApplication(
       Token token, Token begin, Token classKeyword, Token name, Token equals) {
     token = parseMixinApplication(token);
     Token implementsKeyword = null;
     if (optional('implements', token)) {
       implementsKeyword = token;
@@ skipping 117 matching lines @@
   bool notEofOrValue(String value, Token token) {
     return !identical(token.kind, EOF_TOKEN) &&
         !identical(value, token.stringValue);
   }
 
   bool isGeneralizedFunctionType(Token token) {
     return optional('Function', token) &&
         (optional('<', token.next) || optional('(', token.next));
   }
 
-  Token parseType(Token token, {bool isOptional: false}) {
+  Token parseType(Token token,

[Paul Berry, 2017/06/24 14:16:26]

It would be nice to have a comment here explaining the function of
`continuation` as well as when `null` might be returned.  E.g.:

/// Parses a type (if it is appropriate to do so).
///
/// [continuation] indicates the context in which the type is being
/// parsed.  If the parser determines that the tokens do not actually
/// constitute a type in this context, then `null` is returned and the
/// caller should attempt to parse the tokens in some other way.

[ahe, 2017/06/26 10:14:01]

Done.

+      {bool isOptional: false, TypeContinuation continuation}) {
+    if (continuation != null) {

[ahe, 2017/06/24 10:10:31]

A little bit of context here:

My plan is to move all the peeking to this method (parseType). Once that is
complete, it should be easier to see how to rewrite it to achieve my longer term
goal: solve the type variable scope issue for generalized function types and
improve performance of parser by reducing look ahead.

So this is going to look more and more horrible, as I move stuff here. But it's
horrible stuff that's currently just sprinkled throughout the parser. So the
total measure of horribleness such remain the same :-)

+      switch (continuation) {

[Paul Berry, 2017/06/24 14:16:26]

Nit: It seems odd to have a null check followed by a switch statement on an
enum.  Personally, I think it would be more natural to add another possible
value to the enum which indicates the default behavior.  That way `continuation`
will never be null.

[ahe, 2017/06/26 10:14:00]

Done.

+        case TypeContinuation.Typedef:
+          if (optional('=', peekAfterNominalType(token))) {
+            return null; // This isn't a type, it's a new-style typedef.
+          }
+          break;
+
+        default:
+          throw "Internal error: Unhandled continuation '$continuation'.";
+      }
+    }
     if (isOptional) {
       do {
         if (optional("void", token)) {
           if (isGeneralizedFunctionType(token.next)) {
             // This is a type, parse it.
             break;
           } else {
             listener.handleVoidKeyword(token);
             return token.next;
           }
@@ skipping 2872 matching lines @@
     return reportUnrecoverableError(
         token, () => code.format(uri, token.charOffset, string));
   }
 }
 
 typedef FastaMessage NoArgument(Uri uri, int charOffset);
 
 typedef FastaMessage TokenArgument(Uri uri, int charOffset, Token token);
 
 typedef FastaMessage StringArgument(Uri uri, int charOffset, String string);