Refactor dartdoc parsing

This alters the way that dartdoc is parsed,
preventing null comment references from being pushed onto the stack.
This removes the need to remove those nulls prior to creating
the documentation comment AST nodes and allows for fixed sized lists.

This also address comments in:
* https://dart-review.googlesource.com/c/sdk/+/68520
* https://dart-review.googlesource.com/c/sdk/+/68461

Change-Id: I679c425b5d1f3f7954281d226815a80e73dcc033
Reviewed-on: https://dart-review.googlesource.com/68780
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>

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

@@ -260,6 +260,10 @@ class AstCloner implements AstVisitor<AstNode> {
 
   @override
   CommentReference visitCommentReference(CommentReference node) {
+    // Comment references have a token stream
+    // separate from the compilation unit's token stream.
+    // Clone the tokens in that stream here and add them to _clondedTokens
+    // for use when cloning the comment reference.
     Token token = node.beginToken;
     Token lastCloned = new Token.eof(-1);
     while (token != null) {

pkg/analyzer/lib/src/fasta/ast_builder.dart

@@ -283,7 +283,7 @@ class AstBuilder extends StackListener {
       }
     } else if (context == IdentifierContext.enumValueDeclaration) {
       List<Annotation> metadata = pop();
-      Comment comment = _parseDocumentationCommentOpt(token.precedingComments);
+      Comment comment = _findComment(null, token);
       push(ast.enumConstantDeclaration(comment, metadata, identifier));
     } else {
       push(identifier);
@@ -2561,17 +2561,8 @@ class AstBuilder extends StackListener {
 
   @override
   void handleCommentReferenceText(String referenceSource, int referenceOffset) {
-    ScannerResult result = scanString(referenceSource);
-    if (result.hasErrors) {
-      handleNoCommentReference();
-    } else {
-      Token token = result.tokens;
-      do {
-        token.offset += referenceOffset;
-        token = token.next;
-      } while (!token.isEof);
-      parser.parseOneCommentReference(result.tokens);
-    }
+    push(referenceSource);
+    push(referenceOffset);
   }
 
   @override
@@ -2585,11 +2576,6 @@ class AstBuilder extends StackListener {
     push(ast.commentReference(newKeyword, identifier));
   }
 
-  @override
-  void handleNoCommentReference() {
-    push(NullValue.CommentReference);
-  }
-
   ParameterKind _toAnalyzerParameterKind(FormalParameterKind type) {
     if (type == FormalParameterKind.optionalPositional) {
       return ParameterKind.POSITIONAL;
@@ -2601,87 +2587,61 @@ class AstBuilder extends StackListener {
   }
 
   Comment _findComment(List<Annotation> metadata, Token tokenAfterMetadata) {
-    Token commentsOnNext = tokenAfterMetadata?.precedingComments;
-    if (commentsOnNext != null) {
-      Comment comment = _parseDocumentationCommentOpt(commentsOnNext);
-      if (comment != null) {
-        return comment;
+    // Find the dartdoc tokens
+    Token dartdoc = parser.findDartDoc(tokenAfterMetadata);
+    if (dartdoc == null) {
+      if (metadata == null) {
+        return null;
       }
-    }
-    if (metadata != null) {
-      for (Annotation annotation in metadata) {
-        Token commentsBeforeAnnotation =
-            annotation.beginToken.precedingComments;
-        if (commentsBeforeAnnotation != null) {
-          Comment comment =
-              _parseDocumentationCommentOpt(commentsBeforeAnnotation);
-          if (comment != null) {
-            return comment;
-          }
+      int index = metadata.length;
+      while (true) {
+        if (index == 0) {
+          return null;
+        }
+        --index;
+        dartdoc = parser.findDartDoc(metadata[index].beginToken);
+        if (dartdoc != null) {
+          break;
         }
       }
     }
-    return null;
-  }
 
-  /// Remove any substrings in the given [comment] that represent in-line code
-  /// in markdown.
-  String removeInlineCodeBlocks(String comment) {
-    int index = 0;
-    while (true) {
-      int beginIndex = comment.indexOf('`', index);
-      if (beginIndex == -1) {
-        break;
-      }
-      int endIndex = comment.indexOf('`', beginIndex + 1);
-      if (endIndex == -1) {
-        break;
-      }
-      comment = comment.substring(0, beginIndex + 1) +
-          ' ' * (endIndex - beginIndex - 1) +
-          comment.substring(endIndex);
-      index = endIndex + 1;
-    }
-    return comment;
-  }
-
-  /// Parse a documentation comment. Return the documentation comment that was
-  /// parsed, or `null` if there was no comment.
-  Comment _parseDocumentationCommentOpt(Token commentToken) {
+    // Build and return the comment
+    List<CommentReference> references = parseCommentReferences(dartdoc);
     List<Token> tokens = <Token>[];
-    while (commentToken != null) {
-      if (commentToken.lexeme.startsWith('/**') ||
-          commentToken.lexeme.startsWith('///')) {
-        if (tokens.isNotEmpty) {
-          if (commentToken.type == TokenType.SINGLE_LINE_COMMENT) {
-            if (tokens[0].type != TokenType.SINGLE_LINE_COMMENT) {
-              tokens.clear();
-            }
-          } else {
-            tokens.clear();
-          }
-        }
-        tokens.add(commentToken);
-      }
-      commentToken = commentToken.next;
+    while (dartdoc != null) {
+      tokens.add(dartdoc);
+      dartdoc = dartdoc.next;
     }
-    if (tokens.isEmpty) {
-      return null;
-    }
-    int count = parser.parseCommentReferences(tokens.first);
-    // TODO(danrubel): If nulls were not added to the list, then this could
-    // be a fixed length list.
-    List<CommentReference> references = new List<CommentReference>()
-      ..length = count;
-    // popTypedList(...) returns `null` if count is zero,
-    // but ast.documentationComment(...) expects a non-null references list.
-    popTypedList(count, references);
-    // TODO(danrubel): Investigate preventing nulls from being added
-    // but for now, just remove them.
-    references.removeWhere((ref) => ref == null);
     return ast.documentationComment(tokens, references);
   }
 
+  List<CommentReference> parseCommentReferences(Token dartdoc) {
+    // Parse dartdoc into potential comment reference source/offset pairs
+    int count = parser.parseCommentReferences(dartdoc);
+    List sourcesAndOffsets = new List(count * 2);
+    popList(count * 2, sourcesAndOffsets);
+
+    // Parse each of the source/offset pairs into actual comment references
+    count = 0;
+    int index = 0;
+    while (index < sourcesAndOffsets.length) {
+      String referenceSource = sourcesAndOffsets[index++];
+      int referenceOffset = sourcesAndOffsets[index++];
+      ScannerResult result = scanString(referenceSource);
+      if (!result.hasErrors) {
+        Token token = result.tokens;
+        if (parser.parseOneCommentReference(token, referenceOffset)) {
+          ++count;
+        }
+      }
+    }
+
+    final references = new List<CommentReference>(count);
+    popTypedList(count, references);
+    return references;
+  }
+
   @override
   void debugEvent(String name) {
     // printEvent('AstBuilder: $name');

pkg/analyzer/test/generated/parser_fasta_test.dart

@@ -857,15 +857,12 @@ class ParserProxy extends analyzer.ParserAdapter {
       }
     }
     expect(tokens[tokens.length - 1].next, isNull);
-    int count = fastaParser.parseCommentReferences(tokens[0]);
-    if (count == null) {
-      return null;
+    List<CommentReference> references =
+        astBuilder.parseCommentReferences(tokens.first);
+    if (astBuilder.stack.isNotEmpty) {
+      throw 'Expected empty stack, but found:'
+          '\n  ${astBuilder.stack.values.join('\n  ')}';
     }
-    List<CommentReference> references = new List<CommentReference>(count);
-    // Since parseCommentReferences(...) returned non-null, then this method
-    // should return non-null in indicating that dartdoc comments were parsed.
-    // popTypedList(...) returns `null` if count is zero.
-    astBuilder.popTypedList(count, references);
     return references;
   }
 

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

@@ -5832,38 +5832,27 @@ class Parser {
     return before;
   }
 
-  /// Parse the comment references in a sequence of comment tokens
-  /// where [token] is the first in the sequence.
-  /// Return the number of comment references parsed
-  /// or `null` if there are no dartdoc comment tokens in the sequence.
-  int parseCommentReferences(Token token) {
-    if (token == null) {
-      return null;
-    }
-    Token singleLineDoc;
-    Token multiLineDoc;
-
-    // Find the first dartdoc token to parse
-    do {
-      String lexeme = token.lexeme;
-      if (lexeme.startsWith('/**')) {
-        singleLineDoc = null;
-        multiLineDoc = token;
-      } else if (lexeme.startsWith('///')) {
-        singleLineDoc ??= token;
-        multiLineDoc = null;
+  /// Return the first dartdoc comment token preceding the given token
+  /// or `null` if no dartdoc token is found.
+  Token findDartDoc(Token token) {
+    Token comments = token.precedingComments;
+    while (comments != null) {
+      String lexeme = comments.lexeme;
+      if (lexeme.startsWith('/**') || lexeme.startsWith('///')) {
+        break;
       }
-      token = token.next;
-    } while (token != null);
-
-    // Parse the comment references
-    if (multiLineDoc != null) {
-      return parseReferencesInMultiLineComment(multiLineDoc);
-    } else if (singleLineDoc != null) {
-      return parseReferencesInSingleLineComments(singleLineDoc);
-    } else {
-      return null;
+      comments = comments.next;
     }
+    return comments;
+  }
+
+  /// Parse the comment references in a sequence of comment tokens
+  /// where [dartdoc] (not null) is the first token in the sequence.
+  /// Return the number of comment references parsed.
+  int parseCommentReferences(Token dartdoc) {
+    return dartdoc.lexeme.startsWith('///')
+        ? parseReferencesInSingleLineComments(dartdoc)
+        : parseReferencesInMultiLineComment(dartdoc);
   }
 
   /// Parse the comment references in a multi-line comment token.
@@ -6001,10 +5990,8 @@ class Parser {
 
   /// Parse the tokens in a single comment reference and generate either a
   /// `handleCommentReference` or `handleNoCommentReference` event.
-  ///
-  /// This is typically called from the listener's
-  /// `handleCommentReferenceText` method.
-  void parseOneCommentReference(Token token) {
+  /// Return `true` if a comment reference was successfully parsed.
+  bool parseOneCommentReference(Token token, int referenceOffset) {
     Token begin = token;
     Token newKeyword = null;
     if (optional('new', token)) {
@@ -6032,15 +6019,17 @@ class Parser {
     }
     if (token.isUserDefinableOperator) {
       if (token.next.isEof) {
-        listener.handleCommentReference(newKeyword, prefix, period, token);
-        return;
+        parseOneCommentReferenceRest(
+            begin, referenceOffset, newKeyword, prefix, period, token);
+        return true;
       }
     } else {
       token = operatorKeyword ?? token;
       if (token.next.isEof) {
         if (token.isIdentifier) {
-          listener.handleCommentReference(newKeyword, prefix, period, token);
-          return;
+          parseOneCommentReferenceRest(
+              begin, referenceOffset, newKeyword, prefix, period, token);
+          return true;
         }
         Keyword keyword = token.keyword;
         if (newKeyword == null &&
@@ -6058,6 +6047,25 @@ class Parser {
       }
     }
     listener.handleNoCommentReference();
+    return false;
+  }
+
+  void parseOneCommentReferenceRest(
+      Token begin,
+      int referenceOffset,
+      Token newKeyword,
+      Token prefix,
+      Token period,
+      Token identifierOrOperator) {
+    // Adjust the token offsets to match the enclosing comment token.
+    Token token = begin;
+    do {
+      token.offset += referenceOffset;
+      token = token.next;
+    } while (!token.isEof);
+
+    listener.handleCommentReference(
+        newKeyword, prefix, period, identifierOrOperator);
   }
 
   /// Given that we have just found bracketed text within the given [comment],

pkg/front_end/lib/src/fasta/source/stack_listener.dart

@@ -32,7 +32,6 @@ enum NullValue {
   BreakTarget,
   CascadeReceiver,
   Combinators,
-  CommentReference,
   Comments,
   ConditionalUris,
   ConditionallySelectedImport,