Centralize unparsing of comment references (#2630)

* Flatten from head

* Fix unparsing of comment references

* simplify

* Fix up comments

Author: jcollins-g

lib/src/comment_references/model_comment_reference.dart

@@ -0,0 +1,29 @@
+// Copyright (c) 2021, 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.
+//
+import 'package:analyzer/dart/ast/ast.dart';
+import 'package:analyzer/dart/element/element.dart';
+import 'package:analyzer/file_system/file_system.dart';
+import 'package:dartdoc/src/model_utils.dart';
+
+/// A stripped down analyzer AST [CommentReference] containing only that
+/// information needed for Dartdoc.  Drops link to the [CommentReference]
+/// and [ResourceProvider] after construction.
+class ModelCommentReference {
+  final String codeRef;
+  final Element staticElement;
+
+  ModelCommentReference(CommentReference ref, ResourceProvider resourceProvider)
+      : codeRef = _referenceText(ref, resourceProvider),
+        staticElement = ref.identifier.staticElement;
+
+  /// "Unparse" the code reference into the raw text associated with the
+  /// [CommentReference].
+  static String _referenceText(
+      CommentReference ref, ResourceProvider resourceProvider) {
+    var contents = getFileContentsFor(
+        (ref.root as CompilationUnit).declaredElement, resourceProvider);
+    return contents.substring(ref.offset, ref.end);
+  }
+}

lib/src/generator/templates.renderers.dart

@@ -614,12 +614,13 @@ class _Renderer_Canonicalization extends RendererBase<Canonicalization> {
                   getValue: (CT_ c) => c.commentRefs,
                   renderVariable: (CT_ c, Property<CT_> self,
                           List<String> remainingNames) =>
-                      self.renderSimpleVariable(
-                          c, remainingNames, 'List<ModelCommentReference>'),
-                  renderIterable:
+                      self.renderSimpleVariable(c, remainingNames,
+                          'Map<String, ModelCommentReference>'),
+                  isNullValue: (CT_ c) => c.commentRefs == null,
+                  renderValue:
                       (CT_ c, RendererBase<CT_> r, List<MustachioNode> ast) {
-                    return c.commentRefs.map(
-                        (e) => renderSimple(e, ast, r.template, parent: r));
+                    return renderSimple(c.commentRefs, ast, r.template,
+                        parent: r);
                   },
                 ),
                 'isCanonical': Property(
@@ -902,6 +903,19 @@ class _Renderer_Category extends RendererBase<Category> {
                         (e) => _render_Class(e, ast, r.template, parent: r));
                   },
                 ),
+                'commentRefs': Property(
+                  getValue: (CT_ c) => c.commentRefs,
+                  renderVariable: (CT_ c, Property<CT_> self,
+                          List<String> remainingNames) =>
+                      self.renderSimpleVariable(c, remainingNames,
+                          'Map<String, ModelCommentReference>'),
+                  isNullValue: (CT_ c) => c.commentRefs == null,
+                  renderValue:
+                      (CT_ c, RendererBase<CT_> r, List<MustachioNode> ast) {
+                    return renderSimple(c.commentRefs, ast, r.template,
+                        parent: r);
+                  },
+                ),
                 'config': Property(
                   getValue: (CT_ c) => c.config,
                   renderVariable: (CT_ c, Property<CT_> self,
@@ -8912,12 +8926,13 @@ class _Renderer_ModelElement extends RendererBase<ModelElement> {
                   getValue: (CT_ c) => c.commentRefs,
                   renderVariable: (CT_ c, Property<CT_> self,
                           List<String> remainingNames) =>
-                      self.renderSimpleVariable(
-                          c, remainingNames, 'List<ModelCommentReference>'),
-                  renderIterable:
+                      self.renderSimpleVariable(c, remainingNames,
+                          'Map<String, ModelCommentReference>'),
+                  isNullValue: (CT_ c) => c.commentRefs == null,
+                  renderValue:
                       (CT_ c, RendererBase<CT_> r, List<MustachioNode> ast) {
-                    return c.commentRefs.map(
-                        (e) => renderSimple(e, ast, r.template, parent: r));
+                    return renderSimple(c.commentRefs, ast, r.template,
+                        parent: r);
                   },
                 ),
                 'compilationUnitElement': Property(
@@ -10394,6 +10409,19 @@ class _Renderer_Package extends RendererBase<Package> {
                         (e) => _render_Category(e, ast, r.template, parent: r));
                   },
                 ),
+                'commentRefs': Property(
+                  getValue: (CT_ c) => c.commentRefs,
+                  renderVariable: (CT_ c, Property<CT_> self,
+                          List<String> remainingNames) =>
+                      self.renderSimpleVariable(c, remainingNames,
+                          'Map<String, ModelCommentReference>'),
+                  isNullValue: (CT_ c) => c.commentRefs == null,
+                  renderValue:
+                      (CT_ c, RendererBase<CT_> r, List<MustachioNode> ast) {
+                    return renderSimple(c.commentRefs, ast, r.template,
+                        parent: r);
+                  },
+                ),
                 'config': Property(
                   getValue: (CT_ c) => c.config,
                   renderVariable: (CT_ c, Property<CT_> self,

lib/src/markdown_processor.dart

@@ -126,10 +126,6 @@ final RegExp nonHTML =
 /// parentheses.
 final _trailingIgnorePattern = RegExp(r'(<.*>|\(.*\)|\?|!)$');
 
-@Deprecated('Public variable intended to be private; will be removed as early '
-    'as Dartdoc 1.0.0')
-RegExp get trailingIgnoreStuff => _trailingIgnorePattern;
-
 /// Things to ignore at the beginning of a doc reference.
 ///
 /// This is intended to catch various keywords that a developer may include in
@@ -139,18 +135,10 @@ RegExp get trailingIgnoreStuff => _trailingIgnorePattern;
 final _leadingIgnorePattern =
     RegExp(r'^(const|final|var)[\s]+', multiLine: true);
 
-@Deprecated('Public variable intended to be private; will be removed as early '
-    'as Dartdoc 1.0.0')
-RegExp get leadingIgnoreStuff => _leadingIgnorePattern;
-
 /// If found, this pattern may indicate a reference to a constructor.
 final _constructorIndicationPattern =
     RegExp(r'(^new[\s]+|\(\)$)', multiLine: true);
 
-@Deprecated('Public variable intended to be private; will be removed as early '
-    'as Dartdoc 1.0.0')
-RegExp get isConstructor => _constructorIndicationPattern;
-
 /// A pattern indicating that text which looks like a doc reference is not
 /// intended to be.
 ///
@@ -288,8 +276,8 @@ ModelElement _getPreferredClass(ModelElement modelElement) {
 }
 
 /// Implements _getMatchingLinkElement via [CommentReferable.referenceBy].
-MatchingLinkResult _getMatchingLinkElementCommentReferable(String codeRef,
-    Warnable warnable, List<ModelCommentReference> commentRefs) {
+MatchingLinkResult _getMatchingLinkElementCommentReferable(
+    String codeRef, Warnable warnable) {
   if (!codeRef.contains(_constructorIndicationPattern) &&
       codeRef.contains(notARealDocReference)) {
     // Don't waste our time on things we won't ever find.
@@ -304,8 +292,8 @@ MatchingLinkResult _getMatchingLinkElementCommentReferable(String codeRef,
 }
 
 /// Returns null if element is a parameter.
-MatchingLinkResult _getMatchingLinkElementLegacy(String codeRef,
-    Warnable warnable, List<ModelCommentReference> commentRefs) {
+MatchingLinkResult _getMatchingLinkElementLegacy(
+    String codeRef, Warnable warnable) {
   if (!codeRef.contains(_constructorIndicationPattern) &&
       codeRef.contains(notARealDocReference)) {
     // Don't waste our time on things we won't ever find.
@@ -322,9 +310,9 @@ MatchingLinkResult _getMatchingLinkElementLegacy(String codeRef,
           message:
               'Comment reference resolution inside extension methods is not yet implemented');
     } else {
-      refModelElement = _MarkdownCommentReference(
-              codeRef, warnable, commentRefs, preferredClass)
-          .computeReferredElement();
+      refModelElement =
+          _MarkdownCommentReference(codeRef, warnable, preferredClass)
+              .computeReferredElement();
     }
   }
 
@@ -368,27 +356,28 @@ MatchingLinkResult _getMatchingLinkElementLegacy(String codeRef,
   return MatchingLinkResult(refModelElement);
 }
 
-/// Given a set of commentRefs, return the one whose name matches the codeRef.
-Element _getRefElementFromCommentRefs(
-    List<ModelCommentReference> commentRefs, String codeRef) {
-  if (commentRefs != null) {
-    for (var ref in commentRefs) {
-      if (ref.name == codeRef) {
-        var isConstrElement = ref.staticElement is ConstructorElement;
-        // Constructors are now handled by library search.
-        if (!isConstrElement) {
-          var refElement = ref.staticElement;
-          if (refElement is PropertyAccessorElement) {
-            // yay we found an accessor that wraps a const, but we really
-            // want the top-level field itself
-            refElement = (refElement as PropertyAccessorElement).variable;
-          }
-          if (refElement is PrefixElement) {
-            // We found a prefix element, but what we really want is the library element.
-            refElement = (refElement as PrefixElement).enclosingElement;
-          }
-          return refElement;
+/// Get the element referred by the [codeRef] in analyzer.
+/// Deletes constructors and otherwise messes with the output for the rest
+/// of the heuristics.
+Element _getRefElementFromCommentRefs(Warnable element, String codeRef) {
+  if (element.commentRefs != null) {
+    var ref = element.commentRefs[codeRef];
+    // ref can be null here if the analyzer failed to recognize this as a
+    // comment reference (e.g. [Foo.operator []]).
+    if (ref != null) {
+      // Constructors are now handled by library search.
+      if (ref.staticElement is! ConstructorElement) {
+        var refElement = ref.staticElement;
+        if (refElement is PropertyAccessorElement) {
+          // yay we found an accessor that wraps a const, but we really
+          // want the top-level field itself
+          refElement = (refElement as PropertyAccessorElement).variable;
         }
+        if (refElement is PrefixElement) {
+          // We found a prefix element, but what we really want is the library element.
+          refElement = (refElement as PrefixElement).enclosingElement;
+        }
+        return refElement;
       }
     }
   }
@@ -403,9 +392,6 @@ class _MarkdownCommentReference {
   /// The element containing the code reference.
   final Warnable element;
 
-  /// A list of [ModelCommentReference]s for this element.
-  final List<ModelCommentReference> commentRefs;
-
   /// Disambiguate inheritance with this class.
   final Class preferredClass;
 
@@ -421,8 +407,7 @@ class _MarkdownCommentReference {
   /// PackageGraph associated with this element.
   PackageGraph packageGraph;
 
-  _MarkdownCommentReference(
-      this.codeRef, this.element, this.commentRefs, this.preferredClass) {
+  _MarkdownCommentReference(this.codeRef, this.element, this.preferredClass) {
     assert(element != null);
     assert(element.packageGraph.allLibrariesAdded);
 
@@ -589,7 +574,7 @@ class _MarkdownCommentReference {
       _codeRefChompedParts ??= codeRefChomped.split('.');
 
   void _reducePreferAnalyzerResolution() {
-    var refElement = _getRefElementFromCommentRefs(commentRefs, codeRef);
+    var refElement = _getRefElementFromCommentRefs(element, codeRef);
     if (results.any((me) => me.element == refElement)) {
       results.removeWhere((me) => me.element != refElement);
     }
@@ -663,26 +648,23 @@ class _MarkdownCommentReference {
   void _findWithoutLeadingIgnoreStuff() {
     if (codeRef.contains(_leadingIgnorePattern)) {
       var newCodeRef = codeRef.replaceFirst(_leadingIgnorePattern, '');
-      results.add(_MarkdownCommentReference(
-              newCodeRef, element, commentRefs, preferredClass)
+      results.add(_MarkdownCommentReference(newCodeRef, element, preferredClass)
           .computeReferredElement());
     }
   }
 
   void _findWithoutTrailingIgnoreStuff() {
     if (codeRef.contains(_trailingIgnorePattern)) {
       var newCodeRef = codeRef.replaceFirst(_trailingIgnorePattern, '');
-      results.add(_MarkdownCommentReference(
-              newCodeRef, element, commentRefs, preferredClass)
+      results.add(_MarkdownCommentReference(newCodeRef, element, preferredClass)
           .computeReferredElement());
     }
   }
 
   void _findWithoutOperatorPrefix() {
     if (codeRef.startsWith(operatorPrefix)) {
       var newCodeRef = codeRef.replaceFirst(operatorPrefix, '');
-      results.add(_MarkdownCommentReference(
-              newCodeRef, element, commentRefs, preferredClass)
+      results.add(_MarkdownCommentReference(newCodeRef, element, preferredClass)
           .computeReferredElement());
     }
   }
@@ -826,7 +808,7 @@ class _MarkdownCommentReference {
   }
 
   void _findAnalyzerReferences() {
-    var refElement = _getRefElementFromCommentRefs(commentRefs, codeRef);
+    var refElement = _getRefElementFromCommentRefs(element, codeRef);
     if (refElement == null) return;
 
     ModelElement refModelElement;
@@ -943,9 +925,8 @@ const _referenceLookupWarnings = {
   PackageWarning.referenceLookupMissingWithNew,
 };
 
-md.Node _makeLinkNode(String codeRef, Warnable warnable,
-    List<ModelCommentReference> commentRefs) {
-  var result = _getMatchingLinkElement(warnable, codeRef, commentRefs);
+md.Node _makeLinkNode(String codeRef, Warnable warnable) {
+  var result = _getMatchingLinkElement(warnable, codeRef);
   var textContent = htmlEscape.convert(codeRef);
   var linkedElement = result.modelElement;
   if (linkedElement != null) {
@@ -974,18 +955,16 @@ md.Node _makeLinkNode(String codeRef, Warnable warnable,
   return md.Element.text('code', textContent);
 }
 
-MatchingLinkResult _getMatchingLinkElement(Warnable warnable, String codeRef,
-    List<ModelCommentReference> commentRefs) {
+MatchingLinkResult _getMatchingLinkElement(Warnable warnable, String codeRef) {
   MatchingLinkResult result, resultOld, resultNew;
   // Do a comparison between result types only if the warnings for them are
   // enabled, because there's a significant performance penalty.
   var doComparison = warnable.config.packageWarningOptions.warningModes.entries
       .where((entry) => _referenceLookupWarnings.contains(entry.key))
       .any((entry) => entry.value != PackageWarningMode.ignore);
   if (doComparison) {
-    resultNew =
-        _getMatchingLinkElementCommentReferable(codeRef, warnable, commentRefs);
-    resultOld = _getMatchingLinkElementLegacy(codeRef, warnable, commentRefs);
+    resultNew = _getMatchingLinkElementCommentReferable(codeRef, warnable);
+    resultOld = _getMatchingLinkElementLegacy(codeRef, warnable);
     if (resultNew.modelElement != null) {
       markdownStats.resolvedNewLookupReferences++;
     }
@@ -996,10 +975,9 @@ MatchingLinkResult _getMatchingLinkElement(Warnable warnable, String codeRef,
     }
   } else {
     if (warnable.config.experimentalReferenceLookup) {
-      result = _getMatchingLinkElementCommentReferable(
-          codeRef, warnable, commentRefs);
+      result = _getMatchingLinkElementCommentReferable(codeRef, warnable);
     } else {
-      result = _getMatchingLinkElementLegacy(codeRef, warnable, commentRefs);
+      result = _getMatchingLinkElementLegacy(codeRef, warnable);
     }
   }
   if (doComparison) {
@@ -1084,13 +1062,12 @@ Iterable<int> findFreeHangingGenericsPositions(String string) sync* {
 }
 
 class MarkdownDocument extends md.Document {
-  factory MarkdownDocument.withElementLinkResolver(
-      Canonicalization element, List<ModelCommentReference> commentRefs) {
+  factory MarkdownDocument.withElementLinkResolver(Canonicalization element) {
     md.Node /*?*/ linkResolver(String name, [String /*?*/ _]) {
       if (name.isEmpty) {
         return null;
       }
-      return _makeLinkNode(name, element, commentRefs);
+      return _makeLinkNode(name, element);
     }
 
     return MarkdownDocument(

lib/src/model/accessor.dart

@@ -4,8 +4,8 @@
 
 import 'package:analyzer/dart/element/element.dart';
 import 'package:analyzer/src/dart/element/member.dart' show ExecutableMember;
-import 'package:dartdoc/src/model/comment_referable.dart';
 import 'package:dartdoc/src/element_type.dart';
+import 'package:dartdoc/src/model/comment_referable.dart';
 import 'package:dartdoc/src/model/model.dart';
 import 'package:dartdoc/src/render/source_code_renderer.dart';
 import 'package:dartdoc/src/utils.dart';

lib/src/model/canonicalization.dart

@@ -2,6 +2,7 @@
 // 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.
 
+import 'package:dartdoc/src/comment_references/model_comment_reference.dart';
 import 'package:dartdoc/src/model/model.dart';
 
 /// Classes extending this class have canonicalization support in Dartdoc.
@@ -10,7 +11,12 @@ abstract class Canonicalization implements Locatable, Documentable {
 
   Library get canonicalLibrary;
 
-  List<ModelCommentReference> get commentRefs => null;
+  /// A map of [ModelCommentReference.codeRef] to [ModelCommentReference].
+  /// This map deduplicates comment references as all identical reference
+  /// strings inside a single documentation comment will point to the same
+  /// place, so it should not be used to count exactly how many references
+  /// there are.
+  Map<String, ModelCommentReference> get commentRefs;
 
   /// Pieces of the location, split to remove 'package:' and slashes.
   Set<String> get locationPieces;