Consider numeric string json for EnumConverter. (dotnet#79432)

* Consider numeric string json for EnumConverter.

* Fix comment: just use bit operation rather than HasFlag.

* Add test.

* Refine code.

* Fix comment: refine comment.

* Revert EnumConverter change.

* Complete check if current value is numeric string value, and then bypass enum parsing by numeric value.

* Add more tests.

* Fix issue on non-netcoreapp target.

* Use new CreateStringEnumOptionsForType() in tests.

* Fix comment: move check logic into TryParseEnumCore; turn to use RegEx.

* use regex generator.

* Add fs test cases about Enum with numeric labels.

* Update fs test case.

* Fix comment: refine test cases in F#.

* Fix comment: refine test cases in F#.

* Fix comment: refine Regex usage.

* Update src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs

* Move Regex to non-generic helper class

* Update src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs

* Remove duplicated logic

* Remove raw pointer usage.

---------

Co-authored-by: Eirik Tsarpalis <eirik.tsarpalis@gmail.com>

src/libraries/System.Text.Json/src/System.Text.Json.csproj

@@ -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" />
   </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)" />

src/libraries/System.Text.Json/src/System/Text/Json/JsonHelpers.cs

@@ -6,6 +6,7 @@
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Runtime.CompilerServices;
+using System.Text.RegularExpressions;
 
 namespace System.Text.Json
 {
@@ -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
     }
 }

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs

@@ -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
@@ -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)
@@ -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);
 
@@ -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;
 
@@ -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)
         {

src/libraries/System.Text.Json/tests/System.Text.Json.FSharp.Tests/EnumTests.fs

@@ -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)
@@ -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)

src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/EnumConverterTests.cs

@@ -3,6 +3,7 @@
 
 using System.Collections.Generic;
 using System.Diagnostics.CodeAnalysis;
+using System.Diagnostics;
 using System.Globalization;
 using System.IO;
 using System.Reflection;
@@ -117,6 +118,27 @@ public void ConvertDayOfWeek(bool useGenericVariant)
             // Not permitting integers should throw
             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
@@ -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());
 
@@ -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);
+        }
     }
 }