Refactored selector matching.

Match selectors from left-to-right, as before we were checking things like property equality (and creating a `PropertyEqualsActivator`) before checking that the control is of the correct type. Also hopefully makes the selector matching logic more readable.

src/Avalonia.Styling/Styling/Activators/AndActivatorBuilder.cs

@@ -0,0 +1,36 @@
+#nullable enable
+
+namespace Avalonia.Styling.Activators
+{
+    internal struct AndActivatorBuilder
+    {
+        private IStyleActivator? _single;
+        private AndActivator? _multiple;
+
+        public void Add(IStyleActivator? activator)
+        {
+            if (activator == null)
+            {
+                return;
+            }
+
+            if (_single is null && _multiple is null)
+            {
+                _single = activator;
+            }
+            else
+            {
+                if (_multiple is null)
+                {
+                    _multiple = new AndActivator();
+                    _multiple.Add(_single!);
+                    _single = null;
+                }
+
+                _multiple.Add(activator);
+            }
+        }
+
+        public IStyleActivator Get() => _single ?? _multiple!;
+    }
+}

src/Avalonia.Styling/Styling/NotSelector.cs

@@ -42,7 +42,7 @@ namespace Avalonia.Styling
         {
             if (_selectorString == null)
             {
-                _selectorString = ":not(" + _argument.ToString() + ")";
+                _selectorString = $"{_previous?.ToString()}:not({_argument})";
             }
 
             return _selectorString;

src/Avalonia.Styling/Styling/Selector.cs

@@ -43,72 +43,28 @@ namespace Avalonia.Styling
         /// <returns>A <see cref="SelectorMatch"/>.</returns>
         public SelectorMatch Match(IStyleable control, bool subscribe = true)
         {
-            IStyleActivator? activator = null;
-            AndActivator? activators = null;
-            var selector = this;
-            var alwaysThisType = true;
-            var hitCombinator = false;
-
-            while (selector != null)
+            // First match the selector until a combinator is found. Selectors are stored from 
+            // right-to-left, so MatchUntilCombinator reverses this order because the type selector
+            // will be on the left.
+            var match = MatchUntilCombinator(control, this, subscribe, out var combinator);
+            
+            // If the pre-combinator selector matches, we can now match the combinator, if any.
+            if (match.IsMatch && combinator is object)
             {
-                hitCombinator |= selector.IsCombinator;
-
-                var match = selector.Evaluate(control, subscribe);
+                match = match.And(combinator.Match(control, subscribe));
 
-                if (!match.IsMatch)
-                {
-                    return hitCombinator ? SelectorMatch.NeverThisInstance : match;
-                }
-                else if (selector.InTemplate && control.TemplatedParent == null)
-                {
-                    return SelectorMatch.NeverThisInstance;
-                }
-                else if (match.Result == SelectorMatchResult.AlwaysThisInstance)
-                {
-                    alwaysThisType = false;
-                }
-                else if (match.Result == SelectorMatchResult.Sometimes)
+                // If we have a combinator then we can never say that we always match a control of
+                // this type, because by definition the combinator matches on things outside of the
+                // control.
+                match = match.Result switch
                 {
-                    if (match.Activator is null)
-                    {
-                        throw new AvaloniaInternalException(
-                            "SelectorMatch returned Sometimes but there is no activator.");
-                    }
-
-                    if (activator is null && activators is null)
-                    {
-                        activator = match.Activator;
-                    }
-                    else
-                    {
-                        if (activators is null)
-                        {
-                            activators = new AndActivator();
-                            activators.Add(activator!);
-                            activator = null;
-                        }
-
-                        activators.Add(match.Activator);
-                    }
-                }
-
-                selector = selector.MovePrevious();
+                    SelectorMatchResult.AlwaysThisType => SelectorMatch.AlwaysThisInstance,
+                    SelectorMatchResult.NeverThisType => SelectorMatch.NeverThisInstance,
+                    _ => match
+                };
             }
 
-            if (activators is object)
-            {
-                return new SelectorMatch(activators);
-            }
-            else if (activator is object)
-            {
-                return new SelectorMatch(activator);
-            }
-            else
-            {
-                return alwaysThisType && !hitCombinator ?
-                    SelectorMatch.AlwaysThisType :
-                    SelectorMatch.AlwaysThisInstance;
-            }
+            return match;
         }
 
         /// <summary>
@@ -126,5 +82,64 @@ namespace Avalonia.Styling
         /// Moves to the previous selector.
         /// </summary>
         protected abstract Selector? MovePrevious();
+
+        private static SelectorMatch MatchUntilCombinator(
+            IStyleable control,
+            Selector start,
+            bool subscribe,
+            out Selector? combinator)
+        {
+            combinator = null;
+
+            var activators = new AndActivatorBuilder();
+            var result = Match(control, start, subscribe, ref activators, ref combinator);
+
+            return result == SelectorMatchResult.Sometimes ?
+                new SelectorMatch(activators.Get()) :
+                new SelectorMatch(result);
+        }
+
+        private static SelectorMatchResult Match(
+            IStyleable control,
+            Selector selector,
+            bool subscribe,
+            ref AndActivatorBuilder activators,
+            ref Selector? combinator)
+        {
+            var previous = selector.MovePrevious();
+
+            // Selectors are stored from right-to-left, so we recurse into the selector in order to
+            // reverse this order, because the type selector will be on the left and is our best
+            // oppurtunity exit early.
+            if (previous != null && !previous.IsCombinator)
+            {
+                var previousMatch = Match(control, previous, subscribe, ref activators, ref combinator);
+
+                if (previousMatch < SelectorMatchResult.Sometimes)
+                {
+                    return previousMatch;
+                }
+            }
+
+            // Match this selector.
+            var match = selector.Evaluate(control, subscribe);
+
+            if (!match.IsMatch)
+            {
+                combinator = null;
+                return match.Result;
+            }
+            else if (match.Activator is object)
+            {
+                activators.Add(match.Activator!);
+            }
+
+            if (previous?.IsCombinator == true)
+            {
+                combinator = previous;
+            }
+
+            return match.Result;
+        }
     }
 }

src/Avalonia.Styling/Styling/SelectorMatch.cs

@@ -81,7 +81,11 @@ namespace Avalonia.Styling
             Activator = match;
         }
 
-        private SelectorMatch(SelectorMatchResult result)
+        /// <summary>
+        /// Initializes a new instance of the <see cref="SelectorMatch"/> class with the specified result.
+        /// </summary>
+        /// <param name="result">The match result.</param>
+        public SelectorMatch(SelectorMatchResult result)
         {
             Result = result;
             Activator = null;
@@ -102,5 +106,30 @@ namespace Avalonia.Styling
         /// change over time.
         /// </summary>
         public IStyleActivator? Activator { get; }
+
+        /// <summary>
+        /// Logical ANDs this <see cref="SelectorMatch"/> with another.
+        /// </summary>
+        /// <param name="other"></param>
+        /// <returns></returns>
+        public SelectorMatch And(in SelectorMatch other)
+        {
+            var result = (SelectorMatchResult)Math.Min((int)Result, (int)other.Result);
+
+            if (result == SelectorMatchResult.Sometimes)
+            {
+                var activators = new AndActivatorBuilder();
+                activators.Add(Activator);
+                activators.Add(other.Activator);
+                return new SelectorMatch(activators.Get());
+            }
+            else
+            {
+                return new SelectorMatch(result);
+            }
+        }
+
+        /// <inheritdoc/>
+        public override string ToString() => Result.ToString();
     }
 }

src/Avalonia.Styling/Styling/TemplateSelector.cs

@@ -41,12 +41,11 @@ namespace Avalonia.Styling
 
         protected override SelectorMatch Evaluate(IStyleable control, bool subscribe)
         {
-            IStyleable templatedParent = control.TemplatedParent as IStyleable;
+            var templatedParent = control.TemplatedParent as IStyleable;
 
             if (templatedParent == null)
             {
-                throw new InvalidOperationException(
-                    "Cannot call Template selector on control with null TemplatedParent.");
+                return SelectorMatch.NeverThisInstance;
             }
 
             return _parent.Match(templatedParent, subscribe);

tests/Avalonia.Styling.UnitTests/SelectorTests_Multiple.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using System.Threading.Tasks;
 using Avalonia.Controls;
 using Avalonia.Controls.Templates;
 using Avalonia.VisualTree;
@@ -152,6 +153,31 @@ namespace Avalonia.Styling.UnitTests
             Assert.Equal(new[] { false, true, false }, values);
         }
 
+        [Fact]
+        public async Task Nested_PropertyEquals()
+        {
+            var control = new Canvas();
+            var parent = new Border { Child = control };
+
+            var target = default(Selector)
+                .OfType<Border>()
+                .PropertyEquals(Border.TagProperty, "foo")
+                .Child()
+                .OfType<Canvas>()
+                .PropertyEquals(Canvas.TagProperty, "bar");
+
+            var match = target.Match(control);
+            Assert.Equal(SelectorMatchResult.Sometimes, match.Result);
+
+            var activator = match.Activator;
+
+            Assert.False(await activator.Take(1));
+            control.Tag = "bar";
+            Assert.False(await activator.Take(1));
+            parent.Tag = "foo";
+            Assert.True(await activator.Take(1));
+        }
+
         [Fact]
         public void TargetType_OfType()
         {