Add convenience / performance related methods to ListViewItemCollection / ControlCollection

ListViewItemCollection doesn't have an AddRange that takes an IEnumerable<ListViewItem>, which necessitates unnecessary array allocations when adding from a collection.

It also doesn't implement a generic IList<T> which makes typed usage and LINQ usage difficult. This adds that.

Also add IEnumerable<Control> to ControlCollection to address the LINQ scenario. Control has IList, but indexed setting throws. I don't want to add IList<Control> until we've had time to evaluate the implications of implicitly doing the public steps with the collection that replicate what those setters should be doing.

Also tweak ArrangedElementCollection so the static empty collection can't be written to.

This doesn't fix the ambiguity of `[..]`, but it does avoid the need for it in some cases.

Author: JeremyKuhne

src/System.Windows.Forms/src/GlobalSuppressions.cs

@@ -18,6 +18,8 @@
 [assembly: SuppressMessage("Naming", "CA1725:Parameter names should match base declaration", Justification = "Public API", Scope = "member", Target = "~M:System.Windows.Forms.FontDialog.RunDialog(System.IntPtr)~System.Boolean")]
 [assembly: SuppressMessage("Naming", "CA1725:Parameter names should match base declaration", Justification = "Public API", Scope = "member", Target = "~M:System.Windows.Forms.ImageListStreamer.GetObjectData(System.Runtime.Serialization.SerializationInfo,System.Runtime.Serialization.StreamingContext)")]
 [assembly: SuppressMessage("Naming", "CA1725:Parameter names should match base declaration", Justification = "Public API", Scope = "member", Target = "~M:System.Windows.Forms.Control.EndInvoke(System.IAsyncResult)~System.Object")]
+[assembly: SuppressMessage("Naming", "CA1725:Parameter names should match base declaration", Justification = "Public API", Scope = "member", Target = "~M:System.Windows.Forms.Control.ControlCollection.IndexOf(System.Windows.Forms.Control)~System.Int32")]
+[assembly: SuppressMessage("Naming", "CA1725:Parameter names should match base declaration", Justification = "Public API", Scope = "member", Target = "~M:System.Windows.Forms.ListView.ListViewItemCollection.CopyTo(System.Array,System.Int32)")]
 [assembly: SuppressMessage("Style", "IDE1006:Naming Styles", Justification = "Public API", Scope = "member", Target = "~F:System.Windows.Forms.FontDialog.EventApply")]
 [assembly: SuppressMessage("Style", "IDE1006:Naming Styles", Justification = "Public API", Scope = "member", Target = "~F:System.Windows.Forms.FileDialog.EventFileOk")]
 [assembly: SuppressMessage("Performance", "CA1815:Override equals and operator equals on value types", Justification = "Public API", Scope = "type", Target = "~T:System.Windows.Forms.ImeModeConversion")]

src/System.Windows.Forms/src/PublicAPI.Unshipped.txt

@@ -0,0 +1,2 @@
+System.Windows.Forms.ListView.ListViewItemCollection.AddRange(System.Collections.Generic.IEnumerable<System.Windows.Forms.ListViewItem!>! collection) -> void
+System.Windows.Forms.ListView.ListViewItemCollection.CopyTo(System.Windows.Forms.ListViewItem![]! array, int arrayIndex) -> void

src/System.Windows.Forms/src/System/Windows/Forms/Control.ControlCollection.cs

@@ -10,10 +10,10 @@ namespace System.Windows.Forms;
 public partial class Control
 {
     /// <summary>
-    ///  Collection of controls...
+    ///  Collection of controls.
     /// </summary>
     [ListBindable(false)]
-    public partial class ControlCollection : ArrangedElementCollection, IList, ICloneable
+    public partial class ControlCollection : ArrangedElementCollection, IList, IEnumerable<Control>, ICloneable
     {
         ///  A caching mechanism for key accessor
         ///  We use an index here rather than control so that we don't have lifetime
@@ -29,10 +29,7 @@ public ControlCollection(Control owner)
         /// <summary>
         ///  Returns true if the collection contains an item with the specified key, false otherwise.
         /// </summary>
-        public virtual bool ContainsKey(string? key)
-        {
-            return IsValidIndex(IndexOfKey(key));
-        }
+        public virtual bool ContainsKey(string? key) => IsValidIndex(IndexOfKey(key));
 
         /// <summary>
         ///  Adds a child control to this control. The control becomes the last control in
@@ -156,11 +153,23 @@ object ICloneable.Clone()
         {
             // Use CreateControlInstance so we get the same type of ControlCollection, but whack the
             // owner so adding controls to this new collection does not affect the control we cloned from.
-            ControlCollection ccOther = Owner.CreateControlsInstance();
+            ControlCollection clone = Owner.CreateControlsInstance();
 
             // We add using InnerList to prevent unnecessary parent cycle checks, etc.
-            ccOther.InnerList.AddRange(InnerList);
-            return ccOther;
+            if (clone.InnerList is List<IArrangedElement> list)
+            {
+                list.AddRange(InnerList);
+            }
+            else
+            {
+                Debug.Fail("Unexpected InnerList type");
+                foreach (Control c in this)
+                {
+                    clone.Add(c);
+                }
+            }
+
+            return clone;
         }
 
         public bool Contains(Control? control) => ((IList)InnerList).Contains(control);
@@ -224,10 +233,7 @@ private static void FindInternal(string key, bool searchAllChildren, ControlColl
             }
         }
 
-        public override IEnumerator GetEnumerator()
-        {
-            return new ControlCollectionEnumerator(this);
-        }
+        public override IEnumerator GetEnumerator() => new ControlCollectionEnumerator(this);
 
         public int IndexOf(Control? control) => ((IList)InnerList).IndexOf(control);
 
@@ -269,10 +275,7 @@ public virtual int IndexOfKey(string? key)
         /// <summary>
         ///  Determines if the index is valid for the collection.
         /// </summary>
-        private bool IsValidIndex(int index)
-        {
-            return ((index >= 0) && (index < Count));
-        }
+        private bool IsValidIndex(int index) => (index >= 0) && (index < Count);
 
         /// <summary>
         ///  Who owns this control collection.
@@ -285,10 +288,9 @@ private bool IsValidIndex(int index)
         /// </summary>
         public virtual void Remove(Control? value)
         {
-            // Sanity check parameter
             if (value is null)
             {
-                return;     // Don't do anything
+                return;
             }
 
             if (value.ParentInternal == Owner)
@@ -317,10 +319,7 @@ void IList.Remove(object? control)
             }
         }
 
-        public void RemoveAt(int index)
-        {
-            Remove(this[index]);
-        }
+        public void RemoveAt(int index) => Remove(this[index]);
 
         /// <summary>
         ///  Removes the child control with the specified key.
@@ -394,7 +393,7 @@ public virtual void Clear()
         ///  Retrieves the index of the specified child control in this array. An ArgumentException
         ///  is thrown if child is not parented to this Control.
         /// </summary>
-        public int GetChildIndex(Control child) => GetChildIndex(child, true);
+        public int GetChildIndex(Control child) => GetChildIndex(child, throwException: true);
 
         /// <summary>
         ///  Retrieves the index of the specified child control in this array. An ArgumentException
@@ -444,5 +443,13 @@ internal virtual void SetChildIndexInternal(Control child, int newIndex)
         /// </summary>
         public virtual void SetChildIndex(Control child, int newIndex) =>
             SetChildIndexInternal(child, newIndex);
+
+        IEnumerator<Control> IEnumerable<Control>.GetEnumerator()
+        {
+            foreach (Control control in InnerList)
+            {
+                yield return control;
+            }
+        }
     }
 }

src/System.Windows.Forms/src/System/Windows/Forms/Control.cs

@@ -4773,20 +4773,14 @@ public bool Contains([NotNullWhen(true)] Control? ctl)
     ///  should not call base.CreateAccessibilityObject.
     /// </summary>
     [EditorBrowsable(EditorBrowsableState.Advanced)]
-    protected virtual AccessibleObject CreateAccessibilityInstance()
-    {
-        return new ControlAccessibleObject(this);
-    }
+    protected virtual AccessibleObject CreateAccessibilityInstance() => new ControlAccessibleObject(this);
 
     /// <summary>
     ///  Constructs the new instance of the Controls collection objects. Subclasses
     ///  should not call base.CreateControlsInstance.
     /// </summary>
     [EditorBrowsable(EditorBrowsableState.Advanced)]
-    protected virtual ControlCollection CreateControlsInstance()
-    {
-        return new ControlCollection(this);
-    }
+    protected virtual ControlCollection CreateControlsInstance() => new ControlCollection(this);
 
     /// <summary>
     ///  Creates a Graphics for this control. The control's brush, font, foreground