Address some low-hanging fruit to use newer/better .NET features

src/Build.UnitTests/BackEnd/CacheAggregator_Tests.cs

@@ -33,10 +33,10 @@ public void NoCachesProducesEmptyCaches()
             var aggregation = aggregator.Aggregate();
 
             aggregation.ConfigCache.ShouldNotBeNull();
-            aggregation.ConfigCache.GetEnumerator().ToEnumerable().ShouldBeEmpty();
+            aggregation.ConfigCache.ShouldBeEmpty();
 
             aggregation.ResultsCache.ShouldNotBeNull();
-            aggregation.ResultsCache.GetEnumerator().ToEnumerable().ShouldBeEmpty();
+            aggregation.ResultsCache.ShouldBeEmpty();
 
             aggregation.LastConfigurationId.ShouldBe(0);
         }
@@ -246,9 +246,9 @@ private void AssertAggregation((ConfigCache configCache, ResultsCache resultsCac
             var currentConfigurationIndex = 0;
             var currentBuildResultIndex = 0;
 
-            var aggregatedConfigs = aggregation.ConfigCache.GetEnumerator().ToArray();
+            var aggregatedConfigs = aggregation.ConfigCache.ToArray();
 
-            var aggregatedResults = aggregation.ResultsCache.GetEnumerator().ToArray();
+            var aggregatedResults = aggregation.ResultsCache.ToArray();
 
             foreach (var (configCache, resultsCache) in inputCaches)
             {

src/Build.UnitTests/BackEnd/ConfigCache_Tests.cs

@@ -101,8 +101,8 @@ public void ConfigCacheShouldBeTranslatable(object obj)
             TranslationHelpers.GetReadTranslator().Translate(ref copy);
 
             // test _configurations
-            var initialConfigurations = initial.GetEnumerator().ToArray();
-            var copiedConfigurations = copy.GetEnumerator().ToArray();
+            var initialConfigurations = initial.ToArray();
+            var copiedConfigurations = copy.ToArray();
 
             Assert.Equal(copiedConfigurations, initialConfigurations, EqualityComparer<BuildRequestConfiguration>.Default);
 

src/Build.UnitTests/BackEnd/ResultsCache_Tests.cs

@@ -80,7 +80,7 @@ public void CacheCanBeEnumerated()
             result2.AddResultsForTarget("result2target1", BuildResultUtilities.GetEmptyFailingTargetResult());
             cache.AddResult(result2);
 
-            var results = cache.GetEnumerator().ToArray();
+            var results = cache.ToArray();
 
             results.Length.ShouldBe(2);
 

src/Build.UnitTests/FileUtilitiesRegex_Tests.cs

@@ -532,7 +532,6 @@ public void PatternEmptyString_LegacyRegex()
         {
             UncPattern.IsMatch(string.Empty).ShouldBeFalse();
             StartsWithUncPattern.IsMatch(string.Empty).ShouldBeFalse();
-            StartsWithUncPattern.Match(string.Empty).Success.ShouldBeFalse();
         }
 
         [Fact]

src/Build.UnitTests/Globbing/MSBuildGlob_Tests.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System;
+using System.Buffers;
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
@@ -70,9 +71,12 @@ public void GlobFromRelativeGlobRootNormalizesRootAgainstCurrentDirectory()
         [Fact]
         public void GlobFromRootWithInvalidPathThrows()
         {
-            foreach (var invalidPathChar in FileUtilities.InvalidPathChars)
+            for (int i = 0; i < 128; i++)
             {
-                Assert.Throws<ArgumentException>(() => MSBuildGlob.Parse(invalidPathChar.ToString(), "*"));
+                if (FileUtilities.InvalidPathChars.Contains((char)i))
+                {
+                    Assert.Throws<ArgumentException>(() => MSBuildGlob.Parse(((char)i).ToString(), "*"));
+                }
             }
         }
 
@@ -182,12 +186,15 @@ public void GlobMatchShouldReturnFalseIfArgumentContainsInvalidPathOrFileCharact
         {
             var glob = MSBuildGlob.Parse("*");
 
-            foreach (var invalidPathChar in FileUtilities.InvalidPathChars)
+            for (int i = 0; i < 128; i++)
             {
-                Assert.False(glob.IsMatch(invalidPathChar.ToString()));
+                if (FileUtilities.InvalidPathChars.Contains((char)i))
+                {
+                    Assert.False(glob.IsMatch(((char)i).ToString()));
+                }
             }
 
-            foreach (var invalidFileChar in FileUtilities.InvalidFileNameChars)
+            foreach (var invalidFileChar in FileUtilities.InvalidFileNameCharsArray)
             {
                 if (invalidFileChar == '\\' || invalidFileChar == '/')
                 {

src/Build.UnitTests/Graph/IsolateProjects_Tests.cs

@@ -338,8 +338,8 @@ public void UndeclaredReferenceBuildResultNotPresentInOutputCache()
             var deserializedOutputCacheRoot = CacheSerialization.DeserializeCaches(outputCaches[topoSortedProjectGraphNodes[1]]);
             deserializedOutputCacheDeclaredReference.exception.ShouldBeNull();
             deserializedOutputCacheRoot.exception.ShouldBeNull();
-            BuildResult[] declaredReferenceBuildResults = deserializedOutputCacheDeclaredReference.ResultsCache.GetEnumerator().ToArray();
-            BuildResult[] rootBuildResults = deserializedOutputCacheRoot.ResultsCache.GetEnumerator().ToArray();
+            BuildResult[] declaredReferenceBuildResults = deserializedOutputCacheDeclaredReference.ResultsCache.ToArray();
+            BuildResult[] rootBuildResults = deserializedOutputCacheRoot.ResultsCache.ToArray();
 
             // Both the root and declared reference projects should only have one build result.
             declaredReferenceBuildResults.Length.ShouldBe(1);

src/Build.UnitTests/Graph/ResultCacheBasedBuilds_Tests.cs

@@ -356,13 +356,13 @@ public void OutputCacheShouldNotContainInformationFromInputCaches()
 
             deserializationInfo.exception.ShouldBeNull();
 
-            var buildResults = deserializationInfo.ResultsCache.GetEnumerator().ToArray();
+            var buildResults = deserializationInfo.ResultsCache.ToArray();
             buildResults.ShouldHaveSingleItem();
 
             var rootNodeBuildResult = buildResults.First();
             rootNodeBuildResult.ResultsByTarget["Build"].Items.Select(i => i.ItemSpec).ToArray().ShouldBe(expectedOutput[rootNode]);
 
-            var configEntries = deserializationInfo.ConfigCache.GetEnumerator().ToArray();
+            var configEntries = deserializationInfo.ConfigCache.ToArray();
             configEntries.ShouldHaveSingleItem();
 
             configEntries.First().ConfigurationId.ShouldBe(rootNodeBuildResult.ConfigurationId);

src/Build/BackEnd/BuildManager/BuildManager.cs

@@ -2801,12 +2801,11 @@ private NodeConfiguration GetNodeConfiguration()
             {
                 // Get the remote loggers
                 ILoggingService loggingService = ((IBuildComponentHost)this).GetComponent<ILoggingService>(BuildComponentType.LoggingService);
-                var remoteLoggers = new List<LoggerDescription>(loggingService.LoggerDescriptions);
 
                 _nodeConfiguration = new NodeConfiguration(
                 -1, /* must be assigned by the NodeManager */
                 _buildParameters,
-                remoteLoggers.ToArray()
+                loggingService.LoggerDescriptions.ToArray()
 #if FEATURE_APPDOMAIN
                 , AppDomain.CurrentDomain.SetupInformation
 #endif

src/Build/BackEnd/BuildManager/CacheAggregator.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using Microsoft.Build.BackEnd;
 using Microsoft.Build.Framework;
 using Microsoft.Build.Internal;
@@ -55,8 +56,8 @@ public CacheAggregation Aggregate()
 
         private void InsertCaches(IConfigCache configCache, IResultsCache resultsCache)
         {
-            var configs = configCache.GetEnumerator().ToArray();
-            var results = resultsCache.GetEnumerator().ToArray();
+            var configs = configCache.ToArray();
+            var results = resultsCache.ToArray();
 
             ErrorUtilities.VerifyThrow(configs.Length == results.Length, "Assuming 1-to-1 mapping between configs and results. Otherwise it means the caches are either not minimal or incomplete");
 

src/Build/BackEnd/Client/MSBuildClientPacketPump.cs

@@ -195,7 +195,7 @@ private void RunReadLoop(Stream localStream, ManualResetEvent localPacketPumpShu
 #if FEATURE_APM
                 IAsyncResult result = localStream.BeginRead(headerByte, 0, headerByte.Length, null, null);
 #else
-                Task<int> readTask = CommunicationsUtilities.ReadAsync(localStream, headerByte, headerByte.Length);
+                Task<int> readTask = CommunicationsUtilities.ReadAsync(localStream, headerByte, headerByte.Length).AsTask();
 #endif
 
                 bool continueReading = true;
@@ -294,7 +294,7 @@ private void RunReadLoop(Stream localStream, ManualResetEvent localPacketPumpShu
 #if FEATURE_APM
                                     result = localStream.BeginRead(headerByte, 0, headerByte.Length, null, null);
 #else
-                                    readTask = CommunicationsUtilities.ReadAsync(localStream, headerByte, headerByte.Length);
+                                    readTask = CommunicationsUtilities.ReadAsync(localStream, headerByte, headerByte.Length).AsTask();
 #endif
                                 }
                             }

src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEngine.cs

@@ -1433,7 +1433,7 @@ private void TraceEngine(string format, params object[] stuff)
                     using (StreamWriter file = FileUtilities.OpenWrite(string.Format(CultureInfo.CurrentCulture, Path.Combine(_debugDumpPath, @"EngineTrace_{0}.txt"), EnvironmentUtilities.CurrentProcessId), append: true))
                     {
                         string message = String.Format(CultureInfo.CurrentCulture, format, stuff);
-                        file.WriteLine("{0}({1})-{2}: {3}", Thread.CurrentThread.Name, Thread.CurrentThread.ManagedThreadId, DateTime.UtcNow.Ticks, message);
+                        file.WriteLine("{0}({1})-{2}: {3}", Thread.CurrentThread.Name, Environment.CurrentManagedThreadId, DateTime.UtcNow.Ticks, message);
                         file.Flush();
                     }
                 }

src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEntry.cs

@@ -511,12 +511,13 @@ private void WaitForResult(BuildRequest newRequest, bool addToIssueList)
                     ErrorUtilities.VerifyThrow(addToIssueList, "Requests with unresolved configurations should always be added to the issue list.");
                     _unresolvedConfigurations ??= new Dictionary<int, List<BuildRequest>>();
 
-                    if (!_unresolvedConfigurations.ContainsKey(newRequest.ConfigurationId))
+                    if (!_unresolvedConfigurations.TryGetValue(newRequest.ConfigurationId, out List<BuildRequest> value))
                     {
-                        _unresolvedConfigurations.Add(newRequest.ConfigurationId, new List<BuildRequest>());
+                        value = new List<BuildRequest>();
+                        _unresolvedConfigurations.Add(newRequest.ConfigurationId, value);
                     }
 
-                    _unresolvedConfigurations[newRequest.ConfigurationId].Add(newRequest);
+                    value.Add(newRequest);
                 }
 
                 if (addToIssueList)

src/Build/BackEnd/Components/Communications/NodeEndpointInProc.cs

@@ -29,7 +29,7 @@ internal class NodeEndpointInProc : INodeEndpoint
         /// <summary>
         /// An object for the two inproc endpoints to synchronize on.
         /// </summary>
-        private static Object s_locker = new Object();
+        private static readonly Object s_locker = new Object();
 
         /// <summary>
         /// The current communication status of the node.

src/Build/BackEnd/Components/Communications/NodeProviderInProc.cs

@@ -382,7 +382,7 @@ private bool InstantiateNode(INodePacketFactory factory)
                 InProcNodeThreadProc();
             });
 #endif
-            _inProcNodeThread.Name = String.Format(CultureInfo.CurrentCulture, "In-proc Node ({0})", _componentHost.Name);
+            _inProcNodeThread.Name = $"In-proc Node ({_componentHost.Name})";
             _inProcNodeThread.IsBackground = true;
 #if FEATURE_THREAD_CULTURE
             _inProcNodeThread.CurrentCulture = _componentHost.BuildParameters.Culture;

src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs

@@ -416,7 +416,11 @@ void CreateNodeContext(int nodeId, Process nodeToReuse, Stream nodeStream)
         /// </summary>
         private string GetProcessesToIgnoreKey(Handshake hostHandshake, int nodeProcessId)
         {
-            return hostHandshake.ToString() + "|" + nodeProcessId.ToString(CultureInfo.InvariantCulture);
+#if NET
+            return string.Create(CultureInfo.InvariantCulture, $"{hostHandshake}|{nodeProcessId}");
+#else
+            return $"{hostHandshake}|{nodeProcessId.ToString(CultureInfo.InvariantCulture)}";
+#endif
         }
 
 #if !FEATURE_PIPEOPTIONS_CURRENTUSERONLY
@@ -833,8 +837,17 @@ public async Task WaitForExitAsync(ILoggingService loggingService)
                 {
                     // Wait up to 100ms until all remaining packets are sent.
                     // We don't need to wait long, just long enough for the Task to start running on the ThreadPool.
-                    await Task.WhenAny(_packetWriteDrainTask, Task.Delay(100));
+#if NET
+                    await _packetWriteDrainTask.WaitAsync(TimeSpan.FromMilliseconds(100)).ConfigureAwait(ConfigureAwaitOptions.SuppressThrowing);
+#else
+                    using (var cts = new CancellationTokenSource(100))
+                    {
+                        await Task.WhenAny(_packetWriteDrainTask, Task.Delay(100, cts.Token));
+                        cts.Cancel();
+                    }
+#endif
                 }
+
                 if (_exitPacketState == ExitPacketState.ExitPacketSent)
                 {
                     CommunicationsUtilities.Trace("Waiting for node with pid = {0} to exit", _process.Id);

src/Build/BackEnd/Components/Communications/TranslatorExtensions.cs

@@ -20,7 +20,7 @@ namespace Microsoft.Build.BackEnd
     /// </summary>
     internal static class TranslatorExtensions
     {
-        private static Lazy<ConcurrentDictionary<Type, ConstructorInfo>> parameterlessConstructorCache = new Lazy<ConcurrentDictionary<Type, ConstructorInfo>>(() => new ConcurrentDictionary<Type, ConstructorInfo>());
+        private static readonly Lazy<ConcurrentDictionary<Type, ConstructorInfo>> parameterlessConstructorCache = new Lazy<ConcurrentDictionary<Type, ConstructorInfo>>(() => new ConcurrentDictionary<Type, ConstructorInfo>());
 
         /// <summary>
         /// Translates a PropertyDictionary of ProjectPropertyInstances.

src/Build/BackEnd/Components/FileAccesses/FileAccessManager.cs

@@ -22,7 +22,7 @@ private record Handlers(Action<BuildRequest, FileAccessData> FileAccessHander, A
         // is used to mark when the file accesses should be considered complete. Only after both this special file access is seen
         // and the build result is reported can plugins be notified about project completion.
         // NOTE! This is currently Windows-specific and will need to change once this feature is opened up to more scenarios.
-        private static readonly string FileAccessCompletionPrefix = BuildParameters.StartupDirectory[0] + @":\{MSBuildFileAccessCompletion}\";
+        private static readonly string FileAccessCompletionPrefix = $@"{BuildParameters.StartupDirectory[0]}:\{{MSBuildFileAccessCompletion}}\";
 
         private IScheduler? _scheduler;
         private IConfigCache? _configCache;

src/Build/BackEnd/Components/Logging/LoggingService.cs

@@ -94,7 +94,7 @@ internal partial class LoggingService : ILoggingService, INodePacketHandler
         /// We use a BindingFlags.Public flag here because the getter is public, so although the setter is internal,
         /// it is only discoverable with Reflection using the Public flag (go figure!)
         /// </remarks>
-        private static Lazy<PropertyInfo> s_projectStartedEventArgsGlobalProperties = new Lazy<PropertyInfo>(() => typeof(ProjectStartedEventArgs).GetProperty("GlobalProperties", BindingFlags.Public | BindingFlags.Instance), LazyThreadSafetyMode.PublicationOnly);
+        private static readonly Lazy<PropertyInfo> s_projectStartedEventArgsGlobalProperties = new Lazy<PropertyInfo>(() => typeof(ProjectStartedEventArgs).GetProperty("GlobalProperties", BindingFlags.Public | BindingFlags.Instance), LazyThreadSafetyMode.PublicationOnly);
 
         /// <summary>
         /// A cached reflection accessor for an internal member.
@@ -103,7 +103,7 @@ internal partial class LoggingService : ILoggingService, INodePacketHandler
         /// We use a BindingFlags.Public flag here because the getter is public, so although the setter is internal,
         /// it is only discoverable with Reflection using the Public flag (go figure!)
         /// </remarks>
-        private static Lazy<PropertyInfo> s_projectStartedEventArgsToolsVersion = new Lazy<PropertyInfo>(() => typeof(ProjectStartedEventArgs).GetProperty("ToolsVersion", BindingFlags.Public | BindingFlags.Instance), LazyThreadSafetyMode.PublicationOnly);
+        private static readonly Lazy<PropertyInfo> s_projectStartedEventArgsToolsVersion = new Lazy<PropertyInfo>(() => typeof(ProjectStartedEventArgs).GetProperty("ToolsVersion", BindingFlags.Public | BindingFlags.Instance), LazyThreadSafetyMode.PublicationOnly);
 
         #region Data
 

src/Build/BackEnd/Components/ProjectCache/ProjectCacheService.cs

@@ -32,7 +32,7 @@ internal sealed class ProjectCacheService : IAsyncDisposable
     {
         private static readonly ParallelOptions s_parallelOptions = new() { MaxDegreeOfParallelism = Environment.ProcessorCount };
 
-        private static HashSet<string> s_projectSpecificPropertyNames = new(StringComparer.OrdinalIgnoreCase) { "TargetFramework", "Configuration", "Platform", "TargetPlatform", "OutputType" };
+        private static readonly HashSet<string> s_projectSpecificPropertyNames = new(StringComparer.OrdinalIgnoreCase) { "TargetFramework", "Configuration", "Platform", "TargetPlatform", "OutputType" };
 
         private readonly BuildManager _buildManager;
         private readonly IBuildComponentHost _componentHost;
@@ -115,8 +115,7 @@ public void InitializePluginsForGraph(
                             foreach (ProjectCacheDescriptor projectCacheDescriptor in GetProjectCacheDescriptors(node.ProjectInstance))
                             {
                                 // Intentionally fire-and-forget to asynchronously initialize the plugin. Any exceptions will bubble up later when querying.
-                                _ = GetProjectCachePluginAsync(projectCacheDescriptor, projectGraph, buildRequestConfiguration: null, requestedTargets, cancellationToken)
-                                    .ContinueWith(t => { }, TaskContinuationOptions.ExecuteSynchronously | TaskContinuationOptions.OnlyOnFaulted);
+                                _ = GetProjectCachePluginAsync(projectCacheDescriptor, projectGraph, buildRequestConfiguration: null, requestedTargets, cancellationToken);
                             }
                         });
                 },
@@ -149,8 +148,7 @@ public void InitializePluginsForVsScenario(
                         projectCacheDescriptor =>
                         {
                             // Intentionally fire-and-forget to asynchronously initialize the plugin. Any exceptions will bubble up later when querying.
-                            _ = GetProjectCachePluginAsync(projectCacheDescriptor, projectGraph: null, buildRequestConfiguration, requestedTargets, cancellationToken)
-                                .ContinueWith(t => { }, TaskContinuationOptions.ExecuteSynchronously | TaskContinuationOptions.OnlyOnFaulted);
+                            _ = GetProjectCachePluginAsync(projectCacheDescriptor, projectGraph: null, buildRequestConfiguration, requestedTargets, cancellationToken);
                         });
                 },
                 cancellationToken);
@@ -449,7 +447,7 @@ public void PostCacheRequest(CacheRequest cacheRequest, CancellationToken cancel
                 },
                 cancellationToken);
 
-            async Task<(CacheResult Result, int ProjectContextId)> ProcessCacheRequestAsync()
+            async ValueTask<(CacheResult Result, int ProjectContextId)> ProcessCacheRequestAsync()
             {
                 EvaluateProjectIfNecessary(cacheRequest.Submission, cacheRequest.Configuration);
 
@@ -499,7 +497,7 @@ void EvaluateProjectIfNecessary(BuildSubmission submission, BuildRequestConfigur
             }
         }
 
-        private async Task<CacheResult> GetCacheResultAsync(BuildRequestData buildRequest, BuildRequestConfiguration buildRequestConfiguration, BuildEventContext buildEventContext, CancellationToken cancellationToken)
+        private async ValueTask<CacheResult> GetCacheResultAsync(BuildRequestData buildRequest, BuildRequestConfiguration buildRequestConfiguration, BuildEventContext buildEventContext, CancellationToken cancellationToken)
         {
             ErrorUtilities.VerifyThrowInternalNull(buildRequest.ProjectInstance, nameof(buildRequest.ProjectInstance));