diff --git a/src/CommandLine/Core/InstanceBuilder.cs b/src/CommandLine/Core/InstanceBuilder.cs index f48127b1..0690aaf9 100644 --- a/src/CommandLine/Core/InstanceBuilder.cs +++ b/src/CommandLine/Core/InstanceBuilder.cs @@ -4,10 +4,11 @@ using System.Collections.Generic; using System.Globalization; using System.Linq; +using System.Reflection; +using System.Threading.Tasks; using CommandLine.Infrastructure; using CSharpx; using RailwaySharp.ErrorHandling; -using System.Reflection; namespace CommandLine.Core { @@ -61,11 +62,12 @@ public static ParserResult Build( .OfType() .Memoize(); - Func makeDefault = () => + Func makeDefaultImmutable = () => ReflectionHelper.CreateDefaultImmutableInstance((from p in specProps select p.Specification.ConversionType).ToArray()); + + Func makeDefault = typeof(T).IsMutable() - ? factory.MapValueOrDefault(f => f(), () => Activator.CreateInstance()) - : ReflectionHelper.CreateDefaultImmutableInstance( - (from p in specProps select p.Specification.ConversionType).ToArray()); + ? () => factory.MapValueOrDefault(f => f(), () => typeof(T).HasParameterlessConstructor() ? Activator.CreateInstance() : makeDefaultImmutable()) + : makeDefaultImmutable; Func, ParserResult> notParsed = errs => new NotParsed(makeDefault().GetType().ToTypeInfo(), errs); @@ -110,7 +112,7 @@ public static ParserResult Build( //build the instance, determining if the type is mutable or not. T instance; - if(typeInfo.IsMutable() == true) + if (typeInfo.IsMutable() && (!factory.IsNothing() || typeInfo.HasParameterlessConstructor())) { instance = BuildMutable(factory, specPropsWithValue, setPropertyErrors); } diff --git a/src/CommandLine/Core/ReflectionExtensions.cs b/src/CommandLine/Core/ReflectionExtensions.cs index 622e1e6e..9dbb9a1d 100644 --- a/src/CommandLine/Core/ReflectionExtensions.cs +++ b/src/CommandLine/Core/ReflectionExtensions.cs @@ -131,7 +131,7 @@ public static bool IsMutable(this Type type) // Find all inherited defined properties and fields on the type var inheritedTypes = type.GetTypeInfo().FlattenHierarchy().Select(i => i.GetTypeInfo()); - foreach (var inheritedType in inheritedTypes) + foreach (var inheritedType in inheritedTypes) { if ( inheritedType.GetTypeInfo().GetProperties(BindingFlags.Public | BindingFlags.Instance).Any(p => p.CanWrite) || @@ -145,6 +145,11 @@ public static bool IsMutable(this Type type) return false; } + public static bool HasParameterlessConstructor(this Type type) + { + return type.GetTypeInfo().GetConstructor(Type.EmptyTypes) != null; + } + public static object CreateDefaultForImmutable(this Type type) { if (type.GetTypeInfo().IsGenericType && type.GetTypeInfo().GetGenericTypeDefinition() == typeof(IEnumerable<>)) @@ -156,7 +161,7 @@ public static object CreateDefaultForImmutable(this Type type) public static object AutoDefault(this Type type) { - if (type.IsMutable()) + if (type.IsMutable() && type.HasParameterlessConstructor()) { return Activator.CreateInstance(type); } diff --git a/src/CommandLine/Parser.cs b/src/CommandLine/Parser.cs index 4301aa52..e7f1e939 100644 --- a/src/CommandLine/Parser.cs +++ b/src/CommandLine/Parser.cs @@ -87,7 +87,7 @@ public ParserResult ParseArguments(IEnumerable args) { if (args == null) throw new ArgumentNullException("args"); - var factory = typeof(T).IsMutable() + var factory = typeof(T).IsMutable() && typeof(T).HasParameterlessConstructor() ? Maybe.Just>(Activator.CreateInstance) : Maybe.Nothing>(); diff --git a/tests/CommandLine.Tests/Unit/Issue890Tests.cs b/tests/CommandLine.Tests/Unit/Issue890Tests.cs new file mode 100644 index 00000000..e1eab1bc --- /dev/null +++ b/tests/CommandLine.Tests/Unit/Issue890Tests.cs @@ -0,0 +1,38 @@ +using System.Linq; +using CommandLine.Tests.Fakes; +using CommandLine.Text; +using FluentAssertions; +using Xunit; +using Xunit.Abstractions; + +//Issue #890 +//When options class is mutable but doesn't have a parameterless constructor parsing fails. + +namespace CommandLine.Tests.Unit +{ + public class Issue890Tests + { + const char OptionSwitch = 'o'; + [Fact] + public void Create_mutable_instance_without_parameterless_ctor_should_not_fail() + { + const string optionValue = "val"; + + var result = Parser.Default.ParseArguments(new string[] { $"-{OptionSwitch}", optionValue }); + + Assert.Equal(ParserResultType.Parsed, result.Tag); + Assert.NotNull(result.Value); + Assert.Equal(optionValue, result.Value.Opt); + Assert.Empty(result.Errors); + } + private class Options + { + public Options(string opt) + { + Opt = opt; + } + [Option(OptionSwitch, "opt", Required = false)] + public string Opt { get; set; } + } + } +}