Handle the intersection of convert function, !nullable & !includeIfNull (#443)

If a field has a conversion function defined – either `JsonKey.toJson`
or a custom `JsonConverter` annotation – handle the case where the
function returns `null` and both `nullable` and `includeIfNull` are
`false`.

Before this change, you could still get `null` values in output JSON
even if `includeIfNull` is `false`.

Prepare to release v2.2.0

Author: kevmoo

json_serializable/CHANGELOG.md

@@ -1,3 +1,9 @@
+## 2.2.0
+
+- If a field has a conversion function defined – either `JsonKey.toJson` or a
+  custom `JsonConverter` annotation – handle the case where the function
+  returns `null` and both `nullable` and `includeIfNull` are `false`.
+
 ## 2.1.2
 
 * Support `package:json_annotation` `>=2.1.0 <2.3.0`.

json_serializable/lib/src/encoder_helper.dart

@@ -8,6 +8,8 @@ import 'package:json_annotation/json_annotation.dart';
 import 'constants.dart';
 import 'helper_core.dart';
 import 'type_helper.dart';
+import 'type_helpers/convert_helper.dart';
+import 'type_helpers/json_converter_helper.dart';
 
 abstract class EncodeHelper implements HelperCore {
   String _fieldAccess(FieldElement field) {
@@ -110,7 +112,8 @@ class ${_wrapperClassName(true)} extends \$JsonMapWrapper {
         String check;
 
         if (!_writeJsonValueNaive(field)) {
-          check = '_v.${field.name} != null';
+          final expression = _wrapCustomEncoder('_v.${field.name}', field);
+          check = '$expression != null';
 
           if (!jsonKeyFor(field).encodeEmptyCollection) {
             assert(!jsonKeyFor(field).includeIfNull);
@@ -248,12 +251,88 @@ class ${_wrapperClassName(true)} extends \$JsonMapWrapper {
 
   /// Returns `true` if the field can be written to JSON 'naively' – meaning
   /// we can avoid checking for `null`.
+  bool _writeJsonValueNaive(FieldElement field) {
+    final jsonKey = jsonKeyFor(field);
+
+    if (jsonKey.includeIfNull) {
+      return true;
+    }
+
+    if (!jsonKey.nullable &&
+        jsonKey.encodeEmptyCollection &&
+        !_fieldHasCustomEncoder(field)) {
+      return true;
+    }
+
+    return false;
+  }
+
+  /// Returns `true` if [field] has a user-defined encoder.
   ///
-  /// `true` if either:
-  ///   `includeIfNull` is `true`
-  ///   or
-  ///   `nullable` is `false` and `encodeEmptyCollection` is true
-  bool _writeJsonValueNaive(FieldElement field) =>
-      jsonKeyFor(field).includeIfNull ||
-      (!jsonKeyFor(field).nullable && jsonKeyFor(field).encodeEmptyCollection);
+  /// This can be either a `toJson` function in [JsonKey] or a [JsonConverter]
+  /// annotation.
+  bool _fieldHasCustomEncoder(FieldElement field) {
+    final helperContext = getHelperContext(field);
+
+    if (helperContext.serializeConvertData != null) {
+      return true;
+    }
+
+    final output = const JsonConverterHelper()
+        .serialize(field.type, 'test', helperContext);
+
+    if (output != null) {
+      return true;
+    }
+    return false;
+  }
+
+  /// If [field] has a user-defined encoder, return [expression] wrapped in
+  /// the corresponding conversion logic so we can do a correct `null` check.
+  ///
+  /// This can be either a `toJson` function in [JsonKey] or a [JsonConverter]
+  /// annotation.
+  ///
+  /// If there is no user-defined encoder, just return [expression] as-is.
+  String _wrapCustomEncoder(String expression, FieldElement field) {
+    final helperContext = getHelperContext(field);
+
+    final convertData = helperContext.serializeConvertData;
+
+    var result = expression;
+    if (convertData != null) {
+      result = toJsonSerializeImpl(
+        getHelperContext(field).serializeConvertData.name,
+        expression,
+        jsonKeyFor(field).nullable,
+      );
+    } else {
+      final output = const JsonConverterHelper()
+          .serialize(field.type, expression, helperContext);
+
+      if (output != null) {
+        result = output.toString();
+      }
+    }
+
+    assert(
+      (result != expression) == _fieldHasCustomEncoder(field),
+      'If the output expression is different, then it should map to a field '
+      'with a custom encoder',
+    );
+
+    if (result == expression) {
+      // No conversion
+      return expression;
+    }
+
+    if (jsonKeyFor(field).nullable) {
+      // If there was a conversion and the field is nullable, wrap the output
+      // in () – there will be null checks that will break the comparison
+      // in the caller
+      result = '($result)';
+    }
+
+    return result;
+  }
 }

json_serializable/lib/src/type_helpers/convert_helper.dart

@@ -31,8 +31,7 @@ class ConvertHelper extends TypeHelper<TypeHelperContextWithConvert> {
     if (toJsonData != null) {
       assert(toJsonData.paramType is TypeParameterType ||
           targetType.isAssignableTo(toJsonData.paramType));
-      final result = '${toJsonData.name}($expression)';
-      return commonNullPrefix(context.nullable, expression, result).toString();
+      return toJsonSerializeImpl(toJsonData.name, expression, context.nullable);
     }
     return null;
   }
@@ -49,3 +48,10 @@ class ConvertHelper extends TypeHelper<TypeHelperContextWithConvert> {
     return null;
   }
 }
+
+/// Exposed to support `EncodeHelper` – not exposed publicly.
+String toJsonSerializeImpl(
+    String toJsonDataName, String expression, bool nullable) {
+  final result = '$toJsonDataName($expression)';
+  return commonNullPrefix(nullable, expression, result).toString();
+}

json_serializable/lib/type_helper.dart

@@ -6,7 +6,7 @@ export 'src/shared_checkers.dart' show simpleJsonTypeChecker, typeArgumentsOf;
 export 'src/type_helper.dart'
     show TypeHelperContext, TypeHelper, UnsupportedTypeError;
 export 'src/type_helpers/big_int_helper.dart';
-export 'src/type_helpers/convert_helper.dart';
+export 'src/type_helpers/convert_helper.dart' hide toJsonSerializeImpl;
 export 'src/type_helpers/date_time_helper.dart';
 export 'src/type_helpers/enum_helper.dart';
 export 'src/type_helpers/iterable_helper.dart';

json_serializable/pubspec.yaml

@@ -1,5 +1,5 @@
 name: json_serializable
-version: 2.1.2
+version: 2.2.0
 author: Dart Team <misc@dartlang.org>
 description: >-
   Automatically generate code for converting to and from JSON by annotating

json_serializable/test/json_serializable_test.dart

@@ -100,6 +100,9 @@ const _expectedAnnotatedTests = [
   'SubTypeWithAnnotatedFieldOverrideExtendsWithOverrides',
   'SubTypeWithAnnotatedFieldOverrideImplements',
   'theAnswer',
+  'ToJsonIncludeIfNullFalseWrapped',
+  'ToJsonNullableFalseIncludeIfNullFalse',
+  'ToJsonNullableFalseIncludeIfNullFalseWrapped',
   'TypedConvertMethods',
   'UnknownCtorParamType',
   'UnknownFieldType',