Skip to content

Test for Enum.IsDefined in TypeConverter.ToEnum is broken in the face of [Flags] #599

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

Closed
ryanmolden opened this issue Mar 15, 2020 · 4 comments

Comments

@ryanmolden
Copy link

The test in CommandLine.Core.TypeConverter.ToEnum which calls Enum.IsDefined is broken if the given enum has the Flags attribute (i.e it ONLY checks if the EXACT given value maps to an enum member, that will never be true for flag combinations of enum members).

This prevents having strongly types enum arguments that are passed on the command line, which is not an uncommon thing (especially in Windows where TONS of interesting data is described via flag style enums).

A workaround is to pass as a raw int/uint/ulong/whatever and just cast inside the program, but thats less ideal than just typing the command line param as what it actually is.

@moh-hassan
Copy link
Collaborator

Thanks @ryanmolden for feedback.
Can you provide test case(s) that show the issue w/o Flags attribute with the assert of expected result.

@ryanmolden
Copy link
Author

ryanmolden commented Mar 15, 2020

Create a C# console app

using CommandLine;
using System;

namespace CommandLineParserBugRepro
{
    class Program
    {
        internal enum NonFlagsEnum : int
        {
            Value1 = 1,
            Value2 = 2,
        }

        [Flags]
        internal enum FlagsEnum : int
        {
            IsGood     = 0x1,
            IsVeryGood = 0x2,
            IsBad      = 0x4,
            IsVeryBad  = 0x8
        }

        private class Options
        {
            [Option('n', "nonFlags", Required = true, HelpText = "Non Flags having enum.")]
            public NonFlagsEnum NonFlags{ get; set; }

            //https://github.com/commandlineparser/commandline/issues/599
            [Option('f', "flags", Required = true, HelpText = "Flags having enum.")]
            public FlagsEnum Flags { get; set; }
        }

        static void Main(string[] args)
        {
            Parser.Default.ParseArguments<Options>(args)
                   .WithParsed<Options>(o => 
                    {
                        Console.WriteLine("Success");
                    });
        }
    }
}

If you invoke this app with this command line

-n 2 -f 1

it prints 'Success'

If you invoke it with this command line

-n 2 -f 3

It prints this

ERROR(S):
Option 'f, flags' is defined with a bad format.
Required option 'f, flags' is missing.

3 is a valid enum value for FlagsEnum, it represents IsGood|IsVeryGood, however there is no single enum member defined with a value of 3 so Enum.IsDefined says false.

You should either eliminate the check entirely OR check if the given enum has the Flags attribute and if so only do the check if that attribute is not present.

As it stands if I want an enum like FlagsEnum as an arg I have to type it as int (to get past this bug) and then cast it to FlagsEnum. This isn't the worst but it would be much nicer if I could just type it as what I want it to be, an enum, and the parser would do the right thing.

@moh-hassan
Copy link
Collaborator

The normal use case of Enum is representing value as string and limiting the permissible values within the enum string values. Help also show these values as string.

-n 2 -f isgood     
# instead of
-n 2 -f 1

Sure enum values can be integer.
Using Flags, can be done as IEnumerable<TEnum>

-n 2 -f isgood|isbad        # values can be lowerCase  separated by | or ,
# instead of    
-n 2 -f 5

I agree with you that TypeConverter.ToEnum should be modified to cover int values of enum with flags, and it will be planned to add this feature.

Can you look for the solution of Flags here.
Also you can use custom type with ctor of string parameter which can parse both int/string values of enum.

@ryanmolden
Copy link
Author

Representing textually is an undesirable step, the textual piece of an enum exists only for programmer convenience, underlying value are just numeric as computers don't do well with strings. Forcing me to break down a flags value into an IEnumerable is just a bad API. I get these flags values from native code in Windows, it comes to me as an ulong, having to process the info to break it down into something the command line parser can understand is just wasted time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants