Breakdown comment reference handling further and tweak performance (#1863)

* Add option for asyncStackTraces

* Added lookup helper, but doesn't seem to actually help much

* More breakdown of the comment reference handling

* dartfmt

Author: jcollins-g

bin/dartdoc.dart

@@ -18,12 +18,16 @@ class DartdocProgramOptionContext extends DartdocGeneratorOptionContext
   DartdocProgramOptionContext(DartdocOptionSet optionSet, Directory dir)
       : super(optionSet, dir);
 
+  bool get asyncStackTraces => optionSet['asyncStackTraces'].valueAt(context);
   bool get help => optionSet['help'].valueAt(context);
   bool get version => optionSet['version'].valueAt(context);
 }
 
 Future<List<DartdocOption>> createDartdocProgramOptions() async {
   return <DartdocOption>[
+    new DartdocOptionArgOnly<bool>('asyncStackTraces', false,
+        help: 'Display coordinated asynchronous stack traces (slow)',
+        negatable: true),
     new DartdocOptionArgOnly<bool>('help', false,
         abbr: 'h', help: 'Show command help.', negatable: false),
     new DartdocOptionArgOnly<bool>('version', false,
@@ -88,7 +92,7 @@ void main(List<String> arguments) async {
         stderr.writeln('\nGeneration failed: ${e}\n${chain.terse}');
         exit(255);
       }
-    });
+    }, when: config.asyncStackTraces);
   } finally {
     // Clear out any cached tool snapshots.
     SnapshotCache.instance.dispose();

lib/src/io_utils.dart

@@ -6,7 +6,6 @@
 library dartdoc.io_utils;
 
 import 'dart:async';
-import 'dart:collection';
 import 'dart:convert';
 import 'dart:io';
 
@@ -89,27 +88,28 @@ class MultiFutureTracker<T> {
   /// Approximate maximum number of simultaneous active Futures.
   final int parallel;
 
-  final Queue<Future<T>> _queue = new Queue();
+  final Set<Future<T>> _trackedFutures = new Set();
 
   MultiFutureTracker(this.parallel);
 
   /// Wait until fewer or equal to this many Futures are outstanding.
   Future<void> _waitUntil(int max) async {
-    while (_queue.length > max) {
-      await Future.any(_queue);
+    while (_trackedFutures.length > max) {
+      await Future.any(_trackedFutures);
     }
   }
 
   /// Generates a [Future] from the given closure and adds it to the queue,
-  /// once the queue is sufficiently empty.
+  /// once the queue is sufficiently empty.  The returned future completes
+  /// when the generated [Future] has been added to the queue.
   Future<void> addFutureFromClosure(Future<T> Function() closure) async {
-    while (_queue.length > parallel - 1) {
-      await Future.any(_queue);
+    while (_trackedFutures.length > parallel - 1) {
+      await Future.any(_trackedFutures);
     }
     Future future = closure();
-    _queue.add(future);
+    _trackedFutures.add(future);
     // ignore: unawaited_futures
-    future.then((f) => _queue.remove(future));
+    future.then((f) => _trackedFutures.remove(future));
   }
 
   /// Wait until all futures added so far have completed.

lib/src/markdown_processor.dart

@@ -15,6 +15,7 @@ import 'package:dartdoc/src/tuple.dart';
 import 'package:dartdoc/src/warnings.dart';
 import 'package:html/parser.dart' show parse;
 import 'package:markdown/markdown.dart' as md;
+import 'package:quiver/iterables.dart' as quiverIterables;
 
 const validHtmlTags = const [
   "a",
@@ -547,8 +548,7 @@ class _MarkdownCommentReference {
         if (codeRefChomped == modelElement.fullyQualifiedNameWithoutLibrary ||
             (modelElement is Library &&
                 codeRefChomped == modelElement.fullyQualifiedName)) {
-          results.add(
-              packageGraph.findCanonicalModelElementFor(modelElement.element));
+          _addCanonicalResult(modelElement, null);
         }
       }
     }
@@ -559,9 +559,7 @@ class _MarkdownCommentReference {
     if (library.modelElementsNameMap.containsKey(codeRefChomped)) {
       for (final modelElement in library.modelElementsNameMap[codeRefChomped]) {
         if (!_ConsiderIfConstructor(modelElement)) continue;
-        results.add(packageGraph.findCanonicalModelElementFor(
-            modelElement.element,
-            preferredClass: preferredClass));
+        _addCanonicalResult(modelElement, preferredClass);
       }
     }
   }
@@ -579,11 +577,11 @@ class _MarkdownCommentReference {
         // might make no sense.  Instead, use the enclosing class from the
         // element in [packageGraph.findRefElementCache], because that element's
         // enclosing class will be preferred from [codeRefChomped]'s perspective.
-        results.add(packageGraph.findCanonicalModelElementFor(
-            modelElement.element,
-            preferredClass: modelElement.enclosingElement is Class
+        _addCanonicalResult(
+            modelElement,
+            modelElement.enclosingElement is Class
                 ? modelElement.enclosingElement
-                : null));
+                : null);
       }
     }
   }
@@ -639,6 +637,12 @@ class _MarkdownCommentReference {
     }
   }
 
+  // Add a result, but make it canonical.
+  void _addCanonicalResult(ModelElement modelElement, Class tryClass) {
+    results.add(packageGraph.findCanonicalModelElementFor(modelElement.element,
+        preferredClass: tryClass));
+  }
+
   /// _getResultsForClass assumes codeRefChomped might be a member of tryClass (inherited or not)
   /// and will add to [results]
   void _getResultsForClass(Class tryClass) {
@@ -653,8 +657,7 @@ class _MarkdownCommentReference {
     } else {
       // People like to use 'this' in docrefs too.
       if (codeRef == 'this') {
-        results
-            .add(packageGraph.findCanonicalModelElementFor(tryClass.element));
+        _addCanonicalResult(tryClass, null);
       } else {
         // TODO(jcollins-g): get rid of reimplementation of identifier resolution
         //                   or integrate into ModelElement in a simpler way.
@@ -666,62 +669,42 @@ class _MarkdownCommentReference {
         //                   Fortunately superChains are short, but optimize this if it matters.
         superChain.addAll(tryClass.superChain.map((t) => t.element as Class));
         for (final c in superChain) {
-          // TODO(jcollins-g): add a hash-map-enabled lookup function to Class?
-          for (final modelElement in c.allModelElements) {
-            if (!_ConsiderIfConstructor(modelElement)) continue;
-            String namePart = modelElement.fullyQualifiedName.split('.').last;
-            if (modelElement is Accessor) {
-              // TODO(jcollins-g): Individual classes should be responsible for
-              // this name comparison munging.
-              namePart = namePart.split('=').first;
-            }
-            // TODO(jcollins-g): fix operators so we can use 'name' here or similar.
-            if (codeRefChomped == namePart) {
-              results.add(packageGraph.findCanonicalModelElementFor(
-                  modelElement.element,
-                  preferredClass: tryClass));
-              continue;
-            }
-            // Handle non-documented class documentation being imported into a
-            // documented class when it refers to itself (with help from caller's
-            // iteration on tryClasses).
-            // TODO(jcollins-g): Fix partial qualifications in _findRefElementInLibrary so it can tell
-            // when it is referenced from a non-documented element?
-            // TODO(jcollins-g): We could probably check this early.
-            if (codeRefChompedParts.first == c.name &&
-                codeRefChompedParts.last == namePart) {
-              results.add(packageGraph.findCanonicalModelElementFor(
-                  modelElement.element,
-                  preferredClass: tryClass));
-              continue;
-            }
-            if (modelElement is Constructor) {
-              // Constructor names don't include the class, so we might miss them in the above search.
-              if (codeRefChompedParts.length > 1) {
-                String codeRefClass =
-                    codeRefChompedParts[codeRefChompedParts.length - 2];
-                String codeRefConstructor = codeRefChompedParts.last;
-                if (codeRefClass == c.name &&
-                    codeRefConstructor ==
-                        modelElement.fullyQualifiedName.split('.').last) {
-                  results.add(packageGraph.findCanonicalModelElementFor(
-                      modelElement.element,
-                      preferredClass: tryClass));
-                  continue;
-                }
-              }
-            }
-          }
-          results.remove(null);
+          _getResultsForSuperChainElement(c, tryClass);
           if (results.isNotEmpty) break;
-          if (c.fullyQualifiedNameWithoutLibrary == codeRefChomped) {
-            results.add(c);
-            break;
-          }
         }
       }
     }
   }
+
+  /// Get any possible results for this class in the superChain.   Returns
+  /// true if we found something.
+  void _getResultsForSuperChainElement(Class c, Class tryClass) {
+    Iterable<ModelElement> membersToCheck;
+    membersToCheck = (c.allModelElementsByNamePart[codeRefChomped] ?? [])
+        .where((m) => _ConsiderIfConstructor(m));
+    for (final ModelElement modelElement in membersToCheck) {
+      // [thing], a member of this class
+      _addCanonicalResult(modelElement, tryClass);
+    }
+    membersToCheck = (c.allModelElementsByNamePart[codeRefChompedParts.last] ??
+            <ModelElement>[])
+        .where((m) => _ConsiderIfConstructor(m));
+    if (codeRefChompedParts.first == c.name) {
+      // [Foo...thing], a member of this class (possibly a parameter).
+      membersToCheck.map((m) => _addCanonicalResult(m, tryClass));
+    } else if (codeRefChompedParts.length > 1 &&
+        codeRefChompedParts[codeRefChompedParts.length - 2] == c.name) {
+      // [....Foo.thing], a member of this class partially specified.
+      membersToCheck
+          .whereType<Constructor>()
+          .map((m) => _addCanonicalResult(m, tryClass));
+    }
+    results.remove(null);
+    if (results.isNotEmpty) return;
+    if (c.fullyQualifiedNameWithoutLibrary == codeRefChomped) {
+      results.add(c);
+    }
+  }
 }
 
 String _linkDocReference(String codeRef, Warnable warnable,

lib/src/model.dart

@@ -531,6 +531,14 @@ class Accessor extends ModelElement implements EnclosedElement {
   @override
   String get kind => 'accessor';
 
+  @override
+  String get namePart {
+    if (_namePart == null) {
+      _namePart = super.namePart.split('=').first;
+    }
+    return _namePart;
+  }
+
   PropertyAccessorElement get _accessor => (element as PropertyAccessorElement);
 }
 
@@ -684,6 +692,21 @@ class Class extends ModelElement
     return _allElements;
   }
 
+  Map<String, List<ModelElement>> _allModelElementsByNamePart;
+
+  /// Helper for [_MarkdownCommentReference._getResultsForClass].
+  Map<String, List<ModelElement>> get allModelElementsByNamePart {
+    if (_allModelElementsByNamePart == null) {
+      _allModelElementsByNamePart = {};
+      for (ModelElement me in allModelElements) {
+        _allModelElementsByNamePart.update(
+            me.namePart, (List<ModelElement> v) => v..add(me),
+            ifAbsent: () => <ModelElement>[me]);
+      }
+    }
+    return _allModelElementsByNamePart;
+  }
+
   /// This class might be canonical for elements it does not contain.
   /// See [Inheritable.canonicalEnclosingElement].
   bool contains(Element element) => allElements.containsKey(element);
@@ -4621,6 +4644,7 @@ class ModelFunctionTyped extends ModelElement
 /// Something that has a name.
 abstract class Nameable {
   String get name;
+  String get fullyQualifiedName => name;
 
   Set<String> _namePieces;
   Set<String> get namePieces {
@@ -4630,11 +4654,22 @@ abstract class Nameable {
     }
     return _namePieces;
   }
+
+  String _namePart;
+
+  /// Utility getter/cache for [_MarkdownCommentReference._getResultsForClass].
+  String get namePart {
+    // TODO(jcollins-g): This should really be the same as 'name', but isn't
+    // because of accessors and operators.
+    if (_namePart == null) {
+      _namePart = fullyQualifiedName.split('.').last;
+    }
+    return _namePart;
+  }
 }
 
 /// Something able to be indexed.
 abstract class Indexable implements Nameable {
-  String get fullyQualifiedName => name;
   String get href;
   String get kind;
   int get overriddenDepth => 0;
@@ -6568,8 +6603,6 @@ class PackageBuilder {
       PerformanceLog log = new PerformanceLog(null);
       AnalysisDriverScheduler scheduler = new AnalysisDriverScheduler(log);
       AnalysisOptionsImpl options = new AnalysisOptionsImpl();
-      options.enableSuperMixins = true;
-      options.previewDart2 = true;
 
       // TODO(jcollins-g): Make use of currently not existing API for managing
       //                   many AnalysisDrivers