Skip to content

Commit

Permalink
Fix #496 - Cryptic error message with immutable option class (#555)
Browse files Browse the repository at this point in the history
  • Loading branch information
moh-hassan authored Dec 30, 2019
1 parent 2063ffc commit 58b4f0b
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 2 deletions.
20 changes: 18 additions & 2 deletions src/CommandLine/Core/InstanceBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,9 @@ private static T BuildImmutable<T>(Type typeInfo, Maybe<Func<T>> factory, IEnume
{
throw new InvalidOperationException($"Type {typeInfo.FullName} appears to be immutable, but no constructor found to accept values.");
}

var values =
try
{
var values =
(from prms in ctor.GetParameters()
join sp in specPropsWithValue on prms.Name.ToLower() equals sp.Property.Name.ToLower() into spv
from sp in spv.DefaultIfEmpty()
Expand All @@ -185,6 +186,21 @@ from sp in spv.DefaultIfEmpty()
var immutable = (T)ctor.Invoke(values);

return immutable;
}
catch (Exception)
{
var ctorArgs = specPropsWithValue
.Select(x => x.Property.Name.ToLowerInvariant()).ToArray();
throw GetException(ctorArgs);
}
Exception GetException(string[] s)
{
var ctorSyntax = s != null ? " Constructor Parameters can be ordered as: " + $"'({string.Join(", ", s)})'" : string.Empty;
var msg =
$"Type {typeInfo.FullName} appears to be Immutable with invalid constructor. Check that constructor arguments have the same name and order of their underlying Type. {ctorSyntax}";
InvalidOperationException invalidOperationException = new InvalidOperationException(msg);
return invalidOperationException;
}
}

}
Expand Down
28 changes: 28 additions & 0 deletions tests/CommandLine.Tests/Fakes/Immutable_Simple_Options.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,32 @@ public Immutable_Simple_Options(string stringValue, IEnumerable<int> intSequence
[Value(0)]
public long LongValue { get { return longValue; } }
}

public class Immutable_Simple_Options_Invalid_Ctor_Args
{
private readonly string stringValue;
private readonly IEnumerable<int> intSequence;
private readonly bool boolValue;
private readonly long longValue;

public Immutable_Simple_Options_Invalid_Ctor_Args(string stringValue1, IEnumerable<int> intSequence2, bool boolValue, long longValue)
{
this.stringValue = stringValue;
this.intSequence = intSequence;
this.boolValue = boolValue;
this.longValue = longValue;
}

[Option(HelpText = "Define a string value here.")]
public string StringValue { get { return stringValue; } }

[Option('i', Min = 3, Max = 4, HelpText = "Define a int sequence here.")]
public IEnumerable<int> IntSequence { get { return intSequence; } }

[Option('x', HelpText = "Define a boolean or switch value here.")]
public bool BoolValue { get { return boolValue; } }

[Value(0)]
public long LongValue { get { return longValue; } }
}
}
16 changes: 16 additions & 0 deletions tests/CommandLine.Tests/Unit/Core/InstanceBuilderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -901,6 +901,22 @@ public void Parse_to_immutable_instance(string[] arguments, Immutable_Simple_Opt
expected.Should().BeEquivalentTo(((Parsed<Immutable_Simple_Options>)result).Value);
}

[Theory]
[MemberData(nameof(ImmutableInstanceData))]
public void Parse_to_immutable_instance_with_Invalid_Ctor_Args(string[] arguments, Immutable_Simple_Options _)
{
// Fixture setup in attributes

// Exercize system
Action act = () => InvokeBuildImmutable<Immutable_Simple_Options_Invalid_Ctor_Args>(
arguments);

// Verify outcome
var expectedMsg =
"Type CommandLine.Tests.Fakes.Immutable_Simple_Options_Invalid_Ctor_Args appears to be Immutable with invalid constructor. Check that constructor arguments have the same name and order of their underlying Type. Constructor Parameters can be ordered as: '(stringvalue, intsequence, boolvalue, longvalue)'";
act.Should().Throw<InvalidOperationException>().WithMessage(expectedMsg);
}

[Fact]
public void Parse_to_type_with_single_string_ctor_builds_up_correct_instance()
{
Expand Down

0 comments on commit 58b4f0b

Please sign in to comment.