From f0acd4d28d70f14b356e056f305441b53f39c54c Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Wed, 18 Sep 2024 14:15:26 +0100 Subject: [PATCH] Fix SG nullability annotations for required and init properties. --- .../gen/Helpers/RoslynExtensions.cs | 2 +- .../gen/JsonSourceGenerator.Emitter.cs | 1 + .../gen/JsonSourceGenerator.Parser.cs | 1 + .../PropertyInitializerGenerationSpec.cs | 2 ++ .../tests/Common/NullableAnnotationsTests.cs | 28 +++++++++++++++++++ .../Serialization/NullableAnnotationsTests.cs | 8 ++++++ 6 files changed, 41 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Text.Json/gen/Helpers/RoslynExtensions.cs b/src/libraries/System.Text.Json/gen/Helpers/RoslynExtensions.cs index 5512321b0e2f0..3701b263a315b 100644 --- a/src/libraries/System.Text.Json/gen/Helpers/RoslynExtensions.cs +++ b/src/libraries/System.Text.Json/gen/Helpers/RoslynExtensions.cs @@ -323,7 +323,7 @@ public static bool IsNullable(this IParameterSymbol parameter) { // Because System.Text.Json cannot distinguish between nullable and non-nullable type parameters, // (e.g. the same metadata is being used for both KeyValuePair and KeyValuePair), - // we derive nullability annotations from the original definition of the field and not instation. + // we derive nullability annotations from the original definition of the field and not its instantiation. // This preserves compatibility with the capabilities of the reflection-based NullabilityInfo reader. parameter = parameter.OriginalDefinition; return !IsInputTypeNonNullable(parameter, parameter.Type); diff --git a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs index 11bf6ae9f3cf6..f2de4d1b06308 100644 --- a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs +++ b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs @@ -754,6 +754,7 @@ private static void GenerateCtorParamMetadataInitFunc(SourceWriter writer, strin Name = {{FormatStringLiteral(spec.Name)}}, ParameterType = typeof({{spec.ParameterType.FullyQualifiedName}}), Position = {{spec.ParameterIndex}}, + IsNullable = {{FormatBoolLiteral(spec.IsNullable)}}, IsMemberInitializer = true, }, """); diff --git a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs index d7c9bc72041b2..d63669f2eacf5 100644 --- a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs +++ b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs @@ -1534,6 +1534,7 @@ private void ProcessMember( ParameterType = property.PropertyType, MatchesConstructorParameter = matchingConstructorParameter is not null, ParameterIndex = matchingConstructorParameter?.ParameterIndex ?? paramCount++, + IsNullable = property.PropertyType.CanBeNull && !property.IsSetterNonNullableAnnotation, }; (propertyInitializers ??= new()).Add(propertyInitializer); diff --git a/src/libraries/System.Text.Json/gen/Model/PropertyInitializerGenerationSpec.cs b/src/libraries/System.Text.Json/gen/Model/PropertyInitializerGenerationSpec.cs index 608ce8e887d72..189dac1784e4f 100644 --- a/src/libraries/System.Text.Json/gen/Model/PropertyInitializerGenerationSpec.cs +++ b/src/libraries/System.Text.Json/gen/Model/PropertyInitializerGenerationSpec.cs @@ -31,5 +31,7 @@ public sealed record PropertyInitializerGenerationSpec public required int ParameterIndex { get; init; } public required bool MatchesConstructorParameter { get; init; } + + public required bool IsNullable { get; init; } } } diff --git a/src/libraries/System.Text.Json/tests/Common/NullableAnnotationsTests.cs b/src/libraries/System.Text.Json/tests/Common/NullableAnnotationsTests.cs index 6ad6cad83c625..5ce2030301c0f 100644 --- a/src/libraries/System.Text.Json/tests/Common/NullableAnnotationsTests.cs +++ b/src/libraries/System.Text.Json/tests/Common/NullableAnnotationsTests.cs @@ -72,6 +72,8 @@ public static IEnumerable GetTypesWithNonNullablePropertyGetter() yield return Wrap(typeof(NotNullableSpecialTypePropertiesClass), nameof(NotNullableSpecialTypePropertiesClass.JsonDocument)); yield return Wrap(typeof(NullableObliviousConstructorParameter), nameof(NullableObliviousConstructorParameter.Property)); yield return Wrap(typeof(NotNullGenericPropertyClass), nameof(NotNullGenericPropertyClass.Property)); + yield return Wrap(typeof(ClassWithNonNullableInitProperty), nameof(ClassWithNonNullableInitProperty.Property)); + yield return Wrap(typeof(ClassWithNonNullableInitProperty), nameof(ClassWithNonNullableRequiredProperty.Property)); static object[] Wrap(Type type, string propertyName) => [type, propertyName]; } @@ -125,6 +127,8 @@ public static IEnumerable GetTypesWithNullablePropertyGetter() yield return Wrap(typeof(NullableObliviousPropertyClass), nameof(NullableObliviousPropertyClass.Property)); yield return Wrap(typeof(GenericPropertyClass), nameof(GenericPropertyClass.Property)); yield return Wrap(typeof(NullableGenericPropertyClass), nameof(NullableGenericPropertyClass.Property)); + yield return Wrap(typeof(ClassWithNullableInitProperty), nameof(ClassWithNullableInitProperty.Property)); + yield return Wrap(typeof(ClassWithNullableInitProperty), nameof(ClassWithNullableRequiredProperty.Property)); static object[] Wrap(Type type, string propertyName) => [type, propertyName]; } @@ -191,6 +195,8 @@ public static IEnumerable GetTypesWithNonNullablePropertySetter() yield return Wrap(typeof(DisallowNullConstructorParameter), nameof(DisallowNullConstructorParameter.Property)); yield return Wrap(typeof(DisallowNullConstructorParameter), nameof(DisallowNullConstructorParameter.Property)); yield return Wrap(typeof(NotNullGenericConstructorParameter), nameof(NotNullGenericConstructorParameter.Property)); + yield return Wrap(typeof(ClassWithNonNullableInitProperty), nameof(ClassWithNonNullableInitProperty.Property)); + yield return Wrap(typeof(ClassWithNonNullableInitProperty), nameof(ClassWithNonNullableRequiredProperty.Property)); static object[] Wrap(Type type, string propertyName) => [type, propertyName]; } @@ -249,6 +255,8 @@ public static IEnumerable GetTypesWithNullablePropertySetter() yield return Wrap(typeof(AllowNullConstructorParameter), nameof(AllowNullConstructorParameter.Property)); yield return Wrap(typeof(GenericConstructorParameter), nameof(GenericConstructorParameter.Property)); yield return Wrap(typeof(NullableGenericConstructorParameter), nameof(NullableGenericConstructorParameter.Property)); + yield return Wrap(typeof(ClassWithNullableInitProperty), nameof(ClassWithNullableInitProperty.Property)); + yield return Wrap(typeof(ClassWithNullableInitProperty), nameof(ClassWithNullableRequiredProperty.Property)); static object[] Wrap(Type type, string propertyName) => [type, propertyName]; } @@ -765,5 +773,25 @@ public class NullableFieldClass [JsonInclude] public string? Field; } + + public class ClassWithNullableInitProperty + { + public string? Property { get; init; } + } + + public class ClassWithNonNullableInitProperty + { + public string Property { get; init; } + } + + public class ClassWithNullableRequiredProperty + { + public required string? Property { get; set; } + } + + public class ClassWithNonNullableRequiredProperty + { + public required string Property { get; set; } + } } } diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/NullableAnnotationsTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/NullableAnnotationsTests.cs index 33fdc55b8a1d3..663229c132e6e 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/NullableAnnotationsTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/NullableAnnotationsTests.cs @@ -65,6 +65,10 @@ protected NullableAnnotationsTests_Metadata(JsonSerializerWrapper serializer) [JsonSerializable(typeof(NullableGenericConstructorParameter))] [JsonSerializable(typeof(NotNullGenericConstructorParameter))] [JsonSerializable(typeof(NotNullablePropertyWithIgnoreConditions))] + [JsonSerializable(typeof(ClassWithNullableInitProperty))] + [JsonSerializable(typeof(ClassWithNonNullableInitProperty))] + [JsonSerializable(typeof(ClassWithNullableRequiredProperty))] + [JsonSerializable(typeof(ClassWithNonNullableRequiredProperty))] internal sealed partial class NullableAnnotationsTestsContext_Metadata : JsonSerializerContext { } } @@ -128,6 +132,10 @@ protected NullableAnnotationsTests_Default(JsonSerializerWrapper serializer) [JsonSerializable(typeof(NullableGenericConstructorParameter))] [JsonSerializable(typeof(NotNullGenericConstructorParameter))] [JsonSerializable(typeof(NotNullablePropertyWithIgnoreConditions))] + [JsonSerializable(typeof(ClassWithNullableInitProperty))] + [JsonSerializable(typeof(ClassWithNonNullableInitProperty))] + [JsonSerializable(typeof(ClassWithNullableRequiredProperty))] + [JsonSerializable(typeof(ClassWithNonNullableRequiredProperty))] internal sealed partial class NullableAnnotationsTestsContext_Default : JsonSerializerContext { }