Feature request: turning off "number" enum parsing

[wouterst79]

I'd like to have an option to only allow name based enum parsing.

At the moment, to make sure numbers aren't recognized as "valid" enum values, I have to use

if (TypeExtensions.TryParse(text, out var value) && TypeExtensions.IsDefined(text))

essentially doing twice the minimum work required.

I'd like to have the ability to turn off (IE through a property on the attribute, or through a separate overload of tryparse) this generated code:

 case string s when byte.TryParse(name, out var val):
     value = (global::Type)val;
     return true;

Thanks for considering!

[andrewlock]

Thanks for the suggestion, I'm in two minds about it 🤔 On the one hand, it makes sense, as it's probably what most people actually want. But on the other hand, it's another configuration switch, and it again goes against the default behaviour, and there's already a trivial fix as you've suggested 🤔

[wouterst79]

Hey @andrewlock - thank you for considering the request, and responding.

I certainly appreciate your concerns - more configuration should be prevented whenever possible.

The reason I chose to integrate the package is to optimize parsing performance. I fondly remember reading how the compiler actually optimizes string based switch statements @ (https://devblogs.microsoft.com/dotnet/performance-improvements-in-net-8/), to include string length, and pre- or post-fixes in strings.

I actually considered moving to the probably faster if (TypeExtensions.TryParse(text, out var value) && !char.IsDigit(text[0]))) in my use case.

In my personal opinion, the design of the original Enum.Parse is actually flawed - it shouldn't just conflate string and number parsing in 1 function. Who knows, maybe the framework team will at some point make yet another overload (though i doubt it).

Definitely agree with the severity assessment - I didn't decide to branch, or make a pull request to get this fixed, and am instead working around it for now.

If you do choose to make the fix when you happen to be adding switches anyway, I'll gladly take advantage of them! :)

[andrewlock]

Makes sense, and to be clear, I agree in general - I think most people actually forget that numbers are valid values 😅

Just FYI, I would make sure to be the "simple" fast check first in your if example:

if (!char.IsDigit(text[0]) && TypeExtensions.TryParse(text, out var value)))

Otherwise you still pay the cost of doing the parse!

I'm going to leave this open for now anyway 🙂

[wouterst79]

I strongly agree that people might forget that numbers are valid values, or - even stronger - be surprised to find that they are (as i was 😅). I hadn't even considered that numbers would be parsed (or that they were part of the input in the first place), and I had to go through a debugging experience to figure out why I got some unexpected values back.

If you wanted to improve on the design, and make number parsing fallback opt-in, I would totally be supportive. Of course I can see how you might want to copy the existing C# API as closely as possible as well.

WRT moving the digit test first, that unfortunately doesn't work in my specific scenario since some of the input text might be 0 length. Numbers are uncommon enough in my data that putting the test second is fast enough.

[andrewlock]

FYI, The StringEnumConverter used by System.Text.Json allows turning off number parsing 😄 https://github.com/dotnet/runtime/blob/main/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs

So there's definitely some prior art