Resolve non-imported symbols in comments [#1153]

lib/src/markdown_processor.dart

@@ -9,13 +9,7 @@ import 'dart:convert';
 
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/element/element.dart'
-    show
-        LibraryElement,
-        Element,
-        ConstructorElement,
-        ClassElement,
-        ParameterElement,
-        PropertyAccessorElement;
+    show LibraryElement, Element, ConstructorElement, ClassElement, ParameterElement, PropertyAccessorElement;
 import 'package:html/parser.dart' show parse;
 import 'package:markdown/markdown.dart' as md;
 
@@ -28,32 +22,31 @@ const List<String> _oneLinerSkipTags = const ["code", "pre"];
 
 final List<md.InlineSyntax> _markdown_syntaxes = [new _InlineCodeSyntax()];
 
+class MatchingLinkResult {
+  final ModelElement element;
+  final String label;
+  MatchingLinkResult(this.element, this.label);
+}
+
 // TODO: this is in the wrong place
 NodeList<CommentReference> _getCommentRefs(ModelElement modelElement) {
   if (modelElement == null) return null;
   if (modelElement.documentation == null && modelElement.canOverride()) {
     var melement = modelElement.overriddenElement;
-    if (melement != null &&
-        melement.element.computeNode() != null &&
-        melement.element.computeNode() is AnnotatedNode) {
-      var docComment = (melement.element.computeNode() as AnnotatedNode)
-          .documentationComment;
+    if (melement != null && melement.element.computeNode() != null && melement.element.computeNode() is AnnotatedNode) {
+      var docComment = (melement.element.computeNode() as AnnotatedNode).documentationComment;
       if (docComment != null) return docComment.references;
       return null;
     }
   }
   if (modelElement.element.computeNode() is AnnotatedNode) {
-    if ((modelElement.element.computeNode() as AnnotatedNode)
-            .documentationComment !=
-        null) {
-      return (modelElement.element.computeNode() as AnnotatedNode)
-          .documentationComment
-          .references;
+    final AnnotatedNode annotatedNode = modelElement.element.computeNode();
+    if (annotatedNode.documentationComment != null) {
+      return annotatedNode.documentationComment.references;
     }
   } else if (modelElement.element is LibraryElement) {
     // handle anonymous libraries
-    if (modelElement.element.computeNode() == null ||
-        modelElement.element.computeNode().parent == null) {
+    if (modelElement.element.computeNode() == null || modelElement.element.computeNode().parent == null) {
       return null;
     }
     var node = modelElement.element.computeNode().parent.parent;
@@ -67,84 +60,113 @@ NodeList<CommentReference> _getCommentRefs(ModelElement modelElement) {
 }
 
 /// Returns null if element is a parameter.
-ModelElement _getMatchingLinkElement(
-    String codeRef, ModelElement element, List<CommentReference> commentRefs,
+MatchingLinkResult _getMatchingLinkElement(String codeRef, ModelElement element, List<CommentReference> commentRefs,
     {bool isConstructor: false}) {
-  if (commentRefs == null) return null;
+  if (commentRefs == null) return new MatchingLinkResult(null, null);
 
   Element refElement;
   bool isEnum = false;
 
   for (CommentReference ref in commentRefs) {
     if (ref.identifier.name == codeRef) {
       bool isConstrElement = ref.identifier.staticElement is ConstructorElement;
-      if (isConstructor && isConstrElement ||
-          !isConstructor && !isConstrElement) {
+      if (isConstructor && isConstrElement || !isConstructor && !isConstrElement) {
         refElement = ref.identifier.staticElement;
         break;
       }
     }
   }
 
   // Did not find an element in scope
-  if (refElement == null) return null;
+  if (refElement == null) {
+    return _findRefElementInLibrary(codeRef, element, commentRefs);
+  }
 
   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.enclosingElement is ClassElement &&
-        (refElement.enclosingElement as ClassElement).isEnum) {
+    if (refElement.enclosingElement is ClassElement && (refElement.enclosingElement as ClassElement).isEnum) {
       isEnum = true;
     }
   }
 
-  if (refElement is ParameterElement) return null;
+  if (refElement is ParameterElement) return new MatchingLinkResult(null, null);
 
   // bug! this can fail to find the right library name if the element's name
   // we're looking for is the same as a name that comes in from an imported
   // library.
   //
   // Don't search through all libraries in the package, actually search
   // in the current scope.
-  Library refLibrary =
-      element.package.findLibraryFor(refElement, scopedTo: element);
+  Library refLibrary = element.package.findLibraryFor(refElement, scopedTo: element);
 
   if (refLibrary != null) {
     // Is there a way to pull this from a registry of known elements?
     // Seems like we're creating too many objects this way.
     if (isEnum) {
-      return new EnumField(refElement, refLibrary);
+      return new MatchingLinkResult(new EnumField(refElement, refLibrary), null);
     }
-    return new ModelElement.from(refElement, refLibrary);
+    return new MatchingLinkResult(new ModelElement.from(refElement, refLibrary), null);
+  }
+  return new MatchingLinkResult(null, null);
+}
+
+MatchingLinkResult _findRefElementInLibrary(String codeRef, ModelElement element, List<CommentReference> commentRefs) {
+  final Library library = element.library;
+  final Package package = library.package;
+  final Map<String, ModelElement> result = {};
+
+  for (final modelElement in package.allModelElements) {
+    if (codeRef == modelElement.fullyQualifiedName) {
+      result[modelElement.fullyQualifiedName] = modelElement;
+    }
+  }
+
+  for (final modelElement in library.allModelElements) {
+    if (codeRef == modelElement.fullyQualifiedNameWithoutLibrary) {
+      result[modelElement.fullyQualifiedName] = modelElement;
+    }
+  }
+
+  if (result.isEmpty) {
+    return new MatchingLinkResult(null, null);
+  } else if (result.length == 1) {
+    return new MatchingLinkResult(result.values.first, result.values.first.name);
+  } else {
+    // TODO: add --fatal-warning, which would make the app crash in case of ambiguous references
+    print(
+        "Ambiguous reference to [${codeRef}] in '${element.fullyQualifiedName}' (${element.sourceFileName}:${element.lineNumber}). " +
+            "We found matches to the following elements: ${result.keys.map((k) => "'${k}'").join(", ")}");
+    return new MatchingLinkResult(null, null);
   }
-  return null;
 }
 
-String _linkDocReference(String reference, ModelElement element,
-    NodeList<CommentReference> commentRefs) {
-  ModelElement linkedElement;
+String _linkDocReference(String reference, ModelElement element, NodeList<CommentReference> commentRefs) {
   // support for [new Constructor] and [new Class.namedCtr]
   var refs = reference.split(' ');
+  MatchingLinkResult result;
   if (refs.length == 2 && refs.first == 'new') {
-    linkedElement = _getMatchingLinkElement(refs[1], element, commentRefs,
-        isConstructor: true);
+    result = _getMatchingLinkElement(refs[1], element, commentRefs, isConstructor: true);
   } else {
-    linkedElement = _getMatchingLinkElement(reference, element, commentRefs);
+    result = _getMatchingLinkElement(reference, element, commentRefs);
   }
+  final ModelElement linkedElement = result.element;
+  final String label = result.label ?? reference;
   if (linkedElement != null) {
     var classContent = '';
     if (linkedElement.isDeprecated) {
       classContent = 'class="deprecated" ';
     }
     // this would be linkedElement.linkedName, but link bodies are slightly
     // different for doc references. sigh.
-    return '<a ${classContent}href="${linkedElement.href}">$reference</a>';
+    return '<a ${classContent}href="${linkedElement.href}">$label</a>';
   } else {
     if (_emitWarning) {
+      // TODO: add --fatal-warning, which would make the app crash in case of ambiguous references
       print("  warning: unresolved doc reference '$reference' (in $element)");
     }
-    return '<code>${HTML_ESCAPE.convert(reference)}</code>';
+    return '<code>${HTML_ESCAPE.convert(label)}</code>';
   }
 }
 
@@ -154,8 +176,7 @@ String _renderMarkdownToHtml(String text, [ModelElement element]) {
     return new md.Text(_linkDocReference(name, element, commentRefs));
   }
 
-  return md.markdownToHtml(text,
-      inlineSyntaxes: _markdown_syntaxes, linkResolver: _linkResolver);
+  return md.markdownToHtml(text, inlineSyntaxes: _markdown_syntaxes, linkResolver: _linkResolver);
 }
 
 class Documentation {
@@ -181,16 +202,13 @@ class Documentation {
       s.remove();
     }
     for (var pre in asHtmlDocument.querySelectorAll('pre')) {
-      if (pre.children.isNotEmpty &&
-          pre.children.length != 1 &&
-          pre.children.first.localName != 'code') {
+      if (pre.children.isNotEmpty && pre.children.length != 1 && pre.children.first.localName != 'code') {
         continue;
       }
 
       if (pre.children.isNotEmpty && pre.children.first.localName == 'code') {
         var code = pre.children.first;
-        pre.classes
-            .addAll(code.classes.where((name) => name.startsWith('language-')));
+        pre.classes.addAll(code.classes.where((name) => name.startsWith('language-')));
       }
 
       bool specifiesLanguage = pre.classes.isNotEmpty;
@@ -201,9 +219,7 @@ class Documentation {
 
     // `trim` fixes issue with line ending differences between mac and windows.
     var asHtml = asHtmlDocument.body.innerHtml?.trim();
-    var asOneLiner = asHtmlDocument.body.children.isEmpty
-        ? ''
-        : asHtmlDocument.body.children.first.innerHtml;
+    var asOneLiner = asHtmlDocument.body.children.isEmpty ? '' : asHtmlDocument.body.children.first.innerHtml;
     if (!asOneLiner.startsWith('<p>')) {
       asOneLiner = '<p>$asOneLiner</p>';
     }

lib/src/model.dart

@@ -1215,6 +1215,30 @@ class Library extends ModelElement {
       return _getPackageName(dir.parent);
     }
   }
+
+  List<ModelElement> _allModelElements;
+  List<ModelElement> get allModelElements {
+    if (_allModelElements == null) {
+      final List<ModelElement> result = [];
+      result
+        ..addAll(library.allClasses)
+        ..addAll(library.constants)
+        ..addAll(library.enums)
+        ..addAll(library.functions)
+        ..addAll(library.properties)
+        ..addAll(library.typedefs);
+
+      library.allClasses.forEach((c) {
+        result.addAll(c.allInstanceMethods);
+        result.addAll(c.allInstanceProperties);
+        result.addAll(c.allOperators);
+        result.addAll(c.staticMethods);
+        result.addAll(c.staticProperties);
+      });
+      _allModelElements = result;
+    }
+    return _allModelElements;
+  }
 }
 
 class Method extends ModelElement
@@ -1312,6 +1336,7 @@ abstract class ModelElement implements Comparable, Nameable, Documentable {
   String _linkedName;
 
   String _fullyQualifiedName;
+  String _fullyQualifiedNameWithoutLibrary;
 
   // WARNING: putting anything into the body of this seems
   // to lead to stack overflows. Need to make a registry of ModelElements
@@ -1425,6 +1450,28 @@ abstract class ModelElement implements Comparable, Nameable, Documentable {
     return (_fullyQualifiedName ??= _buildFullyQualifiedName());
   }
 
+  String get fullyQualifiedNameWithoutLibrary {
+    return (_fullyQualifiedNameWithoutLibrary ??= fullyQualifiedName.split(".").skip(1).join("."));
+  }
+
+  String get sourceFileName => element.source.fullName;
+
+  int _lineNumber;
+  bool _isLineNumberComputed = false;
+  int get lineNumber {
+    if (!_isLineNumberComputed) {
+      var node = element.computeNode();
+      if (node is Declaration && node.element != null) {
+        var element = node.element;
+        var lineNumber = lineNumberCache.lineNumber(
+            element.source.fullName, element.nameOffset);
+        _lineNumber = lineNumber + 1;
+      }
+      _isLineNumberComputed = true;
+    }
+    return _lineNumber;
+  }
+
   bool get hasAnnotations => annotations.isNotEmpty;
 
   @override
@@ -2047,6 +2094,17 @@ class Package implements Nameable, Documentable {
     }
     return new Library(e.library, this);
   }
+
+  List<ModelElement> _allModelElements;
+  List<ModelElement> get allModelElements {
+    if (_allModelElements == null) {
+      _allModelElements = [];
+      this.libraries.forEach((library) {
+        _allModelElements.addAll(library.allModelElements);
+      });
+    }
+    return _allModelElements;
+  }
 }
 
 class PackageCategory implements Comparable<PackageCategory> {
@@ -2128,6 +2186,8 @@ abstract class SourceCodeMixin {
     }
   }
 
+  int get lineNumber;
+
   Element get element;
 
   bool get hasSourceCode => config.includeSource && sourceCode.isNotEmpty;
@@ -2210,21 +2270,9 @@ abstract class SourceCodeMixin {
   }
 
   String get _crossdartUrl {
-    if (_lineNumber != null && _crossdartPath != null) {
+    if (lineNumber != null && _crossdartPath != null) {
       String url = "//www.crossdart.info/p/${_crossdartPath}.html";
-      return "${url}#line-${_lineNumber}";
-    } else {
-      return null;
-    }
-  }
-
-  int get _lineNumber {
-    var node = element.computeNode();
-    if (node is Declaration && node.element != null) {
-      var element = node.element;
-      var lineNumber = lineNumberCache.lineNumber(
-          element.source.fullName, element.nameOffset);
-      return lineNumber + 1;
+      return "${url}#line-${lineNumber}";
     } else {
       return null;
     }

test/model_test.dart

@@ -279,6 +279,14 @@ void main() {
         expect(docsAsHtml, contains('<code>doesStuff</code>'));
       });
 
+      test(
+          'link to unresolved name in the library in this package still should be linked',
+          () {
+        final Class helperClass = exLibrary.classes.firstWhere((c) => c.name == 'Helper');
+        expect(helperClass.documentationAsHtml, contains('<a href="ex/Apple-class.html">Apple</a>'));
+        expect(helperClass.documentationAsHtml, contains('<a href="ex/B-class.html">B</a>'));
+      });
+
       test(
           'link to a name of a class from an imported library that exports the name',
           () {

testing/test_package/lib/src/mylib.dart

@@ -2,6 +2,9 @@ library src.mylib;
 
 void helperFunction(String message) => print(message);
 
+/// Even unresolved references in the same library should be resolved
+/// [Apple]
+/// [ex.B]
 class Helper {
   Helper();
 

testing/test_package_docs/ex/Helper-class.html

@@ -134,6 +134,11 @@ <h5><a href="ex/ex-library.html">ex</a></h5>
 
   <div class="col-xs-12 col-sm-9 col-md-8 main-content">
 
+    <section class="desc markdown">
+      <p>Even unresolved references in the same library should be resolved
+<a href="ex/Apple-class.html">Apple</a>
+<a href="ex/B-class.html">B</a></p>
+    </section>