From ba783e442b31cd3ab8ee4ddc922f2c68de864cc7 Mon Sep 17 00:00:00 2001
From: Jon Sequeira <jonsequeira@gmail.com>
Date: Sun, 12 Apr 2020 11:08:55 -0700
Subject: [PATCH] enable easier custom model binding

---
 .../ConfigureFromMethodTests.cs               |  2 +-
 .../CommandLine.cs                            |  4 +-
 .../Binding/HandlerDescriptorTests.cs         | 29 +-------
 .../Binding/ModelBinderTests.cs               | 70 +++++++++++++++++--
 .../Binding/TestModels.cs                     |  1 -
 src/System.CommandLine/Argument.cs            |  2 +-
 .../Binding/BindingContext.cs                 | 42 +++++------
 src/System.CommandLine/Binding/BoundValue.cs  |  4 +-
 .../Binding/DelegateHandlerDescriptor.cs      |  6 +-
 .../Binding/ExpressionHandlerDescriptor.cs    | 60 ----------------
 .../Binding/HandlerDescriptor.cs              | 11 +--
 .../Binding/IValueDescriptor.cs               |  2 +-
 .../Binding/MethodInfoHandlerDescriptor.cs    | 18 ++---
 src/System.CommandLine/Binding/ModelBinder.cs | 30 ++++----
 .../Binding/ModelDescriptor.cs                | 40 +++--------
 .../Binding/ParameterDescriptor.cs            | 10 +--
 .../Binding/PropertyDescriptor.cs             |  6 +-
 .../Binding/ServiceProviderValueSource.cs     |  2 +-
 .../Binding/TypeDefaultValueSource.cs         |  2 +-
 src/System.CommandLine/Help/HelpBuilder.cs    |  2 +-
 .../Invocation/ModelBindingCommandHandler.cs  | 30 ++++----
 .../Invocation/ServiceProvider.cs             | 18 +++--
 src/System.CommandLine/Option.cs              |  2 +-
 .../Parsing/CommandResultExtensions.cs        |  2 +-
 .../Parsing/ParseOperation.cs                 |  4 +-
 .../Parsing/ParseResultVisitor.cs             |  2 +-
 26 files changed, 170 insertions(+), 231 deletions(-)
 delete mode 100644 src/System.CommandLine/Binding/ExpressionHandlerDescriptor.cs

diff --git a/src/System.CommandLine.DragonFruit.Tests/ConfigureFromMethodTests.cs b/src/System.CommandLine.DragonFruit.Tests/ConfigureFromMethodTests.cs
index 1fa2b13390..0a9db21aba 100644
--- 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
index 380362c94c..63e7bd7f0a 100644
--- 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
index e83094bfd6..15577ff333 100644
--- 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
index bbe56afc62..d5ad82b2b4 100644
--- 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,8 +549,10 @@ 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;
 
@@ -554,5 +560,61 @@ public void Explicit_model_binder_binds_only_to_configured_ctor_parameters()
             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
index 6315f199e0..7e9008287d 100644
--- 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
index 530c983184..3e1a16ca4d 100644
--- 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
index cd3cd81218..850712d4d5 100644
--- 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
index 862df554b4..aeec2a4b8b 100644
--- 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
index ce8ac439e6..a77347dbd5 100644
--- 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
deleted file mode 100644
index c16bb9c2fc..0000000000
--- a/src/System.CommandLine/Binding/ExpressionHandlerDescriptor.cs
+++ /dev/null
@@ -1,60 +0,0 @@
-// 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.Invocation;
-using System.Linq.Expressions;
-
-namespace System.CommandLine.Binding
-{
-    internal class ExpressionHandlerDescriptor : HandlerDescriptor
-    {
-        private readonly LambdaExpression _handlerExpression;
-
-        public ExpressionHandlerDescriptor(LambdaExpression handlerExpression)
-        {
-            _handlerExpression = handlerExpression;
-        }
-
-        public override ICommandHandler GetCommandHandler()
-        {
-            // TODO: (GetCommandHandler) 
-            return null;
-        }
-
-        public override ModelDescriptor Parent
-        {
-            get
-            {
-                switch (_handlerExpression.Body)
-                {
-                    case MethodCallExpression methodCall:
-                        var objectType = methodCall?.Object?.Type;
-                        return objectType != null
-                                   ? ModelDescriptor.FromType(objectType)
-                                   : null;
-
-                    default:
-                        return null;
-                }
-            }
-        }
-
-        protected override IEnumerable<ParameterDescriptor> InitializeParameterDescriptors()
-        {
-            switch (_handlerExpression.Body)
-            {
-                case MethodCallExpression methodCall:
-                    foreach (var p in methodCall.Method.GetParameters())
-                    {
-                        yield return new ParameterDescriptor(p, this);
-                    }
-
-                    yield break;
-
-                default:
-                    throw new NotSupportedException($"Unsupported expression: {_handlerExpression}");
-            }
-        }
-    }
-}
diff --git a/src/System.CommandLine/Binding/HandlerDescriptor.cs b/src/System.CommandLine/Binding/HandlerDescriptor.cs
index 590e9e7973..ba51a5ffc6 100644
--- a/src/System.CommandLine/Binding/HandlerDescriptor.cs
+++ b/src/System.CommandLine/Binding/HandlerDescriptor.cs
@@ -4,7 +4,6 @@
 using System.Collections.Generic;
 using System.CommandLine.Invocation;
 using System.Linq;
-using System.Linq.Expressions;
 using System.Reflection;
 
 namespace System.CommandLine.Binding
@@ -18,7 +17,7 @@ public abstract class HandlerDescriptor : IMethodDescriptor
         public abstract ModelDescriptor Parent { get; }
 
         public IReadOnlyList<ParameterDescriptor> ParameterDescriptors =>
-            _parameterDescriptors ?? (_parameterDescriptors = new List<ParameterDescriptor>(InitializeParameterDescriptors()));
+            _parameterDescriptors ??= new List<ParameterDescriptor>(InitializeParameterDescriptors());
 
         protected abstract IEnumerable<ParameterDescriptor> InitializeParameterDescriptors();
 
@@ -30,13 +29,5 @@ public static HandlerDescriptor FromMethodInfo(MethodInfo methodInfo, object tar
 
         public static HandlerDescriptor FromDelegate(Delegate @delegate) =>
             new DelegateHandlerDescriptor(@delegate);
-
-        public static HandlerDescriptor FromExpression<TModel>(Expression<Action<TModel>> handle) => new ExpressionHandlerDescriptor(handle);
-
-        public static HandlerDescriptor FromExpression<TModel, T>(Expression<Action<TModel, T>> handle) => new ExpressionHandlerDescriptor(handle);
-
-        public static HandlerDescriptor FromExpression<TModel, T1, T2>(Expression<Action<TModel, T1, T2>> handle) => new ExpressionHandlerDescriptor(handle);
-
-        public static HandlerDescriptor FromExpression<TModel, T1, T2, TReturn>(Expression<Func<TModel, T1, T2, TReturn>> handle) => new ExpressionHandlerDescriptor(handle);
     }
 }
diff --git a/src/System.CommandLine/Binding/IValueDescriptor.cs b/src/System.CommandLine/Binding/IValueDescriptor.cs
index 7b6092d338..a54d4deb4e 100644
--- a/src/System.CommandLine/Binding/IValueDescriptor.cs
+++ b/src/System.CommandLine/Binding/IValueDescriptor.cs
@@ -7,7 +7,7 @@ public interface IValueDescriptor
     {
         string ValueName { get; }
 
-        Type Type { get; }
+        Type ValueType { get; }
 
         bool HasDefaultValue { get; }
 
diff --git a/src/System.CommandLine/Binding/MethodInfoHandlerDescriptor.cs b/src/System.CommandLine/Binding/MethodInfoHandlerDescriptor.cs
index 3ab46a3ed2..1ab64c80da 100644
--- a/src/System.CommandLine/Binding/MethodInfoHandlerDescriptor.cs
+++ b/src/System.CommandLine/Binding/MethodInfoHandlerDescriptor.cs
@@ -14,7 +14,7 @@ internal class MethodInfoHandlerDescriptor : HandlerDescriptor
         private readonly object _invocationTarget;
 
         public MethodInfoHandlerDescriptor(
-            MethodInfo handlerMethodInfo, 
+            MethodInfo handlerMethodInfo,
             object target = null)
         {
             _handlerMethodInfo = handlerMethodInfo ??
@@ -24,27 +24,17 @@ public MethodInfoHandlerDescriptor(
 
         public override ICommandHandler GetCommandHandler()
         {
-            var parameterBinders = ParameterDescriptors
-                                   .Select(parameterDescriptor => new ModelBinder(parameterDescriptor))
-                                   .ToList();
-
             if (_invocationTarget == null)
             {
-                var invocationTargetBinder =
-                    _handlerMethodInfo.IsStatic
-                        ? null
-                        : new ModelBinder(_handlerMethodInfo.DeclaringType);
-
                 return new ModelBindingCommandHandler(
                     _handlerMethodInfo,
-                    parameterBinders,
-                    invocationTargetBinder);
+                    ParameterDescriptors);
             }
             else
             {
                 return new ModelBindingCommandHandler(
                     _handlerMethodInfo,
-                    parameterBinders,
+                    ParameterDescriptors,
                     _invocationTarget);
             }
         }
@@ -55,4 +45,4 @@ protected override IEnumerable<ParameterDescriptor> InitializeParameterDescripto
             _handlerMethodInfo.GetParameters()
                               .Select(p => new ParameterDescriptor(p, this));
     }
-}
+}
\ No newline at end of file
diff --git a/src/System.CommandLine/Binding/ModelBinder.cs b/src/System.CommandLine/Binding/ModelBinder.cs
index 41a59c618e..1f08a224f2 100644
--- a/src/System.CommandLine/Binding/ModelBinder.cs
+++ b/src/System.CommandLine/Binding/ModelBinder.cs
@@ -9,8 +9,6 @@ namespace System.CommandLine.Binding
 {
     public class ModelBinder
     {
-        private readonly ModelDescriptor _modelDescriptor;
-
         public ModelBinder(Type modelType) : this(new AnonymousValueDescriptor(modelType))
         {
             if (modelType == null)
@@ -23,9 +21,11 @@ internal ModelBinder(IValueDescriptor valueDescriptor)
         {
             ValueDescriptor = valueDescriptor ?? throw new ArgumentNullException(nameof(valueDescriptor));
 
-            _modelDescriptor = ModelDescriptor.FromType(valueDescriptor.Type);
+            ModelDescriptor = ModelDescriptor.FromType(valueDescriptor.ValueType);
         }
 
+        public ModelDescriptor ModelDescriptor { get; }
+
         public IValueDescriptor ValueDescriptor { get; }
 
         public bool EnforceExplicitBinding { get; set; }
@@ -41,12 +41,12 @@ protected ConstructorDescriptor FindModelConstructorDescriptor(
         {
             var cmpCtorDesc = new ConstructorDescriptor(constructorInfo,
                 // Parent does not matter for comparison and can be invalid.
-                parent: _modelDescriptor);
+                parent: ModelDescriptor);
             var cmpParamDescs = cmpCtorDesc.ParameterDescriptors
                 .Select(GetParameterDescriptorComparands)
                 .ToList();
 
-            return _modelDescriptor.ConstructorDescriptors
+            return ModelDescriptor.ConstructorDescriptors
                 .FirstOrDefault(matchCtorDesc =>
                 {
                     if (matchCtorDesc.Parent.ModelType != constructorInfo.DeclaringType)
@@ -59,15 +59,15 @@ protected ConstructorDescriptor FindModelConstructorDescriptor(
             // Name matching is not necessary for overload descisions.
             static (Type paramType, bool allowNull, bool hasDefaultValue)
                 GetParameterDescriptorComparands(ParameterDescriptor desc) =>
-                (desc.Type, desc.AllowsNull, desc.HasDefaultValue);
+                (desc.ValueType, desc.AllowsNull, desc.HasDefaultValue);
         }
 
         protected IValueDescriptor FindModelPropertyDescriptor(
             Type propertyType, string propertyName)
         {
-            return _modelDescriptor.PropertyDescriptors
+            return ModelDescriptor.PropertyDescriptors
                 .FirstOrDefault(desc =>
-                    desc.Type == propertyType &&
+                    desc.ValueType == propertyType &&
                     string.Equals(desc.ValueName, propertyName, StringComparison.Ordinal)
                     );
         }
@@ -113,7 +113,7 @@ public object CreateInstance(BindingContext context)
                 includeMissingValues: false);
 
             if (values.Count == 1 &&
-                _modelDescriptor.ModelType.IsAssignableFrom(values[0].ValueDescriptor.Type))
+                ModelDescriptor.ModelType.IsAssignableFrom(values[0].ValueDescriptor.ValueType))
             {
                 return values[0].Value;
             }
@@ -131,7 +131,7 @@ private bool TryDefaultConstructorAndPropertiesStrategy(
             out object instance)
         {
             var constructorDescriptors =
-                _modelDescriptor
+                ModelDescriptor
                     .ConstructorDescriptors
                     .OrderByDescending(d => d.ParameterDescriptors.Count);
 
@@ -177,7 +177,7 @@ public void UpdateInstance<T>(T instance, BindingContext bindingContext)
             var boundValues = GetValues(
                 MemberBindingSources,
                 bindingContext,
-                _modelDescriptor.PropertyDescriptors,
+                ModelDescriptor.PropertyDescriptors,
                 includeMissingValues: false);
 
             foreach (var boundValue in boundValues)
@@ -266,7 +266,7 @@ private IValueSource GetValueSource(
         }
 
         public override string ToString() =>
-            $"{_modelDescriptor.ModelType.Name}";
+            $"{ModelDescriptor.ModelType.Name}";
 
         private bool ShouldPassNullToConstructor(ModelDescriptor modelDescriptor,
             ConstructorDescriptor ctor = null)
@@ -281,11 +281,11 @@ private bool ShouldPassNullToConstructor(ModelDescriptor modelDescriptor,
 
         private class AnonymousValueDescriptor : IValueDescriptor
         {
-            public Type Type { get; }
+            public Type ValueType { get; }
 
             public AnonymousValueDescriptor(Type modelType)
             {
-                Type = modelType;
+                ValueType = modelType;
             }
 
             public string ValueName => null;
@@ -294,7 +294,7 @@ public AnonymousValueDescriptor(Type modelType)
 
             public object GetDefaultValue() => null;
 
-            public override string ToString() => $"{Type}";
+            public override string ToString() => $"{ValueType}";
         }
     }
 }
diff --git a/src/System.CommandLine/Binding/ModelDescriptor.cs b/src/System.CommandLine/Binding/ModelDescriptor.cs
index fbd6e89722..66838c9d73 100644
--- a/src/System.CommandLine/Binding/ModelDescriptor.cs
+++ b/src/System.CommandLine/Binding/ModelDescriptor.cs
@@ -19,7 +19,6 @@ public class ModelDescriptor
 
         private List<PropertyDescriptor> _propertyDescriptors;
         private List<ConstructorDescriptor> _constructorDescriptors;
-        private ConstructorDescriptor _targetConstructor;
 
         protected ModelDescriptor(Type modelType)
         {
@@ -28,36 +27,17 @@ protected ModelDescriptor(Type modelType)
         }
 
         public IReadOnlyList<ConstructorDescriptor> ConstructorDescriptors =>
-            _constructorDescriptors
-            ??
-            (_constructorDescriptors =
-                 ModelType.GetConstructors(CommonBindingFlags)
-                          .Select(i => new ConstructorDescriptor(i, this))
-                          .ToList());
-
-        public ConstructorDescriptor TargetConstructor
-        {
-            get
-            {
-                if (_targetConstructor == null && 
-                    ConstructorDescriptors.Count == 1)
-                {
-                    _targetConstructor = ConstructorDescriptors[0];
-                }
-
-
-                return _targetConstructor;
-            }
-        }
+            _constructorDescriptors ??=
+                ModelType.GetConstructors(CommonBindingFlags)
+                         .Select(i => new ConstructorDescriptor(i, this))
+                         .ToList();
 
         public IReadOnlyList<IValueDescriptor> PropertyDescriptors =>
-            _propertyDescriptors
-            ??
-            (_propertyDescriptors =
-                 ModelType.GetProperties(CommonBindingFlags)
-                          .Where(p => p.CanWrite)
-                          .Select(i => new PropertyDescriptor(i, this))
-                          .ToList());
+            _propertyDescriptors ??=
+                ModelType.GetProperties(CommonBindingFlags)
+                         .Where(p => p.CanWrite)
+                         .Select(i => new PropertyDescriptor(i, this))
+                         .ToList();
 
         public Type ModelType { get; }
 
@@ -73,4 +53,4 @@ public static ModelDescriptor FromType(Type type) =>
                 type,
                 _ => new ModelDescriptor(type));
     }
-}
+}
\ No newline at end of file
diff --git a/src/System.CommandLine/Binding/ParameterDescriptor.cs b/src/System.CommandLine/Binding/ParameterDescriptor.cs
index e248416d9f..9df99aeaac 100644
--- a/src/System.CommandLine/Binding/ParameterDescriptor.cs
+++ b/src/System.CommandLine/Binding/ParameterDescriptor.cs
@@ -22,7 +22,7 @@ internal ParameterDescriptor(
 
         public IMethodDescriptor Parent { get; }
 
-        public Type Type => _parameterInfo.ParameterType;
+        public Type ValueType => _parameterInfo.ParameterType;
 
         public bool HasDefaultValue => _parameterInfo.HasDefaultValue;
 
@@ -37,7 +37,7 @@ public bool AllowsNull
                         _allowsNull = true;
                     }
 
-                    if (_parameterInfo.HasDefaultValue && 
+                    if (_parameterInfo.HasDefaultValue &&
                         _parameterInfo.DefaultValue == null)
                     {
                         _allowsNull = true;
@@ -50,9 +50,9 @@ public bool AllowsNull
 
         public object GetDefaultValue() =>
             _parameterInfo.DefaultValue is DBNull
-                ? Type.GetDefaultValueForType()
+                ? ValueType.GetDefaultValueForType()
                 : _parameterInfo.DefaultValue;
 
-        public override string ToString() => $"{Type.Name} {ValueName}";
+        public override string ToString() => $"{ValueType.Name} {ValueName}";
     }
-}
+}
\ No newline at end of file
diff --git a/src/System.CommandLine/Binding/PropertyDescriptor.cs b/src/System.CommandLine/Binding/PropertyDescriptor.cs
index 34dd5d87c7..6e9124b1ee 100644
--- a/src/System.CommandLine/Binding/PropertyDescriptor.cs
+++ b/src/System.CommandLine/Binding/PropertyDescriptor.cs
@@ -25,17 +25,17 @@ internal PropertyDescriptor(
                                     ? Parent + "." + ValueName
                                     : ValueName;
 
-        public Type Type => _propertyInfo.PropertyType;
+        public Type ValueType => _propertyInfo.PropertyType;
 
         public bool HasDefaultValue => false;
 
-        public object GetDefaultValue() => Type.GetDefaultValueForType();
+        public object GetDefaultValue() => ValueType.GetDefaultValueForType();
 
         public void SetValue(object instance, object value)
         {
             _propertyInfo.SetValue(instance, value);
         }
 
-        public override string ToString() => $"{Type.Name} {Path}";
+        public override string ToString() => $"{ValueType.Name} {Path}";
     }
 }
diff --git a/src/System.CommandLine/Binding/ServiceProviderValueSource.cs b/src/System.CommandLine/Binding/ServiceProviderValueSource.cs
index 8d2e0515d4..fc2f034a91 100644
--- a/src/System.CommandLine/Binding/ServiceProviderValueSource.cs
+++ b/src/System.CommandLine/Binding/ServiceProviderValueSource.cs
@@ -10,7 +10,7 @@ public bool TryGetValue(
             BindingContext bindingContext,
             out object boundValue)
         {
-            boundValue = bindingContext.ServiceProvider.GetService(valueDescriptor.Type);
+            boundValue = bindingContext.ServiceProvider.GetService(valueDescriptor.ValueType);
             return true;
         }
     }
diff --git a/src/System.CommandLine/Binding/TypeDefaultValueSource.cs b/src/System.CommandLine/Binding/TypeDefaultValueSource.cs
index dc6f4d891b..bca1ed0140 100644
--- a/src/System.CommandLine/Binding/TypeDefaultValueSource.cs
+++ b/src/System.CommandLine/Binding/TypeDefaultValueSource.cs
@@ -16,7 +16,7 @@ public bool TryGetValue(
             BindingContext bindingContext,
             out object boundValue)
         {
-            boundValue = valueDescriptor.Type.GetDefaultValueForType();
+            boundValue = valueDescriptor.ValueType.GetDefaultValueForType();
             return true;
         }
     }
diff --git a/src/System.CommandLine/Help/HelpBuilder.cs b/src/System.CommandLine/Help/HelpBuilder.cs
index 2aa174ff3f..d4f9e783a6 100644
--- a/src/System.CommandLine/Help/HelpBuilder.cs
+++ b/src/System.CommandLine/Help/HelpBuilder.cs
@@ -311,7 +311,7 @@ private IEnumerable<HelpItem> GetArgumentHelpItems(ISymbol symbol)
 
         protected virtual string ArgumentDescriptor(IArgument argument)
         {
-            if (argument.Type == typeof(bool) || argument.Type == typeof(bool?) )
+            if (argument.ValueType == typeof(bool) || argument.ValueType == typeof(bool?) )
             {
                 return "";
             }
diff --git a/src/System.CommandLine/Invocation/ModelBindingCommandHandler.cs b/src/System.CommandLine/Invocation/ModelBindingCommandHandler.cs
index 2e9ed3a4aa..ac8542746b 100644
--- a/src/System.CommandLine/Invocation/ModelBindingCommandHandler.cs
+++ b/src/System.CommandLine/Invocation/ModelBindingCommandHandler.cs
@@ -15,46 +15,52 @@ internal class ModelBindingCommandHandler : ICommandHandler
         private readonly object _invocationTarget;
         private readonly ModelBinder _invocationTargetBinder;
         private readonly MethodInfo _handlerMethodInfo;
-        private readonly IReadOnlyCollection<ModelBinder> _parameterBinders;
+        private readonly IReadOnlyList<ParameterDescriptor> _parameterDescriptors;
 
         public ModelBindingCommandHandler(
             MethodInfo handlerMethodInfo,
-            IReadOnlyCollection<ModelBinder> parameterBinders,
-            ModelBinder invocationTargetBinder = null)
+            IReadOnlyList<ParameterDescriptor> parameterDescriptors)
         {
-            _invocationTargetBinder = invocationTargetBinder;
             _handlerMethodInfo = handlerMethodInfo;
-            _parameterBinders = parameterBinders;
+            _invocationTargetBinder = _handlerMethodInfo.IsStatic
+                                          ? null
+                                          : new ModelBinder(_handlerMethodInfo.DeclaringType);
+            _parameterDescriptors = parameterDescriptors;
         }
 
         public ModelBindingCommandHandler(
             MethodInfo handlerMethodInfo,
-            IReadOnlyCollection<ModelBinder> parameterBinders,
+            IReadOnlyList<ParameterDescriptor> parameterDescriptors,
             object invocationTarget)
         {
             _invocationTarget = invocationTarget;
             _handlerMethodInfo = handlerMethodInfo;
-            _parameterBinders = parameterBinders;
+            _parameterDescriptors = parameterDescriptors;
         }
 
         public ModelBindingCommandHandler(
             Delegate handlerDelegate,
-            IReadOnlyCollection<ModelBinder> parameterBinders)
+            IReadOnlyList<ParameterDescriptor> parameterDescriptors)
         {
             _handlerDelegate = handlerDelegate;
-            _parameterBinders = parameterBinders;
+            _parameterDescriptors = parameterDescriptors;
         }
 
         public async Task<int> InvokeAsync(InvocationContext context)
         {
             var bindingContext = context.BindingContext;
 
+            var parameterBinders = _parameterDescriptors
+                                   .Select(p => bindingContext.GetModelBinder(p))
+                                   .ToList();
+
             var invocationArguments =
-                _parameterBinders.Select(p => p.CreateInstance(bindingContext))
+                parameterBinders
+                    .Select(binder => binder.CreateInstance(bindingContext))
                     .ToArray();
 
             var invocationTarget = _invocationTarget ??
-                _invocationTargetBinder?.CreateInstance(bindingContext);
+                                   _invocationTargetBinder?.CreateInstance(bindingContext);
 
             object result;
             if (_handlerDelegate == null)
@@ -71,4 +77,4 @@ public async Task<int> InvokeAsync(InvocationContext context)
             return await CommandHandler.GetResultCodeAsync(result, context);
         }
     }
-}
+}
\ No newline at end of file
diff --git a/src/System.CommandLine/Invocation/ServiceProvider.cs b/src/System.CommandLine/Invocation/ServiceProvider.cs
index ab1136b23d..86970c5a76 100644
--- a/src/System.CommandLine/Invocation/ServiceProvider.cs
+++ b/src/System.CommandLine/Invocation/ServiceProvider.cs
@@ -7,29 +7,27 @@
 using System.CommandLine.Parsing;
 using System.Threading;
 
+#nullable enable
+
 namespace System.CommandLine.Invocation
 {
     internal class ServiceProvider : IServiceProvider
     {
-        private readonly BindingContext _bindingContext;
-
         private readonly Dictionary<Type, Func<IServiceProvider, object>> _services;
 
         public ServiceProvider(BindingContext bindingContext)
         {
-            _bindingContext = bindingContext ?? throw new ArgumentNullException(nameof(bindingContext));
-
             _services = new Dictionary<Type, Func<IServiceProvider, object>>
                         {
-                            [typeof(ParseResult)] = _ => _bindingContext.ParseResult,
-                            [typeof(IConsole)] = _ => _bindingContext.Console,
+                            [typeof(ParseResult)] = _ => bindingContext.ParseResult,
+                            [typeof(IConsole)] = _ => bindingContext.Console,
                             [typeof(CancellationToken)] = _ => CancellationToken.None,
-                            [typeof(IHelpBuilder)] = _ => _bindingContext.ParseResult.Parser.Configuration.HelpBuilderFactory(_bindingContext),
-                            [typeof(BindingContext)] = _ => _bindingContext
+                            [typeof(IHelpBuilder)] = _ => bindingContext.ParseResult.Parser.Configuration.HelpBuilderFactory(bindingContext),
+                            [typeof(BindingContext)] = _ => bindingContext
                         };
         }
 
-        public void AddService<T>(Func<IServiceProvider, T> factory) => _services[typeof(T)] = p => factory(p);
+        public void AddService<T>(Func<IServiceProvider, T> factory) => _services[typeof(T)] = p => factory(p)!;
 
         public void AddService(Type serviceType, Func<IServiceProvider, object> factory) => _services[serviceType] = factory;
 
@@ -42,7 +40,7 @@ public object GetService(Type serviceType)
                 return factory(this);
             }
 
-            return null;
+            return null!;
         }
     }
 }
diff --git a/src/System.CommandLine/Option.cs b/src/System.CommandLine/Option.cs
index 6189310bf4..4cd0fce9e1 100644
--- a/src/System.CommandLine/Option.cs
+++ b/src/System.CommandLine/Option.cs
@@ -48,7 +48,7 @@ public virtual Argument Argument
 
         string IValueDescriptor.ValueName => Name;
 
-        Type IValueDescriptor.Type => Argument.ArgumentType;
+        Type IValueDescriptor.ValueType => Argument.ArgumentType;
 
         bool IValueDescriptor.HasDefaultValue => Argument.HasDefaultValue;
 
diff --git a/src/System.CommandLine/Parsing/CommandResultExtensions.cs b/src/System.CommandLine/Parsing/CommandResultExtensions.cs
index ad52b843fd..8933f9a4dd 100644
--- a/src/System.CommandLine/Parsing/CommandResultExtensions.cs
+++ b/src/System.CommandLine/Parsing/CommandResultExtensions.cs
@@ -50,7 +50,7 @@ internal static bool TryGetValueForOption(
 
             if (symbolResult is OptionResult optionResult)
             {
-                if (optionResult.ConvertIfNeeded(valueDescriptor.Type) is SuccessfulArgumentConversionResult successful)
+                if (optionResult.ConvertIfNeeded(valueDescriptor.ValueType) is SuccessfulArgumentConversionResult successful)
                 {
                     value = successful.Value;
                     return true;
diff --git a/src/System.CommandLine/Parsing/ParseOperation.cs b/src/System.CommandLine/Parsing/ParseOperation.cs
index 77879fde02..04eebeb3c2 100644
--- a/src/System.CommandLine/Parsing/ParseOperation.cs
+++ b/src/System.CommandLine/Parsing/ParseOperation.cs
@@ -207,11 +207,11 @@ private void ParseOptionArguments(OptionNode optionNode)
                         return;
                     }
                 }
-                else if (argument.Type == typeof(bool))
+                else if (argument.ValueType == typeof(bool))
                 {
                     if (ArgumentConverter.ConvertObject(
                             argument,
-                            argument.Type,
+                            argument.ValueType,
                             CurrentToken.Value) is FailedArgumentTypeConversionResult)
                     {
                         return;
diff --git a/src/System.CommandLine/Parsing/ParseResultVisitor.cs b/src/System.CommandLine/Parsing/ParseResultVisitor.cs
index 72b98d0e73..0ce30dc196 100644
--- a/src/System.CommandLine/Parsing/ParseResultVisitor.cs
+++ b/src/System.CommandLine/Parsing/ParseResultVisitor.cs
@@ -353,7 +353,7 @@ private void PopulateDefaultValues()
                     }
 
                     if (symbolResult is OptionResult o &&
-                        o.Option.Argument.Type == typeof(bool) &&
+                        o.Option.Argument.ValueType == typeof(bool) &&
                         o.Children.Count == 0)
                     {
                         o.Children.Add(