Refactor special case for how canonical enclosing containers are calculated (#3925)

There is some complicated logic in the calculations for determining a canonical
enclosing container, when we find ourselves examining a "hidden" interface. The
list of "hidden" interfaces, which we should not consider when determining a
canonical enclosing container, is exactly one: `Interceptor`. But it's
important that it stay buried.

The logic was hard to read, so first, I split out a complicated if-condition
into a few local variables. The logic also needs to track the "current"
container in the inheritance list, and the "previous", and the "previous
visible." This was complicated when using a for-in loop. I changed it to use a
standard for loop with indices.

Then I saw that we only ever find outself asking a container for it's
"memberByExample" in the case when we're looking for one of Object's members
(`toString`, `noSuchMethod`, `operator ==`, `hashCode`, and `runtimeType`. We
can inline the logic to fetch a member by example. This allows us to delete a
late field on every Container.

Also, the package graph `_invisibleAnnotations` and `_inheritThrough` fields
had very complicated initializers which I simplified with if-elements.

Author: srawlins

lib/src/generator/templates.runtime_renderers.dart

@@ -16364,7 +16364,6 @@ const _invisibleGetters = {
     'hasFooterVersion',
     'hashCode',
     'implementers',
-    'inheritThrough',
     'inheritanceManager',
     'libraries',
     'libraryCount',

lib/src/model/comment_referable.dart

@@ -223,14 +223,13 @@ extension on Scope {
 /// A set of utility methods for helping build
 /// [CommentReferable.referenceChildren] out of collections of other
 /// [CommentReferable]s.
-extension CommentReferableEntryGenerators on Iterable<CommentReferable> {
+extension CommentReferableEntryGenerators<T extends CommentReferable>
+    on Iterable<T> {
   /// Creates reference entries for this Iterable.
   ///
   /// If there is a conflict with [referable], the included [MapEntry] uses
   /// [referable]'s [CommentReferable.referenceName] as a prefix.
-  Map<String, CommentReferable> explicitOnCollisionWith(
-          CommentReferable referable) =>
-      {
+  Map<String, T> explicitOnCollisionWith(CommentReferable referable) => {
         for (var r in this)
           if (r.referenceName == referable.referenceName)
             '${referable.referenceName}.${r.referenceName}': r
@@ -239,13 +238,13 @@ extension CommentReferableEntryGenerators on Iterable<CommentReferable> {
       };
 
   /// A mapping from each [CommentReferable]'s name to itself.
-  Map<String, CommentReferable> get asMapByName => {
+  Map<String, T> get asMapByName => {
         for (var r in this) r.referenceName: r,
       };
 
   /// Returns all values not of this type.
-  List<CommentReferable> whereNotType<T>() => [
+  List<T> whereNotType<U>() => [
         for (var referable in this)
-          if (referable is! T) referable,
+          if (referable is! U) referable,
       ];
 }

lib/src/model/container.dart

@@ -156,33 +156,6 @@ abstract class Container extends ModelElement
   late final Set<Element> _allElements =
       allModelElements.map((e) => e.element).toSet();
 
-  late final Map<String, List<ModelElement>> _membersByName = () {
-    var membersByName = <String, List<ModelElement>>{};
-    for (var element in allModelElements) {
-      membersByName.putIfAbsent(element.name, () => []).add(element);
-    }
-    return membersByName;
-  }();
-
-  /// Given a [ModelElement] that is a member of some other class, returns
-  /// the member of this class that has the same name and runtime type.
-  ///
-  /// This enables object substitution for canonicalization, such as Interceptor
-  /// for Object.
-  T memberByExample<T extends ModelElement>(T example) {
-    // [T] is insufficiently specific to disambiguate between different
-    // subtypes of [Inheritable] or other mixins/implementations of
-    // [ModelElement] via [Iterable.whereType].
-    var possibleMembers = _membersByName[example.name]!
-        .where((e) => e.runtimeType == example.runtimeType);
-    if (example is Accessor) {
-      possibleMembers = possibleMembers
-          .where((e) => example.isGetter == (e as Accessor).isGetter);
-    }
-    assert(possibleMembers.length == 1);
-    return possibleMembers.first as T;
-  }
-
   bool get hasPublicStaticFields => staticFields.any((e) => e.isPublic);
 
   List<Field> get publicStaticFieldsSorted =>

lib/src/model/inheritable.dart

@@ -5,6 +5,7 @@
 import 'package:analyzer/dart/element/element.dart';
 import 'package:collection/collection.dart' show IterableExtension;
 import 'package:dartdoc/src/model/attribute.dart';
+import 'package:dartdoc/src/model/comment_referable.dart';
 import 'package:dartdoc/src/model/model.dart';
 import 'package:dartdoc/src/special_elements.dart';
 
@@ -58,22 +59,40 @@ mixin Inheritable on ContainerMember {
       var searchElement = element.declaration;
       // TODO(jcollins-g): generate warning if an inherited element's definition
       // is in an intermediate non-canonical class in the inheritance chain?
-      Container? previous;
-      Container? previousNonSkippable;
       Container? found;
-      for (var c in _inheritance.reversed) {
-        // Filter out mixins.
-        if (c.containsElement(searchElement)) {
-          if ((packageGraph.inheritThrough.contains(previous) &&
-                  c != definingEnclosingContainer) ||
-              (packageGraph.inheritThrough.contains(c) &&
-                  c == definingEnclosingContainer)) {
-            return previousNonSkippable!
-                .memberByExample(this)
-                .canonicalEnclosingContainer;
+      var reverseInheritance = _inheritance.reversed.toList();
+      for (var i = 0; i < reverseInheritance.length; i++) {
+        var container = reverseInheritance[i];
+        if (container.containsElement(searchElement)) {
+          var previousIsHiddenAndNotDefining = i > 0 &&
+              _isHiddenInterface(reverseInheritance[i - 1]) &&
+              container != definingEnclosingContainer;
+          var thisIsHiddenAndDefining = _isHiddenInterface(container) &&
+              container == definingEnclosingContainer;
+          // If the previous container in the search is one of the "hidden"
+          // interfaces, and it's not this member's defining container, OR if
+          // this container in the search is one of the "hidden" interfaces,
+          // and it is also this member's defining container, then we can just
+          // immediately return the canonical enclosing container of the
+          // overridden member in the previous, non-hidden container in the
+          // inheritance.
+          if (previousIsHiddenAndNotDefining || thisIsHiddenAndDefining) {
+            var previousVisible = reverseInheritance
+                .take(i)
+                .lastWhere((e) => !_isHiddenInterface(e));
+            var membersInPreviousVisible = previousVisible.allModelElements
+                .where((e) => e.name == name)
+                .whereType<Inheritable>()
+                .whereNotType<Field>();
+            assert(
+                membersInPreviousVisible.length == 1,
+                'found multiple members named "$name" in '
+                '"${previousVisible.name}": '
+                '${membersInPreviousVisible.toList()}');
+            return membersInPreviousVisible.first.canonicalEnclosingContainer;
           }
-          var canonicalContainer =
-              packageGraph.findCanonicalModelElementFor(c) as Container?;
+          var canonicalContainer = packageGraph
+              .findCanonicalModelElementFor(container) as Container?;
           // TODO(jcollins-g): invert this lookup so traversal is recursive
           // starting from the ModelElement.
           if (canonicalContainer != null) {
@@ -83,10 +102,6 @@ mixin Inheritable on ContainerMember {
             break;
           }
         }
-        previous = c;
-        if (!packageGraph.inheritThrough.contains(c)) {
-          previousNonSkippable = c;
-        }
       }
       if (found != null) {
         return found;
@@ -99,6 +114,8 @@ mixin Inheritable on ContainerMember {
     return super.computeCanonicalEnclosingContainer();
   }
 
+  bool _isHiddenInterface(Container? c) => packageGraph.isHiddenInterface(c);
+
   /// A roughly ordered list of this element's enclosing container's inheritance
   /// chain.
   ///

lib/src/model/package_graph.dart

@@ -706,29 +706,28 @@ class PackageGraph with CommentReferable, Nameable {
           ?.linkedName ??
       'Object';
 
-  /// The set of [Class]es which should _not_ be presented as implementers.
+  /// The set of [Class]es which should _not_ be considered the canonical
+  /// enclosing container of any container member.
   ///
   /// Add classes here if they are similar to Interceptor in that they are to be
   /// ignored even when they are the implementers of [Inheritable]s, and the
   /// class these inherit from should instead claim implementation.
-  late final Set<Class> inheritThrough = () {
-    var interceptorSpecialClass = specialClasses[SpecialClass.interceptor];
-    if (interceptorSpecialClass == null) {
-      return const <Class>{};
-    }
+  late final Set<Class> _inheritThrough = {
+    if (specialClasses[SpecialClass.interceptor] case var interceptor?)
+      interceptor,
+  };
 
-    return {interceptorSpecialClass};
-  }();
+  /// Whether [c] is a "hidden" interface.
+  ///
+  /// A hidden interface should never be considered the canonical enclosing
+  /// container of a container member.
+  bool isHiddenInterface(Container? c) => _inheritThrough.contains(c);
 
   /// The set of [Class] objects that are similar to 'pragma' in that we should
   /// never count them as documentable annotations.
-  late final Set<Class> _invisibleAnnotations = () {
-    var pragmaSpecialClass = specialClasses[SpecialClass.pragma];
-    if (pragmaSpecialClass == null) {
-      return const <Class>{};
-    }
-    return {pragmaSpecialClass};
-  }();
+  late final Set<Class> _invisibleAnnotations = {
+    if (specialClasses[SpecialClass.pragma] case var pragma?) pragma,
+  };
 
   bool isAnnotationVisible(Class class_) =>
       !_invisibleAnnotations.contains(class_);