Random cleanup

src/Build.OM.UnitTests/Definition/Project_Tests.cs

@@ -2107,10 +2107,6 @@ public void BuildEvaluationUsesCustomLoggers()
             {
                 result = project.Build(new ILogger[] { mockLogger });
             }
-            catch
-            {
-                throw;
-            }
             finally
             {
                 project.ProjectCollection.UnregisterAllLoggers();

src/Build.UnitTests/BackEnd/AssemblyTaskFactory_Tests.cs

@@ -11,6 +11,7 @@
 using Microsoft.Build.Construction;
 using InvalidProjectFileException = Microsoft.Build.Exceptions.InvalidProjectFileException;
 using Xunit;
+using Shouldly;
 
 namespace Microsoft.Build.UnitTests.BackEnd
 {
@@ -255,8 +256,8 @@ public void VerifyGoodTaskInstantiation()
                     new AppDomainSetup(),
 #endif
                     false);
-                Assert.NotNull(createdTask);
-                Assert.False(createdTask is TaskHostTask);
+                createdTask.ShouldNotBeNull();
+                createdTask.ShouldNotBeOfType<TaskHostTask>();
             }
             finally
             {

src/Build.UnitTests/BackEnd/BuildManager_Tests.cs

@@ -1679,11 +1679,11 @@ public void CancelledBuildWithDelay40()
         [Fact]
         public void CancelledBuildInTaskHostWithDelay40()
         {
-            string contents = CleanupFileContents(@"
+            string contents = CleanupFileContents(@$"
 <Project xmlns='msbuildnamespace' ToolsVersion='msbuilddefaulttoolsversion'>
  <UsingTask TaskName='Microsoft.Build.Tasks.Exec' AssemblyName='Microsoft.Build.Tasks.Core, Version=msbuildassemblyversion, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a' TaskFactory='TaskHostFactory' />
  <Target Name='test'>
-    <Exec Command='" + Helpers.GetSleepCommand(TimeSpan.FromSeconds(10)) + @"'/>
+    <Exec Command='{Helpers.GetSleepCommand(TimeSpan.FromSeconds(10))}'/>
     <Message Text='[errormessage]'/>
  </Target>
 </Project>

src/Build.UnitTests/BackEnd/TargetBuilder_Tests.cs

@@ -1599,19 +1599,10 @@ private ProjectInstance CreateTestProject(string projectBodyContents, string ini
                     File.Create("testProject.proj").Dispose();
                     break;
                 }
-                catch (Exception ex)
+                // If all the retries failed, fail with the actual problem instead of some difficult-to-understand issue later.
+                catch (Exception ex) when (retries < 4)
                 {
-                    if (retries < 4)
-                    {
-                        Console.WriteLine(ex.ToString());
-                    }
-                    else
-                    {
-                        // All the retries have failed. We will now fail with the
-                        // actual problem now instead of with some more difficult-to-understand
-                        // issue later.
-                        throw;
-                    }
+                    Console.WriteLine(ex.ToString());
                 }
             }
 

src/Build.UnitTests/BackEnd/TaskHostFactory_Tests.cs

@@ -27,7 +27,7 @@ public void TaskNodesDieAfterBuild()
     </Target>
 </Project>";
                 TransientTestFile project = env.CreateFile("testProject.csproj", pidTaskProject);
-                ProjectInstance projectInstance = new ProjectInstance(project.Path);
+                ProjectInstance projectInstance = new(project.Path);
                 projectInstance.Build().ShouldBeTrue();
                 string processId = projectInstance.GetPropertyValue("PID");
                 string.IsNullOrEmpty(processId).ShouldBeFalse();

src/Build.UnitTests/EscapingInProjects_Tests.cs

@@ -22,6 +22,7 @@
 using Xunit;
 using Xunit.Abstractions;
 using Microsoft.Build.Shared;
+using Shouldly;
 
 namespace Microsoft.Build.UnitTests.EscapingInProjects_Tests
 {
@@ -126,11 +127,11 @@ public void SemicolonInPropertyPassedIntoStringParam_UsingTaskHost()
         [Fact]
         public void SemicolonInPropertyPassedIntoITaskItemParam()
         {
-            MockLogger logger = Helpers.BuildProjectWithNewOMExpectSuccess(String.Format(@"
+            MockLogger logger = Helpers.BuildProjectWithNewOMExpectSuccess(@$"
 
                 <Project ToolsVersion=`msbuilddefaulttoolsversion` xmlns=`http://schemas.microsoft.com/developer/msbuild/2003`>
 
-                    <UsingTask TaskName=`Microsoft.Build.UnitTests.EscapingInProjects_Tests.MyTestTask` AssemblyFile=`{0}` />
+                    <UsingTask TaskName=`Microsoft.Build.UnitTests.EscapingInProjects_Tests.MyTestTask` AssemblyFile=`{new Uri(Assembly.GetExecutingAssembly().EscapedCodeBase).LocalPath}` />
 
                     <PropertyGroup>
                         <MyPropertyWithSemicolons>abc %3b def %3b ghi</MyPropertyWithSemicolons>
@@ -142,7 +143,7 @@ public void SemicolonInPropertyPassedIntoITaskItemParam()
 
                 </Project>
 
-                ", new Uri(Assembly.GetExecutingAssembly().EscapedCodeBase).LocalPath),
+                ",
                 logger: new MockLogger(_output));
 
             logger.AssertLogContains("Received TaskItemParam: 123 abc ; def ; ghi 789");
@@ -715,14 +716,14 @@ public void EscapedWildcardsShouldNotBeExpanded()
         [Trait("Category", "mono-osx-failing")]
         public void EscapedWildcardsShouldNotBeExpanded_InTaskHost()
         {
-            MockLogger logger = new MockLogger();
+            MockLogger logger = new();
 
             try
             {
                 // Populate the project directory with three physical files on disk -- a.weirdo, b.weirdo, c.weirdo.
                 EscapingInProjectsHelper.CreateThreeWeirdoFiles();
                 Project project = ObjectModelHelpers.CreateInMemoryProject(@"
-                <Project ToolsVersion=`msbuilddefaulttoolsversion` xmlns=`http://schemas.microsoft.com/developer/msbuild/2003`>
+                <Project>
                     <UsingTask TaskName=`Message` AssemblyFile=`$(MSBuildToolsPath)\Microsoft.Build.Tasks.Core.dll` TaskFactory=`TaskHostFactory` />
 
                     <Target Name=`t`>
@@ -734,8 +735,7 @@ public void EscapedWildcardsShouldNotBeExpanded_InTaskHost()
                 </Project>
                 ");
 
-                bool success = project.Build(logger);
-                Assert.True(success); // "Build failed.  See test output (Attachments in Azure Pipelines) for details"
+                project.Build(logger).ShouldBeTrue("Build failed.  See test output (Attachments in Azure Pipelines) for details");
                 logger.AssertLogContains("[*]");
             }
             finally

src/Build/BackEnd/BuildManager/BuildManager.cs

@@ -1719,10 +1719,9 @@ void IssueBuildSubmissionToSchedulerImpl(BuildSubmission submission, bool allowM
                         HandleNewRequest(Scheduler.VirtualNode, blocker);
                     }
                 }
-                catch (Exception ex) when (!ExceptionHandling.IsCriticalException(ex))
+                catch (Exception ex) when (IsInvalidProjectOrIORelatedException(ex))
                 {
-                    var projectException = ex as InvalidProjectFileException;
-                    if (projectException != null)
+                    if (ex is InvalidProjectFileException projectException)
                     {
                         if (!projectException.HasBeenLogged)
                         {
@@ -1731,10 +1730,6 @@ void IssueBuildSubmissionToSchedulerImpl(BuildSubmission submission, bool allowM
                             projectException.HasBeenLogged = true;
                         }
                     }
-                    else if ((ex is BuildAbortedException) || ExceptionHandling.NotExpectedException(ex))
-                    {
-                        throw;
-                    }
 
                     lock (_syncLock)
                     {
@@ -1744,7 +1739,7 @@ void IssueBuildSubmissionToSchedulerImpl(BuildSubmission submission, bool allowM
                             _legacyThreadingData.MainThreadSubmissionId = -1;
                         }
 
-                        if (projectException == null)
+                        if (ex is not InvalidProjectFileException)
                         {
                             var buildEventContext = new BuildEventContext(submission.SubmissionId, 1, BuildEventContext.InvalidProjectInstanceId, BuildEventContext.InvalidProjectContextId, BuildEventContext.InvalidTargetId, BuildEventContext.InvalidTaskId);
                             ((IBuildComponentHost)this).LoggingService.LogFatalBuildError(buildEventContext, ex, new BuildEventFileInfo(submission.BuildRequestData.ProjectFullPath));
@@ -1764,6 +1759,11 @@ void IssueBuildSubmissionToSchedulerImpl(BuildSubmission submission, bool allowM
             }
         }
 
+        private bool IsInvalidProjectOrIORelatedException(Exception e)
+        {
+            return !ExceptionHandling.IsCriticalException(e) && !ExceptionHandling.NotExpectedException(e) && e is not BuildAbortedException;
+        }
+
         private void ExecuteGraphBuildScheduler(GraphBuildSubmission submission)
         {
             try
@@ -1847,15 +1847,15 @@ private void ExecuteGraphBuildScheduler(GraphBuildSubmission submission)
                         submission.SubmissionId,
                         new ReadOnlyDictionary<ProjectGraphNode, BuildResult>(resultsPerNode ?? new Dictionary<ProjectGraphNode, BuildResult>())));
             }
-            catch (Exception ex) when (!ExceptionHandling.IsCriticalException(ex))
+            catch (Exception ex) when (IsInvalidProjectOrIORelatedException(ex))
             {
                 GraphBuildResult result = null;
 
                 // ProjectGraph throws an aggregate exception with InvalidProjectFileException inside when evaluation fails
                 if (ex is AggregateException aggregateException && aggregateException.InnerExceptions.All(innerException => innerException is InvalidProjectFileException))
                 {
                     // Log each InvalidProjectFileException encountered during ProjectGraph creation
-                    foreach (var innerException in aggregateException.InnerExceptions)
+                    foreach (Exception innerException in aggregateException.InnerExceptions)
                     {
                         var projectException = (InvalidProjectFileException) innerException;
                         if (!projectException.HasBeenLogged)
@@ -1873,23 +1873,16 @@ private void ExecuteGraphBuildScheduler(GraphBuildSubmission submission)
                     BuildEventContext projectBuildEventContext = new BuildEventContext(submission.SubmissionId, 1, BuildEventContext.InvalidProjectInstanceId, BuildEventContext.InvalidProjectContextId, BuildEventContext.InvalidTargetId, BuildEventContext.InvalidTaskId);
                     ((IBuildComponentHost)this).LoggingService.LogInvalidProjectFileError(projectBuildEventContext, new InvalidProjectFileException(ex.Message, ex));
                 }
-                else if (ex is BuildAbortedException || ExceptionHandling.NotExpectedException(ex))
-                {
-                    throw;
-                }
                 else
                 {
                     // Arbitrarily just choose the first entry point project's path
-                    var projectFile = submission.BuildRequestData.ProjectGraph?.EntryPointNodes.First().ProjectInstance.FullPath
+                    string projectFile = submission.BuildRequestData.ProjectGraph?.EntryPointNodes.First().ProjectInstance.FullPath
                         ?? submission.BuildRequestData.ProjectGraphEntryPoints?.First().ProjectFile;
                     BuildEventContext buildEventContext = new BuildEventContext(submission.SubmissionId, 1, BuildEventContext.InvalidProjectInstanceId, BuildEventContext.InvalidProjectContextId, BuildEventContext.InvalidTargetId, BuildEventContext.InvalidTaskId);
                     ((IBuildComponentHost)this).LoggingService.LogFatalBuildError(buildEventContext, ex, new BuildEventFileInfo(projectFile));
                 }
 
-                if (result == null)
-                {
-                    result = new GraphBuildResult(submission.SubmissionId, ex);
-                }
+                result ??= new GraphBuildResult(submission.SubmissionId, ex);
 
                 ReportResultsToSubmission(result);
 

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

@@ -230,8 +230,7 @@ public bool ResolveConfigurationRequest(int unresolvedConfigId, int configId)
         {
             lock (GlobalLock)
             {
-                List<BuildRequest> requests = null;
-                if (_unresolvedConfigurations?.TryGetValue(unresolvedConfigId, out requests) != true)
+                if (_unresolvedConfigurations?.TryGetValue(unresolvedConfigId, out List<BuildRequest> requests) != true)
                 {
                     return false;
                 }

src/Build/BackEnd/Components/Caching/ConfigCache.cs

@@ -118,8 +118,7 @@ public BuildRequestConfiguration GetMatchingConfiguration(ConfigurationMetadata
             ErrorUtilities.VerifyThrowArgumentNull(configMetadata, nameof(configMetadata));
             lock (_lockObject)
             {
-                int configId;
-                if (!_configurationIdsByMetadata.TryGetValue(configMetadata, out configId))
+                if (!_configurationIdsByMetadata.TryGetValue(configMetadata, out int configId))
                 {
                     return null;
                 }
@@ -214,10 +213,9 @@ public List<int> ClearNonExplicitlyLoadedConfigurations()
             {
                 foreach (KeyValuePair<ConfigurationMetadata, int> metadata in _configurationIdsByMetadata)
                 {
-                    BuildRequestConfiguration configuration;
                     int configId = metadata.Value;
 
-                    if (_configurations.TryGetValue(configId, out configuration))
+                    if (_configurations.TryGetValue(configId, out BuildRequestConfiguration configuration))
                     {
                         // We do not want to retain this configuration
                         if (!configuration.ExplicitlyLoaded)

src/Build/BackEnd/Components/Caching/ResultsCache.cs

@@ -222,8 +222,7 @@ public void ClearResultsForConfiguration(int configurationId)
         {
             lock (_resultsByConfiguration)
             {
-                BuildResult removedResult;
-                _resultsByConfiguration.TryRemove(configurationId, out removedResult);
+                _resultsByConfiguration.TryRemove(configurationId, out BuildResult removedResult);
 
                 removedResult?.ClearCachedFiles();
             }

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

@@ -126,8 +126,7 @@ public NodeInfo CreateNode(NodeConfiguration configuration, NodeAffinity nodeAff
         public void SendData(int node, INodePacket packet)
         {
             // Look up the node provider for this node in the mapping.
-            INodeProvider provider;
-            if (!_nodeIdToProvider.TryGetValue(node, out provider))
+            if (!_nodeIdToProvider.TryGetValue(node, out INodeProvider provider))
             {
                 ErrorUtilities.ThrowInternalError("Node {0} does not have a provider.", node);
             }

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

@@ -455,9 +455,8 @@ internal static string GetMSBuildLocationFromHostContext(HandshakeOptions hostCo
         /// </summary>
         internal bool AcquireAndSetUpHost(HandshakeOptions hostContext, INodePacketFactory factory, INodePacketHandler handler, TaskHostConfiguration configuration)
         {
-            NodeContext context;
             bool nodeCreationSucceeded;
-            if (!_nodeContexts.TryGetValue(hostContext, out context))
+            if (!_nodeContexts.ContainsKey(hostContext))
             {
                 nodeCreationSucceeded = CreateNode(hostContext, factory, handler, configuration);
             }
@@ -469,7 +468,7 @@ internal bool AcquireAndSetUpHost(HandshakeOptions hostContext, INodePacketFacto
 
             if (nodeCreationSucceeded)
             {
-                context = _nodeContexts[hostContext];
+                NodeContext context = _nodeContexts[hostContext];
                 _nodeIdToPacketFactory[(int)hostContext] = factory;
                 _nodeIdToPacketHandler[(int)hostContext] = handler;
 

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

@@ -284,8 +284,7 @@ protected LoggingService(LoggerMode loggerMode, int nodeId)
             string queueCapacityEnvironment = Environment.GetEnvironmentVariable("MSBUILDLOGGINGQUEUECAPACITY");
             if (!String.IsNullOrEmpty(queueCapacityEnvironment))
             {
-                uint localQueueCapacity;
-                if (UInt32.TryParse(queueCapacityEnvironment, out localQueueCapacity))
+                if (UInt32.TryParse(queueCapacityEnvironment, out uint localQueueCapacity))
                 {
                     _queueCapacity = localQueueCapacity;
                 }
@@ -1315,17 +1314,8 @@ private void ShutdownLogger(ILogger logger)
             {
                 logger?.Shutdown();
             }
-            catch (LoggerException)
+            catch (Exception e) when (!ExceptionHandling.IsCriticalException(e) && e is not LoggerException)
             {
-                throw;
-            }
-            catch (Exception e)
-            {
-                if (ExceptionHandling.IsCriticalException(e))
-                {
-                    throw;
-                }
-
                 InternalLoggerException.Throw(e, null, "FatalErrorDuringLoggerShutdown", false, logger.GetType().Name);
             }
         }
@@ -1493,8 +1483,7 @@ private void RouteBuildEvent(KeyValuePair<int, BuildEventArgs> nodeEvent)
             TryRaiseProjectStartedEvent(nodeEvent.Value);
 
             // Get the sink which will handle the build event, then send the event to that sink
-            IBuildEventSink sink;
-            bool gotSink = _eventSinkDictionary.TryGetValue(nodeEvent.Key, out sink);
+            bool gotSink = _eventSinkDictionary.TryGetValue(nodeEvent.Key, out IBuildEventSink sink);
             if (gotSink && sink != null)
             {
                 // Sinks in the eventSinkDictionary are expected to not be null.
@@ -1586,17 +1575,8 @@ private void InitializeLogger(ILogger logger, IEventSource sourceForLogger)
                     logger.Initialize(sourceForLogger);
                 }
             }
-            catch (LoggerException)
+            catch (Exception e) when (!ExceptionHandling.IsCriticalException(e) && e is not LoggerException)
             {
-                throw;
-            }
-            catch (Exception e)
-            {
-                if (ExceptionHandling.IsCriticalException(e))
-                {
-                    throw;
-                }
-
                 InternalLoggerException.Throw(e, null, "FatalErrorWhileInitializingLogger", true, logger.GetType().Name);
             }