Skip to content
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

Example with Option with IEnumerable<string> broken - Sequence contains more than one element #377

Open
ivberg-zz opened this issue Dec 7, 2018 · 6 comments

Comments

@ivberg-zz
Copy link

Running the latest commandlineparser v2.3.0 on .Net Core 2.1.

The library example doesn't seem to work at all parsing IEnumerable
[Option('r', "read", Required = true, HelpText = "Input files to be processed.")]
public IEnumerable InputFiles { get; set; }

If you specify --read file1 file2 you get this exception:

System.InvalidOperationException
HResult=0x80131509
Message=Sequence contains more than one element
Source=System.Linq
StackTrace:

System.Linq.dll!System.Linq.Enumerable.Single<System.Type>(System.Collections.Generic.IEnumerable<System.Type> source) Line 45 C#
CommandLine.dll!CommandLine.Core.InstanceBuilder.Build.AnonymousMethod__0_23(CommandLine.Core.SpecificationProperty sp) Unknown
CommandLine.dll!CommandLine.Core.ReflectionExtensions.SetProperties.AnonymousMethod__0(System.__Canon current, CommandLine.Core.SpecificationProperty specProp) Unknown
System.Linq.dll!System.Linq.Enumerable.Aggregate<CommandLine.Core.SpecificationProperty, LogMerger.Program.Options>(System.Collections.Generic.IEnumerable<CommandLine.Core.SpecificationProperty> source, LogMerger.Program.Options seed, System.Func<LogMerger.Program.Options, CommandLine.Core.SpecificationProperty, LogMerger.Program.Options> func) Line 55 C#
CommandLine.dll!CommandLine.Core.InstanceBuilder.Build.AnonymousMethod__16() Unknown
CommandLine.dll!CommandLine.Core.InstanceBuilder.Build.AnonymousMethod__7() Unknown
CommandLine.dll!CommandLine.Parser.ParseArguments<LogMerger.Program.Options>(System.Collections.Generic.IEnumerable args) Unknown

@ivberg-zz
Copy link
Author

Actually, this was an issue instead with Parsing a Dict
public Dictionary<string, string> Dict { get; set; }

Is this supported?

@nemec
Copy link
Contributor

nemec commented Dec 7, 2018

No, unfortunately dicts are not supported

@ericnewton76
Copy link
Member

A dictionary would be hard to support... what generates the keys for the values?

I would believe this is undefined in the getopt specs that this library strives to follow.

@ivberg-zz
Copy link
Author

I think you are correct Eric that it would be hard to support. I think this is more about protecting against dumb new user and a clearer exception behavior than implementing support :). As a C# dev, Dictionary is popular and for better or worse, it's what I expected/hoped to use here as one of the cmd line options. The above is actually misleading, as I thought the IEnumerable is what was causing the issue, but instead the Dict used as another possible cmd-line parameter (turns out didn't need it)

My suggestion is the user adds odd or not supported stuff, that we have a clearer exception than
System.InvalidOperationException - Message=Sequence contains more than one element

@ericnewton76
Copy link
Member

Indeed. The error is misleading.

@PentaPinguino
Copy link

PentaPinguino commented Jul 23, 2023

Hi everyone,

I see that this issue has been open for a few years now, I'd like to make a recap and see this issue closed.

First of all, I noticed that the example code reported in the first message contains an error because IEnumerable should be IEnumerable<string> instead.

[Option('r', "read", Required = true, HelpText = "Input files to be processed.")]
public IEnumerable InputFiles { get; set; }

If we try to parse the following options, the library correctly throws a System.InvalidOperationException exception with a nice error message Non scalar properties should be sequence of type IEnumerable<T>..

internal class Options_IEnumerable_Without_Type
{
    [Option('r', "read", Required = true, HelpText = "Input files to be processed.")]
    public IEnumerable InputFiles { get; set; }
}

As @ivberg-zz and @eric pointed out later, this issue is actually about improving the error message when the developed tries to use an unsupported type (i.e. a dictionary).

I noticed that the user @phaniva made a pull request phaniva:issue-377 with a proposed solution of adding a new specification guard that ensures that all sequence type require exactly one argument.

private static Func<Specification, bool> GaurdAgainstUnsupportedSequenceTypes()
{
    return spec => spec.TargetType == TargetType.Sequence && spec.ConversionType.GetGenericArguments().Length != 1;
}

The problem with this solution is that the error Non scalar properties should be sequence of type IEnumerable<T>. defined in TypeConverter.ChangeTypeSequence will be bypassed - that means that developers will receive a different error message when trying to parse a IEnumerable without a specified type.

In my opinion there are two possible solutions to the problem:

  1. To ensure - inside ChangeTypeSequence - that the sequence has exactly one generic argument.
  2. To add two new rules inside SpecificationGuards that ensure that sequence types have exactly one generic argument.

Is it fair to have some checks inside the ChangeTypeSequence and some checks inside SpecificationGuards? Shouldn't we maybe try to move all checks to SpecificationGuards?

Thanks in advance and kind regards,
Davide

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

Successfully merging a pull request may close this issue.

4 participants