Consider numeric string json for EnumConverter. #79432

lateapexearlyspeed · 2022-12-09T04:14:28Z

ghost · 2022-12-09T04:14:39Z

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

Issue Details

Fix #58247

Author:	lateapexearlyspeed
Assignees:	-
Labels:	`area-System.Text.Json`
Milestone:	-

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

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

eiriktsarpalis · 2022-12-20T14:57:48Z

@lateapexearlyspeed there appear to be merge conflicts, could you try rebasing your branch over the latest main?

…nsiderNumericStringEnum # Conflicts: # src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs

lateapexearlyspeed · 2023-01-16T05:02:01Z

there appear to be merge conflicts

Hi @layomia @eiriktsarpalis , conflict resolved.

jeffhandley · 2023-02-14T02:06:27Z

@layomia / @dotnet/area-system-text-json Could you give this a re-review please? The conflicts were resolved and it looks like it could be ready to merge. It'd be nice to get this in for Preview 2. Thanks!

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

eiriktsarpalis

The CheckIfValidNamedEnumLiteral is not a correct check for valid IL identifiers.

…ass enum parsing by numeric value.

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

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

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

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

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

eiriktsarpalis · 2023-10-02T15:53:33Z

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

+        [GeneratedRegex(NumericPattern)]
+        private static partial Regex NumericRegex();
+#else
+        private static readonly Regex s_numericRegex = new(NumericPattern);


Non-source generated regexes should make use of RegexOptions.Compiled.

eiriktsarpalis · 2023-10-02T15:54:56Z

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

-            bool success = Enum.TryParse(source, out T result) || Enum.TryParse(source, ignoreCase: true, out result);
+            bool success;
+            T result;
+            if ((_converterOptions & EnumConverterOptions.AllowNumbers) != 0 || !NumericRegex().IsMatch(source))


Shouldn't the NumericRegex() method be storing the result in s_numericRegex for consistency?

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

…ion/Converters/Value/EnumConverter.cs

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

…ion/Converters/Value/EnumConverter.cs

eiriktsarpalis

Thanks!

Consider numeric string json for EnumConverter.

5d95f67

dotnet-issue-labeler bot added the area-System.Text.Json label Dec 9, 2022

ghost added the community-contribution Indicates that the PR has been added by a community member label Dec 9, 2022

jkotas reviewed Dec 9, 2022

View reviewed changes

...raries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs Outdated Show resolved Hide resolved

lateapexearlyspeed added 2 commits December 9, 2022 13:58

Fix comment: just use bit operation rather than HasFlag.

098204a

Add test.

afeb8e1

lateapexearlyspeed marked this pull request as ready for review December 14, 2022 07:23

Refine code.

732cb3a

layomia added this to the 8.0.0 milestone Dec 19, 2022

layomia assigned lateapexearlyspeed Dec 19, 2022

layomia reviewed Dec 19, 2022

View reviewed changes

lateapexearlyspeed added 2 commits January 13, 2023 18:45

Merge remote-tracking branch 'origin/main' into lateapexearlyspeed-Co…

58dc738

…nsiderNumericStringEnum # Conflicts: # src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs

Fix comment: refine comment.

05555d9

eiriktsarpalis assigned layomia and unassigned lateapexearlyspeed Jan 16, 2023

lateapexearlyspeed requested a review from layomia February 14, 2023 01:41

eiriktsarpalis reviewed Feb 14, 2023

View reviewed changes

...raries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs Outdated Show resolved Hide resolved

eiriktsarpalis requested changes Feb 14, 2023

View reviewed changes

ghost added needs-author-action An issue or pull request that requires more info or actions from the author. and removed needs-author-action An issue or pull request that requires more info or actions from the author. labels Feb 14, 2023

lateapexearlyspeed added 3 commits February 20, 2023 10:16

Revert EnumConverter change.

180c9bb

Complete check if current value is numeric string value, and then byp…

44029a9

…ass enum parsing by numeric value.

Add more tests.

d745c60

lateapexearlyspeed requested review from eiriktsarpalis and removed request for layomia February 23, 2023 14:53

build-analysis bot mentioned this pull request Feb 23, 2023

Roslyn source generator crash on mono/linux/arm64 #81123

Closed

eiriktsarpalis reviewed Oct 1, 2023

View reviewed changes

src/libraries/System.Text.Json/tests/System.Text.Json.FSharp.Tests/EnumTests.fs Outdated Show resolved Hide resolved

eiriktsarpalis reviewed Oct 1, 2023

View reviewed changes

src/libraries/System.Text.Json/src/System.Text.Json.csproj Show resolved Hide resolved

eiriktsarpalis reviewed Oct 1, 2023

View reviewed changes

...raries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs Outdated Show resolved Hide resolved

ViktorHofer reviewed Oct 1, 2023

View reviewed changes

src/libraries/System.Text.Json/src/System.Text.Json.csproj Outdated Show resolved Hide resolved

ViktorHofer reviewed Oct 1, 2023

View reviewed changes

src/libraries/System.Text.Json/src/System.Text.Json.csproj Outdated Show resolved Hide resolved

Fix comment: refine test cases in F#.

c214d39

eiriktsarpalis reviewed Oct 2, 2023

View reviewed changes

src/libraries/System.Text.Json/tests/System.Text.Json.FSharp.Tests/EnumTests.fs Outdated Show resolved Hide resolved

eiriktsarpalis reviewed Oct 2, 2023

View reviewed changes

src/libraries/System.Text.Json/tests/System.Text.Json.FSharp.Tests/EnumTests.fs Outdated Show resolved Hide resolved

This was referenced Oct 2, 2023

Tracking issue for CI build timeouts #76454

Closed

System.Data.OleDb.Tests timeout in net48 x86 Release leg #87783

Open

Fix comment: refine test cases in F#.

002f5d5

eiriktsarpalis reviewed Oct 2, 2023

View reviewed changes

...raries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs Outdated Show resolved Hide resolved

build-analysis bot mentioned this pull request Oct 2, 2023

NuGet failing with Response status code does not indicate success: 503 (Service Unavailable) dotnet/arcade#11723

Open

5 tasks

Fix comment: refine Regex usage.

cd997df

build-analysis bot mentioned this pull request Oct 4, 2023

[7.0][mono][arm64] futex-internal.h: No such file or directory - futex_wait_cancelable native crash in Microsoft.Extensions.Logging.Generators.Roslyn3.11.Tests #87444

Closed

eiriktsarpalis reviewed Oct 4, 2023

View reviewed changes

...raries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs Outdated Show resolved Hide resolved

eiriktsarpalis added 2 commits October 4, 2023 14:59

Update src/libraries/System.Text.Json/src/System/Text/Json/Serializat…

f07828b

…ion/Converters/Value/EnumConverter.cs

Move Regex to non-generic helper class

f29e91f

eiriktsarpalis reviewed Oct 5, 2023

View reviewed changes

...raries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs Outdated Show resolved Hide resolved

eiriktsarpalis added 3 commits October 5, 2023 13:26

Update src/libraries/System.Text.Json/src/System/Text/Json/Serializat…

5ceba05

…ion/Converters/Value/EnumConverter.cs

Remove duplicated logic

ee9efd0

Remove raw pointer usage.

357d84c

eiriktsarpalis approved these changes Oct 5, 2023

View reviewed changes

build-analysis bot mentioned this pull request Oct 5, 2023

Test infra failure on iOS.Device.Aot.Test.WorkItemExecution CI leg #92792

Closed

3 tasks

ericstj assigned eiriktsarpalis and unassigned layomia Oct 5, 2023

eiriktsarpalis merged commit 7d429e4 into dotnet:main Oct 6, 2023
106 of 109 checks passed

ghost locked as resolved and limited conversation to collaborators Nov 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Consider numeric string json for EnumConverter. #79432

Consider numeric string json for EnumConverter. #79432

lateapexearlyspeed commented Dec 9, 2022

ghost commented Dec 9, 2022

eiriktsarpalis commented Dec 20, 2022

lateapexearlyspeed commented Jan 16, 2023

jeffhandley commented Feb 14, 2023

eiriktsarpalis left a comment

eiriktsarpalis Oct 2, 2023

lateapexearlyspeed Oct 4, 2023

eiriktsarpalis Oct 2, 2023

lateapexearlyspeed Oct 4, 2023

eiriktsarpalis left a comment

Consider numeric string json for EnumConverter. #79432

Consider numeric string json for EnumConverter. #79432

Conversation

lateapexearlyspeed commented Dec 9, 2022

ghost commented Dec 9, 2022

eiriktsarpalis commented Dec 20, 2022

lateapexearlyspeed commented Jan 16, 2023

jeffhandley commented Feb 14, 2023

eiriktsarpalis left a comment

Choose a reason for hiding this comment

eiriktsarpalis Oct 2, 2023

Choose a reason for hiding this comment

lateapexearlyspeed Oct 4, 2023

Choose a reason for hiding this comment

eiriktsarpalis Oct 2, 2023

Choose a reason for hiding this comment

lateapexearlyspeed Oct 4, 2023

Choose a reason for hiding this comment

eiriktsarpalis left a comment

Choose a reason for hiding this comment