Merge pull request #286 from e673/issue283

ericnewton76 · web-flow · commit 3b825c31b08e · 2018-08-13T18:39:34.000-04:00
Implement new functionality suggested in Issue 283
diff --git a/src/CommandLine/Core/InstanceBuilder.cs b/src/CommandLine/Core/InstanceBuilder.cs
@@ -80,22 +80,23 @@ public static ParserResult<T> Build<T>(
                 var specPropsWithValue =
                     optionSpecPropsResult.SucceededWith().Concat(valueSpecPropsResult.SucceededWith());
 
-                Func<T> buildMutable = () =>
+                var setPropertyErrors = new List<Error>();                
+
+                Func <T> buildMutable = () =>
                 {
                     var mutable = factory.MapValueOrDefault(f => f(), Activator.CreateInstance<T>());
-                    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<T> 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
@@ -80,51 +80,30 @@ public static TargetType ToTargetType(this Type type)
                                    : TargetType.Scalar;
         }
 
-        public static T SetProperties<T>(
+        public static IEnumerable<Error> SetProperties<T>(
             this T instance,
             IEnumerable<SpecificationProperty> specProps,
             Func<SpecificationProperty, bool> predicate,
             Func<SpecificationProperty, object> 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<T>(this PropertyInfo property, T instance, object value)
+        private static IEnumerable<Error> SetValue<T>(this SpecificationProperty specProp, T instance, object value)
         {
-            Action<Exception> 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<Error>();
             }
-#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
@@ -60,7 +60,11 @@ public enum ErrorType
         /// <summary>
         /// Value of <see cref="CommandLine.VersionRequestedError"/> type.
         /// </summary>
-        VersionRequestedError
+        VersionRequestedError,
+        /// <summary>
+        /// Value of <see cref="CommandLine.SetValueExceptionError"/> type.
+        /// </summary>
+        SetValueExceptionError
     }
 
     /// <summary>
@@ -471,4 +475,37 @@ internal VersionRequestedError()
         {
         }
     }
+
+    /// <summary>
+    /// Models as error generated when exception is thrown at Property.SetValue
+    /// </summary>
+    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;
+        }
+
+        /// <summary>
+        /// The expection thrown from Property.SetValue
+        /// </summary>
+        public Exception Exception
+        {
+            get { return exception; }
+        }
+
+        /// <summary>
+        /// The value that had to be set to the property
+        /// </summary>
+        public object Value
+        {
+            get { return value; }
+        }
+
+    }
 }
diff --git a/src/CommandLine/Text/SentenceBuilder.cs b/src/CommandLine/Text/SentenceBuilder.cs
@@ -137,6 +137,9 @@ public override Func<Error, string> 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
@@ -62,6 +62,7 @@
     <Compile Include="Fakes\Options_With_Default_Set_To_Sequence.cs" />
     <Compile Include="Fakes\Options_With_Guid.cs" />
     <Compile Include="Fakes\Options_With_Option_And_Value_Of_String_Type.cs" />
+    <Compile Include="Fakes\Options_With_Property_Throwing_Exception.cs" />
     <Compile Include="Fakes\Options_With_SetName_That_Ends_With_Previous_SetName.cs" />
     <Compile Include="Fakes\Options_With_Shuffled_Index_Values.cs" />
     <Compile Include="Fakes\Options_With_Uri_And_SimpleType.cs" />
diff --git a/tests/CommandLine.Tests/Fakes/Options_With_Property_Throwing_Exception.cs 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
@@ -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<Options_With_Property_Throwing_Exception>(
+                new[] { "-e", "bad" });
+
+            // Verify outcome
+            ((NotParsed<Options_With_Property_Throwing_Exception>)result).Errors.ShouldBeEquivalentTo(expectedResult);
+
+            // Teardown
+        }
+
+
         [Theory]
         [InlineData(new[] { "--stringvalue", "x-" }, "x-")]
         [InlineData(new[] { "--stringvalue", "x--" }, "x--")]

Original file line number	Diff line number	Diff line change
`@@ -80,51 +80,30 @@ public static TargetType ToTargetType(this Type type)`
`80`	`80`	`: TargetType.Scalar;`
`81`	`81`	`}`
`82`	`82`
`83`		`- public static T SetProperties<T>(`
	`83`	`+ public static IEnumerable<Error> SetProperties<T>(`
`84`	`84`	`this T instance,`
`85`	`85`	`IEnumerable<SpecificationProperty> specProps,`
`86`	`86`	`Func<SpecificationProperty, bool> predicate,`
`87`	`87`	`Func<SpecificationProperty, object> selector)`
`88`	`88`	`{`
`89`		`- return specProps.Where(predicate).Aggregate(`
`90`		`- instance,`
`91`		`- (current, specProp) =>`
`92`		`- {`
`93`		`- specProp.Property.SetValue(current, selector(specProp));`
`94`		`- return instance;`
`95`		`- });`
	`89`	`+ return specProps.Where(predicate).SelectMany(specProp => specProp.SetValue(instance, selector(specProp)));`
`96`	`90`	`}`
`97`	`91`
`98`		`- private static T SetValue<T>(this PropertyInfo property, T instance, object value)`
	`92`	`+ private static IEnumerable<Error> SetValue<T>(this SpecificationProperty specProp, T instance, object value)`
`99`	`93`	`{`
`100`		`- Action<Exception> fail = inner => {`
`101`		`- throw new InvalidOperationException("Cannot set value to target instance.", inner);`
`102`		`- };`
`103`		`-`
`104`	`94`	`try`
`105`	`95`	`{`
`106`		`- property.SetValue(instance, value, null);`
`107`		`- }`
`108`		`-#if !PLATFORM_DOTNET`
`109`		`- catch (TargetException e)`
`110`		`- {`
`111`		`- fail(e);`
	`96`	`+ specProp.Property.SetValue(instance, value, null);`
	`97`	`+ return Enumerable.Empty<Error>();`
`112`	`98`	`}`
`113`		`-#endif`
`114`		`- catch (TargetParameterCountException e)`
`115`		`- {`
`116`		`- fail(e);`
`117`		`- }`
`118`		`- catch (MethodAccessException e)`
	`99`	`+ catch (TargetInvocationException e)`
`119`	`100`	`{`
`120`		`- fail(e);`
	`101`	`+ return new[] { new SetValueExceptionError(specProp.Specification.FromSpecification(), e.InnerException, value) };`
`121`	`102`	`}`
`122`		`- catch (TargetInvocationException e)`
	`103`	`+ catch (Exception e)`
`123`	`104`	`{`
`124`		`- fail(e);`
	`105`	`+ return new[] { new SetValueExceptionError(specProp.Specification.FromSpecification(), e, value) };`
`125`	`106`	`}`
`126`		`-`
`127`		`- return instance;`
`128`	`107`	`}`
`129`	`108`
`130`	`109`	`public static object CreateEmptyArray(this Type type)`