j-maly / CommandLineParser

Command line parser. Declarative arguments support. Rich set of argument types (switches, enums, files, etc...). Mutually exclusive arguments validations.
MIT License
137 stars 30 forks source link

nullable int always set to 0? #49

Closed StefH closed 7 years ago

StefH commented 7 years ago
  [ValueArgument(typeof(int), "MaxRequestLogCount", Optional = true, DefaultValue = null)]
  public int? MaxRequestLogCount { get; set; }

When not providing this argument, the default value is set to 0 instead of null ? Do I miss something?

And typeof(int?) crashes ?

  [ValueArgument(typeof(int?), "MaxRequestLogCount", Optional = true, DefaultValue = null)]
  public int? MaxRequestLogCount { get; set; }
j-maly commented 7 years ago

[ValueArgument(typeof(int))] - I am surprised that this does not crash. property.SetValue(null) where property is of type int.... weird

But this

[ValueArgument(typeof(int?), "MaxRequestLogCount", Optional = true, DefaultValue = null)]
public int? MaxRequestLogCount { get; set; }

seem to work fine for me. And isn't DefaultValue=null redundant?

StefH commented 7 years ago

Maybe it's just Net Core 2.0 related???

See https://github.com/WireMock-Net/WireMock.Net/tree/master/examples/WireMock.Net.StandAlone.NETCoreApp

And the command is: dotnet run -f netcoreapp2.0 --MaxRequestLogCount 100

The error is like:

Type System.Nullable`1[System.Int32] of argument 100 is not a built-in type.Set ConvertValueHandler to a conversion routine for this type or define static method Parse(string stringValue, Cu
ltureInfo cultureInfo) that can Parse your type from string.

Usage:
        --Port[optional]... Port to listen on.

        --Urls[optional]... URL(s) to listen on.

        --AllowPartialMapping[optional]... Allow Partial Mapping (default set to false).

        --StartAdminInterface[optional]... Start the AdminInterface (default set to true).

        --ReadStaticMappings[optional]... Read StaticMappings from ./__admin/mappings (default set to true).

        --ProxyURL[optional]... The ProxyURL to use.

        --SaveProxyMapping[optional]... Save the proxied request and response mapping files in ./__admin/mappings.  (default set to true).

        --X509Certificate2ThumbprintOrSubjectName[optional]... The X509Certificate2 Thumbprint or SubjectName to use.

        --AdminUsername[optional]... The username needed for __admin access.

        --AdminPassword[optional]... The password needed for __admin access.

        --RequestLogExpirationDuration[optional]... The RequestLog expiration in hours (optional).

        --MaxRequestLogCount[optional]... The MaxRequestLog count (optional).

Unhandled Exception: CommandLineParser.Exceptions.InvalidConversionException: Type System.Nullable`1[System.Int32] of argument 100 is not a built-in type.Set ConvertValueHandler to a convers
ion routine for this type or define static method Parse(string stringValue, CultureInfo cultureInfo) that can Parse your type from string.
   at CommandLineParser.Arguments.ValueArgument`1.DefaultConvert(String stringValue)
   at CommandLineParser.Arguments.ValueArgument`1.Convert(String stringValue)
   at CommandLineParser.Arguments.ValueArgument`1.Parse(IList`1 args, Int32& i)
   at CommandLineParser.CommandLineParser.ParseCommandLine(String[] args)
   at WireMock.Net.StandAlone.StandAloneApp.Start(String[] args) in C:\Users\StefHeyenrath\Documents\GitHub\WireMock.Net\src\WireMock.Net.StandAlone\StandAloneApp.cs:line 146
   at WireMock.Net.StandAlone.NETCoreApp.Program.Main(String[] args) in C:\Users\StefHeyenrath\Documents\GitHub\WireMock.Net\examples\WireMock.Net.StandAlone.NETCoreApp\Program.cs:line 14
j-maly commented 7 years ago

Yeah, could be .Net version. Could you take a look whether there is some change in the implementation of int32? .tryparse/parse in NC2.0?

On Aug 20, 2017 7:26 PM, "Stef Heyenrath" notifications@github.com wrote:

Maybe it's just Net Core 2.0 related???

See https://github.com/WireMock-Net/WireMock.Net/tree/master/ examples/WireMock.Net.StandAlone.NETCoreApp

And the command is: dotnet run -f netcoreapp2.0 --MaxRequestLogCount 100

The error is like:

Type System.Nullable`1[System.Int32] of argument 100 is not a built-in type.Set ConvertValueHandler to a conversion routine for this type or define static method Parse(string stringValue, Cu ltureInfo cultureInfo) that can Parse your type from string.

Usage: --Port[optional]... Port to listen on.

    --Urls[optional]... URL(s) to listen on.

    --AllowPartialMapping[optional]... Allow Partial Mapping

(default set to false).

    --StartAdminInterface[optional]... Start the AdminInterface

(default set to true).

    --ReadStaticMappings[optional]... Read StaticMappings from

./__admin/mappings (default set to true).

    --ProxyURL[optional]... The ProxyURL to use.

    --SaveProxyMapping[optional]... Save the proxied request and

response mapping files in ./__admin/mappings. (default set to true).

    --X509Certificate2ThumbprintOrSubjectName[optional]... The

X509Certificate2 Thumbprint or SubjectName to use.

    --AdminUsername[optional]... The username needed for __admin access.

    --AdminPassword[optional]... The password needed for __admin access.

    --RequestLogExpirationDuration[optional]... The RequestLog

expiration in hours (optional).

    --MaxRequestLogCount[optional]... The MaxRequestLog count (optional).

Unhandled Exception: CommandLineParser.Exceptions.InvalidConversionException: Type System.Nullable1[System.Int32] of argument 100 is not a built-in type.Set ConvertValueHandler to a convers ion routine for this type or define static method Parse(string stringValue, CultureInfo cultureInfo) that can Parse your type from string. at CommandLineParser.Arguments.ValueArgument1.DefaultConvert(String stringValue) at CommandLineParser.Arguments.ValueArgument1.Convert(String stringValue) at CommandLineParser.Arguments.ValueArgument1.Parse(IList`1 args, Int32& i) at CommandLineParser.CommandLineParser.ParseCommandLine(String[] args) at WireMock.Net.StandAlone.StandAloneApp.Start(String[] args) in C:\Users\StefHeyenrath\Documents\GitHub\WireMock.Net\src\WireMock.Net.StandAlone\StandAloneApp.cs:line 146 at WireMock.Net.StandAlone.NETCoreApp.Program.Main(String[] args) in C:\Users\StefHeyenrath\Documents\GitHub\WireMock.Net\examples\WireMock.Net.StandAlone.NETCoreApp\Program.cs:line 14

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/j-maly/CommandLineParser/issues/49#issuecomment-323599065, or mute the thread https://github.com/notifications/unsubscribe-auth/ABX8-oVS1EZWVTEDtNHr9kXIxV7Zu4VJks5saGwegaJpZM4O8nid .

StefH commented 7 years ago

I doubt if this is Net Core 2.0 related, actually I think that this code (https://github.com/j-maly/CommandLineParser/blob/e891473b225d49c48fcb53508cd15489c1b4a3f8/src/CommandLineArgumentsParser/Arguments/ValueArgument.cs#L388) cannot work if it's a Nullable type.

I think you need this code:

if (valueType.IsGenericType && valueType.GetGenericTypeDefinition() == typeof(Nullable<>))
{
    valueType = Nullable.GetUnderlyingType(valueType);
}
MethodInfo mi = valueType.GetMethod("Parse", new [] { typeof(string), typeof(CultureInfo)});
if (mi != null)
{
  if (mi.IsStatic && mi.ReturnType == valueType)
  {
    return stringValue != null ? (TValue)mi.Invoke(null, new object[] { stringValue, _cultureInfo }) : null;
  }
}

See my example at: https://github.com/WireMock-Net/WireMock.Net-docker/blob/master/CommandLineParserTest/Program.cs#L16-L19

Also verify for null: https://github.com/WireMock-Net/WireMock.Net-docker/blob/master/CommandLineParserTest/Program.cs#L46

StefH commented 7 years ago

Should I make a PR + unit-tests for this issue?