Avoid boxing when enumerating project xml children

In a recent profile of a graph construction, it was observed that a
large amount of boxing was happening for
ProjectElementSiblingEnumerable. This change simplifies how xml children
are enumerated by adding an internal ChildrenEnumerable property which
directly exposes the ProjectElementSiblingEnumerable which should avoid
boxing, at least in some code paths (the public API makes it hard to
avoid everywhere...).

Additionally, a very common usage of enumerating children was to do
Children.OfType<T> and wrap it in a ReadOnlyCollection<T>, so I
introduced a GetChildrenOfType (and GetChildrenReversedOfType) method
which exposes an ICollection<T> which does the same thing but without
the boxing of ProjectElementSiblingEnumerable and without the OfType
class. It's just 1 collection allocation.

Author: dfederm

src/BannedSymbols.txt

@@ -1 +1,2 @@
 M:System.Globalization.CompareInfo.IndexOf(System.String,System.Char);CompareInfo.IndexOf can unexpectedly allocate strings--use string.IndexOf
+P:Microsoft.Build.Construction.ProjectElementContainer.Children;Use ChildrenEnumerable instead to avoid boxing

src/Build.OM.UnitTests/Construction/ProjectImportElement_Tests.cs

@@ -28,7 +28,7 @@ public void ReadNone()
         {
             ProjectRootElement project = ProjectRootElement.Create();
 
-            Assert.Null(project.Imports.GetEnumerator().Current);
+            Assert.Empty(project.Imports);
         }
 
         /// <summary>

src/Build.OM.UnitTests/Construction/ProjectImportGroupElement_Tests.cs

@@ -143,7 +143,7 @@ public void ReadNone()
         {
             ProjectRootElement project = ProjectRootElement.Create();
 
-            Assert.Null(project.Imports.GetEnumerator().Current);
+            Assert.Empty(project.Imports);
         }
 
         /// <summary>
@@ -162,7 +162,7 @@ public void ReadNoChild()
 
             ProjectImportGroupElement importGroup = (ProjectImportGroupElement)Helpers.GetFirst(project.ImportGroups);
 
-            Assert.Null(project.Imports.GetEnumerator().Current);
+            Assert.Empty(project.Imports);
             Assert.Equal(0, Helpers.Count(importGroup.Imports));
         }
 

src/Build.OM.UnitTests/Construction/ProjectItemDefinitionGroupElement_Tests.cs

@@ -24,7 +24,7 @@ public void ReadNone()
         {
             ProjectRootElement project = ProjectRootElement.Create();
             Assert.Equal(0, Helpers.Count(project.Children));
-            Assert.Null(project.ItemDefinitionGroups.GetEnumerator().Current);
+            Assert.Empty(project.ItemDefinitionGroups);
         }
 
         /// <summary>

src/Build.OM.UnitTests/Construction/ProjectItemGroupElement_tests.cs

@@ -24,7 +24,7 @@ public void ReadNoItemGroup()
         {
             ProjectRootElement project = ProjectRootElement.Create();
             Assert.Equal(0, Helpers.Count(project.Children));
-            Assert.Null(project.ItemGroups.GetEnumerator().Current);
+            Assert.Empty(project.ItemGroups);
         }
 
         /// <summary>

src/Build.OM.UnitTests/Construction/ProjectPropertyGroupElement_Tests.cs

@@ -23,7 +23,7 @@ public void ReadNoPropertyGroup()
         {
             ProjectRootElement project = ProjectRootElement.Create();
             Assert.Equal(0, Helpers.Count(project.Children));
-            Assert.Null(project.PropertyGroups.GetEnumerator().Current);
+            Assert.Empty(project.PropertyGroups);
         }
 
         /// <summary>

src/Build.OM.UnitTests/Construction/ProjectTargetElement_Tests.cs

@@ -41,7 +41,7 @@ public void AddTargetInvalidName()
         public void ReadNoTarget()
         {
             ProjectRootElement project = ProjectRootElement.Create();
-            Assert.Null(project.Targets.GetEnumerator().Current);
+            Assert.Empty(project.Targets);
         }
 
         /// <summary>

src/Build.OM.UnitTests/Construction/ProjectUsingTaskElement_Tests.cs

@@ -25,7 +25,7 @@ public void ReadNone()
         {
             ProjectRootElement project = ProjectRootElement.Create();
 
-            Assert.Null(project.UsingTasks.GetEnumerator().Current);
+            Assert.Empty(project.UsingTasks);
         }
 
         /// <summary>

src/Build.OM.UnitTests/Construction/UsingTaskParameterGroup_Tests.cs

@@ -65,7 +65,7 @@ public void ReadEmptyParameterGroup()
             UsingTaskParameterGroupElement parameterGroup = GetParameterGroupXml(s_contentEmptyParameterGroup);
             Assert.NotNull(parameterGroup);
             Assert.Equal(0, parameterGroup.Count);
-            Assert.Null(parameterGroup.Parameters.GetEnumerator().Current);
+            Assert.Empty(parameterGroup.Parameters);
         }
 
         /// <summary>

src/Build/Construction/ProjectChooseElement.cs

@@ -3,9 +3,7 @@
 
 using System.Collections.Generic;
 using System.Diagnostics;
-using System.Linq;
 using System.Xml;
-using Microsoft.Build.Collections;
 using Microsoft.Build.ObjectModelRemoting;
 using Microsoft.Build.Shared;
 
@@ -60,7 +58,7 @@ public override string Condition
         /// Get the When children.
         /// Will contain at least one entry.
         /// </summary>
-        public ICollection<ProjectWhenElement> WhenElements => new ReadOnlyCollection<ProjectWhenElement>(Children.OfType<ProjectWhenElement>());
+        public ICollection<ProjectWhenElement> WhenElements => GetChildrenOfType<ProjectWhenElement>();
 
         /// <summary>
         /// Get any Otherwise child.