Fix regression in canonicalization from extension methods (#2179)

* Return the canonical element we bothered to spend time looking for.

* Fix enums too

* Add test for canonicalization edge case

Author: jcollins-g

lib/src/model/container_member.dart

@@ -6,16 +6,6 @@ import 'package:dartdoc/src/model/model.dart';
 
 /// A [ModelElement] that is a [Container] member.
 mixin ContainerMember on ModelElement implements EnclosedElement {
-  /// True if this [ContainerMember] is inherited from a different class.
-  bool get isInherited;
-
-  /// True if this [ContainerMember] is overriding a superclass.
-  bool get isOverride;
-
-  /// True if this [ContainerMember] has a parameter whose type is overridden
-  /// by a subtype.
-  bool get isCovariant;
-
   /// True if this [ContainerMember] is from an applicable [Extension].
   /// False otherwise, including if this [ContainerMember]'s [enclosingElement]
   /// is the extension it was declared in.
@@ -39,9 +29,6 @@ mixin ContainerMember on ModelElement implements EnclosedElement {
   @override
   Set<String> get features {
     Set<String> _features = super.features;
-    if (isOverride) _features.add('override');
-    if (isInherited) _features.add('inherited');
-    if (isCovariant) _features.add('covariant');
     if (isExtended) _features.add('extended');
     return _features;
   }
@@ -61,11 +48,14 @@ mixin ContainerMember on ModelElement implements EnclosedElement {
 
   Container computeCanonicalEnclosingContainer() {
     // TODO(jcollins-g): move Extension specific code to [Extendable]
-    if (enclosingElement is! Extension ||
-        (enclosingElement is Extension && enclosingElement.isDocumented)) {
+    if (enclosingElement is Extension && enclosingElement.isDocumented) {
       return packageGraph
           .findCanonicalModelElementFor(enclosingElement.element);
     }
+    if (enclosingElement is! Extension) {
+      return packageGraph
+          .findCanonicalModelElementFor(element.enclosingElement);
+    }
     return null;
   }
 }

lib/src/model/inheritable.dart

@@ -21,8 +21,28 @@ import 'package:dartdoc/src/special_elements.dart';
 /// place in the documentation, and we pick a canonical class because that's
 /// the one in the public namespace that will be documented.
 mixin Inheritable on ContainerMember {
+  /// True if this [Inheritable] is inherited from a different class.
+  bool get isInherited;
+
+  /// True if this [Inheritable] has a parameter whose type is overridden
+  /// by a subtype.
+  bool get isCovariant;
+
+  @override
+  Set<String> get features {
+    Set<String> _features = super.features;
+    if (isOverride) _features.add('override');
+    if (isInherited) _features.add('inherited');
+    if (isCovariant) _features.add('covariant');
+    return _features;
+  }
+
+  @override
+  Library get canonicalLibrary => canonicalEnclosingContainer?.canonicalLibrary;
+
   @override
   ModelElement buildCanonicalModelElement() {
+    // TODO(jcollins-g): factor out extension logic into [Extendable]
     if (canonicalEnclosingContainer is Extension) {
       return this;
     }
@@ -34,6 +54,9 @@ mixin Inheritable on ContainerMember {
                   m.name == name && m.isPropertyAccessor == isPropertyAccessor,
               orElse: () => null);
     }
+    if (canonicalEnclosingContainer != null) {
+      throw UnimplementedError('${canonicalEnclosingContainer}: unknown type');
+    }
     return null;
   }
 
@@ -77,8 +100,14 @@ mixin Inheritable on ContainerMember {
       if (definingEnclosingContainer.isCanonical &&
           definingEnclosingContainer.isPublic) {
         assert(definingEnclosingContainer == found);
+      }
+      if (found != null) {
         return found;
       }
+    } else if (!isInherited && definingEnclosingContainer is! Extension) {
+      // TODO(jcollins-g): factor out extension logic into [Extendable].
+      return packageGraph
+          .findCanonicalModelElementFor(element.enclosingElement);
     }
     return super.computeCanonicalEnclosingContainer();
   }
@@ -104,7 +133,7 @@ mixin Inheritable on ContainerMember {
 
   bool _isOverride;
 
-  @override
+  /// True if this [Inheritable] is overriding a superclass.
   bool get isOverride {
     if (_isOverride == null) {
       // The canonical version of the enclosing element -- not canonicalEnclosingElement,

lib/src/model/model_element.dart

@@ -647,7 +647,7 @@ abstract class ModelElement extends Canonicalization
     if (!_canonicalLibraryIsSet) {
       // This is not accurate if we are constructing the Package.
       assert(packageGraph.allLibrariesAdded);
-      // Since we're may be looking for a library, find the [Element] immediately
+      // Since we're looking for a library, find the [Element] immediately
       // contained by a [CompilationUnitElement] in the tree.
       Element topLevelElement = element;
       while (topLevelElement != null &&

test/model_test.dart

@@ -1389,6 +1389,17 @@ void main() {
   });
 
   group('Class edge cases', () {
+    test('Inherit from private class across private library to public library',
+        () {
+      Class GadgetExtender = packageGraph.localPublicLibraries
+          .firstWhere((l) => l.name == 'gadget_extender')
+          .allClasses
+          .firstWhere((c) => c.name == 'GadgetExtender');
+      Field gadgetGetter =
+          GadgetExtender.allFields.firstWhere((f) => f.name == 'gadgetGetter');
+      expect(gadgetGetter.isCanonical, isTrue);
+    });
+
     test(
         'ExecutableElements from private classes and from public interfaces (#1561)',
         () {

test/src/utils.dart

@@ -17,7 +17,7 @@ import 'package:path/path.dart' as path;
 /// The number of public libraries in testing/test_package, minus 2 for
 /// the excluded libraries listed in the initializers for _testPackageGraphMemo
 /// and minus 1 for the <nodoc> tag in the 'excluded' library.
-const int kTestPackagePublicLibraries = 15;
+const int kTestPackagePublicLibraries = 16;
 
 final RegExp quotables = RegExp(r'[ "\r\n\$]');
 final RegExp observatoryPortRegexp =

testing/test_package/lib/gadget_extender.dart

@@ -0,0 +1,7 @@
+// Copyright (c) 2020, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'src/gadget.dart';
+
+class GadgetExtender extends Gadget {}

testing/test_package/lib/src/gadget.dart

@@ -0,0 +1,10 @@
+// Copyright (c) 2020, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+class _GadgetBase {
+  int get gadgetGetter => 5;
+}
+
+
+class Gadget extends _GadgetBase {}