Fixed Selector.ValidateNestingSelector not calling overrides when iterating up through parent selectors (#19947)

* Fixed Selector.ValidateNestingSelector not calling overrides when iterating up through parent selectors

Changed ValidateNestingSelector to call recursively, walking up
the hierarchy of parent selectors. This means function overrides
are taken into account when walking up the tree. Now if a
selector has an OrSelector as its parent, it doesn't throw an
exception.

* Updated ToString methods to surround OrSelectors with parenthesis where useful

src/Avalonia.Base/Styling/ChildSelector.cs

@@ -31,7 +31,7 @@ namespace Avalonia.Styling
         {
             if (_selectorString == null)
             {
-                _selectorString = _parent.ToString(owner) + " > ";
+                _selectorString = _parent.ToString(owner, true) + " > ";
             }
 
             return _selectorString;

src/Avalonia.Base/Styling/DescendentSelector.cs

@@ -29,7 +29,7 @@ namespace Avalonia.Styling
         {
             if (_selectorString == null)
             {
-                _selectorString = _parent.ToString(owner) + ' ';
+                _selectorString = _parent.ToString(owner, true) + ' ';
             }
 
             return _selectorString;

src/Avalonia.Base/Styling/NotSelector.cs

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

src/Avalonia.Base/Styling/NthChildSelector.cs

@@ -109,9 +109,9 @@ namespace Avalonia.Styling
         public override string ToString(Style? owner)
         {
             var expectedCapacity = NthLastChildSelectorName.Length + 8;
-            var stringBuilder =  StringBuilderCache.Acquire(expectedCapacity);
+            var stringBuilder = StringBuilderCache.Acquire(expectedCapacity);
-            stringBuilder.Append(_previous?.ToString(owner));
+            stringBuilder.Append(_previous?.ToString(owner, true));
-            
+
             stringBuilder.Append(':');
             stringBuilder.Append(_reversed ? NthLastChildSelectorName : NthChildSelectorName);
             stringBuilder.Append('(');

src/Avalonia.Base/Styling/OrSelector.cs

@@ -45,11 +45,19 @@ namespace Avalonia.Styling
         internal override Type? TargetType => _targetType ??= EvaluateTargetType();
 
         /// <inheritdoc/>
-        public override string ToString(Style? owner)
+        public override string ToString(Style? owner) => ToString(owner, false);
+
+        /// <inheritdoc/>
+        internal override string ToString(Style? owner, bool hasNext)
         {
             if (_selectorString == null)
             {
-                _selectorString = string.Join(", ", _selectors.Select(x => x.ToString(owner)));
+                _selectorString = string.Join(", ", _selectors.Select(x => x.ToString(owner, true)));
+
+                if (hasNext)
+                {
+                    _selectorString = $"({_selectorString})";
+                }
             }
 
             return _selectorString;
@@ -97,13 +105,13 @@ namespace Avalonia.Styling
         private protected override Selector? MovePrevious() => null;
         private protected override Selector? MovePreviousOrParent() => null;
 
-        internal override void ValidateNestingSelector(bool inControlTheme)
+        internal override void ValidateNestingSelector(bool inControlTheme, int templateCount = 0)
         {
             var count = _selectors.Count;
 
             for (var i = 0; i < count; i++)
             {
-                _selectors[i].ValidateNestingSelector(inControlTheme);
+                _selectors[i].ValidateNestingSelector(inControlTheme, templateCount);
             }
         }
 

src/Avalonia.Base/Styling/PropertyEqualsSelector.cs

@@ -45,7 +45,7 @@ namespace Avalonia.Styling
 
                 if (_previous != null)
                 {
-                    builder.Append(_previous.ToString(owner));
+                    builder.Append(_previous.ToString(owner, true));
                 }
 
                 builder.Append('[');
@@ -85,7 +85,6 @@ namespace Avalonia.Styling
                     ? SelectorMatch.AlwaysThisInstance
                     : SelectorMatch.NeverThisInstance;
             }
-            
         }
 
         private protected override Selector? MovePrevious() => _previous;

src/Avalonia.Base/Styling/Selector.cs

@@ -47,7 +47,7 @@ namespace Avalonia.Styling
             // right-to-left, so MatchUntilCombinator reverses this order because the type selector
             // will be on the left.
             var match = MatchUntilCombinator(control, this, parent, subscribe, out var combinator);
-            
+
             // If the pre-combinator selector matches, we can now match the combinator, if any.
             if (match.IsMatch && combinator is object)
             {
@@ -76,6 +76,10 @@ namespace Avalonia.Styling
         /// <param name="owner">The owner style.</param>
         public abstract string ToString(Style? owner);
 
+        /// <inheritdoc cref="ToString(Style?)"/>
+        /// <param name="hasNext">Whether there is a selector that comes after this one.</param>
+        internal virtual string ToString(Style? owner, bool hasNext) => ToString(owner);
+
         /// <summary>
         /// Evaluates the selector for a match.
         /// </summary>
@@ -100,30 +104,31 @@ namespace Avalonia.Styling
         /// </summary>
         private protected abstract Selector? MovePreviousOrParent();
 
-        internal virtual void ValidateNestingSelector(bool inControlTheme)
+        internal virtual void ValidateNestingSelector(bool inControlTheme, int templateCount = 0)
         {
             var s = this;
-            var templateCount = 0;
 
-            do
+            if (inControlTheme)
             {
-                if (inControlTheme)
+                if (!s.InTemplate && s.IsCombinator)
-                {
+                    throw new InvalidOperationException(
-                    if (!s.InTemplate && s.IsCombinator)
+                        "ControlTheme style may not directly contain a child or descendent selector.");
-                        throw new InvalidOperationException(
+                if (s is TemplateSelector && templateCount++ > 0)
-                            "ControlTheme style may not directly contain a child or descendent selector.");
+                    throw new InvalidOperationException(
-                    if (s is TemplateSelector && templateCount++ > 0)
+                        "ControlTemplate styles cannot contain multiple template selectors.");
-                        throw new InvalidOperationException(
+            }
-                            "ControlTemplate styles cannot contain multiple template selectors.");
-                }
 
-                var previous = s.MovePreviousOrParent();
+            var previous = s.MovePreviousOrParent();
 
-                if (previous is null && s is not NestingSelector)
+            if (previous is null)
+            {
+                if (s is not NestingSelector)
                     throw new InvalidOperationException("Child styles must have a nesting selector.");
-
+            }
-                s = previous;
+            else
-            } while (s is not null);
+            {
+                previous.ValidateNestingSelector(inControlTheme, templateCount);
+            }
         }
 
         private static SelectorMatch MatchUntilCombinator(

src/Avalonia.Base/Styling/Selectors.cs

@@ -1,6 +1,5 @@
 using System;
 using System.Collections.Generic;
-using System.Linq;
 
 namespace Avalonia.Styling
 {
@@ -124,7 +123,7 @@ namespace Avalonia.Styling
         {
             return new NotSelector(previous, argument(null));
         }
-        
+
         /// <summary>
         /// Returns a selector which inverts the results of selector argument.
         /// </summary>

src/Avalonia.Base/Styling/TemplateSelector.cs

@@ -30,7 +30,7 @@ namespace Avalonia.Styling
         {
             if (_selectorString == null)
             {
-                _selectorString = _parent.ToString(owner) + " /template/ ";
+                _selectorString = _parent.ToString(owner, true) + " /template/ ";
             }
 
             return _selectorString;

src/Avalonia.Base/Styling/TypeNameAndClassSelector.cs

@@ -143,7 +143,7 @@ namespace Avalonia.Styling
 
             if (_previous != null)
             {
-                builder.Append(_previous.ToString(owner));
+                builder.Append(_previous.ToString(owner, true));
             }
 
             if (TargetType != null)

tests/Avalonia.Base.UnitTests/Styling/SelectorTests_Or.cs

@@ -1,3 +1,4 @@
+using System;
 using Avalonia.Controls;
 using Avalonia.Styling;
 using Xunit;
@@ -89,6 +90,24 @@ namespace Avalonia.Base.UnitTests.Styling
 
             Assert.Equal(null, target.TargetType);
         }
+        
+        [Fact]
+        public void ValidateNestingSelector_Checks_Children_When_Parent_Is_An_OrSelector()
+        {
+            var target = Selectors.Or(
+                default(Selector).Class("foo"),
+                default(Selector).Class("bar")
+            ).Name("baz");
+
+            Assert.Throws<InvalidOperationException>(() => target.ValidateNestingSelector(false));
+            
+            target = Selectors.Or(
+                default(Selector).Nesting().Class("foo"),
+                default(Selector).Nesting().Class("bar")
+            ).Name("baz");
+            
+            target.ValidateNestingSelector(false);
+        }
 
         public class Control1 : Control
         {