From fa703a987dcb736a737b63343022169ec103a794 Mon Sep 17 00:00:00 2001 From: Kathleen Dollard Date: Mon, 18 May 2020 15:36:13 -0700 Subject: [PATCH 1/3] Sync to upstream --- src/System.CommandLine/Option.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.CommandLine/Option.cs b/src/System.CommandLine/Option.cs index 75a6910208..74efa6be3c 100644 --- a/src/System.CommandLine/Option.cs +++ b/src/System.CommandLine/Option.cs @@ -45,7 +45,7 @@ public virtual Argument Argument IArgument IOption.Argument => Argument; public bool Required { get; set; } - + string IValueDescriptor.ValueName => Name; Type IValueDescriptor.ValueType => Argument.ArgumentType; From 0b64118facb6580f8e7b832efa3458b4d07e00d5 Mon Sep 17 00:00:00 2001 From: Kathleen Dollard Date: Wed, 5 Aug 2020 07:23:04 -0700 Subject: [PATCH 2/3] Prior to GetValue refactoring --- .../ModelBindingCommandHandlerTests.cs | 50 ++++++++++++ .../Invocation/InvocationExtensions.cs | 26 ++++++ .../Invocation/ModelBindingCommandHandler.cs | 80 +++++++++++++++++-- 3 files changed, 149 insertions(+), 7 deletions(-) create mode 100644 src/System.CommandLine/Invocation/InvocationExtensions.cs diff --git a/src/System.CommandLine.Tests/Binding/ModelBindingCommandHandlerTests.cs b/src/System.CommandLine.Tests/Binding/ModelBindingCommandHandlerTests.cs index 1d7d239114..b0b09359f4 100644 --- a/src/System.CommandLine.Tests/Binding/ModelBindingCommandHandlerTests.cs +++ b/src/System.CommandLine.Tests/Binding/ModelBindingCommandHandlerTests.cs @@ -458,6 +458,56 @@ public async Task Handler_method_receives_command_arguments_bound_to_the_specifi c.AssertBoundValue(boundValue); } + [Theory] + [InlineData(typeof(ClassWithCtorParameter))] + [InlineData(typeof(ClassWithSetter))] + [InlineData(typeof(ClassWithCtorParameter))] + [InlineData(typeof(ClassWithSetter))] + [InlineData(typeof(FileInfo))] + [InlineData(typeof(FileInfo[]))] + [InlineData(typeof(string[]))] + [InlineData(typeof(List))] + [InlineData(typeof(int[]))] + [InlineData(typeof(List))] + public async Task Handler_method_receives_command_arguments_explicitly_bound_to_the_specified_type( + Type type) + { + var c = _bindingCases[type]; + + var captureMethod = GetType() + .GetMethod(nameof(CaptureMethod), BindingFlags.NonPublic | BindingFlags.Static) + .MakeGenericMethod(c.ParameterType); + var parameter = captureMethod.GetParameters().First(); + + var handler = CommandHandler.Create(captureMethod); + + var argument = new Argument + { + Name = "value", + ArgumentType = c.ParameterType + }; + + var command = new Command( + "command") + { + argument + }; + handler.BindParameter(parameter, argument); + command.Handler = handler; + + var parseResult = command.Parse(c.CommandLine); + + var invocationContext = new InvocationContext(parseResult); + + await handler.InvokeAsync(invocationContext); + + var boundValue = ((BoundValueCapturer)invocationContext.InvocationResult).BoundValue; + + boundValue.Should().BeOfType(c.ParameterType); + + c.AssertBoundValue(boundValue); + } + private static void CaptureMethod(T value, InvocationContext invocationContext) { invocationContext.InvocationResult = new BoundValueCapturer(value); diff --git a/src/System.CommandLine/Invocation/InvocationExtensions.cs b/src/System.CommandLine/Invocation/InvocationExtensions.cs new file mode 100644 index 0000000000..7d267b2c93 --- /dev/null +++ b/src/System.CommandLine/Invocation/InvocationExtensions.cs @@ -0,0 +1,26 @@ +using System.Reflection; + +namespace System.CommandLine.Invocation +{ + public static class InvocationExtensions + { + public static void BindParameter(this ICommandHandler handler, ParameterInfo param, Option option) + { + // check for nulls + if (!(handler is ModelBindingCommandHandler bindingHandler)) + { + throw new InvalidOperationException("Cannot bind to this type of handler"); + } + bindingHandler.BindParameter(param, option); + } + + public static void BindParameter(this ICommandHandler handler, ParameterInfo param, Argument argument) + { + if (!(handler is ModelBindingCommandHandler bindingHandler)) + { + throw new InvalidOperationException("Cannot bind to this type of handler"); + } + bindingHandler.BindParameter(param, argument); + } + } +} diff --git a/src/System.CommandLine/Invocation/ModelBindingCommandHandler.cs b/src/System.CommandLine/Invocation/ModelBindingCommandHandler.cs index b037b26243..11da478040 100644 --- a/src/System.CommandLine/Invocation/ModelBindingCommandHandler.cs +++ b/src/System.CommandLine/Invocation/ModelBindingCommandHandler.cs @@ -17,6 +17,8 @@ internal class ModelBindingCommandHandler : ICommandHandler private readonly ModelBinder? _invocationTargetBinder; private readonly MethodInfo? _handlerMethodInfo; private readonly IReadOnlyList _parameterDescriptors; + private Dictionary _invokeArgumentBindingSources { get; } = + new Dictionary(); public ModelBindingCommandHandler( MethodInfo handlerMethodInfo, @@ -51,14 +53,22 @@ public async Task InvokeAsync(InvocationContext context) { var bindingContext = context.BindingContext; - var parameterBinders = _parameterDescriptors - .Select(p => bindingContext.GetModelBinder(p)) - .ToList(); + var invocationArguments = new object?[_parameterDescriptors.Count()]; + var length = _parameterDescriptors.Count(); - var invocationArguments = - parameterBinders - .Select(binder => binder.CreateInstance(bindingContext)) - .ToArray(); + for (int i = 0; i < length; i++) + { + var paramDesc = _parameterDescriptors[i]; + if (_invokeArgumentBindingSources.TryGetValue(paramDesc, out var valueSource)) + { + invocationArguments[i] = ValueFromValueSource(paramDesc, valueSource, bindingContext); + } + else + { + var binder = bindingContext.GetModelBinder(paramDesc); + invocationArguments[i] = binder.CreateInstance(bindingContext); + } + } var invocationTarget = _invocationTarget ?? _invocationTargetBinder?.CreateInstance(bindingContext); @@ -77,5 +87,61 @@ public async Task InvokeAsync(InvocationContext context) return await CommandHandler.GetResultCodeAsync(result, context); } + + private object? ValueFromValueSource(ParameterDescriptor paramDesc, IValueSource valueSource, BindingContext bindingContext) + { + BoundValue? boundValue; + if (valueSource is null) + { + // If there is no source to bind from, no value can be bound. + return null; + } + if (bindingContext.TryBindToScalarValue( + paramDesc, + valueSource, + out boundValue)) + { + // boundValue has been set + } + else if ( paramDesc.HasDefaultValue) + { + boundValue = BoundValue.DefaultForValueDescriptor(paramDesc); + } + if (!(boundValue is null)) + { + return boundValue.Value; + } + var parameterBinder = bindingContext.GetModelBinder(paramDesc); + return parameterBinder.CreateInstance(bindingContext); + } + + public void BindParameter(ParameterInfo param, Argument argument) + { + var _ = argument ?? throw new InvalidOperationException("You must specify an argument to bind"); + BindValueSource(param, new SpecificSymbolValueSource(argument)); + } + + public void BindParameter(ParameterInfo param, Option option) + { + var _ = option ?? throw new InvalidOperationException("You must specify an argument to bind"); + BindValueSource(param, new SpecificSymbolValueSource(option)); + } + + private void BindValueSource(ParameterInfo param, IValueSource valueSource) + { + var paramDesc = FindParameterDescriptor(param); + if (paramDesc is null) + { + throw new InvalidOperationException("You must bind to a parameter on this handler"); + } + _invokeArgumentBindingSources.Add(paramDesc, valueSource); + } + + private ParameterDescriptor? FindParameterDescriptor(ParameterInfo? param) + => param == null + ? null + : _parameterDescriptors + .FirstOrDefault(x => x.ValueName == param.Name && + x.ValueType == param.ParameterType); } } \ No newline at end of file From 4294e94a1d7f03bf137c4be2a5b318de5941d1d5 Mon Sep 17 00:00:00 2001 From: Kathleen Dollard Date: Wed, 5 Aug 2020 09:43:21 -0700 Subject: [PATCH 3/3] Refactor GetBoundValue --- src/System.CommandLine/Binding/ModelBinder.cs | 41 +++++++++++-------- .../Invocation/ModelBindingCommandHandler.cs | 41 ++++--------------- 2 files changed, 32 insertions(+), 50 deletions(-) diff --git a/src/System.CommandLine/Binding/ModelBinder.cs b/src/System.CommandLine/Binding/ModelBinder.cs index 60bd9f29e6..96319bb21c 100644 --- a/src/System.CommandLine/Binding/ModelBinder.cs +++ b/src/System.CommandLine/Binding/ModelBinder.cs @@ -84,13 +84,13 @@ public void BindConstructorArgumentFromValue(ParameterInfo parameter, if (ctorDesc is null) throw new ArgumentException(paramName: nameof(parameter), message: "Parameter is not described by any of the model constructor descriptors."); - + var paramDesc = ctorDesc.ParameterDescriptors[parameter.Position]; ConstructorArgumentBindingSources[paramDesc] = new SpecificSymbolValueSource(valueDescriptor); } - public void BindMemberFromValue(PropertyInfo property, + public void BindMemberFromValue(PropertyInfo property, IValueDescriptor valueDescriptor) { var propertyDescriptor = FindModelPropertyDescriptor( @@ -108,9 +108,9 @@ public void BindMemberFromValue(PropertyInfo property, var values = GetValues( // No binding sources, as were are attempting to bind a value // for the model itself, not for its ctor args or its members. - bindingSources: null, - bindingContext: context, - new[] { ValueDescriptor }, + bindingSources: null, + bindingContext: context, + new[] { ValueDescriptor }, includeMissingValues: false); if (values.Count == 1 && @@ -140,7 +140,7 @@ private bool TryDefaultConstructorAndPropertiesStrategy( { var boundConstructorArguments = GetValues( ConstructorArgumentBindingSources, - context, + context, constructor.ParameterDescriptors, true); @@ -201,14 +201,7 @@ private IReadOnlyList GetValues( var valueSource = GetValueSource(bindingSources, bindingContext, valueDescriptor); - BoundValue? boundValue; - if (!bindingContext.TryBindToScalarValue( - valueDescriptor, - valueSource, - out boundValue) && valueDescriptor.HasDefaultValue) - { - boundValue = BoundValue.DefaultForValueDescriptor(valueDescriptor); - } + BoundValue? boundValue = GetBoundValue(valueSource, bindingContext, valueDescriptor); if (boundValue is null) { @@ -219,13 +212,12 @@ private IReadOnlyList GetValues( { if (parameterDescriptor.HasDefaultValue) boundValue = BoundValue.DefaultForValueDescriptor(parameterDescriptor); - else if (parameterDescriptor.AllowsNull && + else if (parameterDescriptor.AllowsNull && ShouldPassNullToConstructor(constructorDescriptor.Parent, constructorDescriptor)) boundValue = BoundValue.DefaultForType(valueDescriptor); } } } - if (boundValue != null) { values.Add(boundValue); @@ -235,6 +227,21 @@ private IReadOnlyList GetValues( return values; } + internal static BoundValue? GetBoundValue(IValueSource valueSource, BindingContext bindingContext, + IValueDescriptor valueDescriptor) + { + BoundValue? boundValue; + if (!bindingContext.TryBindToScalarValue( + valueDescriptor, + valueSource, + out boundValue) && valueDescriptor.HasDefaultValue) + { + boundValue = BoundValue.DefaultForValueDescriptor(valueDescriptor); + } + + return boundValue; + } + private IValueSource GetValueSource( IDictionary? bindingSources, BindingContext bindingContext, @@ -264,7 +271,7 @@ private IValueSource GetValueSource( public override string ToString() => $"{ModelDescriptor.ModelType.Name}"; - private bool ShouldPassNullToConstructor(ModelDescriptor modelDescriptor, + private static bool ShouldPassNullToConstructor(ModelDescriptor modelDescriptor, ConstructorDescriptor? ctor = null) { if (!(ctor is null)) diff --git a/src/System.CommandLine/Invocation/ModelBindingCommandHandler.cs b/src/System.CommandLine/Invocation/ModelBindingCommandHandler.cs index 11da478040..a81f251049 100644 --- a/src/System.CommandLine/Invocation/ModelBindingCommandHandler.cs +++ b/src/System.CommandLine/Invocation/ModelBindingCommandHandler.cs @@ -61,13 +61,15 @@ public async Task InvokeAsync(InvocationContext context) var paramDesc = _parameterDescriptors[i]; if (_invokeArgumentBindingSources.TryGetValue(paramDesc, out var valueSource)) { - invocationArguments[i] = ValueFromValueSource(paramDesc, valueSource, bindingContext); - } - else - { - var binder = bindingContext.GetModelBinder(paramDesc); - invocationArguments[i] = binder.CreateInstance(bindingContext); + var boundValue = ModelBinder.GetBoundValue(valueSource, bindingContext, paramDesc); + if (!(boundValue is null)) + { + invocationArguments[i] = boundValue.Value; + continue; + } } + var binder = bindingContext.GetModelBinder(paramDesc); + invocationArguments[i] = binder.CreateInstance(bindingContext); } var invocationTarget = _invocationTarget ?? @@ -88,33 +90,6 @@ public async Task InvokeAsync(InvocationContext context) return await CommandHandler.GetResultCodeAsync(result, context); } - private object? ValueFromValueSource(ParameterDescriptor paramDesc, IValueSource valueSource, BindingContext bindingContext) - { - BoundValue? boundValue; - if (valueSource is null) - { - // If there is no source to bind from, no value can be bound. - return null; - } - if (bindingContext.TryBindToScalarValue( - paramDesc, - valueSource, - out boundValue)) - { - // boundValue has been set - } - else if ( paramDesc.HasDefaultValue) - { - boundValue = BoundValue.DefaultForValueDescriptor(paramDesc); - } - if (!(boundValue is null)) - { - return boundValue.Value; - } - var parameterBinder = bindingContext.GetModelBinder(paramDesc); - return parameterBinder.CreateInstance(bindingContext); - } - public void BindParameter(ParameterInfo param, Argument argument) { var _ = argument ?? throw new InvalidOperationException("You must specify an argument to bind");