Fix detection of toJson/fromJson in nested types

Author: kevmoo

json_serializable/CHANGELOG.md

@@ -4,6 +4,8 @@
 
 * Small change to how nullable `Map` values are deserialized.
 
+* Improve detection of `toJson` and `fromJson` in nested types.
+
 ## 0.5.4+1
 
 * Fixed a bug introduced in `0.5.4` in some cases where enum values are nested

json_serializable/lib/src/generator_helper.dart

@@ -27,7 +27,7 @@ Future<String> generate(JsonSerializableGenerator generator, Element element,
   }
 
   var classElement = element as ClassElement;
-  var classAnnotation = _valueForAnnotation(annotation);
+  var classAnnotation = valueForAnnotation(annotation);
   var helper = new _GeneratorHelper(generator, classElement, classAnnotation);
   return helper._generate();
 }
@@ -441,13 +441,6 @@ String _nameAccess(FieldElement field) => jsonKeyFor(field).name ?? field.name;
 String _safeNameAccess(FieldElement field) =>
     escapeDartString(_nameAccess(field));
 
-JsonSerializable _valueForAnnotation(ConstantReader annotation) =>
-    new JsonSerializable(
-        createToJson: annotation.read('createToJson').boolValue,
-        createFactory: annotation.read('createFactory').boolValue,
-        nullable: annotation.read('nullable').boolValue,
-        includeIfNull: annotation.read('includeIfNull').boolValue);
-
 InvalidGenerationSourceError _createInvalidGenerationError(
     String targetMember, FieldElement field, UnsupportedTypeError e) {
   var extra = (field.type != e.type) ? ' because of type `${e.type}`' : '';

json_serializable/lib/src/type_helpers/json_helper.dart

@@ -4,8 +4,13 @@
 
 import 'package:analyzer/dart/element/element.dart';
 import 'package:analyzer/dart/element/type.dart';
+
+import 'package:json_annotation/json_annotation.dart';
+import 'package:source_gen/source_gen.dart';
+
 import '../shared_checkers.dart';
 import '../type_helper.dart';
+import '../type_helper_context.dart';
 import '../utils.dart';
 
 class JsonHelper extends TypeHelper {
@@ -17,11 +22,7 @@ class JsonHelper extends TypeHelper {
   /// provided objects.
   @override
   String serialize(DartType targetType, String expression, _) {
-    // TODO(kevmoo): This should be checking for toJson method, but toJson might
-    //   be gone during generation, so we'll have to check for the annotation.
-    // In the mean time, just assume the `canSerialize` logic will work most of
-    //   the time.
-    if (!_canDeserialize(targetType)) {
+    if (!_canSerialize(targetType)) {
       return null;
     }
 
@@ -31,17 +32,30 @@ class JsonHelper extends TypeHelper {
   @override
   String deserialize(
       DartType targetType, String expression, DeserializeContext context) {
-    if (!_canDeserialize(targetType)) {
+    if (targetType is! InterfaceType) {
       return null;
     }
 
-    var classElement = targetType.element as ClassElement;
-    var fromJsonCtor =
-        classElement.constructors.firstWhere((ce) => ce.name == 'fromJson');
-    // TODO: should verify that this type is a valid JSON type...but for now...
-    var asCastType = fromJsonCtor.parameters.first.type;
+    var type = targetType as InterfaceType;
+    var classElement = type.element;
+
+    var fromJsonCtor = classElement.constructors
+        .singleWhere((ce) => ce.name == 'fromJson', orElse: () => null);
 
-    var asCast = asStatement(asCastType);
+    String asCast;
+    if (fromJsonCtor != null) {
+      // TODO: should verify that this type is a valid JSON type...but for now...
+      var asCastType = fromJsonCtor.parameters.first.type;
+      asCast = asStatement(asCastType);
+    } else if (_getAnnotation(type)?.createFactory == true) {
+      if ((context as TypeHelperContext).anyMap) {
+        asCast = ' as Map';
+      } else {
+        asCast = ' as Map<String, dynamic>';
+      }
+    } else {
+      return null;
+    }
 
     // TODO: the type could be imported from a library with a prefix!
     // github.com/dart-lang/json_serializable/issues/19
@@ -51,17 +65,55 @@ class JsonHelper extends TypeHelper {
   }
 }
 
-bool _canDeserialize(DartType type) {
-  if (type is! InterfaceType) return false;
+bool _canSerialize(DartType type) {
+  if (type is InterfaceType) {
+    var toJsonMethod = _getMethod(type, 'toJson');
 
-  var classElement = type.element as ClassElement;
+    if (toJsonMethod != null) {
+      // TODO: validate there are no required parameters
+      return true;
+    }
 
-  for (var ctor in classElement.constructors) {
-    if (ctor.name == 'fromJson') {
-      // TODO: validate that there are the right number and type of arguments
+    if (_getAnnotation(type)?.createToJson == true) {
+      // TODO: consider logging that we're assuming a user will wire up the
+      // generated mixin at some point...
       return true;
     }
   }
-
   return false;
 }
+
+JsonSerializable _getAnnotation(InterfaceType source) {
+  var annotations = const TypeChecker.fromRuntime(JsonSerializable)
+      .annotationsOfExact(source.element, throwOnUnresolved: false)
+      .toList();
+
+  if (annotations.isEmpty) {
+    return null;
+  }
+
+  return valueForAnnotation(new ConstantReader(annotations.single));
+}
+
+MethodElement _getMethod(DartType type, String methodName) {
+  if (type is InterfaceType) {
+    var method = type.element.getMethod(methodName);
+    if (method != null) {
+      return method;
+    }
+
+    var match = [type.interfaces, type.mixins]
+        .expand((e) => e)
+        .map((type) => _getMethod(type, methodName))
+        .firstWhere((value) => value != null, orElse: () => null);
+
+    if (match != null) {
+      return match;
+    }
+
+    if (type.superclass != null) {
+      return _getMethod(type.superclass, methodName);
+    }
+  }
+  return null;
+}

json_serializable/lib/src/utils.dart

@@ -10,8 +10,16 @@ import 'package:analyzer/dart/element/type.dart';
 import 'package:analyzer/src/dart/resolver/inheritance_manager.dart'
     show InheritanceManager;
 
+import 'package:json_annotation/json_annotation.dart';
 import 'package:source_gen/source_gen.dart';
 
+JsonSerializable valueForAnnotation(ConstantReader annotation) =>
+    new JsonSerializable(
+        createToJson: annotation.read('createToJson').boolValue,
+        createFactory: annotation.read('createFactory').boolValue,
+        nullable: annotation.read('nullable').boolValue,
+        includeIfNull: annotation.read('includeIfNull').boolValue);
+
 bool isEnum(DartType targetType) =>
     targetType is InterfaceType && targetType.element.isEnum;
 

json_serializable/test/json_serializable_test.dart

@@ -51,18 +51,22 @@ void main() {
       () => _registerTests(const JsonSerializableGenerator(useWrappers: true)));
 }
 
+Future<String> _runForElementNamed(
+    JsonSerializableGenerator generator, String name) async {
+  var library = new LibraryReader(_compilationUnit.element.library);
+  var element = library.allElements.singleWhere((e) => e.name == name);
+  var annotation = generator.typeChecker.firstAnnotationOf(element);
+  var generated = await generator.generateForAnnotatedElement(
+      element, new ConstantReader(annotation), null);
+
+  var output = _formatter.format(generated);
+  printOnFailure(output);
+  return output;
+}
+
 void _registerTests(JsonSerializableGenerator generator) {
-  Future<String> runForElementNamed(String name) async {
-    var library = new LibraryReader(_compilationUnit.element.library);
-    var element = library.allElements.singleWhere((e) => e.name == name);
-    var annotation = generator.typeChecker.firstAnnotationOf(element);
-    var generated = await generator.generateForAnnotatedElement(
-        element, new ConstantReader(annotation), null);
-
-    var output = _formatter.format(generated);
-    printOnFailure(output);
-    return output;
-  }
+  Future<String> runForElementNamed(String name) =>
+      _runForElementNamed(generator, name);
 
   void expectThrows(String elementName, messageMatcher, [todoMatcher]) {
     todoMatcher ??= isEmpty;
@@ -228,7 +232,20 @@ abstract class _$OrderSerializerMixin {
     test('class with child json-able object', () async {
       var output = await runForElementNamed('ParentObject');
 
-      expect(output, contains('new ChildObject.fromJson'));
+      expect(
+          output,
+          contains("new ChildObject.fromJson(json['child'] "
+              'as Map<String, dynamic>)'));
+    });
+
+    test('class with child json-able object - anyMap', () async {
+      var output = await _runForElementNamed(
+          new JsonSerializableGenerator(
+              anyMap: true, useWrappers: generator.useWrappers),
+          'ParentObject');
+
+      expect(
+          output, contains("new ChildObject.fromJson(json['child'] as Map)"));
     });
 
     test('class with child list of json-able objects', () async {

json_serializable/test/src/json_serializable_test_input.dart

@@ -52,7 +52,7 @@ class FinalFieldsNotSetInCtor {
   FinalFieldsNotSetInCtor();
 }
 
-@JsonSerializable()
+@JsonSerializable(createToJson: false)
 class FromJsonOptionalParameters {
   final ChildWithFromJson child;
 
@@ -74,8 +74,6 @@ class ParentObject {
 class ChildObject {
   int number;
   String str;
-
-  factory ChildObject.fromJson(json) => null;
 }
 
 @JsonSerializable()