Skip to content

Tighten types and visibilities #3559

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 0 additions & 43 deletions lib/src/generator/templates.runtime_renderers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7663,13 +7663,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,
Expand All @@ -7690,18 +7683,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,
Expand Down Expand Up @@ -7835,18 +7816,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,
Expand All @@ -7869,18 +7838,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,
Expand Down
4 changes: 4 additions & 0 deletions lib/src/model/inheriting_container.dart
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,7 @@ abstract class InheritingContainer extends Container
hasPublicSuperChainReversed ||
hasPotentiallyApplicableExtensions;

@visibleForTesting
bool get hasPublicInheritedMethods => publicInheritedMethods.isNotEmpty;

bool get hasPublicSuperChainReversed => publicSuperChainReversed.isNotEmpty;
Expand All @@ -303,6 +304,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
Expand Down Expand Up @@ -332,6 +334,7 @@ abstract class InheritingContainer extends Container

bool get isSealed;

@visibleForTesting
Iterable<Field> get publicInheritedFields =>
model_utils.filterNonPublic(inheritedFields);

Expand All @@ -347,6 +350,7 @@ abstract class InheritingContainer extends Container
bool get publicInheritedInstanceOperators =>
publicInstanceOperators.every((f) => f.isInherited);

@visibleForTesting
Iterable<Method> get publicInheritedMethods =>
model_utils.filterNonPublic(inheritedMethods);

Expand Down
6 changes: 3 additions & 3 deletions lib/src/model/model_element.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down Expand Up @@ -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 ||
Expand Down
43 changes: 22 additions & 21 deletions lib/src/model/package_graph.dart
Original file line number Diff line number Diff line change
Expand Up @@ -280,8 +280,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 = {};
Expand Down Expand Up @@ -533,7 +533,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;

Expand Down Expand Up @@ -675,7 +674,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) {
Expand Down Expand Up @@ -728,8 +727,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
Expand Down Expand Up @@ -768,20 +767,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);
Expand All @@ -790,7 +791,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.
Expand Down