Better error unknown (#16)

* Improve error tests
* Better error handling when hitting unknown fields
* Update Dart SDK dependency

Author: kevmoo

.travis.yml

@@ -3,17 +3,10 @@ sudo: false
 dart:
   - dev
   - stable
-  - 1.22.1
 dart_task:
   - test
   - dartfmt
   - dartanalyzer
-matrix:
-  exclude:
-    - dart: 1.22.1
-      dart_task: dartfmt
-    - dart: 1.22.1
-      dart_task: dartanalyzer
 
 # Only building master means that we don't run two builds for each pull request.
 branches:

CHANGELOG.md

@@ -1,3 +1,8 @@
+## 0.1.1
+
+* Fail generation when undefined types are encountered.
+  Throw a helpful error.
+
 ## 0.1.0+1
 
 * Fix homepage in `pubspec.yaml`.

lib/src/json_serializable_generator.dart

@@ -58,6 +58,17 @@ class JsonSerializableGenerator
     // TODO: support overriding the field set with an annotation option
     var fieldsList = classElement.fields.where((e) => !e.isStatic).toList();
 
+    var undefinedFields =
+        fieldsList.where((fe) => fe.type.isUndefined).toList();
+    if (undefinedFields.isNotEmpty) {
+      var description =
+          undefinedFields.map((fe) => "`${fe.displayName}`").join(', ');
+
+      throw new InvalidGenerationSourceError(
+          'At least one field has an invalid type: $description.',
+          todo: 'Check names and imports.');
+    }
+
     // 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));
@@ -151,6 +162,19 @@ class JsonSerializableGenerator
       fieldsToSet.remove(arg.name);
     }
 
+    var undefinedArgs = [ctorArguments, ctorNamedArguments]
+        .expand((l) => l)
+        .where((pe) => pe.type.isUndefined)
+        .toList();
+    if (undefinedArgs.isNotEmpty) {
+      var description =
+          undefinedArgs.map((fe) => "`${fe.displayName}`").join(', ');
+
+      throw new InvalidGenerationSourceError(
+          'At least one constructor argument has an invalid type: $description.',
+          todo: 'Check names and imports.');
+    }
+
     // these are fields to skip – now to find them
     var finalFields =
         fieldsToSet.values.where((field) => field.isFinal).toSet();
@@ -296,15 +320,13 @@ class JsonSerializableGenerator
       {ParameterElement ctorParam}) {
     name = _fieldToJsonMapKey(name, field);
     var result = "json['$name']";
-    return _writeAccessToJsonValue(result, field.type, ctorParam: ctorParam);
+
+    var targetType = ctorParam?.type ?? field.type;
+    return _writeAccessToJsonValue(result, targetType);
   }
 
   String _writeAccessToJsonValue(String varExpression, DartType searchType,
-      {ParameterElement ctorParam, int depth: 0}) {
-    if (ctorParam != null) {
-      searchType = ctorParam.type as InterfaceType;
-    }
-
+      {int depth: 0}) {
     for (var helper in _typeHelpers) {
       if (helper.canDeserialize(searchType)) {
         return "$varExpression == null ? null : "

pubspec.yaml

@@ -1,10 +1,10 @@
 name: json_serializable
-version: 0.1.0+1
+version: 0.1.1-dev
 author: Dart Team <misc@dartlang.org>
 description: Generates utilities to aid in serializing to/from JSON.
 homepage: https://github.com/dart-lang/json_serializable
 environment:
-  sdk: '>=1.22.1 <2.0.0-dev.infinity'
+  sdk: '>=1.24.0 <2.0.0-dev.infinity'
 dependencies:
   analyzer: '>=0.29.10 <0.31.0'
   build: ^0.9.0

test/json_serializable_test.dart

@@ -9,8 +9,6 @@ import 'dart:async';
 
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/element/element.dart';
-import 'package:analyzer/src/generated/source.dart';
-import 'package:analyzer/src/generated/source_io.dart';
 import 'package:analyzer/src/string_source.dart';
 import 'package:json_serializable/json_serializable.dart';
 import 'package:path/path.dart' as p;
@@ -25,17 +23,43 @@ void main() {
     test('const field', () async {
       var element = await _getClassForCodeString('theAnswer');
 
-      expect(_generator.generate(element, null),
-          throwsInvalidGenerationSourceError);
-      // TODO: validate the properties on the thrown error
+      expect(
+          _generator.generate(element, null),
+          throwsInvalidGenerationSourceError(
+              'Generator cannot target `const dynamic theAnswer`.',
+              'Remove the JsonSerializable annotation from `const dynamic theAnswer`.'));
     });
 
     test('method', () async {
       var element = await _getClassForCodeString('annotatedMethod');
 
-      expect(_generator.generate(element, null),
-          throwsInvalidGenerationSourceError);
-      // TODO: validate the properties on the thrown error
+      expect(
+          _generator.generate(element, null),
+          throwsInvalidGenerationSourceError(
+              'Generator cannot target `annotatedMethod`.',
+              'Remove the JsonSerializable annotation from `annotatedMethod`.'));
+    });
+  });
+
+  group('unknown types', () {
+    test('in constructor arguments', () async {
+      var element = await _getClassForCodeString('UnknownCtorParamType');
+
+      expect(
+          _generator.generate(element, null),
+          throwsInvalidGenerationSourceError(
+              'At least one constructor argument has an invalid type: `number`.',
+              'Check names and imports.'));
+    });
+
+    test('in fields', () async {
+      var element = await _getClassForCodeString('UnknownFieldType');
+
+      expect(
+          _generator.generate(element, null),
+          throwsInvalidGenerationSourceError(
+              'At least one field has an invalid type: `number`.',
+              'Check names and imports.'));
     });
   });
 
@@ -126,14 +150,14 @@ Future<Element> _getClassForCodeString(String name) async {
 }
 
 Future<CompilationUnit> _getCompilationUnitForString(String projectPath) async {
-  Source source = new StringSource(_testSource, 'test content');
+  var source = new StringSource(_testSource, 'test content');
 
   var foundFiles = await getDartFiles(projectPath,
       searchList: [p.join(getPackagePath(), 'test', 'test_files')]);
 
   var context = await getAnalysisContextForProjectPath(projectPath, foundFiles);
 
-  LibraryElement libElement = context.computeLibraryElement(source);
+  var libElement = context.computeLibraryElement(source);
   return context.resolveCompilationUnit(source, libElement);
 }
 
@@ -207,4 +231,16 @@ class ParentObjectWithDynamicChildren {
   String str;
   List<dynamic> children;
 }
+
+@JsonSerializable()
+class UnknownCtorParamType {
+  int number
+  
+  UnknownCtorParamType(Bob number) : this.number = number;
+}
+
+@JsonSerializable()
+class UnknownFieldType {
+  Bob number
+}
 ''';

test/test_utils.dart

@@ -27,17 +27,22 @@ String getPackagePath() {
   return _packagePathCache;
 }
 
-final Matcher throwsInvalidGenerationSourceError =
-    throwsA(isInvalidGenerationSourceError);
+Matcher throwsInvalidGenerationSourceError(messageMatcher, todoMatcher) =>
+    throwsA(allOf(
+        new isInstanceOf<InvalidGenerationSourceError>(),
+        new FeatureMatcher<InvalidGenerationSourceError>(
+            'todo', (e) => e.todo, todoMatcher),
+        new FeatureMatcher<InvalidGenerationSourceError>(
+            'message', (e) => e.message, messageMatcher)));
 
-const Matcher isInvalidGenerationSourceError =
-    const _InvalidGenerationSourceError();
+// TODO(kevmoo) add this to pkg/matcher – is nice!
+class FeatureMatcher<T> extends CustomMatcher {
+  final dynamic Function(T value) _feature;
 
-class _InvalidGenerationSourceError extends TypeMatcher {
-  const _InvalidGenerationSourceError() : super("InvalidGenerationSourceError");
+  FeatureMatcher(String name, this._feature, matcher)
+      : super("`$name`", "`$name`", matcher);
 
-  @override
-  bool matches(item, Map matchState) => item is InvalidGenerationSourceError;
+  featureValueOf(covariant T actual) => _feature(actual);
 }
 
 /// Returns all of the declarations in [unit], including [unit] as the first