Commandline: Immutable constructor argument order issues result in NullReferenceException

[ohadschn]

For example:

    class FooOptions
    {
        public FooOptions(string option, Uri foo)
        {
            Foo = foo;
            Option = option;
        }

        [Option("foo")]
        public Uri Foo { get; }    

        [Option("option")]
        public string Option { get;  }
    }

Will result in:

nhandled Exception: System.NullReferenceException: Object reference not set to an instance of an object.
   at CommandLine.Infrastructure.ReflectionHelper.CreateDefaultImmutableInstance(Type type, Type[] constructorTypes)
   at CommandLine.Core.ReflectionExtensions.AutoDefault(Type type)
   at CommandLine.Core.InstanceChooser.<>c__DisplayClass1_0.<MatchVerb>b__1()
   at CommandLine.Core.InstanceBuilder.<>c__0`1.<Build>b__0_0(Func`1 f)
   at CSharpx.MaybeExtensions.MapValueOrDefault[T1,T2](Maybe`1 maybe, Func`2 func, T2 noneValue)
   at CommandLine.Core.InstanceBuilder.Build[T](Maybe`1 factory, Func`3 tokenizer, IEnumerable`1 arguments, StringComparer nameComparer, Boolean ignoreValueCase, CultureInfo parsingCulture, IEnumerable`1 nonFatalErrors)
   at CommandLine.Core.InstanceChooser.MatchVerb(Func`3 tokenizer, IEnumerable`1 verbs, IEnumerable`1 arguments, StringComparer nameComparer, CultureInfo parsingCulture, IEnumerable`1 nonFatalErrors)
   at CommandLine.Core.InstanceChooser.<>c__DisplayClass0_0.<Choose>b__0()
   at CommandLine.Core.InstanceChooser.Choose(Func`3 tokenizer, IEnumerable`1 types, IEnumerable`1 arguments, StringComparer nameComparer, CultureInfo parsingCulture, IEnumerable`1 nonFatalErrors)
   at CommandLine.Parser.ParseArguments(IEnumerable`1 args, Type[] types)
   at CommandLine.ParserExtensions.ParseArguments[T1,T2](Parser parser, IEnumerable`1 args)
   at CommandLineParserTest.Program.Main()

The ideal solution IMO would be to find the right ctor by param names (rather than order). But even if that's not implemented, a better and more meaningful exception would be very helpful in debugging such issues.

[nemec]

You're right, in theory the order of properties/fields in a class is undefined (although in practice, it's processed line-by-line). We should not rely on the order as it's defined in the class.

[ohadschn]

Agreed. BTW, this can get even more confusing when option class inheritance is in play. Spoiler: the inherited options (from the base options class) are expected at the _end_ of the constructor parameter list.

[johndkane]

I'm opening up conversation about this issue because it interests me to find a solution.

Throwing out a few ideas for consideration and discussion:

• To not introduce breaking changes the default/classic behaviour should remain intact which is: the immutable logic assumes the constructor parameters are in the same order as the properties are found on the class, top to bottom;
• any new auto-mapping mechanism (by parameter name) should be explicitly enabled by the developer...
• Introduce a new ParserSettings setting to cause mapping of incoming arguments to ctor by param names;
• introduce the parser setting AutoMap=false(default)/true, or allow it to accept an enum value to additionally support case matching like enum {Off, Exact, CaseInsensitive}.
• Alternatively, because .NET Attributes are already in use in the project, a new [AutoMap] attribute can be introduced (allowed on class or constructor) to invoke the auto-mapping.
• The benefit of putting an [AutoMap] attribute on the constructor instead of class is to select a specific constructor in an overloaded scenario.

Is there any interest in supporting direct property setters to bypass the constructor? This might help reduce complication of option class inheritance.

[ohadschn]

Is there any interest in supporting direct property setters to bypass the constructor?

This defeats the point of immutable types (I suppose one could make the setters private but it would still not be truly immutable).

[moh-hassan]

The Exception, as described in #499, have been changed to:

Type appears to be immutable, but no constructor found for type to accept values.

[ohadschn]

@moh-hassan I would add some text like:

Note that constructor argument order must match property definition order in the options class (inherited options are expected at the end of the constructor parameter list).

Otherwise a client may not understand _why_ no matching constructor was found...