Merge pull request #10572 from Enscape/fixes/coercion-type-safety

Fixed coerce methods being called on incompatible types

Avalonia.Desktop.slnf

@@ -8,9 +8,9 @@
       "samples\\GpuInterop\\GpuInterop.csproj",
       "samples\\IntegrationTestApp\\IntegrationTestApp.csproj",
       "samples\\MiniMvvm\\MiniMvvm.csproj",
+      "samples\\ReactiveUIDemo\\ReactiveUIDemo.csproj",
       "samples\\SampleControls\\ControlSamples.csproj",
       "samples\\Sandbox\\Sandbox.csproj",
-      "samples\\ReactiveUIDemo\\ReactiveUIDemo.csproj",
       "src\\Avalonia.Base\\Avalonia.Base.csproj",
       "src\\Avalonia.Build.Tasks\\Avalonia.Build.Tasks.csproj",
       "src\\Avalonia.Controls.ColorPicker\\Avalonia.Controls.ColorPicker.csproj",
@@ -41,6 +41,7 @@
       "src\\Windows\\Avalonia.Direct2D1\\Avalonia.Direct2D1.csproj",
       "src\\Windows\\Avalonia.Win32.Interop\\Avalonia.Win32.Interop.csproj",
       "src\\Windows\\Avalonia.Win32\\Avalonia.Win32.csproj",
+      "src\\tools\\Avalonia.Generators\\Avalonia.Generators.csproj",
       "src\\tools\\DevAnalyzers\\DevAnalyzers.csproj",
       "src\\tools\\DevGenerators\\DevGenerators.csproj",
       "src\\tools\\PublicAnalyzers\\Avalonia.Analyzers.csproj",
@@ -63,4 +64,4 @@
       "tests\\Avalonia.UnitTests\\Avalonia.UnitTests.csproj"
     ]
   }
-}
+}

src/Avalonia.Base/AvaloniaProperty.cs

@@ -4,7 +4,6 @@ using System.Diagnostics.CodeAnalysis;
 using Avalonia.Data;
 using Avalonia.Data.Core;
 using Avalonia.PropertyStore;
-using Avalonia.Styling;
 using Avalonia.Utilities;
 
 namespace Avalonia
@@ -20,12 +19,20 @@ namespace Avalonia
         public static readonly object UnsetValue = new UnsetValueType();
 
         private static int s_nextId;
+
+        /// <summary>
+        /// Provides a metadata object for types which have no metadata of their own.
+        /// </summary>
         private readonly AvaloniaPropertyMetadata _defaultMetadata;
+
+        /// <summary>
+        /// Provides a fast path when the property has no metadata overrides.
+        /// </summary>
+        private KeyValuePair<Type, AvaloniaPropertyMetadata>? _singleMetadata;
+
         private readonly Dictionary<Type, AvaloniaPropertyMetadata> _metadata;
         private readonly Dictionary<Type, AvaloniaPropertyMetadata> _metadataCache = new Dictionary<Type, AvaloniaPropertyMetadata>();
 
-        private bool _hasMetadataOverrides;
-
         /// <summary>
         /// Initializes a new instance of the <see cref="AvaloniaProperty"/> class.
         /// </summary>
@@ -57,7 +64,8 @@ namespace Avalonia
             Id = s_nextId++;
 
             _metadata.Add(ownerType, metadata ?? throw new ArgumentNullException(nameof(metadata)));
-            _defaultMetadata = metadata;
+            _defaultMetadata = metadata.GenerateTypeSafeMetadata();
+            _singleMetadata = new(ownerType, metadata);
         }
 
         /// <summary>
@@ -80,9 +88,6 @@ namespace Avalonia
             Id = source.Id;
             _defaultMetadata = source._defaultMetadata;
 
-            // Properties that have different owner can't use fast path for metadata.
-            _hasMetadataOverrides = true;
-
             if (metadata != null)
             {
                 _metadata.Add(ownerType, metadata);
@@ -453,33 +458,14 @@ namespace Avalonia
         }
 
         /// <summary>
-        /// Gets the property metadata for the specified type.
+        /// Gets the <see cref="AvaloniaPropertyMetadata"/> which applies to this property when it is used with the specified type.
         /// </summary>
-        /// <typeparam name="T">The type.</typeparam>
-        /// <returns>
-        /// The property metadata.
-        /// </returns>
-        public AvaloniaPropertyMetadata GetMetadata<T>() where T : AvaloniaObject
-        {
-            return GetMetadata(typeof(T));
-        }
+        /// <typeparam name="T">The type for which to retrieve metadata.</typeparam>
+        public AvaloniaPropertyMetadata GetMetadata<T>() where T : AvaloniaObject => GetMetadata(typeof(T));
 
-        /// <summary>
-        /// Gets the property metadata for the specified type.
-        /// </summary>
-        /// <param name="type">The type.</param>
-        /// <returns>
-        /// The property metadata.
-        /// </returns>
-        public AvaloniaPropertyMetadata GetMetadata(Type type)
-        {
-            if (!_hasMetadataOverrides)
-            {
-                return _defaultMetadata;
-            }
-
-            return GetMetadataWithOverrides(type);
-        }
+        /// <inheritdoc cref="GetMetadata{T}"/>
+        /// <param name="type">The type for which to retrieve metadata.</param>
+        public AvaloniaPropertyMetadata GetMetadata(Type type) => GetMetadataWithOverrides(type);
 
         /// <summary>
         /// Checks whether the <paramref name="value"/> is valid for the property.
@@ -578,7 +564,7 @@ namespace Avalonia
             _metadata.Add(type, metadata);
             _metadataCache.Clear();
 
-            _hasMetadataOverrides = true;
+            _singleMetadata = null;
         }
 
         protected abstract IObservable<AvaloniaPropertyChangedEventArgs> GetChanged();
@@ -595,7 +581,12 @@ namespace Avalonia
                 return result;
             }
 
-            Type? currentType = type;
+            if (_singleMetadata is { } singleMetadata)
+            {
+                return _metadataCache[type] = singleMetadata.Key.IsAssignableFrom(type) ? singleMetadata.Value : _defaultMetadata;
+            }
+
+            var currentType = type;
 
             while (currentType != null)
             {
@@ -609,13 +600,11 @@ namespace Avalonia
                 currentType = currentType.BaseType;
             }
 
-            _metadataCache[type] = _defaultMetadata;
-
-            return _defaultMetadata;
+            return _metadataCache[type] = _defaultMetadata;
         }
 
         bool IPropertyInfo.CanGet => true;
-        bool IPropertyInfo.CanSet => true;
+        bool IPropertyInfo.CanSet => !IsReadOnly;
         object? IPropertyInfo.Get(object target) => ((AvaloniaObject)target).GetValue(this);
         void IPropertyInfo.Set(object target, object? value) => ((AvaloniaObject)target).SetValue(this, value);
     }

src/Avalonia.Base/AvaloniaPropertyMetadata.cs

@@ -5,7 +5,7 @@ namespace Avalonia
     /// <summary>
     /// Base class for avalonia property metadata.
     /// </summary>
-    public class AvaloniaPropertyMetadata
+    public abstract class AvaloniaPropertyMetadata
     {
         private BindingMode _defaultBindingMode;
 
@@ -61,5 +61,13 @@ namespace Avalonia
 
             EnableDataValidation ??= baseMetadata.EnableDataValidation;
         }
+
+        /// <summary>
+        /// Gets a copy of this object configured for use with any owner type.
+        /// </summary>
+        /// <remarks>
+        /// For example, delegates which receive the owner object should be removed.
+        /// </remarks>
+        public abstract AvaloniaPropertyMetadata GenerateTypeSafeMetadata();
     }
 }

src/Avalonia.Base/DirectPropertyMetadata`1.cs

@@ -45,5 +45,7 @@ namespace Avalonia
                 UnsetValue ??= src.UnsetValue;
             }
         }
+
+        public override AvaloniaPropertyMetadata GenerateTypeSafeMetadata() => new DirectPropertyMetadata<TValue>(UnsetValue, DefaultBindingMode, EnableDataValidation);
     }
 }

src/Avalonia.Base/StyledProperty.cs

@@ -18,7 +18,10 @@ namespace Avalonia
         /// <param name="ownerType">The type of the class that registers the property.</param>
         /// <param name="metadata">The property metadata.</param>
         /// <param name="inherits">Whether the property inherits its value.</param>
-        /// <param name="validate">A value validation callback.</param>
+        /// <param name="validate">
+        /// <para>A method which returns "false" for values that are never valid for this property.</para>
+        /// <para>This method is not part of the property's metadata and so cannot be changed after registration.</para>
+        /// </param>
         /// <param name="notifying">A <see cref="AvaloniaProperty.Notifying"/> callback.</param>
         public StyledProperty(
             string name,
@@ -41,7 +44,7 @@ namespace Avalonia
         }
 
         /// <summary>
-        /// Gets the value validation callback for the property.
+        /// A method which returns "false" for values that are never valid for this property.
         /// </summary>
         public Func<TValue, bool>? ValidateValue { get; }
 

src/Avalonia.Base/StyledPropertyMetadata`1.cs

@@ -58,5 +58,7 @@ namespace Avalonia
                 }
             }
         }
+
+        public override AvaloniaPropertyMetadata GenerateTypeSafeMetadata() => new StyledPropertyMetadata<TValue>(DefaultValue, DefaultBindingMode, enableDataValidation: EnableDataValidation ?? false);
     }
 }

tests/Avalonia.Base.UnitTests/AvaloniaPropertyTests.cs

@@ -2,8 +2,6 @@ using System;
 using System.Collections.Generic;
 using Avalonia.Data;
 using Avalonia.PropertyStore;
-using Avalonia.Styling;
-using Avalonia.Utilities;
 using Xunit;
 
 namespace Avalonia.Base.UnitTests
@@ -29,7 +27,7 @@ namespace Avalonia.Base.UnitTests
         [Fact]
         public void GetMetadata_Returns_Supplied_Value()
         {
-            var metadata = new AvaloniaPropertyMetadata();
+            var metadata = new TestMetadata();
             var target = new TestProperty<string>("test", typeof(Class1), metadata);
 
             Assert.Same(metadata, target.GetMetadata<Class1>());
@@ -38,26 +36,30 @@ namespace Avalonia.Base.UnitTests
         [Fact]
         public void GetMetadata_Returns_Supplied_Value_For_Derived_Class()
         {
-            var metadata = new AvaloniaPropertyMetadata();
+            var metadata = new TestMetadata();
             var target = new TestProperty<string>("test", typeof(Class1), metadata);
 
             Assert.Same(metadata, target.GetMetadata<Class2>());
         }
 
         [Fact]
-        public void GetMetadata_Returns_Supplied_Value_For_Unrelated_Class()
+        public void GetMetadata_Returns_TypeSafe_Metadata_For_Unrelated_Class()
         {
-            var metadata = new AvaloniaPropertyMetadata();
+            var metadata = new TestMetadata(BindingMode.OneWayToSource, true, x => { _ = (StyledElement)x; });
             var target = new TestProperty<string>("test", typeof(Class3), metadata);
 
-            Assert.Same(metadata, target.GetMetadata<Class2>());
+            var targetMetadata = (TestMetadata)target.GetMetadata<Class2>();
+
+            Assert.Equal(metadata.DefaultBindingMode, targetMetadata.DefaultBindingMode);
+            Assert.Equal(metadata.EnableDataValidation, targetMetadata.EnableDataValidation);
+            Assert.Equal(null, targetMetadata.OwnerSpecificAction);
         }
 
         [Fact]
         public void GetMetadata_Returns_Overridden_Value()
         {
-            var metadata = new AvaloniaPropertyMetadata();
-            var overridden = new AvaloniaPropertyMetadata();
+            var metadata = new TestMetadata();
+            var overridden = new TestMetadata();
             var target = new TestProperty<string>("test", typeof(Class1), metadata);
 
             target.OverrideMetadata<Class2>(overridden);
@@ -68,9 +70,9 @@ namespace Avalonia.Base.UnitTests
         [Fact]
         public void OverrideMetadata_Should_Merge_Values()
         {
-            var metadata = new AvaloniaPropertyMetadata(BindingMode.TwoWay);
+            var metadata = new TestMetadata(BindingMode.TwoWay);
             var notify = (Action<AvaloniaObject, bool>)((a, b) => { });
-            var overridden = new AvaloniaPropertyMetadata();
+            var overridden = new TestMetadata();
             var target = new TestProperty<string>("test", typeof(Class1), metadata);
 
             target.OverrideMetadata<Class2>(overridden);
@@ -131,15 +133,31 @@ namespace Avalonia.Base.UnitTests
         [Fact]
         public void PropertyMetadata_BindingMode_Default_Returns_OneWay()
         {
-            var data = new AvaloniaPropertyMetadata(defaultBindingMode: BindingMode.Default);
+            var data = new TestMetadata(defaultBindingMode: BindingMode.Default);
 
             Assert.Equal(BindingMode.OneWay, data.DefaultBindingMode);
         }
 
+        private class TestMetadata : AvaloniaPropertyMetadata
+        {
+            public Action<AvaloniaObject> OwnerSpecificAction { get; }
+
+            public TestMetadata(BindingMode defaultBindingMode = BindingMode.Default, 
+                bool? enableDataValidation = null,
+                Action<AvaloniaObject> ownerSpecificAction = null) 
+                : base(defaultBindingMode, enableDataValidation)
+            {
+                OwnerSpecificAction = ownerSpecificAction;
+            }
+
+            public override AvaloniaPropertyMetadata GenerateTypeSafeMetadata() =>
+                new TestMetadata(DefaultBindingMode, EnableDataValidation, null);
+        }
+
         private class TestProperty<TValue> : AvaloniaProperty<TValue>
         {
-            public TestProperty(string name, Type ownerType, AvaloniaPropertyMetadata metadata = null)
-                : base(name, ownerType, metadata ?? new AvaloniaPropertyMetadata())
+            public TestProperty(string name, Type ownerType, TestMetadata metadata = null)
+                : base(name, ownerType, metadata ?? new TestMetadata())
             {
             }