Skip to content

Commit d87107b

Browse files
committed
fix NRE when binding no args to list type property
1 parent 2557b0c commit d87107b

File tree

3 files changed

+68
-30
lines changed

3 files changed

+68
-30
lines changed

src/System.CommandLine.Tests/Binding/ModelBinderTests.cs

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
using FluentAssertions;
1111
using Xunit;
1212
using System.Reflection;
13+
using System.Threading.Tasks;
1314

1415
namespace System.CommandLine.Tests.Binding
1516
{
@@ -627,12 +628,7 @@ public void Default_values_from_options_with_the_same_type_are_bound_and_use_the
627628
second.Should().Be(2);
628629
}
629630

630-
public class MyModel
631-
{
632-
public List<string> Abc { get; set; }
633-
634-
public string AbcDef { get; set; }
635-
}
631+
636632

637633
[Fact]
638634
public void Binder_does_not_match_on_partial_name()
@@ -642,10 +638,10 @@ public void Binder_does_not_match_on_partial_name()
642638
new Option<List<string>>("--abc")
643639
};
644640

645-
MyModel boundValue = default;
641+
ClassWithOnePropertyNameThatIsSubstringOfAnother boundValue = default;
646642

647643
command.Handler = CommandHandler.Create(
648-
(MyModel s) =>
644+
(ClassWithOnePropertyNameThatIsSubstringOfAnother s) =>
649645
{
650646
boundValue = s;
651647
}
@@ -660,5 +656,25 @@ public void Binder_does_not_match_on_partial_name()
660656
.Should()
661657
.Be("1");
662658
}
659+
660+
[Fact]
661+
public async Task Empty_input_is_bound_correctly_to_list_type_properties()
662+
{
663+
ClassWithListTypePropertiesAndDefaultCtor boundInstance = default;
664+
665+
var cmd = new RootCommand
666+
{
667+
Handler = CommandHandler.Create((ClassWithListTypePropertiesAndDefaultCtor value) =>
668+
{
669+
boundInstance = value;
670+
})
671+
};
672+
673+
var result = cmd.Parse();
674+
675+
await result.InvokeAsync();
676+
677+
boundInstance.Should().NotBeNull();
678+
}
663679
}
664680
}

src/System.CommandLine.Tests/Binding/TestModels.cs

Lines changed: 28 additions & 14 deletions
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.Threading.Tasks;
56

67
namespace System.CommandLine.Tests.Binding
@@ -29,21 +30,11 @@ public class ClassWithMultiLetterSetters
2930
public bool BoolOption { get; set; }
3031
}
3132

32-
public class ClassWithSettersAndCtorParametersWithDifferentNames
33+
public class ClassWithListTypePropertiesAndDefaultCtor
3334
{
34-
public ClassWithSettersAndCtorParametersWithDifferentNames(
35-
int i = 123,
36-
string s = "the default",
37-
bool b = false)
38-
{
39-
IntOption = i;
40-
StringOption = s;
41-
BoolOption = b;
42-
}
35+
public List<string> Strings { get; set; }
4336

44-
public int IntOption { get; set; }
45-
public string StringOption { get; set; }
46-
public bool BoolOption { get; set; }
37+
public IEnumerable<int> BoolOption { get; set; }
4738
}
4839

4940
public class ClassWithCtorParameter<T>
@@ -92,7 +83,6 @@ public class ClassWithMultipleCtor
9283
{
9384
public ClassWithMultipleCtor()
9485
{
95-
9686
}
9787

9888
public ClassWithMultipleCtor(int intProperty)
@@ -102,4 +92,28 @@ public ClassWithMultipleCtor(int intProperty)
10292

10393
public int IntProperty { get; }
10494
}
95+
96+
public class ClassWithSettersAndCtorParametersWithDifferentNames
97+
{
98+
public ClassWithSettersAndCtorParametersWithDifferentNames(
99+
int i = 123,
100+
string s = "the default",
101+
bool b = false)
102+
{
103+
IntOption = i;
104+
StringOption = s;
105+
BoolOption = b;
106+
}
107+
108+
public int IntOption { get; set; }
109+
public string StringOption { get; set; }
110+
public bool BoolOption { get; set; }
111+
}
112+
113+
public class ClassWithOnePropertyNameThatIsSubstringOfAnother
114+
{
115+
public List<string> Abc { get; set; }
116+
117+
public string AbcDef { get; set; }
118+
}
105119
}

src/System.CommandLine/Binding/ModelBinder.cs

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -65,24 +65,32 @@ public void BindMemberFromValue(PropertyInfo property, IValueDescriptor valueDes
6565
return newInstance;
6666
}
6767

68-
private (bool success, object? newInstance, bool anyNonDefaults) CreateInstanceInternal(
69-
BindingContext bindingContext)
68+
private (bool success, object? newInstance, bool anyNonDefaults) CreateInstanceInternal(BindingContext bindingContext)
7069
{
7170
if (DisallowedBindingType())
7271
{
7372
throw new InvalidOperationException($"The type {ModelDescriptor.ModelType} cannot be bound");
7473
}
74+
7575
if (ShortCutTheBinding())
7676
{
7777
return GetSimpleModelValue(MemberBindingSources, bindingContext);
7878
}
7979

8080
var constructorAndArgs = GetBestConstructorAndArgs(bindingContext);
81-
var constructor = constructorAndArgs.Constructor;
82-
var boundValues = constructorAndArgs.BoundValues;
83-
var nonDefaultsUsed = constructorAndArgs.NonDefaultsUsed;
8481

85-
return InstanceFromSpecificConstructor(bindingContext, constructor, boundValues, ref nonDefaultsUsed);
82+
if (constructorAndArgs is null)
83+
{
84+
return GetSimpleModelValue(ConstructorArgumentBindingSources, bindingContext);
85+
}
86+
else
87+
{
88+
var constructor = constructorAndArgs.Constructor;
89+
var boundValues = constructorAndArgs.BoundValues;
90+
var nonDefaultsUsed = constructorAndArgs.NonDefaultsUsed;
91+
92+
return InstanceFromSpecificConstructor(bindingContext, constructor, boundValues, ref nonDefaultsUsed);
93+
}
8694
}
8795

8896
private bool DisallowedBindingType()
@@ -153,7 +161,7 @@ private bool UpdateInstanceInternalNotifyIfNonDefaultsUsed<T>(T instance, Bindin
153161
return anyNonDefaults;
154162
}
155163

156-
private ConstructorAndArgs GetBestConstructorAndArgs(BindingContext bindingContext)
164+
private ConstructorAndArgs? GetBestConstructorAndArgs(BindingContext bindingContext)
157165
{
158166
var constructorDescriptors =
159167
ModelDescriptor
@@ -184,7 +192,7 @@ private ConstructorAndArgs GetBestConstructorAndArgs(BindingContext bindingConte
184192
}
185193
}
186194

187-
return bestNonMatching!;
195+
return bestNonMatching;
188196
}
189197

190198
internal static (IReadOnlyList<BoundValue> boundValues, bool anyNonDefaults) GetBoundValues(

0 commit comments

Comments
 (0)