Refactor containers to better reflect current Dart behavior #2770
Conversation
| @@ -0,0 +1,617 @@ | |||
| // Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file | |||
| import 'package:dartdoc/src/quiver.dart' as quiver; | ||
| import 'package:meta/meta.dart'; | ||
|
|
||
| /// An mixin to build an [InheritingContainer] capable of being constructed |
| .map((e) => ModelElement.from(e, library, packageGraph) as Constructor); | ||
|
|
||
| @override | ||
| bool get hasPublicConstructors => publicConstructors.isNotEmpty; |
There was a problem hiding this comment.
I see that publicConstructors is not memoized, so this call may be expensive if made repeatedly. What's funny is that publicConstructorsSorted is memoized, so using that one instead of publicConstructors would be cheaper on successive calls... not sure if there is an AI here; maybe hasPublicConstructors is not called repeatedly...
There was a problem hiding this comment.
hmmm. all the has* calls tend to be used a fair bit, but generally only once or at most twice per object. This one is only used once; the calls that are part of hasModifiers are used twice. I think I will just punt on this as if I mess with this now I'll need to do it again with the NNBD pass as the implementation is different. But there may be some other nearby areas where we could gain from caching the lists in a smarter way.
There was a problem hiding this comment.
I decided to cache constructors -- that is used in a few places, and will help.
| DefinedElementType get supertype => _supertype; | ||
|
|
||
| @Deprecated('Field intended to be final; setter will be removed as early as ' | ||
| 'Dartdoc 1.0.0') |
There was a problem hiding this comment.
Perhaps now is a good time to make final?
|
|
||
| List<TypeParameter> _typeParameters; | ||
|
|
||
| // a stronger hash? |
There was a problem hiding this comment.
this looks like leftovers from previous cut and paste refactoring adventures. removing
This is essentially a continuation and mop up after #2206. The PR extracts a parent class,
InheritingContainer, fromClassforContainers that participate in inheritance. It also extracts a mixin,Constructable, for containers that can have programmer referable constructors, a mixinTypeImplementingfor containers that can implement types and be implemented, and a mixinMixedInTypesfor containers that can mix-in mixins.This is because:
Extensionsare really a container that doesn't do any of these things and so needs a different common parent.Enumsparticipate in inheritance (e.g.toString()) but can not be implemented nor implement other things, nor can they be constructed.Mixins can implement types and participate in inheritance but are not constructable nor can they mix in mixins.Classes do all of these things including mixing in mixins.This is a breaking change as it changes the templates to no longer allow trying to render Mixin constructors, you can no longer refer to the constructor of a Mixin in comment references, and some useless things the templates were doing are now deleted and can no longer be attempted in custom templates, either.