Many fixes

CHANGELOG.md

@@ -0,0 +1,7 @@
+## 0.1.0
+
+* Split off from [source_gen](https://pub.dartlang.org/packages/source_gen).
+* Add `/* unsafe */` comments to generated output likely to be unsafe.
+* Support (de)serializing values in `Map`.
+* Fix ordering of fields when they are initialized via constructor.
+* Don't use static members when calculating fields to (de)serialize.

analysis_options.yaml

@@ -1,38 +1,40 @@
 analyzer:
- strong-mode: true
- errors:
-   unused_element: error
-   unused_import: error
-   unused_local_variable: error
-   dead_code: error
+  strong-mode: true
+  errors:
+    unused_element: error
+    unused_import: error
+    unused_local_variable: error
+    dead_code: error
 linter:
   rules:
-     # Errors
-     - avoid_empty_else
-     - comment_references
-     - control_flow_in_finally
-     - empty_statements
-     #- hash_and_equals
-     - implementation_imports
-     - test_types_in_equals
-     - throw_in_finally
-     - unrelated_type_equality_checks
-     - valid_regexps
+    # Errors
+    - avoid_empty_else
+    - comment_references
+    - control_flow_in_finally
+    - empty_statements
+    #- hash_and_equals
+    - implementation_imports
+    - test_types_in_equals
+    - throw_in_finally
+    - unrelated_type_equality_checks
+    - valid_regexps
 
-     # Style
-     #- annotate_overrides
-     - avoid_init_to_null
-     - avoid_return_types_on_setters
-     - await_only_futures
-     - camel_case_types
-     - directives_ordering
-     - empty_catches
-     - empty_constructor_bodies
-     - library_names
-     - library_prefixes
-     - non_constant_identifier_names
-     - only_throw_errors
-     - prefer_final_fields
-     - prefer_is_not_empty
-     - slash_for_doc_comments
-     - type_init_formals
+    # Style
+    #- annotate_overrides
+    - avoid_init_to_null
+    - avoid_return_types_on_setters
+    - await_only_futures
+    - camel_case_types
+    - directives_ordering
+    - empty_catches
+    - empty_constructor_bodies
+    - library_names
+    - library_prefixes
+    - non_constant_identifier_names
+    - only_throw_errors
+    - prefer_final_fields
+    - prefer_is_not_empty
+    # Waiting for linter 0.1.33 to land in SDK
+    #- prefer_single_quotes
+    - slash_for_doc_comments
+    - type_init_formals

example/example.dart

@@ -4,8 +4,7 @@
 
 library source_gen.example.example;
 
-import 'package:source_gen/generators/json_serializable.dart';
-import 'package:source_gen/generators/json_literal.dart';
+import 'package:json_serializable/annotations.dart';
 
 part 'example.g.dart';
 

lib/src/json_serializable_generator.dart

@@ -3,15 +3,16 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'dart:async';
+import 'dart:collection';
 
 import 'package:analyzer/dart/element/element.dart';
 import 'package:analyzer/dart/element/type.dart';
 import 'package:analyzer/analyzer.dart';
 import 'package:source_gen/source_gen.dart';
-import 'package:source_gen/src/utils.dart' show friendlyNameForElement;
 
 import 'json_serializable.dart';
 import 'type_helper.dart';
+import 'utils.dart';
 
 // TODO: toJson option to omit null/empty values
 class JsonSerializableGenerator
@@ -55,11 +56,16 @@ class JsonSerializableGenerator
 
     // Get all of the fields that need to be assigned
     // TODO: support overriding the field set with an annotation option
-    var fields = classElement.fields.fold(<String, FieldElement>{},
-        (Map<String, FieldElement> map, field) {
-      map[field.name] = field;
-      return map;
-    });
+    var fieldsList = classElement.fields.where((e) => !e.isStatic).toList();
+
+    // Sort these in the order in which they appear in the class
+    // Sadly, `classElement.fields` puts properties after fields
+    fieldsList.sort((a, b) => a.nameOffset.compareTo(b.nameOffset));
+
+    // Explicitly using `LinkedHashMap` – we want these ordered.
+    var fields = new LinkedHashMap<String, FieldElement>.fromIterable(
+        fieldsList,
+        key: (f) => f.name);
 
     // Get the constructor to use for the factory
 
@@ -201,36 +207,91 @@ class JsonSerializableGenerator
       }
     }
 
-    var isList = false;
+    if (fieldType.isDynamic ||
+        fieldType.isObject ||
+        _stringBoolNumChecker.isAssignableFromType(fieldType)) {
+      return expression;
+    }
+
     if (_coreIterableChecker.isAssignableFromType(fieldType)) {
-      if (_coreListChecker.isAssignableFromType(fieldType)) {
-        isList = true;
-      }
-      var substitute = "v$depth";
-      var subFieldValue = _fieldToJsonMapValue(substitute,
-          _getIterableGenericType(fieldType as InterfaceType), depth + 1);
-
-      // In the case of trivial JSON types (int, String, etc), `subFieldValue`
-      // will be identical to `substitute` – so no explicit mapping is needed.
-      // If they are not equal, then we to write out the substitution.
-      if (subFieldValue != substitute) {
-        // TODO: the type could be imported from a library with a prefix!
-        expression = "${expression}?.map(($substitute) => $subFieldValue)";
-
-        // expression now represents an Iterable (even if it started as a List
-        // ...resetting `isList` to `false`.
-        isList = false;
-      }
+      return _listFieldToJsonMapValue(expression, fieldType, depth);
     }
 
-    if (!isList && _coreIterableChecker.isAssignableFromType(fieldType)) {
-      // Then we need to add `?.toList()
+    if (_coreMapChecker.isAssignableFromType(fieldType)) {
+      return _mapFieldToJsonMapValue(expression, fieldType, depth);
+    }
+
+    return "$expression /* unsafe */";
+  }
+
+  String _listFieldToJsonMapValue(
+      String expression, DartType fieldType, int depth) {
+    assert(_coreListChecker.isAssignableFromType(fieldType));
+
+    // This block will yield a regular list, which works fine for JSON
+    // Although it's possible that child elements may be marked unsafe
+
+    var isList = _coreListChecker.isAssignableFromType(fieldType);
+    var substitute = "v$depth";
+    var subFieldValue = _fieldToJsonMapValue(substitute,
+        _getIterableGenericType(fieldType as InterfaceType), depth + 1);
+
+    // In the case of trivial JSON types (int, String, etc), `subFieldValue`
+    // will be identical to `substitute` – so no explicit mapping is needed.
+    // If they are not equal, then we to write out the substitution.
+    if (subFieldValue != substitute) {
+      // TODO: the type could be imported from a library with a prefix!
+      expression = "${expression}?.map(($substitute) => $subFieldValue)";
+
+      // expression now represents an Iterable (even if it started as a List
+      // ...resetting `isList` to `false`.
+      isList = false;
+    }
+
+    if (!isList) {
+      // If the static type is not a List, generate one.
       expression += "?.toList()";
     }
 
     return expression;
   }
 
+  String _mapFieldToJsonMapValue(
+      String expression, DartType fieldType, int depth) {
+    assert(_coreMapChecker.isAssignableFromType(fieldType));
+    var args = _getTypeArguments(fieldType, _coreMapChecker);
+    assert(args.length == 2);
+
+    var keyArg = args.first;
+    var valueType = args.last;
+
+    // We're not going to handle converting key types at the moment
+    // So the only safe types for key are dynamic/Object/String
+    var safeKey = keyArg.isDynamic ||
+        keyArg.isObject ||
+        _coreStringChecker.isExactlyType(keyArg);
+
+    var safeValue = valueType.isDynamic ||
+        valueType.isObject ||
+        _stringBoolNumChecker.isAssignableFromType(valueType);
+
+    if (safeKey) {
+      if (safeValue) {
+        return expression;
+      }
+
+      var substitute = "v$depth";
+      var subFieldValue =
+          _fieldToJsonMapValue(substitute, valueType, depth + 1);
+
+      return "$expression == null ? null :"
+          "new Map<String, dynamic>.fromIterables("
+          "$expression.keys,"
+          "$expression.values.map(($substitute) => $subFieldValue))";
+    }
+    return "$expression /* unsafe */";
+  }
+
   String _jsonMapAccessToField(String name, FieldElement field,
       {ParameterElement ctorParam}) {
     name = _fieldToJsonMapKey(name, field);
@@ -264,22 +325,56 @@ class JsonSerializableGenerator
         return varExpression;
       }
 
-      var output = "($varExpression as List)?.map(($itemVal) => "
-          "${_writeAccessToJsonValue(itemVal, iterableGenericType, depth: depth+1)}"
-          ")";
+      var output = "($varExpression as List)?.map(($itemVal) => $itemSubVal)";
 
       if (_coreListChecker.isAssignableFromType(searchType)) {
         output += "?.toList()";
       }
 
       return output;
-    }
+    } else if (_coreMapChecker.isAssignableFromType(searchType)) {
+      // Just pass through if
+      //    key:   dynamic, Object, String
+      //    value: dynamic, Object
+      var typeArgs = _getTypeArguments(searchType, _coreMapChecker);
+      assert(typeArgs.length == 2);
+      var keyArg = typeArgs.first;
+      var valueArg = typeArgs.last;
+
+      // We're not going to handle converting key types at the moment
+      // So the only safe types for key are dynamic/Object/String
+      var safeKey = keyArg.isDynamic ||
+          keyArg.isObject ||
+          _coreStringChecker.isExactlyType(keyArg);
+
+      if (!safeKey) {
+        return "$varExpression /* unsafe key type */";
+      }
+
+      // this is the trivial case. Do a runtime cast to the known type of JSON
+      // map values - `Map<String, dynamic>`
+      if (valueArg.isDynamic || valueArg.isObject) {
+        return "$varExpression as Map<String, dynamic>";
+      }
 
-    if (!searchType.isDynamic && !searchType.isObject) {
+      var itemVal = "v$depth";
+      var itemSubVal =
+          _writeAccessToJsonValue(itemVal, valueArg, depth: depth + 1);
+
+      // In this case, we're going to create a new Map with matching reified
+      // types.
+      return "$varExpression == null ? null :"
+          "new Map<String, $valueArg>.fromIterables("
+          "($varExpression as Map).keys,"
+          "($varExpression as Map).values.map(($itemVal) => $itemSubVal))";
+    } else if (searchType.isDynamic || searchType.isObject) {
+      // just return it as-is. We'll hope it's safe.
+      return varExpression;
+    } else if (_stringBoolNumChecker.isAssignableFromType(searchType)) {
       return "$varExpression as $searchType";
     }
 
-    return varExpression;
+    return "$varExpression /* unsafe */";
   }
 }
 
@@ -297,11 +392,14 @@ String _fieldToJsonMapKey(String fieldName, FieldElement field) {
   return fieldName;
 }
 
-DartType _getIterableGenericType(InterfaceType type) {
+DartType _getIterableGenericType(InterfaceType type) =>
+    _getTypeArguments(type, _coreIterableChecker).single;
+
+List<DartType> _getTypeArguments(InterfaceType type, TypeChecker checker) {
   var iterableImplementation =
-      _getImplementationType(type, _coreIterableChecker) as InterfaceType;
+      _getImplementationType(type, checker) as InterfaceType;
 
-  return iterableImplementation.typeArguments.single;
+  return iterableImplementation?.typeArguments;
 }
 
 DartType _getImplementationType(DartType type, TypeChecker checker) {
@@ -313,7 +411,9 @@ DartType _getImplementationType(DartType type, TypeChecker checker) {
         .map((type) => _getImplementationType(type, checker))
         .firstWhere((value) => value != null, orElse: () => null);
 
-    if (match != null) return match;
+    if (match != null) {
+      return match;
+    }
 
     if (type.superclass != null) {
       return _getImplementationType(type.superclass, checker);
@@ -325,3 +425,13 @@ DartType _getImplementationType(DartType type, TypeChecker checker) {
 final _coreIterableChecker = const TypeChecker.fromUrl('dart:core#Iterable');
 
 final _coreListChecker = const TypeChecker.fromUrl('dart:core#List');
+
+final _coreMapChecker = const TypeChecker.fromUrl('dart:core#Map');
+
+final _coreStringChecker = const TypeChecker.fromUrl('dart:core#String');
+
+final _stringBoolNumChecker = new TypeChecker.any([
+  _coreStringChecker,
+  new TypeChecker.fromUrl('dart:core#bool'),
+  new TypeChecker.fromUrl('dart:core#num')
+]);

lib/src/utils.dart

@@ -0,0 +1,39 @@
+// Copyright (c) 2017, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:analyzer/dart/element/element.dart';
+
+// Copied from pkg/source_gen - lib/src/utils.
+String friendlyNameForElement(Element element) {
+  var friendlyName = element.displayName;
+
+  if (friendlyName == null) {
+    throw new ArgumentError(
+        'Cannot get friendly name for $element - ${element.runtimeType}.');
+  }
+
+  var names = <String>[friendlyName];
+  if (element is ClassElement) {
+    names.insert(0, 'class');
+    if (element.isAbstract) {
+      names.insert(0, 'abstract');
+    }
+  }
+  if (element is VariableElement) {
+    names.insert(0, element.type.toString());
+
+    if (element.isConst) {
+      names.insert(0, 'const');
+    }
+
+    if (element.isFinal) {
+      names.insert(0, 'final');
+    }
+  }
+  if (element is LibraryElement) {
+    names.insert(0, 'library');
+  }
+
+  return names.join(' ');
+}