Skip to content

Commit 703b2cd

Browse files
jonsequiturjonsequitur
authored
Merge pull request #780 from jonsequitur/remove-TryConvert
remove TryConvertArgument<T>
2 parents 5c4eafa + 9eb9496 commit 703b2cd

File tree

4 files changed

+114
-210
lines changed

4 files changed

+114
-210
lines changed

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

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,13 @@
22
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
33

44
using System.Collections.Generic;
5+
using System.CommandLine.Builder;
6+
using System.CommandLine.Invocation;
57
using System.CommandLine.Parsing;
68
using System.IO;
79
using FluentAssertions;
810
using System.Linq;
11+
using System.Threading.Tasks;
912
using Xunit;
1013

1114
namespace System.CommandLine.Tests
@@ -263,6 +266,106 @@ public void Command_ArgumentResult_Parent_is_set_correctly_when_token_is_implici
263266
.Should()
264267
.Be(command);
265268
}
269+
270+
[Fact]
271+
public async Task Custom_argument_parser_is_only_called_once()
272+
{
273+
var callCount = 0;
274+
var handlerWasCalled = false;
275+
276+
var command = new RootCommand
277+
{
278+
Handler = CommandHandler.Create<int>(Run)
279+
};
280+
command.AddOption(new Option("--value")
281+
{
282+
Argument = new Argument<int>(result =>
283+
{
284+
callCount++;
285+
return int.Parse(result.Tokens.Single().Value);
286+
})
287+
});
288+
289+
await command.InvokeAsync("--value 42");
290+
291+
callCount.Should().Be(1);
292+
handlerWasCalled.Should().BeTrue();
293+
294+
void Run(int value) => handlerWasCalled = true;
295+
}
296+
297+
[Fact]
298+
public void Default_value_and_custom_argument_parser_can_be_used_together()
299+
{
300+
var argument = new Argument<int>(_ => 789, true);
301+
argument.SetDefaultValue(123);
302+
303+
var result = argument.Parse("");
304+
305+
var argumentResult = result.FindResultFor(argument);
306+
307+
argumentResult
308+
.GetValueOrDefault<int>()
309+
.Should()
310+
.Be(123);
311+
}
312+
313+
[Fact]
314+
public void When_custom_conversion_fails_then_an_option_does_not_accept_further_arguments()
315+
{
316+
var command = new Command("the-command")
317+
{
318+
new Argument<string>(),
319+
new Option("-x")
320+
{
321+
Argument = new Argument<string>(argResult =>
322+
{
323+
argResult.ErrorMessage = "nope";
324+
return default;
325+
})
326+
}
327+
};
328+
329+
var result = command.Parse("the-command -x nope yep");
330+
331+
result.CommandResult.Tokens.Count.Should().Be(1);
332+
}
333+
334+
[Fact]
335+
public void When_argument_cannot_be_parsed_as_the_specified_type_then_getting_value_throws()
336+
{
337+
var command = new Command("the-command")
338+
{
339+
new Option(new[] { "-o", "--one" })
340+
{
341+
Argument = new Argument<int>(argumentResult =>
342+
{
343+
if (int.TryParse(argumentResult.Tokens.Select(t => t.Value).Single(), out var value))
344+
{
345+
return value;
346+
}
347+
348+
argumentResult.ErrorMessage = $"'{argumentResult.Tokens.Single().Value}' is not an integer";
349+
350+
return default;
351+
}),
352+
Description = ""
353+
}
354+
};
355+
356+
var result = command.Parse("the-command -o not-an-int");
357+
358+
Action getValue = () =>
359+
result.CommandResult.ValueForOption("o");
360+
361+
getValue.Should()
362+
.Throw<InvalidOperationException>()
363+
.Which
364+
.Message
365+
.Should()
366+
.Be("'not-an-int' is not an integer");
367+
}
368+
266369
}
267370

268371
protected override Symbol CreateSymbol(string name)

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

Lines changed: 0 additions & 171 deletions
Original file line numberDiff line numberDiff line change
@@ -2,55 +2,16 @@
22
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
33

44
using System.Collections.Generic;
5-
using System.CommandLine.Builder;
6-
using System.CommandLine.Invocation;
75
using System.CommandLine.Parsing;
86
using System.IO;
97
using FluentAssertions;
108
using System.Linq;
11-
using System.Threading.Tasks;
129
using Xunit;
1310

1411
namespace System.CommandLine.Tests.Binding
1512
{
1613
public class TypeConversionTests
1714
{
18-
[Fact]
19-
public void Custom_types_and_conversion_logic_can_be_specified()
20-
{
21-
var argument = new Argument<MyCustomType>((ArgumentResult parsed, out MyCustomType value) =>
22-
{
23-
var custom = new MyCustomType();
24-
foreach (var a in parsed.Tokens)
25-
{
26-
custom.Add(a.Value);
27-
}
28-
29-
value = custom;
30-
return true;
31-
})
32-
{
33-
Arity = ArgumentArity.ZeroOrMore,
34-
Name = "arg"
35-
};
36-
37-
var parser = new Parser(
38-
new Command("custom")
39-
{
40-
argument
41-
});
42-
43-
var result = parser.Parse("custom one two three");
44-
45-
var customType = result.CommandResult
46-
.GetArgumentValueOrDefault<MyCustomType>("arg");
47-
48-
customType
49-
.Values
50-
.Should()
51-
.BeEquivalentTo("one", "two", "three");
52-
}
53-
5415
[Fact]
5516
public void Option_argument_with_arity_of_one_can_be_bound_without_custom_conversion_logic_if_the_type_has_a_constructor_that_takes_a_single_string()
5617
{
@@ -254,41 +215,6 @@ public void Bool_does_not_parse_as_the_default_value_when_the_option_has_been_ap
254215
.Be(true);
255216
}
256217

257-
[Fact]
258-
public void When_argument_cannot_be_parsed_as_the_specified_type_then_getting_value_throws()
259-
{
260-
var command = new Command("the-command")
261-
{
262-
new Option(new[] { "-o", "--one" })
263-
{
264-
Argument = new Argument<int>((ArgumentResult argumentResult, out int value) =>
265-
{
266-
if (int.TryParse(argumentResult.Tokens.Select(t => t.Value).Single(), out value))
267-
{
268-
return true;
269-
}
270-
271-
argumentResult.ErrorMessage = $"'{argumentResult.Tokens.Single().Value}' is not an integer";
272-
273-
return false;
274-
}),
275-
Description = ""
276-
}
277-
};
278-
279-
var result = command.Parse("the-command -o not-an-int");
280-
281-
Action getValue = () =>
282-
result.CommandResult.ValueForOption("o");
283-
284-
getValue.Should()
285-
.Throw<InvalidOperationException>()
286-
.Which
287-
.Message
288-
.Should()
289-
.Be("'not-an-int' is not an integer");
290-
}
291-
292218
[Fact]
293219
public void By_default_an_option_with_zero_or_one_argument_parses_as_the_argument_string_value_by_default()
294220
{
@@ -669,48 +595,6 @@ public void Specifying_an_option_argument_overrides_the_default_value()
669595
value.Should().Be(456);
670596
}
671597

672-
[Fact]
673-
public void When_custom_converter_is_specified_and_an_argument_is_of_the_wrong_type_then_an_error_is_returned()
674-
{
675-
var command = new Command("tally")
676-
{
677-
new Argument<int>((ArgumentResult symbolResult, out int value) =>
678-
{
679-
value = default;
680-
symbolResult.ErrorMessage = "Could not parse int";
681-
return false;
682-
})
683-
};
684-
685-
var result = command.Parse("tally one");
686-
687-
result.Errors
688-
.Select(e => e.Message)
689-
.Should()
690-
.Contain("Could not parse int");
691-
}
692-
693-
[Fact]
694-
public void When_custom_conversion_fails_then_an_option_does_not_accept_further_arguments()
695-
{
696-
var command = new Command("the-command")
697-
{
698-
new Argument<string>(),
699-
new Option("-x")
700-
{
701-
Argument = new Argument<string>((ArgumentResult symbolResult, out string value) =>
702-
{
703-
value = null;
704-
return false;
705-
})
706-
}
707-
};
708-
709-
var result = command.Parse("the-command -x nope yep");
710-
711-
result.CommandResult.Tokens.Count.Should().Be(1);
712-
}
713-
714598
[Fact]
715599
public void Values_can_be_correctly_converted_to_int_without_the_parser_specifying_a_custom_converter()
716600
{
@@ -961,61 +845,6 @@ public void When_getting_an_array_of_values_and_specifying_a_conversion_type_tha
961845
.Be("Option '-x' expects a single argument but 2 were provided.");
962846
}
963847

964-
[Fact]
965-
public async Task Custom_argument_converter_is_only_called_once()
966-
{
967-
var callCount = 0;
968-
var handlerWasCalled = false;
969-
970-
var parser = new CommandLineBuilder(
971-
new RootCommand
972-
{
973-
Handler = CommandHandler.Create<int>(Run)
974-
})
975-
.AddOption(new Option("--value")
976-
{
977-
Argument = new Argument<int>(TryConvertInt)
978-
})
979-
.UseDefaults()
980-
.Build();
981-
982-
await parser.InvokeAsync("--value 42");
983-
984-
callCount.Should().Be(1);
985-
handlerWasCalled.Should().BeTrue();
986-
987-
bool TryConvertInt(ArgumentResult result, out int value)
988-
{
989-
callCount++;
990-
return int.TryParse(result.Tokens.Single().Value, out value);
991-
}
992-
993-
void Run(int value) => handlerWasCalled = true;
994-
}
995-
996-
[Fact]
997-
public void Default_value_and_custom_argument_converter_can_be_used_together()
998-
{
999-
bool TryConvertArgument(SymbolResult _, out int value)
1000-
{
1001-
value = 789;
1002-
return true;
1003-
}
1004-
1005-
var argument = new Argument<int>(
1006-
TryConvertArgument,
1007-
() => 123);
1008-
1009-
var result = new RootCommand { argument }.Parse("");
1010-
1011-
var argumentResult = result.FindResultFor(argument);
1012-
1013-
argumentResult
1014-
.GetValueOrDefault<int>()
1015-
.Should()
1016-
.Be(123);
1017-
}
1018-
1019848
public class MyCustomType
1020849
{
1021850
private readonly List<string> values = new List<string>();

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

Lines changed: 11 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// Copyright (c) .NET Foundation and contributors. All rights reserved.
22
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
33

4-
using System.CommandLine.Binding;
54
using System.CommandLine.Parsing;
65

76
namespace System.CommandLine
@@ -38,35 +37,16 @@ public Argument(Func<T> getDefaultValue) : this()
3837
SetDefaultValueFactory(() => getDefaultValue());
3938
}
4039

41-
public Argument(TryConvertArgument<T> convert, Func<T> getDefaultValue = default) : this()
40+
public Argument(
41+
string name,
42+
ParseArgument<T> parse,
43+
bool isDefault = false) : this()
4244
{
43-
if (convert == null)
45+
if (!string.IsNullOrWhiteSpace(name))
4446
{
45-
throw new ArgumentNullException(nameof(convert));
47+
Name = name;
4648
}
4749

48-
ConvertArguments = (ArgumentResult result, out object value) =>
49-
{
50-
if (convert(result, out var valueObj))
51-
{
52-
value = valueObj;
53-
return true;
54-
}
55-
else
56-
{
57-
value = default;
58-
return false;
59-
}
60-
};
61-
62-
if (getDefaultValue != default)
63-
{
64-
SetDefaultValueFactory(() => getDefaultValue());
65-
}
66-
}
67-
68-
public Argument(ParseArgument<T> parse, bool isDefault = false) : this()
69-
{
7050
if (parse == null)
7151
{
7252
throw new ArgumentNullException(nameof(parse));
@@ -80,7 +60,7 @@ public Argument(ParseArgument<T> parse, bool isDefault = false) : this()
8060
ConvertArguments = (ArgumentResult argumentResult, out object value) =>
8161
{
8262
var result = parse(argumentResult);
83-
63+
8464
if (string.IsNullOrEmpty(argumentResult.ErrorMessage))
8565
{
8666
value = result;
@@ -93,5 +73,9 @@ public Argument(ParseArgument<T> parse, bool isDefault = false) : this()
9373
}
9474
};
9575
}
76+
77+
public Argument(ParseArgument<T> parse, bool isDefault = false) : this(null, parse, isDefault)
78+
{
79+
}
9680
}
9781
}

0 commit comments

Comments
 (0)