Tighten types and visibilities (#3559)

srawlins · web-flow · commit 17c5e5a7f506 · 2023-10-30T15:41:33.000-07:00
diff --git a/lib/src/generator/templates.runtime_renderers.dart b/lib/src/generator/templates.runtime_renderers.dart
@@ -7589,13 +7589,6 @@ class _Renderer_InheritingContainer extends RendererBase<InheritingContainer> {
                       self.renderSimpleVariable(c, remainingNames, 'bool'),
                   getBool: (CT_ c) => c.hasModifiers == true,
                 ),
-                'hasPublicInheritedMethods': Property(
-                  getValue: (CT_ c) => c.hasPublicInheritedMethods,
-                  renderVariable: (CT_ c, Property<CT_> self,
-                          List<String> remainingNames) =>
-                      self.renderSimpleVariable(c, remainingNames, 'bool'),
-                  getBool: (CT_ c) => c.hasPublicInheritedMethods == true,
-                ),
                 'hasPublicSuperChainReversed': Property(
                   getValue: (CT_ c) => c.hasPublicSuperChainReversed,
                   renderVariable: (CT_ c, Property<CT_> self,
@@ -7616,18 +7609,6 @@ class _Renderer_InheritingContainer extends RendererBase<InheritingContainer> {
                             parent: r));
                   },
                 ),
-                'inheritedFields': Property(
-                  getValue: (CT_ c) => c.inheritedFields,
-                  renderVariable: (CT_ c, Property<CT_> self,
-                          List<String> remainingNames) =>
-                      self.renderSimpleVariable(
-                          c, remainingNames, 'Iterable<Field>'),
-                  renderIterable: (CT_ c, RendererBase<CT_> r,
-                      List<MustachioNode> ast, StringSink sink) {
-                    return c.inheritedFields.map((e) =>
-                        _render_Field(e, ast, r.template, sink, parent: r));
-                  },
-                ),
                 'inheritedMethods': Property(
                   getValue: (CT_ c) => c.inheritedMethods,
                   renderVariable: (CT_ c, Property<CT_> self,
@@ -7761,18 +7742,6 @@ class _Renderer_InheritingContainer extends RendererBase<InheritingContainer> {
                         parent: r);
                   },
                 ),
-                'publicInheritedFields': Property(
-                  getValue: (CT_ c) => c.publicInheritedFields,
-                  renderVariable: (CT_ c, Property<CT_> self,
-                          List<String> remainingNames) =>
-                      self.renderSimpleVariable(
-                          c, remainingNames, 'Iterable<Field>'),
-                  renderIterable: (CT_ c, RendererBase<CT_> r,
-                      List<MustachioNode> ast, StringSink sink) {
-                    return c.publicInheritedFields.map((e) =>
-                        _render_Field(e, ast, r.template, sink, parent: r));
-                  },
-                ),
                 'publicInheritedInstanceFields': Property(
                   getValue: (CT_ c) => c.publicInheritedInstanceFields,
                   renderVariable: (CT_ c, Property<CT_> self,
@@ -7795,18 +7764,6 @@ class _Renderer_InheritingContainer extends RendererBase<InheritingContainer> {
                   getBool: (CT_ c) =>
                       c.publicInheritedInstanceOperators == true,
                 ),
-                'publicInheritedMethods': Property(
-                  getValue: (CT_ c) => c.publicInheritedMethods,
-                  renderVariable: (CT_ c, Property<CT_> self,
-                          List<String> remainingNames) =>
-                      self.renderSimpleVariable(
-                          c, remainingNames, 'Iterable<Method>'),
-                  renderIterable: (CT_ c, RendererBase<CT_> r,
-                      List<MustachioNode> ast, StringSink sink) {
-                    return c.publicInheritedMethods.map((e) =>
-                        _render_Method(e, ast, r.template, sink, parent: r));
-                  },
-                ),
                 'publicInterfaces': Property(
                   getValue: (CT_ c) => c.publicInterfaces,
                   renderVariable: (CT_ c, Property<CT_> self,
diff --git a/lib/src/model/inheriting_container.dart b/lib/src/model/inheriting_container.dart
@@ -287,6 +287,7 @@ abstract class InheritingContainer extends Container
       hasPublicSuperChainReversed ||
       hasPotentiallyApplicableExtensions;
 
+  @visibleForTesting
   bool get hasPublicInheritedMethods => publicInheritedMethods.isNotEmpty;
 
   bool get hasPublicSuperChainReversed => publicSuperChainReversed.isNotEmpty;
@@ -299,6 +300,7 @@ abstract class InheritingContainer extends Container
   /// purposes in abstract classes.
   List<InheritingContainer> get inheritanceChain;
 
+  @visibleForTesting
   Iterable<Field> get inheritedFields => allFields.where((f) => f.isInherited);
 
   @override
@@ -328,6 +330,7 @@ abstract class InheritingContainer extends Container
 
   bool get isSealed;
 
+  @visibleForTesting
   Iterable<Field> get publicInheritedFields =>
       model_utils.filterNonPublic(inheritedFields);
 
@@ -343,6 +346,7 @@ abstract class InheritingContainer extends Container
   bool get publicInheritedInstanceOperators =>
       publicInstanceOperators.every((f) => f.isInherited);
 
+  @visibleForTesting
   Iterable<Method> get publicInheritedMethods =>
       model_utils.filterNonPublic(inheritedMethods);
 
diff --git a/lib/src/model/model_element.dart b/lib/src/model/model_element.dart
@@ -287,7 +287,7 @@ abstract class ModelElement extends Canonicalization
   /// Caches a newly-created [ModelElement] from [ModelElement._from] or
   /// [ModelElement._fromPropertyInducingElement].
   static void _cacheNewModelElement(
-      Element e, ModelElement? newModelElement, Library library,
+      Element e, ModelElement newModelElement, Library library,
       {Container? enclosingContainer}) {
     // TODO(jcollins-g): Reenable Parameter caching when dart-lang/sdk#30146
     //                   is fixed?
@@ -355,8 +355,8 @@ abstract class ModelElement extends Canonicalization
     PropertyAccessorElement e,
     Library library,
     PackageGraph packageGraph, {
-    Container? enclosingContainer,
-    Member? originalMember,
+    required Container? enclosingContainer,
+    required Member? originalMember,
   }) {
     // Accessors can be part of a [Container], or a part of a [Library].
     if (e.enclosingElement is ExtensionElement ||
diff --git a/lib/src/model/package_graph.dart b/lib/src/model/package_graph.dart
@@ -285,8 +285,8 @@ class PackageGraph with CommentReferable, Nameable, ModelBuilder {
 
   /// All [ModelElement]s constructed for this package; a superset of
   /// [_allModelElements].
-  final Map<(Element element, Library? library, Container? enclosingElement),
-      ModelElement?> allConstructedModelElements = {};
+  final Map<(Element element, Library library, Container? enclosingElement),
+      ModelElement> allConstructedModelElements = {};
 
   /// Anything that might be inheritable, place here for later lookup.
   final Map<(Element, Library), Set<ModelElement>> allInheritableElements = {};
@@ -538,7 +538,6 @@ class PackageGraph with CommentReferable, Nameable, ModelBuilder {
       if (modelElement is Dynamic) continue;
       // TODO: see [Accessor.enclosingCombo]
       if (modelElement is Accessor) continue;
-      if (modelElement == null) continue;
       final href = modelElement.href;
       if (href == null) continue;
 
@@ -680,7 +679,7 @@ class PackageGraph with CommentReferable, Nameable, ModelBuilder {
     return buffer.toString();
   }
 
-  final Map<Element?, Library?> _canonicalLibraryFor = {};
+  final Map<Element, Library?> _canonicalLibraryFor = {};
 
   /// Tries to find a top level library that references this element.
   Library? _findCanonicalLibraryFor(Element e) {
@@ -733,8 +732,8 @@ class PackageGraph with CommentReferable, Nameable, ModelBuilder {
       lib ??= modelBuilder.fromElement(enclosingElement.library) as Library?;
       // TODO(keertip): Find a better way to exclude members of extensions
       // when libraries are specified using the "--include" flag.
-      if (lib?.isDocumented == true) {
-        return modelBuilder.from(e, lib!);
+      if (lib != null && lib.isDocumented) {
+        return modelBuilder.from(e, lib);
       }
     }
     // TODO(jcollins-g): The data structures should be changed to eliminate
@@ -773,20 +772,22 @@ class PackageGraph with CommentReferable, Nameable, ModelBuilder {
 
   ModelElement? _findCanonicalModelElementForAmbiguous(Element e, Library? lib,
       {InheritingContainer? preferredClass}) {
-    var candidates = <ModelElement?>{};
-    var constructedWithKey = allConstructedModelElements[(e, lib, null)];
-    if (constructedWithKey != null) {
-      candidates.add(constructedWithKey);
-    }
-    var constructedWithKeyWithClass =
-        allConstructedModelElements[(e, lib, preferredClass)];
-    if (constructedWithKeyWithClass != null) {
-      candidates.add(constructedWithKeyWithClass);
-    }
-    if (candidates.isEmpty) {
-      candidates = {
-        ...?allInheritableElements[(e, lib)]?.where((me) => me.isCanonical),
-      };
+    var candidates = <ModelElement>{};
+    if (lib != null) {
+      var constructedWithKey = allConstructedModelElements[(e, lib, null)];
+      if (constructedWithKey != null) {
+        candidates.add(constructedWithKey);
+      }
+      var constructedWithKeyWithClass =
+          allConstructedModelElements[(e, lib, preferredClass)];
+      if (constructedWithKeyWithClass != null) {
+        candidates.add(constructedWithKeyWithClass);
+      }
+      if (candidates.isEmpty) {
+        candidates = {
+          ...?allInheritableElements[(e, lib)]?.where((me) => me.isCanonical),
+        };
+      }
     }
 
     var canonicalClass = findCanonicalModelElementFor(e.enclosingElement);
@@ -795,7 +796,7 @@ class PackageGraph with CommentReferable, Nameable, ModelBuilder {
           .where((m) => m.element == e));
     }
 
-    var matches = {...candidates.whereNotNull().where((me) => me.isCanonical)};
+    var matches = {...candidates.where((me) => me.isCanonical)};
 
     // It's possible to find [Accessor]s but no combos.  Be sure that if we
     // have accessors, we find their combos too.
