Use quiver's concat for iterables in Class construction and eliminate duplicitous array/set creation and sorting (#1847)
diff --git a/lib/src/html/html_generator_instance.dart b/lib/src/html/html_generator_instance.dart
index 134fe5a..3fbe707 100644
--- a/lib/src/html/html_generator_instance.dart
+++ b/lib/src/html/html_generator_instance.dart
@@ -148,17 +148,17 @@
generateProperty(_packageGraph, lib, clazz, property);
}
- for (var property in filterNonDocumented(clazz.propertiesForPages)) {
+ for (var property in filterNonDocumented(clazz.allInstanceFields)) {
if (!property.isCanonical) continue;
generateProperty(_packageGraph, lib, clazz, property);
}
- for (var method in filterNonDocumented(clazz.methodsForPages)) {
+ for (var method in filterNonDocumented(clazz.allInstanceMethods)) {
if (!method.isCanonical) continue;
generateMethod(_packageGraph, lib, clazz, method);
}
- for (var operator in filterNonDocumented(clazz.operatorsForPages)) {
+ for (var operator in filterNonDocumented(clazz.allOperators)) {
if (!operator.isCanonical) continue;
generateMethod(_packageGraph, lib, clazz, operator);
}
@@ -186,17 +186,17 @@
generateProperty(_packageGraph, lib, mixin, property);
}
- for (var property in filterNonDocumented(mixin.propertiesForPages)) {
+ for (var property in filterNonDocumented(mixin.allInstanceFields)) {
if (!property.isCanonical) continue;
generateProperty(_packageGraph, lib, mixin, property);
}
- for (var method in filterNonDocumented(mixin.methodsForPages)) {
+ for (var method in filterNonDocumented(mixin.allInstanceMethods)) {
if (!method.isCanonical) continue;
generateMethod(_packageGraph, lib, mixin, method);
}
- for (var operator in filterNonDocumented(mixin.operatorsForPages)) {
+ for (var operator in filterNonDocumented(mixin.allOperators)) {
if (!operator.isCanonical) continue;
generateMethod(_packageGraph, lib, mixin, operator);
}
@@ -209,13 +209,13 @@
for (var eNum in filterNonDocumented(lib.enums)) {
generateEnum(_packageGraph, lib, eNum);
- for (var property in filterNonDocumented(eNum.propertiesForPages)) {
+ for (var property in filterNonDocumented(eNum.allInstanceFields)) {
generateProperty(_packageGraph, lib, eNum, property);
}
- for (var operator in filterNonDocumented(eNum.operatorsForPages)) {
+ for (var operator in filterNonDocumented(eNum.allOperators)) {
generateMethod(_packageGraph, lib, eNum, operator);
}
- for (var method in filterNonDocumented(eNum.methodsForPages)) {
+ for (var method in filterNonDocumented(eNum.allInstanceMethods)) {
generateMethod(_packageGraph, lib, eNum, method);
}
}
diff --git a/lib/src/model.dart b/lib/src/model.dart
index 86f0137..7dec3eb 100644
--- a/lib/src/model.dart
+++ b/lib/src/model.dart
@@ -374,7 +374,7 @@
Class parentClass =
new ModelElement.fromElement(t.element, packageGraph);
List<Field> possibleFields = [];
- possibleFields.addAll(parentClass.allInstanceProperties);
+ possibleFields.addAll(parentClass.allInstanceFields);
possibleFields.addAll(parentClass.staticProperties);
String fieldName = accessor.name.replaceFirst('=', '');
Field foundField = possibleFields.firstWhere(
@@ -417,7 +417,7 @@
// enclosingCombo always gets set at accessor creation time, somehow, to
// avoid this.
// TODO(jcollins-g): This also doesn't work for private accessors sometimes.
- (enclosingElement as Class).allFields;
+ (enclosingElement as Class).allInstanceFields;
}
assert(_enclosingCombo != null);
}
@@ -598,19 +598,14 @@
List<Method> _allMethods;
List<Operator> _operators;
List<Operator> _inheritedOperators;
- List<Operator> _allOperators;
- final Set<Operator> _genPageOperators = new Set();
List<Method> _inheritedMethods;
List<Method> _staticMethods;
List<Method> _instanceMethods;
- List<Method> _allInstanceMethods;
- final Set<Method> _genPageMethods = new Set();
List<Field> _fields;
List<Field> _staticFields;
List<Field> _constants;
List<Field> _instanceFields;
List<Field> _inheritedProperties;
- List<Field> _allInstanceProperties;
Class(ClassElement element, Library library, PackageGraph packageGraph)
: super(element, library, packageGraph, null) {
@@ -632,17 +627,7 @@
.toList(growable: false);
}
- List<Method> get allInstanceMethods {
- if (_allInstanceMethods != null) return _allInstanceMethods;
- _allInstanceMethods = []
- ..addAll([]
- ..addAll(instanceMethods)
- ..sort(byName))
- ..addAll([]
- ..addAll(inheritedMethods)
- ..sort(byName));
- return _allInstanceMethods;
- }
+ Iterable<Method> get allInstanceMethods => quiverIterables.concat([instanceMethods, inheritedMethods]);
Iterable<Method> get allPublicInstanceMethods =>
filterNonPublic(allInstanceMethods);
@@ -650,48 +635,17 @@
bool get allPublicInstanceMethodsInherited =>
instanceMethods.every((f) => f.isInherited);
- List<Field> get allInstanceProperties {
- if (_allInstanceProperties != null) return _allInstanceProperties;
+ Iterable<Field> get allInstanceFields => quiverIterables.concat([instanceProperties, inheritedProperties]);
- // TODO best way to make this a fixed length list?
- _allInstanceProperties = []
- ..addAll([]
- ..addAll(instanceProperties)
- ..sort(byName))
- ..addAll([]
- ..addAll(inheritedProperties)
- ..sort(byName));
- return _allInstanceProperties;
- }
-
- Iterable<Accessor> get allAccessors {
- return []
- ..addAll(allInstanceProperties.expand((f) {
- List<Accessor> getterSetters = [];
- if (f.hasGetter) getterSetters.add(f.getter);
- if (f.hasSetter) getterSetters.add(f.setter);
- return getterSetters;
- }))
- ..addAll(constants.map<Accessor>((c) => c.getter));
- }
+ Iterable<Accessor> get allAccessors => quiverIterables.concat([allInstanceFields.expand((f) => f.allAccessors), constants.map((c) => c.getter)]);
Iterable<Field> get allPublicInstanceProperties =>
- filterNonPublic(allInstanceProperties);
+ filterNonPublic(allInstanceFields);
bool get allPublicInstancePropertiesInherited =>
allPublicInstanceProperties.every((f) => f.isInherited);
- List<Operator> get allOperators {
- if (_allOperators != null) return _allOperators;
- _allOperators = []
- ..addAll([]
- ..addAll(operators)
- ..sort(byName))
- ..addAll([]
- ..addAll(inheritedOperators)
- ..sort(byName));
- return _allOperators;
- }
+ Iterable<Operator> get allOperators => quiverIterables.concat([operators, inheritedOperators]);
Iterable<Operator> get allPublicOperators => filterNonPublic(allOperators);
@@ -700,7 +654,7 @@
List<Field> get constants {
if (_constants != null) return _constants;
- _constants = allFields.where((f) => f.isConst).toList(growable: false)
+ _constants = _allFields.where((f) => f.isConst).toList(growable: false)
..sort(byName);
return _constants;
@@ -760,7 +714,7 @@
_allModelElements = new List.from(
quiverIterables.concat([
allInstanceMethods,
- allInstanceProperties,
+ allInstanceFields,
allAccessors,
allOperators,
constants,
@@ -861,7 +815,7 @@
List<Method> get inheritedMethods {
if (_inheritedMethods == null) {
- _inheritedMethods = new List<Method>();
+ _inheritedMethods = <Method>[];
Set<String> methodNames = _methods.map((m) => m.element.name).toSet();
Set<ExecutableElement> inheritedMethodElements =
@@ -876,7 +830,6 @@
Method m = new ModelElement.from(e, library, packageGraph,
enclosingClass: this);
_inheritedMethods.add(m);
- _genPageMethods.add(m);
}
_inheritedMethods.sort(byName);
}
@@ -902,7 +855,6 @@
Operator o = new ModelElement.from(e, library, packageGraph,
enclosingClass: this);
_inheritedOperators.add(o);
- _genPageOperators.add(o);
}
_inheritedOperators.sort(byName);
}
@@ -914,7 +866,7 @@
List<Field> get inheritedProperties {
if (_inheritedProperties == null) {
- _inheritedProperties = allFields.where((f) => f.isInherited).toList()
+ _inheritedProperties = _allFields.where((f) => f.isInherited).toList()
..sort(byName);
}
return _inheritedProperties;
@@ -930,8 +882,6 @@
.where((m) => !m.isStatic && !m.isOperator)
.toList(growable: false)
..sort(byName);
-
- _genPageMethods.addAll(_instanceMethods);
return _instanceMethods;
}
@@ -939,7 +889,7 @@
List<Field> get instanceProperties {
if (_instanceFields != null) return _instanceFields;
- _instanceFields = allFields
+ _instanceFields = _allFields
.where((f) => !f.isStatic && !f.isInherited && !f.isConst)
.toList(growable: false)
..sort(byName);
@@ -989,8 +939,6 @@
@override
String get kind => 'class';
- List<Method> get methodsForPages => _genPageMethods.toList(growable: false);
-
List<DefinedElementType> get mixins => _mixins;
Iterable<DefinedElementType> get publicMixins => filterNonPublic(mixins);
@@ -1005,30 +953,11 @@
.cast<Operator>()
.toList(growable: false)
..sort(byName);
- _genPageOperators.addAll(_operators);
-
return _operators;
}
Iterable<Operator> get publicOperators => filterNonPublic(operators);
- List<Operator> get operatorsForPages =>
- new UnmodifiableListView(_genPageOperators.toList());
-
- // TODO: make this method smarter about hierarchies and overrides. Right
- // now, we're creating a flat list. We're not paying attention to where
- // these methods are actually coming from. This might turn out to be a
- // problem if we want to show that info later.
- List<Field> _propertiesForPages;
- List<Field> get propertiesForPages {
- if (_propertiesForPages == null) {
- _propertiesForPages = []
- ..addAll(allInstanceProperties)
- ..sort(byName);
- }
- return _propertiesForPages;
- }
-
List<Method> get staticMethods {
if (_staticMethods != null) return _staticMethods;
@@ -1042,9 +971,8 @@
List<Field> get staticProperties {
if (_staticFields != null) return _staticFields;
- _staticFields = allFields
- .where((f) => f.isStatic)
- .where((f) => !f.isConst)
+ _staticFields = _allFields
+ .where((f) => f.isStatic && !f.isConst)
.toList(growable: false)
..sort(byName);
@@ -1124,7 +1052,9 @@
return __inheritedElements;
}
- List<Field> get allFields {
+ /// Internal only because subclasses are allowed to override how
+ /// these are mapped to [allInheritedFields] and so forth.
+ List<Field> get _allFields {
if (_fields != null) return _fields;
_fields = [];
Set<PropertyAccessorElement> inheritedAccessors = new Set()
@@ -1243,12 +1173,18 @@
return _allMethods;
}
+ List<TypeParameter> _typeParameters;
// a stronger hash?
@override
- List<TypeParameter> get typeParameters => _cls.typeParameters.map((f) {
+ List<TypeParameter> get typeParameters {
+ if (_typeParameters == null) {
+ _typeParameters = _cls.typeParameters.map((f) {
var lib = new Library(f.enclosingElement.library, packageGraph);
return new ModelElement.from(f, lib, packageGraph) as TypeParameter;
}).toList();
+ }
+ return _typeParameters;
+ }
@override
bool operator ==(o) =>
@@ -1595,9 +1531,6 @@
}
@override
- List<Field> get propertiesForPages => allInstanceProperties;
-
- @override
String get kind => 'enum';
}
@@ -1859,6 +1792,12 @@
abstract class GetterSetterCombo implements ModelElement {
Accessor get getter;
+ Iterable<Accessor> get allAccessors sync* {
+ for (Accessor a in [getter, setter]) {
+ if (a != null) yield a;
+ }
+ }
+
Set<String> get comboFeatures {
Set<String> allFeatures = new Set();
if (hasExplicitGetter && hasPublicGetter)
diff --git a/test/model_test.dart b/test/model_test.dart
index a7c7ff5..5b6004f 100644
--- a/test/model_test.dart
+++ b/test/model_test.dart
@@ -982,11 +982,11 @@
ImplementingThingy2 = fakeLibrary.classes
.firstWhere((c) => c.name == 'ImplementingThingy2');
- aImplementingThingy = ImplementingThingy2.allInstanceProperties
+ aImplementingThingy = ImplementingThingy2.allInstanceFields
.firstWhere((m) => m.name == 'aImplementingThingy');
aImplementingThingyMethod = ImplementingThingy2.allInstanceMethods
.firstWhere((m) => m.name == 'aImplementingThingyMethod');
- aImplementingThingyField = ImplementingThingy2.allInstanceProperties
+ aImplementingThingyField = ImplementingThingy2.allInstanceFields
.firstWhere((m) => m.name == 'aImplementingThingyField');
aImplementingThingyAccessor = aImplementingThingyField.getter;
});
@@ -1420,13 +1420,13 @@
fakeLibrary.publicMixins.firstWhere((m) => m.name == 'GenericMixin');
TypeInferenceMixedIn =
classes.firstWhere((c) => c.name == 'TypeInferenceMixedIn');
- overrideByEverything = TypeInferenceMixedIn.allFields
+ overrideByEverything = TypeInferenceMixedIn.allInstanceFields
.firstWhere((f) => f.name == 'overrideByEverything');
- overrideByGenericMixin = TypeInferenceMixedIn.allFields
+ overrideByGenericMixin = TypeInferenceMixedIn.allInstanceFields
.firstWhere((f) => f.name == 'overrideByGenericMixin');
- overrideByBoth = TypeInferenceMixedIn.allFields
+ overrideByBoth = TypeInferenceMixedIn.allInstanceFields
.firstWhere((f) => f.name == 'overrideByBoth');
- overrideByModifierClass = TypeInferenceMixedIn.allFields
+ overrideByModifierClass = TypeInferenceMixedIn.allInstanceFields
.firstWhere((f) => f.name == 'overrideByModifierClass');
});
@@ -1460,10 +1460,10 @@
test(('Verify non-overridden members have right canonical classes'), () {
final Field member =
- TypeInferenceMixedIn.allFields.firstWhere((f) => f.name == 'member');
- final Field modifierMember = TypeInferenceMixedIn.allFields
+ TypeInferenceMixedIn.allInstanceFields.firstWhere((f) => f.name == 'member');
+ final Field modifierMember = TypeInferenceMixedIn.allInstanceFields
.firstWhere((f) => f.name == 'modifierMember');
- final Field mixinMember = TypeInferenceMixedIn.allFields
+ final Field mixinMember = TypeInferenceMixedIn.allInstanceFields
.firstWhere((f) => f.name == 'mixinMember');
expect(member.canonicalEnclosingElement, equals(GenericClass));
expect(modifierMember.canonicalEnclosingElement, equals(ModifierClass));
@@ -1480,22 +1480,22 @@
equals(ModifierClass));
expect(
overrideByEverything.documentationFrom.first,
- equals(GenericClass.allFields
+ equals(GenericClass.allInstanceFields
.firstWhere((f) => f.name == 'overrideByEverything')
.getter));
expect(
overrideByGenericMixin.documentationFrom.first,
- equals(GenericClass.allFields
+ equals(GenericClass.allInstanceFields
.firstWhere((f) => f.name == 'overrideByGenericMixin')
.getter));
expect(
overrideByBoth.documentationFrom.first,
- equals(GenericClass.allFields
+ equals(GenericClass.allInstanceFields
.firstWhere((f) => f.name == 'overrideByBoth')
.getter));
expect(
overrideByModifierClass.documentationFrom.first,
- equals(GenericClass.allFields
+ equals(GenericClass.allInstanceFields
.firstWhere((f) => f.name == 'overrideByModifierClass')
.getter));
});
@@ -1573,7 +1573,7 @@
expect(CatString.hasInstanceProperties, isFalse);
expect(CatString.instanceProperties, isEmpty);
expect(CatString.hasPublicProperties, isTrue);
- expect(CatString.allInstanceProperties, isNotEmpty);
+ expect(CatString.allInstanceFields, isNotEmpty);
});
test('has enclosing element', () {
@@ -2420,7 +2420,7 @@
documentedPartialFieldInSubclassOnly = UnusualProperties.allModelElements
.firstWhere((e) => e.name == 'documentedPartialFieldInSubclassOnly');
- isEmpty = CatString.allInstanceProperties
+ isEmpty = CatString.allInstanceFields
.firstWhere((p) => p.name == 'isEmpty');
dynamicGetter = LongFirstLine.instanceProperties
.firstWhere((p) => p.name == 'dynamicGetter');
@@ -2429,40 +2429,40 @@
lengthX = fakeLibrary.classes
.firstWhere((c) => c.name == 'WithGetterAndSetter')
- .allInstanceProperties
+ .allInstanceFields
.firstWhere((c) => c.name == 'lengthX');
var appleClass =
exLibrary.allClasses.firstWhere((c) => c.name == 'Apple');
sFromApple =
- appleClass.allInstanceProperties.firstWhere((p) => p.name == 's');
+ appleClass.allInstanceFields.firstWhere((p) => p.name == 's');
mFromApple =
- appleClass.allInstanceProperties.singleWhere((p) => p.name == 'm');
+ appleClass.allInstanceFields.singleWhere((p) => p.name == 'm');
mInB = exLibrary.allClasses
.firstWhere((c) => c.name == 'B')
- .allInstanceProperties
+ .allInstanceFields
.firstWhere((p) => p.name == 'm');
autoCompress = exLibrary.allClasses
.firstWhere((c) => c.name == 'B')
- .allInstanceProperties
+ .allInstanceFields
.firstWhere((p) => p.name == 'autoCompress');
ExtraSpecialListLength = fakeLibrary.classes
.firstWhere((c) => c.name == 'SpecialList')
- .allInstanceProperties
+ .allInstanceFields
.firstWhere((f) => f.name == 'length');
aProperty = fakeLibrary.classes
.firstWhere((c) => c.name == 'AClassWithFancyProperties')
- .allInstanceProperties
+ .allInstanceFields
.firstWhere((f) => f.name == 'aProperty');
covariantField = fakeLibrary.classes
.firstWhere((c) => c.name == 'CovariantMemberParams')
- .allInstanceProperties
+ .allInstanceFields
.firstWhere((f) => f.name == 'covariantField');
covariantSetter = fakeLibrary.classes
.firstWhere((c) => c.name == 'CovariantMemberParams')
- .allInstanceProperties
+ .allInstanceFields
.firstWhere((f) => f.name == 'covariantSetter');
});
@@ -2731,7 +2731,7 @@
});
test('if overridden, gets documentation from superclasses', () {
- final doc = classB.allInstanceProperties
+ final doc = classB.allInstanceFields
.firstWhere((p) => p.name == "s")
.getter
.documentation;
@@ -2742,7 +2742,7 @@
"has correct linked return type if the return type is a parameterized typedef",
() {
Class apple = exLibrary.classes.firstWhere((c) => c.name == 'Apple');
- final fieldWithTypedef = apple.allInstanceProperties
+ final fieldWithTypedef = apple.allInstanceFields
.firstWhere((m) => m.name == "fieldWithTypedef");
expect(
fieldWithTypedef.linkedReturnType,
@@ -2908,8 +2908,8 @@
customClassPrivate = fakeLibrary.constants
.firstWhere((c) => c.name == 'CUSTOM_CLASS_PRIVATE');
aStaticConstField =
- Dog.allFields.firstWhere((f) => f.name == 'aStaticConstField');
- aName = Dog.allFields.firstWhere((f) => f.name == 'aName');
+ Dog.constants.firstWhere((f) => f.name == 'aStaticConstField');
+ aName = Dog.constants.firstWhere((f) => f.name == 'aName');
});
test('substrings of the constant values type are not linked (#1535)', () {
