diff --git a/src/CommandLine/Core/InstanceBuilder.cs b/src/CommandLine/Core/InstanceBuilder.cs index 0c24fddd..fd9ac4d7 100644 --- a/src/CommandLine/Core/InstanceBuilder.cs +++ b/src/CommandLine/Core/InstanceBuilder.cs @@ -80,22 +80,23 @@ public static ParserResult Build( var specPropsWithValue = optionSpecPropsResult.SucceededWith().Concat(valueSpecPropsResult.SucceededWith()); - Func buildMutable = () => + var setPropertyErrors = new List(); + + Func buildMutable = () => { var mutable = factory.MapValueOrDefault(f => f(), Activator.CreateInstance()); - mutable = - mutable.SetProperties(specPropsWithValue, sp => sp.Value.IsJust(), sp => sp.Value.FromJustOrFail()) - .SetProperties( - specPropsWithValue, - sp => sp.Value.IsNothing() && sp.Specification.DefaultValue.IsJust(), - sp => sp.Specification.DefaultValue.FromJustOrFail()) - .SetProperties( - specPropsWithValue, - sp => - sp.Value.IsNothing() && sp.Specification.TargetType == TargetType.Sequence - && sp.Specification.DefaultValue.MatchNothing(), - sp => sp.Property.PropertyType.GetTypeInfo().GetGenericArguments().Single().CreateEmptyArray()); - return mutable; + setPropertyErrors.AddRange(mutable.SetProperties(specPropsWithValue, sp => sp.Value.IsJust(), sp => sp.Value.FromJustOrFail())); + setPropertyErrors.AddRange(mutable.SetProperties( + specPropsWithValue, + sp => sp.Value.IsNothing() && sp.Specification.DefaultValue.IsJust(), + sp => sp.Specification.DefaultValue.FromJustOrFail())); + setPropertyErrors.AddRange(mutable.SetProperties( + specPropsWithValue, + sp => + sp.Value.IsNothing() && sp.Specification.TargetType == TargetType.Sequence + && sp.Specification.DefaultValue.MatchNothing(), + sp => sp.Property.PropertyType.GetTypeInfo().GetGenericArguments().Single().CreateEmptyArray())); + return mutable; }; Func buildImmutable = () => @@ -121,6 +122,7 @@ join sp in specPropsWithValue on prms.Name.ToLower() equals sp.Property.Name.ToL .Concat(optionSpecPropsResult.SuccessfulMessages()) .Concat(valueSpecPropsResult.SuccessfulMessages()) .Concat(validationErrors) + .Concat(setPropertyErrors) .Memorize(); var warnings = from e in allErrors where nonFatalErrors.Contains(e.Tag) select e; diff --git a/src/CommandLine/Core/ReflectionExtensions.cs b/src/CommandLine/Core/ReflectionExtensions.cs index 068b3034..1fd3e515 100644 --- a/src/CommandLine/Core/ReflectionExtensions.cs +++ b/src/CommandLine/Core/ReflectionExtensions.cs @@ -80,51 +80,30 @@ public static TargetType ToTargetType(this Type type) : TargetType.Scalar; } - public static T SetProperties( + public static IEnumerable SetProperties( this T instance, IEnumerable specProps, Func predicate, Func selector) { - return specProps.Where(predicate).Aggregate( - instance, - (current, specProp) => - { - specProp.Property.SetValue(current, selector(specProp)); - return instance; - }); + return specProps.Where(predicate).SelectMany(specProp => specProp.SetValue(instance, selector(specProp))); } - private static T SetValue(this PropertyInfo property, T instance, object value) + private static IEnumerable SetValue(this SpecificationProperty specProp, T instance, object value) { - Action fail = inner => { - throw new InvalidOperationException("Cannot set value to target instance.", inner); - }; - try { - property.SetValue(instance, value, null); - } -#if !PLATFORM_DOTNET - catch (TargetException e) - { - fail(e); + specProp.Property.SetValue(instance, value, null); + return Enumerable.Empty(); } -#endif - catch (TargetParameterCountException e) - { - fail(e); - } - catch (MethodAccessException e) + catch (TargetInvocationException e) { - fail(e); + return new[] { new SetValueExceptionError(specProp.Specification.FromSpecification(), e.InnerException, value) }; } - catch (TargetInvocationException e) + catch (Exception e) { - fail(e); + return new[] { new SetValueExceptionError(specProp.Specification.FromSpecification(), e, value) }; } - - return instance; } public static object CreateEmptyArray(this Type type) diff --git a/src/CommandLine/Error.cs b/src/CommandLine/Error.cs index 475ac8e3..2b586912 100644 --- a/src/CommandLine/Error.cs +++ b/src/CommandLine/Error.cs @@ -60,7 +60,11 @@ public enum ErrorType /// /// Value of type. /// - VersionRequestedError + VersionRequestedError, + /// + /// Value of type. + /// + SetValueExceptionError } /// @@ -471,4 +475,37 @@ internal VersionRequestedError() { } } + + /// + /// Models as error generated when exception is thrown at Property.SetValue + /// + public sealed class SetValueExceptionError : NamedError + { + private readonly Exception exception; + private readonly object value; + + internal SetValueExceptionError(NameInfo nameInfo, Exception exception, object value) + : base(ErrorType.SetValueExceptionError, nameInfo) + { + this.exception = exception; + this.value = value; + } + + /// + /// The expection thrown from Property.SetValue + /// + public Exception Exception + { + get { return exception; } + } + + /// + /// The value that had to be set to the property + /// + public object Value + { + get { return value; } + } + + } } \ No newline at end of file diff --git a/src/CommandLine/Text/SentenceBuilder.cs b/src/CommandLine/Text/SentenceBuilder.cs index 1c28e959..1c150b67 100644 --- a/src/CommandLine/Text/SentenceBuilder.cs +++ b/src/CommandLine/Text/SentenceBuilder.cs @@ -137,6 +137,9 @@ public override Func FormatError case ErrorType.RepeatedOptionError: return "Option '".JoinTo(((RepeatedOptionError)error).NameInfo.NameText, "' is defined multiple times."); + case ErrorType.SetValueExceptionError: + var setValueError = (SetValueExceptionError)error; + return "Error setting value to option '".JoinTo(setValueError.NameInfo.NameText, "': ", setValueError.Exception.Message); } throw new InvalidOperationException(); }; diff --git a/tests/CommandLine.Tests/CommandLine.Tests.csproj b/tests/CommandLine.Tests/CommandLine.Tests.csproj index 5914a3a9..90b0ccd5 100644 --- a/tests/CommandLine.Tests/CommandLine.Tests.csproj +++ b/tests/CommandLine.Tests/CommandLine.Tests.csproj @@ -61,6 +61,7 @@ + diff --git a/tests/CommandLine.Tests/Fakes/Options_With_Property_Throwing_Exception.cs b/tests/CommandLine.Tests/Fakes/Options_With_Property_Throwing_Exception.cs new file mode 100644 index 00000000..031dcb6c --- /dev/null +++ b/tests/CommandLine.Tests/Fakes/Options_With_Property_Throwing_Exception.cs @@ -0,0 +1,29 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace CommandLine.Tests.Fakes +{ + class Options_With_Property_Throwing_Exception + { + private string optValue; + + [Option('e')] + public string OptValue + { + get + { + return optValue; + } + set + { + if (value != "good") + throw new ArgumentException("Invalid value, only accept 'good' value"); + + optValue = value; + } + } + } +} diff --git a/tests/CommandLine.Tests/Unit/Core/InstanceBuilderTests.cs b/tests/CommandLine.Tests/Unit/Core/InstanceBuilderTests.cs index 7d7544e6..6ff592e4 100644 --- a/tests/CommandLine.Tests/Unit/Core/InstanceBuilderTests.cs +++ b/tests/CommandLine.Tests/Unit/Core/InstanceBuilderTests.cs @@ -984,6 +984,23 @@ public void Parse_to_type_with_single_string_ctor_builds_up_correct_instance() // Teardown } + [Fact] + public void Parse_option_with_exception_thrown_from_setter_generates_SetValueExceptionError() + { + // Fixture setup + var expectedResult = new[] { new SetValueExceptionError(new NameInfo("e", ""), new ArgumentException(), "bad") }; + + // Exercize system + var result = InvokeBuild( + new[] { "-e", "bad" }); + + // Verify outcome + ((NotParsed)result).Errors.ShouldBeEquivalentTo(expectedResult); + + // Teardown + } + + [Theory] [InlineData(new[] { "--stringvalue", "x-" }, "x-")] [InlineData(new[] { "--stringvalue", "x--" }, "x--")]