Add support for the new function-type syntax.

pkg/compiler/lib/src/parser/parser.dart

Index: pkg/compiler/lib/src/parser/parser.dart
diff --git a/pkg/compiler/lib/src/parser/parser.dart b/pkg/compiler/lib/src/parser/parser.dart
index 988cbf32c63d165be72ac4250dba5fc1b794c509..642f1fc4b653feec561b92ac2882c2930e90f209 100644
--- a/pkg/compiler/lib/src/parser/parser.dart
+++ b/pkg/compiler/lib/src/parser/parser.dart
@@ -362,12 +362,20 @@ class Parser {
 
   Token parseTypedef(Token token) {
     Token typedefKeyword = token;
-    listener.beginFunctionTypeAlias(token);
-    token = parseReturnTypeOpt(token.next);
-    token = parseIdentifier(token);
-    token = parseTypeVariablesOpt(token);
-    token = parseFormalParameters(token);
-    listener.endFunctionTypeAlias(typedefKeyword, token);
+    listener.beginTypedef(token);
+    Token equals;
+    if (optional('=', peekAfterType(token.next))) {
+      token = parseIdentifier(token.next);
+      token = parseTypeVariablesOpt(token);
+      equals = expect('=', token);
+      token = parseType(equals);
+    } else {
+      token = parseReturnTypeOpt(token.next);
+      token = parseIdentifier(token);
+      token = parseTypeVariablesOpt(token);
+      token = parseFormalParameters(token);
+    }
+    listener.endTypedef(typedefKeyword, equals, token);
     return expect(';', token);
   }
 
@@ -380,10 +388,13 @@ class Parser {
     return token;
   }
 
-  Token parseReturnTypeOpt(Token token) {
+  Token parseReturnTypeOpt(Token token, {bool allowFunctionType: true}) {
     if (identical(token.stringValue, 'void')) {
       listener.handleVoidKeyword(token);
       return token.next;
+    } else if (!allowFunctionType && isGeneralizedFunctionType(token)) {
+      listener.handleNoType(token);
+      return token;
     } else {
       return parseTypeOpt(token);
     }
@@ -398,7 +409,11 @@ class Parser {
     }
   }
 
-  Token parseFormalParameters(Token token) {
+  /// Parses the formal parameter list of a function.
+  ///
+  /// If [inFunctionType] is true, then the names may be omitted (except for
+  /// named arguments). If it is false, then the types may be omitted.
+  Token parseFormalParameters(Token token, {bool inFunctionType: false}) {
     Token begin = token;
     listener.beginFormalParameters(begin);
     expect('(', token);
@@ -411,40 +426,64 @@ class Parser {
       ++parameterCount;
       String value = token.stringValue;
       if (identical(value, '[')) {
-        token = parseOptionalFormalParameters(token, false);
+        token = parseOptionalFormalParameters(
+            token, false, inFunctionType: inFunctionType);
         break;
       } else if (identical(value, '{')) {
-        token = parseOptionalFormalParameters(token, true);
+        token = parseOptionalFormalParameters(
+            token, true, inFunctionType: inFunctionType);
         break;
       }
-      token = parseFormalParameter(token, FormalParameterType.REQUIRED);
+      token = parseFormalParameter(token, FormalParameterType.REQUIRED,
+          inFunctionType: inFunctionType);
     } while (optional(',', token));
     listener.endFormalParameters(parameterCount, begin, token);
     return expect(')', token);
   }
 
-  Token parseFormalParameter(Token token, FormalParameterType type) {
+  Token parseFormalParameter(Token token, FormalParameterType type,
+      {bool inFunctionType}) {
     token = parseMetadataStar(token, forParameter: true);
     listener.beginFormalParameter(token);
     token = parseModifiers(token);
-    // TODO(ahe): Validate that there are formal parameters if void.
-    token = parseReturnTypeOpt(token);
+    bool isNamedParameter = type == FormalParameterType.NAMED;
+
     Token thisKeyword = null;
-    if (optional('this', token)) {
-      thisKeyword = token;
-      // TODO(ahe): Validate field initializers are only used in
-      // constructors, and not for function-typed arguments.
-      token = expect('.', token.next);
+    if (inFunctionType && isNamedParameter) {
+      token = parseTypeOpt(token);
+      token = parseIdentifier(token);
+    } else if (inFunctionType) {
+      token = parseType(token);
+      if (token.isIdentifier()) {
+        token = parseIdentifier(token);
+      } else {
+        listener.handleNoName(token);
+      }
+    } else {
+      // TODO(ahe): Validate that there are formal parameters if void.
+      token = parseReturnTypeOpt(token);
+      if (optional('this', token)) {
+        thisKeyword = token;
+        // TODO(ahe): Validate field initializers are only used in
+        // constructors, and not for function-typed arguments.
+        token = expect('.', token.next);
+      }
+      token = parseIdentifier(token);
     }
-    token = parseIdentifier(token);
-    if (optional('(', token)) {
-      listener.handleNoTypeVariables(token);
-      token = parseFormalParameters(token);
-      listener.handleFunctionTypedFormalParameter(token);
-    } else if (optional('<', token)) {
-      token = parseTypeVariablesOpt(token);
-      token = parseFormalParameters(token);
-      listener.handleFunctionTypedFormalParameter(token);
+
+    // Generalized function types don't allow inline function types.
+    // The following isn't allowed:
+    //    int Function(int bar(String x)).

[Siggi Cherem (dart-lang), 2016/12/29 22:46:57]

should we report an error instead in that case?

(e.g. check that the next token is not '(' or '<' when inFunctionType is true)?

[floitsch, 2016/12/30 14:55:47]

In some sense that's what's happening. Just automatically by the parser.
Example output:

Error: 'void' is a reserved word and can't be used here.
Try using a different name.
typedef F = Function(void f());
                     ^^^^
Error: Expected ')' before this.
typedef F = Function(f());
                      ^

+    if (!inFunctionType) {
+      if (optional('(', token)) {
+        listener.handleNoTypeVariables(token);
+        token = parseFormalParameters(token);
+        listener.handleFunctionTypedFormalParameter(token);
+      } else if (optional('<', token)) {
+        token = parseTypeVariablesOpt(token);
+        token = parseFormalParameters(token);
+        listener.handleFunctionTypedFormalParameter(token);
+      }
     }
     String value = token.stringValue;
     if ((identical('=', value)) || (identical(':', value))) {
@@ -464,7 +503,8 @@ class Parser {
     return token;
   }
 
-  Token parseOptionalFormalParameters(Token token, bool isNamed) {
+  Token parseOptionalFormalParameters(Token token, bool isNamed,
+      {bool inFunctionType}) {
     Token begin = token;
     listener.beginOptionalFormalParameters(begin);
     assert((isNamed && optional('{', token)) || optional('[', token));
@@ -478,7 +518,8 @@ class Parser {
       }
       var type =
           isNamed ? FormalParameterType.NAMED : FormalParameterType.POSITIONAL;
-      token = parseFormalParameter(token, type);
+      token =
+          parseFormalParameter(token, type, inFunctionType: inFunctionType);
       ++parameterCount;
     } while (optional(',', token));
     if (parameterCount == 0) {
@@ -497,6 +538,10 @@ class Parser {
   }
 
   Token parseTypeOpt(Token token) {
+    if (isGeneralizedFunctionType(token)) {
+      // Function type without return type.
+      return parseType(token);
+    }
     Token peek = peekAfterIfType(token);
     if (peek != null && (peek.isIdentifier() || optional('this', peek))) {
       return parseType(token);
@@ -800,16 +845,58 @@ class Parser {
         !identical(value, token.stringValue);
   }
 
+  bool isGeneralizedFunctionType(Token token) {
+    // TODO(floitsch): don't use string comparison, but the keyword-state
+    // table is currently not set up to deal with upper-case characters.
+    return (optional('<', token.next) || optional('(', token.next)) &&
+        token.value == "Function";
+  }
+
   Token parseType(Token token) {
     Token begin = token;
-    if (isValidTypeReference(token)) {
-      token = parseIdentifier(token);
-      token = parseQualifiedRestOpt(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 {
-      token = listener.expectedType(token);
+      if (isValidTypeReference(token)) {
+        if (isGeneralizedFunctionType(token)) {

[Siggi Cherem (dart-lang), 2016/12/29 22:46:58]

is this branch ever executed?

[floitsch, 2016/12/30 14:55:47]

Nope. that was old code that I didn't clean up.
Good catch. thanks.
done.

+          // A function type without return type.
+          return parseFunctionType(token);
+        } else {
+          token = parseIdentifier(token);
+          token = parseQualifiedRestOpt(token);
+        }
+      } else {
+        token = listener.expectedType(token);
+      }
+      token = parseTypeArgumentsOpt(token);
+      listener.endType(begin, token);
+    }
+
+    // While we see a `Function(` treat the pushed type as return type.
+    // For example: `int Function() Function(int) Function(String x)`.
+    while (isGeneralizedFunctionType(token)) {
+      token = parseFunctionType(token);
     }
-    token = parseTypeArgumentsOpt(token);
-    listener.endType(begin, token);
+    return token;
+  }
+
+  /// Parses a generalized function type.
+  ///
+  /// The return type must already be pushed.
+  Token parseFunctionType(Token token) {
+    // TODO(floitsch): don't use string comparison, but the keyword-state
+    // table is currently not set up to deal with upper-case characters.
+    if (token.value != "Function") {
+      return listener.expected("Function", token);
+    }
+    Token functionToken = token;
+    token = token.next;
+    token = parseTypeVariablesOpt(token);
+    token = parseFormalParameters(token, inFunctionType: true);
+    listener.endFunctionType(functionToken, token);
     return token;
   }
 
@@ -1146,26 +1233,55 @@ class Parser {
         hasName = true;
       }
       identifiers = identifiers.prepend(token);
-      if (isValidTypeReference(token)) {
-        // type ...
-        if (optional('.', token.next)) {
-          // type '.' ...
-          if (token.next.next.isIdentifier()) {
-            // type '.' identifier
-            token = token.next.next;
+
+      if (!isGeneralizedFunctionType(token)) {
+        // Read a potential return type.
+        if (isValidTypeReference(token)) {
+          // type ...
+          if (optional('.', token.next)) {
+            // type '.' ...
+            if (token.next.next.isIdentifier()) {
+              // type '.' identifier
+              token = token.next.next;
+            }
+          }
+          if (optional('<', token.next)) {
+            if (token.next is BeginGroupToken) {
+              BeginGroupToken beginGroup = token.next;
+              if (beginGroup.endGroup == null) {
+                listener.unmatched(beginGroup);
+              }
+              token = beginGroup.endGroup;
+            }
           }
         }
-        if (optional('<', token.next)) {
-          if (token.next is BeginGroupToken) {
-            BeginGroupToken beginGroup = token.next;
+        token = token.next;
+      }
+      while (isGeneralizedFunctionType(token)) {
+        token = token.next;
+        if (optional('<', token)) {
+          if (token is BeginGroupToken) {
+            BeginGroupToken beginGroup = token;
             if (beginGroup.endGroup == null) {
               listener.unmatched(beginGroup);
             }
-            token = beginGroup.endGroup;
+            token = beginGroup.endGroup.next;
           }
         }
+        if (!optional('(', token)) {
+          if (optional(';', token)) {
+            listener.recoverableError(token, "expected '('");
+          }
+          token = listener.unexpected(token);
+        }
+        if (token is BeginGroupToken) {
+          BeginGroupToken beginGroup = token;
+          if (beginGroup.endGroup == null) {
+            listener.unmatched(beginGroup);
+          }
+          token = beginGroup.endGroup.next;
+        }
       }
-      token = token.next;
     }
     return const Link<Token>();
   }
@@ -1268,22 +1384,47 @@ class Parser {
    */
   Token peekAfterType(Token token) {
     // We are looking at "identifier ...".
-    Token peek = token.next;
-    if (identical(peek.kind, PERIOD_TOKEN)) {
-      if (peek.next.isIdentifier()) {
-        // Look past a library prefix.
-        peek = peek.next.next;
+    Token peek = token;
+    if (!isGeneralizedFunctionType(token)) {
+      peek = token.next;

[Siggi Cherem (dart-lang), 2016/12/29 22:46:58]

suggestion: consider introducing 2 functions to make it simpler to follow:
 - peekAfterNominalType (for what used to be peekAfterType)
 - peekAfterFunctionType (for the new code)

[floitsch, 2016/12/30 14:55:48]

Done.

+      if (identical(peek.kind, PERIOD_TOKEN)) {
+        if (peek.next.isIdentifier()) {
+          // Look past a library prefix.
+          peek = peek.next.next;
+        }
+      }
+      // We are looking at "qualified ...".
+      if (identical(peek.kind, LT_TOKEN)) {
+        // Possibly generic type.
+        // We are looking at "qualified '<'".
+        BeginGroupToken beginGroupToken = peek;
+        Token gtToken = beginGroupToken.endGroup;
+        if (gtToken != null) {
+          // We are looking at "qualified '<' ... '>' ...".
+          peek = gtToken.next;
+        }
       }
     }
-    // We are looking at "qualified ...".
-    if (identical(peek.kind, LT_TOKEN)) {
-      // Possibly generic type.
-      // We are looking at "qualified '<'".
-      BeginGroupToken beginGroupToken = peek;
-      Token gtToken = beginGroupToken.endGroup;
-      if (gtToken != null) {
-        // We are looking at "qualified '<' ... '>' ...".
-        return gtToken.next;
+
+    // We might have just skipped over the return value of the function type.
+    // Check again, if we are now at a function type position.
+    if (isGeneralizedFunctionType(peek)) {

[Siggi Cherem (dart-lang), 2016/12/29 22:46:58]

do we need a loop here, like you had in parseType?

If so, then let's definitely add the helper functions. I imagine the code would
look more like:

Token peekAfterType(Token token) {
  Token peek = !isGeneralizedFunctionType(token)
      ? peekAfterNominalType(token)
      : token;

  // We are looking at "type Function ..."
  while (isGeneralizedFunctionType(peek)) {
    peek = peekAfterFunctionType(peek);
  }

  return peek;

[floitsch, 2016/12/30 14:55:48]

I'm not sure the loop is necessary, but I don't want to prove it isn't. It's
definitely cleaner and less error-prone doing the loop.
Code looks better now.
thanks.
done.

+      peek = peek.next;  // Skip over the Function.

[Siggi Cherem (dart-lang), 2016/12/29 22:46:57]

nit: reword "the Function" to "the `Function` token"

[floitsch, 2016/12/30 14:55:47]

Done.

+      if (identical(peek.kind, LT_TOKEN)) {

[Siggi Cherem (dart-lang), 2016/12/29 22:46:58]

consider adding here and below comments describing what we are looking at
(similar to those above):

  // We are looking at "type Function '<' ... '>' ..."
  // We are looking at "type Function type-args '(' ... ')' ..."

[floitsch, 2016/12/30 14:55:48]

Done.

+        BeginGroupToken beginGroupToken = peek;
+        Token closeToken = beginGroupToken.endGroup;
+        if (closeToken != null) {
+          peek = closeToken.next;
+        }
+      }
+
+      // There must be an open paren to be valid, but we are not checking here.
+      if (identical(peek.kind, OPEN_PAREN_TOKEN)) {
+        BeginGroupToken beginGroupToken = peek;
+        Token closeToken = beginGroupToken.endGroup;
+        if (closeToken != null) {
+          peek = closeToken.next;
+        }
       }
     }
     return peek;
@@ -1828,7 +1969,8 @@ class Parser {
       int afterIdKind = afterId.kind;
       if (identical(afterIdKind, EQ_TOKEN) ||
           identical(afterIdKind, SEMICOLON_TOKEN) ||
-          identical(afterIdKind, COMMA_TOKEN)) {
+          identical(afterIdKind, COMMA_TOKEN) ||
+          isGeneralizedFunctionType(identifier)) {

[Siggi Cherem (dart-lang), 2016/12/29 22:46:58]

why do we need this case? could this be handled by the changes to peekAfterType?

[floitsch, 2016/12/30 14:55:48]

Yes. thanks for figuring this out.
I should have paid more attention when I ran into this issue in my tests.

         // We are looking at "type identifier" followed by '=', ';', ','.
         return parseVariablesDeclaration(token);
       } else if (identical(afterIdKind, OPEN_PAREN_TOKEN)) {