Improve documentation of `BinderType` and `BindingSource` properties (#7218)

- add regression test for #4939
- add `[BindProperty]` doc comments
- add `<remarks>` to `BinderType` properties that recommend setting `BindingSource` in some cases

smaller issues:
- catch invalid `BinderType` values up front
- complete `BindingSource.ModelBinding` implementation: `IValueProvider` filtering was faulty

nits:
- accept VS suggestions e.g. remove unused variables
- "model binder" -> `<see cref="IModelBinder" /> implementation` in some doc comments

src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingInfo.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using Microsoft.AspNetCore.Mvc.Abstractions;
 
 namespace Microsoft.AspNetCore.Mvc.ModelBinding
 {
@@ -12,6 +13,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
     /// </summary>
     public class BindingInfo
     {
+        private Type _binderType;
+
         /// <summary>
         /// Creates a new <see cref="BindingInfo"/>.
         /// </summary>
@@ -48,9 +51,30 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
         public string BinderModelName { get; set; }
 
         /// <summary>
-        /// Gets or sets the <see cref="Type"/> of the model binder used to bind the model.
+        /// Gets or sets the <see cref="Type"/> of the <see cref="IModelBinder"/> implementation used to bind the
+        /// model.
         /// </summary>
-        public Type BinderType { get; set; }
+        /// <remarks>
+        /// Also set <see cref="BindingSource"/> if the specified <see cref="IModelBinder"/> implementation does not
+        /// use values from form data, route values or the query string.
+        /// </remarks>
+        public Type BinderType
+        {
+            get => _binderType;
+            set
+            {
+                if (value != null && !typeof(IModelBinder).IsAssignableFrom(value))
+                {
+                    throw new ArgumentException(
+                        Resources.FormatBinderType_MustBeIModelBinder(
+                            value.FullName,
+                            typeof(IModelBinder).FullName),
+                        nameof(value));
+                }
+
+                _binderType = value;
+            }
+        }
 
         /// <summary>
         /// Gets or sets the <see cref="ModelBinding.IPropertyFilterProvider"/>.
@@ -246,4 +270,4 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
             }
         }
     }
-}
+}

src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingSource.cs

@@ -144,7 +144,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
         /// <remarks>
         /// <para>
         /// For sources based on a <see cref="IValueProvider"/>, setting <see cref="IsGreedy"/> to <c>false</c>
-        /// will most closely describe the behavior. This value is used inside the default model binders to 
+        /// will most closely describe the behavior. This value is used inside the default model binders to
         /// determine whether or not to attempt to bind properties of a model.
         /// </para>
         /// <para>
@@ -177,7 +177,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
         /// When using this method, it is expected that the left-hand-side is metadata specified
         /// on a property or parameter for model binding, and the right hand side is a source of
         /// data used by a model binder or value provider.
-        /// 
+        ///
         /// This distinction is important as the left-hand-side may be a composite, but the right
         /// may not.
         /// </remarks>
@@ -196,7 +196,17 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
                 throw new ArgumentException(message, nameof(bindingSource));
             }
 
-            return this == bindingSource;
+            if (this == bindingSource)
+            {
+                return true;
+            }
+
+            if (this == ModelBinding)
+            {
+                return bindingSource == Form || bindingSource == Path || bindingSource == Query;
+            }
+
+            return false;
         }
 
         /// <inheritdoc />
@@ -220,9 +230,9 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
         /// <inheritdoc />
         public static bool operator ==(BindingSource s1, BindingSource s2)
         {
-            if (object.ReferenceEquals(s1, null))
+            if (s1 is null)
             {
-                return object.ReferenceEquals(s2, null);
+                return s2 is null;
             }
 
             return s1.Equals(s2);
@@ -234,4 +244,4 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
             return !(s1 == s2);
         }
     }
-}
+}

src/Mvc/Mvc.Abstractions/src/ModelBinding/IModelBinder.cs

@@ -24,7 +24,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
         /// </para>
         /// <para>
         /// A model binder that completes successfully should set <see cref="ModelBindingContext.Result"/> to
-        /// a value returned from <see cref="ModelBindingResult.Success"/>. 
+        /// a value returned from <see cref="ModelBindingResult.Success"/>.
         /// </para>
         /// </returns>
         Task BindModelAsync(ModelBindingContext bindingContext);

src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelBindingContext.cs

@@ -123,8 +123,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
         public abstract ModelBindingResult Result { get; set; }
 
         /// <summary>
-        /// Pushes a layer of state onto this context. Model binders will call this as part of recursion when binding
+        /// Pushes a layer of state onto this context. <see cref="IModelBinder"/> implementations will call this as
-        /// properties or collection items.
+        /// part of recursion when binding properties or collection items.
         /// </summary>
         /// <param name="modelMetadata">
         /// <see cref="ModelBinding.ModelMetadata"/> to assign to the <see cref="ModelMetadata"/> property.
@@ -143,8 +143,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
             object model);
 
         /// <summary>
-        /// Pushes a layer of state onto this context. Model binders will call this as part of recursion when binding
+        /// Pushes a layer of state onto this context. <see cref="IModelBinder"/> implementations will call this as
-        /// properties or collection items.
+        /// part of recursion when binding properties or collection items.
         /// </summary>
         /// <returns>
         /// A <see cref="NestedScope"/> scope object which should be used in a <c>using</c> statement where

src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadata.cs

@@ -326,7 +326,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
         public abstract bool ValidateChildren { get; }
 
         /// <summary>
-        /// Gets a value that indicates if the model, or one of it's properties, or elements has associatated validators.
+        /// Gets a value that indicates if the model, or one of it's properties, or elements has associated validators.
         /// </summary>
         /// <remarks>
         /// When <see langword="false"/>, validation can be assume that the model is valid (<see cref="ModelValidationState.Valid"/>) without

src/Mvc/Mvc.Abstractions/src/Properties/Resources.Designer.cs

@@ -276,6 +276,20 @@ namespace Microsoft.AspNetCore.Mvc.Abstractions
         internal static string FormatBindingSource_FormFile()
             => GetString("BindingSource_FormFile");
 
+        /// <summary>
+        /// The type '{0}' must implement '{1}' to be used as a model binder.
+        /// </summary>
+        internal static string BinderType_MustBeIModelBinder
+        {
+            get => GetString("BinderType_MustBeIModelBinder");
+        }
+
+        /// <summary>
+        /// The type '{0}' must implement '{1}' to be used as a model binder.
+        /// </summary>
+        internal static string FormatBinderType_MustBeIModelBinder(object p0, object p1)
+            => string.Format(CultureInfo.CurrentCulture, GetString("BinderType_MustBeIModelBinder"), p0, p1);
+
         private static string GetString(string name, params string[] formatterNames)
         {
             var value = _resourceManager.GetString(name);

src/Mvc/Mvc.Abstractions/src/Resources.resx

@@ -174,4 +174,7 @@
   <data name="BindingSource_FormFile" xml:space="preserve">
     <value>FormFile</value>
   </data>
+  <data name="BinderType_MustBeIModelBinder" xml:space="preserve">
+    <value>The type '{0}' must implement '{1}' to be used as a model binder.</value>
+  </data>
 </root>

src/Mvc/Mvc.Abstractions/test/ModelBinding/BindingInfoTest.cs

@@ -2,7 +2,7 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
-using System.Linq;
+using Microsoft.AspNetCore.Mvc.ModelBinding.Binders;
 using Moq;
 using Xunit;
 
@@ -83,14 +83,14 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
             // Arrange
             var attributes = new object[]
             {
-                new ModelBinderAttribute { BinderType = typeof(object), Name = "Test" },
+                new ModelBinderAttribute { BinderType = typeof(ComplexTypeModelBinder), Name = "Test" },
             };
             var modelType = typeof(Guid);
             var provider = new TestModelMetadataProvider();
             provider.ForType(modelType).BindingDetails(metadata =>
             {
                 metadata.BindingSource = BindingSource.Special;
-                metadata.BinderType = typeof(string);
+                metadata.BinderType = typeof(SimpleTypeModelBinder);
                 metadata.BinderModelName = "Different";
             });
             var modelMetadata = provider.GetMetadataForType(modelType);
@@ -100,7 +100,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
 
             // Assert
             Assert.NotNull(bindingInfo);
-            Assert.Same(typeof(object), bindingInfo.BinderType);
+            Assert.Same(typeof(ComplexTypeModelBinder), bindingInfo.BinderType);
             Assert.Same("Test", bindingInfo.BinderModelName);
         }
 
@@ -108,13 +108,18 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
         public void GetBindingInfo_WithAttributesAndModelMetadata_UsesBinderNameFromModelMetadata_WhenNotFoundViaAttributes()
         {
             // Arrange
-            var attributes = new object[] { new ModelBinderAttribute(typeof(object)), new ControllerAttribute(), new BindNeverAttribute(), };
+            var attributes = new object[]
+            {
+                new ModelBinderAttribute(typeof(ComplexTypeModelBinder)),
+                new ControllerAttribute(),
+                new BindNeverAttribute(),
+            };
             var modelType = typeof(Guid);
             var provider = new TestModelMetadataProvider();
             provider.ForType(modelType).BindingDetails(metadata =>
             {
                 metadata.BindingSource = BindingSource.Special;
-                metadata.BinderType = typeof(string);
+                metadata.BinderType = typeof(SimpleTypeModelBinder);
                 metadata.BinderModelName = "Different";
             });
             var modelMetadata = provider.GetMetadataForType(modelType);
@@ -124,7 +129,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
 
             // Assert
             Assert.NotNull(bindingInfo);
-            Assert.Same(typeof(object), bindingInfo.BinderType);
+            Assert.Same(typeof(ComplexTypeModelBinder), bindingInfo.BinderType);
             Assert.Same("Different", bindingInfo.BinderModelName);
             Assert.Same(BindingSource.Custom, bindingInfo.BindingSource);
         }
@@ -138,7 +143,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
             var provider = new TestModelMetadataProvider();
             provider.ForType(modelType).BindingDetails(metadata =>
             {
-                metadata.BinderType = typeof(string);
+                metadata.BinderType = typeof(ComplexTypeModelBinder);
             });
             var modelMetadata = provider.GetMetadataForType(modelType);
 
@@ -147,14 +152,19 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
 
             // Assert
             Assert.NotNull(bindingInfo);
-            Assert.Same(typeof(string), bindingInfo.BinderType);
+            Assert.Same(typeof(ComplexTypeModelBinder), bindingInfo.BinderType);
         }
 
         [Fact]
         public void GetBindingInfo_WithAttributesAndModelMetadata_UsesBinderSourceFromModelMetadata_WhenNotFoundViaAttributes()
         {
             // Arrange
-            var attributes = new object[] { new BindPropertyAttribute(), new ControllerAttribute(), new BindNeverAttribute(), };
+            var attributes = new object[]
+            {
+                new BindPropertyAttribute(),
+                new ControllerAttribute(),
+                new BindNeverAttribute(),
+            };
             var modelType = typeof(Guid);
             var provider = new TestModelMetadataProvider();
             provider.ForType(modelType).BindingDetails(metadata =>
@@ -175,7 +185,12 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
         public void GetBindingInfo_WithAttributesAndModelMetadata_UsesPropertyPredicateProviderFromModelMetadata_WhenNotFoundViaAttributes()
         {
             // Arrange
-            var attributes = new object[] { new ModelBinderAttribute(typeof(object)), new ControllerAttribute(), new BindNeverAttribute(), };
+            var attributes = new object[]
+            {
+                new ModelBinderAttribute(typeof(ComplexTypeModelBinder)),
+                new ControllerAttribute(),
+                new BindNeverAttribute(),
+            };
             var propertyFilterProvider = Mock.Of<IPropertyFilterProvider>();
             var modelType = typeof(Guid);
             var provider = new TestModelMetadataProvider();

src/Mvc/Mvc.Analyzers/test/TestFiles/TopLevelParameterNameAnalyzerTest/IsProblematicParameter_ReturnsFalse_ForParametersWithCustomModelBinder.cs

@@ -1,9 +1,12 @@
-namespace Microsoft.AspNetCore.Mvc.Analyzers.TopLevelParameterNameAnalyzerTestFiles
+using Microsoft.AspNetCore.Mvc.ModelBinding.Binders;
+
+namespace Microsoft.AspNetCore.Mvc.Analyzers.TopLevelParameterNameAnalyzerTestFiles
 {
     public class IsProblematicParameter_ReturnsFalse_ForParametersWithCustomModelBinder
     {
         public string Model { get; set; }
 
-        public void ActionMethod([ModelBinder(typeof(object))] IsProblematicParameter_ReturnsFalse_ForParametersWithCustomModelBinder model) { }
+        public void ActionMethod(
+            [ModelBinder(typeof(SimpleTypeModelBinder))] IsProblematicParameter_ReturnsFalse_ForParametersWithCustomModelBinder model) { }
     }
 }

src/Mvc/Mvc.Analyzers/test/TestFiles/TopLevelParameterNameAnalyzerTest/IsProblematicParameter_ReturnsTrue_IfPropertyWithModelBindingAttributeHasSameNameAsParameter.cs

@@ -1,10 +1,13 @@
-namespace Microsoft.AspNetCore.Mvc.Analyzers.TopLevelParameterNameAnalyzerTestFiles
+using Microsoft.AspNetCore.Mvc.ModelBinding.Binders;
+
+namespace Microsoft.AspNetCore.Mvc.Analyzers.TopLevelParameterNameAnalyzerTestFiles
 {
     public class IsProblematicParameter_ReturnsTrue_IfPropertyWithModelBindingAttributeHasSameNameAsParameter
     {
-        [ModelBinder(typeof(object), Name = "model")]
+        [ModelBinder(typeof(ComplexTypeModelBinder), Name = "model")]
         public string Different { get; set; }
 
-        public void ActionMethod(IsProblematicParameter_ReturnsTrue_IfPropertyWithModelBindingAttributeHasSameNameAsParameter model) { }
+        public void ActionMethod(
+            IsProblematicParameter_ReturnsTrue_IfPropertyWithModelBindingAttributeHasSameNameAsParameter model) { }
     }
 }

src/Mvc/Mvc.Analyzers/test/TestFiles/TopLevelParameterNameAnalyzerTest/SpecifiesModelTypeTests.cs

@@ -1,11 +1,16 @@
-namespace Microsoft.AspNetCore.Mvc.Analyzers.TopLevelParameterNameAnalyzerTestFiles
+using Microsoft.AspNetCore.Mvc.ModelBinding.Binders;
+
+namespace Microsoft.AspNetCore.Mvc.Analyzers.TopLevelParameterNameAnalyzerTestFiles
 {
     public class SpecifiesModelTypeTests
     {
-        public void SpecifiesModelType_ReturnsFalse_IfModelBinderDoesNotSpecifyType([ModelBinder(Name = "Name")] object model) { }
+        public void SpecifiesModelType_ReturnsFalse_IfModelBinderDoesNotSpecifyType(
+            [ModelBinder(Name = "Name")] object model) { }
 
-        public void SpecifiesModelType_ReturnsTrue_IfModelBinderSpecifiesTypeFromConstructor([ModelBinder(typeof(object))] object model) { }
+        public void SpecifiesModelType_ReturnsTrue_IfModelBinderSpecifiesTypeFromConstructor(
+            [ModelBinder(typeof(SimpleTypeModelBinder))] object model) { }
 
-        public void SpecifiesModelType_ReturnsTrue_IfModelBinderSpecifiesTypeFromProperty([ModelBinder(BinderType = typeof(object))] object model) { }
+        public void SpecifiesModelType_ReturnsTrue_IfModelBinderSpecifiesTypeFromProperty(
+            [ModelBinder(BinderType = typeof(SimpleTypeModelBinder))] object model) { }
     }
 }

src/Mvc/Mvc.ApiExplorer/test/DefaultApiDescriptionProviderTest.cs

@@ -1490,7 +1490,6 @@ namespace Microsoft.AspNetCore.Mvc.Description
         {
             // Arrange
             var action = CreateActionDescriptor(nameof(AcceptsCycle));
-            var parameterDescriptor = action.Parameters.Single();
 
             // Act
             var descriptions = GetApiDescriptions(action);
@@ -2070,7 +2069,7 @@ namespace Microsoft.AspNetCore.Mvc.Description
         {
         }
 
-        private void FromCustom([ModelBinder(BinderType = typeof(BodyModelBinder))] int id)
+        private void FromCustom([ModelBinder(typeof(BodyModelBinder))] int id)
         {
         }
 

src/Mvc/Mvc.Core/src/BindPropertyAttribute.cs

@@ -2,24 +2,65 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
+using Microsoft.AspNetCore.Mvc.Core;
 using Microsoft.AspNetCore.Mvc.ModelBinding;
 
 namespace Microsoft.AspNetCore.Mvc
 {
+    /// <summary>
+    /// An attribute that can specify a model name or type of <see cref="IModelBinder"/> to use for binding the
+    /// associated property.
+    /// </summary>
+    /// <remarks>
+    /// Similar to <see cref="ModelBinderAttribute"/>. Unlike that attribute, <see cref="BindPropertyAttribute"/>
+    /// applies only to properties and adds an <see cref="IRequestPredicateProvider"/> implementation that by default
+    /// indicates the property should not be bound for HTTP GET requests (see also <see cref="SupportsGet"/>).
+    /// </remarks>
     [AttributeUsage(AttributeTargets.Property, AllowMultiple = false, Inherited = true)]
     public class BindPropertyAttribute : Attribute, IModelNameProvider, IBinderTypeProviderMetadata, IRequestPredicateProvider
     {
         private static readonly Func<ActionContext, bool> _supportsAllRequests = (c) => true;
-
         private static readonly Func<ActionContext, bool> _supportsNonGetRequests = IsNonGetRequest;
 
         private BindingSource _bindingSource;
+        private Type _binderType;
 
+        /// <summary>
+        /// Gets or sets an indication the associated property should be bound in HTTP GET requests. If
+        /// <see langword="true"/>, the property should be bound in all requests. Otherwise, the property should not be
+        /// bound in HTTP GET requests.
+        /// </summary>
+        /// <value>Defaults to <see langword="false"/>.</value>
         public bool SupportsGet { get; set; }
 
-        public Type BinderType { get; set; }
+        /// <inheritdoc />
+        /// <remarks>
+        /// Subclass this attribute and set <see cref="BindingSource"/> if <see cref="BindingSource.Custom"/> is not
+        /// correct for the specified (non-<see langword="null"/>) <see cref="IModelBinder"/> implementation.
+        /// </remarks>
+        public Type BinderType
+        {
+            get => _binderType;
+            set
+            {
+                if (value != null && !typeof(IModelBinder).IsAssignableFrom(value))
+                {
+                    throw new ArgumentException(
+                        Resources.FormatBinderType_MustBeIModelBinder(
+                            value.FullName,
+                            typeof(IModelBinder).FullName),
+                        nameof(value));
+                }
+
+                _binderType = value;
+            }
+        }
 
         /// <inheritdoc />
+        /// <value>
+        /// If <see cref="BinderType"/> is <see langword="null"/>, defaults to <see langword="null"/>. Otherwise,
+        /// defaults to <see cref="BindingSource.Custom"/>. May be overridden in a subclass.
+        /// </value>
         public virtual BindingSource BindingSource
         {
             get

src/Mvc/Mvc.Core/src/ModelBinderAttribute.cs

@@ -2,6 +2,8 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
+using System.ComponentModel;
+using Microsoft.AspNetCore.Mvc.Core;
 using Microsoft.AspNetCore.Mvc.ModelBinding;
 
 namespace Microsoft.AspNetCore.Mvc
@@ -27,6 +29,7 @@ namespace Microsoft.AspNetCore.Mvc
     public class ModelBinderAttribute : Attribute, IModelNameProvider, IBinderTypeProviderMetadata
     {
         private BindingSource _bindingSource;
+        private Type _binderType;
 
         /// <summary>
         /// Initializes a new instance of <see cref="ModelBinderAttribute"/>.
@@ -39,19 +42,48 @@ namespace Microsoft.AspNetCore.Mvc
         /// Initializes a new instance of <see cref="ModelBinderAttribute"/>.
         /// </summary>
         /// <param name="binderType">A <see cref="Type"/> which implements <see cref="IModelBinder"/>.</param>
+        /// <remarks>
+        /// Subclass this attribute and set <see cref="BindingSource"/> if <see cref="BindingSource.Custom"/> is not
+        /// correct for the specified <paramref name="binderType"/>.
+        /// </remarks>
         public ModelBinderAttribute(Type binderType)
         {
             if (binderType == null)
             {
                 throw new ArgumentNullException(nameof(binderType));
             }
+
             BinderType = binderType;
         }
 
         /// <inheritdoc />
-        public Type BinderType { get; set; }
+        /// <remarks>
+        /// Subclass this attribute and set <see cref="BindingSource"/> if <see cref="BindingSource.Custom"/> is not
+        /// correct for the specified (non-<see langword="null"/>) <see cref="IModelBinder"/> implementation.
+        /// </remarks>
+        public Type BinderType
+        {
+            get => _binderType;
+            set
+            {
+                if (value != null && !typeof(IModelBinder).IsAssignableFrom(value))
+                {
+                    throw new ArgumentException(
+                        Resources.FormatBinderType_MustBeIModelBinder(
+                            value.FullName,
+                            typeof(IModelBinder).FullName),
+                        nameof(value));
+                }
+
+                _binderType = value;
+            }
+        }
 
         /// <inheritdoc />
+        /// <value>
+        /// If <see cref="BinderType"/> is <see langword="null"/>, defaults to <see langword="null"/>. Otherwise,
+        /// defaults to <see cref="BindingSource.Custom"/>. May be overridden in a subclass.
+        /// </value>
         public virtual BindingSource BindingSource
         {
             get
@@ -72,4 +104,4 @@ namespace Microsoft.AspNetCore.Mvc
         /// <inheritdoc />
         public string Name { get; set; }
     }
-}
+}

src/Mvc/Mvc.Core/src/ModelBinding/Binders/BinderTypeModelBinder.cs

@@ -2,7 +2,6 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
-using System.Reflection;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Mvc.Core;
 using Microsoft.Extensions.DependencyInjection;
@@ -28,7 +27,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
                 throw new ArgumentNullException(nameof(binderType));
             }
 
-            if (!typeof(IModelBinder).GetTypeInfo().IsAssignableFrom(binderType.GetTypeInfo()))
+            if (!typeof(IModelBinder).IsAssignableFrom(binderType))
             {
                 throw new ArgumentException(
                     Resources.FormatBinderType_MustBeIModelBinder(

src/Mvc/Mvc.Core/src/ModelBinding/Metadata/BindingMetadata.cs

@@ -2,6 +2,7 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
+using Microsoft.AspNetCore.Mvc.Core;
 
 namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata
 {
@@ -10,6 +11,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata
     /// </summary>
     public class BindingMetadata
     {
+        private Type _binderType;
         private DefaultModelBindingMessageProvider _messageProvider;
 
         /// <summary>
@@ -25,10 +27,30 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata
         public string BinderModelName { get; set; }
 
         /// <summary>
-        /// Gets or sets the <see cref="Type"/> of the model binder used to bind the model.
+        /// Gets or sets the <see cref="Type"/> of the <see cref="IModelBinder"/> implementation used to bind the
-        /// See <see cref="ModelMetadata.BinderType"/>.
+        /// model. See <see cref="ModelMetadata.BinderType"/>.
         /// </summary>
-        public Type BinderType { get; set; }
+        /// <remarks>
+        /// Also set <see cref="BindingSource"/> if the specified <see cref="IModelBinder"/> implementation does not
+        /// use values from form data, route values or the query string.
+        /// </remarks>
+        public Type BinderType
+        {
+            get => _binderType;
+            set
+            {
+                if (value != null && !typeof(IModelBinder).IsAssignableFrom(value))
+                {
+                    throw new ArgumentException(
+                        Resources.FormatBinderType_MustBeIModelBinder(
+                            value.FullName,
+                            typeof(IModelBinder).FullName),
+                        nameof(value));
+                }
+
+                _binderType = value;
+            }
+        }
 
         /// <summary>
         /// Gets or sets a value indicating whether or not the property can be model bound.
@@ -76,4 +98,4 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata
         /// </summary>
         public IPropertyFilterProvider PropertyFilterProvider { get; set; }
     }
-}
+}

src/Mvc/Mvc.Core/test/ApplicationModels/DefaultApplicationModelProviderTest.cs

@@ -10,6 +10,7 @@ using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc.ActionConstraints;
 using Microsoft.AspNetCore.Mvc.Filters;
 using Microsoft.AspNetCore.Mvc.ModelBinding;
+using Microsoft.AspNetCore.Mvc.ModelBinding.Binders;
 using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata;
 using Microsoft.AspNetCore.Mvc.Routing;
 using Microsoft.Extensions.Options;
@@ -1269,7 +1270,7 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels
         {
             public string Property { get; set; }
 
-            [ModelBinder(typeof(object))]
+            [ModelBinder(typeof(ComplexTypeModelBinder))]
             public string BinderType { get; set; }
 
             [FromRoute]
@@ -1306,7 +1307,7 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels
 
             // Assert
             var bindingInfo = property.BindingInfo;
-            Assert.Same(typeof(object), bindingInfo.BinderType);
+            Assert.Same(typeof(ComplexTypeModelBinder), bindingInfo.BinderType);
         }
 
         [Fact]
@@ -1334,7 +1335,8 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels
         public void CreatePropertyModel_AppliesBindPropertyAttributeDeclaredOnBaseType()
         {
             // Arrange
-            var propertyInfo = typeof(DerivedFromBindPropertyController).GetProperty(nameof(DerivedFromBindPropertyController.DerivedProperty));
+            var propertyInfo = typeof(DerivedFromBindPropertyController).GetProperty(
+                nameof(DerivedFromBindPropertyController.DerivedProperty));
 
             // Act
             var property = Provider.CreatePropertyModel(propertyInfo);

src/Mvc/Mvc.Core/test/ApplicationModels/InferParameterBindingInfoConventionTest.cs

@@ -10,7 +10,7 @@ using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc.ModelBinding;
-using Microsoft.Extensions.Logging.Abstractions;
+using Microsoft.AspNetCore.Mvc.ModelBinding.Binders;
 using Microsoft.Extensions.Options;
 using Xunit;
 
@@ -740,7 +740,6 @@ Environment.NewLine + "int b";
         private static InferParameterBindingInfoConvention GetConvention(
             IModelMetadataProvider modelMetadataProvider = null)
         {
-            var loggerFactory = NullLoggerFactory.Instance;
             modelMetadataProvider = modelMetadataProvider ?? new EmptyModelMetadataProvider();
             return new InferParameterBindingInfoConvention(modelMetadataProvider);
         }
@@ -999,7 +998,7 @@ Environment.NewLine + "int b";
         private class ParameterWithBindingInfo
         {
             [HttpGet("test")]
-            public IActionResult Action([ModelBinder(typeof(object))] Car car) => null;
+            public IActionResult Action([ModelBinder(typeof(ComplexTypeModelBinder))] Car car) => null;
         }
     }
 }

src/Mvc/Mvc.Core/test/ModelBinding/Metadata/BindingSourceTest.cs

@@ -26,6 +26,31 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
                 expected);
         }
 
+        public static TheoryData<BindingSource> ModelBinding_MatchData
+        {
+            get
+            {
+                return new TheoryData<BindingSource>
+                {
+                    BindingSource.Form,
+                    BindingSource.ModelBinding,
+                    BindingSource.Path,
+                    BindingSource.Query,
+                };
+            }
+        }
+
+        [Theory]
+        [MemberData(nameof(ModelBinding_MatchData))]
+        public void ModelBinding_CanAcceptDataFrom_Match(BindingSource bindingSource)
+        {
+            // Act
+            var result = BindingSource.ModelBinding.CanAcceptDataFrom(bindingSource);
+
+            // Assert
+            Assert.True(result);
+        }
+
         [Fact]
         public void BindingSource_CanAcceptDataFrom_Match()
         {
@@ -36,6 +61,33 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
             Assert.True(result);
         }
 
+        public static TheoryData<BindingSource> ModelBinding_NoMatchData
+        {
+            get
+            {
+                return new TheoryData<BindingSource>
+                {
+                    BindingSource.Body,
+                    BindingSource.Custom,
+                    BindingSource.FormFile,
+                    BindingSource.Header,
+                    BindingSource.Services,
+                    BindingSource.Special,
+                };
+            }
+        }
+
+        [Theory]
+        [MemberData(nameof(ModelBinding_NoMatchData))]
+        public void ModelBinding_CanAcceptDataFrom_NoMatch(BindingSource bindingSource)
+        {
+            // Act
+            var result = BindingSource.ModelBinding.CanAcceptDataFrom(bindingSource);
+
+            // Assert
+            Assert.False(result);
+        }
+
         [Fact]
         public void BindingSource_CanAcceptDataFrom_NoMatch()
         {
@@ -46,4 +98,4 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
             Assert.False(result);
         }
     }
-}
+}

src/Mvc/Mvc.Core/test/ModelBinding/Metadata/ModelBinderAttributeTest.cs

@@ -25,22 +25,39 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
         public void BinderType_DefaultCustomBindingSource()
         {
             // Arrange
-            var attribute = new ModelBinderAttribute();
+            var attribute = new ModelBinderAttribute
-            attribute.BinderType = typeof(ByteArrayModelBinder);
+            {
+                BinderType = typeof(ByteArrayModelBinder),
+            };
 
             // Act
             var source = attribute.BindingSource;
 
             // Assert
-            Assert.Equal(BindingSource.Custom, source);
+            Assert.Same(BindingSource.Custom, source);
+        }
+
+        [Fact]
+        public void BinderTypePassedToConstructor_DefaultCustomBindingSource()
+        {
+            // Arrange
+            var attribute = new ModelBinderAttribute(typeof(ByteArrayModelBinder));
+
+            // Act
+            var source = attribute.BindingSource;
+
+            // Assert
+            Assert.Same(BindingSource.Custom, source);
         }
 
         [Fact]
         public void BinderType_SettingBindingSource_OverridesDefaultCustomBindingSource()
         {
             // Arrange
-            var attribute = new FromQueryModelBinderAttribute();
+            var attribute = new FromQueryModelBinderAttribute
-            attribute.BinderType = typeof(ByteArrayModelBinder);
+            {
+                BinderType = typeof(ByteArrayModelBinder)
+            };
 
             // Act
             var source = attribute.BindingSource;
@@ -54,4 +71,4 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
             public override BindingSource BindingSource => BindingSource.Query;
         }
     }
-}
+}

src/Mvc/Mvc.Core/test/ModelBinding/ModelBinderFactoryTest.cs

@@ -2,6 +2,7 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
+using System.Threading.Tasks;
 using Microsoft.AspNetCore.Mvc.ModelBinding.Binders;
 using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata;
 using Microsoft.Extensions.DependencyInjection;
@@ -289,12 +290,12 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
                 var halfBindingInfo = new BindingInfo
                 {
                     BinderModelName = "expected name",
-                    BinderType = typeof(Widget),
+                    BinderType = typeof(WidgetBinder),
                 };
                 var fullBindingInfo = new BindingInfo
                 {
                     BinderModelName = "expected name",
-                    BinderType = typeof(Widget),
+                    BinderType = typeof(WidgetBinder),
                     BindingSource = BindingSource.Services,
                     PropertyFilterProvider = propertyFilterProvider,
                 };
@@ -303,7 +304,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
                 var differentBindingMetadata = new BindingMetadata
                 {
                     BinderModelName = "not the expected name",
-                    BinderType = typeof(WidgetId),
+                    BinderType = typeof(WidgetIdBinder),
                     BindingSource = BindingSource.ModelBinding,
                     PropertyFilterProvider = Mock.Of<IPropertyFilterProvider>(),
                 };
@@ -315,7 +316,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
                 var fullBindingMetadata = new BindingMetadata
                 {
                     BinderModelName = "expected name",
-                    BinderType = typeof(Widget),
+                    BinderType = typeof(WidgetBinder),
                     BindingSource = BindingSource.Services,
                     PropertyFilterProvider = propertyFilterProvider,
                 };
@@ -650,10 +651,26 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
             public WidgetId Id { get; set; }
         }
 
+        private class WidgetBinder : IModelBinder
+        {
+            public Task BindModelAsync(ModelBindingContext bindingContext)
+            {
+                throw new NotImplementedException();
+            }
+        }
+
         private class WidgetId
         {
         }
 
+        private class WidgetIdBinder : IModelBinder
+        {
+            public Task BindModelAsync(ModelBindingContext bindingContext)
+            {
+                throw new NotImplementedException();
+            }
+        }
+
         private class Employee
         {
             public Employee Manager { get; set; }

src/Mvc/Mvc.Core/test/ModelBinding/ParameterBinderTest.cs

@@ -12,6 +12,7 @@ using Microsoft.AspNetCore.JsonPatch;
 using Microsoft.AspNetCore.Mvc.Abstractions;
 using Microsoft.AspNetCore.Mvc.Controllers;
 using Microsoft.AspNetCore.Mvc.DataAnnotations;
+using Microsoft.AspNetCore.Mvc.ModelBinding.Binders;
 using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata;
 using Microsoft.AspNetCore.Mvc.ModelBinding.Validation;
 using Microsoft.AspNetCore.Testing;
@@ -37,7 +38,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
                 var bindingInfoWithName = new BindingInfo
                 {
                     BinderModelName = "bindingInfoName",
-                    BinderType = typeof(Person),
+                    BinderType = typeof(SimpleTypeModelBinder),
                 };
 
                 // parameterBindingInfo, metadataBinderModelName, parameterName, expectedBinderModelName
@@ -208,7 +209,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
             var modelBindingResult = ModelBindingResult.Success(null);
 
             // Act
-            var result = await parameterBinder.BindModelAsync(
+            await parameterBinder.BindModelAsync(
                 actionContext,
                 CreateMockModelBinder(modelBindingResult),
                 CreateMockValueProvider(),

src/Mvc/Mvc.DataAnnotations/test/ModelMetadataProviderTest.cs

@@ -7,6 +7,7 @@ using System.ComponentModel.DataAnnotations;
 using System.Linq;
 using System.Runtime.Serialization;
 using Microsoft.AspNetCore.Mvc.ModelBinding;
+using Microsoft.AspNetCore.Mvc.ModelBinding.Binders;
 using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata;
 using Microsoft.AspNetCore.Testing;
 using Microsoft.Extensions.Options;
@@ -629,7 +630,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations
             var attributes = new[]
             {
                 new TestBinderTypeProvider(),
-                new TestBinderTypeProvider() { BinderType = typeof(string) }
+                new TestBinderTypeProvider() { BinderType = typeof(ComplexTypeModelBinder) }
             };
 
             var provider = CreateProvider(attributes);
@@ -638,7 +639,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations
             var metadata = provider.GetMetadataForType(typeof(string));
 
             // Assert
-            Assert.Same(typeof(string), metadata.BinderType);
+            Assert.Same(typeof(ComplexTypeModelBinder), metadata.BinderType);
         }
 
         [Fact]
@@ -647,8 +648,8 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations
             // Arrange
             var attributes = new[]
             {
-                new TestBinderTypeProvider() { BinderType = typeof(int) },
+                new TestBinderTypeProvider() { BinderType = typeof(ComplexTypeModelBinder) },
-                new TestBinderTypeProvider() { BinderType = typeof(string) }
+                new TestBinderTypeProvider() { BinderType = typeof(SimpleTypeModelBinder) }
             };
 
             var provider = CreateProvider(attributes);
@@ -657,7 +658,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations
             var metadata = provider.GetMetadataForType(typeof(string));
 
             // Assert
-            Assert.Same(typeof(int), metadata.BinderType);
+            Assert.Same(typeof(ComplexTypeModelBinder), metadata.BinderType);
         }
 
         [Fact]

src/Mvc/test/Mvc.FunctionalTests/ApiBehaviorTest.cs

@@ -138,7 +138,6 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests
                 {"Name", new[] {"The field Name must be a string with a minimum length of 5 and a maximum length of 30."}},
                 {"Zip", new[] { @"The field Zip must match the regular expression '\d{5}'."}}
             };
-            var contactString = JsonConvert.SerializeObject(contactModel);
 
             // Act
             var response = await CustomInvalidModelStateClient.PostAsJsonAsync("/contact/PostWithVnd", contactModel);

src/Mvc/test/Mvc.FunctionalTests/InputFormatterTests.cs

@@ -92,7 +92,6 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests
 
             // Act
             var response = await Client.PostAsync("http://localhost/JsonFormatter/ReturnInput/", content);
-            var responseBody = await response.Content.ReadAsStringAsync();
 
             // Assert
             Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode);
@@ -157,7 +156,6 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests
 
             // Act
             var response = await Client.PostAsync("http://localhost/JsonFormatter/ReturnInput/", content);
-            var responseBody = await response.Content.ReadAsStringAsync();
 
             // Assert
             Assert.Equal(HttpStatusCode.UnsupportedMediaType, response.StatusCode);
@@ -213,7 +211,6 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests
 
             // Act
             var response = await Client.PostAsync("http://localhost/InputFormatter/ReturnInput/", content);
-            var responseBody = await response.Content.ReadAsStringAsync();
 
             // Assert
             Assert.Equal(HttpStatusCode.UnsupportedMediaType, response.StatusCode);
@@ -314,4 +311,4 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests
                 });
         }
     }
-}
+}

src/Mvc/test/Mvc.FunctionalTests/RazorPagesTest.cs

@@ -790,7 +790,6 @@ Hello from /Pages/WithViewStart/Index.cshtml!";
             // Arrange
             var name = "TestName";
             var age = 123;
-            var expected = $"Name = {name}, Age = {age}";
             var request = new HttpRequestMessage(HttpMethod.Post, "Pages/PropertyBinding/PolymorphicBinding")
             {
                 Content = new FormUrlEncodedContent(new Dictionary<string, string>

src/Mvc/test/Mvc.IntegrationTests/ComplexTypeModelBinderIntegrationTest.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Globalization;
 using System.IO;
 using System.Text;
 using System.Threading.Tasks;
@@ -10,7 +11,9 @@ using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Http.Internal;
 using Microsoft.AspNetCore.Mvc.Abstractions;
 using Microsoft.AspNetCore.Mvc.ModelBinding;
+using Microsoft.AspNetCore.Mvc.ModelBinding.Binders;
 using Microsoft.AspNetCore.Testing;
+using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Primitives;
 using Xunit;
 
@@ -3201,6 +3204,96 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests
             Assert.Null(state.Value.RawValue);
         }
 
+        private class TestModel
+        {
+            public TestInnerModel[] InnerModels { get; set; } = Array.Empty<TestInnerModel>();
+        }
+
+        private class TestInnerModel
+        {
+            [ModelBinder(BinderType = typeof(NumberModelBinder))]
+            public decimal Rate { get; set; }
+        }
+
+        private class NumberModelBinder : IModelBinder
+        {
+            private readonly NumberStyles _supportedStyles = NumberStyles.Float | NumberStyles.AllowThousands;
+            private DecimalModelBinder _innerBinder;
+
+            public NumberModelBinder(ILoggerFactory loggerFactory)
+            {
+                _innerBinder = new DecimalModelBinder(_supportedStyles, loggerFactory);
+            }
+
+            public Task BindModelAsync(ModelBindingContext bindingContext)
+            {
+                return _innerBinder.BindModelAsync(bindingContext);
+            }
+        }
+
+        // Regression test for #4939.
+        [Fact]
+        public async Task ComplexTypeModelBinder_ReportsFailureToCollectionModelBinder_CustomBinder()
+        {
+            // Arrange
+            var parameter = new ParameterDescriptor()
+            {
+                Name = "parameter",
+                ParameterType = typeof(TestModel),
+            };
+
+            var testContext = ModelBindingTestHelper.GetTestContext(request =>
+            {
+                request.QueryString = new QueryString(
+                    "?parameter.InnerModels[0].Rate=1,000.00&parameter.InnerModels[1].Rate=2000");
+            });
+
+            var modelState = testContext.ModelState;
+            var metadata = GetMetadata(testContext, parameter);
+            var modelBinder = GetModelBinder(testContext, parameter, metadata);
+            var valueProvider = await CompositeValueProvider.CreateAsync(testContext);
+            var parameterBinder = ModelBindingTestHelper.GetParameterBinder(testContext);
+
+            // Act
+            var modelBindingResult = await parameterBinder.BindModelAsync(
+                testContext,
+                modelBinder,
+                valueProvider,
+                parameter,
+                metadata,
+                value: null);
+
+            // Assert
+            Assert.True(modelBindingResult.IsModelSet);
+
+            var model = Assert.IsType<TestModel>(modelBindingResult.Model);
+            Assert.NotNull(model.InnerModels);
+            Assert.Collection(
+                model.InnerModels,
+                item => Assert.Equal(1000, item.Rate),
+                item => Assert.Equal(2000, item.Rate));
+
+            Assert.True(modelState.IsValid);
+            Assert.Collection(
+                modelState,
+                kvp =>
+                {
+                    Assert.Equal("parameter.InnerModels[0].Rate", kvp.Key);
+                    Assert.Equal("1,000.00", kvp.Value.AttemptedValue);
+                    Assert.Empty(kvp.Value.Errors);
+                    Assert.Equal("1,000.00", kvp.Value.RawValue);
+                    Assert.Equal(ModelValidationState.Valid, kvp.Value.ValidationState);
+                },
+                kvp =>
+                {
+                    Assert.Equal("parameter.InnerModels[1].Rate", kvp.Key);
+                    Assert.Equal("2000", kvp.Value.AttemptedValue);
+                    Assert.Empty(kvp.Value.Errors);
+                    Assert.Equal("2000", kvp.Value.RawValue);
+                    Assert.Equal(ModelValidationState.Valid, kvp.Value.ValidationState);
+                });
+        }
+
         private class Person6
         {
             public string Name { get; set; }