Skip to content

Commit 7ec428c

Browse files
layomialayomia
committed
Fix bugs with generation for ctor param default values (dotnet#62798)
* Fix bugs with generation for ctor param default values * Address review feedback * Address feedback
1 parent 25d2a1f commit 7ec428c

File tree

8 files changed

+224
-30
lines changed

8 files changed

+224
-30
lines changed

‎src/libraries/System.Text.Json/Common/JsonConstants.cs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,9 @@ internal static partial class JsonConstants
77
{
88
// The maximum number of parameters a constructor can have where it can be supported by the serializer.
99
public const int MaxParameterCount = 64;
10+
11+
// Standard format for double and single on non-inbox frameworks.
12+
public const string DoubleFormatString = "G17";
13+
public const string SingleFormatString = "G9";
1014
}
1115
}

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

Lines changed: 70 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44
using System.Collections.Generic;
55
using System.Diagnostics;
66
using System.Diagnostics.CodeAnalysis;
7+
using System.Globalization;
78
using System.Reflection;
9+
using System.Reflection.Metadata;
810
using System.Text.Json;
911
using System.Text.Json.Reflection;
1012
using System.Text.Json.Serialization;
@@ -759,17 +761,17 @@ private string GeneratePropMetadataInitFunc(TypeGenerationSpec typeGenerationSpe
759761
sb.Append($@"
760762
{JsonPropertyInfoValuesTypeRef}<{memberTypeCompilableName}> {infoVarName} = new {JsonPropertyInfoValuesTypeRef}<{memberTypeCompilableName}>()
761763
{{
762-
IsProperty = {ToCSharpKeyword(memberMetadata.IsProperty)},
763-
IsPublic = {ToCSharpKeyword(memberMetadata.IsPublic)},
764-
IsVirtual = {ToCSharpKeyword(memberMetadata.IsVirtual)},
764+
IsProperty = {FormatBool(memberMetadata.IsProperty)},
765+
IsPublic = {FormatBool(memberMetadata.IsPublic)},
766+
IsVirtual = {FormatBool(memberMetadata.IsVirtual)},
765767
DeclaringType = typeof({memberMetadata.DeclaringTypeRef}),
766768
PropertyTypeInfo = {memberTypeFriendlyName},
767769
Converter = {converterValue},
768770
Getter = {getterValue},
769771
Setter = {setterValue},
770772
IgnoreCondition = {ignoreConditionNamedArg},
771-
HasJsonInclude = {ToCSharpKeyword(memberMetadata.HasJsonInclude)},
772-
IsExtensionData = {ToCSharpKeyword(memberMetadata.IsExtensionData)},
773+
HasJsonInclude = {FormatBool(memberMetadata.HasJsonInclude)},
774+
IsExtensionData = {FormatBool(memberMetadata.IsExtensionData)},
773775
NumberHandling = {GetNumberHandlingAsStr(memberMetadata.NumberHandling)},
774776
PropertyName = ""{clrPropertyName}"",
775777
JsonPropertyName = {jsonPropertyNameValue}
@@ -805,19 +807,19 @@ private string GenerateCtorParamMetadataInitFunc(TypeGenerationSpec typeGenerati
805807
for (int i = 0; i < paramCount; i++)
806808
{
807809
ParameterInfo reflectionInfo = parameters[i].ParameterInfo;
808-
809-
string parameterTypeRef = reflectionInfo.ParameterType.GetCompilableName();
810+
Type parameterType = reflectionInfo.ParameterType;
811+
string parameterTypeRef = parameterType.GetCompilableName();
810812

811813
object? defaultValue = reflectionInfo.GetDefaultValue();
812-
string defaultValueAsStr = GetParamDefaultValueAsString(defaultValue, parameterTypeRef);
814+
string defaultValueAsStr = GetParamDefaultValueAsString(defaultValue, parameterType, parameterTypeRef);
813815

814816
sb.Append(@$"
815817
{InfoVarName} = new()
816818
{{
817819
Name = ""{reflectionInfo.Name!}"",
818820
ParameterType = typeof({parameterTypeRef}),
819821
Position = {reflectionInfo.Position},
820-
HasDefaultValue = {ToCSharpKeyword(reflectionInfo.HasDefaultValue)},
822+
HasDefaultValue = {FormatBool(reflectionInfo.HasDefaultValue)},
821823
DefaultValue = {defaultValueAsStr}
822824
}};
823825
{parametersVarName}[{i}] = {InfoVarName};
@@ -1192,10 +1194,10 @@ private string GetLogicForDefaultSerializerOptionsInit()
11921194
private static {JsonSerializerOptionsTypeRef} {DefaultOptionsStaticVarName} {{ get; }} = new {JsonSerializerOptionsTypeRef}()
11931195
{{
11941196
DefaultIgnoreCondition = {JsonIgnoreConditionTypeRef}.{options.DefaultIgnoreCondition},
1195-
IgnoreReadOnlyFields = {ToCSharpKeyword(options.IgnoreReadOnlyFields)},
1196-
IgnoreReadOnlyProperties = {ToCSharpKeyword(options.IgnoreReadOnlyProperties)},
1197-
IncludeFields = {ToCSharpKeyword(options.IncludeFields)},
1198-
WriteIndented = {ToCSharpKeyword(options.WriteIndented)},{namingPolicyInit}
1197+
IgnoreReadOnlyFields = {FormatBool(options.IgnoreReadOnlyFields)},
1198+
IgnoreReadOnlyProperties = {FormatBool(options.IgnoreReadOnlyProperties)},
1199+
IncludeFields = {FormatBool(options.IncludeFields)},
1200+
WriteIndented = {FormatBool(options.WriteIndented)},{namingPolicyInit}
11991201
}};";
12001202
}
12011203

@@ -1316,20 +1318,64 @@ private static string GetNumberHandlingAsStr(JsonNumberHandling? numberHandling)
13161318
: "default";
13171319

13181320
private static string GetCreateValueInfoMethodRef(string typeCompilableName) => $"{CreateValueInfoMethodName}<{typeCompilableName}>";
1319-
}
13201321

1321-
private static string ToCSharpKeyword(bool value) => value.ToString().ToLowerInvariant();
1322+
private static string FormatBool(bool value) => value ? "true" : "false";
13221323

1323-
private static string GetParamDefaultValueAsString(object? value, string objectTypeAsStr)
1324-
{
1325-
switch (value)
1324+
private string GetParamDefaultValueAsString(object? value, Type type, string typeRef)
13261325
{
1327-
case null:
1328-
return $"default({objectTypeAsStr})";
1329-
case bool boolVal:
1330-
return ToCSharpKeyword(boolVal);
1331-
default:
1332-
return value!.ToString();
1326+
if (value == null)
1327+
{
1328+
return $"default({typeRef})";
1329+
}
1330+
1331+
if (type.IsEnum)
1332+
{
1333+
// Roslyn gives us an instance of the underlying type, which is numerical.
1334+
#if DEBUG
1335+
Type runtimeType = _generationSpec.MetadataLoadContext.Resolve(value.GetType());
1336+
Debug.Assert(_generationSpec.IsNumberType(runtimeType));
1337+
#endif
1338+
1339+
// Return the numeric value.
1340+
return FormatNumber();
1341+
}
1342+
1343+
switch (value)
1344+
{
1345+
case string @string:
1346+
return SymbolDisplay.FormatLiteral(@string, quote: true); ;
1347+
case char @char:
1348+
return SymbolDisplay.FormatLiteral(@char, quote: true);
1349+
case double.NegativeInfinity:
1350+
return "double.NegativeInfinity";
1351+
case double.PositiveInfinity:
1352+
return "double.PositiveInfinity";
1353+
case double.NaN:
1354+
return "double.NaN";
1355+
case double @double:
1356+
return $"({typeRef})({@double.ToString(JsonConstants.DoubleFormatString, CultureInfo.InvariantCulture)})";
1357+
case float.NegativeInfinity:
1358+
return "float.NegativeInfinity";
1359+
case float.PositiveInfinity:
1360+
return "float.PositiveInfinity";
1361+
case float.NaN:
1362+
return "float.NaN";
1363+
case float @float:
1364+
return $"({typeRef})({@float.ToString(JsonConstants.SingleFormatString, CultureInfo.InvariantCulture)})";
1365+
case decimal.MaxValue:
1366+
return "decimal.MaxValue";
1367+
case decimal.MinValue:
1368+
return "decimal.MinValue";
1369+
case decimal @decimal:
1370+
return @decimal.ToString(CultureInfo.InvariantCulture);
1371+
case bool @bool:
1372+
return FormatBool(@bool);
1373+
default:
1374+
// Assume this is a number.
1375+
return FormatNumber();
1376+
}
1377+
1378+
string FormatNumber() => $"({typeRef})({Convert.ToString(value, CultureInfo.InvariantCulture)})";
13331379
}
13341380
}
13351381
}

‎src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,9 @@ public Parser(Compilation compilation, in JsonSourceGenerationContext sourceGene
375375
GuidType = _guidType,
376376
StringType = _stringType,
377377
NumberTypes = _numberTypes,
378+
#if DEBUG
379+
MetadataLoadContext = _metadataLoadContext,
380+
#endif
378381
};
379382
}
380383

‎src/libraries/System.Text.Json/gen/SourceGenerationSpec.cs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,18 @@
44
using System;
55
using System.Collections.Generic;
66
using System.Text;
7+
using System.Text.Json.Reflection;
8+
using Microsoft.CodeAnalysis;
79

810
namespace System.Text.Json.SourceGeneration
911
{
1012
internal sealed class SourceGenerationSpec
1113
{
1214
public List<ContextGenerationSpec> ContextGenerationSpecList { get; init; }
1315

16+
#if DEBUG
17+
public MetadataLoadContextInternal MetadataLoadContext { get; init; }
18+
#endif
1419
public Type BooleanType { get; init; }
1520
public Type ByteArrayType { get; init; }
1621
public Type CharType { get; init; }

‎src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteValues.Double.cs‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -109,9 +109,7 @@ private static bool TryFormatDouble(double value, Span<byte> destination, out in
109109
#if BUILDING_INBOX_LIBRARY
110110
return Utf8Formatter.TryFormat(value, destination, out bytesWritten);
111111
#else
112-
const string FormatString = "G17";
113-
114-
string utf16Text = value.ToString(FormatString, CultureInfo.InvariantCulture);
112+
string utf16Text = value.ToString(JsonConstants.DoubleFormatString, CultureInfo.InvariantCulture);
115113

116114
// Copy the value to the destination, if it's large enough.
117115

‎src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteValues.Float.cs‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -109,9 +109,7 @@ private static bool TryFormatSingle(float value, Span<byte> destination, out int
109109
#if BUILDING_INBOX_LIBRARY
110110
return Utf8Formatter.TryFormat(value, destination, out bytesWritten);
111111
#else
112-
const string FormatString = "G9";
113-
114-
string utf16Text = value.ToString(FormatString, CultureInfo.InvariantCulture);
112+
string utf16Text = value.ToString(JsonConstants.SingleFormatString, CultureInfo.InvariantCulture);
115113

116114
// Copy the value to the destination, if it's large enough.
117115

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

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System.Collections.Generic;
55
using System.Collections.Specialized;
66
using System.Diagnostics.CodeAnalysis;
7+
using System.Reflection;
78
using System.Threading.Tasks;
89
using Xunit;
910

@@ -1322,5 +1323,142 @@ public class ClassWithIgnoredSameType
13221323

13231324
public ClassWithIgnoredSameType(ClassWithIgnoredSameType prop) { }
13241325
}
1326+
1327+
public async Task TestClassWithDefaultCtorParams()
1328+
{
1329+
ClassWithDefaultCtorParams obj = new ClassWithDefaultCtorParams(
1330+
new Point_2D_Struct_WithAttribute(1, 2),
1331+
DayOfWeek.Sunday,
1332+
(DayOfWeek)(-2),
1333+
(DayOfWeek)19,
1334+
BindingFlags.CreateInstance | BindingFlags.FlattenHierarchy,
1335+
SampleEnumUInt32.MinZero,
1336+
"Hello world!",
1337+
null,
1338+
"xzy",
1339+
'c',
1340+
' ',
1341+
23,
1342+
4,
1343+
-40,
1344+
double.Epsilon,
1345+
23,
1346+
4,
1347+
-40,
1348+
float.MinValue,
1349+
1,
1350+
2,
1351+
3,
1352+
4,
1353+
5,
1354+
6,
1355+
7,
1356+
8,
1357+
9,
1358+
10);
1359+
1360+
string json = await JsonSerializerWrapperForString.SerializeWrapper(obj);
1361+
obj = await JsonSerializerWrapperForString.DeserializeWrapper<ClassWithDefaultCtorParams>(json);
1362+
JsonTestHelper.AssertJsonEqual(json, await JsonSerializerWrapperForString.SerializeWrapper(obj));
1363+
}
1364+
1365+
public class ClassWithDefaultCtorParams
1366+
{
1367+
public Point_2D_Struct_WithAttribute Struct { get; }
1368+
public DayOfWeek Enum1 { get; }
1369+
public DayOfWeek Enum2 { get; }
1370+
public DayOfWeek Enum3 { get; }
1371+
public BindingFlags Enum4 { get; }
1372+
public SampleEnumUInt32 Enum5 { get; }
1373+
public string Str1 { get; }
1374+
public string Str2 { get; }
1375+
public string Str3 { get; }
1376+
public char Char1 { get; }
1377+
public char Char2 { get; }
1378+
public double Double1 { get; }
1379+
public double Double2 { get; }
1380+
public double Double3 { get; }
1381+
public double Double4 { get; }
1382+
public double Double5 { get; }
1383+
public float Float1 { get; }
1384+
public float Float2 { get; }
1385+
public float Float3 { get; }
1386+
public float Float4 { get; }
1387+
public float Float5 { get; }
1388+
public byte Byte { get; }
1389+
public decimal Decimal1 { get; }
1390+
public decimal Decimal2 { get; }
1391+
public short Short { get; }
1392+
public sbyte Sbyte { get; }
1393+
public int Int { get; }
1394+
public long Long { get; }
1395+
public ushort UShort { get; }
1396+
public uint UInt { get; }
1397+
public ulong ULong { get; }
1398+
1399+
public ClassWithDefaultCtorParams(
1400+
Point_2D_Struct_WithAttribute @struct = default,
1401+
DayOfWeek enum1 = default,
1402+
DayOfWeek enum2 = DayOfWeek.Sunday,
1403+
DayOfWeek enum3 = DayOfWeek.Sunday | DayOfWeek.Monday,
1404+
BindingFlags enum4 = BindingFlags.CreateInstance | BindingFlags.ExactBinding,
1405+
SampleEnumUInt32 enum5 = SampleEnumUInt32.MinZero,
1406+
string str1 = "abc",
1407+
string str2 = "",
1408+
string str3 = "\n\r⁉️\'\"\u200D\f\t\v\0\a\b\\\'\"",
1409+
char char1 = 'a',
1410+
char char2 = '\u200D',
1411+
double double1 = double.NegativeInfinity,
1412+
double double2 = double.PositiveInfinity,
1413+
double double3 = double.NaN,
1414+
double double4 = double.MaxValue,
1415+
double double5 = double.Epsilon,
1416+
float float1 = float.NegativeInfinity,
1417+
float float2 = float.PositiveInfinity,
1418+
float float3 = float.NaN,
1419+
float float4 = float.MinValue,
1420+
float float5 = float.Epsilon,
1421+
byte @byte = byte.MinValue,
1422+
decimal @decimal1 = decimal.MinValue,
1423+
decimal @decimal2 = decimal.MaxValue,
1424+
short @short = short.MinValue,
1425+
sbyte @sbyte = sbyte.MaxValue,
1426+
int @int = int.MinValue,
1427+
long @long = long.MaxValue,
1428+
ushort @ushort = ushort.MinValue,
1429+
uint @uint = uint.MaxValue,
1430+
ulong @ulong = ulong.MinValue)
1431+
{
1432+
Struct = @struct;
1433+
Enum1 = enum1;
1434+
Enum2 = enum2;
1435+
Enum3 = enum3;
1436+
Enum4 = enum4;
1437+
Enum5 = enum5;
1438+
Str1 = str1;
1439+
Str2 = str2;
1440+
Str3 = str3;
1441+
Char1 = char1;
1442+
Char2 = char2;
1443+
Double1 = double1;
1444+
Double2 = double2;
1445+
Double3 = double3;
1446+
Double4 = double4;
1447+
Float1 = float1;
1448+
Float2 = float2;
1449+
Float3 = float3;
1450+
Float4 = float4;
1451+
Byte = @byte;
1452+
Decimal1 = @decimal1;
1453+
Decimal2 = @decimal2;
1454+
Short = @short;
1455+
Sbyte = @sbyte;
1456+
Int = @int;
1457+
Long = @long;
1458+
UShort = @ushort;
1459+
UInt = @uint;
1460+
ULong = @ulong;
1461+
}
1462+
}
13251463
}
13261464
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ protected ConstructorTests_Metadata(JsonSerializerWrapperForString stringWrapper
130130
[JsonSerializable(typeof(LargeType_IgnoredProp_Bind_ParamWithDefaultValue))]
131131
[JsonSerializable(typeof(LargeType_IgnoredProp_Bind_Param))]
132132
[JsonSerializable(typeof(ClassWithIgnoredSameType))]
133+
[JsonSerializable(typeof(ClassWithDefaultCtorParams))]
133134
internal sealed partial class ConstructorTestsContext_Metadata : JsonSerializerContext
134135
{
135136
}
@@ -251,6 +252,7 @@ public ConstructorTests_Default()
251252
[JsonSerializable(typeof(LargeType_IgnoredProp_Bind_ParamWithDefaultValue))]
252253
[JsonSerializable(typeof(LargeType_IgnoredProp_Bind_Param))]
253254
[JsonSerializable(typeof(ClassWithIgnoredSameType))]
255+
[JsonSerializable(typeof(ClassWithDefaultCtorParams))]
254256
internal sealed partial class ConstructorTestsContext_Default : JsonSerializerContext
255257
{
256258
}

0 commit comments

Comments
 (0)