Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider numeric string json for EnumConverter. #79432

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
5d95f67
Consider numeric string json for EnumConverter.
lateapexearlyspeed Dec 9, 2022
098204a
Fix comment: just use bit operation rather than HasFlag.
lateapexearlyspeed Dec 9, 2022
afeb8e1
Add test.
lateapexearlyspeed Dec 14, 2022
732cb3a
Refine code.
lateapexearlyspeed Dec 17, 2022
58dc738
Merge remote-tracking branch 'origin/main' into lateapexearlyspeed-Co…
lateapexearlyspeed Jan 13, 2023
05555d9
Fix comment: refine comment.
lateapexearlyspeed Jan 16, 2023
180c9bb
Revert EnumConverter change.
lateapexearlyspeed Feb 20, 2023
44029a9
Complete check if current value is numeric string value, and then byp…
lateapexearlyspeed Feb 23, 2023
d745c60
Add more tests.
lateapexearlyspeed Feb 23, 2023
5886f87
Fix issue on non-netcoreapp target.
lateapexearlyspeed Mar 9, 2023
1201c89
Merge branch 'main' into lateapexearlyspeed-ConsiderNumericStringEnum
lateapexearlyspeed Jul 8, 2023
cca8cc2
Use new CreateStringEnumOptionsForType() in tests.
lateapexearlyspeed Jul 10, 2023
045f851
Merge remote-tracking branch 'upstream/main' into lateapexearlyspeed-…
lateapexearlyspeed Sep 26, 2023
ec0024d
Fix comment: move check logic into TryParseEnumCore; turn to use RegEx.
lateapexearlyspeed Sep 28, 2023
1d15cce
use regex generator.
lateapexearlyspeed Sep 30, 2023
92ca867
Add fs test cases about Enum with numeric labels.
lateapexearlyspeed Sep 30, 2023
541f3c5
Update fs test case.
lateapexearlyspeed Sep 30, 2023
c214d39
Fix comment: refine test cases in F#.
lateapexearlyspeed Oct 2, 2023
002f5d5
Fix comment: refine test cases in F#.
lateapexearlyspeed Oct 2, 2023
cd997df
Fix comment: refine Regex usage.
lateapexearlyspeed Oct 4, 2023
f07828b
Update src/libraries/System.Text.Json/src/System/Text/Json/Serializat…
eiriktsarpalis Oct 4, 2023
f29e91f
Move Regex to non-generic helper class
eiriktsarpalis Oct 4, 2023
5ceba05
Update src/libraries/System.Text.Json/src/System/Text/Json/Serializat…
eiriktsarpalis Oct 5, 2023
ee9efd0
Remove duplicated logic
eiriktsarpalis Oct 5, 2023
357d84c
Remove raw pointer usage.
eiriktsarpalis Oct 5, 2023
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
10 changes: 9 additions & 1 deletion src/libraries/System.Text.Json/src/System.Text.Json.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -387,8 +387,16 @@ The System.Text.Json library is built-in as part of the shared framework in .NET
<Reference Include="System.Runtime.Loader" />
<Reference Include="System.Text.Encoding.Extensions" />
<Reference Include="System.Threading" />
<Reference Include="System.Text.RegularExpressions" />
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
</ItemGroup>


<ItemGroup>
<ProjectReference Include="$(LibrariesProjectRoot)System.Text.RegularExpressions\gen\System.Text.RegularExpressions.Generator.csproj"
ReferenceOutputAssembly="false"
SetTargetFramework="TargetFramework=netstandard2.0"
OutputItemType="Analyzer" />
</ItemGroup>

<ItemGroup Condition="'$(TargetFrameworkIdentifier)' != '.NETCoreApp'">
<PackageReference Include="System.Buffers" Version="$(SystemBuffersVersion)" />
<PackageReference Include="System.Memory" Version="$(SystemMemoryVersion)" />
Expand Down
15 changes: 15 additions & 0 deletions src/libraries/System.Text.Json/src/System/Text/Json/JsonHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Collections.Generic;
using System.Diagnostics;
using System.Runtime.CompilerServices;
using System.Text.RegularExpressions;

namespace System.Text.Json
{
Expand Down Expand Up @@ -174,5 +175,19 @@ public static bool HasAllSet(this BitArray bitArray)
return true;
}
#endif

/// <summary>
/// Gets a Regex instance for recognizing integer representations of enums.
/// </summary>
public static readonly Regex IntegerRegex = CreateIntegerRegex();
private const string IntegerRegexPattern = @"^\s*(\+|\-)?[0-9]+\s*$";
private const int IntegerRegexTimeoutMs = 200;

#if NETCOREAPP
[GeneratedRegex(IntegerRegexPattern, RegexOptions.None, matchTimeoutMilliseconds: IntegerRegexTimeoutMs)]
private static partial Regex CreateIntegerRegex();
#else
private static Regex CreateIntegerRegex() => new(IntegerRegexPattern, RegexOptions.Compiled, TimeSpan.FromMilliseconds(IntegerRegexTimeoutMs));
#endif
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -107,14 +107,15 @@ public override T Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerial
}

#if NETCOREAPP
if (TryParseEnumCore(ref reader, options, out T value))
if (TryParseEnumCore(ref reader, out T value))
#else
string? enumString = reader.GetString();
if (TryParseEnumCore(enumString, options, out T value))
if (TryParseEnumCore(enumString, out T value))
#endif
{
return value;
}

#if NETCOREAPP
return ReadEnumUsingNamingPolicy(reader.GetString());
#else
Expand Down Expand Up @@ -270,9 +271,9 @@ public override void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions
internal override T ReadAsPropertyNameCore(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
#if NETCOREAPP
bool success = TryParseEnumCore(ref reader, options, out T value);
bool success = TryParseEnumCore(ref reader, out T value);
#else
bool success = TryParseEnumCore(reader.GetString(), options, out T value);
bool success = TryParseEnumCore(reader.GetString(), out T value);
#endif

if (!success)
Expand All @@ -283,7 +284,7 @@ internal override T ReadAsPropertyNameCore(ref Utf8JsonReader reader, Type typeT
return value;
}

internal override unsafe void WriteAsPropertyNameCore(Utf8JsonWriter writer, T value, JsonSerializerOptions options, bool isWritingExtensionDataProperty)
internal override void WriteAsPropertyNameCore(Utf8JsonWriter writer, T value, JsonSerializerOptions options, bool isWritingExtensionDataProperty)
{
ulong key = ConvertToUInt64(value);

Expand Down Expand Up @@ -322,43 +323,50 @@ internal override unsafe void WriteAsPropertyNameCore(Utf8JsonWriter writer, T v
return;
}

#pragma warning disable 8500 // address of managed type
switch (s_enumTypeCode)
{
// Use Unsafe.As instead of raw pointers for .NET Standard support.
// https://github.com/dotnet/runtime/issues/84895

case TypeCode.Int32:
writer.WritePropertyName(*(int*)&value);
writer.WritePropertyName(Unsafe.As<T, int>(ref value));
break;
case TypeCode.UInt32:
writer.WritePropertyName(*(uint*)&value);
writer.WritePropertyName(Unsafe.As<T, uint>(ref value));
break;
case TypeCode.UInt64:
writer.WritePropertyName(*(ulong*)&value);
writer.WritePropertyName(Unsafe.As<T, ulong>(ref value));
break;
case TypeCode.Int64:
writer.WritePropertyName(*(long*)&value);
writer.WritePropertyName(Unsafe.As<T, long>(ref value));
break;
case TypeCode.Int16:
writer.WritePropertyName(*(short*)&value);
writer.WritePropertyName(Unsafe.As<T, short>(ref value));
break;
case TypeCode.UInt16:
writer.WritePropertyName(*(ushort*)&value);
writer.WritePropertyName(Unsafe.As<T, ushort>(ref value));
break;
case TypeCode.Byte:
writer.WritePropertyName(*(byte*)&value);
writer.WritePropertyName(Unsafe.As<T, byte>(ref value));
break;
case TypeCode.SByte:
writer.WritePropertyName(*(sbyte*)&value);
writer.WritePropertyName(Unsafe.As<T, sbyte>(ref value));
break;
default:
ThrowHelper.ThrowJsonException();
break;
}
#pragma warning restore 8500
}

private bool TryParseEnumCore(
#if NETCOREAPP
private static bool TryParseEnumCore(ref Utf8JsonReader reader, JsonSerializerOptions options, out T value)
ref Utf8JsonReader reader,
#else
string? source,
#endif
out T value)
{
#if NETCOREAPP
char[]? rentedBuffer = null;
int bufferLength = reader.ValueLength;

Expand All @@ -368,28 +376,29 @@ private static bool TryParseEnumCore(ref Utf8JsonReader reader, JsonSerializerOp

int charsWritten = reader.CopyString(charBuffer);
ReadOnlySpan<char> source = charBuffer.Slice(0, charsWritten);
#endif

// Try parsing case sensitive first
bool success = Enum.TryParse(source, out T result) || Enum.TryParse(source, ignoreCase: true, out result);
bool success;
if ((_converterOptions & EnumConverterOptions.AllowNumbers) != 0 || !JsonHelpers.IntegerRegex.IsMatch(source))
{
// Try parsing case sensitive first
success = Enum.TryParse(source, out value) || Enum.TryParse(source, ignoreCase: true, out value);
}
else
{
success = false;
value = default;
}

#if NETCOREAPP
if (rentedBuffer != null)
{
charBuffer.Slice(0, charsWritten).Clear();
ArrayPool<char>.Shared.Return(rentedBuffer);
}

value = result;
return success;
}
#else
private static bool TryParseEnumCore(string? enumString, JsonSerializerOptions _, out T value)
{
// Try parsing case sensitive first
bool success = Enum.TryParse(enumString, out T result) || Enum.TryParse(enumString, ignoreCase: true, out result);
value = result;
#endif
return success;
}
#endif

private T ReadEnumUsingNamingPolicy(string? enumString)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ let goodEnumJsonStr = $"\"{goodEnum}\""
let options = new JsonSerializerOptions()
options.Converters.Add(new JsonStringEnumConverter())

let optionsDisableNumeric = new JsonSerializerOptions()
optionsDisableNumeric.Converters.Add(new JsonStringEnumConverter(null, false))

[<Fact>]
let ``Deserialize With Exception If Enum Contains Special Char`` () =
let ex = Assert.Throws<TargetInvocationException>(fun () -> JsonSerializer.Deserialize<BadEnum>(badEnumJsonStr, options) |> ignore)
Expand Down Expand Up @@ -58,3 +61,35 @@ let ``Fail Serialize Good Value Of Bad Enum Type`` () =
let ex = Assert.Throws<TargetInvocationException>(fun () -> JsonSerializer.Serialize(badEnumWithGoodValue, options) |> ignore)
Assert.Equal(typeof<InvalidOperationException>, ex.InnerException.GetType())
Assert.Equal("Enum type 'BadEnum' uses unsupported identifer name 'There's a comma, in my name'.", ex.InnerException.Message)

type NumericLabelEnum =
| ``1`` = 1
| ``2`` = 2
| ``3`` = 4

[<Theory>]
[<InlineData("\"1\"")>]
[<InlineData("\"2\"")>]
[<InlineData("\"3\"")>]
[<InlineData("\"4\"")>]
[<InlineData("\"5\"")>]
[<InlineData("\"+1\"")>]
[<InlineData("\"-1\"")>]
[<InlineData("\" 1 \"")>]
[<InlineData("\" +1 \"")>]
[<InlineData("\" -1 \"")>]
let ``Fail Deserialize Numeric label Of Enum When Disallow Integer Values`` (numericValueJsonStr: string) =
Assert.Throws<JsonException>(fun () -> JsonSerializer.Deserialize<NumericLabelEnum>(numericValueJsonStr, optionsDisableNumeric) |> ignore)

[<Theory>]
[<InlineData("\"1\"", NumericLabelEnum.``1``)>]
[<InlineData("\"2\"", NumericLabelEnum.``2``)>]
let ``Successful Deserialize Numeric label Of Enum When Allowing Integer Values`` (numericValueJsonStr: string, expectedEnumValue: NumericLabelEnum) =
let actual = JsonSerializer.Deserialize<NumericLabelEnum>(numericValueJsonStr, options)
Assert.Equal(expectedEnumValue, actual)

[<Fact>]
let ``Successful Deserialize Numeric label Of Enum But as Underlying value When Allowing Integer Values`` () =
let actual = JsonSerializer.Deserialize<NumericLabelEnum>("\"3\"", options)
Assert.NotEqual(NumericLabelEnum.``3``, actual)
Assert.Equal(LanguagePrimitives.EnumOfValue 3, actual)
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Diagnostics;
using System.Globalization;
using System.IO;
using System.Reflection;
Expand Down Expand Up @@ -117,6 +118,27 @@ public void ConvertDayOfWeek(bool useGenericVariant)
// Not permitting integers should throw
layomia marked this conversation as resolved.
Show resolved Hide resolved
options = CreateStringEnumOptionsForType<DayOfWeek>(useGenericVariant, allowIntegerValues: false);
Assert.Throws<JsonException>(() => JsonSerializer.Serialize((DayOfWeek)(-1), options));

// Quoted numbers should throw
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<DayOfWeek>("1", options));
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<DayOfWeek>("-1", options));
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<DayOfWeek>(@"""1""", options));
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<DayOfWeek>(@"""+1""", options));
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<DayOfWeek>(@"""-1""", options));
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<DayOfWeek>(@""" 1 """, options));
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<DayOfWeek>(@""" +1 """, options));
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<DayOfWeek>(@""" -1 """, options));

day = JsonSerializer.Deserialize<DayOfWeek>(@"""Monday""", options);
Assert.Equal(DayOfWeek.Monday, day);

// Numbers-formatted json string should first consider naming policy
options = CreateStringEnumOptionsForType<DayOfWeek>(useGenericVariant, new ToEnumNumberNamingPolicy<DayOfWeek>(), false);
day = JsonSerializer.Deserialize<DayOfWeek>(@"""1""", options);
Assert.Equal(DayOfWeek.Monday, day);

options = CreateStringEnumOptionsForType<DayOfWeek>(useGenericVariant, new ToLowerNamingPolicy(), false);
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<DayOfWeek>(@"""1""", options));
}

public class ToLowerNamingPolicy : JsonNamingPolicy
Expand Down Expand Up @@ -174,10 +196,23 @@ public void ConvertFileAttributes(bool useGenericVariant)
json = JsonSerializer.Serialize((FileAttributes)(-1), options);
Assert.Equal(@"-1", json);

// Not permitting integers should throw
options = CreateStringEnumOptionsForType<FileAttributes>(useGenericVariant, allowIntegerValues: false);
// Not permitting integers should throw
Assert.Throws<JsonException>(() => JsonSerializer.Serialize((FileAttributes)(-1), options));

// Numbers should throw
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<FileAttributes>("1", options));
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<FileAttributes>("-1", options));
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<FileAttributes>(@"""1""", options));
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<FileAttributes>(@"""+1""", options));
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<FileAttributes>(@"""-1""", options));
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<FileAttributes>(@""" 1 """, options));
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<FileAttributes>(@""" +1 """, options));
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<FileAttributes>(@""" -1 """, options));

attributes = JsonSerializer.Deserialize<FileAttributes>(@"""ReadOnly""", options);
Assert.Equal(FileAttributes.ReadOnly, attributes);

// Flag values honor naming policy correctly
options = CreateStringEnumOptionsForType<FileAttributes>(useGenericVariant, new SimpleSnakeCasePolicy());

Expand Down Expand Up @@ -716,22 +751,69 @@ public static void EnumDictionaryKeySerialization()
JsonTestHelper.AssertJsonEqual(expected, JsonSerializer.Serialize(dict, options));
}

[Theory]
[InlineData(typeof(SampleEnumByte), true)]
[InlineData(typeof(SampleEnumByte), false)]
[InlineData(typeof(SampleEnumSByte), true)]
[InlineData(typeof(SampleEnumSByte), false)]
[InlineData(typeof(SampleEnumInt16), true)]
[InlineData(typeof(SampleEnumInt16), false)]
[InlineData(typeof(SampleEnumUInt16), true)]
[InlineData(typeof(SampleEnumUInt16), false)]
[InlineData(typeof(SampleEnumInt32), true)]
[InlineData(typeof(SampleEnumInt32), false)]
[InlineData(typeof(SampleEnumUInt32), true)]
[InlineData(typeof(SampleEnumUInt32), false)]
[InlineData(typeof(SampleEnumInt64), true)]
[InlineData(typeof(SampleEnumInt64), false)]
[InlineData(typeof(SampleEnumUInt64), true)]
[InlineData(typeof(SampleEnumUInt64), false)]
public static void DeserializeNumericStringWithAllowIntegerValuesAsFalse(Type enumType, bool useGenericVariant)
{
JsonSerializerOptions options = CreateStringEnumOptionsForType(enumType, useGenericVariant, allowIntegerValues: false);

Assert.Throws<JsonException>(() => JsonSerializer.Deserialize(@"""1""", enumType, options));
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize(@"""+1""", enumType, options));
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize(@"""-1""", enumType, options));
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize(@""" 1 """, enumType, options));
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize(@""" +1 """, enumType, options));
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize(@""" -1 """, enumType, options));
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize(@$"""{ulong.MaxValue}""", enumType, options));
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize(@$""" {ulong.MaxValue} """, enumType, options));
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize(@$"""+{ulong.MaxValue}""", enumType, options));
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize(@$""" +{ulong.MaxValue} """, enumType, options));
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize(@$"""{long.MinValue}""", enumType, options));
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize(@$""" {long.MinValue} """, enumType, options));
}

private class ToEnumNumberNamingPolicy<T> : JsonNamingPolicy where T : struct, Enum
{
public override string ConvertName(string name) => Enum.TryParse(name, out T value) ? value.ToString("D") : name;
}

private class ZeroAppenderPolicy : JsonNamingPolicy
{
public override string ConvertName(string name) => name + "0";
}

private static JsonSerializerOptions CreateStringEnumOptionsForType<TEnum>(bool useGenericVariant, JsonNamingPolicy? namingPolicy = null, bool allowIntegerValues = true) where TEnum : struct, Enum
private static JsonSerializerOptions CreateStringEnumOptionsForType(Type enumType, bool useGenericVariant, JsonNamingPolicy? namingPolicy = null, bool allowIntegerValues = true)
{
Debug.Assert(enumType.IsEnum);

return new JsonSerializerOptions
{
Converters =
{
useGenericVariant
? new JsonStringEnumConverter<TEnum>(namingPolicy, allowIntegerValues)
: new JsonStringEnumConverter(namingPolicy, allowIntegerValues)
? (JsonConverter)Activator.CreateInstance(typeof(JsonStringEnumConverter<>).MakeGenericType(enumType), namingPolicy, allowIntegerValues)
: new JsonStringEnumConverter(namingPolicy, allowIntegerValues)
}
};
}

private static JsonSerializerOptions CreateStringEnumOptionsForType<TEnum>(bool useGenericVariant, JsonNamingPolicy? namingPolicy = null, bool allowIntegerValues = true) where TEnum : struct, Enum
{
return CreateStringEnumOptionsForType(typeof(TEnum), useGenericVariant, namingPolicy, allowIntegerValues);
}
}
}