Skip to content
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.

Sign up for GitHub

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

Include extension types in 'implementers' list #3658

Merged
merged 3 commits into from
Feb 16, 2024
Merged

Include extension types in 'implementers' list #3658

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a26d201
Include extension types in 'implementors' list
srawlins Feb 15, 2024
48bb68e
Merge branch 'main' into extension-types-implement
srawlins Feb 16, 2024
9df3112
feedback
srawlins Feb 16, 2024
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
38 changes: 32 additions & 6 deletions lib/src/generator/templates.runtime_renderers.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7746,6 +7746,19 @@ class _Renderer_InheritingContainer extends RendererBase<InheritingContainer> {
self.renderSimpleVariable(c, remainingNames, 'bool'),
getBool: (CT_ c) => c.publicInheritedInstanceOperators,
),
'publicInterfaceElements': Property(
getValue: (CT_ c) => c.publicInterfaceElements,
renderVariable: (CT_ c, Property<CT_> self,
List<String> remainingNames) =>
self.renderSimpleVariable(
c, remainingNames, 'Iterable<InheritingContainer>'),
renderIterable: (CT_ c, RendererBase<CT_> r,
List<MustachioNode> ast, StringSink sink) {
return c.publicInterfaceElements.map((e) =>
_render_InheritingContainer(e, ast, r.template, sink,
parent: r));
},
),
'publicInterfaces': Property(
getValue: (CT_ c) => c.publicInterfaces,
renderVariable: (CT_ c, Property<CT_> self,
Expand Down Expand Up @@ -9934,17 +9947,17 @@ class _Renderer_MixedInTypes extends RendererBase<MixedInTypes> {
self.renderSimpleVariable(c, remainingNames, 'bool'),
getBool: (CT_ c) => c.hasPublicMixedInTypes,
),
'mixedInTypes': Property(
getValue: (CT_ c) => c.mixedInTypes,
'mixedInElements': Property(
getValue: (CT_ c) => c.mixedInElements,
renderVariable: (CT_ c, Property<CT_> self,
List<String> remainingNames) =>
self.renderSimpleVariable(
c, remainingNames, 'List<DefinedElementType>'),
c, remainingNames, 'List<InheritingContainer>'),
renderIterable: (CT_ c, RendererBase<CT_> r,
List<MustachioNode> ast, StringSink sink) {
return c.mixedInTypes.map((e) => _render_DefinedElementType(
e, ast, r.template, sink,
parent: r));
return c.mixedInElements.map((e) =>
_render_InheritingContainer(e, ast, r.template, sink,
parent: r));
},
),
'publicMixedInTypes': Property(
Expand Down Expand Up @@ -15267,6 +15280,19 @@ class _Renderer_TypeImplementing extends RendererBase<TypeImplementing> {
self.renderSimpleVariable(c, remainingNames, 'bool'),
getBool: (CT_ c) => c.hasPublicInterfaces,
),
'interfaceElements': Property(
getValue: (CT_ c) => c.interfaceElements,
renderVariable: (CT_ c, Property<CT_> self,
List<String> remainingNames) =>
self.renderSimpleVariable(
c, remainingNames, 'List<InheritingContainer>'),
renderIterable: (CT_ c, RendererBase<CT_> r,
List<MustachioNode> ast, StringSink sink) {
return c.interfaceElements.map((e) =>
_render_InheritingContainer(e, ast, r.template, sink,
parent: r));
},
),
'interfaces': Property(
getValue: (CT_ c) => c.interfaces,
renderVariable: (CT_ c, Property<CT_> self,
Expand Down
4 changes: 2 additions & 2 deletions lib/src/model/class.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,15 @@ class Class extends InheritingContainer
this,

// Caching should make this recursion a little less painful.
for (var container in mixedInTypes.reversed.modelElements)
for (var container in mixedInElements.reversed)
...container.inheritanceChain,

for (var container in superChain.modelElements)
...container.inheritanceChain,

// Interfaces need to come last, because classes in the superChain might
// implement them even when they aren't mentioned.
...interfaces.expandInheritanceChain,
...interfaceElements.expandInheritanceChain,
];

Class(this.element, Library library, PackageGraph packageGraph)
Expand Down
4 changes: 2 additions & 2 deletions lib/src/model/enum.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@ class Enum extends InheritingContainer
@override
late final List<InheritingContainer> inheritanceChain = [
this,
for (var container in mixedInTypes.reversed.modelElements)
for (var container in mixedInElements.reversed)
...container.inheritanceChain,
for (var container in superChain.modelElements)
...container.inheritanceChain,
...interfaces.expandInheritanceChain,
...interfaceElements.expandInheritanceChain,
];

@override
Expand Down
2 changes: 1 addition & 1 deletion lib/src/model/extension_type.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class ExtensionType extends InheritingContainer
@override
late final List<InheritingContainer> inheritanceChain = [
this,
...interfaces.expandInheritanceChain,
...interfaceElements.expandInheritanceChain,
];

@override
Expand Down
51 changes: 35 additions & 16 deletions lib/src/model/inheriting_container.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,12 @@ abstract class InheritingContainer extends Container
Iterable<Method> get publicInheritedMethods =>
model_utils.filterNonPublic(inheritedMethods);

Iterable<DefinedElementType> get publicInterfaces => const [];
Iterable<DefinedElementType> get publicInterfaces;

Iterable<InheritingContainer> get publicInterfaceElements => [
for (var interface in publicInterfaces)
interface.modelElement as InheritingContainer,
];

Iterable<DefinedElementType> get publicSuperChainReversed =>
publicSuperChain.reversed;
Expand Down Expand Up @@ -474,10 +479,15 @@ abstract class InheritingContainer extends Container

/// Add the ability to support mixed-in types to an [InheritingContainer].
mixin MixedInTypes on InheritingContainer {
@visibleForTesting
late final List<DefinedElementType> mixedInTypes = element.mixins
.map((f) => modelBuilder.typeFrom(f, library) as DefinedElementType)
.toList(growable: false);

List<InheritingContainer> get mixedInElements => [
for (var t in mixedInTypes) t.modelElement as InheritingContainer,
];

@override
bool get hasModifiers => super.hasModifiers || hasPublicMixedInTypes;

Expand Down Expand Up @@ -509,9 +519,14 @@ mixin TypeImplementing on InheritingContainer {
/// Interfaces directly implemented by this container.
List<DefinedElementType> get interfaces => _directInterfaces;

/// Returns all the "immediate" public implementors of this
/// [TypeImplementing]. For a [Mixin], this is actually the mixin
/// applications using the [Mixin].
List<InheritingContainer> get interfaceElements => [
for (var interface in interfaces)
interface.modelElement as InheritingContainer,
];

/// All the "immediate" public implementors of this [TypeImplementing].
///
/// For a [Mixin], this is actually the mixin applications using the [Mixin].
///
/// If this [InheritingContainer] has a private implementor, then that is
/// counted as a proxy for any public implementors of that private container.
Expand All @@ -527,16 +542,17 @@ mixin TypeImplementing on InheritingContainer {
if (implementor.isPublicAndPackageDocumented) {
result.add(implementor);
} else {
model_utils
.findCanonicalFor(
packageGraph.implementors[implementor] ?? const [])
.forEach(addToResult);
var implementors = packageGraph.implementors[implementor];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we name this something slightly different from line 545 (or the opposite)? My eyes are glazing over see them because they're so similar haha.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And also keep implementor vs implementer spelling consistent.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've renamed the lower one immediateImplementors.

It looks like everything in this file is implementor; the display name in the HTML is "implementer", which I think is more correct. But to change everything in this file would make the diff even bigger. I'd like to address that in a follow-up.

if (implementors != null) {
model_utils.findCanonicalFor(implementors).forEach(addToResult);
}
}
}

model_utils
.findCanonicalFor(packageGraph.implementors[this] ?? const [])
.forEach(addToResult);
var immediateImplementors = packageGraph.implementors[this];
if (immediateImplementors != null) {
model_utils.findCanonicalFor(immediateImplementors).forEach(addToResult);
}
return result;
}

Expand Down Expand Up @@ -587,11 +603,14 @@ extension on InterfaceElement {
}

extension DefinedElementTypeIterableExtension on Iterable<DefinedElementType> {
/// Expands the [ModelElement] for each element to its inheritance chain.
Iterable<InheritingContainer> get expandInheritanceChain =>
expand((e) => (e.modelElement as InheritingContainer).inheritanceChain);

/// Returns the [ModelElement] for each element.
/// The [ModelElement] for each element.
Iterable<InheritingContainer> get modelElements =>
map((e) => e.modelElement as InheritingContainer);
}

extension InheritingContainerIterableExtension
on Iterable<InheritingContainer> {
/// Expands each element to its inheritance chain.
Iterable<InheritingContainer> get expandInheritanceChain =>
expand((e) => e.inheritanceChain);
}
4 changes: 2 additions & 2 deletions lib/src/model/mixin.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,14 @@ class Mixin extends InheritingContainer with TypeImplementing {
this,

// Mix-in interfaces come before other interfaces.
...superclassConstraints.expandInheritanceChain,
...superclassConstraints.modelElements.expandInheritanceChain,

for (var container in superChain.modelElements)
...container.inheritanceChain,

// Interfaces need to come last, because classes in the superChain might
// implement them even when they aren't mentioned.
...interfaces.expandInheritanceChain,
...interfaceElements.expandInheritanceChain,
];

Mixin(this.element, super.library, super.packageGraph);
Expand Down
45 changes: 25 additions & 20 deletions lib/src/model/package_graph.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,9 @@ class PackageGraph with CommentReferable, Nameable, ModelBuilder {
// all packages are picked up.
for (var package in _documentedPackages) {
for (var library in package.libraries) {
_addToImplementors(library.allClasses);
_addToImplementors(library.mixins);
_addToImplementers(library.allClasses);
_addToImplementers(library.mixins);
_addToImplementers(library.extensionTypes);
_extensions.addAll(library.extensions);
}
if (package.isLocal && !package.hasPublicLibraries) {
Expand Down Expand Up @@ -580,7 +581,7 @@ class PackageGraph with CommentReferable, Nameable, ModelBuilder {
return hrefMap;
}

void _addToImplementors(Iterable<InheritingContainer> containers) {
void _addToImplementers(Iterable<InheritingContainer> containers) {
assert(!allImplementorsAdded);

// Private containers may not be included in documentation, but may still be
Expand All @@ -589,7 +590,7 @@ class PackageGraph with CommentReferable, Nameable, ModelBuilder {
var privates = <InheritingContainer>[];

void checkAndAddContainer(
InheritingContainer implemented, InheritingContainer implementor) {
InheritingContainer implemented, InheritingContainer implementer) {
if (!implemented.isPublic) {
privates.add(implemented);
}
Expand All @@ -598,36 +599,40 @@ class PackageGraph with CommentReferable, Nameable, ModelBuilder {
var list = _implementors.putIfAbsent(implemented, () => []);
// TODO(srawlins): This would be more efficient if we created a
// SplayTreeSet keyed off of `.element`.
if (!list.any((l) => l.element == implementor.element)) {
list.add(implementor);
if (!list.any((l) => l.element == implementer.element)) {
list.add(implementer);
}
}

void addImplementor(InheritingContainer clazz) {
var supertype = clazz.supertype;
void addImplementer(InheritingContainer container) {
var supertype = container.supertype;
if (supertype != null) {
checkAndAddContainer(
supertype.modelElement as InheritingContainer, clazz);
supertype.modelElement as InheritingContainer, container);
}
if (clazz is Class) {
for (var type in clazz.mixedInTypes) {
checkAndAddContainer(type.modelElement as InheritingContainer, clazz);
if (container is Class) {
for (var element in container.mixedInElements) {
checkAndAddContainer(element, container);
}
for (var type in clazz.interfaces) {
checkAndAddContainer(type.modelElement as InheritingContainer, clazz);
for (var element in container.interfaceElements) {
checkAndAddContainer(element, container);
}
} else if (container is ExtensionType) {
for (var element in container.interfaceElements) {
checkAndAddContainer(element, container);
}
}
for (var type in clazz.publicInterfaces) {
checkAndAddContainer(type.modelElement as InheritingContainer, clazz);
for (var element in container.publicInterfaceElements) {
checkAndAddContainer(element, container);
}
}

containers.forEach(addImplementor);
containers.forEach(addImplementer);

// [privates] may grow while processing; use a for loop, rather than a
// for-each loop, to avoid concurrent modification errors.
for (var i = 0; i < privates.length; i++) {
addImplementor(privates[i]);
addImplementer(privates[i]);
}
}

Expand Down Expand Up @@ -661,10 +666,10 @@ class PackageGraph with CommentReferable, Nameable, ModelBuilder {
?.linkedName ??
'Object';

/// The set of [Class]es which should _not_ be presented as implementors.
/// The set of [Class]es which should _not_ be presented as implementers.
///
/// Add classes here if they are similar to Interceptor in that they are to be
/// ignored even when they are the implementors of [Inheritable]s, and the
/// 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];
Expand Down
47 changes: 28 additions & 19 deletions test/end2end/model_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1820,8 +1820,8 @@ void main() {

test('Verify inheritance/mixin structure and type inference', () {
expect(
TypeInferenceMixedIn.mixedInTypes
.map<String>((DefinedElementType t) => t.modelElement.name),
TypeInferenceMixedIn.mixedInElements
.map<String>((element) => element.name),
orderedEquals(['GenericMixin']));
expect(
TypeInferenceMixedIn.mixedInTypes.first.typeArguments
Expand Down Expand Up @@ -1983,15 +1983,15 @@ void main() {
});

test('mixins', () {
expect(Apple.mixedInTypes, hasLength(0));
expect(Apple.mixedInElements, hasLength(0));
});

test('mixins private', () {
expect(F.mixedInTypes, hasLength(1));
expect(F.mixedInElements, hasLength(1));
});

test('interfaces', () {
var interfaces = Dog.interfaces;
var interfaces = Dog.interfaceElements;
expect(interfaces, hasLength(2));
expect(interfaces[0].name, 'Cat');
expect(interfaces[1].name, 'E');
Expand Down Expand Up @@ -4399,28 +4399,37 @@ String? topLevelFunction(int param1, bool param2, Cool coolBeans,

test('a class that implements Future<void>', () {
expect(
ImplementsFutureVoid.linkedName,
equals(
'<a href="${htmlBasePlaceholder}fake/ImplementsFutureVoid-class.html">ImplementsFutureVoid</a>'));
ImplementsFutureVoid.linkedName,
equals(
'<a href="${htmlBasePlaceholder}fake/ImplementsFutureVoid-class.html">ImplementsFutureVoid</a>',
),
);
var FutureVoid =
ImplementsFutureVoid.interfaces.firstWhere((c) => c.name == 'Future');
ImplementsFutureVoid.interfaces.firstWhere((e) => e.name == 'Future');
expect(
FutureVoid.linkedName,
equals(
'Future<span class="signature">&lt;<wbr><span class="type-parameter">void</span>&gt;</span>'));
FutureVoid.linkedName,
equals(
'Future<span class="signature">&lt;<wbr><span class="type-parameter">void</span>&gt;</span>',
),
);
});

test('Verify that a mixin with a void type parameter works', () {
expect(
ATypeTakingClassMixedIn.linkedName,
equals(
'<a href="${htmlBasePlaceholder}fake/ATypeTakingClassMixedIn-class.html">ATypeTakingClassMixedIn</a>'));
ATypeTakingClassMixedIn.linkedName,
equals(
'<a href="${htmlBasePlaceholder}fake/ATypeTakingClassMixedIn-class.html">ATypeTakingClassMixedIn</a>',
),
);
var ATypeTakingClassVoid = ATypeTakingClassMixedIn.mixedInTypes
.firstWhere((c) => c.name == 'ATypeTakingClass');
.firstWhere((e) => e.name == 'ATypeTakingClass');
expect(
ATypeTakingClassVoid.linkedName,
equals(
'<a href="${htmlBasePlaceholder}fake/ATypeTakingClass-class.html">ATypeTakingClass</a><span class="signature">&lt;<wbr><span class="type-parameter">void</span>&gt;</span>'));
ATypeTakingClassVoid.linkedName,
equals(
'<a href="${htmlBasePlaceholder}fake/ATypeTakingClass-class.html">ATypeTakingClass</a>'
'<span class="signature">&lt;<wbr><span class="type-parameter">void</span>&gt;</span>',
),
);
});
});

Expand Down
Loading