Keep SelectingItemsControl selection values until ItemsSource is set (#18634)

* Add failing tests for SelectedItem/SelectedIndex without an ItemsSource

* Keep SelectedItem/SelectedIndex until ItemsSource is set

* Add failing tests for setting SelectedValue without an ItemsSource

* Keep SelectedValue until ItemsSource is set

src/Avalonia.Controls/ItemCollection.cs

@@ -11,15 +11,10 @@ namespace Avalonia.Controls
     /// </summary>
     public class ItemCollection : ItemsSourceView, IList
     {
-// Suppress "Avoid zero-length array allocations": This is a sentinel value and must be unique.
-#pragma warning disable CA1825
-        private static readonly object?[] s_uninitialized = new object?[0];
-#pragma warning restore CA1825
-
         private Mode _mode;
 
         internal ItemCollection()
-            : base(s_uninitialized)
+            : base(UninitializedSource)
         {
         }
 
@@ -100,7 +95,7 @@ namespace Avalonia.Controls
             {
                 if (IsReadOnly)
                     ThrowIsItemsSource();
-                if (Source == s_uninitialized)
+                if (Source == UninitializedSource)
                     SetSource(CreateDefaultCollection());
                 return Source;
             }

src/Avalonia.Controls/ItemsSourceView.cs

@@ -27,6 +27,13 @@ namespace Avalonia.Controls
         /// </summary>
         public static ItemsSourceView Empty { get; } = new ItemsSourceView(Array.Empty<object?>());
 
+        /// <summary>
+        /// Gets an instance representing an uninitialized source.
+        /// </summary>
+        [SuppressMessage("Performance", "CA1825:Avoid zero-length array allocations", Justification = "This is a sentinel value and must be unique.")]
+        [SuppressMessage("ReSharper", "UseCollectionExpression", Justification = "This is a sentinel value and must be unique.")]
+        internal static object?[] UninitializedSource { get; } = new object?[0];
+
         private IList _source;
         private NotifyCollectionChangedEventHandler? _collectionChanged;
         private NotifyCollectionChangedEventHandler? _preCollectionChanged;
@@ -49,6 +56,9 @@ namespace Avalonia.Controls
         /// </summary>
         public IList Source => _source;
 
+        internal IList? TryGetInitializedSource()
+            => _source == UninitializedSource ? null : _source;
+
         /// <summary>
         /// Retrieves the item at the specified index.
         /// </summary>

src/Avalonia.Controls/Primitives/SelectingItemsControl.cs

@@ -592,10 +592,7 @@ namespace Avalonia.Controls.Primitives
         {
             base.OnInitialized();
 
-            if (_selection is object)
-            {
-                _selection.Source = ItemsView.Source;
-            }
+            TryInitializeSelectionSource(_selection, _updateState is null);
         }
 
         /// <inheritdoc />
@@ -896,8 +893,8 @@ namespace Avalonia.Controls.Primitives
 
         private void OnItemsViewSourceChanged(object? sender, EventArgs e)
         {
-            if (_selection is not null && _updateState is null)
-                _selection.Source = ItemsView.Source;
+            if (_updateState is null)
+                TryInitializeSelectionSource(_selection, true);
         }
 
         /// <summary>
@@ -1202,7 +1199,7 @@ namespace Avalonia.Controls.Primitives
         {
             if (_updateState is null)
             {
-                model.Source = ItemsView.Source;
+                TryInitializeSelectionSource(model, false);
             }
 
             model.PropertyChanged += OnSelectionModelPropertyChanged;
@@ -1237,6 +1234,32 @@ namespace Avalonia.Controls.Primitives
             }
         }
 
+        private void TryInitializeSelectionSource(ISelectionModel? selection, bool shouldSelectItemFromSelectedValue)
+        {
+            if (selection is not null && ItemsView.TryGetInitializedSource() is { } source)
+            {
+                // InternalSelectionModel keeps the SelectedIndex and SelectedItem values before the ItemsSource is set.
+                // However, SelectedValue isn't part of that model, so we have to set the SelectedItem from
+                // SelectedValue manually now that we have a source.
+                //
+                // While this works, this is messy: we effectively have "lazy selection initialization" in 3 places:
+                //  - UpdateState (all selection properties, for BeginInit/EndInit)
+                //  - InternalSelectionModel (SelectedIndex/SelectedItem)
+                //  - SelectedItemsControl (SelectedValue)
+                //
+                // There's the opportunity to have a single place responsible for this logic.
+                // TODO12 (or 13): refactor this.
+                if (shouldSelectItemFromSelectedValue && selection.SelectedIndex == -1 && selection.SelectedItem is null)
+                {
+                    var item = FindItemWithValue(SelectedValue);
+                    if (item != AvaloniaProperty.UnsetValue)
+                        selection.SelectedItem = item;
+                }
+
+                selection.Source = source;
+            }
+        }
+
         private void DeinitializeSelectionModel(ISelectionModel? model)
         {
             if (model is object)
@@ -1266,7 +1289,7 @@ namespace Avalonia.Controls.Primitives
 
                 if (_selection is InternalSelectionModel s)
                 {
-                    s.Update(ItemsView.Source, state.SelectedItems);
+                    s.Update(ItemsView.TryGetInitializedSource(), state.SelectedItems);
                 }
                 else
                 {
@@ -1275,7 +1298,7 @@ namespace Avalonia.Controls.Primitives
                         SelectedItems = state.SelectedItems.Value;
                     }
 
-                    Selection.Source = ItemsView.Source;
+                    TryInitializeSelectionSource(Selection, false);
                 }
 
                 if (state.SelectedValue.HasValue)

tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs

@@ -441,7 +441,7 @@ namespace Avalonia.Controls.UnitTests.Primitives
         }
 
         [Fact]
-        public void Setting_SelectedIndex_Out_Of_Bounds_Should_Clear_Selection()
+        public void Setting_SelectedIndex_Out_Of_Bounds_With_ItemsSource_Should_Clear_Selection()
         {
             var items = new[]
             {
@@ -462,11 +462,50 @@ namespace Avalonia.Controls.UnitTests.Primitives
         }
 
         [Fact]
-        public void Setting_SelectedItem_To_Non_Existent_Item_Should_Clear_Selection()
+        public void Setting_SelectedIndex_Out_Of_Bounds_Without_ItemsSource_Should_Keep_Selection_Until_ItemsSource_Is_Set()
         {
             var target = new SelectingItemsControl
             {
-                Template = Template(),
+                Template = Template()
+            };
+
+            target.ApplyTemplate();
+            target.SelectedIndex = 2;
+
+            Assert.Equal(2, target.SelectedIndex);
+
+            target.ItemsSource = Array.Empty<Item>();
+
+            Assert.Equal(-1, target.SelectedIndex);
+        }
+
+        [Fact]
+        public void Setting_SelectedIndex_Without_ItemsSource_Should_Keep_Selection_If_Index_Exists_When_ItemsSource_IsSet()
+        {
+            var target = new SelectingItemsControl
+            {
+                Template = Template()
+            };
+
+            target.ApplyTemplate();
+            target.SelectedIndex = 2;
+
+            Assert.Equal(2, target.SelectedIndex);
+
+            var items = new Item[] { new(), new(), new(), new() };
+            target.ItemsSource = items;
+
+            Assert.Equal(2, target.SelectedIndex);
+            Assert.Same(items[2], target.SelectedItem);
+        }
+
+        [Fact]
+        public void Setting_SelectedItem_To_Non_Existent_Item_With_ItemsSource_Should_Clear_Selection()
+        {
+            var target = new SelectingItemsControl
+            {
+                ItemsSource = Array.Empty<Item>(),
+                Template = Template()
             };
 
             target.ApplyTemplate();
@@ -476,6 +515,50 @@ namespace Avalonia.Controls.UnitTests.Primitives
             Assert.Null(target.SelectedItem);
         }
 
+        [Fact]
+        public void Setting_SelectedItem_To_Non_Existent_Item_Without_ItemsSource_Should_Keep_Selection_Until_ItemsSource_Is_Set()
+        {
+            var item = new Item();
+
+            var target = new SelectingItemsControl
+            {
+                Template = Template()
+            };
+
+            target.ApplyTemplate();
+            target.SelectedItem = item;
+
+            Assert.Equal(-1, target.SelectedIndex);
+            Assert.Same(item, target.SelectedItem);
+
+            target.ItemsSource = Array.Empty<Item>();
+
+            Assert.Equal(-1, target.SelectedIndex);
+            Assert.Null(target.SelectedItem);
+        }
+
+        [Fact]
+        public void Setting_SelectedItem_Without_ItemsSource_Should_Keep_Selection_If_Item_Exists_When_ItemsSource_IsSet()
+        {
+            var item = new Item();
+
+            var target = new SelectingItemsControl
+            {
+                Template = Template()
+            };
+
+            target.ApplyTemplate();
+            target.SelectedItem = item;
+
+            Assert.Equal(-1, target.SelectedIndex);
+            Assert.Same(item, target.SelectedItem);
+
+            target.ItemsSource = new[] { new(), new(), item, new() };
+
+            Assert.Equal(2, target.SelectedIndex);
+            Assert.Same(item, target.SelectedItem);
+        }
+
         [Fact]
         public void Adding_Selected_Item_Should_Update_Selection()
         {

tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests_SelectedValue.cs

@@ -170,6 +170,53 @@ namespace Avalonia.Controls.UnitTests.Primitives
             Assert.Equal(items[2].Name, sic.SelectedValue);
         }
 
+        [Fact]
+        public void Setting_SelectedValue_To_Non_Existent_Item_Without_ItemsSource_Should_Keep_Selection_Until_ItemsSource_Is_Set()
+        {
+            var target = new SelectingItemsControl
+            {
+                Template = Template(),
+                SelectedValueBinding = new Binding("Name")
+            };
+
+            target.ApplyTemplate();
+            target.SelectedValue = "Item2";
+
+            Assert.Equal(-1, target.SelectedIndex);
+            Assert.Null(target.SelectedItem);
+            Assert.Same("Item2", target.SelectedValue);
+
+            target.ItemsSource = Array.Empty<TestClass>();
+
+            Assert.Equal(-1, target.SelectedIndex);
+            Assert.Null(target.SelectedItem);
+            Assert.Null(target.SelectedValue);
+        }
+
+        [Fact]
+        public void Setting_SelectedValue_Without_ItemsSource_Should_Keep_Selection_If_Item_Exists_When_ItemsSource_IsSet()
+        {
+            var target = new SelectingItemsControl
+            {
+                Template = Template(),
+                SelectedValueBinding = new Binding("Name")
+            };
+
+            target.ApplyTemplate();
+            target.SelectedValue = "Item2";
+
+            Assert.Equal(-1, target.SelectedIndex);
+            Assert.Null(target.SelectedItem);
+            Assert.Same("Item2", target.SelectedValue);
+
+            var items = TestClass.GetItems();
+            target.ItemsSource = items;
+
+            Assert.Equal(2, target.SelectedIndex);
+            Assert.Same(items[2], target.SelectedItem);
+            Assert.Equal("Item2", target.SelectedValue);
+        }
+
         [Fact]
         public void Setting_SelectedValue_During_Initialize_Should_Take_Priority_Over_Previous_Value()
         {