From: Eirik Tsarpalis Date: Thu, 9 Feb 2023 15:37:47 +0000 (+0000) Subject: Fix a couple of JsonConstructor bugs & clean up JsonParameterInfo implementation... X-Git-Tag: accepted/tizen/unified/riscv/20231226.055536~4136 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=27fd2cdca0e0b08a19000413c4e16a4fdb3559bb;p=platform%2Fupstream%2Fdotnet%2Fruntime.git Fix a couple of JsonConstructor bugs & clean up JsonParameterInfo implementation. (#81842) * Fix a couple of JsonConstructor bugs & clean up JsonParameterInfo implementation. * add test coverage for ignored null values * fix whitespace --- diff --git a/src/libraries/System.Text.Json/Common/ReflectionExtensions.cs b/src/libraries/System.Text.Json/Common/ReflectionExtensions.cs index ed3414e..11fba3f 100644 --- a/src/libraries/System.Text.Json/Common/ReflectionExtensions.cs +++ b/src/libraries/System.Text.Json/Common/ReflectionExtensions.cs @@ -319,14 +319,34 @@ namespace System.Text.Json.Reflection public static object? GetDefaultValue(this ParameterInfo parameterInfo) { + Type parameterType = parameterInfo.ParameterType; object? defaultValue = parameterInfo.DefaultValue; + if (defaultValue is null) + { + return null; + } + // DBNull.Value is sometimes used as the default value (returned by reflection) of nullable params in place of null. - if (defaultValue == DBNull.Value && parameterInfo.ParameterType != typeof(DBNull)) + if (defaultValue == DBNull.Value && parameterType != typeof(DBNull)) { return null; } +#if !BUILDING_SOURCE_GENERATOR + // Default values of enums or nullable enums are represented using the underlying type and need to be cast explicitly + // cf. https://github.com/dotnet/runtime/issues/68647 + if (parameterType.IsEnum) + { + return Enum.ToObject(parameterType, defaultValue); + } + + if (Nullable.GetUnderlyingType(parameterType) is Type underlyingType && underlyingType.IsEnum) + { + return Enum.ToObject(underlyingType, defaultValue); + } +#endif + return defaultValue; } diff --git a/src/libraries/System.Text.Json/src/System.Text.Json.csproj b/src/libraries/System.Text.Json/src/System.Text.Json.csproj index 16003dc..ea0c128 100644 --- a/src/libraries/System.Text.Json/src/System.Text.Json.csproj +++ b/src/libraries/System.Text.Json/src/System.Text.Json.csproj @@ -243,7 +243,6 @@ The System.Text.Json library is built-in as part of the shared framework in .NET - diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Arguments.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Arguments.cs index 2b33133..ddf6972 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Arguments.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Arguments.cs @@ -9,9 +9,9 @@ namespace System.Text.Json /// internal sealed class Arguments { - public TArg0 Arg0 = default!; - public TArg1 Arg1 = default!; - public TArg2 Arg2 = default!; - public TArg3 Arg3 = default!; + public TArg0? Arg0; + public TArg1? Arg1; + public TArg2? Arg2; + public TArg3? Arg3; } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.Large.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.Large.cs index c7ba3c9..e34aae0 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.Large.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.Large.cs @@ -19,11 +19,11 @@ namespace System.Text.Json.Serialization.Converters Debug.Assert(jsonParameterInfo.ShouldDeserialize); Debug.Assert(jsonParameterInfo.Options != null); - bool success = jsonParameterInfo.ConverterBase.TryReadAsObject(ref reader, TypeToConvert, jsonParameterInfo.Options!, ref state, out object? arg); + bool success = jsonParameterInfo.EffectiveConverter.TryReadAsObject(ref reader, TypeToConvert, jsonParameterInfo.Options!, ref state, out object? arg); if (success && !(arg == null && jsonParameterInfo.IgnoreNullTokensOnRead)) { - ((object[])state.Current.CtorArgumentState!.Arguments)[jsonParameterInfo.ClrInfo.Position] = arg!; + ((object[])state.Current.CtorArgumentState!.Arguments)[jsonParameterInfo.Position] = arg!; // if this is required property IgnoreNullTokensOnRead will always be false because we don't allow for both to be true state.Current.MarkRequiredPropertyAsRead(jsonParameterInfo.MatchingProperty); @@ -60,7 +60,7 @@ namespace System.Text.Json.Serialization.Converters for (int i = 0; i < typeInfo.ParameterCount; i++) { JsonParameterInfo parameterInfo = cache[i].Value; - arguments[parameterInfo.ClrInfo.Position] = parameterInfo.DefaultValue; + arguments[parameterInfo.Position] = parameterInfo.DefaultValue; } state.Current.CtorArgumentState!.Arguments = arguments; diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.Small.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.Small.cs index 5806092..2fb3f32 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.Small.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.Small.cs @@ -31,7 +31,7 @@ namespace System.Text.Json.Serialization.Converters bool success; - switch (jsonParameterInfo.ClrInfo.Position) + switch (jsonParameterInfo.Position) { case 0: success = TryRead(ref state, ref reader, jsonParameterInfo, out arguments.Arg0); @@ -57,19 +57,17 @@ namespace System.Text.Json.Serialization.Converters scoped ref ReadStack state, ref Utf8JsonReader reader, JsonParameterInfo jsonParameterInfo, - out TArg arg) + out TArg? arg) { Debug.Assert(jsonParameterInfo.ShouldDeserialize); - Debug.Assert(jsonParameterInfo.Options != null); var info = (JsonParameterInfo)jsonParameterInfo; - var converter = (JsonConverter)jsonParameterInfo.ConverterBase; - bool success = converter.TryRead(ref reader, info.PropertyType, info.Options!, ref state, out TArg? value); + bool success = info.EffectiveConverter.TryRead(ref reader, info.ParameterType, info.Options, ref state, out TArg? value); - arg = value == null && jsonParameterInfo.IgnoreNullTokensOnRead - ? (TArg?)info.DefaultValue! // Use default value specified on parameter, if any. - : value!; + arg = value is null && jsonParameterInfo.IgnoreNullTokensOnRead + ? info.DefaultValue // Use default value specified on parameter, if any. + : value; if (success) { @@ -92,31 +90,23 @@ namespace System.Text.Json.Serialization.Converters for (int i = 0; i < typeInfo.ParameterCount; i++) { JsonParameterInfo parameterInfo = cache[i].Value; - - // We can afford not to set default values for ctor arguments when we should't deserialize because the - // type parameters of the `Arguments` type provide default semantics that work well with value types. - if (parameterInfo.ShouldDeserialize) + switch (parameterInfo.Position) { - int position = parameterInfo.ClrInfo.Position; - - switch (position) - { - case 0: - arguments.Arg0 = ((JsonParameterInfo)parameterInfo).TypedDefaultValue!; - break; - case 1: - arguments.Arg1 = ((JsonParameterInfo)parameterInfo).TypedDefaultValue!; - break; - case 2: - arguments.Arg2 = ((JsonParameterInfo)parameterInfo).TypedDefaultValue!; - break; - case 3: - arguments.Arg3 = ((JsonParameterInfo)parameterInfo).TypedDefaultValue!; - break; - default: - Debug.Fail("More than 4 params: we should be in override for LargeObjectWithParameterizedConstructorConverter."); - throw new InvalidOperationException(); - } + case 0: + arguments.Arg0 = ((JsonParameterInfo)parameterInfo).DefaultValue; + break; + case 1: + arguments.Arg1 = ((JsonParameterInfo)parameterInfo).DefaultValue; + break; + case 2: + arguments.Arg2 = ((JsonParameterInfo)parameterInfo).DefaultValue; + break; + case 3: + arguments.Arg3 = ((JsonParameterInfo)parameterInfo).DefaultValue; + break; + default: + Debug.Fail("More than 4 params: we should be in override for LargeObjectWithParameterizedConstructorConverter."); + throw new InvalidOperationException(); } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.cs index 5a7f33e..a389a11 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.cs @@ -445,7 +445,7 @@ namespace System.Text.Json.Serialization.Converters // Returning false below will cause the read-ahead functionality to finish the read. state.Current.PropertyState = StackFramePropertyState.ReadValue; - if (!SingleValueReadWithReadAhead(jsonParameterInfo.ConverterBase.RequiresReadAhead, ref reader, ref state)) + if (!SingleValueReadWithReadAhead(jsonParameterInfo.EffectiveConverter.RequiresReadAhead, ref reader, ref state)) { return false; } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverter.cs index a32063e..89bc7a2 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverter.cs @@ -97,8 +97,6 @@ namespace System.Text.Json.Serialization throw new InvalidOperationException(); } - internal abstract JsonParameterInfo CreateJsonParameterInfo(); - internal abstract JsonConverter CreateCastingConverter(); /// diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterFactory.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterFactory.cs index 441cbf5..6d492a1 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterFactory.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterFactory.cs @@ -32,13 +32,6 @@ namespace System.Text.Json.Serialization /// public abstract JsonConverter? CreateConverter(Type typeToConvert, JsonSerializerOptions options); - internal override JsonParameterInfo CreateJsonParameterInfo() - { - Debug.Fail("We should never get here."); - - throw new InvalidOperationException(); - } - internal sealed override Type? KeyType => null; internal sealed override Type? ElementType => null; diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs index ff811d8..d2a5a70 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs @@ -56,11 +56,6 @@ namespace System.Text.Json.Serialization return new JsonTypeInfo(this, options); } - internal sealed override JsonParameterInfo CreateJsonParameterInfo() - { - return new JsonParameterInfo(); - } - internal sealed override JsonConverter CreateCastingConverter() { if (this is JsonConverter conv) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultValueHolder.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultValueHolder.cs deleted file mode 100644 index 224376d..0000000 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultValueHolder.cs +++ /dev/null @@ -1,42 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Diagnostics.CodeAnalysis; - -namespace System.Text.Json.Serialization.Metadata -{ - /// - /// Helper class used for calculating the default value for a given System.Type instance. - /// - internal sealed class DefaultValueHolder - { - [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2067:UnrecognizedReflectionPattern", - Justification = "GetUninitializedObject is only called on a struct. You can always create an instance of a struct.")] - private DefaultValueHolder(Type type) - { - if (type.IsValueType && Nullable.GetUnderlyingType(type) == null) - { -#if NETCOREAPP - DefaultValue = System.Runtime.CompilerServices.RuntimeHelpers.GetUninitializedObject(type); -#else - DefaultValue = System.Runtime.Serialization.FormatterServices.GetUninitializedObject(type); -#endif - } - } - - /// - /// Returns the default value for the specified type. - /// - public object? DefaultValue { get; } - - /// - /// Returns true if contains only default values. - /// - public bool IsDefaultValue(object value) => DefaultValue is null ? value is null : DefaultValue.Equals(value); - - /// - /// Creates a holder instance representing a type. - /// - public static DefaultValueHolder CreateHolder(Type type) => new DefaultValueHolder(type); - } -} diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonParameterInfo.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonParameterInfo.cs index 2f9ea58..5112d8b 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonParameterInfo.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonParameterInfo.cs @@ -11,108 +11,43 @@ namespace System.Text.Json.Serialization.Metadata /// internal abstract class JsonParameterInfo { - private JsonTypeInfo? _jsonTypeInfo; + public JsonConverter EffectiveConverter => MatchingProperty.EffectiveConverter; - public JsonConverter ConverterBase { get; private set; } = null!; + // The default value of the parameter. This is `DefaultValue` of the `ParameterInfo`, if specified, or the `default` for the `ParameterType`. + public object? DefaultValue { get; private protected init; } - private protected bool MatchingPropertyCanBeNull { get; private set; } + public bool IgnoreNullTokensOnRead { get; } - // The default value of the parameter. This is `DefaultValue` of the `ParameterInfo`, if specified, or the CLR `default` for the `ParameterType`. - public object? DefaultValue { get; private protected set; } - - public bool IgnoreNullTokensOnRead { get; private set; } - - // Options can be referenced here since all JsonPropertyInfos originate from a JsonTypeInfo that is cached on JsonSerializerOptions. - public JsonSerializerOptions? Options { get; set; } // initialized in Init method + public JsonSerializerOptions Options { get; } // The name of the parameter as UTF-8 bytes. - public byte[] NameAsUtf8Bytes { get; private set; } = null!; + public byte[] NameAsUtf8Bytes { get; } - public JsonNumberHandling? NumberHandling { get; private set; } + public JsonNumberHandling? NumberHandling { get; } - // Using a field to avoid copy semantics. - public JsonParameterInfoValues ClrInfo = null!; + public int Position { get; } - public JsonTypeInfo JsonTypeInfo - { - get - { - Debug.Assert(Options != null); - Debug.Assert(ShouldDeserialize); - return _jsonTypeInfo ??= Options.GetTypeInfoInternal(PropertyType); - } - set - { - // Used by JsonMetadataServices. - Debug.Assert(_jsonTypeInfo == null); - _jsonTypeInfo = value; - } - } + public JsonTypeInfo JsonTypeInfo => MatchingProperty.JsonTypeInfo; - public Type PropertyType { get; set; } = null!; + public Type ParameterType { get; } - public bool ShouldDeserialize { get; private set; } + public bool ShouldDeserialize { get; } - public JsonPropertyInfo MatchingProperty { get; private set; } = null!; + public JsonPropertyInfo MatchingProperty { get; } - public virtual void Initialize(JsonParameterInfoValues parameterInfo, JsonPropertyInfo matchingProperty, JsonSerializerOptions options) + public JsonParameterInfo(JsonParameterInfoValues parameterInfoValues, JsonPropertyInfo matchingProperty) { + Debug.Assert(matchingProperty.IsConfigured); + MatchingProperty = matchingProperty; - ClrInfo = parameterInfo; - Options = options; - ShouldDeserialize = true; + ShouldDeserialize = !matchingProperty.IsIgnored; + Options = matchingProperty.Options; + Position = parameterInfoValues.Position; - PropertyType = matchingProperty.PropertyType; - NameAsUtf8Bytes = matchingProperty.NameAsUtf8Bytes!; - ConverterBase = matchingProperty.EffectiveConverter; + ParameterType = matchingProperty.PropertyType; + NameAsUtf8Bytes = matchingProperty.NameAsUtf8Bytes; IgnoreNullTokensOnRead = matchingProperty.IgnoreNullTokensOnRead; NumberHandling = matchingProperty.EffectiveNumberHandling; - MatchingPropertyCanBeNull = matchingProperty.PropertyTypeCanBeNull; - } - - /// - /// Create a parameter that is ignored at run time. It uses the same type (typeof(sbyte)) to help - /// prevent issues with unsupported types and helps ensure we don't accidently (de)serialize it. - /// - public static JsonParameterInfo CreateIgnoredParameterPlaceholder( - JsonParameterInfoValues parameterInfo, - JsonPropertyInfo matchingProperty, - bool sourceGenMode) - { - JsonParameterInfo jsonParameterInfo = new JsonParameterInfo(); - jsonParameterInfo.ClrInfo = parameterInfo; - jsonParameterInfo.PropertyType = matchingProperty.PropertyType; - jsonParameterInfo.NameAsUtf8Bytes = matchingProperty.NameAsUtf8Bytes!; - - // TODO: https://github.com/dotnet/runtime/issues/60082. - // Default value initialization for params mapping to ignored properties doesn't - // account for the default value of optional parameters. This should be fixed. - - if (sourceGenMode) - { - // The value in the matching JsonPropertyInfo instance matches the parameter type. - jsonParameterInfo.DefaultValue = matchingProperty.DefaultValue; - } - else - { - // The value in the created JsonPropertyInfo instance (sbyte) - // doesn't match the parameter type, use reflection to get the default value. - Type parameterType = parameterInfo.ParameterType; - - DefaultValueHolder holder; - if (matchingProperty.Options.TryGetTypeInfoCached(parameterType, out JsonTypeInfo? typeInfo)) - { - holder = typeInfo.DefaultValueHolder; - } - else - { - holder = DefaultValueHolder.CreateHolder(parameterInfo.ParameterType); - } - - jsonParameterInfo.DefaultValue = holder.DefaultValue; - } - - return jsonParameterInfo; } } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonParameterInfoOfT.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonParameterInfoOfT.cs index 180bd8c..5c1f57a 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonParameterInfoOfT.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonParameterInfoOfT.cs @@ -11,36 +11,22 @@ namespace System.Text.Json.Serialization.Metadata /// internal sealed class JsonParameterInfo : JsonParameterInfo { - public T TypedDefaultValue { get; private set; } = default!; + public new JsonConverter EffectiveConverter => MatchingProperty.EffectiveConverter; + public new JsonPropertyInfo MatchingProperty { get; } + public new T? DefaultValue { get; } - public override void Initialize(JsonParameterInfoValues parameterInfo, JsonPropertyInfo matchingProperty, JsonSerializerOptions options) + public JsonParameterInfo(JsonParameterInfoValues parameterInfoValues, JsonPropertyInfo matchingPropertyInfo) + : base(parameterInfoValues, matchingPropertyInfo) { - base.Initialize(parameterInfo, matchingProperty, options); - InitializeDefaultValue(matchingProperty); - } - - private void InitializeDefaultValue(JsonPropertyInfo matchingProperty) - { - Debug.Assert(ClrInfo.ParameterType == matchingProperty.PropertyType); + Debug.Assert(parameterInfoValues.ParameterType == typeof(T)); + Debug.Assert(matchingPropertyInfo.IsConfigured); - if (ClrInfo.HasDefaultValue) - { - object? defaultValue = ClrInfo.DefaultValue; + MatchingProperty = matchingPropertyInfo; + DefaultValue = parameterInfoValues.HasDefaultValue && parameterInfoValues.DefaultValue is not null + ? (T)parameterInfoValues.DefaultValue + : default; - if (defaultValue == null && !matchingProperty.PropertyTypeCanBeNull) - { - DefaultValue = TypedDefaultValue; - } - else - { - DefaultValue = defaultValue; - TypedDefaultValue = (T)defaultValue!; - } - } - else - { - DefaultValue = TypedDefaultValue; - } + base.DefaultValue = DefaultValue; } } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs index 90bfd38..220f489 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs @@ -527,6 +527,11 @@ namespace System.Text.Json.Serialization.Metadata || (shouldCheckForRequiredKeyword && memberInfo.HasRequiredMemberAttribute()); } + /// + /// Creates a instance whose type matches that of the current property. + /// + internal abstract JsonParameterInfo CreateJsonParameterInfo(JsonParameterInfoValues parameterInfoValues); + internal abstract bool GetMemberAndWriteJson(object obj, ref WriteStack state, Utf8JsonWriter writer); internal abstract bool GetMemberAndWriteJsonExtensionData(object obj, ref WriteStack state, Utf8JsonWriter writer); diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfoOfT.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfoOfT.cs index 772f589..f813f2c 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfoOfT.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfoOfT.cs @@ -111,6 +111,8 @@ namespace System.Text.Json.Serialization.Metadata internal override object? DefaultValue => default(T); internal override bool PropertyTypeCanBeNull => default(T) is null; + internal override JsonParameterInfo CreateJsonParameterInfo(JsonParameterInfoValues parameterInfoValues) + => new JsonParameterInfo(parameterInfoValues, this); internal new JsonConverter EffectiveConverter { diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs index 123b5a1..e8d5e1b 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs @@ -25,7 +25,7 @@ namespace System.Text.Json.Serialization.Metadata internal const string JsonObjectTypeName = "System.Text.Json.Nodes.JsonObject"; - internal delegate T ParameterizedConstructorDelegate(TArg0 arg0, TArg1 arg1, TArg2 arg2, TArg3 arg3); + internal delegate T ParameterizedConstructorDelegate(TArg0? arg0, TArg1? arg1, TArg2? arg2, TArg3? arg3); /// /// Indices of required properties. @@ -422,12 +422,6 @@ namespace System.Text.Json.Serialization.Metadata private protected abstract JsonPropertyInfo CreatePropertyInfoForTypeInfo(); /// - /// Returns a helper class used for computing the default value. - /// - internal DefaultValueHolder DefaultValueHolder => _defaultValueHolder ??= DefaultValueHolder.CreateHolder(Type); - private DefaultValueHolder? _defaultValueHolder; - - /// /// Gets or sets the type-level override. /// /// @@ -1090,8 +1084,6 @@ namespace System.Text.Json.Serialization.Metadata Debug.Assert(ParameterCache is null); JsonParameterInfoValues[] jsonParameters = ParameterInfoValues ?? Array.Empty(); - bool sourceGenMode = Options.TypeInfoResolver is JsonSerializerContext; - var parameterCache = new JsonPropertyDictionary(Options.PropertyNameCaseInsensitive, jsonParameters.Length); // Cache the lookup from object property name to JsonPropertyInfo using a case-insensitive comparer. @@ -1136,7 +1128,7 @@ namespace System.Text.Json.Serialization.Metadata Debug.Assert(matchingEntry.JsonPropertyInfo != null); JsonPropertyInfo jsonPropertyInfo = matchingEntry.JsonPropertyInfo; - JsonParameterInfo jsonParameterInfo = CreateConstructorParameter(parameterInfo, jsonPropertyInfo, sourceGenMode, Options); + JsonParameterInfo jsonParameterInfo = jsonPropertyInfo.CreateJsonParameterInfo(parameterInfo); parameterCache.Add(jsonPropertyInfo.Name, jsonParameterInfo); } // It is invalid for the extension data property to bind to a constructor argument. @@ -1256,25 +1248,6 @@ namespace System.Text.Json.Serialization.Metadata return new JsonPropertyDictionary(Options.PropertyNameCaseInsensitive, capacity); } - private static JsonParameterInfo CreateConstructorParameter( - JsonParameterInfoValues parameterInfo, - JsonPropertyInfo jsonPropertyInfo, - bool sourceGenMode, - JsonSerializerOptions options) - { - if (jsonPropertyInfo.IsIgnored) - { - return JsonParameterInfo.CreateIgnoredParameterPlaceholder(parameterInfo, jsonPropertyInfo, sourceGenMode); - } - - JsonConverter converter = jsonPropertyInfo.EffectiveConverter; - JsonParameterInfo jsonParameterInfo = converter.CreateJsonParameterInfo(); - - jsonParameterInfo.Initialize(parameterInfo, jsonPropertyInfo, options); - - return jsonParameterInfo; - } - private static JsonTypeInfoKind GetTypeInfoKind(Type type, JsonConverter converter) { if (type == typeof(object) && converter.CanBePolymorphic) diff --git a/src/libraries/System.Text.Json/tests/Common/ConstructorTests/ConstructorTests.ParameterMatching.cs b/src/libraries/System.Text.Json/tests/Common/ConstructorTests/ConstructorTests.ParameterMatching.cs index 8256b89..1199c7a 100644 --- a/src/libraries/System.Text.Json/tests/Common/ConstructorTests/ConstructorTests.ParameterMatching.cs +++ b/src/libraries/System.Text.Json/tests/Common/ConstructorTests/ConstructorTests.ParameterMatching.cs @@ -361,6 +361,14 @@ namespace System.Text.Json.Serialization.Tests } [Fact] + public async Task IgnoreNullValues_SetDefaultConstructorParameter_ToConstructorArguments_ThatCanBeNull() + { + var options = new JsonSerializerOptions { IgnoreNullValues = true }; + NullArgTester result = await Serializer.DeserializeWrapper(@"{""String"":null}", options); + Assert.Equal("defaultStr", result.String); + } + + [Fact] public async Task NumerousSimpleAndComplexParameters() { var obj = await Serializer.DeserializeWrapper(ObjWCtorMixedParams.s_json); @@ -1219,7 +1227,7 @@ namespace System.Text.Json.Serialization.Tests { string json = @"{""Prop"":20}"; var obj1 = await Serializer.DeserializeWrapper(json); - Assert.Equal(0, obj1.Prop); + Assert.Equal(5, obj1.Prop); var obj2 = await Serializer.DeserializeWrapper(json); Assert.Equal(0, obj2.Prop); @@ -1248,7 +1256,7 @@ namespace System.Text.Json.Serialization.Tests { string json = @"{""Prop"":20}"; var obj1 = await Serializer.DeserializeWrapper(json); - Assert.Equal(0, obj1.Prop); + Assert.Equal(5, obj1.Prop); var obj2 = await Serializer.DeserializeWrapper(json); Assert.Equal(0, obj2.Prop); @@ -1532,5 +1540,64 @@ namespace System.Text.Json.Serialization.Tests ULong = @ulong; } } + + [Fact] + public async Task TestTypeWithEnumParameters() + { + // Regression test for https://github.com/dotnet/runtime/issues/68647 + + var value = new TypeWithEnumParameters(); + + string json = await Serializer.SerializeWrapper(value); + Assert.Equal("""{"Value":"One","NullableValue":"Two"}""", json); + + value = await Serializer.DeserializeWrapper(json); + Assert.Equal(MyEnum.One, value.Value); + Assert.Equal(MyEnum.Two, value.NullableValue); + + value = await Serializer.DeserializeWrapper("{}"); + Assert.Equal(MyEnum.One, value.Value); + Assert.Equal(MyEnum.Two, value.NullableValue); + } + + public sealed class TypeWithEnumParameters + { + public MyEnum Value { get; } + public MyEnum? NullableValue { get; } + + [JsonConstructor] + public TypeWithEnumParameters(MyEnum value = MyEnum.One, MyEnum? nullableValue = MyEnum.Two) + { + Value = value; + NullableValue = nullableValue; + } + } + + [JsonConverter(typeof(JsonStringEnumConverter))] + public enum MyEnum + { + One = 1, + Two = 2, + } + + [Fact] + public async Task TestClassWithIgnoredPropertyDefaultParam() + { + // Regression test for https://github.com/dotnet/runtime/issues/60082 + + string json = "{}"; + ClassWithIgnoredPropertyDefaultParam result = await Serializer.DeserializeWrapper(json); + Assert.Equal(5, result.Y); + } + + public class ClassWithIgnoredPropertyDefaultParam + { + public int X { get; } + + [JsonIgnore] + public int Y { get; } + + public ClassWithIgnoredPropertyDefaultParam(int x, int y = 5) => (X, Y) = (x, y); + } } } diff --git a/src/libraries/System.Text.Json/tests/Common/TestClasses/TestClasses.Constructor.cs b/src/libraries/System.Text.Json/tests/Common/TestClasses/TestClasses.Constructor.cs index 28e9e2d..f4bf80d 100644 --- a/src/libraries/System.Text.Json/tests/Common/TestClasses/TestClasses.Constructor.cs +++ b/src/libraries/System.Text.Json/tests/Common/TestClasses/TestClasses.Constructor.cs @@ -907,12 +907,14 @@ namespace System.Text.Json.Serialization.Tests public Point_3D_Struct Point3DStruct { get; } public ImmutableArray ImmutableArray { get; } public int Int { get; } + public string String { get; } - public NullArgTester(Point_3D_Struct point3DStruct, ImmutableArray immutableArray, int @int = 50) + public NullArgTester(Point_3D_Struct point3DStruct, ImmutableArray immutableArray, int @int = 50, string @string = "defaultStr") { Point3DStruct = point3DStruct; ImmutableArray = immutableArray; Int = @int; + String = @string; } } @@ -2057,7 +2059,7 @@ namespace System.Text.Json.Serialization.Tests public void Verify() { Assert.Equal(0, X); - Assert.Equal(0, Y); // We don't set parameter default value here. + Assert.Equal(5, Y); // We use the default parameter of the constructor. } } diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/ConstructorTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/ConstructorTests.cs index c4a5905..b41c9ed 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/ConstructorTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/ConstructorTests.cs @@ -143,6 +143,8 @@ namespace System.Text.Json.SourceGeneration.Tests [JsonSerializable(typeof(ClassWithManyConstructorParameters))] [JsonSerializable(typeof(ClassWithInvalidArray))] [JsonSerializable(typeof(ClassWithInvalidDictionary))] + [JsonSerializable(typeof(TypeWithEnumParameters))] + [JsonSerializable(typeof(ClassWithIgnoredPropertyDefaultParam))] internal sealed partial class ConstructorTestsContext_Metadata : JsonSerializerContext { } @@ -281,6 +283,8 @@ namespace System.Text.Json.SourceGeneration.Tests [JsonSerializable(typeof(ClassWithManyConstructorParameters))] [JsonSerializable(typeof(ClassWithInvalidArray))] [JsonSerializable(typeof(ClassWithInvalidDictionary))] + [JsonSerializable(typeof(TypeWithEnumParameters))] + [JsonSerializable(typeof(ClassWithIgnoredPropertyDefaultParam))] internal sealed partial class ConstructorTestsContext_Default : JsonSerializerContext { }