Fix function parameter references and empty href handling (#2709)

* It 'works' but no tests for new functionality yet

* dartfmt

* add tests for type parameters, handle more constructor parameter edge cases

* dartfmt rebuild

* remove debugging

* This was never correct, delete hrefs for type parameters

* Correct a problem with inherited docs

* Remember, we need both

* Correct remaining SDK issues

* Remove commented out code block

Author: jcollins-g

lib/dartdoc.dart

@@ -377,7 +377,7 @@ class Dartdoc {
     var links = doc.querySelectorAll('a');
     var stringLinks = links
         .map((link) => link.attributes['href'])
-        .where((href) => href != null)
+        .where((href) => href != null && href != '')
         .toList();
 
     return Tuple2(stringLinks, baseHref);

lib/src/model/comment_referable.dart

@@ -37,16 +37,9 @@ extension on Scope {
 /// [CommentReferable.referenceChildren] out of collections of other
 /// [CommmentReferable]s.
 extension CommentReferableEntryGenerators on Iterable<CommentReferable> {
-  /// Discards all entries that do not collide with [referable],
-  /// and generates an [MapEntry] using [referable]'s
-  /// [CommentReferable.referenceName] as a prefix for the item that does.
-  Iterable<MapEntry<String, CommentReferable>> onlyExplicitOnCollisionWith(
-          CommentReferable referable) =>
-      where((r) => r.referenceName == referable.referenceName).map(
-          (r) => MapEntry('${referable.referenceName}.${r.referenceName}', r));
-
-  /// Like [onlyExplicitOnCollisionWith], except does not discard non-conflicting
-  /// items.
+  /// Creates ordinary references except if there is a conflict with
+  /// [referable], it will generate a [MapEntry] using [referable]'s
+  /// [CommentReferable.referenceName] as a prefix for the conflicting item.
   Iterable<MapEntry<String, CommentReferable>> explicitOnCollisionWith(
           CommentReferable referable) =>
       map((r) {

lib/src/model/getter_setter_combo.dart

@@ -244,12 +244,7 @@ mixin GetterSetterCombo on ModelElement {
     if (_referenceChildren == null) {
       _referenceChildren = {};
       if (hasParameters) {
-        // Parameters in combos rarely matter for anything and are not part
-        // of the usual interface to a combo, so only reference them as part of
-        // [Container] fallbacks or if someone wants to explicitly specify a
-        // colliding parameter name.
-        _referenceChildren
-            .addEntries(parameters.onlyExplicitOnCollisionWith(this));
+        _referenceChildren.addEntries(parameters.explicitOnCollisionWith(this));
       }
       _referenceChildren
           .addEntries(modelType.typeArguments.explicitOnCollisionWith(this));

lib/src/model/parameter.dart

@@ -13,7 +13,6 @@ class Parameter extends ModelElement implements EnclosedElement {
       ParameterElement element, Library library, PackageGraph packageGraph,
       {ParameterMember originalMember})
       : super(element, library, packageGraph, originalMember);
-
   String get defaultValue {
     if (!hasDefaultValue) return null;
     return element.defaultValueCode;
@@ -85,17 +84,16 @@ class Parameter extends ModelElement implements EnclosedElement {
   Map<String, CommentReferable> get referenceChildren {
     if (_referenceChildren == null) {
       _referenceChildren = {};
-      if (isCallable) {
-        _referenceChildren
-            .addEntriesIfAbsent(parameters.explicitOnCollisionWith(this));
+      var _modelType = modelType;
+      if (_modelType is Callable) {
+        _referenceChildren.addEntriesIfAbsent(
+            _modelType.parameters.explicitOnCollisionWith(this));
       }
       _referenceChildren.addEntriesIfAbsent(
           modelType.typeArguments.explicitOnCollisionWith(this));
-      if (modelType is Callable) {
-        _referenceChildren.addEntriesIfAbsent((modelType as Callable)
-            .returnType
-            .typeArguments
-            .explicitOnCollisionWith(this));
+      if (_modelType is Callable) {
+        _referenceChildren.addEntriesIfAbsent(
+            _modelType.returnType.typeArguments.explicitOnCollisionWith(this));
       }
     }
     return _referenceChildren;

test/end2end/model_test.dart

@@ -2363,7 +2363,8 @@ void main() {
           function1,
           topLevelFunction,
           aFunctionUsingRenamedLib;
-      TopLevelVariable incorrectDocReference,
+      TopLevelVariable aSetterWithFunctionParameter,
+          incorrectDocReference,
           incorrectDocReferenceFromEx,
           nameWithTwoUnderscores,
           nameWithSingleUnderscore,
@@ -2392,6 +2393,7 @@ void main() {
           redHerring,
           yetAnotherName,
           somethingShadowyParameter;
+      Parameter fParam, fParamA, fParamB, fParamC;
       Field forInheriting,
           action,
           initializeMe,
@@ -2515,9 +2517,32 @@ void main() {
             .firstWhere((m) => m.name == 'aMethod');
         yetAnotherName =
             aMethod.allParameters.firstWhere((p) => p.name == 'yetAnotherName');
+
+        aSetterWithFunctionParameter = fakeLibrary.properties
+            .firstWhere((p) => p.name == 'aSetterWithFunctionParameter');
+        fParam = aSetterWithFunctionParameter.parameters
+            .firstWhere((p) => p.name == 'fParam');
+        fParamA = (fParam.modelType as Callable)
+            .parameters
+            .firstWhere((p) => p.name == 'fParamA');
+        fParamB = (fParam.modelType as Callable)
+            .parameters
+            .firstWhere((p) => p.name == 'fParamB');
+        fParamC = (fParam.modelType as Callable)
+            .parameters
+            .firstWhere((p) => p.name == 'fParamC');
       });
 
       group('Parameter references work properly', () {
+        test('via a setter with a function parameter', () {
+          expect(bothLookup(aSetterWithFunctionParameter, 'fParam.fParamA'),
+              equals(MatchingLinkResult(fParamA)));
+          expect(bothLookup(aSetterWithFunctionParameter, 'fParam.fParamB'),
+              equals(MatchingLinkResult(fParamB)));
+          expect(bothLookup(aSetterWithFunctionParameter, 'fParam.fParamC'),
+              equals(MatchingLinkResult(fParamC)));
+        });
+
         test('in class scope overridden by fields', () {
           expect(bothLookup(FactoryConstructorThings, 'aName'),
               equals(MatchingLinkResult(aNameField)));

testing/test_package/lib/fake.dart

@@ -1306,4 +1306,8 @@ class TypeParameterThingsExtendedQ extends TypeParameterThings<String, FactoryCo
   @override
   /// I override documentation so I can reference [QTypeParam].
   FactoryConstructorThings aMethod<QTypeParam>(String aParam, QTypeParam anotherParam) => null;
-}
+}
+
+/// We should still be able to reference [fParam.fParamA], [fParam.fParamB],
+/// and [fParam.fParamC].
+void set aSetterWithFunctionParameter(bool fParam(int fParamA, String fParamB, String fParamC)) {}