initial generic method comment parsing

pkg/analyzer/lib/src/generated/parser.dart

 // Copyright (c) 2014, 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 engine.parser;
 
 import 'dart:collection';
 import "dart:math" as math;
 
 import 'ast.dart';
@@ skipping 2105 matching lines @@
    * initializer, with no intervening parens, braces, or brackets.
    */
   bool _inInitializer = false;
 
   /**
    * A flag indicating whether the parser is to parse generic method syntax.
    */
   bool parseGenericMethods = false;
 
   /**
+   * A flag indicating whether to parse generic method comments, of the form
+   * `/*=T*/` and `/*<T>*/`.
+   */
+  bool parseGenericMethodComments = false;

[Jennifer Messerly, 2015/11/11 18:28:06]

aside, we don't actually need this flag in the parser -- it just tells us we
don't need to look for the special TokenTypes that the scanner will have
produced. After chatting with Leaf & Vijay, it sounds like maybe we can remove
this flag here, if that sounds good.

[Paul Berry, 2015/11/11 19:26:53]

My two cents: I'd actually lean slightly toward leaving the flag here, since it
will make it easier to find the code we need to remove once generic methods are
implemented for real and we want to get rid of the comment syntax.

But I don't feel terribly strongly either way.

[Jennifer Messerly, 2015/11/11 19:30:25]

good point.

[Brian Wilkerson, 2015/11/11 21:53:42]

I agree, I'd leave the flag in. You'll probably eventually want to add it to
AnalysisOptions and use that to initialize the field, then we can plumb it all
the way through to the .analysis_options file so that you can test the code more
easily.

That said, we had a conversation in the DDC meeting this morning about whether
this should be a separate flag, or whether it should be implied by strongMode.
I'm fine with the way you have it.

+
+  /**
    * Initialize a newly created parser to parse the content of the given
    * [_source] and to report any errors that are found to the given
    * [_errorListener].
    */
   Parser(this._source, this._errorListener);
 
   void set currentToken(Token currentToken) {
     this._currentToken = currentToken;
   }
 
@@ skipping 372 matching lines @@
           commentAndMetadata,
           modifiers.externalKeyword,
           _validateModifiersForConstructor(modifiers),
           modifiers.factoryKeyword,
           parseSimpleIdentifier(),
           getAndAdvance(),
           parseSimpleIdentifier(),
           parseFormalParameterList());
     } else if (_tokenMatches(_peek(), TokenType.OPEN_PAREN)) {
       SimpleIdentifier methodName = parseSimpleIdentifier();
+      TypeParameterList typeParameters = _parseGenericCommentTypeParameters();
       FormalParameterList parameters = parseFormalParameterList();
       if (_matches(TokenType.COLON) ||
           modifiers.factoryKeyword != null ||
           methodName.name == className) {
         return _parseConstructor(
             commentAndMetadata,
             modifiers.externalKeyword,
             _validateModifiersForConstructor(modifiers),
             modifiers.factoryKeyword,
             methodName,
             null,
             null,
             parameters);
       }
       _validateModifiersForGetterOrSetterOrMethod(modifiers);
       _validateFormalParameterList(parameters);
       return _parseMethodDeclarationAfterParameters(
           commentAndMetadata,
           modifiers.externalKeyword,
           modifiers.staticKeyword,
           null,
           methodName,
-          null,
+          typeParameters,
           parameters);
     } else if (_peek()
         .matchesAny([TokenType.EQ, TokenType.COMMA, TokenType.SEMICOLON])) {
       if (modifiers.constKeyword == null &&
           modifiers.finalKeyword == null &&
           modifiers.varKeyword == null) {
         _reportErrorForCurrentToken(
             ParserErrorCode.MISSING_CONST_FINAL_VAR_OR_TYPE);
       }
       return _parseInitializedIdentifierList(commentAndMetadata,
@@ skipping 59 matching lines @@
         return _parseInitializedIdentifierList(
             commentAndMetadata,
             modifiers.staticKeyword,
             _validateModifiersForField(modifiers),
             type);
       } finally {
         _unlockErrorListener();
       }
     } else if (_tokenMatches(_peek(), TokenType.OPEN_PAREN)) {
       SimpleIdentifier methodName = parseSimpleIdentifier();
+      TypeParameterList typeParameters = _parseGenericCommentTypeParameters();
       FormalParameterList parameters = parseFormalParameterList();
       if (methodName.name == className) {
         _reportErrorForNode(ParserErrorCode.CONSTRUCTOR_WITH_RETURN_TYPE, type);
         return _parseConstructor(
             commentAndMetadata,
             modifiers.externalKeyword,
             _validateModifiersForConstructor(modifiers),
             modifiers.factoryKeyword,
             methodName,
             null,
             null,
             parameters);
       }
       _validateModifiersForGetterOrSetterOrMethod(modifiers);
       _validateFormalParameterList(parameters);
       return _parseMethodDeclarationAfterParameters(
           commentAndMetadata,
           modifiers.externalKeyword,
           modifiers.staticKeyword,
           type,
           methodName,
-          null,
+          typeParameters,
           parameters);
     } else if (parseGenericMethods && _tokenMatches(_peek(), TokenType.LT)) {
       return _parseMethodDeclarationAfterReturnType(commentAndMetadata,
           modifiers.externalKeyword, modifiers.staticKeyword, type);
     } else if (_tokenMatches(_peek(), TokenType.OPEN_CURLY_BRACKET)) {
       // We have found "TypeName identifier {", and are guessing that this is a
       // getter without the keyword 'get'.
       _validateModifiersForGetterOrSetterOrMethod(modifiers);
       _reportErrorForCurrentToken(ParserErrorCode.MISSING_GET);
       _currentToken = _injectToken(
@@ skipping 476 matching lines @@
   }
 
   /**
    * Parse a function expression. Return the function expression that was
    * parsed.
    *
    *     functionExpression ::=
    *         typeParameters? formalParameterList functionExpressionBody
    */
   FunctionExpression parseFunctionExpression() {
-    TypeParameterList typeParameters = null;
-    if (parseGenericMethods && _matches(TokenType.LT)) {
-      typeParameters = parseTypeParameterList();
-    }
+    TypeParameterList typeParameters = _parseGenericMethodTypeParameters();
     FormalParameterList parameters = parseFormalParameterList();
     _validateFormalParameterList(parameters);
     FunctionBody body =
         _parseFunctionBody(false, ParserErrorCode.MISSING_FUNCTION_BODY, true);
     return new FunctionExpression(typeParameters, parameters, body);
   }
 
   /**
    * Parse an if-null expression.  Return the if-null expression that was
    * parsed.
@@ skipping 106 matching lines @@
   NormalFormalParameter parseNormalFormalParameter() {
     CommentAndMetadata commentAndMetadata = _parseCommentAndMetadata();
     FinalConstVarOrType holder = _parseFinalConstVarOrType(true);
     Token thisKeyword = null;
     Token period = null;
     if (_matchesKeyword(Keyword.THIS)) {
       thisKeyword = getAndAdvance();
       period = _expect(TokenType.PERIOD);
     }
     SimpleIdentifier identifier = parseSimpleIdentifier();
-    TypeParameterList typeParameters = null;
-    if (parseGenericMethods && _matches(TokenType.LT)) {
-      typeParameters = parseTypeParameterList();
-    }
+    TypeParameterList typeParameters = _parseGenericMethodTypeParameters();
     if (_matches(TokenType.OPEN_PAREN)) {
       FormalParameterList parameters = parseFormalParameterList();
       if (thisKeyword == null) {
         if (holder.keyword != null) {
           _reportErrorForToken(
               ParserErrorCode.FUNCTION_TYPED_PARAMETER_VAR, holder.keyword);
         }
         return new FunctionTypedFormalParameter(
             commentAndMetadata.comment,
             commentAndMetadata.metadata,
@@ skipping 187 matching lines @@
     return new TypeArgumentList(leftBracket, arguments, rightBracket);
   }
 
   /**
    * Parse a type name. Return the type name that was parsed.
    *
    *     type ::=
    *         qualified typeArguments?
    */
   TypeName parseTypeName() {
+    TypeName result = _parseTypeName();
+    // If this is followed by a generic method type comment, allow the comment
+    // type to replace the real type name.
+    // TODO(jmesserly): this feels like a big hammer. Can we restrict it to
+    // only work inside generic methods?

[Jennifer Messerly, 2015/11/11 18:28:06]

This feature weirds me out. I kind of wonder if it would be nicer to handle
these comments later, after we already know we're in a generic method, and we
know what type parameter names to expect. Then we could only match comments that
exactly refer to those type parameters.

That said, this is all a very temporary measure, and searching our corpus of
code didn't find any other uses of comments of the form /*=T*/.

[Brian Wilkerson, 2015/11/11 21:53:42]

Given that this only applies to return types and parameter types, perhaps the
extra work should be done there rather than for all type names.

[Paul Berry, 2015/11/11 22:01:58]

I would think it could potentially apply to other types names, such as those
used in local variable declarations, e.g.:

void f/*<T>*/() {
  var/*=T*/ x;
}

[Jennifer Messerly, 2015/11/11 22:04:12]

EDIT: Paul beat me to it. But here was my comment.

in theory it's to cover all places generic type parameters can appear. Some
examples:

    List<num/*=T*/> foo;

    foo = new List<num/*=T*/>();
    
   <String, dynamic/*=V*/>{}

    if (x is dynamic/*=K*/) { ... }

    var firstClassType = dynamic/*=T*/;

(I suspect the best choice for the "replaced" type will be whatever the upper
type bound is. Typically `dynamic` but can be other types if `T extends
SomeType` ... I wish this could be enforced but doesn't seem possible in the
parser.)

+    if (_injectGenericCommentTypeAssign()) {
+      return _parseTypeName();
+    }
+    return result;
+  }
+
+  TypeName _parseTypeName() {
     Identifier typeName;
     if (_matchesKeyword(Keyword.VAR)) {
       _reportErrorForCurrentToken(ParserErrorCode.VAR_AS_TYPE_NAME);
       typeName = new SimpleIdentifier(getAndAdvance());
     } else if (_matchesIdentifier()) {
       typeName = parsePrefixedIdentifier();
     } else {
       typeName = _createSyntheticIdentifier();
       _reportErrorForCurrentToken(ParserErrorCode.EXPECTED_TYPE_NAME);
     }
-    TypeArgumentList typeArguments = null;
-    if (_matches(TokenType.LT)) {
-      typeArguments = parseTypeArgumentList();
-    }
+    TypeArgumentList typeArguments = _maybeParseTypeArguments();
     return new TypeName(typeName, typeArguments);
   }
 
   /**
    * Parse a type parameter. Return the type parameter that was parsed.
    *
    *     typeParameter ::=
    *         metadata name ('extends' bound)?
    */
   TypeParameter parseTypeParameter() {
@@ skipping 399 matching lines @@
    * either the given token is not a begin token or it does not have an end
    * token associated with it.
    */
   Token _getEndToken(Token beginToken) {
     if (beginToken is BeginToken) {
       return beginToken.endToken;
     }
     return null;
   }
 
+  bool _injectGenericComment(TokenType type, int prefixLen) {
+    if (parseGenericMethodComments) {

[Jennifer Messerly, 2015/11/11 18:28:06]

as noted above, this guard shouldn't really be necessary, but it lets us skip
the comment search. Premature optimization?

+      CommentToken t = _currentToken.precedingComments;
+      for (; t != null; t = t.next) {
+        if (t.type == TokenType.GENERIC_METHOD_TYPE_LIST) {

[Jennifer Messerly, 2015/11/11 20:52:27]

Bug here. the /*=T*/ comments weren't working because of it. The tests weren't
checking the tree well enough to catch it. Fixing this & improving the test
coverage by validating the parameter and return types.

+          String comment = t.lexeme.substring(prefixLen, t.lexeme.length - 2);
+          Token list = _scanGenericMethodComment(comment, t.offset + prefixLen);

[Paul Berry, 2015/11/11 19:26:53]

What will happen if the comment is matched by
Scanner._matchGenericMethodCommentType() but is not actually grammatically valid
(e.g. "/*<hi, mom!>*/").  Will the parser get horribly confused?

[Jennifer Messerly, 2015/11/11 19:30:25]

Should be exactly as if you wrote that without the comment, a parse error of
some sort.

That's a good point though. We could use some more negative tests for these
cases in parser_test. I'll add some.

+          if (list != null) {
+            // TODO(jmesserly): detach the old comment token?

[Brian Wilkerson, 2015/11/11 21:53:42]

It shouldn't actually hurt anything if you leave it.

[Jennifer Messerly, 2015/11/12 00:20:00]

Sounds good. Removed the TODO.

+            // Insert the tokens into the stream.
+            _injectTokenList(list);
+            return true;
+          }
+        }
+      }
+    }
+    return false;
+  }
+
+  /**
+     * Matches a generic comment type substitution and injects it into the token

[Jennifer Messerly, 2015/11/11 18:28:06]

oops, fixed this locally. Not sure how it happened

+     * stream. Returns true if a match was injected, otherwise false.
+     *
+     * These comments are of the form `/*=T*/`, in other words, a [TypeName]
+     * inside a slash-star comment, preceded by equals sign.
+     */
+  bool _injectGenericCommentTypeAssign() {
+    return _injectGenericComment(TokenType.GENERIC_METHOD_TYPE_ASSIGN, 3);
+  }
+
+  /**
+   * Matches a generic comment type parameters and injects them into the token
+   * stream. Returns true if a match was injected, otherwise false.
+   *
+   * These comments are of the form `/*<K, V>*/`, in other words, a
+   * [TypeParameterList] or [TypeArgumentList] inside a slash-star comment.
+   */
+  bool _injectGenericCommentTypeList() {
+    return _injectGenericComment(TokenType.GENERIC_METHOD_TYPE_LIST, 2);
+  }
+
   /**
    * Inject the given [token] into the token stream immediately before the
    * current token.
    */
   Token _injectToken(Token token) {
     Token previous = _currentToken.previous;
     token.setNext(_currentToken);
     previous.setNext(token);
     return token;
   }
 
+  void _injectTokenList(Token firstToken) {

[Jennifer Messerly, 2015/11/11 18:28:06]

I used the same pattern as the existing _injectToken.

Is there anything to be worried about here? It is mutating the token linked
list, but we seem to do that in other places. Not sure if I should be marking
each token as synthetic or something like that?

Anyway something to try out in the UI -- what does this colorize like.

+    // Scanner creates a cyclic EOF token.
+    Token lastToken = firstToken;
+    while (lastToken.next.type != TokenType.EOF) {
+      lastToken = lastToken.next;
+    }
+    // Inject these new tokens into the stream.
+    Token previous = _currentToken.previous;
+    lastToken.setNext(_currentToken);
+    previous.setNext(firstToken);
+    _currentToken = firstToken;
+  }
+
   /**
    * Return `true` if the current token appears to be the beginning of a
    * function declaration.
    */
   bool _isFunctionDeclaration() {
     if (_matchesKeyword(Keyword.VOID)) {
       return true;
     }
     Token afterReturnType = _skipTypeName(_currentToken);
     if (afterReturnType == null) {
@@ skipping 331 matching lines @@
       _tokenMatchesKeyword(_currentToken, keyword);
 
   /**
    * Return `true` if the current token matches the given [identifier].
    */
   bool _matchesString(String identifier) =>
       _currentToken.type == TokenType.IDENTIFIER &&
           _currentToken.lexeme == identifier;
 
   /**
+   * Parse a [TypeArgumentList] if present, otherwise return null.
+   * This also supports the comment form, if enabled: `/*<T>*/`
+   */
+  TypeArgumentList _maybeParseTypeArguments() {

[Brian Wilkerson, 2015/11/11 21:53:42]

For consistency with "_parseOptionalReturnType", perhaps
"_parseOptionalTypeArguments"?

[Jennifer Messerly, 2015/11/12 00:20:00]

Done.

+    if (_matches(TokenType.LT) || _injectGenericCommentTypeList()) {
+      return parseTypeArgumentList();
+    }
+    return null;
+  }
+
+  /**
    * If the current token has the given [type], then advance to the next token
    * and return `true`. Otherwise, return `false` without advancing. This method
    * should not be invoked with an argument value of [TokenType.GT].
    */
   bool _optional(TokenType type) {
     if (_matches(type)) {
       _advance();
       return true;
     }
     return false;
@@ skipping 70 matching lines @@
     }
     //
     // A primary expression can start with an identifier. We resolve the
     // ambiguity by determining whether the primary consists of anything other
     // than an identifier and/or is followed by an assignableSelector.
     //
     Expression expression = _parsePrimaryExpression();
     bool isOptional = primaryAllowed || expression is SimpleIdentifier;
     while (true) {
       while (_isLikelyParameterList()) {
-        TypeArgumentList typeArguments = null;
-        if (_matches(TokenType.LT)) {
-          typeArguments = parseTypeArgumentList();
-        }
+        TypeArgumentList typeArguments = _maybeParseTypeArguments();
         ArgumentList argumentList = parseArgumentList();
         if (expression is SimpleIdentifier) {
           expression = new MethodInvocation(null, null,
               expression as SimpleIdentifier, typeArguments, argumentList);
         } else if (expression is PrefixedIdentifier) {
           PrefixedIdentifier identifier = expression as PrefixedIdentifier;
           expression = new MethodInvocation(
               identifier.prefix,
               identifier.period,
               identifier.identifier,
@@ skipping 188 matching lines @@
       }
     } else {
       _reportErrorForToken(ParserErrorCode.MISSING_IDENTIFIER, _currentToken,
           [_currentToken.lexeme]);
       functionName = _createSyntheticIdentifier();
     }
     assert((expression == null && functionName != null) ||
         (expression != null && functionName == null));
     if (_isLikelyParameterList()) {
       while (_isLikelyParameterList()) {
-        TypeArgumentList typeArguments = null;
-        if (_matches(TokenType.LT)) {
-          typeArguments = parseTypeArgumentList();
-        }
+        TypeArgumentList typeArguments = _maybeParseTypeArguments();
         if (functionName != null) {
           expression = new MethodInvocation(expression, period, functionName,
               typeArguments, parseArgumentList());
           period = null;
           functionName = null;
         } else if (expression == null) {
           // It should not be possible to get here.
           expression = new MethodInvocation(expression, period,
               _createSyntheticIdentifier(), typeArguments, parseArgumentList());
         } else {
           expression = new FunctionExpressionInvocation(
               expression, typeArguments, parseArgumentList());
         }
       }
     } else if (functionName != null) {
       expression = new PropertyAccess(expression, period, functionName);
       period = null;
     }
     assert(expression != null);
     bool progress = true;
     while (progress) {
       progress = false;
       Expression selector = _parseAssignableSelector(expression, true);
       if (!identical(selector, expression)) {
         expression = selector;
         progress = true;
         while (_isLikelyParameterList()) {
-          TypeArgumentList typeArguments = null;
-          if (_matches(TokenType.LT)) {
-            typeArguments = parseTypeArgumentList();
-          }
+          TypeArgumentList typeArguments = _maybeParseTypeArguments();
           if (expression is PropertyAccess) {
             PropertyAccess propertyAccess = expression as PropertyAccess;
             expression = new MethodInvocation(
                 propertyAccess.target,
                 propertyAccess.operator,
                 propertyAccess.propertyName,
                 typeArguments,
                 parseArgumentList());
           } else {
             expression = new FunctionExpressionInvocation(
@@ skipping 1397 matching lines @@
     bool isGetter = false;
     if (_matchesKeyword(Keyword.GET) &&
         !_tokenMatches(_peek(), TokenType.OPEN_PAREN)) {
       keyword = getAndAdvance();
       isGetter = true;
     } else if (_matchesKeyword(Keyword.SET) &&
         !_tokenMatches(_peek(), TokenType.OPEN_PAREN)) {
       keyword = getAndAdvance();
     }
     SimpleIdentifier name = parseSimpleIdentifier();
-    TypeParameterList typeParameters = null;
-    if (parseGenericMethods && _matches(TokenType.LT)) {
-      typeParameters = parseTypeParameterList();
-    }
+    TypeParameterList typeParameters = _parseGenericMethodTypeParameters();
     FormalParameterList parameters = null;
     if (!isGetter) {
       if (_matches(TokenType.OPEN_PAREN)) {
         parameters = parseFormalParameterList();
         _validateFormalParameterList(parameters);
       } else {
         _reportErrorForCurrentToken(
             ParserErrorCode.MISSING_FUNCTION_PARAMETERS);
         parameters = new FormalParameterList(
             _createSyntheticToken(TokenType.OPEN_PAREN),
@@ skipping 133 matching lines @@
         commentAndMetadata.metadata,
         keyword,
         returnType,
         name,
         typeParameters,
         parameters,
         semicolon);
   }
 
   /**
+     * Parses generic type parameters from a comment.
+     *
+     * Normally this is handled by [_parseGenericMethodTypeParameters], but if the

[Jennifer Messerly, 2015/11/11 18:28:06]

fixed this locally, will be in next upload

+     * code already handles the normal generic type parameters, the comment
+     * matcher can be called directly. For example, we may have already tried
+     * matching `<` (less than sign) in a method declaration, and be currently
+     * on the `(` (open paren) because we didn't find it. In that case, this
+     * function will parse the preceding comment such as `/*<T, R>*/`.
+     */
+  TypeParameterList _parseGenericCommentTypeParameters() {
+    if (_injectGenericCommentTypeList()) {
+      return parseTypeParameterList();
+    }
+    return null;
+  }
+
+  /**
+   * Parse the generic method or function's type parameters.
+   *
+   * For backwards compatibility this can optionally use comments.
+   * See [parseGenericMethodComments].
+   */
+  TypeParameterList _parseGenericMethodTypeParameters() {
+    if (parseGenericMethods && _matches(TokenType.LT) ||
+        _injectGenericCommentTypeList()) {
+      return parseTypeParameterList();
+    }
+  }
+
+  /**
    * Parse a getter. The [commentAndMetadata] is the documentation comment and
    * metadata to be associated with the declaration. The externalKeyword] is the
    * 'external' token. The staticKeyword] is the static keyword, or `null` if
    * the getter is not static. The [returnType] the return type that has already
    * been parsed, or `null` if there was no return type. Return the getter that
    * was parsed.
    *
    *     getter ::=
    *         getterSignature functionBody?
    *
@@ skipping 271 matching lines @@
   /**
    * Parse a list or map literal. The [modifier] is the 'const' modifier
    * appearing before the literal, or `null` if there is no modifier. Return the
    * list or map literal that was parsed.
    *
    *     listOrMapLiteral ::=
    *         listLiteral
    *       | mapLiteral
    */
   TypedLiteral _parseListOrMapLiteral(Token modifier) {
-    TypeArgumentList typeArguments = null;
-    if (_matches(TokenType.LT)) {
-      typeArguments = parseTypeArgumentList();
-    }
+    TypeArgumentList typeArguments = _maybeParseTypeArguments();
     if (_matches(TokenType.OPEN_CURLY_BRACKET)) {
       return _parseMapLiteral(modifier, typeArguments);
     } else if (_matches(TokenType.OPEN_SQUARE_BRACKET) ||
         _matches(TokenType.INDEX)) {
       return _parseListLiteral(modifier, typeArguments);
     }
     _reportErrorForCurrentToken(ParserErrorCode.EXPECTED_LIST_OR_MAP_LITERAL);
     return new ListLiteral(
         modifier,
         typeArguments,
@@ skipping 113 matching lines @@
    *     functionDeclaration ::=
    *         'external'? 'static'? functionSignature functionBody
    *       | 'external'? functionSignature ';'
    */
   MethodDeclaration _parseMethodDeclarationAfterReturnType(
       CommentAndMetadata commentAndMetadata,
       Token externalKeyword,
       Token staticKeyword,
       TypeName returnType) {
     SimpleIdentifier methodName = parseSimpleIdentifier();
-    TypeParameterList typeParameters = null;
-    if (parseGenericMethods && _matches(TokenType.LT)) {
-      typeParameters = parseTypeParameterList();
-    }
+    TypeParameterList typeParameters = _parseGenericMethodTypeParameters();
     FormalParameterList parameters;
     if (!_matches(TokenType.OPEN_PAREN) &&
         (_matches(TokenType.OPEN_CURLY_BRACKET) ||
             _matches(TokenType.FUNCTION))) {
       _reportErrorForToken(
           ParserErrorCode.MISSING_METHOD_PARAMETERS, _currentToken.previous);
       parameters = new FormalParameterList(
           _createSyntheticToken(TokenType.OPEN_PAREN),
           null,
           null,
@@ skipping 447 matching lines @@
    */
   Expression _parsePostfixExpression() {
     Expression operand = _parseAssignableExpression(true);
     if (_matches(TokenType.OPEN_SQUARE_BRACKET) ||
         _matches(TokenType.PERIOD) ||
         _matches(TokenType.QUESTION_PERIOD) ||
         _matches(TokenType.OPEN_PAREN) ||
         (parseGenericMethods && _matches(TokenType.LT))) {
       do {
         if (_isLikelyParameterList()) {
-          TypeArgumentList typeArguments = null;
-          if (_matches(TokenType.LT)) {
-            typeArguments = parseTypeArgumentList();
-          }
+          TypeArgumentList typeArguments = _maybeParseTypeArguments();
           ArgumentList argumentList = parseArgumentList();
           if (operand is PropertyAccess) {
             PropertyAccess access = operand as PropertyAccess;
             operand = new MethodInvocation(access.target, access.operator,
                 access.propertyName, typeArguments, argumentList);
           } else {
             operand = new FunctionExpressionInvocation(
                 operand, typeArguments, argumentList);
           }
         } else {
@@ skipping 979 matching lines @@
   void _reportErrorForToken(ErrorCode errorCode, Token token,
       [List<Object> arguments]) {
     if (token.type == TokenType.EOF) {
       token = token.previous;
     }
     _reportError(new AnalysisError(_source, token.offset,
         math.max(token.length, 1), errorCode, arguments));
   }
 
   /**
+   * Scans the generic method comment, and returns the tokens, otherwise
+   * returns null.
+   */
+  Token _scanGenericMethodComment(String code, int offset) {
+    BooleanErrorListener listener = new BooleanErrorListener();
+    Scanner scanner =
+        new Scanner(null, new SubSequenceReader(code, offset), listener);
+    scanner.setSourceStart(1, 1);
+    Token firstToken = scanner.tokenize();
+    if (listener.errorReported) {
+      // TODO(jmesserly): should we report these messages?

[Brian Wilkerson, 2015/11/11 21:53:42]

It would help you find bugs while using this feature, but given that it is
short-lived it doesn't seem necessary.

[Jennifer Messerly, 2015/11/12 00:20:00]

Good point. Removed the TODO.

+      return null;
+    }
+    return firstToken;
+  }
+
+  /**
    * Skips a block with all containing blocks.
    */
   void _skipBlock() {
     Token endToken = (_currentToken as BeginToken).endToken;
     if (endToken == null) {
       endToken = _currentToken.next;
       while (!identical(endToken, _currentToken)) {
         _currentToken = endToken;
         endToken = _currentToken.next;
       }
@@ skipping 316 matching lines @@
    * This method must be kept in sync with [parseTypeArgumentList].
    *
    *     typeArguments ::=
    *         '<' typeList '>'
    *
    *     typeList ::=
    *         type (',' type)*
    */
   Token _skipTypeArgumentList(Token startToken) {
     Token token = startToken;
-    if (!_tokenMatches(token, TokenType.LT)) {
+    if (!_tokenMatches(token, TokenType.LT) &&
+        !_injectGenericCommentTypeList()) {
       return null;
     }
     token = _skipTypeName(token.next);
     if (token == null) {
       // If the start token '<' is followed by '>'
       // then assume this should be type argument list but is missing a type
       token = startToken.next;
       if (_tokenMatches(token, TokenType.GT)) {
         return token.next;
       }
@@ skipping 2619 matching lines @@
   }
 
   /**
    * Copy resolution data from the [fromNode] to the [toNode].
    */
   static void copyResolutionData(AstNode fromNode, AstNode toNode) {
     ResolutionCopier copier = new ResolutionCopier();
     copier._isEqualNodes(fromNode, toNode);
   }
 }