fix #25739, incorrectly expanding a generic comment multiple times

[email protected], [email protected]

Review URL: https://codereview.chromium.org/1686173002 .

Author: John Messerly

pkg/analyzer/lib/src/dart/ast/token.dart

@@ -35,7 +35,7 @@ class BeginToken extends SimpleToken {
 /**
  * A begin token that is preceded by comments.
  */
-class BeginTokenWithComment extends BeginToken {
+class BeginTokenWithComment extends BeginToken implements TokenWithComment {
   /**
    * The first comment in the list of comments that precede this token.
    */
@@ -79,9 +79,9 @@ class BeginTokenWithComment extends BeginToken {
  */
 class CommentToken extends StringToken {
   /**
-   * The [Token] that contains this comment.
+   * The token that contains this comment.
    */
-  Token parent;
+  TokenWithComment parent;
 
   /**
    * Initialize a newly created token to represent a token of the given [type]
@@ -92,6 +92,21 @@ class CommentToken extends StringToken {
 
   @override
   CommentToken copy() => new CommentToken(type, _value, offset);
+
+  /**
+   * Remove this comment token from the list.
+   *
+   * This is used when we decide to interpret the comment as syntax.
+   */
+  void remove() {
+    if (previous != null) {
+      previous.setNextWithoutSettingPrevious(next);
+      next?.previous = previous;
+    } else {
+      assert(parent.precedingComments == this);
+      parent.precedingComments = next;
+    }
+  }
 }
 
 /**
@@ -149,7 +164,7 @@ class KeywordToken extends SimpleToken {
 /**
  * A keyword token that is preceded by comments.
  */
-class KeywordTokenWithComment extends KeywordToken {
+class KeywordTokenWithComment extends KeywordToken implements TokenWithComment {
   /**
    * The first comment in the list of comments that precede this token.
    */
@@ -340,7 +355,7 @@ class StringToken extends SimpleToken {
 /**
  * A string token that is preceded by comments.
  */
-class StringTokenWithComment extends StringToken {
+class StringTokenWithComment extends StringToken implements TokenWithComment {
   /**
    * The first comment in the list of comments that precede this token.
    */

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

@@ -3951,6 +3951,8 @@ class Parser {
           String comment = t.lexeme.substring(prefixLen, t.lexeme.length - 2);
           Token list = _scanGenericMethodComment(comment, t.offset + prefixLen);
           if (list != null) {
+            // Remove the token from the comment stream.
+            t.remove();
             // Insert the tokens into the stream.
             _injectTokenList(list);
             return true;

pkg/analyzer/test/generated/parser_test.dart

@@ -1531,6 +1531,50 @@ class Foo {
         reason: 'parser recovers what it can');
   }
 
+  void test_method_invalidTypeParameterExtends() {
+    // Regression test for https://github.com/dart-lang/sdk/issues/25739.
+
+    // TODO(jmesserly): ideally we'd be better at parser recovery here.
+    enableGenericMethods = true;
+    MethodDeclaration method = parse3(
+        "parseClassMember",
+        <Object>["C"],
+        "f<E>(E extends num p);",
+        [
+          ParserErrorCode.MISSING_IDENTIFIER, // `extends` is a keyword
+          ParserErrorCode.EXPECTED_TOKEN, // comma
+          ParserErrorCode.EXPECTED_TOKEN, // close paren
+          ParserErrorCode.MISSING_FUNCTION_BODY
+        ]);
+    expect(method.parameters.toString(), '(E, extends)',
+        reason: 'parser recovers what it can');
+  }
+
+
+  void test_method_invalidTypeParameterExtendsComment() {
+    // Regression test for https://github.com/dart-lang/sdk/issues/25739.
+
+    // TODO(jmesserly): ideally we'd be better at parser recovery here.
+    // Also, this behavior is slightly different from how we would parse a
+    // normal generic method, because we "discover" the comment at a different
+    // point in the parser. This has a slight effect on the AST that results
+    // from error recovery.
+    enableGenericMethodComments = true;
+    MethodDeclaration method = parse3(
+        "parseClassMember",
+        <Object>["C"],
+        "f/*<E>*/(dynamic/*=E extends num*/p);",
+        [
+          ParserErrorCode.MISSING_IDENTIFIER, // `extends` is a keyword
+          ParserErrorCode.EXPECTED_TOKEN, // comma
+          ParserErrorCode.MISSING_IDENTIFIER, // `extends` is a keyword
+          ParserErrorCode.EXPECTED_TOKEN, // close paren
+          ParserErrorCode.MISSING_FUNCTION_BODY
+        ]);
+    expect(method.parameters.toString(), '(E extends, extends)',
+        reason: 'parser recovers what it can');
+  }
+
   void test_missingAssignableSelector_identifiersAssigned() {
     parseExpression("x.y = y;");
   }

pkg/analyzer/test/src/task/strong/strong_test_helper.dart

@@ -74,7 +74,7 @@ void check() {
 
   // Extract expectations from the comments in the test files, and
   // check that all errors we emit are included in the expected map.
-  var allLibraries = reachableLibraries(initialLibrary.element.library);
+  var allLibraries = _reachableLibraries(initialLibrary.element.library);
   for (var lib in allLibraries) {
     for (var unit in lib.units) {
       var errors = <AnalysisError>[];
@@ -101,11 +101,11 @@ void checkFile(String content) {
   check();
 }
 
-SourceSpanWithContext createSpanHelper(
+SourceSpanWithContext _createSpanHelper(
     LineInfo lineInfo, int start, Source source, String content,
     {int end}) {
-  var startLoc = locationForOffset(lineInfo, source.uri, start);
-  var endLoc = locationForOffset(lineInfo, source.uri, end ?? start);
+  var startLoc = _locationForOffset(lineInfo, source.uri, start);
+  var endLoc = _locationForOffset(lineInfo, source.uri, end ?? start);
 
   var lineStart = startLoc.offset - startLoc.column;
   // Find the end of the line. This is not exposed directly on LineInfo, but
@@ -119,7 +119,7 @@ SourceSpanWithContext createSpanHelper(
 
   if (end == null) {
     end = lineEnd;
-    endLoc = locationForOffset(lineInfo, source.uri, lineEnd);
+    endLoc = _locationForOffset(lineInfo, source.uri, lineEnd);
   }
 
   var text = content.substring(start, end);
@@ -137,7 +137,7 @@ String _errorCodeName(ErrorCode errorCode) {
   }
 }
 
-initStrongModeTests() {
+void initStrongModeTests() {
   setUp(() {
     AnalysisEngine.instance.processRequiredPlugins();
     files = new MemoryResourceProvider();
@@ -151,15 +151,14 @@ initStrongModeTests() {
   });
 }
 
-// TODO(jmesserly): can we reuse the same mock SDK as Analyzer tests?
-SourceLocation locationForOffset(LineInfo lineInfo, Uri uri, int offset) {
+SourceLocation _locationForOffset(LineInfo lineInfo, Uri uri, int offset) {
   var loc = lineInfo.getLocation(offset);
   return new SourceLocation(offset,
       sourceUrl: uri, line: loc.lineNumber - 1, column: loc.columnNumber - 1);
 }
 
 /// Returns all libraries transitively imported or exported from [start].
-List<LibraryElement> reachableLibraries(LibraryElement start) {
+List<LibraryElement> _reachableLibraries(LibraryElement start) {
   var results = <LibraryElement>[];
   var seen = new Set();
   void find(LibraryElement lib) {
@@ -237,9 +236,11 @@ List<_ErrorExpectation> _findExpectedErrors(Token beginToken) {
       if (c.type == TokenType.MULTI_LINE_COMMENT) {
         String value = c.lexeme.substring(2, c.lexeme.length - 2);
         if (value.contains(':')) {
-          var offset = c.end;
-          if (c.next?.type == TokenType.GENERIC_METHOD_TYPE_LIST) {
-            offset += 2;
+          int offset = t.offset;
+          Token previous = t.previous;
+          while (previous != null && previous.offset > c.offset) {
+            offset = previous.offset;
+            previous = previous.previous;
           }
           for (var expectCode in value.split(',')) {
             var expected = _ErrorExpectation.parse(offset, expectCode);
@@ -266,7 +267,7 @@ void _reportFailure(
   String formatActualError(AnalysisError error) {
     int offset = error.offset;
     int length = error.length;
-    var span = createSpanHelper(
+    var span = _createSpanHelper(
         unit.lineInfo, offset, unit.element.source, sourceCode,
         end: offset + length);
     var levelName = _actualErrorLevel(error).name.toLowerCase();
@@ -276,7 +277,7 @@ void _reportFailure(
 
   String formatExpectedError(_ErrorExpectation error) {
     int offset = error.offset;
-    var span = createSpanHelper(
+    var span = _createSpanHelper(
         unit.lineInfo, offset, unit.element.source, sourceCode);
     var levelName = error.level.toString().toLowerCase();
     return '@$offset $levelName:${error.typeName}\n' + span.message('');