From 33679377f8a806d30357510cf8451a126dd6a4e1 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 19 May 2022 16:02:14 +0200 Subject: [PATCH] Validate presence of nesting selector. --- src/Avalonia.Base/Styling/ChildSelector.cs | 1 + .../Styling/DescendentSelector.cs | 1 + src/Avalonia.Base/Styling/NestingSelector.cs | 1 + src/Avalonia.Base/Styling/NotSelector.cs | 1 + src/Avalonia.Base/Styling/NthChildSelector.cs | 1 + src/Avalonia.Base/Styling/OrSelector.cs | 13 ++++++ .../Styling/PropertyEqualsSelector.cs | 1 + src/Avalonia.Base/Styling/Selector.cs | 2 + src/Avalonia.Base/Styling/Style.cs | 5 ++- src/Avalonia.Base/Styling/StyleChildren.cs | 6 +-- src/Avalonia.Base/Styling/TemplateSelector.cs | 1 + .../Styling/TypeNameAndClassSelector.cs | 1 + .../Styling/SelectorTests_Nesting.cs | 42 +++++++++++++++++++ 13 files changed, 71 insertions(+), 5 deletions(-) diff --git a/src/Avalonia.Base/Styling/ChildSelector.cs b/src/Avalonia.Base/Styling/ChildSelector.cs index 8675ca8820..34f3a76b61 100644 --- a/src/Avalonia.Base/Styling/ChildSelector.cs +++ b/src/Avalonia.Base/Styling/ChildSelector.cs @@ -65,5 +65,6 @@ namespace Avalonia.Styling } protected override Selector? MovePrevious() => null; + internal override bool HasValidNestingSelector() => _parent.HasValidNestingSelector(); } } diff --git a/src/Avalonia.Base/Styling/DescendentSelector.cs b/src/Avalonia.Base/Styling/DescendentSelector.cs index b164ad1c69..4ffaff6861 100644 --- a/src/Avalonia.Base/Styling/DescendentSelector.cs +++ b/src/Avalonia.Base/Styling/DescendentSelector.cs @@ -70,5 +70,6 @@ namespace Avalonia.Styling } protected override Selector? MovePrevious() => null; + internal override bool HasValidNestingSelector() => _parent.HasValidNestingSelector(); } } diff --git a/src/Avalonia.Base/Styling/NestingSelector.cs b/src/Avalonia.Base/Styling/NestingSelector.cs index 741eb7e9ca..f1c797c768 100644 --- a/src/Avalonia.Base/Styling/NestingSelector.cs +++ b/src/Avalonia.Base/Styling/NestingSelector.cs @@ -25,5 +25,6 @@ namespace Avalonia.Styling } protected override Selector? MovePrevious() => null; + internal override bool HasValidNestingSelector() => true; } } diff --git a/src/Avalonia.Base/Styling/NotSelector.cs b/src/Avalonia.Base/Styling/NotSelector.cs index c4beca6b9e..cdc3254d38 100644 --- a/src/Avalonia.Base/Styling/NotSelector.cs +++ b/src/Avalonia.Base/Styling/NotSelector.cs @@ -67,5 +67,6 @@ namespace Avalonia.Styling } protected override Selector? MovePrevious() => _previous; + internal override bool HasValidNestingSelector() => _argument.HasValidNestingSelector(); } } diff --git a/src/Avalonia.Base/Styling/NthChildSelector.cs b/src/Avalonia.Base/Styling/NthChildSelector.cs index cbb5e64772..047bf434da 100644 --- a/src/Avalonia.Base/Styling/NthChildSelector.cs +++ b/src/Avalonia.Base/Styling/NthChildSelector.cs @@ -105,6 +105,7 @@ namespace Avalonia.Styling } protected override Selector? MovePrevious() => _previous; + internal override bool HasValidNestingSelector() => _previous?.HasValidNestingSelector() ?? false; public override string ToString() { diff --git a/src/Avalonia.Base/Styling/OrSelector.cs b/src/Avalonia.Base/Styling/OrSelector.cs index a34712caaf..913c27bf0c 100644 --- a/src/Avalonia.Base/Styling/OrSelector.cs +++ b/src/Avalonia.Base/Styling/OrSelector.cs @@ -104,6 +104,19 @@ namespace Avalonia.Styling protected override Selector? MovePrevious() => null; + internal override bool HasValidNestingSelector() + { + foreach (var selector in _selectors) + { + if (!selector.HasValidNestingSelector()) + { + return false; + } + } + + return true; + } + private Type? EvaluateTargetType() { Type? result = null; diff --git a/src/Avalonia.Base/Styling/PropertyEqualsSelector.cs b/src/Avalonia.Base/Styling/PropertyEqualsSelector.cs index 34474fb7ab..7a37daf087 100644 --- a/src/Avalonia.Base/Styling/PropertyEqualsSelector.cs +++ b/src/Avalonia.Base/Styling/PropertyEqualsSelector.cs @@ -90,6 +90,7 @@ namespace Avalonia.Styling } protected override Selector? MovePrevious() => _previous; + internal override bool HasValidNestingSelector() => _previous?.HasValidNestingSelector() ?? false; internal static bool Compare(Type propertyType, object? propertyValue, object? value) { diff --git a/src/Avalonia.Base/Styling/Selector.cs b/src/Avalonia.Base/Styling/Selector.cs index 6f7f754b7b..1e06f3d375 100644 --- a/src/Avalonia.Base/Styling/Selector.cs +++ b/src/Avalonia.Base/Styling/Selector.cs @@ -86,6 +86,8 @@ namespace Avalonia.Styling /// protected abstract Selector? MovePrevious(); + internal abstract bool HasValidNestingSelector(); + private static SelectorMatch MatchUntilCombinator( IStyleable control, Selector start, diff --git a/src/Avalonia.Base/Styling/Style.cs b/src/Avalonia.Base/Styling/Style.cs index 7a83322355..a8707e00c0 100644 --- a/src/Avalonia.Base/Styling/Style.cs +++ b/src/Avalonia.Base/Styling/Style.cs @@ -186,8 +186,9 @@ namespace Avalonia.Styling if (parent?.Selector is not null) { if (Selector is null) - throw new InvalidOperationException("Nested styles must have a selector."); - // TODO: Validate that selector contains & in the right place. + throw new InvalidOperationException("Child styles must have a selector."); + if (!Selector.HasValidNestingSelector()) + throw new InvalidOperationException("Child styles must have a nesting selector."); } Parent = parent; diff --git a/src/Avalonia.Base/Styling/StyleChildren.cs b/src/Avalonia.Base/Styling/StyleChildren.cs index 64d838e4ce..5f8635f155 100644 --- a/src/Avalonia.Base/Styling/StyleChildren.cs +++ b/src/Avalonia.Base/Styling/StyleChildren.cs @@ -11,23 +11,23 @@ namespace Avalonia.Styling protected override void InsertItem(int index, IStyle item) { - base.InsertItem(index, item); (item as Style)?.SetParent(_owner); + base.InsertItem(index, item); } protected override void RemoveItem(int index) { var item = Items[index]; + (item as Style)?.SetParent(null); if (_owner.Owner is IResourceHost host) (item as IResourceProvider)?.RemoveOwner(host); - (item as Style)?.SetParent(null); base.RemoveItem(index); } protected override void SetItem(int index, IStyle item) { - base.SetItem(index, item); (item as Style)?.SetParent(_owner); + base.SetItem(index, item); if (_owner.Owner is IResourceHost host) (item as IResourceProvider)?.AddOwner(host); } diff --git a/src/Avalonia.Base/Styling/TemplateSelector.cs b/src/Avalonia.Base/Styling/TemplateSelector.cs index 6f0f9e0900..b0a2dae8d6 100644 --- a/src/Avalonia.Base/Styling/TemplateSelector.cs +++ b/src/Avalonia.Base/Styling/TemplateSelector.cs @@ -49,5 +49,6 @@ namespace Avalonia.Styling } protected override Selector? MovePrevious() => null; + internal override bool HasValidNestingSelector() => _parent?.HasValidNestingSelector() ?? false; } } diff --git a/src/Avalonia.Base/Styling/TypeNameAndClassSelector.cs b/src/Avalonia.Base/Styling/TypeNameAndClassSelector.cs index ae9b14635f..24d5d6bbbf 100644 --- a/src/Avalonia.Base/Styling/TypeNameAndClassSelector.cs +++ b/src/Avalonia.Base/Styling/TypeNameAndClassSelector.cs @@ -140,6 +140,7 @@ namespace Avalonia.Styling } protected override Selector? MovePrevious() => _previous; + internal override bool HasValidNestingSelector() => _previous?.HasValidNestingSelector() ?? false; private string BuildSelectorString() { diff --git a/tests/Avalonia.Base.UnitTests/Styling/SelectorTests_Nesting.cs b/tests/Avalonia.Base.UnitTests/Styling/SelectorTests_Nesting.cs index f7e8793ede..c5f779cbbb 100644 --- a/tests/Avalonia.Base.UnitTests/Styling/SelectorTests_Nesting.cs +++ b/tests/Avalonia.Base.UnitTests/Styling/SelectorTests_Nesting.cs @@ -219,6 +219,48 @@ namespace Avalonia.Base.UnitTests.Styling Assert.Throws(() => nested.Selector.Match(control, parent)); } + [Fact] + public void Adding_Child_With_No_Nesting_Selector_Fails() + { + var control = new Control1(); + var parent = new Style(x => x.OfType()); + var child = new Style(x => x.Class("foo")); + + Assert.Throws(() => parent.Children.Add(child)); + } + + [Fact] + public void Adding_Combinator_Selector_Child_With_No_Nesting_Selector_Fails() + { + var control = new Control1(); + var parent = new Style(x => x.OfType()); + var child = new Style(x => x.Class("foo").Descendant().Class("bar")); + + Assert.Throws(() => parent.Children.Add(child)); + } + + [Fact] + public void Adding_Or_Selector_Child_With_No_Nesting_Selector_Fails() + { + var control = new Control1(); + var parent = new Style(x => x.OfType()); + var child = new Style(x => Selectors.Or( + x.Nesting().Class("foo"), + x.Class("bar"))); + + Assert.Throws(() => parent.Children.Add(child)); + } + + [Fact] + public void Can_Add_Child_Without_Nesting_Selector_To_Style_Without_Selector() + { + var control = new Control1(); + var parent = new Style(); + var child = new Style(x => x.Class("foo")); + + parent.Children.Add(child); + } + public class Control1 : Control { }