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

Remove constraint on T for ParseArguments with factory #590

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/CommandLine/Core/InstanceBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public static ParserResult<T> Build<T>(

Func<T> makeDefault = () =>
typeof(T).IsMutable()
? factory.MapValueOrDefault(f => f(), Activator.CreateInstance<T>())
? factory.MapValueOrDefault(f => f(), () => Activator.CreateInstance<T>())
: ReflectionHelper.CreateDefaultImmutableInstance<T>(
(from p in specProps select p.Specification.ConversionType).ToArray());

Expand Down Expand Up @@ -128,7 +128,7 @@ public static ParserResult<T> Build<T>(

private static T BuildMutable<T>(Maybe<Func<T>> factory, IEnumerable<SpecificationProperty> specPropsWithValue, List<Error> setPropertyErrors )
{
var mutable = factory.MapValueOrDefault(f => f(), Activator.CreateInstance<T>());
var mutable = factory.MapValueOrDefault(f => f(), () => Activator.CreateInstance<T>());

setPropertyErrors.AddRange(
mutable.SetProperties(
Expand Down
17 changes: 14 additions & 3 deletions src/CommandLine/Core/ReflectionExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -133,10 +133,21 @@ public static bool IsMutable(this Type type)
if(type == typeof(object))
return true;

var props = type.GetTypeInfo().GetProperties(BindingFlags.Public | BindingFlags.Instance).Any(p => p.CanWrite);
var fields = type.GetTypeInfo().GetFields(BindingFlags.Public | BindingFlags.Instance).Any();
// Find all inherited defined properties and fields on the type
var inheritedTypes = type.GetTypeInfo().FlattenHierarchy().Select(i => i.GetTypeInfo());

return props || fields;
foreach (var inheritedType in inheritedTypes)
{
if (
inheritedType.GetTypeInfo().GetProperties(BindingFlags.Public | BindingFlags.Instance).Any(p => p.CanWrite) ||
inheritedType.GetTypeInfo().GetFields(BindingFlags.Public | BindingFlags.Instance).Any()
)
{
return true;
}
}

return false;
}

public static object CreateDefaultForImmutable(this Type type)
Expand Down
8 changes: 8 additions & 0 deletions src/CommandLine/Infrastructure/CSharpx/Maybe.cs
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,14 @@ public static T2 MapValueOrDefault<T1, T2>(this Maybe<T1> maybe, Func<T1, T2> fu
return maybe.MatchJust(out value1) ? func(value1) : noneValue;
}

/// <summary>
/// If contains a values executes a mapping function over it, otherwise returns the value from <paramref name="noneValueFactory"/>.
/// </summary>
public static T2 MapValueOrDefault<T1, T2>(this Maybe<T1> maybe, Func<T1, T2> func, Func<T2> noneValueFactory) {
T1 value1;
return maybe.MatchJust(out value1) ? func(value1) : noneValueFactory();
}

/// <summary>
/// Returns an empty list when given <see cref="CSharpx.Nothing{T}"/> or a singleton list when given a <see cref="CSharpx.Just{T}"/>.
/// </summary>
Expand Down
1 change: 0 additions & 1 deletion src/CommandLine/Parser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,6 @@ public ParserResult<T> ParseArguments<T>(IEnumerable<string> args)
/// and a sequence of <see cref="CommandLine.Error"/>.</returns>
/// <exception cref="System.ArgumentNullException">Thrown if one or more arguments are null.</exception>
public ParserResult<T> ParseArguments<T>(Func<T> factory, IEnumerable<string> args)
where T : new()
{
if (factory == null) throw new ArgumentNullException("factory");
if (!typeof(T).IsMutable()) throw new ArgumentException("factory");
Expand Down
19 changes: 19 additions & 0 deletions tests/CommandLine.Tests/Fakes/Mutable_Without_Empty_Constructor.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Copyright 2005-2015 Giacomo Stelluti Scala & Contributors. All rights reserved. See License.md in the project root for license information.

namespace CommandLine.Tests.Fakes
{
class Mutable_Without_Empty_Constructor
{
[Option("amend", HelpText = "Used to amend the tip of the current branch.")]
public bool Amend { get; set; }

private Mutable_Without_Empty_Constructor()
{
}

public static Mutable_Without_Empty_Constructor Create()
{
return new Mutable_Without_Empty_Constructor();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
namespace CommandLine.Tests.Fakes
{
class Options_With_Only_Explicit_Interface : IInterface_With_Two_Scalar_Options
{
bool IInterface_With_Two_Scalar_Options.Verbose { get; set; }

string IInterface_With_Two_Scalar_Options.InputFile { get; set; }
}
}
14 changes: 14 additions & 0 deletions tests/CommandLine.Tests/Unit/Core/InstanceBuilderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -787,6 +787,20 @@ public void Specifying_options_two_or_more_times_with_mixed_short_long_options_g
((NotParsed<Simple_Options>)result).Errors.Should().HaveCount(x => x == expected);
}

[Theory]
[InlineData(new[] { "--inputfile=file1.bin" }, "file1.bin")]
[InlineData(new[] { "--inputfile", "file2.txt" }, "file2.txt")]
public void Can_define_options_on_explicit_interface_properties(string[] arguments, string expected)
{
// Exercize system
var result = InvokeBuild<Options_With_Only_Explicit_Interface>(
arguments);

// Verify outcome
expected.Should().BeEquivalentTo(((IInterface_With_Two_Scalar_Options)((Parsed<Options_With_Only_Explicit_Interface>)result).Value).InputFile);
}


[Theory]
[InlineData(new[] { "--inputfile=file1.bin" }, "file1.bin")]
[InlineData(new[] { "--inputfile", "file2.txt" }, "file2.txt")]
Expand Down
29 changes: 29 additions & 0 deletions tests/CommandLine.Tests/Unit/Issue591ests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
using System.Linq;
using CommandLine.Tests.Fakes;
using CommandLine.Text;
using FluentAssertions;
using Xunit;
using Xunit.Abstractions;

//Issue #591
//When options class is only having explicit interface declarations, it should be detected as mutable.

namespace CommandLine.Tests.Unit
{
public class Issue591ests
{
[Fact]
public void Parse_option_with_only_explicit_interface_implementation()
{
string actual = string.Empty;

var arguments = new[] { "--inputfile", "file2.txt" };
var result = Parser.Default.ParseArguments<Options_With_Only_Explicit_Interface>(arguments);
result.WithParsed(options => {
actual = ((IInterface_With_Two_Scalar_Options)options).InputFile;
});

actual.Should().Be("file2.txt");
}
}
}
29 changes: 29 additions & 0 deletions tests/CommandLine.Tests/Unit/Issue70Tests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
using System.Linq;
using CommandLine.Tests.Fakes;
using CommandLine.Text;
using FluentAssertions;
using Xunit;
using Xunit.Abstractions;

//Issue #70
//When the factory overload is used for ParseArguments, there should be no constraint not having an empty constructor.

namespace CommandLine.Tests.Unit
{
public class Issue70Tests
{
[Fact]
public void Create_instance_with_factory_method_should_not_fail()
{
bool actual = false;

var arguments = new[] { "--amend" };
var result = Parser.Default.ParseArguments(() => Mutable_Without_Empty_Constructor.Create(), arguments);
result.WithParsed(options => {
actual = options.Amend;
});

actual.Should().BeTrue();
}
}
}