From 5210926c03146492258bceb33ecf60a299e12652 Mon Sep 17 00:00:00 2001
From: Steven Kirk <grokys@users.noreply.github.com>
Date: Fri, 20 Jul 2018 20:22:58 +0200
Subject: [PATCH] Disallow setting unregistered properties.

---
 src/Avalonia.Base/AvaloniaPropertyRegistry.cs | 52 ++-----------------
 .../AvaloniaPropertyTypeConverter.cs          | 26 +++++-----
 .../AvaloniaPropertyRegistryTests.cs          | 11 ----
 .../AvaloniaPropertyConverterTest.cs          | 36 +++++++++++--
 .../Xaml/StyleTests.cs                        | 26 ++++++++++
 5 files changed, 75 insertions(+), 76 deletions(-)

diff --git a/src/Avalonia.Base/AvaloniaPropertyRegistry.cs b/src/Avalonia.Base/AvaloniaPropertyRegistry.cs
index c0a4ace6ed..e29e7339ae 100644
--- a/src/Avalonia.Base/AvaloniaPropertyRegistry.cs
+++ b/src/Avalonia.Base/AvaloniaPropertyRegistry.cs
@@ -106,7 +106,7 @@ namespace Avalonia
         }
 
         /// <summary>
-        /// Finds a registered non-attached property on a type by name.
+        /// Finds a registered property on a type by name.
         /// </summary>
         /// <param name="type">The type.</param>
         /// <param name="name">The property name.</param>
@@ -130,7 +130,7 @@ namespace Avalonia
         }
 
         /// <summary>
-        /// Finds a registered non-attached property on a type by name.
+        /// Finds a registered property on an object by name.
         /// </summary>
         /// <param name="o">The object.</param>
         /// <param name="name">The property name.</param>
@@ -148,52 +148,6 @@ namespace Avalonia
             return FindRegistered(o.GetType(), name);
         }
 
-        /// <summary>
-        /// Finds a registered attached property on a type by name.
-        /// </summary>
-        /// <param name="type">The type.</param>
-        /// <param name="ownerType">The owner type.</param>
-        /// <param name="name">The property name.</param>
-        /// <returns>
-        /// The registered property or null if no matching property found.
-        /// </returns>
-        /// <exception cref="InvalidOperationException">
-        /// The property name contains a '.'.
-        /// </exception>
-        public AvaloniaProperty FindRegisteredAttached(Type type, Type ownerType, string name)
-        {
-            Contract.Requires<ArgumentNullException>(type != null);
-            Contract.Requires<ArgumentNullException>(ownerType != null);
-            Contract.Requires<ArgumentNullException>(name != null);
-
-            if (name.Contains('.'))
-            {
-                throw new InvalidOperationException("Attached properties not supported.");
-            }
-
-            return GetRegisteredAttached(type).FirstOrDefault(x => x.Name == name);
-        }
-
-        /// <summary>
-        /// Finds a registered non-attached property on a type by name.
-        /// </summary>
-        /// <param name="o">The object.</param>
-        /// <param name="ownerType">The owner type.</param>
-        /// <param name="name">The property name.</param>
-        /// <returns>
-        /// The registered property or null if no matching property found.
-        /// </returns>
-        /// <exception cref="InvalidOperationException">
-        /// The property name contains a '.'.
-        /// </exception>
-        public AvaloniaProperty FindRegisteredAttached(AvaloniaObject o, Type ownerType, string name)
-        {
-            Contract.Requires<ArgumentNullException>(o != null);
-            Contract.Requires<ArgumentNullException>(name != null);
-
-            return FindRegisteredAttached(o.GetType(), ownerType, name);
-        }
-
         /// <summary>
         /// Checks whether a <see cref="AvaloniaProperty"/> is registered on a type.
         /// </summary>
@@ -287,4 +241,4 @@ namespace Avalonia
             _attachedCache.Clear();
         }
     }
-}
\ No newline at end of file
+}
diff --git a/src/Markup/Avalonia.Markup.Xaml/Converters/AvaloniaPropertyTypeConverter.cs b/src/Markup/Avalonia.Markup.Xaml/Converters/AvaloniaPropertyTypeConverter.cs
index 6cdf0452d0..2470778685 100644
--- a/src/Markup/Avalonia.Markup.Xaml/Converters/AvaloniaPropertyTypeConverter.cs
+++ b/src/Markup/Avalonia.Markup.Xaml/Converters/AvaloniaPropertyTypeConverter.cs
@@ -4,6 +4,7 @@
 using System;
 using System.ComponentModel;
 using System.Globalization;
+using Avalonia.Controls;
 using Avalonia.Markup.Parsers;
 using Avalonia.Markup.Xaml.Parsers;
 using Avalonia.Markup.Xaml.Templates;
@@ -21,26 +22,27 @@ namespace Avalonia.Markup.Xaml.Converters
 
         public override object ConvertFrom(ITypeDescriptorContext context, CultureInfo culture, object value)
         {
+            var registry = AvaloniaPropertyRegistry.Instance;
             var parser = new PropertyParser();
             var reader = new Reader((string)value);
             var (ns, owner, propertyName) = parser.Parse(reader);
-            var ownerType = TryResolveOwnerByName(context, ns, owner) ??
-                context.GetFirstAmbientValue<ControlTemplate>()?.TargetType ??
-                context.GetFirstAmbientValue<Style>()?.Selector?.TargetType;
+            var ownerType = TryResolveOwnerByName(context, ns, owner);
+            var targetType = context.GetFirstAmbientValue<ControlTemplate>()?.TargetType ??
+                context.GetFirstAmbientValue<Style>()?.Selector?.TargetType ??
+                typeof(Control);
+            var effectiveOwner = ownerType ?? targetType;
+            var property = registry.FindRegistered(effectiveOwner, propertyName);
 
-            if (ownerType == null)
+            if (property == null)
             {
-                throw new XamlLoadException(
-                    $"Could not determine the owner type for property '{propertyName}'. " +
-                    "Please fully qualify the property name or specify a target type on " +
-                    "the containing template.");
+                throw new XamlLoadException($"Could not find property '{effectiveOwner.Name}.{propertyName}'.");
             }
 
-            var property = AvaloniaPropertyRegistry.Instance.FindRegistered(ownerType, propertyName);
-
-            if (property == null)
+            if (effectiveOwner != targetType &&
+                !property.IsAttached &&
+                !registry.IsRegistered(targetType, property))
             {
-                throw new XamlLoadException($"Could not find AvaloniaProperty '{ownerType.Name}.{propertyName}'.");
+                throw new XamlLoadException($"Property '{effectiveOwner.Name}.{propertyName}' is not registered on '{targetType}'.");
             }
 
             return property;
diff --git a/tests/Avalonia.Base.UnitTests/AvaloniaPropertyRegistryTests.cs b/tests/Avalonia.Base.UnitTests/AvaloniaPropertyRegistryTests.cs
index 8b2e500d37..c34e26ac5c 100644
--- a/tests/Avalonia.Base.UnitTests/AvaloniaPropertyRegistryTests.cs
+++ b/tests/Avalonia.Base.UnitTests/AvaloniaPropertyRegistryTests.cs
@@ -99,17 +99,6 @@ namespace Avalonia.Base.UnitTests
             Assert.Null(result);
         }
 
-        [Fact]
-        public void FindRegisteredAttached_Finds_Property()
-        {
-            var result = AvaloniaPropertyRegistry.Instance.FindRegisteredAttached(
-                typeof(Class1),
-                typeof(AttachedOwner),
-                "Attached");
-
-            Assert.Equal(AttachedOwner.AttachedProperty, result);
-        }
-
         private class Class1 : AvaloniaObject
         {
             public static readonly StyledProperty<string> FooProperty =
diff --git a/tests/Avalonia.Markup.Xaml.UnitTests/Converters/AvaloniaPropertyConverterTest.cs b/tests/Avalonia.Markup.Xaml.UnitTests/Converters/AvaloniaPropertyConverterTest.cs
index a028e246a4..f6f56231bc 100644
--- a/tests/Avalonia.Markup.Xaml.UnitTests/Converters/AvaloniaPropertyConverterTest.cs
+++ b/tests/Avalonia.Markup.Xaml.UnitTests/Converters/AvaloniaPropertyConverterTest.cs
@@ -10,6 +10,7 @@ using Xunit;
 using System.ComponentModel;
 using Portable.Xaml;
 using Portable.Xaml.Markup;
+using Avalonia.Controls;
 
 namespace Avalonia.Markup.Xaml.UnitTests.Converters
 {
@@ -26,7 +27,8 @@ namespace Avalonia.Markup.Xaml.UnitTests.Converters
         public void ConvertFrom_Finds_Fully_Qualified_Property()
         {
             var target = new AvaloniaPropertyTypeConverter();
-            var context = CreateContext();
+            var style = new Style(x => x.OfType<Class1>());
+            var context = CreateContext(style);
             var result = target.ConvertFrom(context, null, "Class1.Foo");
 
             Assert.Equal(Class1.FooProperty, result);
@@ -47,7 +49,8 @@ namespace Avalonia.Markup.Xaml.UnitTests.Converters
         public void ConvertFrom_Finds_Attached_Property()
         {
             var target = new AvaloniaPropertyTypeConverter();
-            var context = CreateContext();
+            var style = new Style(x => x.OfType<Class1>());
+            var context = CreateContext(style);
             var result = target.ConvertFrom(context, null, "AttachedOwner.Attached");
 
             Assert.Equal(AttachedOwner.AttachedProperty, result);
@@ -57,12 +60,37 @@ namespace Avalonia.Markup.Xaml.UnitTests.Converters
         public void ConvertFrom_Finds_Attached_Property_With_Parentheses()
         {
             var target = new AvaloniaPropertyTypeConverter();
-            var context = CreateContext();
+            var style = new Style(x => x.OfType<Class1>());
+            var context = CreateContext(style);
             var result = target.ConvertFrom(context, null, "(AttachedOwner.Attached)");
 
             Assert.Equal(AttachedOwner.AttachedProperty, result);
         }
 
+        [Fact]
+        public void ConvertFrom_Throws_For_Nonexistent_Property()
+        {
+            var target = new AvaloniaPropertyTypeConverter();
+            var style = new Style(x => x.OfType<Class1>());
+            var context = CreateContext(style);
+
+            var ex = Assert.Throws<XamlLoadException>(() => target.ConvertFrom(context, null, "Nonexistent"));
+
+            Assert.Equal("Could not find property 'Class1.Nonexistent'.", ex.Message);
+        }
+
+        [Fact]
+        public void ConvertFrom_Throws_For_Nonexistent_Attached_Property()
+        {
+            var target = new AvaloniaPropertyTypeConverter();
+            var style = new Style(x => x.OfType<Class1>());
+            var context = CreateContext(style);
+
+            var ex = Assert.Throws<XamlLoadException>(() => target.ConvertFrom(context, null, "AttachedOwner.NonExistent"));
+
+            Assert.Equal("Could not find property 'AttachedOwner.NonExistent'.", ex.Message);
+        }
+
         private ITypeDescriptorContext CreateContext(Style style = null)
         {
             var tdMock = new Mock<ITypeDescriptorContext>();
@@ -126,4 +154,4 @@ namespace Avalonia.Markup.Xaml.UnitTests.Converters
                 AvaloniaProperty.RegisterAttached<AttachedOwner, Class1, string>("Attached");
         }
     }
-}
\ No newline at end of file
+}
diff --git a/tests/Avalonia.Markup.Xaml.UnitTests/Xaml/StyleTests.cs b/tests/Avalonia.Markup.Xaml.UnitTests/Xaml/StyleTests.cs
index 4517aa6aa1..1d2f90d383 100644
--- a/tests/Avalonia.Markup.Xaml.UnitTests/Xaml/StyleTests.cs
+++ b/tests/Avalonia.Markup.Xaml.UnitTests/Xaml/StyleTests.cs
@@ -7,6 +7,7 @@ using Avalonia.Markup.Xaml.Styling;
 using Avalonia.Media;
 using Avalonia.Styling;
 using Avalonia.UnitTests;
+using Portable.Xaml;
 using Xunit;
 
 namespace Avalonia.Markup.Xaml.UnitTests.Xaml
@@ -172,5 +173,30 @@ namespace Avalonia.Markup.Xaml.UnitTests.Xaml
                 Assert.Equal(Dock.Right, DockPanel.GetDock(textBlock));
             }
         }
+
+        [Fact]
+        public void Disallows_Setting_Non_Registered_Property()
+        {
+            using (UnitTestApplication.Start(TestServices.StyledWindow))
+            {
+                var xaml = @"
+<Window xmlns='https://github.com/avaloniaui'
+        xmlns:x='http://schemas.microsoft.com/winfx/2006/xaml'
+        xmlns:local='clr-namespace:Avalonia.Markup.Xaml.UnitTests.Xaml;assembly=Avalonia.Markup.Xaml.UnitTests'>
+    <Window.Styles>
+        <Style Selector='TextBlock'>
+            <Setter Property='Button.IsDefault' Value='True'/>
+        </Style>
+    </Window.Styles>
+    <TextBlock/>
+</Window>";
+                var loader = new AvaloniaXamlLoader();
+                var ex = Assert.Throws<XamlObjectWriterException>(() => loader.Load(xaml));
+
+                Assert.Equal(
+                    "Property 'Button.IsDefault' is not registered on 'Avalonia.Controls.TextBlock'.",
+                    ex.InnerException.Message);
+            }
+        }
     }
 }