Merge pull request #427 from davidmorgan/fix-generic-inherit

Fix code generation when inherited generic fields are made non-generic.

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.

built_value_generator/lib/src/fields.dart

@@ -3,24 +3,33 @@
 // 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';
 
 /// 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<FieldElement> 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<FieldElement> collectFields(ClassElement element) {
+BuiltList<FieldElement> collectFieldsForType(InterfaceType type) {
   final fields = <FieldElement>[];
   // 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<InterfaceType>.from(element.interfaces)
-    ..addAll(element.mixins)
-    ..forEach((interface) => fields.addAll(collectFields(interface.element)));
+  new Set<InterfaceType>.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<FieldElement> collectFields(ClassElement element) {
   return new BuiltList<FieldElement>.build((b) => b
     ..addAll(fieldSet)
     ..where((field) =>
-        element.lookUpInheritedConcreteGetter(field.name, element.library) ==
-        null));
+        type.lookUpInheritedGetter(field.name, thisType: false)?.isAbstract ??
+        true));
+}
+
+BuiltList<FieldElement> _fieldElementsForType(InterfaceType type) {
+  final result = new ListBuilder<FieldElement>();
+  for (final accessor in type.accessors) {
+    if (accessor.isSetter) continue;
+    result.add(accessor.variable as FieldElement);
+  }
+  return result.build();
 }

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.

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.

end_to_end_test/lib/generics.dart

@@ -95,3 +95,17 @@ abstract class CustomBuilderGenericValueBuilder<T>
       _$CustomBuilderGenericValueBuilder<T>;
   CustomBuilderGenericValueBuilder._();
 }
+
+abstract class Generic<T> {
+  T get value;
+}
+
+abstract class ConcreteGeneric
+    implements Built<ConcreteGeneric, ConcreteGenericBuilder>, Generic<int> {
+  static Serializer<ConcreteGeneric> get serializer =>
+      _$concreteGenericSerializer;
+
+  factory ConcreteGeneric([updates(ConcreteGenericBuilder b)]) =
+      _$ConcreteGeneric;
+  ConcreteGeneric._();
+}

end_to_end_test/lib/serializers.dart

@@ -27,6 +27,7 @@ part 'serializers.g.dart';
   CompoundValue,
   CompoundValueExplicitNoNesting,
   CompoundValueNoNesting,
+  ConcreteGeneric,
   EnumWithInt,
   FieldDiscoveryValue,
   Fish,

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);
+    });
+  });
 }

example/lib/collections.g.dart

@@ -1,4 +1,5 @@
 // This is generated code; please do not modify by hand.
+
 part of collections;
 
 // **************************************************************************

example/lib/enums.g.dart

@@ -1,4 +1,5 @@
 // This is generated code; please do not modify by hand.
+
 part of enums;
 
 // **************************************************************************

example/lib/generics.g.dart

@@ -1,4 +1,5 @@
 // This is generated code; please do not modify by hand.
+
 part of generics;
 
 // **************************************************************************

example/lib/interfaces.g.dart

@@ -1,4 +1,5 @@
 // This is generated code; please do not modify by hand.
+
 part of interfaces;
 
 // **************************************************************************

example/lib/polymorphism.g.dart

@@ -1,4 +1,5 @@
 // This is generated code; please do not modify by hand.
+
 part of polymorphism;
 
 // **************************************************************************

example/lib/serializers.g.dart

@@ -1,4 +1,5 @@
 // This is generated code; please do not modify by hand.
+
 part of serializers;
 
 // **************************************************************************

example/lib/values.g.dart

@@ -1,4 +1,5 @@
 // This is generated code; please do not modify by hand.
+
 part of values;
 
 // **************************************************************************