Skip to content

feature: allow undefined types as long as they have a converter #243

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 21, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 2 additions & 17 deletions json_serializable/lib/src/decode_helper.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

import 'package:analyzer/dart/element/element.dart';
import 'package:json_annotation/json_annotation.dart';
import 'package:source_gen/source_gen.dart';

import 'helper_core.dart';
import 'json_literal_generator.dart';
Expand Down Expand Up @@ -247,8 +246,8 @@ _ConstructorData _writeConstructorInvocation(
usedCtorParamsAndFields.add(arg.name);
}

_validateConstructorArguments(
classElement, constructorArguments.followedBy(namedConstructorArguments));
warnUndefinedElements(
constructorArguments.followedBy(namedConstructorArguments));

// fields that aren't already set by the constructor and that aren't final
var remainingFieldsForInvocationBody =
Expand Down Expand Up @@ -291,17 +290,3 @@ class _ConstructorData {
_ConstructorData(
this.content, this.fieldsToSet, this.usedCtorParamsAndFields);
}

void _validateConstructorArguments(
ClassElement element, Iterable<ParameterElement> constructorArguments) {
var undefinedArgs =
constructorArguments.where((pe) => pe.type.isUndefined).toList();
if (undefinedArgs.isNotEmpty) {
var description = undefinedArgs.map((fe) => '`${fe.name}`').join(', ');

throw new InvalidGenerationSourceError(
'At least one constructor argument has an invalid type: $description.',
todo: 'Check names and imports.',
element: element);
}
}
33 changes: 27 additions & 6 deletions json_serializable/lib/src/helper_core.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// BSD-style license that can be found in the LICENSE file.

import 'package:analyzer/dart/element/element.dart';

import 'package:build/build.dart';
import 'package:json_annotation/json_annotation.dart';
import 'package:source_gen/source_gen.dart';

Expand Down Expand Up @@ -46,13 +46,24 @@ abstract class HelperCore {

InvalidGenerationSourceError createInvalidGenerationError(
String targetMember, FieldElement field, UnsupportedTypeError e) {
var extra = (field.type != e.type) ? ' because of type `${e.type}`' : '';
var message = 'Could not generate `$targetMember` code for `${field.name}`';

var todo = 'Make sure all of the types are serializable.';

var message = 'Could not generate `$targetMember` code for '
'`${field.name}`$extra.\n${e.reason}';
if (e.type.isUndefined) {
message = '$message because the type is undefined.';
todo = "Check your imports. If you're trying to generate code for a "
'Platform-provided type, you may have to specify a custom '
'`$targetMember` in the associated `@JsonKey` annotation.';
} else {
if (field.type != e.type) {
message = '$message because of type `${e.type}`';
}

message = '$message.\n${e.reason}';
}

return new InvalidGenerationSourceError(message,
todo: 'Make sure all of the types are serializable.', element: field);
return new InvalidGenerationSourceError(message, todo: todo, element: field);
}

/// Returns a [String] representing the type arguments that exist on
Expand Down Expand Up @@ -86,3 +97,13 @@ String genericClassArguments(ClassElement element, bool withConstraints) {
.join(', ');
return '<$values>';
}

void warnUndefinedElements(Iterable<VariableElement> elements) {
for (var element in elements.where((fe) => fe.type.isUndefined)) {
var span = spanForElement(element);
log.warning('''
This element has an undefined type. It may causes issues when generated code.
${span.start.toolString}
${span.highlight()}''');
}
}
10 changes: 1 addition & 9 deletions json_serializable/lib/src/json_serializable_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -247,15 +247,7 @@ Set<FieldElement> _createSortedFieldSet(ClassElement element) {
}
}

var undefinedFields = fieldsList.where((fe) => fe.type.isUndefined).toList();
if (undefinedFields.isNotEmpty) {
var description = undefinedFields.map((fe) => '`${fe.name}`').join(', ');

throw new InvalidGenerationSourceError(
'At least one field has an invalid type: $description.',
todo: 'Check names and imports.',
element: undefinedFields.first);
}
warnUndefinedElements(fieldsList);

// Sort these in the order in which they appear in the class
// Sadly, `classElement.fields` puts properties after fields
Expand Down
6 changes: 6 additions & 0 deletions json_serializable/lib/src/type_helpers/value_helper.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ class ValueHelper extends TypeHelper {
@override
String serialize(
DartType targetType, String expression, SerializeContext context) {
if (targetType.isUndefined) {
return null;
}
if (targetType.isDynamic ||
targetType.isObject ||
simpleJsonTypeChecker.isAssignableFromType(targetType)) {
Expand All @@ -26,6 +29,9 @@ class ValueHelper extends TypeHelper {
@override
String deserialize(
DartType targetType, String expression, DeserializeContext context) {
if (targetType.isUndefined) {
return null;
}
if (targetType.isDynamic || targetType.isObject) {
// just return it as-is. We'll hope it's safe.
return expression;
Expand Down
45 changes: 35 additions & 10 deletions json_serializable/test/json_serializable_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -168,19 +168,44 @@ class _$TrivialNestedNonNullableJsonMapWrapper extends $JsonMapWrapper {
'Remove the JsonSerializable annotation from `annotatedMethod`.');
});
});

group('unknown types', () {
test('in constructor arguments', () {
expectThrows(
'UnknownCtorParamType',
'At least one constructor argument has an invalid type: `number`.',
'Check names and imports.');
String flavorMessage(String flavor) =>
'Could not generate `$flavor` code for `number` '
'because the type is undefined.';

String flavorTodo(String flavor) =>
'Check your imports. If you\'re trying to generate code for a '
'Platform-provided type, you may have to specify a custom `$flavor` '
'in the associated `@JsonKey` annotation.';

group('fromJson', () {
var msg = flavorMessage('fromJson');
var todo = flavorTodo('fromJson');
test('in constructor arguments', () {
expectThrows('UnknownCtorParamType', msg, todo);
});

test('in fields', () {
expectThrows('UnknownFieldType', msg, todo);
});
});

test('in fields', () {
expectThrows(
'UnknownFieldType',
'At least one field has an invalid type: `number`.',
'Check names and imports.');
group('toJson', () {
test('in fields', () {
expectThrows('UnknownFieldTypeToJsonOnly', flavorMessage('toJson'),
flavorTodo('toJson'));
});
});

test('with proper convert methods', () {
var output = runForElementNamed('UnknownFieldTypeWithConvert');
expect(output, contains("_everythingIs42(json['number'])"));
if (generator.useWrappers) {
expect(output, contains('_everythingIs42(_v.number)'));
} else {
expect(output, contains('_everythingIs42(number)'));
}
});
});

Expand Down
15 changes: 15 additions & 0 deletions json_serializable/test/src/json_serializable_test_input.dart
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,21 @@ class UnknownFieldType {
Bob number;
}

@JsonSerializable(createFactory: false)
class UnknownFieldTypeToJsonOnly {
// ignore: undefined_class
Bob number;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that in Dart 2 this will be a compile time error so migrating this test to use modular kernel will likely be an issue, but you could cross that bridge when you come to it (or possibly exclude this file from that builder).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ack – it was either this or depend on Flutter in the Test 😄

}

@JsonSerializable()
class UnknownFieldTypeWithConvert {
@JsonKey(fromJson: _everythingIs42, toJson: _everythingIs42)
// ignore: undefined_class
Bob number;
}

dynamic _everythingIs42(Object input) => 42;

@JsonSerializable(createFactory: false)
class NoSerializeFieldType {
Stopwatch watch;
Expand Down