Improve encoding of values to handle null cases (#1223)

Specifically with enums and converter functions when
`includeIfNull` is `false`.

Enum values with `null` as an output where not properly wrapped.
This is now fixed.

Fixes #1202

Also 'unwrap' the cases where conversion functions never return `null`.

Author: kevmoo

json_serializable/CHANGELOG.md

@@ -1,3 +1,8 @@
+## 6.5.2
+
+- Better handling of `null` when encoding `enum` values or values with
+  conversions.
+
 ## 6.5.1
 
 - Fixed `BigInt`, `DateTime`, and `Uri` support for `JsonKey.defaultValue` with

json_serializable/lib/src/encoder_helper.dart

@@ -4,10 +4,10 @@
 
 import 'package:analyzer/dart/element/element.dart';
 import 'package:analyzer/dart/element/nullability_suffix.dart';
-import 'package:json_annotation/json_annotation.dart';
 import 'package:source_helper/source_helper.dart';
 
 import 'constants.dart';
+import 'enum_utils.dart';
 import 'helper_core.dart';
 import 'type_helpers/generic_factory_helper.dart';
 import 'type_helpers/json_converter_helper.dart';
@@ -163,19 +163,32 @@ abstract class EncodeHelper implements HelperCore {
   bool _writeJsonValueNaive(FieldElement field) {
     final jsonKey = jsonKeyFor(field);
 
-    return jsonKey.includeIfNull ||
-        (!field.type.isNullableType && !_fieldHasCustomEncoder(field));
-  }
+    if (jsonKey.includeIfNull) {
+      return true;
+    }
 
-  /// Returns `true` if [field] has a user-defined encoder.
-  ///
-  /// This can be either a `toJson` function in [JsonKey] or a [JsonConverter]
-  /// annotation.
-  bool _fieldHasCustomEncoder(FieldElement field) {
     final helperContext = getHelperContext(field);
-    return helperContext.serializeConvertData != null ||
-        const JsonConverterHelper()
-                .serialize(field.type, 'test', helperContext) !=
-            null;
+
+    final serializeConvertData = helperContext.serializeConvertData;
+    if (serializeConvertData != null) {
+      return !serializeConvertData.returnType.isNullableType;
+    }
+
+    final nullableEncodeConverter =
+        hasConverterNullEncode(field.type, helperContext);
+
+    if (nullableEncodeConverter != null) {
+      return !nullableEncodeConverter;
+    }
+
+    // We can consider enums as kinda like having custom converters
+    // same rules apply. If `null` is in the set of encoded values, we
+    // should not write naive
+    final enumWithNullValue = enumFieldWithNullInEncodeMap(field.type);
+    if (enumWithNullValue != null) {
+      return !enumWithNullValue;
+    }
+
+    return !field.type.isNullableType;
   }
 }

json_serializable/lib/src/enum_utils.dart

@@ -14,9 +14,38 @@ import 'utils.dart';
 String constMapName(DartType targetType) =>
     '_\$${targetType.element2!.name}EnumMap';
 
+/// If [targetType] is not an enum, return `null`.
+///
+/// Otherwise, returns `true` if one of the encoded values of the enum is
+/// `null`.
+bool? enumFieldWithNullInEncodeMap(DartType targetType) {
+  final enumMap = _enumMap(targetType);
+
+  if (enumMap == null) return null;
+
+  return enumMap.values.contains(null);
+}
+
 String? enumValueMapFromType(
   DartType targetType, {
   bool nullWithNoAnnotation = false,
+}) {
+  final enumMap =
+      _enumMap(targetType, nullWithNoAnnotation: nullWithNoAnnotation);
+
+  if (enumMap == null) return null;
+
+  final items = enumMap.entries
+      .map((e) => '  ${targetType.element2!.name}.${e.key.name}: '
+          '${jsonLiteralAsDart(e.value)},')
+      .join();
+
+  return 'const ${constMapName(targetType)} = {\n$items\n};';
+}
+
+Map<FieldElement, Object?>? _enumMap(
+  DartType targetType, {
+  bool nullWithNoAnnotation = false,
 }) {
   final annotation = _jsonEnumChecker.firstAnnotationOf(targetType.element2!);
   final jsonEnum = _fromAnnotation(annotation);
@@ -27,21 +56,14 @@ String? enumValueMapFromType(
     return null;
   }
 
-  final enumMap = {
+  return {
     for (var field in enumFields)
       field: _generateEntry(
         field: field,
         jsonEnum: jsonEnum,
         targetType: targetType,
       ),
   };
-
-  final items = enumMap.entries
-      .map((e) => '  ${targetType.element2!.name}.${e.key.name}: '
-          '${jsonLiteralAsDart(e.value)},')
-      .join();
-
-  return 'const ${constMapName(targetType)} = {\n$items\n};';
 }
 
 Object? _generateEntry({

json_serializable/lib/src/type_helper_ctx.dart

@@ -135,13 +135,12 @@ ConvertData? _convertData(DartObject obj, FieldElement element, bool isFrom) {
         'positional parameter.');
   }
 
+  final returnType = executableElement.returnType;
   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.
@@ -176,5 +175,5 @@ ConvertData? _convertData(DartObject obj, FieldElement element, bool isFrom) {
     }
   }
 
-  return ConvertData(executableElement.qualifiedName, argType);
+  return ConvertData(executableElement.qualifiedName, argType, returnType);
 }

json_serializable/lib/src/type_helpers/convert_helper.dart

@@ -14,8 +14,9 @@ import '../type_helper.dart';
 class ConvertData {
   final String name;
   final DartType paramType;
+  final DartType returnType;
 
-  ConvertData(this.name, this.paramType);
+  ConvertData(this.name, this.paramType, this.returnType);
 }
 
 abstract class TypeHelperContextWithConvert extends TypeHelperContext {

json_serializable/lib/src/type_helpers/enum_helper.dart

@@ -29,7 +29,8 @@ class EnumHelper extends TypeHelper<TypeHelperContextWithConfig> {
 
     context.addMember(memberContent);
 
-    if (targetType.isNullableType) {
+    if (targetType.isNullableType ||
+        enumFieldWithNullInEncodeMap(targetType) == true) {
       return '${constMapName(targetType)}[$expression]';
     } else {
       return '${constMapName(targetType)}[$expression]!';

json_serializable/lib/src/type_helpers/json_converter_helper.dart

@@ -143,6 +143,24 @@ class _JsonConvertData {
       accessor.isEmpty ? '' : '.$accessor';
 }
 
+/// If there is no converter for the params, return `null`.
+///
+/// Otherwise, returns `true` if the converter has a null return value.
+///
+/// Used to make sure we create a smart encoding function.
+bool? hasConverterNullEncode(
+  DartType targetType,
+  TypeHelperContextWithConfig ctx,
+) {
+  final data = _typeConverter(targetType, ctx);
+
+  if (data == null) {
+    return null;
+  }
+
+  return data.jsonType.isNullableType;
+}
+
 _JsonConvertData? _typeConverter(
   DartType targetType,
   TypeHelperContextWithConfig ctx,

json_serializable/pubspec.yaml

@@ -1,5 +1,5 @@
 name: json_serializable
-version: 6.5.1
+version: 6.5.2
 description: >-
   Automatically generate code for converting to and from JSON by annotating
   Dart classes.

json_serializable/test/integration/converter_examples.dart

@@ -0,0 +1,97 @@
+import 'dart:convert';
+
+import 'package:json_annotation/json_annotation.dart';
+
+part 'converter_examples.g.dart';
+
+@JsonEnum(valueField: 'value')
+enum Issue1202RegressionEnum {
+  normalValue(42),
+  nullValue(null);
+
+  const Issue1202RegressionEnum(this.value);
+
+  final int? value;
+}
+
+@JsonSerializable(includeIfNull: false)
+class Issue1202RegressionClass {
+  @JsonKey(fromJson: _fromJson, toJson: _toJson)
+  final int valueWithFunctions;
+
+  @_Issue1202RegressionNotNullConverter()
+  final int notNullableValueWithConverter;
+
+  final Issue1202RegressionEnum value;
+  final int? normalNullableValue;
+
+  @_Issue1202RegressionConverter()
+  final int notNullableValueWithNullableConverter;
+
+  @JsonKey(fromJson: _fromJsonNullable, toJson: _toJsonNullable)
+  final int valueWithNullableFunctions;
+
+  Issue1202RegressionClass({
+    required this.value,
+    required this.normalNullableValue,
+    required this.notNullableValueWithNullableConverter,
+    required this.notNullableValueWithConverter,
+    required this.valueWithFunctions,
+    required this.valueWithNullableFunctions,
+  });
+
+  factory Issue1202RegressionClass.fromJson(Map<String, dynamic> json) =>
+      _$Issue1202RegressionClassFromJson(json);
+
+  Map<String, dynamic> toJson() => _$Issue1202RegressionClassToJson(this);
+
+  @override
+  bool operator ==(Object other) =>
+      other is Issue1202RegressionClass &&
+      jsonEncode(other) == jsonEncode(this);
+
+  @override
+  int get hashCode => jsonEncode(this).hashCode;
+
+  static int _fromJsonNullable(String? json) {
+    if (json == null) return _default;
+    return int.parse(json);
+  }
+
+  static String? _toJsonNullable(int object) {
+    if (object == _default) return null;
+    return object.toString();
+  }
+
+  static int _fromJson(String json) => int.parse(json);
+
+  static String _toJson(int object) => object.toString();
+}
+
+const _default = 42;
+
+class _Issue1202RegressionConverter extends JsonConverter<int, String?> {
+  const _Issue1202RegressionConverter();
+
+  @override
+  int fromJson(String? json) {
+    if (json == null) return _default;
+    return int.parse(json);
+  }
+
+  @override
+  String? toJson(int object) {
+    if (object == _default) return null;
+    return object.toString();
+  }
+}
+
+class _Issue1202RegressionNotNullConverter extends JsonConverter<int, String> {
+  const _Issue1202RegressionNotNullConverter();
+
+  @override
+  int fromJson(String json) => int.parse(json);
+
+  @override
+  String toJson(int object) => object.toString();
+}