Skip to content

Include nullability information for generic class #873

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
Apr 13, 2021
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
6 changes: 5 additions & 1 deletion json_serializable/lib/src/decode_helper.dart
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,11 @@ abstract class DecodeHelper implements HelperCore {
'effect because both `disallowNullValue` and `required` are set to '
'`true`.');
}
if (contextHelper.deserializeConvertData != null) {

final deserializeConvertData = contextHelper.deserializeConvertData;

if (deserializeConvertData != null &&
!deserializeConvertData.nullableToAllowDefault) {
log.warning('The field `${field.name}` has both `defaultValue` and '
'`fromJson` defined which likely won\'t work for your scenario.\n'
'Instead of using `defaultValue`, set `nullable: false` and handle '
Expand Down
14 changes: 6 additions & 8 deletions json_serializable/lib/src/helper_core.dart
Original file line number Diff line number Diff line change
Expand Up @@ -141,14 +141,12 @@ String typeToCode(
if (type.isDynamic) {
return 'dynamic';
} else if (type is InterfaceType) {
final typeArguments = type.typeArguments;
if (typeArguments.isEmpty) {
final nullablePostfix = (type.isNullableType || forceNullable) ? '?' : '';
return '${type.element.name}$nullablePostfix';
} else {
final typeArgumentsCode = typeArguments.map(typeToCode).join(', ');
return '${type.element.name}<$typeArgumentsCode>';
}
return [
type.element.name,
if (type.typeArguments.isNotEmpty)
'<${type.typeArguments.map(typeToCode).join(', ')}>',
(type.isNullableType || forceNullable) ? '?' : '',
].join();
}
throw UnimplementedError('(${type.runtimeType}) $type');
}
31 changes: 23 additions & 8 deletions json_serializable/lib/src/type_helper_ctx.dart
Original file line number Diff line number Diff line change
Expand Up @@ -125,23 +125,34 @@ ConvertData? _convertData(DartObject obj, FieldElement element, bool isFrom) {
'positional parameter.');
}

var nullableToAllowDefault = false;

final argType = executableElement.parameters.first.type;
if (isFrom) {
final hasDefaultValue =
!jsonKeyAnnotation(element).read('defaultValue').isNull;

final returnType = executableElement.returnType;

if (returnType is TypeParameterType) {
// We keep things simple in this case. We rely on inferred type arguments
// to the `fromJson` function.
// TODO: consider adding error checking here if there is confusion.
} else if (!returnType.isAssignableTo(element.type)) {
final returnTypeCode = typeToCode(returnType);
final elementTypeCode = typeToCode(element.type);
throwUnsupported(
element,
'The `$paramName` function `${executableElement.name}` return type '
'`$returnTypeCode` is not compatible with field type '
'`$elementTypeCode`.');
if (returnType.promoteNonNullable().isAssignableTo(element.type) &&
hasDefaultValue) {
// noop
} else {
final returnTypeCode = typeToCode(returnType);
final elementTypeCode = typeToCode(element.type);
throwUnsupported(
element,
'The `$paramName` function `${executableElement.name}` return type '
'`$returnTypeCode` is not compatible with field type '
'`$elementTypeCode`.');
}
}
nullableToAllowDefault = hasDefaultValue && returnType.isNullableType;
} else {
if (argType is TypeParameterType) {
// We keep things simple in this case. We rely on inferred type arguments
Expand All @@ -164,5 +175,9 @@ ConvertData? _convertData(DartObject obj, FieldElement element, bool isFrom) {
name = '${executableElement.enclosingElement.name}.$name';
}

return ConvertData(name, argType);
return ConvertData(
name,
argType,
nullableToAllowDefault: nullableToAllowDefault,
);
}
10 changes: 9 additions & 1 deletion json_serializable/lib/src/type_helpers/convert_helper.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,15 @@ class ConvertData {
final String name;
final DartType paramType;

ConvertData(this.name, this.paramType);
/// `true` if the function returns a nullable value AND there is a default
/// value assigned.
final bool nullableToAllowDefault;

ConvertData(
this.name,
this.paramType, {
required this.nullableToAllowDefault,
});
}

abstract class TypeHelperContextWithConvert extends TypeHelperContext {
Expand Down
5 changes: 5 additions & 0 deletions json_serializable/lib/src/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -202,3 +202,8 @@ Map<FieldElement, dynamic>? enumFieldsMap(DartType targetType) {
/// Otherwise, `null`.
Iterable<FieldElement>? iterateEnumFields(DartType targetType) =>
enumFieldsMap(targetType)?.keys;

extension DartTypeExtension on DartType {
DartType promoteNonNullable() =>
element?.library?.typeSystem.promoteToNonNull(this) ?? this;
}
2 changes: 2 additions & 0 deletions json_serializable/test/json_serializable_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ const _expectedAnnotatedTests = {
'OverrideGetterExampleI613',
'PrivateFieldCtorClass',
'PropInMixinI448Regression',
'Reproduce869NullableGenericType',
'Reproduce869NullableGenericTypeWithDefault',
'SetSupport',
'SubclassedJsonKey',
'SubType',
Expand Down
58 changes: 58 additions & 0 deletions json_serializable/test/src/to_from_json_test_input.dart
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,64 @@ class BadToFuncReturnType {
late String field;
}

@ShouldThrow(
'Error with `@JsonKey` on `values`. The `fromJson` function `_fromList` '
'return type `List<int>?` is not compatible with field type `List<int>`.',
element: 'values',
)
@JsonSerializable()
class Reproduce869NullableGenericType {
@JsonKey(
toJson: _toList, // nullable
fromJson: _fromList, // nullable
)
late final List<int> values;
}

@ShouldGenerate(
r'''
Reproduce869NullableGenericTypeWithDefault
_$Reproduce869NullableGenericTypeWithDefaultFromJson(
Map<String, dynamic> json) =>
Reproduce869NullableGenericTypeWithDefault()
..values = _fromList(json['values'] as List?) ?? []
..valuesNullable = _fromList(json['valuesNullable'] as List?) ?? [];

Map<String, dynamic> _$Reproduce869NullableGenericTypeWithDefaultToJson(
Reproduce869NullableGenericTypeWithDefault instance) =>
<String, dynamic>{
'values': _toList(instance.values),
'valuesNullable': _toList(instance.valuesNullable),
};
''',
)
@JsonSerializable()
class Reproduce869NullableGenericTypeWithDefault {
@JsonKey(
toJson: _toList, // nullable
fromJson: _fromList, // nullable
defaultValue: <int>[],
)
late List<int> values;

@JsonKey(
toJson: _toList, // nullable
fromJson: _fromList, // nullable
defaultValue: <int>[],
)
List<int>? valuesNullable;
}

List<int>? _fromList(List? pairs) {
return pairs?.map((it) {
return it as int;
}).toList(growable: false);
}

List<List>? _toList(List<int>? pairs) {
return pairs?.map((it) => [it]).toList(growable: false);
}

@ShouldThrow(
'Error with `@JsonKey` on `field`. The `toJson` function '
'`_twoArgFunction` must have one positional parameter.',
Expand Down