Skip to content

Commit 16e687b

Browse files
KebooKeboo
authored
Default empty collections when argument not specified (#1171)
* Handling non-generic option * Assert item not set * More test, non-generic support * More non-generic tests * Adding string case * Updated with length check to avoid LINQ overhead
1 parent bd3a466 commit 16e687b

File tree

3 files changed

+192
-13
lines changed

3 files changed

+192
-13
lines changed

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

Lines changed: 124 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
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.Collections.Generic;
45
using System.CommandLine.Parsing;
56
using System.Linq;
67
using FluentAssertions;
@@ -56,7 +57,6 @@ public void RawAliases_is_aware_of_added_alias()
5657
option.HasAlias("--added").Should().BeTrue();
5758
}
5859

59-
6060
[Fact]
6161
public void A_prefixed_alias_can_be_added_to_an_option()
6262
{
@@ -331,6 +331,129 @@ public void Option_T_default_value_is_validated()
331331
.BeEquivalentTo(new[] { "ERR" });
332332
}
333333

334+
[Fact]
335+
public void Option_of_string_defaults_to_empty_when_not_specified()
336+
{
337+
var option = new Option<string>("-x");
338+
339+
var result = option.Parse("");
340+
result.HasOption(option)
341+
.Should()
342+
.BeFalse();
343+
result.ValueForOption(option)
344+
.Should()
345+
.BeEmpty();
346+
}
347+
348+
[Fact]
349+
public void Option_of_IEnumerable_of_T_defaults_to_empty_when_not_specified()
350+
{
351+
var option = new Option<IEnumerable<string>>("-x");
352+
353+
var result = option.Parse("");
354+
result.HasOption(option)
355+
.Should()
356+
.BeFalse();
357+
result.ValueForOption(option)
358+
.Should()
359+
.BeEmpty();
360+
}
361+
362+
[Fact]
363+
public void Option_of_Array_defaults_to_empty_when_not_specified()
364+
{
365+
var option = new Option<string[]>("-x");
366+
367+
var result = option.Parse("");
368+
result.HasOption(option)
369+
.Should()
370+
.BeFalse();
371+
result.ValueForOption(option)
372+
.Should()
373+
.BeEmpty();
374+
}
375+
376+
[Fact]
377+
public void Option_of_List_defaults_to_empty_when_not_specified()
378+
{
379+
var option = new Option<List<string>>("-x");
380+
381+
var result = option.Parse("");
382+
result.HasOption(option)
383+
.Should()
384+
.BeFalse();
385+
result.ValueForOption(option)
386+
.Should()
387+
.BeEmpty();
388+
}
389+
390+
[Fact]
391+
public void Option_of_IList_of_T_defaults_to_empty_when_not_specified()
392+
{
393+
var option = new Option<IList<string>>("-x");
394+
395+
var result = option.Parse("");
396+
result.HasOption(option)
397+
.Should()
398+
.BeFalse();
399+
result.ValueForOption(option)
400+
.Should()
401+
.BeEmpty();
402+
}
403+
404+
[Fact]
405+
public void Option_of_IList_defaults_to_empty_when_not_specified()
406+
{
407+
var option = new Option<System.Collections.IList>("-x");
408+
var result = option.Parse("");
409+
result.HasOption(option)
410+
.Should()
411+
.BeFalse();
412+
result.ValueForOption(option)
413+
.Should()
414+
.BeEmpty();
415+
}
416+
417+
[Fact]
418+
public void Option_of_ICollection_defaults_to_empty_when_not_specified()
419+
{
420+
var option = new Option<System.Collections.ICollection>("-x");
421+
var result = option.Parse("");
422+
result.HasOption(option)
423+
.Should()
424+
.BeFalse();
425+
result.ValueForOption(option)
426+
.Should()
427+
.BeEmpty();
428+
}
429+
430+
[Fact]
431+
public void Option_of_IEnumerable_defaults_to_empty_when_not_specified()
432+
{
433+
var option = new Option<System.Collections.IEnumerable>("-x");
434+
var result = option.Parse("");
435+
result.HasOption(option)
436+
.Should()
437+
.BeFalse();
438+
result.ValueForOption(option)
439+
.Should()
440+
.BeEmpty();
441+
}
442+
443+
[Fact]
444+
public void Option_of_ICollection_of_T_defaults_to_empty_when_not_specified()
445+
{
446+
var option = new Option<ICollection<string>>("-x");
447+
448+
var result = option.Parse("");
449+
result.HasOption(option)
450+
.Should()
451+
.BeFalse();
452+
result.ValueForOption(option)
453+
.Should()
454+
.BeEmpty();
455+
}
456+
334457
[Fact]
335458
public void Single_option_arg_is_matched_when_disallowing_multiple_args_per_option_token()
336459
{

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

Lines changed: 65 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,11 @@ static IList CreateArray(Type itemType, int capacity)
195195
.GetInterfaces()
196196
.FirstOrDefault(IsEnumerable);
197197

198-
return enumerableInterface?.GenericTypeArguments[0];
198+
return enumerableInterface?.GenericTypeArguments switch
199+
{
200+
{ Length: 1 } genericTypeArguments => genericTypeArguments[0],
201+
_ => null
202+
};
199203
}
200204

201205
internal static bool IsEnumerable(this Type type)
@@ -330,17 +334,68 @@ internal static ArgumentConversionResult ConvertIfNeeded(
330334
[return: MaybeNull]
331335
internal static T GetValueOrDefault<T>(this ArgumentConversionResult result)
332336
{
333-
switch (result)
337+
return result switch
334338
{
335-
case SuccessfulArgumentConversionResult successful:
336-
return (T)successful.Value!;
337-
case FailedArgumentConversionResult failed:
338-
throw new InvalidOperationException(failed.ErrorMessage);
339-
case NoArgumentConversionResult _:
340-
return default!;
341-
default:
342-
return default!;
339+
SuccessfulArgumentConversionResult successful => (T)successful.Value!,
340+
FailedArgumentConversionResult failed => throw new InvalidOperationException(failed.ErrorMessage),
341+
NoArgumentConversionResult _ => default!,
342+
_ => default!,
343+
};
344+
}
345+
346+
[return: MaybeNull]
347+
internal static T GetDefaultValue<T>()
348+
{
349+
return (T)GetDefaultValue(typeof(T));
350+
}
351+
352+
private static MethodInfo EnumerableEmptyMethod { get; }
353+
= typeof(Enumerable).GetMethod(nameof(Enumerable.Empty));
354+
355+
internal static object? GetDefaultValue(Type type)
356+
{
357+
if (type == typeof(string)) return "";
358+
if (GetItemTypeIfEnumerable(type) is Type itemType)
359+
{
360+
if (type.IsArray)
361+
{
362+
return CreateEmptyArray(itemType);
363+
}
364+
if (type.IsGenericType)
365+
{
366+
return type.GetGenericTypeDefinition() switch
367+
{
368+
Type enumerable when enumerable == typeof(IEnumerable<>) => GetEmptyEnumerable(itemType),
369+
Type list when list == typeof(List<>) => GetEmptyList(itemType),
370+
Type array when array == typeof(IList<>) ||
371+
array == typeof(ICollection<>) => CreateEmptyArray(itemType),
372+
_ => null
373+
};
374+
}
343375
}
376+
return type switch
377+
{
378+
Type nonGeneric
379+
when nonGeneric == typeof(IList) ||
380+
nonGeneric == typeof(ICollection) ||
381+
nonGeneric == typeof(IEnumerable)
382+
=> CreateEmptyArray(typeof(object)),
383+
_ => null
384+
};
385+
386+
static object GetEmptyList(Type itemType)
387+
{
388+
return Activator.CreateInstance(typeof(List<>).MakeGenericType(itemType));
389+
}
390+
391+
static IEnumerable GetEmptyEnumerable(Type itemType)
392+
{
393+
var genericMethod = EnumerableEmptyMethod.MakeGenericMethod(itemType);
394+
return (IEnumerable)genericMethod.Invoke(null, new object[0]);
395+
}
396+
397+
static Array CreateEmptyArray(Type itemType)
398+
=> Array.CreateInstance(itemType, 0);
344399
}
345400

346401
public static bool TryConvertBoolArgument(ArgumentResult argumentResult, out object? value)

‎src/System.CommandLine/Parsing/ParseResult.cs

Lines changed: 3 additions & 2 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.Generic;
5+
using System.CommandLine.Binding;
56
using System.Diagnostics.CodeAnalysis;
67
using System.Linq;
78

@@ -146,7 +147,7 @@ public T ValueForOption<T>(Option<T> option)
146147
return t;
147148
}
148149

149-
return default;
150+
return (T)ArgumentConverter.GetDefaultValue(option.Argument.ArgumentType);
150151
}
151152

152153
[return: MaybeNull]
@@ -158,7 +159,7 @@ public T ValueForOption<T>(Option option)
158159
return t;
159160
}
160161

161-
return default;
162+
return (T)ArgumentConverter.GetDefaultValue(option.Argument.ArgumentType);
162163
}
163164

164165
[return: MaybeNull]

0 commit comments

Comments
 (0)