Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/libraries/System.Text.Json/Common/JsonConstants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,9 @@ internal static partial class JsonConstants
{
// The maximum number of parameters a constructor can have where it can be supported by the serializer.
public const int MaxParameterCount = 64;

// Standard format for double and single on non-inbox frameworks.
public const string DoubleFormatString = "G17";
public const string SingleFormatString = "G9";
}
}
120 changes: 106 additions & 14 deletions src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
using System.Collections.Generic;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using System.Reflection;
using System.Reflection.Metadata;
using System.Text.Json;
using System.Text.Json.Reflection;
using System.Text.Json.Serialization;
Expand Down Expand Up @@ -807,11 +809,11 @@ private string GenerateCtorParamMetadataInitFunc(TypeGenerationSpec typeGenerati
for (int i = 0; i < paramCount; i++)
{
ParameterInfo reflectionInfo = parameters[i].ParameterInfo;

string parameterTypeRef = reflectionInfo.ParameterType.GetCompilableName();
Type parameterType = reflectionInfo.ParameterType;
string parameterTypeRef = parameterType.GetCompilableName();

object? defaultValue = reflectionInfo.GetDefaultValue();
string defaultValueAsStr = GetParamDefaultValueAsString(defaultValue, parameterTypeRef);
string defaultValueAsStr = GetParamDefaultValueAsString(defaultValue, parameterType, parameterTypeRef);

sb.Append(@$"
{InfoVarName} = new()
Expand Down Expand Up @@ -1316,20 +1318,110 @@ private static string GetNumberHandlingAsStr(JsonNumberHandling? numberHandling)
: "default";

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

private static string ToCSharpKeyword(bool value) => value.ToString().ToLowerInvariant();
private static string ToCSharpKeyword(bool value) => value.ToString().ToLowerInvariant();

private static string GetParamDefaultValueAsString(object? value, string objectTypeAsStr)
{
switch (value)
private string GetParamDefaultValueAsString(object? value, Type type, string typeRef)
{
if (value == null)
{
return $"default({typeRef})";
}

if (type.IsEnum)
{
// Roslyn gives us an instance of the underlying type, which is numerical.
#if DEBUG
Type runtimeType = _generationSpec.MetadataLoadContext.Resolve(value.GetType());
Debug.Assert(_generationSpec.IsNumberType(runtimeType));
#endif

// Return the numeric value.
return $"({typeRef})({value.ToString()})";
}

switch (value)
{
case string @string:
return QuoteString(@string);
case char @char:
return String.Format("'{0}'", EscapeChar((char)value, inString: false));
case double.NegativeInfinity:
return "double.NegativeInfinity";
case double.PositiveInfinity:
return "double.PositiveInfinity";
case double.NaN:
return "double.NaN";
case double @double:
return $"({typeRef})({@double.ToString(JsonConstants.DoubleFormatString, CultureInfo.InvariantCulture)})";
case float.NegativeInfinity:
return "float.NegativeInfinity";
case float.PositiveInfinity:
return "float.PositiveInfinity";
case float.NaN:
return "float.NaN";
case float @float:
return $"({typeRef})({@float.ToString(JsonConstants.SingleFormatString, CultureInfo.InvariantCulture)})";
case decimal.MaxValue:
return "decimal.MaxValue";
case decimal.MinValue:
return "decimal.MinValue";
case decimal @decimal:
return @decimal.ToString(CultureInfo.InvariantCulture);
case bool @bool:
return ToCSharpKeyword(@bool);
default:
// Assume this is a number.
return $"({typeRef})({value.ToString()})";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't need to use invariant culture? What if the current culture is one that uses something other than - for a negative sign?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to use invariant culture and avoid generating invalid C#. I plan to figure out some testing for this scenario in a follow up.

cc @ericstj on whether the dotnet/arcade implementations has similar localization concerns - https://github.com/dotnet/arcade/blob/0cd94b1d02c03377d99f3739beb191591f6abee5/src/Microsoft.Cci.Extensions/Writers/CSharp/CSDeclarationWriter.Attributes.cs#L313

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @safern. It does special case certain types and use invariant culture, but not all (including int). It's possible it has the issue but we don't notice since it's only used when folks run genapi which tends to be done by devs on english systems who review the diff it generates.

I'm a bit surprised we have to write this much ourselves. Does Roslyn have an API to give us the formatted default value?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see @stephentoub pointed this out here: #62798 (comment)

Nice! @safern take note, GenAPI could use this.

}
}

// The following logic to quote string constants with proper escaping is copied from the WriteMetadataConstant method from
// https://github.com/dotnet/arcade/blob/0cd94b1d02c03377d99f3739beb191591f6abee5/src/Microsoft.Cci.Extensions/Writers/CSharp/CSDeclarationWriter.Attributes.cs#L241

private static string QuoteString(string str)
{
case null:
return $"default({objectTypeAsStr})";
case bool boolVal:
return ToCSharpKeyword(boolVal);
default:
return value!.ToString();
StringBuilder sb = new StringBuilder("\"");

foreach (char ch in str)
{
sb.Append(EscapeChar(ch, inString: true));
}

sb.Append("\"");

return sb.ToString();
}

private static string EscapeChar(char c, bool inString)
{
switch (c)
{
case '\r': return @"\r";
case '\n': return @"\n";
case '\f': return @"\f";
case '\t': return @"\t";
case '\v': return @"\v";
case '\0': return @"\0";
case '\a': return @"\a";
case '\b': return @"\b";
case '\\': return @"\\";
case '\'': return inString ? "'" : @"\'";
case '"': return inString ? "\\\"" : "\"";
}

var cat = CharUnicodeInfo.GetUnicodeCategory(c);
if (cat == UnicodeCategory.Control ||
cat == UnicodeCategory.LineSeparator ||
cat == UnicodeCategory.Format ||
cat == UnicodeCategory.Surrogate ||
cat == UnicodeCategory.PrivateUse ||
cat == UnicodeCategory.OtherNotAssigned)
{
return String.Format("\\u{0:X4}", (int)c);
}

return c.ToString();
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,9 @@ public Parser(Compilation compilation, in JsonSourceGenerationContext sourceGene
GuidType = _guidType,
StringType = _stringType,
NumberTypes = _numberTypes,
#if DEBUG
MetadataLoadContext = _metadataLoadContext,
#endif
};
}

Expand Down
5 changes: 5 additions & 0 deletions src/libraries/System.Text.Json/gen/SourceGenerationSpec.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,18 @@
using System;
using System.Collections.Generic;
using System.Text;
using System.Text.Json.Reflection;
using Microsoft.CodeAnalysis;

namespace System.Text.Json.SourceGeneration
{
internal sealed class SourceGenerationSpec
{
public List<ContextGenerationSpec> ContextGenerationSpecList { get; init; }

#if DEBUG
public MetadataLoadContextInternal MetadataLoadContext { get; init; }
#endif
public Type BooleanType { get; init; }
public Type ByteArrayType { get; init; }
public Type CharType { get; init; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,7 @@ private static bool TryFormatDouble(double value, Span<byte> destination, out in
#if BUILDING_INBOX_LIBRARY
return Utf8Formatter.TryFormat(value, destination, out bytesWritten);
#else
const string FormatString = "G17";

string utf16Text = value.ToString(FormatString, CultureInfo.InvariantCulture);
string utf16Text = value.ToString(JsonConstants.DoubleFormatString, CultureInfo.InvariantCulture);

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,7 @@ private static bool TryFormatSingle(float value, Span<byte> destination, out int
#if BUILDING_INBOX_LIBRARY
return Utf8Formatter.TryFormat(value, destination, out bytesWritten);
#else
const string FormatString = "G9";

string utf16Text = value.ToString(FormatString, CultureInfo.InvariantCulture);
string utf16Text = value.ToString(JsonConstants.SingleFormatString, CultureInfo.InvariantCulture);

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Collections.Generic;
using System.Collections.Specialized;
using System.Diagnostics.CodeAnalysis;
using System.Reflection;
using System.Threading.Tasks;
using Xunit;

Expand Down Expand Up @@ -1385,5 +1386,148 @@ public struct StructWithExplicitParameterlessCtor
public long A;
public StructWithExplicitParameterlessCtor() => A = 42;
}

public async Task TestClassWithEmptyStringDefaultCtorParam()
{
ClassWithDefaultCtorParams obj = new ClassWithDefaultCtorParams("value", "val", 'v', 1, 2, 3, 4, 5, 6);
string json = await JsonSerializerWrapperForString.SerializeWrapper(obj);
}

public async Task TestClassWithDefaultCtorParams()
{
ClassWithDefaultCtorParams obj = new ClassWithDefaultCtorParams(
new Point_2D_Struct_WithAttribute(1, 2),
DayOfWeek.Sunday,
(DayOfWeek)(-2),
(DayOfWeek)19,
BindingFlags.CreateInstance | BindingFlags.FlattenHierarchy,
SampleEnumUInt32.MinZero,
"Hello world!",
null,
"xzy",
'c',
' ',
23,
4,
-40,
double.Epsilon,
23,
4,
-40,
float.MinValue,
1,
2,
3,
4,
5,
6,
7,
8,
9,
10);

string json = await JsonSerializerWrapperForString.SerializeWrapper(obj);
obj = await JsonSerializerWrapperForString.DeserializeWrapper<ClassWithDefaultCtorParams>(json);
JsonTestHelper.AssertJsonEqual(json, await JsonSerializerWrapperForString.SerializeWrapper(obj));
}

public class ClassWithDefaultCtorParams
{
public Point_2D_Struct_WithAttribute Struct { get; }
public DayOfWeek Enum1 { get; }
public DayOfWeek Enum2 { get; }
public DayOfWeek Enum3 { get; }
public BindingFlags Enum4 { get; }
public SampleEnumUInt32 Enum5 { get; }
public string Str1 { get; }
public string Str2 { get; }
public string Str3 { get; }
public char Char1 { get; }
public char Char2 { get; }
public double Double1 { get; }
public double Double2 { get; }
public double Double3 { get; }
public double Double4 { get; }
public double Double5 { get; }
public float Float1 { get; }
public float Float2 { get; }
public float Float3 { get; }
public float Float4 { get; }
public float Float5 { get; }
public byte Byte { get; }
public decimal Decimal1 { get; }
public decimal Decimal2 { get; }
public short Short { get; }
public sbyte Sbyte { get; }
public int Int { get; }
public long Long { get; }
public ushort UShort { get; }
public uint UInt { get; }
public ulong ULong { get; }

public ClassWithDefaultCtorParams(
Point_2D_Struct_WithAttribute @struct = default,
DayOfWeek enum1 = default,
DayOfWeek enum2 = DayOfWeek.Sunday,
DayOfWeek enum3 = DayOfWeek.Sunday | DayOfWeek.Monday,
BindingFlags enum4 = BindingFlags.CreateInstance | BindingFlags.ExactBinding,
SampleEnumUInt32 enum5 = SampleEnumUInt32.MinZero,
string str1 = "abc",
string str2 = "",
string str3 = "\n\r⁉️\'\"\u200D\f\t\v\0\a\b\\\'\"",
char char1 = 'a',
char char2 = '\u200D',
double double1 = double.NegativeInfinity,
double double2 = double.PositiveInfinity,
double double3 = double.NaN,
double double4 = double.MaxValue,
double double5 = double.Epsilon,
float float1 = float.NegativeInfinity,
float float2 = float.PositiveInfinity,
float float3 = float.NaN,
float float4 = float.MinValue,
float float5 = float.Epsilon,
byte @byte = byte.MinValue,
decimal @decimal1 = decimal.MinValue,
decimal @decimal2 = decimal.MaxValue,
short @short = short.MinValue,
sbyte @sbyte = sbyte.MaxValue,
int @int = int.MinValue,
long @long = long.MaxValue,
ushort @ushort = ushort.MinValue,
uint @uint = uint.MaxValue,
ulong @ulong = ulong.MinValue)
{
Struct = @struct;
Enum1 = enum1;
Enum2 = enum2;
Enum3 = enum3;
Enum4 = enum4;
Enum5 = enum5;
Str1 = str1;
Str2 = str2;
Str3 = str3;
Char1 = char1;
Char2 = char2;
Double1 = double1;
Double2 = double2;
Double3 = double3;
Double4 = double4;
Float1 = float1;
Float2 = float2;
Float3 = float3;
Float4 = float4;
Byte = @byte;
Decimal1 = @decimal1;
Decimal2 = @decimal2;
Short = @short;
Sbyte = @sbyte;
Int = @int;
Long = @long;
UShort = @ushort;
UInt = @uint;
ULong = @ulong;
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ protected ConstructorTests_Metadata(JsonSerializerWrapperForString stringWrapper
[JsonSerializable(typeof(LargeType_IgnoredProp_Bind_ParamWithDefaultValue))]
[JsonSerializable(typeof(LargeType_IgnoredProp_Bind_Param))]
[JsonSerializable(typeof(ClassWithIgnoredSameType))]
[JsonSerializable(typeof(ClassWithDefaultCtorParams))]
internal sealed partial class ConstructorTestsContext_Metadata : JsonSerializerContext
{
}
Expand Down Expand Up @@ -251,6 +252,7 @@ public ConstructorTests_Default()
[JsonSerializable(typeof(LargeType_IgnoredProp_Bind_ParamWithDefaultValue))]
[JsonSerializable(typeof(LargeType_IgnoredProp_Bind_Param))]
[JsonSerializable(typeof(ClassWithIgnoredSameType))]
[JsonSerializable(typeof(ClassWithDefaultCtorParams))]
internal sealed partial class ConstructorTestsContext_Default : JsonSerializerContext
{
}
Expand Down