Warnings enhancements from #1524 (#1539)

* first piece

* Finish warnings breakout

* dartfmt

* Review comment

Author: jcollins-g

bin/dartdoc.dart

@@ -174,7 +174,7 @@ main(List<String> arguments) async {
       assert(message.isNotEmpty);
 
       if (record.level < logging.Level.WARNING) {
-        if (message.endsWith('...')) {
+        if (showProgress && message.endsWith('...')) {
           // Assume there may be more progress to print, so omit the trailing
           // newline
           writingProgress = true;

lib/src/markdown_processor.dart

@@ -137,6 +137,8 @@ final RegExp isConstructor = new RegExp(r'^new[\s]+', multiLine: true);
 // Covers anything with leading digits/symbols, empty string, weird punctuation, spaces.
 final RegExp notARealDocReference = new RegExp(r'''(^[^\w]|^[\d]|[,"'/]|^$)''');
 
+final RegExp operatorPrefix = new RegExp(r'^operator[ ]*');
+
 final HtmlEscape htmlEscape = const HtmlEscape(HtmlEscapeMode.ELEMENT);
 
 final List<md.InlineSyntax> _markdown_syntaxes = [
@@ -397,8 +399,8 @@ Element _findRefElementInLibrary(String codeRef, ModelElement element,
   final Set<ModelElement> results = new Set();
 
   // This might be an operator.  Strip the operator prefix and try again.
-  if (results.isEmpty && codeRef.startsWith('operator')) {
-    String newCodeRef = codeRef.replaceFirst('operator', '');
+  if (results.isEmpty && codeRef.startsWith(operatorPrefix)) {
+    String newCodeRef = codeRef.replaceFirst(operatorPrefix, '');
     return _findRefElementInLibrary(
         newCodeRef, element, commentRefs, preferredClass);
   }
@@ -594,9 +596,13 @@ Element _findRefElementInLibrary(String codeRef, ModelElement element,
   } else if (results.length == 1) {
     result = results.first.element;
   } else {
-    element.warn(PackageWarning.ambiguousDocReference,
-        message:
-            "[$codeRef] => ${results.map((r) => "'${r.fullyQualifiedName}'").join(", ")}");
+    // Squelch ambiguous doc reference warnings for parameters, because we
+    // don't link those anyway.
+    if (!results.every((r) => r is Parameter)) {
+      element.warn(PackageWarning.ambiguousDocReference,
+          message:
+              "[$codeRef] => ${results.map((r) => "'${r.fullyQualifiedName}'").join(", ")}");
+    }
     result = results.first.element;
   }
   return result;
@@ -635,6 +641,11 @@ void _getResultsForClass(Class tryClass, String codeRefChomped,
         for (final modelElement in c.allModelElements) {
           if (!_ConsiderIfConstructor(codeRef, 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(package.findCanonicalModelElementFor(

lib/src/model.dart

@@ -85,7 +85,7 @@ int byFeatureOrdering(String a, String b) {
   return compareAsciiLowerCaseNatural(a, b);
 }
 
-final RegExp _locationSplitter = new RegExp(r"(package:|[\\/;.])");
+final RegExp locationSplitter = new RegExp(r"(package:|[\\/;.])");
 
 /// Mixin for subclasses of ModelElement representing Elements that can be
 /// inherited from one class to another.
@@ -2152,6 +2152,7 @@ ModelElement resolveMultiplyInheritedElement(
 /// ModelElement will reference itself as part of the "wrong" [Library]
 /// from the public interface perspective.
 abstract class ModelElement extends Nameable
+    with Warnable
     implements Comparable, Documentable {
   final Element _element;
   final Library _library;
@@ -2325,14 +2326,6 @@ abstract class ModelElement extends Nameable
     return library.package.libraryElementReexportedBy[this.element.library];
   }
 
-  Set<String> get locationPieces {
-    return new Set()
-      ..addAll(element.location
-          .toString()
-          .split(_locationSplitter)
-          .where((s) => s.isNotEmpty));
-  }
-
   // Use components of this element's location to return a score for library
   // location.
   ScoredCandidate scoreElementWithLibrary(Library lib) {
@@ -3318,7 +3311,7 @@ abstract class Nameable {
   String get name;
 
   Set<String> get namePieces => new Set()
-    ..addAll(name.split(_locationSplitter).where((s) => s.isNotEmpty));
+    ..addAll(name.split(locationSplitter).where((s) => s.isNotEmpty));
 }
 
 class Operator extends Method {
@@ -3379,7 +3372,7 @@ class Operator extends Method {
   String get typeName => 'operator';
 }
 
-class Package extends Nameable implements Documentable {
+class Package extends Nameable with Documentable, Warnable {
   // Library objects serving as entry points for documentation.
   final List<Library> _libraries = [];
 
@@ -3483,10 +3476,30 @@ class Package extends Nameable implements Documentable {
         extendedDebug: extendedDebug);
   }
 
+  final Set<Tuple3<Element, PackageWarning, String>> _warnAlreadySeen =
+      new Set();
   void warnOnElement(Warnable warnable, PackageWarning kind,
       {String message,
       Iterable<Locatable> referredFrom,
       Iterable<String> extendedDebug}) {
+    var newEntry = new Tuple3(warnable?.element, kind, message);
+    if (_warnAlreadySeen.contains(newEntry)) {
+      return;
+    }
+    // Warnings can cause other warnings.  Queue them up via the stack but
+    // don't allow warnings we're already working on to get in there.
+    _warnAlreadySeen.add(newEntry);
+    _warnOnElement(warnable, kind,
+        message: message,
+        referredFrom: referredFrom,
+        extendedDebug: extendedDebug);
+    _warnAlreadySeen.remove(newEntry);
+  }
+
+  void _warnOnElement(Warnable warnable, PackageWarning kind,
+      {String message,
+      Iterable<Locatable> referredFrom,
+      Iterable<String> extendedDebug}) {
     if (warnable != null) {
       // This sort of warning is only applicable to top level elements.
       if (kind == PackageWarning.ambiguousReexport) {

lib/src/warnings.dart

@@ -3,6 +3,7 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'package:analyzer/dart/element/element.dart';
+import 'package:dartdoc/src/model.dart';
 import 'package:tuple/tuple.dart';
 
 import 'config.dart';
@@ -82,10 +83,18 @@ final Map<PackageWarning, PackageWarningHelpText> packageWarningText = const {
 };
 
 /// Something that package warnings can be called on.
+/// TODO(jcollins-g): Complete object model refactoring for #1524.
 abstract class Warnable implements Locatable {
   void warn(PackageWarning warning,
       {String message, Iterable<Locatable> referredFrom});
   Warnable get enclosingElement;
+
+  Set<String> get locationPieces {
+    return new Set.from(element.location
+        .toString()
+        .split(locationSplitter)
+        .where((s) => s.isNotEmpty));
+  }
 }
 
 /// Something that can be located for warning purposes.
@@ -240,7 +249,7 @@ class PackageWarningCounter {
   int get errorCount {
     return _warningCounts.keys
         .map((w) => options.asErrors.contains(w) ? _warningCounts[w] : 0)
-        .reduce((a, b) => a + b);
+        .fold(0, (a, b) => a + b);
   }
 
   int get warningCount {
@@ -249,7 +258,7 @@ class PackageWarningCounter {
             options.asWarnings.contains(w) && !options.asErrors.contains(w)
                 ? _warningCounts[w]
                 : 0)
-        .reduce((a, b) => a + b);
+        .fold(0, (a, b) => a + b);
   }
 
   @override

pubspec.lock

@@ -350,4 +350,4 @@ packages:
     source: hosted
     version: "2.1.13"
 sdks:
-  dart: ">=1.23.0 <=2.0.0-dev.6.0"
+  dart: ">=1.23.0 <=2.0.0-dev.7.0"