Ignore overriden properties in component parameters (#13198)

* Ignore overriden properties in component parameters

* Modify MemberAssignment to return shadowed properties, but not inherited properties
* Modify ComponentProperties to throw if a Parameter property is non-public

Fixes https://github.com/aspnet/AspNetCore/issues/13162

src/Components/Components/src/Reflection/ComponentProperties.cs

@@ -238,9 +238,15 @@ namespace Microsoft.AspNetCore.Components.Reflection
                         continue;
                     }
 
+                    var propertyName = propertyInfo.Name;
+                    if (parameterAttribute != null && (propertyInfo.SetMethod == null || !propertyInfo.SetMethod.IsPublic))
+                    {
+                        throw new InvalidOperationException(
+                            $"The type '{targetType.FullName}' declares a parameter matching the name '{propertyName}' that is not public. Parameters must be public.");
+                    }
+
                     var propertySetter = MemberAssignment.CreatePropertySetter(targetType, propertyInfo, cascading: cascadingParameterAttribute != null);
 
-                    var propertyName = propertyInfo.Name;
                     if (WritersByName.ContainsKey(propertyName))
                     {
                         throw new InvalidOperationException(

src/Components/Components/src/Reflection/MemberAssignment.cs

@@ -13,17 +13,34 @@ namespace Microsoft.AspNetCore.Components.Reflection
         public static IEnumerable<PropertyInfo> GetPropertiesIncludingInherited(
             Type type, BindingFlags bindingFlags)
         {
+            var dictionary = new Dictionary<string, List<PropertyInfo>>();
+
             while (type != null)
             {
                 var properties = type.GetProperties(bindingFlags)
                     .Where(prop => prop.DeclaringType == type);
                 foreach (var property in properties)
                 {
-                    yield return property;
+                    if (!dictionary.TryGetValue(property.Name, out var others))
+                    {
+                        others = new List<PropertyInfo>();
+                        dictionary.Add(property.Name, others);
+                    }
+
+                    if (others.Any(other => other.GetMethod?.GetBaseDefinition() == property.GetMethod?.GetBaseDefinition()))
+                    {
+                        // This is an inheritance case. We can safely ignore the value of property since
+                        // we have seen a more derived value.
+                        continue;
+                    }
+
+                    others.Add(property);
                 }
 
                 type = type.BaseType;
             }
+
+            return dictionary.Values.SelectMany(p => p);
         }
 
         public static IPropertySetter CreatePropertySetter(Type targetType, PropertyInfo property, bool cascading)

src/Components/Components/test/ParameterViewTest.Assignment.cs

@@ -6,7 +6,6 @@ using System.Collections;
 using System.Collections.Generic;
 using System.Linq;
 using Microsoft.AspNetCore.Components.Rendering;
-using Microsoft.AspNetCore.Components.RenderTree;
 using Xunit;
 
 namespace Microsoft.AspNetCore.Components
@@ -71,6 +70,24 @@ namespace Microsoft.AspNetCore.Components
             Assert.Equal(456, target.DerivedClassIntProp);
         }
 
+        [Fact]
+        public void IncomingParameterMatchesOverridenParameter_ThatDoesNotHasAttribute()
+        {
+            // Test for https://github.com/aspnet/AspNetCore/issues/13162
+            // Arrange
+            var parameters = new ParameterViewBuilder
+            {
+                { nameof(DerivedType.VirtualProp), 123 },
+            }.Build();
+            var target = new DerivedType();
+
+            // Act
+            parameters.SetParameterProperties(target);
+
+            // Assert
+            Assert.Equal(123, target.VirtualProp);
+        }
+
         [Fact]
         public void NoIncomingParameterMatchesDeclaredParameter_LeavesValueUnchanged()
         {
@@ -172,6 +189,43 @@ namespace Microsoft.AspNetCore.Components
                 ex.Message);
         }
 
+        [Fact]
+        public void IncomingParameterMatchesPropertyNotPublic_Throws()
+        {
+            // Arrange
+            var target = new HasNonPublicPropertyWithParameterAttribute();
+            var parameters = new ParameterViewBuilder
+            {
+                { nameof(HasNonPublicPropertyWithParameterAttribute.IntProp), 123 },
+            }.Build();
+
+            // Act
+            var ex = Assert.Throws<InvalidOperationException>(
+                () => parameters.SetParameterProperties(target));
+
+            // Assert
+            Assert.Equal(default, target.IntProp);
+            Assert.Equal(
+                $"The type '{typeof(HasNonPublicPropertyWithParameterAttribute).FullName}' declares a parameter matching the name '{nameof(HasNonPublicPropertyWithParameterAttribute.IntProp)}' that is not public. Parameters must be public.",
+                ex.Message);
+        }
+
+        [Fact]
+        public void IncomingCascadingParameterMatchesPropertyNotPublic_Works()
+        {
+            // Arrange
+            var target = new HasNonPublicCascadingParameter();
+            var builder = new ParameterViewBuilder();
+            builder.Add("Cascading", "Test", cascading: true);
+            var parameters = builder.Build();
+
+            // Act
+            parameters.SetParameterProperties(target);
+
+            // Assert
+            Assert.Equal("Test", target.GetCascadingValue());
+        }
+
         [Fact]
         public void IncomingNonCascadingValueMatchesCascadingParameter_Throws()
         {
@@ -497,13 +551,9 @@ namespace Microsoft.AspNetCore.Components
 
         class HasInstanceProperties
         {
-            // "internal" to show we're not requiring public accessors, but also
-            // to keep the assertions simple in the tests
-
             [Parameter] public int IntProp { get; set; }
             [Parameter] public string StringProp { get; set; }
 
-            // Also a truly private one to show there's nothing special about 'internal'
             [Parameter] public object ObjectProp { get; set; }
 
             public static string ObjectPropName => nameof(ObjectProp);
@@ -521,6 +571,12 @@ namespace Microsoft.AspNetCore.Components
 
         class HasPropertyWithoutParameterAttribute
         {
+            public int IntProp { get; set; }
+        }
+
+        class HasNonPublicPropertyWithParameterAttribute
+        {
+            [Parameter]
             internal int IntProp { get; set; }
         }
 
@@ -539,6 +595,16 @@ namespace Microsoft.AspNetCore.Components
             [Parameter] public int DerivedClassIntProp { get; set; }
         }
 
+        class BaseType
+        {
+            [Parameter] public virtual int VirtualProp { get; set; }
+        }
+
+        class DerivedType : BaseType
+        {
+            public override int VirtualProp { get; set; }
+        }
+
         class HasParametersVaryingOnlyByCase
         {
             [Parameter] public object MyValue { get; set; }
@@ -575,6 +641,13 @@ namespace Microsoft.AspNetCore.Components
             [Parameter(CaptureUnmatchedValues = true)] public KeyValuePair<string, object>[] CaptureUnmatchedValuesProp { get; set; }
         }
 
+        class HasNonPublicCascadingParameter
+        {
+            [CascadingParameter] private string Cascading { get; set; }
+
+            public string GetCascadingValue() => Cascading;
+        }
+
         class ParameterViewBuilder : IEnumerable
         {
             private readonly List<(string Name, object Value, bool Cascading)> _keyValuePairs

src/Components/Components/test/RenderTreeDiffBuilderTest.cs

@@ -2231,7 +2231,7 @@ namespace Microsoft.AspNetCore.Components.Test
             public object ObjectProperty { get; set; }
 
             [Parameter]
-            public string ReadonlyProperty { get; private set; }
+            public string ReadonlyProperty { get; set; }
 
             [Parameter]
             public string PrivateProperty { get; set; }

src/Components/Components/test/RendererTest.cs

@@ -3791,10 +3791,10 @@ namespace Microsoft.AspNetCore.Components.Test
         private class FakeComponent : IComponent
         {
             [Parameter]
-            public int IntProperty { get; private set; }
+            public int IntProperty { get; set; }
 
             [Parameter]
-            public string StringProperty { get; private set; }
+            public string StringProperty { get; set; }
 
             [Parameter]
             public object ObjectProperty { get; set; }
@@ -3927,7 +3927,7 @@ namespace Microsoft.AspNetCore.Components.Test
         private class ReRendersParentComponent : AutoRenderComponent
         {
             [Parameter]
-            public TestComponent Parent { get; private set; }
+            public TestComponent Parent { get; set; }
 
             private bool _isFirstTime = true;
 
@@ -4057,7 +4057,7 @@ namespace Microsoft.AspNetCore.Components.Test
             public bool Disposed { get; private set; }
 
             [Parameter]
-            public Action DisposeAction { get; private set; }
+            public Action DisposeAction { get; set; }
 
             public void Dispose()
             {