Skip to content

Commit e219656

Browse files
jonsequiturjonsequitur
committed
remove Option.ArgumentIsHidden and Option.ArgumentType; exclude option argument's name from binding; R: cleanup
1 parent c079d2b commit e219656

File tree

7 files changed

+38
-128
lines changed

7 files changed

+38
-128
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ public void Options_are_not_built_for_infrastructure_types_exposed_by_method_par
220220
var options = handlerMethod.BuildOptions();
221221

222222
options.Should()
223-
.NotContain(o => o.ArgumentType == type);
223+
.NotContain(o => o.ValueType == type);
224224
}
225225

226226
[Fact]

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

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,6 @@ public async Task Option_arguments_are_bound_by_name_to_method_parameters(
3737
var command = new Command("the-command")
3838
{
3939
new Option("--value", argumentType: type)
40-
{
41-
ArgumentName = "value"
42-
}
4340
};
4441

4542
var console = new TestConsole();
@@ -72,9 +69,6 @@ public async Task Option_arguments_are_bound_by_name_to_the_properties_of_method
7269
var command = new Command("the-command")
7370
{
7471
new Option("--value", argumentType: type)
75-
{
76-
ArgumentName = "value"
77-
}
7872
};
7973

8074
var console = new TestConsole();
@@ -107,9 +101,6 @@ public async Task Option_arguments_are_bound_by_name_to_the_constructor_paramete
107101
var command = new Command("the-command")
108102
{
109103
new Option("--value", argumentType: type)
110-
{
111-
ArgumentName = "value"
112-
}
113104
};
114105

115106
var console = new TestConsole();
@@ -120,26 +111,6 @@ await handler.InvokeAsync(
120111
console.Out.ToString().Should().Be($"ClassWithCtorParameter<{type.Name}>: {expectedValue}");
121112
}
122113

123-
[Fact]
124-
public void When_name_is_not_among_aliases_then_binder_will_bind_option_by_name()
125-
{
126-
var rootCommand = new RootCommand
127-
{
128-
new Option<string[]>("-n")
129-
{
130-
ArgumentName = "name"
131-
}
132-
};
133-
134-
string[] receivedHeaders = null;
135-
136-
rootCommand.Handler = CommandHandler.Create((string[] name) => receivedHeaders = name);
137-
138-
rootCommand.Invoke("-n one -n two");
139-
140-
receivedHeaders.Should().BeEquivalentTo("one", "two");
141-
}
142-
143114
[Theory]
144115
[InlineData(typeof(string), "hello", "hello")]
145116
[InlineData(typeof(int), "123", 123)]
@@ -190,9 +161,6 @@ public async Task Unspecified_option_arguments_with_no_default_value_are_bound_t
190161
var command = new Command("command")
191162
{
192163
new Option("-x", argumentType: parameterType)
193-
{
194-
ArgumentName = "value"
195-
}
196164
};
197165

198166
command.Handler = handler;

‎src/System.CommandLine.Tests/Help/HelpBuilderTests.cs

Lines changed: 0 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1161,30 +1161,6 @@ public void Options_section_properly_wraps()
11611161
_console.Out.ToString().Should().Contain(expected);
11621162
}
11631163

1164-
[Fact]
1165-
public void Options_section_does_not_contain_hidden_argument()
1166-
{
1167-
var command = new Command("the-command", "Does things.");
1168-
var opt1 = new Option<int>("option1")
1169-
{
1170-
ArgumentName = "the-hidden",
1171-
ArgumentIsHidden = true,
1172-
};
1173-
var opt2 = new Option<int>("option2")
1174-
{
1175-
ArgumentName = "the-visible",
1176-
ArgumentIsHidden = false
1177-
};
1178-
command.AddOption(opt1);
1179-
command.AddOption(opt2);
1180-
1181-
_helpBuilder.Write(command);
1182-
var help = _console.Out.ToString();
1183-
1184-
help.Should().NotContain("the-hidden");
1185-
help.Should().Contain("the-visible");
1186-
}
1187-
11881164
[Fact]
11891165
public void Required_options_are_indicated()
11901166
{
@@ -1329,27 +1305,6 @@ public void Help_describes_default_value_for_option_with_argument_having_default
13291305
help.Should().Contain($"[default: the-arg-value]");
13301306
}
13311307

1332-
[Fact]
1333-
public void Help_should_not_contain_default_value_for_hidden_argument_defined_for_option()
1334-
{
1335-
var command = new Command("the-command", "command help")
1336-
{
1337-
new Option(new[] { "-arg"}, getDefaultValue: () => "the-arg-value")
1338-
{
1339-
ArgumentName = "the-arg",
1340-
ArgumentIsHidden = true
1341-
}
1342-
};
1343-
1344-
HelpBuilder helpBuilder = GetHelpBuilder(LargeMaxWidth);
1345-
1346-
helpBuilder.Write(command);
1347-
1348-
var help = _console.Out.ToString();
1349-
1350-
help.Should().NotContain($"[default: the-arg-value]");
1351-
}
1352-
13531308
[Fact]
13541309
public void Option_help_can_be_requested_in_isolation()
13551310
{

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

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -153,12 +153,9 @@ public void Command_GetSuggestions_with_text_to_match_orders_by_match_position_t
153153
public void When_an_option_has_a_default_value_it_will_still_be_suggested()
154154
{
155155
var parser = new Parser(
156-
new Option<string[]>("--apple", "kinds of apples")
157-
{
158-
ArgumentName = "cortland"
159-
},
160-
new Option<string[]>("--banana", "kinds of bananas"),
161-
new Option<string>("--cherry", "kinds of cherries"));
156+
new Option<string>("--apple", getDefaultValue: () => "cortland"),
157+
new Option<string>("--banana"),
158+
new Option<string>("--cherry"));
162159

163160
var result = parser.Parse("");
164161

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

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,6 @@ internal BoundValue(
1010
IValueDescriptor valueDescriptor,
1111
IValueSource valueSource)
1212
{
13-
if (value != null &&
14-
!valueDescriptor.ValueType.IsInstanceOfType(value))
15-
{
16-
throw new ArgumentException($"Value {value} ({value.GetType()}) must be an instance of type {valueDescriptor.ValueType}");
17-
}
18-
1913
Value = value;
2014
ValueDescriptor = valueDescriptor;
2115
ValueSource = valueSource;
@@ -53,5 +47,4 @@ public static BoundValue DefaultForValueDescriptor(IValueDescriptor valueDescrip
5347
valueSource);
5448
}
5549
}
56-
5750
}

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

Lines changed: 33 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1-
using System.Collections.Generic;
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 System.Collections.Generic;
25
using System.Linq;
36
using System.Reflection;
47

@@ -69,17 +72,17 @@ public void BindMemberFromValue(PropertyInfo property, IValueDescriptor valueDes
6972
{
7073
throw new InvalidOperationException($"The type {ModelDescriptor.ModelType} cannot be bound");
7174
}
72-
if (ShortCutTheBinding(bindingContext))
75+
if (ShortCutTheBinding())
7376
{
7477
return GetSimpleModelValue(MemberBindingSources, bindingContext);
7578
}
79+
7680
var constructorAndArgs = GetBestConstructorAndArgs(bindingContext);
7781
var constructor = constructorAndArgs.Constructor;
7882
var boundValues = constructorAndArgs.BoundValues;
79-
bool nonDefaultsUsed = constructorAndArgs.NonDefaultsUsed;
80-
return constructor is null
81-
? GetSimpleModelValue(ConstructorArgumentBindingSources, bindingContext)
82-
: InstanceFromSpecificConstructor(bindingContext, constructor, boundValues, ref nonDefaultsUsed);
83+
var nonDefaultsUsed = constructorAndArgs.NonDefaultsUsed;
84+
85+
return InstanceFromSpecificConstructor(bindingContext, constructor, boundValues, ref nonDefaultsUsed);
8386
}
8487

8588
private bool DisallowedBindingType()
@@ -89,7 +92,7 @@ private bool DisallowedBindingType()
8992
modelType.IsConstructedGenericTypeOf(typeof(ReadOnlySpan<>));
9093
}
9194

92-
private bool ShortCutTheBinding(BindingContext bindingContext)
95+
private bool ShortCutTheBinding()
9396
{
9497
var modelType = ModelDescriptor.ModelType;
9598
return modelType.IsPrimitive ||
@@ -98,7 +101,7 @@ private bool ShortCutTheBinding(BindingContext bindingContext)
98101
}
99102

100103
private (bool success, object? newInstance, bool anyNonDefaults) GetSimpleModelValue(
101-
IDictionary<IValueDescriptor, IValueSource>? bindingSources, BindingContext bindingContext)
104+
IDictionary<IValueDescriptor, IValueSource> bindingSources, BindingContext bindingContext)
102105
{
103106
var valueSource = GetValueSource(bindingSources, bindingContext, ValueDescriptor, EnforceExplicitBinding);
104107
return bindingContext.TryBindToScalarValue(ValueDescriptor,
@@ -156,7 +159,9 @@ private ConstructorAndArgs GetBestConstructorAndArgs(BindingContext bindingConte
156159
ModelDescriptor
157160
.ConstructorDescriptors
158161
.OrderByDescending(d => d.ParameterDescriptors.Count);
162+
159163
ConstructorAndArgs? bestNonMatching = null;
164+
160165
foreach (var constructor in constructorDescriptors)
161166
{
162167
var (boundValues, anyNonDefaults) = GetBoundValues(
@@ -171,19 +176,19 @@ private ConstructorAndArgs GetBestConstructorAndArgs(BindingContext bindingConte
171176
{
172177
var match = new ConstructorAndArgs(constructor, boundValues, anyNonDefaults);
173178
if (anyNonDefaults)
174-
{ // based on parameter length, first usable constructor that utilizes CLI definition
179+
{
180+
// based on parameter length, first usable constructor that utilizes CLI definition
175181
return match;
176182
}
177183
bestNonMatching ??= match;
178184
}
179185
}
180-
return bestNonMatching is null
181-
? new ConstructorAndArgs(null, null, false)
182-
: bestNonMatching;
186+
187+
return bestNonMatching!;
183188
}
184189

185190
internal static (IReadOnlyList<BoundValue> boundValues, bool anyNonDefaults) GetBoundValues(
186-
IDictionary<IValueDescriptor, IValueSource>? bindingSources,
191+
IDictionary<IValueDescriptor, IValueSource> bindingSources,
187192
BindingContext bindingContext,
188193
IReadOnlyList<IValueDescriptor> valueDescriptors,
189194
bool enforceExplicitBinding,
@@ -212,13 +217,12 @@ internal static (IReadOnlyList<BoundValue> boundValues, bool anyNonDefaults) Get
212217
return (values, anyNonDefaults);
213218
}
214219

215-
internal static IValueSource GetValueSource(IDictionary<IValueDescriptor, IValueSource>? bindingSources,
220+
internal static IValueSource GetValueSource(IDictionary<IValueDescriptor, IValueSource> bindingSources,
216221
BindingContext bindingContext,
217222
IValueDescriptor valueDescriptor,
218223
bool enforceExplicitBinding)
219224
{
220-
if (!(bindingSources is null) &&
221-
bindingSources.TryGetValue(valueDescriptor, out IValueSource? valueSource))
225+
if (bindingSources.TryGetValue(valueDescriptor, out IValueSource? valueSource))
222226
{
223227
return valueSource;
224228
}
@@ -234,7 +238,7 @@ internal static IValueSource GetValueSource(IDictionary<IValueDescriptor, IValue
234238
// by name and type (or a possible conversion)
235239
return new ParseResultMatchingValueSource();
236240
}
237-
241+
238242
return new MissingValueSource();
239243
}
240244

@@ -277,6 +281,7 @@ internal static (BoundValue? boundValue, bool usedNonDefault) GetBoundValue(
277281
// Logic dropped here - misnamed and purpose unclear: ShouldPassNullToConstructor(constructorDescriptor.Parent, constructorDescriptor))
278282
return (BoundValue.DefaultForType(valueDescriptor), false);
279283
}
284+
280285
return (null, false);
281286
}
282287

@@ -336,19 +341,19 @@ public AnonymousValueDescriptor(Type modelType)
336341

337342
public override string ToString() => $"{ValueType}";
338343
}
339-
}
340-
341-
internal class ConstructorAndArgs
342-
{
343-
public ConstructorDescriptor? Constructor { get; }
344-
public IReadOnlyList<BoundValue>? BoundValues { get; }
345-
public bool NonDefaultsUsed { get; }
346344

347-
public ConstructorAndArgs(ConstructorDescriptor? constructor, IReadOnlyList<BoundValue>? boundValues, bool nonDefaultsUsed)
345+
internal class ConstructorAndArgs
348346
{
349-
Constructor = constructor;
350-
BoundValues = boundValues;
351-
NonDefaultsUsed = nonDefaultsUsed;
347+
public ConstructorDescriptor Constructor { get; }
348+
public IReadOnlyList<BoundValue>? BoundValues { get; }
349+
public bool NonDefaultsUsed { get; }
350+
351+
public ConstructorAndArgs(ConstructorDescriptor constructor, IReadOnlyList<BoundValue>? boundValues, bool nonDefaultsUsed)
352+
{
353+
Constructor = constructor;
354+
BoundValues = boundValues;
355+
NonDefaultsUsed = nonDefaultsUsed;
356+
}
352357
}
353358
}
354359
}

‎src/System.CommandLine/Option.cs

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -112,14 +112,6 @@ public string? ArgumentDescription
112112
set => Argument.Description = value;
113113
}
114114

115-
public bool ArgumentIsHidden
116-
{
117-
get => Argument.IsHidden;
118-
set => Argument.IsHidden = value;
119-
}
120-
121-
public Type ArgumentType => Argument.ArgumentType;
122-
123115
private IEnumerable<Argument> Arguments => Children.OfType<Argument>();
124116

125117
internal bool DisallowBinding { get; set; }
@@ -173,7 +165,7 @@ private protected override void RemoveAlias(string alias)
173165

174166
string IValueDescriptor.ValueName => Name;
175167

176-
Type IValueDescriptor.ValueType => Argument.ArgumentType;
168+
public Type ValueType => Argument.ArgumentType;
177169

178170
bool IValueDescriptor.HasDefaultValue => Argument.HasDefaultValue;
179171

0 commit comments

Comments
 (0)