Refactor parseType in preparation for bigger changes.

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 183 matching lines @@
 /// the prefix `skip`.
 ///
 /// Parse methods generate events (by calling methods on [listener]) and return
 /// the next token to parse. Peek methods do not generate events (except for
 /// errors) and may return null. Skip methods are like parse methods, but skip
 /// over some parts of the file being parsed.
 ///
 /// Parse methods are generally named `parseGrammarProductionSuffix`. The
 /// suffix can be one of `opt`, or `star`. `opt` means zero or one matches,
 /// `star` means zero or more matches. For example, [parseMetadataStar]
-/// corresponds to this grammar snippet: `metadata*`, and [parseTypeOpt]
-/// corresponds to: `type?`.
+/// corresponds to this grammar snippet: `metadata*`, and [parseArgumentsOpt]
+/// corresponds to: `arguments?`.
 ///
 /// ## Implementation Notes
 ///
 /// The parser assumes that keywords, built-in identifiers, and other special
 /// words (pseudo-keywords) are all canonicalized. To extend the parser to
 /// recognize a new identifier, one should modify
 /// [keyword.dart](../scanner/keyword.dart) and ensure the identifier is added
 /// to the keyword table.
 ///
 /// As a consequence of this, one should not use `==` to compare strings in the
@@ skipping 399 matching lines @@
     Token typedefKeyword = token;
     listener.beginFunctionTypeAlias(token);
     Token equals;
     if (optional('=', peekAfterNominalType(token.next))) {
       token = parseIdentifier(token.next, IdentifierContext.typedefDeclaration);
       token = parseTypeVariablesOpt(token);
       equals = token;
       token = expect('=', token);
       token = parseType(token);
     } else {
-      token = parseReturnTypeOpt(token.next);
+      token = parseType(token.next, isOptional: true);
       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);
     token = parseType(token);
     Token withKeyword = token;
     token = expect('with', token);
     token = parseTypeList(token);
     listener.endMixinApplication(withKeyword);
     return token;
   }
 
-  Token parseReturnTypeOpt(Token token) {
-    if (identical(token.stringValue, 'void')) {
-      if (isGeneralizedFunctionType(token.next)) {
-        return parseType(token);
-      } else {
-        listener.handleVoidKeyword(token);
-        return token.next;
-      }
-    } else {
-      return parseTypeOpt(token);
-    }
-  }
-
   Token parseFormalParametersOpt(Token token, MemberKind kind) {
     if (optional('(', token)) {
       return parseFormalParameters(token, kind);
     } else {
       listener.handleNoFormalParameters(token, kind);
       return token;
     }
   }
 
   Token skipFormalParameters(Token token, MemberKind kind) {
@@ skipping 159 matching lines @@
               : codeEmptyOptionalParameterList);
     }
     listener.endOptionalFormalParameters(parameterCount, begin, token);
     if (isNamed) {
       return expect('}', token);
     } else {
       return expect(']', token);
     }
   }
 
-  Token parseTypeOpt(Token token) {
-    if (isGeneralizedFunctionType(token)) {
-      // Function type without return type.
-      return parseType(token);
-    }
-    token = listener.injectGenericCommentTypeAssign(token);
-    Token peek = peekAfterIfType(token);
-    if (peek != null && (peek.isIdentifier || optional('this', peek))) {
-      return parseType(token);
-    }
-    listener.handleNoType(token);
-    return token;
-  }
-
   bool isValidTypeReference(Token token) {
     int kind = token.kind;
     if (IDENTIFIER_TOKEN == kind) return true;
     if (KEYWORD_TOKEN == kind) {
       String value = token.type.lexeme;
       return token.type.isPseudo ||
           (identical(value, 'dynamic')) ||
           (identical(value, 'void'));
     }
     return false;
@@ skipping 299 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) {
+  Token parseType(Token token, {bool isOptional: false}) {
+    if (isOptional) {

[danrubel, 2017/06/19 15:31:06]

for better performance, should this block be placed into a parseTypeSomething
which then calls parseType?

[ahe, 2017/06/19 16:46:49]

Great idea, but I have much bigger plans for this :-)

+      do {
+        if (optional("void", token)) {
+          if (isGeneralizedFunctionType(token.next)) {
+            // This is a type, parse it.
+            break;
+          } else {
+            listener.handleVoidKeyword(token);
+            return token.next;
+          }
+        } else {
+          if (isGeneralizedFunctionType(token)) {
+            // Function type without return type, parse it.
+            break;
+          }
+          token = listener.injectGenericCommentTypeAssign(token);
+          Token peek = peekAfterIfType(token);
+          if (peek != null && (peek.isIdentifier || optional('this', peek))) {
+            // This is a type followed by an identifier, parse it.
+            break;
+          }
+          listener.handleNoType(token);
+          return token;
+        }
+      } while (false);

[danrubel, 2017/06/19 15:31:06]

Why is all this in a do while(false)? It does not feel or look like a loop so
this structure is confusing. I recommend refactoring this as if/else-if/else
blocks.

[ahe, 2017/06/19 16:46:49]

It was for the breaks. I agree it isn't nice, but again, much bigger plans for
this :-)

+    }
     Token begin = token;
     if (isGeneralizedFunctionType(token)) {
       // A function type without return type.
       // Push the non-existing return type first. The loop below will
       // generate the full type.
       listener.handleNoType(token);
     } else if (optional("void", token) &&
         isGeneralizedFunctionType(token.next)) {
       listener.handleVoidKeyword(token);
       token = token.next;
@@ skipping 213 matching lines @@
     if (externalModifier != null) {
       parseModifier(externalModifier);
       listener.handleModifiers(1);
     } else {
       listener.handleModifiers(0);
     }
 
     if (type == null) {
       listener.handleNoType(name);
     } else {
-      parseReturnTypeOpt(type);
+      parseType(type, isOptional: true);
     }
     Token token =
         parseIdentifier(name, IdentifierContext.topLevelFunctionDeclaration);
 
     bool isGetter = false;
     if (getOrSet == null) {
       token = parseTypeVariablesOpt(token);
     } else {
       isGetter = optional("get", getOrSet);
       listener.handleNoTypeVariables(token);
@@ skipping 379 matching lines @@
           reportRecoverableErrorCodeWithToken(token, codeExtraneousModifier);
           token = token.next;
         }
       } else {
         break;
       }
     }
     listener.handleModifiers(count);
 
     Token beforeType = token;
-    if (returnTypeAllowed) {
-      token = parseReturnTypeOpt(token);
-    } else {
-      token = typeRequired ? parseType(token) : parseTypeOpt(token);
-    }
+    token = parseType(token, isOptional: returnTypeAllowed || !typeRequired);
     if (typeRequired && beforeType == token) {
       reportRecoverableErrorCode(token, codeTypeRequired);
     }
     if (hasVar && beforeType != token) {
       reportRecoverableErrorCode(beforeType, codeTypeAfterVar);
     }
     return token;
   }
 
   /// Returns the first token after the type starting at [token].
@@ skipping 284 matching lines @@
         count++;
       }
       listener.handleModifiers(count);
     }
 
     parseModifierList(modifiers);
 
     if (type == null) {
       listener.handleNoType(name);
     } else {
-      parseReturnTypeOpt(type);
+      parseType(type, isOptional: true);
     }
     Token token;
     if (optional('operator', name)) {
       token = parseOperatorName(name);
       if (staticModifier != null) {
         reportRecoverableErrorCodeWithToken(
             staticModifier, codeExtraneousModifier);
       }
     } else {
       token = parseIdentifier(name, IdentifierContext.methodDeclaration);
@@ skipping 113 matching lines @@
     listener.beginFunctionDeclaration(token);
     token = parseFunction(token);
     listener.endFunctionDeclaration(token);
     return token;
   }
 
   Token parseFunctionExpression(Token token) {
     Token beginToken = token;
     listener.beginFunction(token);
     listener.handleModifiers(0);
-    token = parseReturnTypeOpt(token);
+    token = parseType(token, isOptional: true);
     listener.beginFunctionName(token);
     token = parseIdentifier(token, IdentifierContext.functionExpressionName);
     listener.endFunctionName(beginToken, token);
     token = parseTypeVariablesOpt(token);
     token = parseFormalParameters(token, MemberKind.Local);
     listener.handleNoInitializers();
     AsyncModifier savedAsyncModifier = asyncState;
     token = parseAsyncModifier(token);
     bool isBlock = optional('{', token);
     token = parseFunctionBody(token, true, false);
@@ skipping 1773 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);