enable easier custom model binding

dotnet · Apr 13, 2020 · ba783e4 · ba783e4
1 parent 25e6b36
commit ba783e4
Show file tree

Hide file tree

Showing 26 changed files with 170 additions and 231 deletions.
diff --git a/src/System.CommandLine.DragonFruit.Tests/ConfigureFromMethodTests.cs b/src/System.CommandLine.DragonFruit.Tests/ConfigureFromMethodTests.cs
@@ -162,7 +162,7 @@ public void Parameters_named_arguments_generate_command_arguments_having_the_cor
 
             var rootCommandArgument = parser.Configuration.RootCommand.Arguments.Single();
 
-            rootCommandArgument.Type
+            rootCommandArgument.ValueType
                                .Should()
                                .Be(expectedType);
         }

diff --git a/src/System.CommandLine.DragonFruit/CommandLine.cs b/src/System.CommandLine.DragonFruit/CommandLine.cs
@@ -279,7 +279,7 @@ public static IEnumerable<Option> BuildOptions(this MethodInfo type)
                                };
 
             foreach (var option in descriptor.ParameterDescriptors
-                                             .Where(d => !omittedTypes.Contains (d.Type))
+                                             .Where(d => !omittedTypes.Contains (d.ValueType))
                                              .Where(d => !_argumentParameterNames.Contains(d.ValueName))
                                              .Select(p => p.BuildOption()))
             {
@@ -291,7 +291,7 @@ public static Option BuildOption(this ParameterDescriptor parameter)
         {
             var argument = new Argument
                            {
-                               ArgumentType = parameter.Type
+                               ArgumentType = parameter.ValueType
                            };
 
             if (parameter.HasDefaultValue)

diff --git a/src/System.CommandLine.Tests/Binding/HandlerDescriptorTests.cs b/src/System.CommandLine.Tests/Binding/HandlerDescriptorTests.cs
@@ -41,7 +41,7 @@ public void It_provides_the_types_of_the_handler_parameters(Type parameterType)
                 var descriptor = HandlerDescriptor.FromMethodInfo(method);
 
                 descriptor.ParameterDescriptors
-                          .Select(p => p.Type)
+                          .Select(p => p.ValueType)
                           .Should()
                           .BeEquivalentSequenceTo(parameterType);
             }
@@ -76,7 +76,7 @@ public void It_provides_the_types_of_the_handler_parameters(Type parameterType)
                 var descriptor = HandlerDescriptor.FromMethodInfo(method);
 
                 descriptor.ParameterDescriptors
-                          .Select(p => p.Type)
+                          .Select(p => p.ValueType)
                           .Should()
                           .BeEquivalentSequenceTo(parameterType);
             }
@@ -85,30 +85,5 @@ public void It_provides_the_types_of_the_handler_parameters(Type parameterType)
         public void Handler<T>(T value)
         {
         }
-
-        public class FromExpression
-        {
-            [Fact]
-            public void Handler_descriptor_describes_the_parameter_names_of_the_handler_method()
-            {
-                var descriptor = HandlerDescriptor.FromExpression<ClassWithInvokeAndDefaultCtor, string, int, Task<int>>((model, s, i) => model.Invoke(s, i));
-
-                descriptor.ParameterDescriptors
-                          .Select(p => p.ValueName)
-                          .Should()
-                          .BeEquivalentSequenceTo("stringParam", "intParam");
-            }
-
-            [Fact]
-            public void Handler_descriptor_describes_the_parameter_types_of_the_handler_method()
-            {
-                var descriptor = HandlerDescriptor.FromExpression<ClassWithInvokeAndDefaultCtor, string, int, Task<int>>((model, s, i) => model.Invoke(s, i));
-
-                descriptor.ParameterDescriptors
-                          .Select(p => p.Type)
-                          .Should()
-                          .BeEquivalentSequenceTo(typeof(string), typeof(int));
-            }
-        }
     }
 }
diff --git a/src/System.CommandLine.Tests/Binding/ModelBinderTests.cs b/src/System.CommandLine.Tests/Binding/ModelBinderTests.cs
@@ -2,6 +2,8 @@
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
 using System.CommandLine.Binding;
+using System.CommandLine.Builder;
+using System.CommandLine.Invocation;
 using System.CommandLine.Parsing;
 using System.IO;
 using FluentAssertions;
@@ -523,8 +525,10 @@ public void Explicit_model_binder_binds_only_to_configured_properties()
             var parser = new Parser(intOption, stringOption);
 
             var bindingContext = new BindingContext(parser.Parse("--int-property 42 --string-property Hello"));
-            var binder = new ModelBinder<ClassWithMultiLetterSetters>()
-            { EnforceExplicitBinding = true };
+            var binder = new ModelBinder<ClassWithMultiLetterSetters>
+            {
+                EnforceExplicitBinding = true
+            };
             binder.BindMemberFromValue(obj => obj.IntOption, intOption);
             var instance = binder.CreateInstance(bindingContext) as ClassWithMultiLetterSetters;
 
@@ -545,14 +549,72 @@ public void Explicit_model_binder_binds_only_to_configured_ctor_parameters()
             var paramInfo = ctor.GetParameters().First();
 
             var bindingContext = new BindingContext(parser.Parse("-a 42 -b Hello"));
-            var binder = new ModelBinder<ClassWithMultiLetterCtorParameters>()
-            { EnforceExplicitBinding = true };
+            var binder = new ModelBinder<ClassWithMultiLetterCtorParameters>
+            {
+                EnforceExplicitBinding = true
+            };
             binder.BindConstructorArgumentFromValue(paramInfo, intOption);
             var instance = binder.CreateInstance(bindingContext) as ClassWithMultiLetterCtorParameters;
 
             instance.Should().NotBeNull();
             instance.IntOption.Should().Be(42);
             instance.StringOption.Should().Be("the default");
         }
+
+        [Fact]
+        public void Custom_ModelBinders_specified_via_BindingContext_can_be_used_for_option_binding()
+        {
+            ClassWithSetter<int> boundInstance = null;
+
+            var rootCommand = new RootCommand
+            {
+                new Option<int>("--value")
+            };
+
+            rootCommand.Handler = CommandHandler.Create<ClassWithSetter<int>>(x => boundInstance = x);
+
+            var parser = new CommandLineBuilder(rootCommand)
+                         .UseMiddleware(context =>
+                         {
+                             var binder = new ModelBinder<ClassWithSetter<int>>();
+
+                             binder.BindMemberFromValue(instance => instance.Value, _ => 456);
+
+                             context.BindingContext.AddModelBinder(binder);
+                         })
+                         .Build();
+
+            parser.Invoke("--value 123");
+
+            boundInstance.Value.Should().Be(456);
+        }
+
+        [Fact]
+        public void Custom_ModelBinders_specified_via_BindingContext_can_be_used_for_command_argument_binding()
+        {
+            ClassWithSetter<int> boundInstance = null;
+
+            var rootCommand = new RootCommand
+            {
+                new Argument<int>()
+            };
+
+            rootCommand.Handler = CommandHandler.Create<ClassWithSetter<int>>(x => boundInstance = x);
+
+            var parser = new CommandLineBuilder(rootCommand)
+                         .UseMiddleware(context =>
+                         {
+                             var binder = new ModelBinder<ClassWithSetter<int>>();
+
+                             binder.BindMemberFromValue(instance => instance.Value, _ => 456);
+
+                             context.BindingContext.AddModelBinder(binder);
+                         })
+                         .Build();
+
+            parser.Invoke("123");
+
+            boundInstance.Value.Should().Be(456);
+        }
     }
 }
diff --git a/src/System.CommandLine.Tests/Binding/TestModels.cs b/src/System.CommandLine.Tests/Binding/TestModels.cs
@@ -1,7 +1,6 @@
 // Copyright (c) .NET Foundation and contributors. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
-using System;
 using System.Threading.Tasks;
 
 namespace System.CommandLine.Tests.Binding

diff --git a/src/System.CommandLine/Argument.cs b/src/System.CommandLine/Argument.cs
@@ -217,6 +217,6 @@ public override IEnumerable<string> GetSuggestions(string textToMatch = null)
 
         string IValueDescriptor.ValueName => Name;
 
-        Type IValueDescriptor.Type => ArgumentType;
+        Type IValueDescriptor.ValueType => ArgumentType;
     }
 }
diff --git a/src/System.CommandLine/Binding/BindingContext.cs b/src/System.CommandLine/Binding/BindingContext.cs
@@ -1,21 +1,25 @@
 // Copyright (c) .NET Foundation and contributors. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
+using System.Collections.Generic;
 using System.CommandLine.Help;
 using System.CommandLine.Invocation;
 using System.CommandLine.IO;
 using System.CommandLine.Parsing;
 using System.Linq;
 
+#nullable enable
+
 namespace System.CommandLine.Binding
 {
     public sealed class BindingContext
     {
         private IConsole _console;
+        private readonly Dictionary<Type, ModelBinder> _modelBindersByValueDescriptor = new Dictionary<Type, ModelBinder>();
 
         public BindingContext(
             ParseResult parseResult,
-            IConsole console = null)
+            IConsole? console = default)
         {
             _console = console ?? new SystemConsole();
 
@@ -25,7 +29,7 @@ public BindingContext(
 
         public ParseResult ParseResult { get; set; }
 
-        internal IConsoleFactory ConsoleFactory { get; set; }
+        internal IConsoleFactory? ConsoleFactory { get; set; }
 
         internal IHelpBuilder HelpBuilder => (IHelpBuilder)ServiceProvider.GetService(typeof(IHelpBuilder));
 
@@ -46,18 +50,16 @@ public IConsole Console
 
         internal ServiceProvider ServiceProvider { get; }
 
-        public void AddService(Type serviceType, Func<IServiceProvider, object> factory)
-        {
-            if (serviceType == null)
-            {
-                throw new ArgumentNullException(nameof(serviceType));
-            }
+        public void AddModelBinder(ModelBinder binder) => 
+            _modelBindersByValueDescriptor.Add(binder.ValueDescriptor.ValueType, binder);
 
-            if (factory == null)
-            {
-                throw new ArgumentNullException(nameof(factory));
-            }
+        public ModelBinder GetModelBinder(IValueDescriptor valueDescriptor) =>
+            _modelBindersByValueDescriptor.GetOrAdd(
+                valueDescriptor.ValueType, 
+                _ => new ModelBinder(valueDescriptor));
 
+        public void AddService(Type serviceType, Func<IServiceProvider, object> factory)
+        {
             ServiceProvider.AddService(serviceType, factory);
         }
 
@@ -68,31 +70,31 @@ public void AddService<T>(Func<IServiceProvider, T> factory)
                 throw new ArgumentNullException(nameof(factory));
             }
 
-            ServiceProvider.AddService(typeof(T), s => factory(s));
+            ServiceProvider.AddService(typeof(T), s => factory(s)!);
         }
 
         internal bool TryGetValueSource(
             IValueDescriptor valueDescriptor,
-            out IValueSource valueSource)
+            out IValueSource? valueSource)
         {
-            if (ServiceProvider.AvailableServiceTypes.Contains(valueDescriptor.Type))
+            if (ServiceProvider.AvailableServiceTypes.Contains(valueDescriptor.ValueType))
             {
                 valueSource = new ServiceProviderValueSource();
                 return true;
             }
 
-            valueSource = null;
+            valueSource = default;
             return false;
         }
 
         internal bool TryBindToScalarValue(
             IValueDescriptor valueDescriptor,
             IValueSource valueSource,
-            out BoundValue boundValue)
+            out BoundValue? boundValue)
         {
             if (valueSource.TryGetValue(valueDescriptor, this, out var value))
             {
-                if (value == null || valueDescriptor.Type.IsInstanceOfType(value))
+                if (value == null || valueDescriptor.ValueType.IsInstanceOfType(value))
                 {
                     boundValue = new BoundValue(value, valueDescriptor, valueSource);
                     return true;
@@ -101,7 +103,7 @@ internal bool TryBindToScalarValue(
                 {
                     var parsed = ArgumentConverter.ConvertObject(
                         valueDescriptor as IArgument ?? new Argument(valueDescriptor.ValueName), 
-                        valueDescriptor.Type, 
+                        valueDescriptor.ValueType, 
                         value);
 
                     if (parsed is SuccessfulArgumentConversionResult successful)
@@ -112,7 +114,7 @@ internal bool TryBindToScalarValue(
                 }
             }
 
-            boundValue = null;
+            boundValue = default;
             return false;
         }
     }

diff --git a/src/System.CommandLine/Binding/BoundValue.cs b/src/System.CommandLine/Binding/BoundValue.cs
@@ -11,9 +11,9 @@ internal BoundValue(
             IValueSource valueSource)
         {
             if (value != null &&
-                !valueDescriptor.Type.IsInstanceOfType(value))
+                !valueDescriptor.ValueType.IsInstanceOfType(value))
             {
-                throw new ArgumentException($"Value {value} ({value.GetType()}) must be an instance of type {valueDescriptor.Type}");
+                throw new ArgumentException($"Value {value} ({value.GetType()}) must be an instance of type {valueDescriptor.ValueType}");
             }
 
             Value = value;

diff --git a/src/System.CommandLine/Binding/DelegateHandlerDescriptor.cs b/src/System.CommandLine/Binding/DelegateHandlerDescriptor.cs
@@ -18,13 +18,9 @@ public DelegateHandlerDescriptor(Delegate handlerDelegate)
 
         public override ICommandHandler GetCommandHandler()
         {
-            var parameterBinders = ParameterDescriptors
-                                   .Select(p => new ModelBinder(p))
-                                   .ToList();
-
             return new ModelBindingCommandHandler(
                 _handlerDelegate,
-                parameterBinders);
+                ParameterDescriptors);
         }
 
         public override ModelDescriptor Parent => null;

diff --git a/src/System.CommandLine/Binding/ExpressionHandlerDescriptor.cs b/src/System.CommandLine/Binding/ExpressionHandlerDescriptor.cs