Skip to content

Commit f5ee0ab

Browse files
jonsequiturKeboo
jonsequitur
and
Keboo
authored
fix #728 (#730)
* fix #728 * Update src/System.CommandLine/Argument.cs Co-Authored-By: Kevin B <[email protected]> * fix #728 Co-authored-by: Kevin B <[email protected]>
1 parent f26d980 commit f5ee0ab

File tree

8 files changed

+106
-27
lines changed

8 files changed

+106
-27
lines changed

‎src/System.CommandLine.DragonFruit.Tests/ConfigureFromMethodTests.cs‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,8 @@ public async Task Single_character_parameters_generate_aliases_that_accept_a_sin
7171

7272
[Theory]
7373
[InlineData(nameof(Method_having_string_argument), 1, 1)]
74-
[InlineData(nameof(Method_having_string_argument_with_default_value), 0, 1)]
74+
[InlineData(nameof(Method_having_string_argument_with_null_default_value), 0, 1)]
75+
[InlineData(nameof(Method_having_string_argument_with_non_null_default_value), 0, 1)]
7576
[InlineData(nameof(Method_having_string_array_arguments), 0, byte.MaxValue)]
7677
[InlineData(nameof(Method_having_string_array_arguments_with_default_value), 0, byte.MaxValue)]
7778
[InlineData(nameof(Method_having_FileInfo_argument), 1, 1)]
@@ -95,7 +96,7 @@ public void Parameters_named_arguments_generate_command_arguments_having_the_cor
9596

9697
[Theory]
9798
[InlineData(nameof(Method_having_string_argument), "argument")]
98-
[InlineData(nameof(Method_having_string_argument_with_default_value), "argument")]
99+
[InlineData(nameof(Method_having_string_argument_with_null_default_value), "argument")]
99100
[InlineData(nameof(Method_having_string_array_arguments), "arguments")]
100101
[InlineData(nameof(Method_having_string_array_arguments_with_default_value), "arguments")]
101102
[InlineData(nameof(Method_having_FileInfo_argument), "argument")]
@@ -116,7 +117,7 @@ public void Parameters_named_arguments_generate_command_arguments_having_the_cor
116117

117118
[Theory]
118119
[InlineData(nameof(Method_having_string_argument))]
119-
[InlineData(nameof(Method_having_string_argument_with_default_value))]
120+
[InlineData(nameof(Method_having_string_argument_with_null_default_value))]
120121
[InlineData(nameof(Method_having_string_array_arguments))]
121122
[InlineData(nameof(Method_having_string_array_arguments_with_default_value))]
122123
[InlineData(nameof(Method_having_FileInfo_argument))]
@@ -145,7 +146,7 @@ public void Options_are_not_generated_for_command_argument_parameters(string met
145146

146147
[Theory]
147148
[InlineData(nameof(Method_having_string_argument), typeof(string))]
148-
[InlineData(nameof(Method_having_string_argument_with_default_value), typeof(string))]
149+
[InlineData(nameof(Method_having_string_argument_with_null_default_value), typeof(string))]
149150
[InlineData(nameof(Method_having_string_array_arguments), typeof(string[]))]
150151
[InlineData(nameof(Method_having_string_array_arguments_with_default_value), typeof(string[]))]
151152
[InlineData(nameof(Method_having_FileInfo_argument), typeof(FileInfo))]
@@ -265,7 +266,11 @@ internal void Method_having_string_argument(string stringOption, int intOption,
265266
{
266267
}
267268

268-
internal void Method_having_string_argument_with_default_value(string stringOption, int intOption, string argument = null)
269+
internal void Method_having_string_argument_with_null_default_value(string stringOption, int intOption, string argument = null)
270+
{
271+
}
272+
273+
internal void Method_having_string_argument_with_non_null_default_value(string stringOption, int intOption, string argument = "the-default-value")
269274
{
270275
}
271276

‎src/System.CommandLine.DragonFruit/CommandLine.cs‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,14 @@ public static void ConfigureFromMethod(
172172

173173
if (argsParam.HasDefaultValue)
174174
{
175-
argument.SetDefaultValue(argsParam.DefaultValue);
175+
if (argsParam.DefaultValue != null)
176+
{
177+
argument.SetDefaultValue(argsParam.DefaultValue);
178+
}
179+
else
180+
{
181+
argument.SetDefaultValueFactory(() => null);
182+
}
176183
}
177184

178185
command.AddArgument(argument);
@@ -289,7 +296,7 @@ public static Option BuildOption(this ParameterDescriptor parameter)
289296

290297
if (parameter.HasDefaultValue)
291298
{
292-
argument.SetDefaultValue(parameter.GetDefaultValue);
299+
argument.SetDefaultValueFactory(parameter.GetDefaultValue);
293300
}
294301

295302
var option = new Option(

‎src/System.CommandLine.Tests/ArgumentTests.cs‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
// Copyright (c) .NET Foundation and contributors. All rights reserved.
2+
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
3+
4+
using FluentAssertions;
5+
using Xunit;
6+
7+
namespace System.CommandLine.Tests
8+
{
9+
public class ArgumentTests : SymbolTests
10+
{
11+
[Fact]
12+
public void By_default_there_is_no_default_value()
13+
{
14+
var argument = new Argument();
15+
16+
argument.HasDefaultValue.Should().BeFalse();
17+
}
18+
19+
[Fact]
20+
public void When_default_value_is_set_to_null_then_HasDefaultValue_is_true()
21+
{
22+
var argument = new Argument();
23+
24+
argument.SetDefaultValue(null);
25+
26+
argument.HasDefaultValue.Should().BeTrue();
27+
}
28+
29+
[Fact]
30+
public void When_default_value_factory_is_set_then_HasDefaultValue_is_true()
31+
{
32+
var argument = new Argument();
33+
34+
argument.SetDefaultValueFactory(() => null);
35+
36+
argument.HasDefaultValue.Should().BeTrue();
37+
}
38+
39+
[Fact]
40+
public void When_there_is_no_default_value_then_GetDefaultValue_throws()
41+
{
42+
var argument = new Argument<string>("the-arg");
43+
44+
argument.Invoking(a => a.GetDefaultValue())
45+
.Should()
46+
.Throw<InvalidOperationException>()
47+
.Which
48+
.Message
49+
.Should()
50+
.Be("Argument \"the-arg\" does not have a default value");
51+
}
52+
53+
protected override Symbol CreateSymbol(string name)
54+
{
55+
return new Argument(name);
56+
}
57+
}
58+
}

‎src/System.CommandLine.Tests/Binding/TypeConversionTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -396,7 +396,7 @@ public void
396396
}
397397
};
398398

399-
option.Argument.SetDefaultValue(() => "the-default");
399+
option.Argument.SetDefaultValueFactory(() => "the-default");
400400

401401
IReadOnlyCollection<Symbol> symbols = new[]
402402
{

‎src/System.CommandLine.Tests/SymbolResultTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ public void Default_values_are_reevaluated_and_not_cached_between_parses()
4040
};
4141

4242
var i = 0;
43-
option.Argument.SetDefaultValue(() => ++i);
43+
option.Argument.SetDefaultValueFactory(() => ++i);
4444

4545
var result1 = option.Parse("");
4646
var result2 = option.Parse("");

‎src/System.CommandLine/Argument.cs‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ namespace System.CommandLine
1111
{
1212
public class Argument : Symbol, IArgument
1313
{
14-
private Func<object> _defaultValue;
14+
private Func<object> _getDefaultValue;
1515
private readonly List<string> _suggestions = new List<string>();
1616
private readonly List<ISuggestionSource> _suggestionSources = new List<ISuggestionSource>();
1717
private IArgumentArity _arity;
@@ -114,13 +114,27 @@ bool DefaultConvert(SymbolResult symbol, out object value)
114114

115115
public void AddValidator(ValidateSymbol<ArgumentResult> validator) => Validators.Add(validator);
116116

117-
public object GetDefaultValue() => _defaultValue?.Invoke();
117+
public object GetDefaultValue()
118+
{
119+
if (_getDefaultValue is null)
120+
{
121+
throw new InvalidOperationException($"Argument \"{Name}\" does not have a default value");
122+
}
118123

119-
public void SetDefaultValue(object value) => SetDefaultValue(() => value);
124+
return _getDefaultValue.Invoke();
125+
}
120126

121-
public void SetDefaultValue(Func<object> value) => _defaultValue = value;
127+
public void SetDefaultValue(object value)
128+
{
129+
SetDefaultValueFactory(() => value);
130+
}
131+
132+
public void SetDefaultValueFactory(Func<object> getValue)
133+
{
134+
_getDefaultValue = getValue ?? throw new ArgumentNullException(nameof(getValue));
135+
}
122136

123-
public bool HasDefaultValue => _defaultValue != null;
137+
public bool HasDefaultValue => _getDefaultValue != null;
124138

125139
internal static Argument None => new Argument { Arity = ArgumentArity.Zero };
126140

‎src/System.CommandLine/ArgumentArity.cs‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -122,13 +122,8 @@ internal static IArgumentArity Default(Type type, Argument argument, ISymbol par
122122
}
123123

124124
if (parent is ICommand &&
125-
type.IsNullable())
126-
{
127-
return ZeroOrOne;
128-
}
129-
130-
if (parent is ICommand &&
131-
argument.HasDefaultValue)
125+
(argument.HasDefaultValue ||
126+
type.IsNullable()))
132127
{
133128
return ZeroOrOne;
134129
}

‎src/System.CommandLine/Argument{T}.cs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,17 +18,17 @@ public Argument() : base(null)
1818
ArgumentType = typeof(T);
1919
}
2020

21-
public Argument(string name, Func<T> defaultValue) : this(name)
21+
public Argument(string name, Func<T> getDefaultValue) : this(name)
2222
{
23-
SetDefaultValue(() => defaultValue());
23+
SetDefaultValueFactory(() => getDefaultValue());
2424
}
2525

2626
public Argument(Func<T> defaultValue) : this()
2727
{
28-
SetDefaultValue(() => defaultValue());
28+
SetDefaultValueFactory(() => defaultValue());
2929
}
3030

31-
public Argument(TryConvertArgument<T> convert, Func<T> defaultValue = default) : this()
31+
public Argument(TryConvertArgument<T> convert, Func<T> getDefaultValue = default) : this()
3232
{
3333
if (convert == null)
3434
{
@@ -49,9 +49,9 @@ public Argument(TryConvertArgument<T> convert, Func<T> defaultValue = default) :
4949
}
5050
};
5151

52-
if (defaultValue != default)
52+
if (getDefaultValue != default)
5353
{
54-
SetDefaultValue(() => defaultValue());
54+
SetDefaultValueFactory(() => getDefaultValue());
5555
}
5656
}
5757
}

0 commit comments

Comments
 (0)