Connect interface types across private intermediate classes (#2495)

* First fix attempt

* tweak and fix tests

* add test

Author: jcollins-g

lib/src/model/class.dart

@@ -41,7 +41,7 @@ class Class extends Container
       'Dartdoc 1.0.0')
   set supertype(DefinedElementType value) => _supertype = value;
 
-  final List<DefinedElementType> _interfaces;
+  final List<DefinedElementType> _directInterfaces;
 
   Class(ClassElement element, Library library, PackageGraph packageGraph)
       : _mixedInTypes = element.mixins
@@ -52,7 +52,7 @@ class Class extends Container
         _supertype = element.supertype?.element?.supertype == null
             ? null
             : ElementType.from(element.supertype, library, packageGraph),
-        _interfaces = element.interfaces
+        _directInterfaces = element.interfaces
             .map<DefinedElementType>(
                 (f) => ElementType.from(f, library, packageGraph))
             .toList(growable: false),
@@ -258,10 +258,46 @@ class Class extends Container
   Iterable<Field> get publicInheritedFields =>
       model_utils.filterNonPublic(inheritedFields);
 
-  List<DefinedElementType> get interfaces => _interfaces;
-
-  Iterable<DefinedElementType> get publicInterfaces =>
-      model_utils.filterNonPublic(interfaces);
+  /// Interfaces directly implemented by this class.
+  List<DefinedElementType> get interfaces => _directInterfaces;
+
+  /// The public interfaces may include substitutions for intermediate
+  /// private interfaces, and so unlike other public* methods, is not
+  /// a strict subset of [interfaces].
+  Iterable<DefinedElementType> get publicInterfaces sync* {
+    for (var i in _directInterfaces) {
+      /// Do not recurse if we can find an element here.
+      if (i.element.canonicalModelElement != null) {
+        yield i;
+        continue;
+      }
+      // Public types used to be unconditionally exposed here.  However,
+      // if the packages are [DocumentLocation.missing] we generally treat types
+      // defined in them as actually defined in a documented package.
+      // That translates to them being defined here, but in 'src/' or similar,
+      // and so, are now always hidden.
+
+      // This type is not backed by a canonical Class; search
+      // the superchain and publicInterfaces of this interface to pretend
+      // as though the hidden class didn't exist and this class was declared
+      // directly referencing the canonical classes further up the chain.
+      if (i.element is Class) {
+        var hiddenClass = i.element as Class;
+        if (hiddenClass.publicSuperChain.isNotEmpty) {
+          yield hiddenClass.publicSuperChain.first;
+        }
+        yield* hiddenClass.publicInterfaces;
+      } else {
+        assert(
+            false,
+            'Can not handle intermediate non-public interfaces '
+            'created by ModelElements that are not classes or mixins:  '
+            '${fullyQualifiedName} contains an interface {$i}, '
+            'defined by ${i.element}');
+        continue;
+      }
+    }
+  }
 
   bool get isAbstract => element.isAbstract;
 

lib/src/model/package_graph.dart

@@ -618,6 +618,9 @@ class PackageGraph {
       for (var type in class_.interfaces) {
         checkAndAddClass(type.element, class_);
       }
+      for (var type in class_.publicInterfaces) {
+        checkAndAddClass(type.element, class_);
+      }
     }
 
     classes.forEach(addImplementor);

test/end2end/model_test.dart

@@ -1313,6 +1313,30 @@ void main() {
   });
 
   group('Class edge cases', () {
+    // This is distinct from inheritance in the language and the analyzer
+    // implementation.
+    test(
+        'Implementor chain is correctly rewritten through intermediate private classes',
+        () {
+      var implementorsLibrary = packageGraph.publicLibraries
+          .firstWhere((l) => l.name == 'implementors');
+      var ImplementerOfDeclaredPrivateClasses = implementorsLibrary.classes
+          .firstWhere((c) => c.name == 'ImplementerOfDeclaredPrivateClasses');
+      var ImplementerOfThings = implementorsLibrary.classes
+          .firstWhere((c) => c.name == 'ImplementerOfThings');
+      var ImplementBase = implementorsLibrary.classes
+          .firstWhere((c) => c.name == 'ImplementBase');
+
+      expect(ImplementerOfThings.publicInterfaces.first.element,
+          equals(ImplementBase));
+      expect(ImplementerOfDeclaredPrivateClasses.publicInterfaces.first.element,
+          equals(ImplementBase));
+
+      expect(ImplementBase.publicImplementors,
+          contains(ImplementerOfDeclaredPrivateClasses));
+      expect(ImplementBase.publicImplementors, contains(ImplementerOfThings));
+    });
+
     test('Overrides from intermediate abstract classes are picked up correctly',
         () {
       var IntermediateAbstractSubclass = fakeLibrary.allClasses
@@ -3410,8 +3434,8 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans,
           ImplementsFutureVoid.linkedName,
           equals(
               '<a href="${HTMLBASE_PLACEHOLDER}fake/ImplementsFutureVoid-class.html">ImplementsFutureVoid</a>'));
-      var FutureVoid = ImplementsFutureVoid.publicInterfaces
-          .firstWhere((c) => c.name == 'Future');
+      var FutureVoid =
+          ImplementsFutureVoid.interfaces.firstWhere((c) => c.name == 'Future');
       expect(
           FutureVoid.linkedName,
           equals(

test/src/utils.dart

@@ -14,7 +14,7 @@ import 'package:dartdoc/src/package_meta.dart';
 /// 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 = 17;
+const int kTestPackagePublicLibraries = 18;
 
 final _resourceProvider = pubPackageMetaProvider.resourceProvider;
 final _pathContext = _resourceProvider.pathContext;

testing/test_package/lib/implementors.dart

@@ -0,0 +1,16 @@
+// Copyright (c) 2021, 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.
+
+// A library useful to check implementer chain edge cases.
+library implementors;
+
+import 'src/intermediate_implements.dart';
+
+abstract class ImplementBase {}
+
+abstract class _APrivateThingToImplement implements ImplementBase {}
+
+abstract class ImplementerOfThings implements IntermediateImplementer {}
+
+abstract class ImplementerOfDeclaredPrivateClasses implements _APrivateThingToImplement {}

testing/test_package/lib/src/intermediate_implements.dart

@@ -0,0 +1,10 @@
+// Copyright (c) 2021, 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.
+
+// A src-private library for intermediate classes to implement.
+library intermediate_implements;
+
+import 'package:test_package/implementors.dart';
+
+abstract class IntermediateImplementer implements ImplementBase {}