diff --git a/CHANGELOG.md b/CHANGELOG.md index 940903cf..496b497f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +# 5.4.6 + +- Fix code generation when inherited generic fields are made non-generic. + # 5.4.5 - Improve error message on failure to deserialize. diff --git a/built_value_generator/lib/src/fields.dart b/built_value_generator/lib/src/fields.dart index a6408eae..ffe8edd6 100644 --- a/built_value_generator/lib/src/fields.dart +++ b/built_value_generator/lib/src/fields.dart @@ -3,6 +3,7 @@ // license that can be found in the LICENSE file. import 'dart:collection'; + import 'package:analyzer/dart/element/element.dart'; import 'package:analyzer/dart/element/type.dart'; import 'package:built_collection/built_collection.dart'; @@ -10,17 +11,25 @@ import 'package:built_collection/built_collection.dart'; /// Gets fields, including from interfaces. Fields from interfaces are only /// returned if they are not also implemented by a mixin. /// -/// If a field is overriden then just the closest (overriding) field is +/// If a field is overridden then just the closest (overriding) field is +/// returned. +BuiltList collectFields(ClassElement element) => + collectFieldsForType(element.type); + +/// Gets fields, including from interfaces. Fields from interfaces are only +/// returned if they are not also implemented by a mixin. +/// +/// If a field is overridden then just the closest (overriding) field is /// returned. -BuiltList collectFields(ClassElement element) { +BuiltList collectFieldsForType(InterfaceType type) { final fields = []; // Add fields from this class before interfaces, so they're added to the set // first below. Re-added fields from interfaces are ignored. - fields.addAll(element.fields); + fields.addAll(_fieldElementsForType(type)); - new Set.from(element.interfaces) - ..addAll(element.mixins) - ..forEach((interface) => fields.addAll(collectFields(interface.element))); + new Set.from(type.interfaces) + ..addAll(type.mixins) + ..forEach((interface) => fields.addAll(collectFieldsForType(interface))); // Overridden fields have multiple declarations, so deduplicate by adding // to a set that compares on field name. @@ -33,6 +42,15 @@ BuiltList collectFields(ClassElement element) { return new BuiltList.build((b) => b ..addAll(fieldSet) ..where((field) => - element.lookUpInheritedConcreteGetter(field.name, element.library) == - null)); + type.lookUpInheritedGetter(field.name, thisType: false)?.isAbstract ?? + true)); +} + +BuiltList _fieldElementsForType(InterfaceType type) { + final result = new ListBuilder(); + for (final accessor in type.accessors) { + if (accessor.isSetter) continue; + result.add(accessor.variable as FieldElement); + } + return result.build(); } diff --git a/built_value_generator/lib/src/serializer_source_field.dart b/built_value_generator/lib/src/serializer_source_field.dart index dffe873f..ace565b9 100644 --- a/built_value_generator/lib/src/serializer_source_field.dart +++ b/built_value_generator/lib/src/serializer_source_field.dart @@ -71,11 +71,23 @@ abstract class SerializerSourceField /// The [type] plus any import prefix. @memoized - String get typeWithPrefix => - (element.getter.computeNode() as MethodDeclaration) - ?.returnType - ?.toString() ?? - 'dynamic'; + String get typeWithPrefix { + final typeFromAst = (element.getter.computeNode() as MethodDeclaration) + ?.returnType + ?.toString() ?? + 'dynamic'; + final typeFromElement = type; + + // If the type is a function, we can't use the element result; it is + // formatted incorrectly. + if (typeFromElement.contains('(')) return typeFromAst; + + // If the type does not have an import prefix, prefer the element result. + // It handles inherited generics correctly. + if (!typeFromAst.contains('.')) return typeFromElement; + + return typeFromAst; + } /// Returns the type with import prefix if the compilation unit matches, /// otherwise the type with no import prefix. diff --git a/built_value_generator/lib/src/value_source_field.dart b/built_value_generator/lib/src/value_source_field.dart index 4f5a3252..3369bc4e 100644 --- a/built_value_generator/lib/src/value_source_field.dart +++ b/built_value_generator/lib/src/value_source_field.dart @@ -46,11 +46,23 @@ abstract class ValueSourceField /// The [type] plus any import prefix. @memoized - String get typeWithPrefix => - (element.getter.computeNode() as MethodDeclaration) - ?.returnType - ?.toString() ?? - 'dynamic'; + String get typeWithPrefix { + final typeFromAst = (element.getter.computeNode() as MethodDeclaration) + ?.returnType + ?.toString() ?? + 'dynamic'; + final typeFromElement = type; + + // If the type is a function, we can't use the element result; it is + // formatted incorrectly. + if (typeFromElement.contains('(')) return typeFromAst; + + // If the type does not have an import prefix, prefer the element result. + // It handles inherited generics correctly. + if (!typeFromAst.contains('.')) return typeFromElement; + + return typeFromAst; + } /// Returns the type with import prefix if the compilation unit matches, /// otherwise the type with no import prefix. diff --git a/end_to_end_test/lib/generics.dart b/end_to_end_test/lib/generics.dart index dd55e5be..2bac7587 100644 --- a/end_to_end_test/lib/generics.dart +++ b/end_to_end_test/lib/generics.dart @@ -95,3 +95,17 @@ abstract class CustomBuilderGenericValueBuilder _$CustomBuilderGenericValueBuilder; CustomBuilderGenericValueBuilder._(); } + +abstract class Generic { + T get value; +} + +abstract class ConcreteGeneric + implements Built, Generic { + static Serializer get serializer => + _$concreteGenericSerializer; + + factory ConcreteGeneric([updates(ConcreteGenericBuilder b)]) = + _$ConcreteGeneric; + ConcreteGeneric._(); +} diff --git a/end_to_end_test/lib/generics.g.dart b/end_to_end_test/lib/generics.g.dart index d2d9b2e8..ef3c030f 100644 --- a/end_to_end_test/lib/generics.g.dart +++ b/end_to_end_test/lib/generics.g.dart @@ -24,6 +24,8 @@ Serializer _$genericContainerSerializer = new _$GenericContainerSerializer(); Serializer _$nestedGenericContainerSerializer = new _$NestedGenericContainerSerializer(); +Serializer _$concreteGenericSerializer = + new _$ConcreteGenericSerializer(); class _$GenericValueSerializer implements StructuredSerializer { @override @@ -308,6 +310,46 @@ class _$NestedGenericContainerSerializer } } +class _$ConcreteGenericSerializer + implements StructuredSerializer { + @override + final Iterable types = const [ConcreteGeneric, _$ConcreteGeneric]; + @override + final String wireName = 'ConcreteGeneric'; + + @override + Iterable serialize(Serializers serializers, ConcreteGeneric object, + {FullType specifiedType: FullType.unspecified}) { + final result = [ + 'value', + serializers.serialize(object.value, specifiedType: const FullType(int)), + ]; + + return result; + } + + @override + ConcreteGeneric deserialize(Serializers serializers, Iterable serialized, + {FullType specifiedType: FullType.unspecified}) { + final result = new ConcreteGenericBuilder(); + + final iterator = serialized.iterator; + while (iterator.moveNext()) { + final key = iterator.current as String; + iterator.moveNext(); + final dynamic value = iterator.current; + switch (key) { + case 'value': + result.value = serializers.deserialize(value, + specifiedType: const FullType(int)) as int; + break; + } + } + + return result.build(); + } +} + class _$GenericValue extends GenericValue { @override final T value; @@ -878,3 +920,79 @@ class _$CustomBuilderGenericValueBuilder return _$result; } } + +class _$ConcreteGeneric extends ConcreteGeneric { + @override + final int value; + + factory _$ConcreteGeneric([void updates(ConcreteGenericBuilder b)]) => + (new ConcreteGenericBuilder()..update(updates)).build(); + + _$ConcreteGeneric._({this.value}) : super._() { + if (value == null) + throw new BuiltValueNullFieldError('ConcreteGeneric', 'value'); + } + + @override + ConcreteGeneric rebuild(void updates(ConcreteGenericBuilder b)) => + (toBuilder()..update(updates)).build(); + + @override + ConcreteGenericBuilder toBuilder() => + new ConcreteGenericBuilder()..replace(this); + + @override + bool operator ==(dynamic other) { + if (identical(other, this)) return true; + if (other is! ConcreteGeneric) return false; + return value == other.value; + } + + @override + int get hashCode { + return $jf($jc(0, value.hashCode)); + } + + @override + String toString() { + return (newBuiltValueToStringHelper('ConcreteGeneric')..add('value', value)) + .toString(); + } +} + +class ConcreteGenericBuilder + implements Builder { + _$ConcreteGeneric _$v; + + int _value; + int get value => _$this._value; + set value(int value) => _$this._value = value; + + ConcreteGenericBuilder(); + + ConcreteGenericBuilder get _$this { + if (_$v != null) { + _value = _$v.value; + _$v = null; + } + return this; + } + + @override + void replace(ConcreteGeneric other) { + if (other == null) throw new ArgumentError.notNull('other'); + _$v = other as _$ConcreteGeneric; + } + + @override + void update(void updates(ConcreteGenericBuilder b)) { + if (updates != null) updates(this); + } + + @override + _$ConcreteGeneric build() { + final _$result = _$v ?? new _$ConcreteGeneric._(value: value); + replace(_$result); + return _$result; + } +} diff --git a/end_to_end_test/lib/serializers.dart b/end_to_end_test/lib/serializers.dart index 2a7f7e54..5b48434c 100644 --- a/end_to_end_test/lib/serializers.dart +++ b/end_to_end_test/lib/serializers.dart @@ -27,6 +27,7 @@ part 'serializers.g.dart'; CompoundValue, CompoundValueExplicitNoNesting, CompoundValueNoNesting, + ConcreteGeneric, EnumWithInt, FieldDiscoveryValue, Fish, diff --git a/end_to_end_test/lib/serializers.g.dart b/end_to_end_test/lib/serializers.g.dart index 6d18a468..9a8d44ec 100644 --- a/end_to_end_test/lib/serializers.g.dart +++ b/end_to_end_test/lib/serializers.g.dart @@ -23,6 +23,7 @@ Serializers _$serializers = (new Serializers().toBuilder() ..add(CompoundValue.serializer) ..add(CompoundValueExplicitNoNesting.serializer) ..add(CompoundValueNoNesting.serializer) + ..add(ConcreteGeneric.serializer) ..add(DiscoverableValue.serializer) ..add(EnumWithInt.serializer) ..add(FieldDiscoveryValue.serializer) diff --git a/end_to_end_test/test/generics_serializer_test.dart b/end_to_end_test/test/generics_serializer_test.dart index 06ae5365..182e73f2 100644 --- a/end_to_end_test/test/generics_serializer_test.dart +++ b/end_to_end_test/test/generics_serializer_test.dart @@ -372,4 +372,17 @@ void main() { data); }); }); + + group('ConcreteGeneric with unknown specifiedType', () { + final data = new ConcreteGeneric((b) => b..value = 1); + final serialized = ['ConcreteGeneric', 'value', 1]; + + test('can be serialized', () { + expect(serializers.serialize(data), serialized); + }); + + test('can be deserialized', () { + expect(serializers.deserialize(serialized), data); + }); + }); } diff --git a/example/lib/collections.g.dart b/example/lib/collections.g.dart index 4e06d140..1e201be9 100644 --- a/example/lib/collections.g.dart +++ b/example/lib/collections.g.dart @@ -1,4 +1,5 @@ // This is generated code; please do not modify by hand. + part of collections; // ************************************************************************** diff --git a/example/lib/enums.g.dart b/example/lib/enums.g.dart index 0a465338..cd16b3ca 100644 --- a/example/lib/enums.g.dart +++ b/example/lib/enums.g.dart @@ -1,4 +1,5 @@ // This is generated code; please do not modify by hand. + part of enums; // ************************************************************************** diff --git a/example/lib/generics.g.dart b/example/lib/generics.g.dart index 81d58d18..9f6452ca 100644 --- a/example/lib/generics.g.dart +++ b/example/lib/generics.g.dart @@ -1,4 +1,5 @@ // This is generated code; please do not modify by hand. + part of generics; // ************************************************************************** diff --git a/example/lib/interfaces.g.dart b/example/lib/interfaces.g.dart index 5aec42b7..929161e7 100644 --- a/example/lib/interfaces.g.dart +++ b/example/lib/interfaces.g.dart @@ -1,4 +1,5 @@ // This is generated code; please do not modify by hand. + part of interfaces; // ************************************************************************** diff --git a/example/lib/polymorphism.g.dart b/example/lib/polymorphism.g.dart index fc2b4ef1..54bd78ef 100644 --- a/example/lib/polymorphism.g.dart +++ b/example/lib/polymorphism.g.dart @@ -1,4 +1,5 @@ // This is generated code; please do not modify by hand. + part of polymorphism; // ************************************************************************** diff --git a/example/lib/serializers.g.dart b/example/lib/serializers.g.dart index 17658e12..60f80e74 100644 --- a/example/lib/serializers.g.dart +++ b/example/lib/serializers.g.dart @@ -1,4 +1,5 @@ // This is generated code; please do not modify by hand. + part of serializers; // ************************************************************************** diff --git a/example/lib/values.g.dart b/example/lib/values.g.dart index 2999d999..474b9ba5 100644 --- a/example/lib/values.g.dart +++ b/example/lib/values.g.dart @@ -1,4 +1,5 @@ // This is generated code; please do not modify by hand. + part of values; // **************************************************************************