From 3bba95c6322ab71329d1c1551f772b1ec9dc20d4 Mon Sep 17 00:00:00 2001
From: Janice Collins <jcollins@google.com>
Date: Mon, 14 Jun 2021 13:31:31 -0700
Subject: [PATCH 1/8] Handle different name styles in constructor

---
 lib/src/model/class.dart           | 16 ++++++++++++++++
 test/end2end/model_test.dart       |  9 ++++++++-
 testing/test_package/lib/fake.dart |  2 ++
 3 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/lib/src/model/class.dart b/lib/src/model/class.dart
index 01c6bdc2a5..620851c065 100644
--- a/lib/src/model/class.dart
+++ b/lib/src/model/class.dart
@@ -605,6 +605,22 @@ class Class extends Container
   @override
   Iterable<Field> get constantFields => allFields.where((f) => f.isConst);
 
+  Map<String, CommentReferable> _referenceChildren;
+  @override
+  Map<String, CommentReferable> get referenceChildren {
+    if (_referenceChildren == null) {
+      _referenceChildren = super.referenceChildren;
+      for (var constructor in constructors) {
+        // For default constructors this is a no-op, but other constructors
+        // sometimes have a prefix attached to them in their "real name".
+        // TODO(jcollins-g): consider disallowing that, and only using
+        // the element name as here?
+        _referenceChildren[constructor.element.name] = constructor;
+      }
+    }
+    return _referenceChildren;
+  }
+
   @override
   Iterable<CommentReferable> get referenceParents => <CommentReferable>[
         ...super.referenceParents,
diff --git a/test/end2end/model_test.dart b/test/end2end/model_test.dart
index 8992d99f63..47c32818e3 100644
--- a/test/end2end/model_test.dart
+++ b/test/end2end/model_test.dart
@@ -2129,6 +2129,7 @@ void main() {
         nameWithTwoUnderscores,
         nameWithSingleUnderscore,
         theOnlyThingInTheLibrary;
+    Constructor aNonDefaultConstructor;
     Class Apple, BaseClass, baseForDocComments, ExtraSpecialList, string;
     Method doAwesomeStuff, anotherMethod;
     // ignore: unused_local_variable
@@ -2147,6 +2148,7 @@ void main() {
           .firstWhere((c) => c.name == 'String');
       baseForDocComments =
           fakeLibrary.classes.firstWhere((c) => c.name == 'BaseForDocComments');
+      aNonDefaultConstructor = baseForDocComments.constructors.firstWhere((c) => c.name == 'BaseForDocComments.aNonDefaultConstructor');
       doAwesomeStuff = baseForDocComments.instanceMethods
           .firstWhere((m) => m.name == 'doAwesomeStuff');
       anotherMethod = baseForDocComments.instanceMethods
@@ -2222,6 +2224,12 @@ void main() {
     }
 
     test('Verify basic linking inside class', () {
+      expect(bothLookup(doAwesomeStuff, 'aNonDefaultConstructor'),
+          equals(MatchingLinkResult(aNonDefaultConstructor)));
+
+      expect(bothLookup(doAwesomeStuff, 'BaseForDocComments.aNonDefaultConstructor'),
+          equals(MatchingLinkResult(aNonDefaultConstructor)));
+
       expect(bothLookup(doAwesomeStuff, 'this'),
           equals(MatchingLinkResult(baseForDocComments)));
 
@@ -2280,7 +2288,6 @@ void main() {
           equals(MatchingLinkResult(BaseClass)));
 
       // A bracket operator within this class.
-      // TODO(jcollins-g): operator lookups not yet implemented with the new lookup code.
       expect(bothLookup(doAwesomeStuff, 'operator []'),
           equals(MatchingLinkResult(bracketOperator)));
 
diff --git a/testing/test_package/lib/fake.dart b/testing/test_package/lib/fake.dart
index 5f2f33580b..cb0432faae 100644
--- a/testing/test_package/lib/fake.dart
+++ b/testing/test_package/lib/fake.dart
@@ -962,6 +962,8 @@ class BaseForDocComments {
 
 
   BaseForDocComments();
+  BaseForDocComments.aNonDefaultConstructor();
+
   factory BaseForDocComments.aFactoryFunction() => null;
 }
 

From 14d52629bf969b6402a9c26c4a0d1310693bccfa Mon Sep 17 00:00:00 2001
From: Janice Collins <jcollins@google.com>
Date: Mon, 14 Jun 2021 13:40:26 -0700
Subject: [PATCH 2/8] dartfmt

---
 test/end2end/model_test.dart | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/test/end2end/model_test.dart b/test/end2end/model_test.dart
index 47c32818e3..96ae2b39f7 100644
--- a/test/end2end/model_test.dart
+++ b/test/end2end/model_test.dart
@@ -2148,7 +2148,8 @@ void main() {
           .firstWhere((c) => c.name == 'String');
       baseForDocComments =
           fakeLibrary.classes.firstWhere((c) => c.name == 'BaseForDocComments');
-      aNonDefaultConstructor = baseForDocComments.constructors.firstWhere((c) => c.name == 'BaseForDocComments.aNonDefaultConstructor');
+      aNonDefaultConstructor = baseForDocComments.constructors.firstWhere(
+          (c) => c.name == 'BaseForDocComments.aNonDefaultConstructor');
       doAwesomeStuff = baseForDocComments.instanceMethods
           .firstWhere((m) => m.name == 'doAwesomeStuff');
       anotherMethod = baseForDocComments.instanceMethods
@@ -2227,7 +2228,9 @@ void main() {
       expect(bothLookup(doAwesomeStuff, 'aNonDefaultConstructor'),
           equals(MatchingLinkResult(aNonDefaultConstructor)));
 
-      expect(bothLookup(doAwesomeStuff, 'BaseForDocComments.aNonDefaultConstructor'),
+      expect(
+          bothLookup(
+              doAwesomeStuff, 'BaseForDocComments.aNonDefaultConstructor'),
           equals(MatchingLinkResult(aNonDefaultConstructor)));
 
       expect(bothLookup(doAwesomeStuff, 'this'),

From 877cbea97e3af38dc3660b6ecf3c869113d6f466 Mon Sep 17 00:00:00 2001
From: Janice Collins <jcollins@google.com>
Date: Mon, 14 Jun 2021 16:18:21 -0700
Subject: [PATCH 3/8] Fixes seem to work.

---
 .../model_comment_reference.dart              | 31 ++++++++++---
 lib/src/comment_references/parser.dart        | 24 +++++-----
 lib/src/markdown_processor.dart               | 44 ++++++++++++++-----
 lib/src/model/constructor.dart                | 12 ++++-
 lib/src/model/container.dart                  | 15 +++++++
 test/comment_referable/parser_test.dart       |  2 +-
 test/end2end/model_test.dart                  | 26 ++++++++++-
 testing/test_package/lib/fake.dart            |  6 ++-
 8 files changed, 126 insertions(+), 34 deletions(-)

diff --git a/lib/src/comment_references/model_comment_reference.dart b/lib/src/comment_references/model_comment_reference.dart
index 4d044da89d..aa7eb0779c 100644
--- a/lib/src/comment_references/model_comment_reference.dart
+++ b/lib/src/comment_references/model_comment_reference.dart
@@ -15,6 +15,7 @@ abstract class ModelCommentReference {
   bool get allowDefaultConstructor;
   String get codeRef;
   bool get hasConstructorHint;
+  bool get hasCallableHint;
   List<String> get referenceBy;
   Element get staticElement;
 
@@ -32,22 +33,42 @@ abstract class ModelCommentReference {
 /// information needed for Dartdoc.  Drops link to the [CommentReference]
 /// and [ResourceProvider] after construction.
 class _ModelCommentReferenceImpl implements ModelCommentReference {
+  bool _allowDefaultConstructor;
   @override
   bool get allowDefaultConstructor {
-    if (parsed.length >= 2) {
-      return parsed[parsed.length - 2] == parsed[parsed.length - 1];
+    if (_allowDefaultConstructor == null) {
+      _allowDefaultConstructor = false;
+      var foundFirst = false;
+      String checkText;
+      // Check for two identically named identifiers at the end of the
+      // parse list, skipping over any junk or other nodes.
+      for (var node in parsed.reversed.whereType<IdentifierNode>()) {
+        if (foundFirst) {
+          if (checkText == node.text) {
+            _allowDefaultConstructor = true;
+          }
+          break;
+        } else {
+          foundFirst = true;
+          checkText = node.text;
+        }
+      }
     }
-    return false;
+    return _allowDefaultConstructor;
   }
 
   @override
   final String codeRef;
 
   @override
-  bool get hasConstructorHint =>
+  bool get hasCallableHint =>
       parsed.isNotEmpty &&
       (parsed.first is ConstructorHintStartNode ||
-          parsed.last is ConstructorHintEndNode);
+          parsed.last is CallableHintEndNode);
+
+  @override
+  bool get hasConstructorHint =>
+      parsed.isNotEmpty && parsed.first is ConstructorHintStartNode;
 
   @override
   List<String> get referenceBy => parsed
diff --git a/lib/src/comment_references/parser.dart b/lib/src/comment_references/parser.dart
index 626a61a5d7..e0b2a7f742 100644
--- a/lib/src/comment_references/parser.dart
+++ b/lib/src/comment_references/parser.dart
@@ -133,7 +133,7 @@ class CommentReferenceParser {
     if (suffixResult.type == _SuffixResultType.notSuffix) {
       // Invalid trailing junk; reject the reference.
       return [];
-    } else if (suffixResult.type == _SuffixResultType.parsedConstructorHint) {
+    } else if (suffixResult.type == _SuffixResultType.parsedCallableHint) {
       children.add(suffixResult.node);
     }
 
@@ -234,10 +234,10 @@ class CommentReferenceParser {
         IdentifierNode(codeRef.substring(startIndex, _index)));
   }
 
-  static const _constructorHintSuffix = '()';
+  static const _callableHintSuffix = '()';
 
   /// ```text
-  /// <suffix> ::= <constructorPostfixHint>
+  /// <suffix> ::= <callableHintSuffix>
   ///   | <trailingJunk>
   ///
   /// <trailingJunk> ::= '<'<CHARACTER>*'>'
@@ -246,7 +246,7 @@ class CommentReferenceParser {
   ///   | '?'
   ///   | '!'
   ///
-  /// <constructorPostfixHint> ::= '()'
+  /// <callableHintSuffix> ::= '()'
   /// ```
   _SuffixParseResult _parseSuffix() {
     var startIndex = _index;
@@ -254,10 +254,10 @@ class CommentReferenceParser {
     if (_atEnd) {
       return _SuffixParseResult.missing;
     }
-    if (_tryMatchLiteral(_constructorHintSuffix)) {
+    if (_tryMatchLiteral(_callableHintSuffix)) {
       if (_atEnd) {
         return _SuffixParseResult.ok(
-            ConstructorHintEndNode(codeRef.substring(startIndex, _index)));
+            CallableHintEndNode(codeRef.substring(startIndex, _index)));
       }
       return _SuffixParseResult.notSuffix;
     }
@@ -386,10 +386,10 @@ enum _SuffixResultType {
   junk, // Found known types of junk it is OK to ignore.
   missing, // There is no suffix here.  Same as EOF as this is a suffix.
   notSuffix, // Found something, but not a valid suffix.
-  parsedConstructorHint, // Parsed a [ConstructorHintEndNode].
+  parsedCallableHint, // Parsed a [CallableHintEndNode].
 }
 
-/// The result of attempting to parse a prefix to a comment reference.
+/// The result of attempting to parse a suffix to a comment reference.
 class _SuffixParseResult {
   final _SuffixResultType type;
 
@@ -398,7 +398,7 @@ class _SuffixParseResult {
   const _SuffixParseResult._(this.type, this.node);
 
   factory _SuffixParseResult.ok(CommentReferenceNode node) =>
-      _SuffixParseResult._(_SuffixResultType.parsedConstructorHint, node);
+      _SuffixParseResult._(_SuffixResultType.parsedCallableHint, node);
 
   static const _SuffixParseResult junk =
       _SuffixParseResult._(_SuffixResultType.junk, null);
@@ -426,14 +426,14 @@ class ConstructorHintStartNode extends CommentReferenceNode {
   String toString() => 'ConstructorHintStartNode["$text"]';
 }
 
-class ConstructorHintEndNode extends CommentReferenceNode {
+class CallableHintEndNode extends CommentReferenceNode {
   @override
   final String text;
 
-  ConstructorHintEndNode(this.text);
+  CallableHintEndNode(this.text);
 
   @override
-  String toString() => 'ConstructorHintEndNode["$text"]';
+  String toString() => 'CallableHintEndNode["$text"]';
 }
 
 /// Represents an identifier.
diff --git a/lib/src/markdown_processor.dart b/lib/src/markdown_processor.dart
index 4ce5c3f030..b13118ffef 100644
--- a/lib/src/markdown_processor.dart
+++ b/lib/src/markdown_processor.dart
@@ -203,6 +203,13 @@ class MatchingLinkResult {
     if (modelElement is Parameter && other.modelElement == null) {
       return true;
     }
+    // Same with TypeParameter.
+    if (other.modelElement is TypeParameter && modelElement == null) {
+      return true;
+    }
+    if (modelElement is TypeParameter && other.modelElement == null) {
+      return true;
+    }
     return false;
   }
 
@@ -286,7 +293,12 @@ bool _rejectDefaultConstructors(CommentReferable referable) {
   return true;
 }
 
-/// Return false unless the passed [referable] is a [Constructor].
+/// Return false unless the passed [referable] represents a callable object.
+/// Allows constructors but does not require them.
+bool _requireCallable(CommentReferable referable) =>
+    referable is ModelElement && referable.isCallable;
+
+/// Return false unless the passed [referable] represents a constructor.
 bool _requireConstructor(CommentReferable referable) =>
     referable is Constructor;
 
@@ -295,17 +307,29 @@ MatchingLinkResult _getMatchingLinkElementCommentReferable(
     String codeRef, Warnable warnable) {
   var commentReference =
       warnable.commentRefs[codeRef] ?? ModelCommentReference.synthetic(codeRef);
-  var filter = commentReference.hasConstructorHint
-      ? _requireConstructor
-      : _rejectDefaultConstructors;
-
-  /// Neither reject, nor require, a default constructor in the event
-  /// the comment reference structure implies one.  (We can not require it
-  /// in case a library name is the same as a member class name and the class
-  /// is the intended lookup).
+
+  bool Function(CommentReferable) filter;
+
   if (commentReference.allowDefaultConstructor) {
-    filter = null;
+    // Neither reject, nor require, a default constructor in the event
+    // the comment reference structure implies one.  (We can not require it
+    // in case a library name is the same as a member class name and the class
+    // is the intended lookup).
+    filter = commentReference.hasCallableHint ? _requireCallable : null;
+  } else if (commentReference.hasConstructorHint &&
+      commentReference.hasCallableHint) {
+    // This takes precedence over the callable hint if both are present --
+    // pick a constructor and only constructor if we see `new`.
+    filter = _requireConstructor;
+  } else if (commentReference.hasCallableHint) {
+    // Trailing parens indicate we are looking for a callable.
+    filter = _requireCallable;
+  } else {
+    // Without hints, reject default constructors to force resolution to the
+    // class.
+    filter = _rejectDefaultConstructors;
   }
+
   var lookupResult =
       warnable.referenceBy(commentReference.referenceBy, filter: filter);
 
diff --git a/lib/src/model/constructor.dart b/lib/src/model/constructor.dart
index a572b3d699..fb35dcf2b9 100644
--- a/lib/src/model/constructor.dart
+++ b/lib/src/model/constructor.dart
@@ -127,8 +127,16 @@ class Constructor extends ModelElement
   Map<String, CommentReferable> get referenceChildren {
     if (_referenceChildren == null) {
       _referenceChildren = {};
-      _referenceChildren
-          .addEntries(allParameters.map((p) => MapEntry(p.name, p)));
+      for (var param in allParameters) {
+        var paramElement = param.element;
+        if (paramElement is FieldFormalParameterElement) {
+          var fieldFormal =
+              ModelElement.fromElement(paramElement.field, packageGraph);
+          _referenceChildren[paramElement.name] = fieldFormal;
+        } else {
+          _referenceChildren[param.name] = param;
+        }
+      }
       _referenceChildren
           .addEntries(typeParameters.map((p) => MapEntry(p.name, p)));
     }
diff --git a/lib/src/model/container.dart b/lib/src/model/container.dart
index 9f2ccc1d22..c798c60e23 100644
--- a/lib/src/model/container.dart
+++ b/lib/src/model/container.dart
@@ -262,7 +262,18 @@ abstract class Container extends ModelElement with TypeParameters {
     if (_referenceChildren == null) {
       _referenceChildren = {};
       for (var modelElement in allModelElements) {
+        // Never directly look up accessors.
         if (modelElement is Accessor) continue;
+        if (modelElement is Constructor) {
+          // Populate default constructor names so they make sense for the
+          // new lookup code.
+          var constructorName = modelElement.element.name;
+          if (constructorName == '') {
+            constructorName = name;
+          }
+          _referenceChildren[constructorName] = modelElement;
+          continue;
+        }
         if (modelElement is Operator) {
           // TODO(jcollins-g): once todo in [Operator.name] is fixed, remove
           // this special case.
@@ -270,6 +281,10 @@ abstract class Container extends ModelElement with TypeParameters {
         } else {
           _referenceChildren[modelElement.name] = modelElement;
         }
+      }
+      // Process unscoped parameters last to make sure they don't override
+      // other options.
+      for (var modelElement in allModelElements) {
         // Don't complain about references to parameter names, but prefer
         // referring to anything else.
         // TODO(jcollins-g): Figure out something good to do in the ecosystem
diff --git a/test/comment_referable/parser_test.dart b/test/comment_referable/parser_test.dart
index d29ade35cb..c6f11a5dcd 100644
--- a/test/comment_referable/parser_test.dart
+++ b/test/comment_referable/parser_test.dart
@@ -11,7 +11,7 @@ void main() {
     var result = CommentReferenceParser(codeRef).parse();
     var hasHint = result.isNotEmpty &&
         (result.first is ConstructorHintStartNode ||
-            result.last is ConstructorHintEndNode);
+            result.last is CallableHintEndNode);
     var stringParts = result.whereType<IdentifierNode>().map((i) => i.text);
     expect(stringParts, equals(parts));
     expect(hasHint, equals(constructorHint));
diff --git a/test/end2end/model_test.dart b/test/end2end/model_test.dart
index d4b9cd628f..776f0ffa6b 100644
--- a/test/end2end/model_test.dart
+++ b/test/end2end/model_test.dart
@@ -2136,13 +2136,13 @@ void main() {
         nameWithTwoUnderscores,
         nameWithSingleUnderscore,
         theOnlyThingInTheLibrary;
-    Constructor aNonDefaultConstructor;
+    Constructor aNonDefaultConstructor, defaultConstructor;
     Class Apple, BaseClass, baseForDocComments, ExtraSpecialList, string;
     Method doAwesomeStuff, anotherMethod;
     // ignore: unused_local_variable
     Operator bracketOperator, bracketOperatorOtherClass;
     Parameter doAwesomeStuffParam;
-    Field forInheriting, action;
+    Field forInheriting, action, initializeMe;
 
     setUpAll(() async {
       nameWithTwoUnderscores = fakeLibrary.constants
@@ -2157,6 +2157,10 @@ void main() {
           fakeLibrary.classes.firstWhere((c) => c.name == 'BaseForDocComments');
       aNonDefaultConstructor = baseForDocComments.constructors.firstWhere(
           (c) => c.name == 'BaseForDocComments.aNonDefaultConstructor');
+      defaultConstructor = baseForDocComments.constructors
+          .firstWhere((c) => c.name == 'BaseForDocComments');
+      initializeMe = baseForDocComments.allFields
+          .firstWhere((f) => f.name == 'initializeMe');
       doAwesomeStuff = baseForDocComments.instanceMethods
           .firstWhere((m) => m.name == 'doAwesomeStuff');
       anotherMethod = baseForDocComments.instanceMethods
@@ -2231,7 +2235,25 @@ void main() {
       return newLookupResult;
     }
 
+    test('Verify basic linking inside a constructor', () {
+      // Field formal parameters worked sometimes by accident in the old code,
+      // but should work reliably now.
+      expect(newLookup(aNonDefaultConstructor, 'initializeMe'),
+          equals(MatchingLinkResult(initializeMe)));
+      expect(newLookup(aNonDefaultConstructor, 'aNonDefaultConstructor'),
+          equals(MatchingLinkResult(aNonDefaultConstructor)));
+      expect(
+          bothLookup(aNonDefaultConstructor,
+              'BaseForDocComments.aNonDefaultConstructor'),
+          equals(MatchingLinkResult(aNonDefaultConstructor)));
+    });
+
     test('Verify basic linking inside class', () {
+      expect(
+          bothLookup(
+              baseForDocComments, 'BaseForDocComments.BaseForDocComments'),
+          equals(MatchingLinkResult(defaultConstructor)));
+
       expect(bothLookup(doAwesomeStuff, 'aNonDefaultConstructor'),
           equals(MatchingLinkResult(aNonDefaultConstructor)));
 
diff --git a/testing/test_package/lib/fake.dart b/testing/test_package/lib/fake.dart
index e183e3c61e..bdbae44f6b 100644
--- a/testing/test_package/lib/fake.dart
+++ b/testing/test_package/lib/fake.dart
@@ -960,9 +960,11 @@ class BaseForDocComments {
 
   String operator [](String key) => "${key}'s value";
 
+  final bool initializeMe;
 
-  BaseForDocComments();
-  BaseForDocComments.aNonDefaultConstructor();
+  BaseForDocComments(this.initializeMe);
+
+  BaseForDocComments.aNonDefaultConstructor(this.initializeMe);
 
   factory BaseForDocComments.aFactoryFunction() => null;
 }

From c2357c7a969241c2e82fed62524ca72ce1116f9e Mon Sep 17 00:00:00 2001
From: Janice Collins <jcollins@google.com>
Date: Mon, 14 Jun 2021 16:22:36 -0700
Subject: [PATCH 4/8] Rebuild

---
 lib/src/generator/templates.runtime_renderers.dart | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/lib/src/generator/templates.runtime_renderers.dart b/lib/src/generator/templates.runtime_renderers.dart
index 8128e08f63..a61d82e5e6 100644
--- a/lib/src/generator/templates.runtime_renderers.dart
+++ b/lib/src/generator/templates.runtime_renderers.dart
@@ -2153,6 +2153,19 @@ class _Renderer_Class extends RendererBase<Class> {
                             parent: r));
                   },
                 ),
+                'referenceChildren': Property(
+                  getValue: (CT_ c) => c.referenceChildren,
+                  renderVariable: (CT_ c, Property<CT_> self,
+                          List<String> remainingNames) =>
+                      self.renderSimpleVariable(
+                          c, remainingNames, 'Map<String, CommentReferable>'),
+                  isNullValue: (CT_ c) => c.referenceChildren == null,
+                  renderValue:
+                      (CT_ c, RendererBase<CT_> r, List<MustachioNode> ast) {
+                    return renderSimple(c.referenceChildren, ast, r.template,
+                        parent: r, getters: _invisibleGetters['Map']);
+                  },
+                ),
                 'referenceParents': Property(
                   getValue: (CT_ c) => c.referenceParents,
                   renderVariable: (CT_ c, Property<CT_> self,

From 7045d9137cedf7d593cd83ebd745fc9d439b20bb Mon Sep 17 00:00:00 2001
From: Janice Collins <jcollins@google.com>
Date: Mon, 14 Jun 2021 16:33:42 -0700
Subject: [PATCH 5/8] rebuild post-merge

---
 lib/src/generator/templates.runtime_renderers.dart | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/src/generator/templates.runtime_renderers.dart b/lib/src/generator/templates.runtime_renderers.dart
index 63e5b7d79b..e024f7adb9 100644
--- a/lib/src/generator/templates.runtime_renderers.dart
+++ b/lib/src/generator/templates.runtime_renderers.dart
@@ -2162,9 +2162,9 @@ class _Renderer_Class extends RendererBase<Class> {
                       self.renderSimpleVariable(
                           c, remainingNames, 'Map<String, CommentReferable>'),
                   isNullValue: (CT_ c) => c.referenceChildren == null,
-                  renderValue:
-                      (CT_ c, RendererBase<CT_> r, List<MustachioNode> ast) {
-                    return renderSimple(c.referenceChildren, ast, r.template,
+                  renderValue: (CT_ c, RendererBase<CT_> r,
+                      List<MustachioNode> ast, StringSink sink) {
+                    renderSimple(c.referenceChildren, ast, r.template, sink,
                         parent: r, getters: _invisibleGetters['Map']);
                   },
                 ),

From d14822283b9f46814ed8e840b7680d5857c77202 Mon Sep 17 00:00:00 2001
From: Janice Collins <jcollins@google.com>
Date: Mon, 14 Jun 2021 16:39:17 -0700
Subject: [PATCH 6/8] fix analysis error in fake

---
 testing/test_package/lib/fake.dart | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/testing/test_package/lib/fake.dart b/testing/test_package/lib/fake.dart
index bdbae44f6b..b7b3678d13 100644
--- a/testing/test_package/lib/fake.dart
+++ b/testing/test_package/lib/fake.dart
@@ -985,6 +985,8 @@ enum MacrosFromAccessors {
 /// Testing if docs for inherited method are correct.
 /// {@category NotSoExcellent}
 class SubForDocComments extends BaseForDocComments {
+  SubForDocComments(bool thing) : super(thing);
+  
   /// Reference to [foo] and [bar]
   void localMethod(String foo, bar) {}
 

From 251447ec35d3015db2b0cddc026004f131421287 Mon Sep 17 00:00:00 2001
From: Janice Collins <jcollins@google.com>
Date: Tue, 15 Jun 2021 09:07:08 -0700
Subject: [PATCH 7/8] Package globals on

---
 lib/src/model/package.dart | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/lib/src/model/package.dart b/lib/src/model/package.dart
index 4bb55857fa..f4f8fa35ec 100644
--- a/lib/src/model/package.dart
+++ b/lib/src/model/package.dart
@@ -408,6 +408,17 @@ class Package extends LibraryContainer
       _referenceChildren = {};
       _referenceChildren
           .addEntries(allLibraries.map((l) => MapEntry(l.name, l)));
+      // Do not override any preexisting data, and insert based on the
+      // public library sort order.
+      // TODO(jcollins-g): warn when results require package-global
+      // lookups like this.
+      for (var lib in publicLibrariesSorted) {
+        for (var referableEntry in lib.referenceChildren.entries) {
+          if (!_referenceChildren.containsKey(referableEntry.key)) {
+            _referenceChildren[referableEntry.key] = referableEntry.value;
+          }
+        }
+      }
     }
     return _referenceChildren;
   }

From 13ddc738230ef70b1e21ffa2ec4134f822eb2de8 Mon Sep 17 00:00:00 2001
From: Janice Collins <jcollins@google.com>
Date: Tue, 15 Jun 2021 15:41:11 -0700
Subject: [PATCH 8/8] Implement package global and graph global lookups

---
 lib/src/comment_references/parser.dart  | 21 +++++++---
 lib/src/markdown_processor.dart         | 27 ++++++++++---
 lib/src/model/library.dart              | 18 +++++++--
 lib/src/model/package.dart              |  1 +
 lib/src/model/package_graph.dart        | 51 ++++++++++++++++---------
 test/comment_referable/parser_test.dart | 34 ++++++++++++-----
 test/end2end/model_test.dart            | 32 +++++++++++++---
 testing/test_package/lib/fake.dart      |  5 ++-
 8 files changed, 139 insertions(+), 50 deletions(-)

diff --git a/lib/src/comment_references/parser.dart b/lib/src/comment_references/parser.dart
index e0b2a7f742..28f558edca 100644
--- a/lib/src/comment_references/parser.dart
+++ b/lib/src/comment_references/parser.dart
@@ -142,7 +142,6 @@ class CommentReferenceParser {
   }
 
   static const _constructorHintPrefix = 'new';
-
   static const _ignorePrefixes = ['const', 'final', 'var'];
 
   /// Implement parsing a prefix to a comment reference.
@@ -153,17 +152,19 @@ class CommentReferenceParser {
   ///
   /// <constructorPrefixHint> ::= 'new '
   ///
-  /// <leadingJunk> ::= ('const' | 'final' | 'var' | 'operator')(' '+)
+  /// <leadingJunk> ::= ('const' | 'final' | 'var')(' '+)
   /// ```
   _PrefixParseResult _parsePrefix() {
     if (_atEnd) {
       return _PrefixParseResult.endOfFile;
     }
-    if (_tryMatchLiteral(_constructorHintPrefix)) {
+    if (_tryMatchLiteral(_constructorHintPrefix,
+        requireTrailingNonidentifier: true)) {
       return _PrefixParseResult.ok(
           ConstructorHintStartNode(_constructorHintPrefix));
     }
-    if (_ignorePrefixes.any((p) => _tryMatchLiteral(p))) {
+    if (_ignorePrefixes
+        .any((p) => _tryMatchLiteral(p, requireTrailingNonidentifier: true))) {
       return _PrefixParseResult.junk;
     }
 
@@ -278,10 +279,12 @@ class CommentReferenceParser {
 
   /// Advances [_index] on match, preserves on non-match.
   bool _tryMatchLiteral(String characters,
-      {bool acceptTrailingWhitespace = true}) {
+      {bool acceptTrailingWhitespace = true,
+      bool requireTrailingNonidentifier = false}) {
     assert(acceptTrailingWhitespace != null);
     if (characters.length + _index > _referenceLength) return false;
-    for (var startIndex = _index;
+    int startIndex;
+    for (startIndex = _index;
         _index - startIndex < characters.length;
         _index++) {
       if (codeRef.codeUnitAt(_index) !=
@@ -290,6 +293,12 @@ class CommentReferenceParser {
         return false;
       }
     }
+    if (requireTrailingNonidentifier) {
+      if (_atEnd || !_nonIdentifierChars.contains(_thisChar)) {
+        _index = startIndex;
+        return false;
+      }
+    }
     if (acceptTrailingWhitespace) _walkPastWhitespace();
     return true;
   }
diff --git a/lib/src/markdown_processor.dart b/lib/src/markdown_processor.dart
index b13118ffef..adde5ea7ab 100644
--- a/lib/src/markdown_processor.dart
+++ b/lib/src/markdown_processor.dart
@@ -191,23 +191,34 @@ class MatchingLinkResult {
 
   bool isEquivalentTo(MatchingLinkResult other) {
     if (this == other) return true;
-    if (modelElement?.canonicalModelElement ==
-        other.modelElement?.canonicalModelElement) return true;
+    var compareThis = modelElement;
+    var compareOther = other.modelElement;
+
+    if (compareThis is Accessor) {
+      compareThis = (compareThis as Accessor).enclosingCombo;
+    }
+
+    if (compareOther is Accessor) {
+      compareOther = (compareOther as Accessor).enclosingCombo;
+    }
+
+    if (compareThis?.canonicalModelElement ==
+        compareOther?.canonicalModelElement) return true;
     // The old implementation just throws away Parameter matches to avoid
     // problems with warning unnecessarily at higher levels of the code.
     // I'd like to fix this at a different layer with the new lookup, so treat
     // this as equivalent to a null type.
-    if (other.modelElement is Parameter && modelElement == null) {
+    if (compareOther is Parameter && compareThis == null) {
       return true;
     }
-    if (modelElement is Parameter && other.modelElement == null) {
+    if (compareThis is Parameter && compareOther == null) {
       return true;
     }
     // Same with TypeParameter.
-    if (other.modelElement is TypeParameter && modelElement == null) {
+    if (compareOther is TypeParameter && compareThis == null) {
       return true;
     }
-    if (modelElement is TypeParameter && other.modelElement == null) {
+    if (compareThis is TypeParameter && compareOther == null) {
       return true;
     }
     return false;
@@ -290,6 +301,10 @@ bool _rejectDefaultConstructors(CommentReferable referable) {
       referable.name == referable.enclosingElement.name) {
     return false;
   }
+  // Avoid accidentally preferring arguments of the default constructor.
+  if (referable is ModelElement && referable.enclosingElement is Constructor) {
+    return false;
+  }
   return true;
 }
 
diff --git a/lib/src/model/library.dart b/lib/src/model/library.dart
index cc153022c0..69ad3de4c9 100644
--- a/lib/src/model/library.dart
+++ b/lib/src/model/library.dart
@@ -662,9 +662,21 @@ class Library extends ModelElement with Categorization, TopLevelContainer {
   Map<String, CommentReferable> _referenceChildren;
   @override
   Map<String, CommentReferable> get referenceChildren {
-    return _referenceChildren ??= Map.fromEntries(
-        element.exportNamespace.definedNames.entries.map((entry) => MapEntry(
-            entry.key, ModelElement.fromElement(entry.value, packageGraph))));
+    if (_referenceChildren == null) {
+      _referenceChildren = Map.fromEntries(
+          element.exportNamespace.definedNames.entries.map((entry) => MapEntry(
+              entry.key, ModelElement.fromElement(entry.value, packageGraph))));
+      // TODO(jcollins-g): warn and get rid of this case where it shows up.
+      // If a user is hiding parts of a prefix import, the user should not
+      // refer to hidden members via the prefix, because that can be
+      // ambiguous.  dart-lang/dartdoc#2683.
+      for (var prefixEntry in prefixToLibrary.entries) {
+        if (!_referenceChildren.containsKey(prefixEntry.key)) {
+          _referenceChildren[prefixEntry.key] = prefixEntry.value.first;
+        }
+      }
+    }
+    return _referenceChildren;
   }
 
   @override
diff --git a/lib/src/model/package.dart b/lib/src/model/package.dart
index f4f8fa35ec..1c07d4e721 100644
--- a/lib/src/model/package.dart
+++ b/lib/src/model/package.dart
@@ -414,6 +414,7 @@ class Package extends LibraryContainer
       // lookups like this.
       for (var lib in publicLibrariesSorted) {
         for (var referableEntry in lib.referenceChildren.entries) {
+          // Avoiding tearoffs for performance reasons.
           if (!_referenceChildren.containsKey(referableEntry.key)) {
             _referenceChildren[referableEntry.key] = referableEntry.value;
           }
diff --git a/lib/src/model/package_graph.dart b/lib/src/model/package_graph.dart
index fed4c27b9f..f8cadf2804 100644
--- a/lib/src/model/package_graph.dart
+++ b/lib/src/model/package_graph.dart
@@ -1030,26 +1030,39 @@ class PackageGraph with CommentReferable, Nameable {
   Map<String, CommentReferable> get referenceChildren {
     if (_referenceChildren == null) {
       _referenceChildren = {};
-
+      // Packages are the top priority.
       _referenceChildren.addEntries(packages.map((p) => MapEntry(p.name, p)));
-      // TODO(jcollins-g): deprecate and start warning for anything needing these
-      // elements.
-      var librariesWithCanonicals =
-          packages.expand((p) => referenceChildren.entries.where((e) {
-                var v = e.value;
-                return v is ModelElement
-                    ? v.canonicalModelElement != null
-                    : true;
-              }));
-      var topLevelsWithCanonicals = librariesWithCanonicals
-          .expand((p) => referenceChildren.entries.where((e) {
-                var v = e.value;
-                return v is ModelElement
-                    ? v.canonicalModelElement != null
-                    : true;
-              }));
-      _referenceChildren.addEntries(librariesWithCanonicals);
-      _referenceChildren.addEntries(topLevelsWithCanonicals);
+
+      // Libraries are next.
+      // TODO(jcollins-g): Warn about directly referencing libraries out of
+      // scope?
+      for (var p in documentedPackages) {
+        for (var lib in p.publicLibrariesSorted) {
+          if (!_referenceChildren.containsKey(lib.name)) {
+            _referenceChildren[lib.name] = lib;
+          }
+        }
+      }
+      // TODO(jcollins-g): Warn about directly referencing top level items
+      // out of scope?
+      for (var p in documentedPackages) {
+        for (var lib in p.publicLibrariesSorted) {
+          for (var me in [
+            ...lib.publicConstants,
+            ...lib.publicFunctions,
+            ...lib.publicProperties,
+            ...lib.publicTypedefs,
+            ...lib.publicExtensions,
+            ...lib.publicClasses,
+            ...lib.publicEnums,
+            ...lib.publicMixins
+          ]) {
+            if (!_referenceChildren.containsKey(me.name)) {
+              _referenceChildren[me.name] = me;
+            }
+          }
+        }
+      }
     }
     return _referenceChildren;
   }
diff --git a/test/comment_referable/parser_test.dart b/test/comment_referable/parser_test.dart
index c6f11a5dcd..eb4155be1b 100644
--- a/test/comment_referable/parser_test.dart
+++ b/test/comment_referable/parser_test.dart
@@ -7,33 +7,36 @@ import 'package:test/test.dart';
 
 void main() {
   void expectParseEquivalent(String codeRef, List<String> parts,
-      {bool constructorHint = false}) {
+      {bool constructorHint = false, bool callableHint = false}) {
     var result = CommentReferenceParser(codeRef).parse();
-    var hasHint = result.isNotEmpty &&
-        (result.first is ConstructorHintStartNode ||
-            result.last is CallableHintEndNode);
+    var hasConstructorHint =
+        result.isNotEmpty && result.first is ConstructorHintStartNode;
+    var hasCallableHint =
+        result.isNotEmpty && result.last is CallableHintEndNode;
     var stringParts = result.whereType<IdentifierNode>().map((i) => i.text);
     expect(stringParts, equals(parts));
-    expect(hasHint, equals(constructorHint));
+    expect(hasConstructorHint, equals(constructorHint));
+    expect(hasCallableHint, equals(callableHint));
   }
 
   void expectParsePassthrough(String codeRef) =>
       expectParseEquivalent(codeRef, [codeRef]);
 
   void expectParseError(String codeRef) {
-    expect(CommentReferenceParser(codeRef).parse(), isEmpty);
+    expect(CommentReferenceParser(codeRef).parse().whereType<IdentifierNode>(),
+        isEmpty);
   }
 
   group('Basic comment reference parsing', () {
     test('Check that basic references parse', () {
       expectParseEquivalent('valid', ['valid']);
       expectParseEquivalent('new valid', ['valid'], constructorHint: true);
-      expectParseEquivalent('valid()', ['valid'], constructorHint: true);
-      expectParseEquivalent('const valid()', ['valid'], constructorHint: true);
+      expectParseEquivalent('valid()', ['valid'], callableHint: true);
+      expectParseEquivalent('const valid()', ['valid'], callableHint: true);
       expectParseEquivalent('final valid', ['valid']);
       expectParseEquivalent('this.is.valid', ['this', 'is', 'valid']);
       expectParseEquivalent('this.is.valid()', ['this', 'is', 'valid'],
-          constructorHint: true);
+          callableHint: true);
       expectParseEquivalent('new this.is.valid', ['this', 'is', 'valid'],
           constructorHint: true);
       expectParseEquivalent('const this.is.valid', ['this', 'is', 'valid']);
@@ -46,6 +49,19 @@ void main() {
       expectParseEquivalent('this.is.valid(things)', ['this', 'is', 'valid']);
     });
 
+    test('Check that cases dependent on prefix resolution not firing parse',
+        () {
+      expectParsePassthrough('constant');
+      expectParsePassthrough('newThingy');
+      expectParsePassthrough('operatorThingy');
+      expectParseEquivalent('operator+', ['+']);
+      expectParseError('const()');
+      // TODO(jcollins-g): might need to revisit these two with constructor
+      // tearoffs?
+      expectParsePassthrough('new');
+      expectParseError('new()');
+    });
+
     test('Check that operator references parse', () {
       expectParsePassthrough('[]');
       expectParsePassthrough('<=');
diff --git a/test/end2end/model_test.dart b/test/end2end/model_test.dart
index 776f0ffa6b..57b2db59a7 100644
--- a/test/end2end/model_test.dart
+++ b/test/end2end/model_test.dart
@@ -2137,12 +2137,17 @@ void main() {
         nameWithSingleUnderscore,
         theOnlyThingInTheLibrary;
     Constructor aNonDefaultConstructor, defaultConstructor;
-    Class Apple, BaseClass, baseForDocComments, ExtraSpecialList, string;
+    Class Apple,
+        BaseClass,
+        baseForDocComments,
+        ExtraSpecialList,
+        string,
+        metaUseResult;
     Method doAwesomeStuff, anotherMethod;
     // ignore: unused_local_variable
     Operator bracketOperator, bracketOperatorOtherClass;
     Parameter doAwesomeStuffParam;
-    Field forInheriting, action, initializeMe;
+    Field forInheriting, action, initializeMe, somethingShadowy;
 
     setUpAll(() async {
       nameWithTwoUnderscores = fakeLibrary.constants
@@ -2153,6 +2158,10 @@ void main() {
           .firstWhere((e) => e.name == 'dart:core')
           .allClasses
           .firstWhere((c) => c.name == 'String');
+      metaUseResult = packageGraph.allLibraries.values
+          .firstWhere((e) => e.name == 'meta')
+          .allClasses
+          .firstWhere((c) => c.name == 'UseResult');
       baseForDocComments =
           fakeLibrary.classes.firstWhere((c) => c.name == 'BaseForDocComments');
       aNonDefaultConstructor = baseForDocComments.constructors.firstWhere(
@@ -2161,6 +2170,8 @@ void main() {
           .firstWhere((c) => c.name == 'BaseForDocComments');
       initializeMe = baseForDocComments.allFields
           .firstWhere((f) => f.name == 'initializeMe');
+      somethingShadowy = baseForDocComments.allFields
+          .firstWhere((f) => f.name == 'somethingShadowy');
       doAwesomeStuff = baseForDocComments.instanceMethods
           .firstWhere((m) => m.name == 'doAwesomeStuff');
       anotherMethod = baseForDocComments.instanceMethods
@@ -2248,12 +2259,23 @@ void main() {
           equals(MatchingLinkResult(aNonDefaultConstructor)));
     });
 
+    test('Deprecated lookup styles still function', () {
+      // dart-lang/dartdoc#2683
+      expect(bothLookup(baseForDocComments, 'aPrefix.UseResult'),
+          equals(MatchingLinkResult(metaUseResult)));
+    });
+
     test('Verify basic linking inside class', () {
       expect(
           bothLookup(
               baseForDocComments, 'BaseForDocComments.BaseForDocComments'),
           equals(MatchingLinkResult(defaultConstructor)));
 
+      // We don't want the parameter on the default constructor, here.
+      expect(
+          bothLookup(baseForDocComments, 'BaseForDocComments.somethingShadowy'),
+          equals(MatchingLinkResult(somethingShadowy)));
+
       expect(bothLookup(doAwesomeStuff, 'aNonDefaultConstructor'),
           equals(MatchingLinkResult(aNonDefaultConstructor)));
 
@@ -2311,8 +2333,7 @@ void main() {
           equals(MatchingLinkResult(theOnlyThingInTheLibrary)));
 
       // A name that exists in this package but is not imported.
-      // TODO(jcollins-g): package-wide lookups are not yet implemented with the new lookup code.
-      expect(originalLookup(doAwesomeStuff, 'doesStuff'),
+      expect(bothLookup(doAwesomeStuff, 'doesStuff'),
           equals(MatchingLinkResult(doesStuff)));
 
       // A name of a class from an import of a library that exported that name.
@@ -2339,8 +2360,7 @@ void main() {
           equals(MatchingLinkResult(forInheriting)));
 
       // Reference to an inherited member in another library via class name.
-      // TODO(jcollins-g): reference to non-imported symbols isn't implemented yet in new lookup.
-      expect(originalLookup(doAwesomeStuff, 'ExtendedBaseReexported.action'),
+      expect(bothLookup(doAwesomeStuff, 'ExtendedBaseReexported.action'),
           equals(MatchingLinkResult(action)));
     });
   });
diff --git a/testing/test_package/lib/fake.dart b/testing/test_package/lib/fake.dart
index b7b3678d13..ed907f458d 100644
--- a/testing/test_package/lib/fake.dart
+++ b/testing/test_package/lib/fake.dart
@@ -55,6 +55,7 @@ import 'dart:collection';
 // ignore: uri_does_not_exist
 //import 'dart:json' as invalidPrefix;
 import 'package:meta/meta.dart' show Required;
+import 'package:meta/meta.dart' as aPrefix show Immutable;
 import 'csspub.dart' as css;
 import 'csspub.dart' as renamedLib2;
 import 'package:test_package/src//import_unusual.dart';
@@ -962,7 +963,9 @@ class BaseForDocComments {
 
   final bool initializeMe;
 
-  BaseForDocComments(this.initializeMe);
+  int somethingShadowy;
+
+  BaseForDocComments(this.initializeMe, [bool somethingShadowy]);
 
   BaseForDocComments.aNonDefaultConstructor(this.initializeMe);