Address feedback

src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs

@@ -797,6 +797,7 @@ private string GenerateCtorParamMetadataInitFunc(TypeGenerationSpec typeGenerati
 private static {JsonParameterInfoValuesTypeRef}[] {typeGenerationSpec.TypeInfoPropertyName}{CtorParamInitMethodNameSuffix}()
 {{
     {JsonParameterInfoValuesTypeRef}[] {parametersVarName} = new {JsonParameterInfoValuesTypeRef}[{paramCount}];
+    {JsonParameterInfoValuesTypeRef} info;
 ");
 
                 for (int i = 0; i < paramCount; i++)
@@ -806,19 +807,18 @@ private string GenerateCtorParamMetadataInitFunc(TypeGenerationSpec typeGenerati
                     string parameterTypeRef = reflectionInfo.ParameterType.GetCompilableName();
 
                     object? defaultValue = reflectionInfo.GetDefaultValue();
-                    string defaultValueAsStr = defaultValue == null
-                        ? $"default({parameterTypeRef})"
-                        : GetParamDefaultValueAsString(defaultValue);
+                    string defaultValueAsStr = GetParamDefaultValueAsString(defaultValue, parameterTypeRef);
 
                     sb.Append(@$"
-    {parametersVarName}[{i}] = new()
+    {InfoVarName} = new()
     {{
         Name = ""{reflectionInfo.Name!}"",
         ParameterType = typeof({parameterTypeRef}),
         Position = {reflectionInfo.Position},
         HasDefaultValue = {ToCSharpKeyword(reflectionInfo.HasDefaultValue)},
         DefaultValue = {defaultValueAsStr}
     }};
+    {parametersVarName}[{i}] = {InfoVarName};
 ");
                 }
 
@@ -1318,12 +1318,12 @@ private static string GetNumberHandlingAsStr(JsonNumberHandling? numberHandling)
 
         private static string ToCSharpKeyword(bool value) => value.ToString().ToLowerInvariant();
 
-        private static string GetParamDefaultValueAsString(object? value)
+        private static string GetParamDefaultValueAsString(object? value, string objectTypeAsStr)
         {
             switch (value)
             {
                 case null:
-                    return "null";
+                    return $"default({objectTypeAsStr})";
                 case bool boolVal:
                     return ToCSharpKeyword(boolVal);
                 default:

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.Small.cs

@@ -92,32 +92,34 @@ protected override void InitializeConstructorArgumentCaches(ref ReadStack state,
                 JsonParameterInfo? parameterInfo = cache[i].Value;
                 Debug.Assert(parameterInfo != null);
 
-                switch (parameterInfo.ClrInfo.Position)
+                // 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)
                 {
-                    case 0:
-                        arguments.Arg0 = GetDefaultValue<TArg0>(parameterInfo);
-                        break;
-                    case 1:
-                        arguments.Arg1 = GetDefaultValue<TArg1>(parameterInfo);
-                        break;
-                    case 2:
-                        arguments.Arg2 = GetDefaultValue<TArg2>(parameterInfo);
-                        break;
-                    case 3:
-                        arguments.Arg3 = GetDefaultValue<TArg3>(parameterInfo);
-                        break;
-                    default:
-                        Debug.Fail("More than 4 params: we should be in override for LargeObjectWithParameterizedConstructorConverter.");
-                        throw new InvalidOperationException();
+                    int position = parameterInfo.ClrInfo.Position;
+
+                    switch (position)
+                    {
+                        case 0:
+                            arguments.Arg0 = ((JsonParameterInfo<TArg0>)parameterInfo).TypedDefaultValue!;
+                            break;
+                        case 1:
+                            arguments.Arg1 = ((JsonParameterInfo<TArg1>)parameterInfo).TypedDefaultValue!;
+                            break;
+                        case 2:
+                            arguments.Arg2 = ((JsonParameterInfo<TArg2>)parameterInfo).TypedDefaultValue!;
+                            break;
+                        case 3:
+                            arguments.Arg3 = ((JsonParameterInfo<TArg3>)parameterInfo).TypedDefaultValue!;
+                            break;
+                        default:
+                            Debug.Fail("More than 4 params: we should be in override for LargeObjectWithParameterizedConstructorConverter.");
+                            throw new InvalidOperationException();
+                    }
                 }
             }
 
             state.Current.CtorArgumentState!.Arguments = arguments;
-
-            static TArg GetDefaultValue<TArg>(JsonParameterInfo parameterInfo)
-                => parameterInfo.ShouldDeserialize
-                ? ((JsonParameterInfo<TArg>)parameterInfo).TypedDefaultValue
-                : parameterInfo.ClrInfo.HasDefaultValue ? (TArg?)parameterInfo.DefaultValue! : default(TArg)!;
         }
     }
 }

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/GenericMethodHolder.cs

@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Text.Json.Reflection;
@@ -12,17 +13,56 @@ namespace System.Text.Json.Serialization.Metadata
     /// </summary>
     internal abstract class GenericMethodHolder
     {
+        private static ConcurrentDictionary<Type, GenericMethodHolder>? s_holders;
+
+        /// <summary>
+        /// Returns the default value for the specified type.
+        /// </summary>
+        public abstract object? DefaultValue { get; }
+
         /// <summary>
         /// Returns true if <param name="value"/> contains only default values.
         /// </summary>
         public abstract bool IsDefaultValue(object value);
+
+        /// <summary>
+        /// Returns a holder instance representing a type.
+        /// </summary>
+        public static GenericMethodHolder GetHolder(Type type)
+        {
+            if (s_holders == null)
+            {
+                s_holders = new ConcurrentDictionary<Type, GenericMethodHolder>();
+                return CreateAndCacheHolder();
+            }
+            else
+            {
+                if (!s_holders.TryGetValue(type, out GenericMethodHolder? holder))
+                {
+                    holder = CreateAndCacheHolder();
+                }
+
+                return holder;
+            }
+
+            GenericMethodHolder CreateAndCacheHolder()
+            {
+                Type holderType = typeof(GenericMethodHolder<>).MakeGenericType(type);
+                GenericMethodHolder holderInstance = (GenericMethodHolder)Activator.CreateInstance(holderType)!;
+                Debug.Assert(s_holders != null);
+                s_holders[type] = holderInstance;
+                return holderInstance;
+            }
+        }
     }
 
     /// <summary>
     /// Generic methods for {T}.
     /// </summary>
     internal sealed class GenericMethodHolder<T> : GenericMethodHolder
     {
+        public override object? DefaultValue => default(T);
+
         public override bool IsDefaultValue(object value)
         {
             // For performance, we only want to call this method for non-nullable value types.

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonParameterInfo.cs

@@ -72,15 +72,37 @@ public virtual void Initialize(JsonParameterInfoValues parameterInfo, JsonProper
             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)
+        /// <summary>
+        /// 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.
+        /// </summary>
+        public static JsonParameterInfo CreateIgnoredParameterPlaceholder(
+            JsonParameterInfoValues parameterInfo,
+            JsonPropertyInfo matchingProperty,
+            bool sourceGenMode)
         {
             JsonParameterInfo jsonParameterInfo = new JsonParameterInfo<sbyte>();
             jsonParameterInfo.ClrInfo = parameterInfo;
             jsonParameterInfo.RuntimePropertyType = matchingProperty.RuntimePropertyType!;
             jsonParameterInfo.NameAsUtf8Bytes = matchingProperty.NameAsUtf8Bytes!;
-            jsonParameterInfo.DefaultValue = parameterInfo.DefaultValue;
+
+            // 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 <T> value in the matching JsonPropertyInfo<T> instance matches the parameter type.
+                jsonParameterInfo.DefaultValue = matchingProperty.DefaultValue;
+            }
+            else
+            {
+                // The <T> value in the created JsonPropertyInfo<T> instance (sbyte)
+                // doesn't match the parameter type, use reflection to get the default value.
+                GenericMethodHolder holder = GenericMethodHolder.GetHolder(parameterInfo.ParameterType);
+                jsonParameterInfo.DefaultValue = holder.DefaultValue;
+            }
+
             return jsonParameterInfo;
         }
     }

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs

@@ -523,5 +523,10 @@ internal JsonTypeInfo RuntimeTypeInfo
         internal string? ClrName { get; set; }
 
         internal bool IsVirtual { get; set; }
+
+        /// <summary>
+        /// Default value used for parameterized ctor invocation.
+        /// </summary>
+        internal abstract object? DefaultValue { get; }
     }
 }

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfoOfT.cs

@@ -28,8 +28,11 @@ internal sealed class JsonPropertyInfo<T> : JsonPropertyInfo
         private bool _propertyTypeEqualsTypeToConvert;
 
         internal Func<object, T>? Get { get; set; }
+
         internal Action<object, T>? Set { get; set; }
 
+        internal override object? DefaultValue => default(T);
+
         public JsonConverter<T> Converter { get; internal set; } = null!;
 
         internal override void Initialize(

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.Cache.cs

@@ -646,7 +646,7 @@ internal void InitializeParameterCache()
                 return;
             }
 
-            InitializeConstructorParameters(array);
+            InitializeConstructorParameters(array, sourceGenMode: true);
             Debug.Assert(ParameterCache != null);
         }
     }

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs

@@ -133,12 +133,7 @@ internal GenericMethodHolder GenericMethods
         {
             get
             {
-                if (_genericMethods == null)
-                {
-                    Type runtimePropertyClass = typeof(GenericMethodHolder<>).MakeGenericType(new Type[] { Type })!;
-                    _genericMethods = (GenericMethodHolder)Activator.CreateInstance(runtimePropertyClass)!;
-                }
-
+                _genericMethods ??= GenericMethodHolder.GetHolder(Type);
                 return _genericMethods;
             }
         }
@@ -454,7 +449,7 @@ public ParameterLookupValue(JsonPropertyInfo jsonPropertyInfo)
             public JsonPropertyInfo JsonPropertyInfo { get; }
         }
 
-        private void InitializeConstructorParameters(JsonParameterInfoValues[] jsonParameters)
+        private void InitializeConstructorParameters(JsonParameterInfoValues[] jsonParameters, bool sourceGenMode = false)
         {
             var parameterCache = new JsonPropertyDictionary<JsonParameterInfo>(Options.PropertyNameCaseInsensitive, jsonParameters.Length);
 
@@ -500,7 +495,7 @@ private void InitializeConstructorParameters(JsonParameterInfoValues[] jsonParam
 
                     Debug.Assert(matchingEntry.JsonPropertyInfo != null);
                     JsonPropertyInfo jsonPropertyInfo = matchingEntry.JsonPropertyInfo;
-                    JsonParameterInfo jsonParameterInfo = CreateConstructorParameter(parameterInfo, jsonPropertyInfo, Options);
+                    JsonParameterInfo jsonParameterInfo = CreateConstructorParameter(parameterInfo, jsonPropertyInfo, sourceGenMode, Options);
                     parameterCache.Add(jsonPropertyInfo.NameAsString, jsonParameterInfo);
                 }
                 // It is invalid for the extension data property to bind with a constructor argument.
@@ -580,11 +575,12 @@ private bool IsValidDataExtensionProperty(JsonPropertyInfo jsonPropertyInfo)
         private static JsonParameterInfo CreateConstructorParameter(
             JsonParameterInfoValues parameterInfo,
             JsonPropertyInfo jsonPropertyInfo,
+            bool sourceGenMode,
             JsonSerializerOptions options)
         {
             if (jsonPropertyInfo.IsIgnored)
             {
-                return JsonParameterInfo.CreateIgnoredParameterPlaceholder(parameterInfo, jsonPropertyInfo);
+                return JsonParameterInfo.CreateIgnoredParameterPlaceholder(parameterInfo, jsonPropertyInfo, sourceGenMode);
             }
 
             JsonConverter converter = jsonPropertyInfo.ConverterBase;

src/libraries/System.Text.Json/tests/Common/ConstructorTests/ConstructorTests.ParameterMatching.cs

@@ -1216,48 +1216,5 @@ public class TypeWithUri
 
             public TypeWithUri(Uri myUri = default) => MyUri = myUri;
         }
-
-        [Fact]
-        public async Task SmallObject_DefaultParamValueUsed_WhenMatchingPropIgnored()
-        {
-            string json = @"{""Prop"":20}";
-            var obj = await JsonSerializerWrapperForString.DeserializeWrapper<SmallType_IgnoredProp_Bind_ParamWithDefaultValue>(json);
-            Assert.Equal(5, obj.Prop);
-        }
-
-        public class SmallType_IgnoredProp_Bind_ParamWithDefaultValue
-        {
-            [JsonIgnore]
-            public int Prop { get; set; }
-
-            public SmallType_IgnoredProp_Bind_ParamWithDefaultValue(int prop = 5)
-                => Prop = prop;
-        }
-
-
-        [Fact]
-        public async Task LargeObject_DefaultParamValueUsed_WhenMatchingPropIgnored()
-        {
-            string json = @"{""Prop"":20}";
-            var obj = await JsonSerializerWrapperForString.DeserializeWrapper<LargeType_IgnoredProp_Bind_ParamWithDefaultValue>(json);
-            Assert.Equal(5, obj.Prop);
-        }
-
-        public class LargeType_IgnoredProp_Bind_ParamWithDefaultValue
-        {
-            public int W { get; set; }
-
-            public int X { get; set; }
-
-            public int Y { get; set; }
-
-            public int Z { get; set; }
-
-            [JsonIgnore]
-            public int Prop { get; set; }
-
-            public LargeType_IgnoredProp_Bind_ParamWithDefaultValue(int w, int x, int y, int z, int prop = 5)
-                => Prop = prop;
-        }
     }
 }

src/libraries/System.Text.Json/tests/Common/TestClasses/TestClasses.Constructor.cs

@@ -2056,7 +2056,7 @@ public void Initialize() { }
         public void Verify()
         {
             Assert.Equal(0, X);
-            Assert.Equal(5, Y);
+            Assert.Equal(0, Y); // We don't set parameter default value here.
         }
     }
 

src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/ConstructorTests.cs

@@ -125,8 +125,6 @@ protected ConstructorTests_Metadata(JsonSerializerWrapperForString stringWrapper
         [JsonSerializable(typeof(JsonElement))]
         [JsonSerializable(typeof(Parameterized_Class_With_ComplexTuple))]
         [JsonSerializable(typeof(Parameterized_Person_Simple))]
-        [JsonSerializable(typeof(SmallType_IgnoredProp_Bind_ParamWithDefaultValue))]
-        [JsonSerializable(typeof(LargeType_IgnoredProp_Bind_ParamWithDefaultValue))]
         internal sealed partial class ConstructorTestsContext_Metadata : JsonSerializerContext
         {
         }
@@ -243,8 +241,6 @@ public ConstructorTests_Default()
         [JsonSerializable(typeof(JsonElement))]
         [JsonSerializable(typeof(Parameterized_Class_With_ComplexTuple))]
         [JsonSerializable(typeof(Parameterized_Person_Simple))]
-        [JsonSerializable(typeof(SmallType_IgnoredProp_Bind_ParamWithDefaultValue))]
-        [JsonSerializable(typeof(LargeType_IgnoredProp_Bind_ParamWithDefaultValue))]
         internal sealed partial class ConstructorTestsContext_Default : JsonSerializerContext
         {
         }