Skip to content

Commit 77c170f

Browse files
jonsequiturjonsequitur
authored
fix #1621 (#1622)
* fix #1621 * fix nullability; a little refactoring
1 parent 790da59 commit 77c170f

File tree

5 files changed

+74
-142
lines changed

5 files changed

+74
-142
lines changed

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

Lines changed: 37 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -660,7 +660,6 @@ public void Options_with_arguments_specified_can_be_correctly_converted_to_bool_
660660
option.Parse("-x true").GetValueForOption<bool>(option).Should().BeTrue();
661661
}
662662

663-
664663
[Fact]
665664
public void Values_can_be_correctly_converted_to_long_without_the_parser_specifying_a_custom_converter()
666665
{
@@ -684,7 +683,7 @@ public void Values_can_be_correctly_converted_to_nullable_long_without_the_parse
684683
[Fact]
685684
public void Values_can_be_correctly_converted_to_short_without_the_parser_specifying_a_custom_converter()
686685
{
687-
var option = new Option<ushort>("-s");
686+
var option = new Option<short>("-s");
688687

689688
var value = option.Parse("-s 1234").GetValueForOption(option);
690689

@@ -701,22 +700,42 @@ public void Values_can_be_correctly_converted_to_nullable_short_without_the_pars
701700
value.Should().Be(1234);
702701
}
703702

703+
[Fact]
704+
public void Values_can_be_correctly_converted_to_ulong_without_the_parser_specifying_a_custom_converter()
705+
{
706+
var option = new Option<ulong>("-x");
707+
708+
var value = option.Parse("-x 1234").GetValueForOption(option);
709+
710+
value.Should().Be(1234);
711+
}
712+
713+
[Fact]
714+
public void Values_can_be_correctly_converted_to_nullable_ulong_without_the_parser_specifying_a_custom_converter()
715+
{
716+
var option = new Option<ulong?>("-x");
717+
718+
var value = option.Parse("-x 1234").GetValueForOption(option);
719+
720+
value.Should().Be(1234);
721+
}
722+
704723
[Fact]
705724
public void Values_can_be_correctly_converted_to_ushort_without_the_parser_specifying_a_custom_converter()
706725
{
707-
var option = new Option<ushort>("-us");
726+
var option = new Option<ushort>("-x");
708727

709-
var value = option.Parse("-us 1234").GetValueForOption(option);
728+
var value = option.Parse("-x 1234").GetValueForOption(option);
710729

711730
value.Should().Be(1234);
712731
}
713732

714733
[Fact]
715734
public void Values_can_be_correctly_converted_to_nullable_ushort_without_the_parser_specifying_a_custom_converter()
716735
{
717-
var option = new Option<ushort?>("-us");
736+
var option = new Option<ushort?>("-x");
718737

719-
var value = option.Parse("-us 1234").GetValueForOption(option);
738+
var value = option.Parse("-x 1234").GetValueForOption(option);
720739

721740
value.Should().Be(1234);
722741
}
@@ -803,6 +822,18 @@ public void Enum_values_can_be_correctly_converted_based_on_enum_value_name_with
803822
value.Should().Be(DayOfWeek.Monday);
804823
}
805824

825+
[Fact]
826+
public void Nullable_enum_values_can_be_correctly_converted_based_on_enum_value_name_without_the_parser_specifying_a_custom_converter()
827+
{
828+
var option = new Option<DayOfWeek?>("-x");
829+
830+
var parseResult = option.Parse("-x Monday");
831+
832+
var value = parseResult.GetValueForOption(option);
833+
834+
value.Should().Be(DayOfWeek.Monday);
835+
}
836+
806837
[Fact]
807838
public void Enum_values_that_cannot_be_parsed_result_in_an_informative_error()
808839
{

‎src/System.CommandLine/Binding/ArgumentConverter.StringConverters.cs‎

Lines changed: 6 additions & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -22,18 +22,6 @@ internal static partial class ArgumentConverter
2222
return false;
2323
},
2424

25-
[typeof(bool?)] = (string token, out object? value) =>
26-
{
27-
if (bool.TryParse(token, out var parsed))
28-
{
29-
value = parsed;
30-
return true;
31-
}
32-
33-
value = default;
34-
return false;
35-
},
36-
3725
[typeof(DateTime)] = (string input, out object? value) =>
3826
{
3927
if (DateTime.TryParse(input, out var parsed))
@@ -46,18 +34,6 @@ internal static partial class ArgumentConverter
4634
return false;
4735
},
4836

49-
[typeof(DateTime?)] = (string input, out object? value) =>
50-
{
51-
if (DateTime.TryParse(input, out var parsed))
52-
{
53-
value = parsed;
54-
return true;
55-
}
56-
57-
value = default;
58-
return false;
59-
},
60-
6137
[typeof(DateTimeOffset)] = (string input, out object? value) =>
6238
{
6339
if (DateTimeOffset.TryParse(input, out var parsed))
@@ -70,19 +46,6 @@ internal static partial class ArgumentConverter
7046
return false;
7147
},
7248

73-
[typeof(DateTimeOffset?)] = (string input, out object? value) =>
74-
{
75-
if (DateTimeOffset.TryParse(input, out var parsed))
76-
{
77-
value = parsed;
78-
return true;
79-
}
80-
81-
value = default;
82-
return false;
83-
},
84-
85-
8649
[typeof(decimal)] = (string input, out object? value) =>
8750
{
8851
if (decimal.TryParse(input, out var parsed))
@@ -95,18 +58,6 @@ internal static partial class ArgumentConverter
9558
return false;
9659
},
9760

98-
[typeof(decimal?)] = (string input, out object? value) =>
99-
{
100-
if (decimal.TryParse(input, out var parsed))
101-
{
102-
value = parsed;
103-
return true;
104-
}
105-
106-
value = default;
107-
return false;
108-
},
109-
11061
[typeof(DirectoryInfo)] = (string path, out object? value) =>
11162
{
11263
value = new DirectoryInfo(path);
@@ -125,18 +76,6 @@ internal static partial class ArgumentConverter
12576
return false;
12677
},
12778

128-
[typeof(double?)] = (string input, out object? value) =>
129-
{
130-
if (double.TryParse(input, out var parsed))
131-
{
132-
value = parsed;
133-
return true;
134-
}
135-
136-
value = default;
137-
return false;
138-
},
139-
14079
[typeof(FileInfo)] = (string path, out object? value) =>
14180
{
14281
value = new FileInfo(path);
@@ -173,18 +112,6 @@ internal static partial class ArgumentConverter
173112
value = default;
174113
return false;
175114
},
176-
177-
[typeof(float?)] = (string input, out object? value) =>
178-
{
179-
if (float.TryParse(input, out var parsed))
180-
{
181-
value = parsed;
182-
return true;
183-
}
184-
185-
value = default;
186-
return false;
187-
},
188115

189116
[typeof(int)] = (string token, out object? value) =>
190117
{
@@ -198,18 +125,6 @@ internal static partial class ArgumentConverter
198125
return false;
199126
},
200127

201-
[typeof(int?)] = (string token, out object? value) =>
202-
{
203-
if (int.TryParse(token, out var intValue))
204-
{
205-
value = intValue;
206-
return true;
207-
}
208-
209-
value = default;
210-
return false;
211-
},
212-
213128
[typeof(long)] = (string token, out object? value) =>
214129
{
215130
if (long.TryParse(token, out var longValue))
@@ -222,18 +137,6 @@ internal static partial class ArgumentConverter
222137
return false;
223138
},
224139

225-
[typeof(long?)] = (string token, out object? value) =>
226-
{
227-
if (long.TryParse(token, out var longValue))
228-
{
229-
value = longValue;
230-
return true;
231-
}
232-
233-
value = default;
234-
return false;
235-
},
236-
237140
[typeof(short)] = (string token, out object? value) =>
238141
{
239142
if (short.TryParse(token, out var shortValue))
@@ -246,21 +149,15 @@ internal static partial class ArgumentConverter
246149
return false;
247150
},
248151

249-
[typeof(short?)] = (string token, out object? value) =>
152+
[typeof(string)] = (string input, out object? value) =>
250153
{
251-
if (short.TryParse(token, out var shortValue))
252-
{
253-
value = shortValue;
254-
return true;
255-
}
256-
257-
value = default;
258-
return false;
154+
value = input;
155+
return true;
259156
},
260157

261-
[typeof(ushort)] = (string token, out object? value) =>
158+
[typeof(ulong)] = (string token, out object? value) =>
262159
{
263-
if (ushort.TryParse(token, out var ushortValue))
160+
if (ulong.TryParse(token, out var ushortValue))
264161
{
265162
value = ushortValue;
266163
return true;
@@ -270,7 +167,7 @@ internal static partial class ArgumentConverter
270167
return false;
271168
},
272169

273-
[typeof(ushort?)] = (string token, out object? value) =>
170+
[typeof(ushort)] = (string token, out object? value) =>
274171
{
275172
if (ushort.TryParse(token, out var ushortValue))
276173
{
@@ -282,12 +179,6 @@ internal static partial class ArgumentConverter
282179
return false;
283180
},
284181

285-
[typeof(string)] = (string input, out object? value) =>
286-
{
287-
value = input;
288-
return true;
289-
},
290-
291182
[typeof(Uri)] = (string input, out object? value) =>
292183
{
293184
if (Uri.TryCreate(input, UriKind.RelativeOrAbsolute, out var uri))

‎src/System.CommandLine/Binding/ArgumentConverter.cs‎

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,11 @@ private static ArgumentConversionResult ConvertString(
4545
string value,
4646
LocalizationResources localizationResources)
4747
{
48+
if (type.TryGetNullableType(out var nullableType))
49+
{
50+
return ConvertString(argument, nullableType, value, localizationResources);
51+
}
52+
4853
if (_stringConverters.TryGetValue(type, out var tryConvert))
4954
{
5055
if (tryConvert(value, out var converted))
@@ -233,21 +238,6 @@ internal static T GetValueOrDefault<T>(this ArgumentConversionResult result)
233238
};
234239
}
235240

236-
public static bool TryConvertBoolArgument(ArgumentResult argumentResult, out object? value)
237-
{
238-
if (argumentResult.Tokens.Count == 0)
239-
{
240-
value = true;
241-
return true;
242-
}
243-
else
244-
{
245-
var success = bool.TryParse(argumentResult.Tokens[0].Value, out var parsed);
246-
value = parsed;
247-
return success;
248-
}
249-
}
250-
251241
public static bool TryConvertArgument(ArgumentResult argumentResult, out object? value)
252242
{
253243
var argument = argumentResult.Argument;

‎src/System.CommandLine/Binding/TypeExtensions.cs‎

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

44
using System.Collections;
5+
using System.Diagnostics.CodeAnalysis;
56

67
namespace System.CommandLine.Binding
78
{
@@ -51,5 +52,24 @@ internal static bool IsNullable(this Type t)
5152
return t.IsGenericType &&
5253
t.GetGenericTypeDefinition() == typeof(Nullable<>);
5354
}
55+
56+
internal static bool TryGetNullableType(
57+
this Type type,
58+
[NotNullWhen(true)] out Type? nullableType)
59+
{
60+
if (type.IsGenericType)
61+
{
62+
var genericTypeDefinition = type.GetGenericTypeDefinition();
63+
64+
if (genericTypeDefinition == typeof(Nullable<>))
65+
{
66+
nullableType = type.GetGenericArguments()[0];
67+
return true;
68+
}
69+
}
70+
71+
nullableType = null;
72+
return false;
73+
}
5474
}
5575
}

‎src/System.CommandLine/Completions/CompletionSource.cs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,21 +45,21 @@ public IEnumerable<CompletionItem> GetCompletions(CompletionContext context)
4545
return _innerCompletionSource.GetCompletions(context);
4646
}
4747

48-
private static ICompletionSource CreateForType(Type t)
48+
private static ICompletionSource CreateForType(Type type)
4949
{
50-
if (t.IsNullable())
50+
if (type.TryGetNullableType(out var nullableType))
5151
{
52-
t = t.GetGenericArguments().Single();
52+
return CreateForType(nullableType);
5353
}
5454

55-
if (t.IsEnum)
55+
if (type.IsEnum)
5656
{
5757
return new AnonymousCompletionSource(_ => GetEnumNames());
5858

59-
IEnumerable<CompletionItem> GetEnumNames() => Enum.GetNames(t).Select(n => new CompletionItem(n));
59+
IEnumerable<CompletionItem> GetEnumNames() => Enum.GetNames(type).Select(n => new CompletionItem(n));
6060
}
6161

62-
if (t == typeof(bool))
62+
if (type == typeof(bool))
6363
{
6464
return new AnonymousCompletionSource(static _ => new CompletionItem[]
6565
{

0 commit comments

Comments
 (0)