diff --git a/src/Build/BackEnd/BuildManager/BuildManager.cs b/src/Build/BackEnd/BuildManager/BuildManager.cs index 3a871f1cca4..2bc9c7e06f6 100644 --- a/src/Build/BackEnd/BuildManager/BuildManager.cs +++ b/src/Build/BackEnd/BuildManager/BuildManager.cs @@ -1727,7 +1727,7 @@ private void LoadSolutionIntoConfiguration(BuildRequestConfiguration config, Bui return; } - ErrorUtilities.VerifyThrow(FileUtilities.IsSolutionFilename(config.ProjectFullPath), "{0} is not a solution", config.ProjectFullPath); + ErrorUtilities.VerifyThrow(FileUtilities.IsSolutionFilename(config.ProjectFullPath), $"{config.ProjectFullPath} is not a solution"); var buildEventContext = request.BuildEventContext; if (buildEventContext == BuildEventContext.Invalid) @@ -1899,7 +1899,7 @@ private void ProcessPacket(int node, INodePacket packet) break; default: - ErrorUtilities.ThrowInternalError("Unexpected packet received by BuildManager: {0}", packet.Type); + ErrorUtilities.ThrowInternalError($"Unexpected packet received by BuildManager: {packet.Type}"); break; } } @@ -2393,7 +2393,7 @@ private void VerifyStateInternal(BuildManagerState requiredState) { if (_buildManagerState != requiredState) { - ErrorUtilities.ThrowInternalError("Expected state {0}, actual state {1}", requiredState, _buildManagerState); + ErrorUtilities.ThrowInternalError($"Expected state {requiredState}, actual state {_buildManagerState}"); } } @@ -2742,7 +2742,7 @@ private void HandleNodeShutdown(int node, NodeShutdown shutdownPacket) _shuttingDown = true; _executionCancellationTokenSource?.Cancel(); - ErrorUtilities.VerifyThrow(_activeNodes.Contains(node), "Unexpected shutdown from node {0} which shouldn't exist.", node); + ErrorUtilities.VerifyThrow(_activeNodes.Contains(node), $"Unexpected shutdown from node {node} which shouldn't exist."); _activeNodes.Remove(node); if (shutdownPacket.Reason != NodeShutdownReason.Requested) @@ -2945,7 +2945,7 @@ private void PerformSchedulingActions(IEnumerable responses) break; default: - ErrorUtilities.ThrowInternalError("Scheduling action {0} not handled.", response.Action); + ErrorUtilities.ThrowInternalError($"Scheduling action {response.Action} not handled."); break; } } @@ -3381,17 +3381,14 @@ private static void LogDeferredMessages(ILoggingService loggingService, IEnumera /// Ensures that the packet type matches the expected type /// /// The instance-type of packet being expected - private static I ExpectPacketType(INodePacket packet, NodePacketType expectedType) where I : class, INodePacket + private static I ExpectPacketType(INodePacket packet, NodePacketType expectedType) + where I : class, INodePacket { I? castPacket = packet as I; - // PERF: Not using VerifyThrow here to avoid boxing of expectedType. - if (castPacket == null) - { - ErrorUtilities.ThrowInternalError("Incorrect packet type: {0} should have been {1}", packet.Type, expectedType); - } + ErrorUtilities.VerifyThrow(castPacket != null, $"Incorrect packet type: {packet.Type} should have been {expectedType}"); - return castPacket!; + return castPacket; } /// diff --git a/src/Build/BackEnd/BuildManager/BuildSubmission.cs b/src/Build/BackEnd/BuildManager/BuildSubmission.cs index 7d996548bd6..b4ab0065f77 100644 --- a/src/Build/BackEnd/BuildManager/BuildSubmission.cs +++ b/src/Build/BackEnd/BuildManager/BuildSubmission.cs @@ -227,8 +227,7 @@ protected internal override void CheckResultValidForCompletion(BuildResult resul // this one.) if (result.ConfigurationId != BuildRequest?.ConfigurationId) { - ErrorUtilities.ThrowInternalError("BuildResult configuration ({0}) doesn't match BuildRequest configuration ({1})", - result.ConfigurationId, BuildRequest?.ConfigurationId); + ErrorUtilities.ThrowInternalError($"BuildResult configuration ({result.ConfigurationId}) doesn't match BuildRequest configuration ({BuildRequest?.ConfigurationId})"); } } diff --git a/src/Build/BackEnd/BuildManager/LegacyThreadingData.cs b/src/Build/BackEnd/BuildManager/LegacyThreadingData.cs index fa4374d7839..3920c1d783e 100644 --- a/src/Build/BackEnd/BuildManager/LegacyThreadingData.cs +++ b/src/Build/BackEnd/BuildManager/LegacyThreadingData.cs @@ -88,7 +88,7 @@ internal void RegisterSubmissionForLegacyThread(int submissionId) { lock (_legacyThreadingEventsLock) { - ErrorUtilities.VerifyThrow(!_legacyThreadingEventsById.ContainsKey(submissionId), "Submission {0} should not already be registered with LegacyThreadingData", submissionId); + ErrorUtilities.VerifyThrow(!_legacyThreadingEventsById.ContainsKey(submissionId), $"Submission {submissionId} should not already be registered with LegacyThreadingData"); _legacyThreadingEventsById[submissionId] = new Tuple( new AutoResetEvent(false), @@ -104,7 +104,7 @@ internal void UnregisterSubmissionForLegacyThread(int submissionId) { lock (_legacyThreadingEventsLock) { - ErrorUtilities.VerifyThrow(_legacyThreadingEventsById.ContainsKey(submissionId), "Submission {0} should have been previously registered with LegacyThreadingData", submissionId); + ErrorUtilities.VerifyThrow(_legacyThreadingEventsById.ContainsKey(submissionId), $"Submission {submissionId} should have been previously registered with LegacyThreadingData"); // Dispose the events _legacyThreadingEventsById[submissionId].Item1?.Dispose(); @@ -127,7 +127,7 @@ internal WaitHandle GetStartRequestBuilderMainThreadEventForSubmission(int submi _legacyThreadingEventsById.TryGetValue(submissionId, out legacyThreadingEvents); } - ErrorUtilities.VerifyThrow(legacyThreadingEvents != null, "We're trying to wait on the legacy thread for submission {0}, but that submission has not been registered.", submissionId); + ErrorUtilities.VerifyThrow(legacyThreadingEvents != null, $"We're trying to wait on the legacy thread for submission {submissionId}, but that submission has not been registered."); return legacyThreadingEvents.Item1; } @@ -145,7 +145,7 @@ internal Task GetLegacyThreadInactiveTask(int submissionId) _legacyThreadingEventsById.TryGetValue(submissionId, out legacyThreadingEvents); } - ErrorUtilities.VerifyThrow(legacyThreadingEvents != null, "We're trying to track when the legacy thread for submission {0} goes inactive, but that submission has not been registered.", submissionId); + ErrorUtilities.VerifyThrow(legacyThreadingEvents != null, $"We're trying to track when the legacy thread for submission {submissionId} goes inactive, but that submission has not been registered."); return legacyThreadingEvents.Item2.ToTask(); } @@ -168,7 +168,7 @@ internal void SignalLegacyThreadStart(RequestBuilder instance) _legacyThreadingEventsById.TryGetValue(submissionId, out legacyThreadingEvents); } - ErrorUtilities.VerifyThrow(legacyThreadingEvents != null, "We're trying to signal that the legacy thread is ready for submission {0} to execute, but that submission has not been registered", submissionId); + ErrorUtilities.VerifyThrow(legacyThreadingEvents != null, $"We're trying to signal that the legacy thread is ready for submission {submissionId} to execute, but that submission has not been registered"); // signal that this submission is currently controlling the legacy thread legacyThreadingEvents.Item1.Set(); @@ -190,7 +190,7 @@ internal void SignalLegacyThreadEnd(int submissionId) _legacyThreadingEventsById.TryGetValue(submissionId, out legacyThreadingEvents); } - ErrorUtilities.VerifyThrow(legacyThreadingEvents != null, "We're trying to signal that submission {0} is done with the legacy thread, but that submission has not been registered", submissionId); + ErrorUtilities.VerifyThrow(legacyThreadingEvents != null, $"We're trying to signal that submission {submissionId} is done with the legacy thread, but that submission has not been registered"); // The legacy thread is now idle legacyThreadingEvents.Item2.Set(); diff --git a/src/Build/BackEnd/Client/MSBuildClientPacketPump.cs b/src/Build/BackEnd/Client/MSBuildClientPacketPump.cs index b23d417f9b7..de04a127a7a 100644 --- a/src/Build/BackEnd/Client/MSBuildClientPacketPump.cs +++ b/src/Build/BackEnd/Client/MSBuildClientPacketPump.cs @@ -261,7 +261,7 @@ private void RunReadLoop(Stream localStream, ManualResetEvent localPacketPumpShu } else { - ErrorUtilities.ThrowInternalError("Incomplete header read. {0} of {1} bytes read", headerBytesRead, headerByte.Length); + ErrorUtilities.ThrowInternalError($"Incomplete header read. {headerBytesRead} of {headerByte.Length} bytes read"); } } @@ -321,7 +321,7 @@ private void RunReadLoop(Stream localStream, ManualResetEvent localPacketPumpShu break; default: - ErrorUtilities.ThrowInternalError("WaitId {0} out of range.", waitId); + ErrorUtilities.ThrowInternalError($"WaitId {waitId} out of range."); break; } } diff --git a/src/Build/BackEnd/Components/BuildComponentFactoryCollection.cs b/src/Build/BackEnd/Components/BuildComponentFactoryCollection.cs index 872b2a62ab9..59e7a9d013e 100644 --- a/src/Build/BackEnd/Components/BuildComponentFactoryCollection.cs +++ b/src/Build/BackEnd/Components/BuildComponentFactoryCollection.cs @@ -136,7 +136,7 @@ public void ReplaceFactory(BuildComponentType componentType, BuildComponentFacto /// The instance to be registered. public void ReplaceFactory(BuildComponentType componentType, IBuildComponent instance) { - ErrorUtilities.VerifyThrow(_componentEntriesByType[componentType].Pattern == CreationPattern.Singleton, "Previously existing factory for type {0} was not a singleton factory.", componentType); + ErrorUtilities.VerifyThrow(_componentEntriesByType[componentType].Pattern == CreationPattern.Singleton, $"Previously existing factory for type {componentType} was not a singleton factory."); _componentEntriesByType[componentType] = new BuildComponentEntry(componentType, instance); } @@ -160,7 +160,7 @@ public IBuildComponent GetComponent(BuildComponentType type) { if (!_componentEntriesByType.TryGetValue(type, out BuildComponentEntry componentEntry)) { - ErrorUtilities.ThrowInternalError("No factory registered for component type {0}", type); + ErrorUtilities.ThrowInternalError($"No factory registered for component type {type}"); } return componentEntry.GetInstance(_host); diff --git a/src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEngine.cs b/src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEngine.cs index b5368c1f8c2..31eb9065599 100644 --- a/src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEngine.cs +++ b/src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEngine.cs @@ -4,14 +4,15 @@ using System; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; -using System.Globalization; using System.IO; +using System.Runtime.CompilerServices; using System.Threading; using System.Threading.Tasks.Dataflow; using Microsoft.Build.BackEnd.Logging; using Microsoft.Build.Execution; using Microsoft.Build.Experimental.BuildCheck.Infrastructure; using Microsoft.Build.Framework; +using Microsoft.Build.Framework.Utilities; using Microsoft.Build.Shared; using Microsoft.Build.Shared.Debugging; using Microsoft.Build.TelemetryInfra; @@ -125,9 +126,14 @@ internal class BuildRequestEngine : IBuildRequestEngine, IBuildComponent private readonly bool _debugDumpState; /// - /// The path where we will store debug files + /// The directory where we will store debug files. /// - private readonly string _debugDumpPath; + private readonly string _debugDumpDirectory; + + /// + /// The file where we will store debug files. + /// + private readonly string _debugDumpFilePath; #if FEATURE_WINDOWSINTEROP /// @@ -142,16 +148,18 @@ internal class BuildRequestEngine : IBuildRequestEngine, IBuildComponent internal BuildRequestEngine() { _debugDumpState = Traits.Instance.DebugScheduler; - _debugDumpPath = FrameworkDebugUtils.DebugPath; + _debugDumpDirectory = FrameworkDebugUtils.DebugPath; #if FEATURE_WINDOWSINTEROP _debugForceCaching = Environment.GetEnvironmentVariable("MSBUILDDEBUGFORCECACHING") == "1"; #endif - if (String.IsNullOrEmpty(_debugDumpPath)) + if (string.IsNullOrEmpty(_debugDumpDirectory)) { - _debugDumpPath = FileUtilities.TempFileDirectory; + _debugDumpDirectory = FileUtilities.TempFileDirectory; } + _debugDumpFilePath = Path.Combine(_debugDumpDirectory, $"EngineTrace_{EnvironmentUtilities.CurrentProcessId}.txt"); + _status = BuildRequestEngineStatus.Uninitialized; _nextUnresolvedConfigurationId = -1; _nextBuildRequestId = 0; @@ -211,7 +219,7 @@ internal BuildRequestEngine() public void InitializeForBuild(NodeLoggingContext loggingContext) { ErrorUtilities.VerifyThrow(_componentHost != null, "BuildRequestEngine not initialized by component host."); - ErrorUtilities.VerifyThrow(_status == BuildRequestEngineStatus.Uninitialized, "Engine must be in the Uninitiailzed state, but is {0}", _status); + ErrorUtilities.VerifyThrow(_status == BuildRequestEngineStatus.Uninitialized, $"Engine must be in the Uninitiailzed state, but is {_status}"); _nodeLoggingContext = loggingContext; @@ -241,8 +249,8 @@ public void CleanupForBuild() QueueAction( () => { - ErrorUtilities.VerifyThrow(_status == BuildRequestEngineStatus.Active || _status == BuildRequestEngineStatus.Idle || _status == BuildRequestEngineStatus.Waiting, "Engine must be Active, Idle or Waiting to clean up, but is {0}.", _status); - TraceEngine("CFB: Cleaning up build. Requests Count {0} Status {1}", _requests.Count, _status); + ErrorUtilities.VerifyThrow(_status == BuildRequestEngineStatus.Active || _status == BuildRequestEngineStatus.Idle || _status == BuildRequestEngineStatus.Waiting, $"Engine must be Active, Idle or Waiting to clean up, but is {_status}."); + TraceEngine($"CFB: Cleaning up build. Requests Count {_requests.Count} Status {_status}"); ErrorUtilities.VerifyThrow(_nodeLoggingContext != null, "Node logging context not set."); // Determine how many requests there are to shut down, then terminate all of their builders. @@ -265,7 +273,7 @@ public void CleanupForBuild() } catch (Exception e) { - TraceEngine("CFB: Shutting down request {0}({1}) (nr {2}) failed due to exception: {3}", entry.Request.GlobalRequestId, entry.Request.ConfigurationId, entry.Request.NodeRequestId, e.ToString()); + TraceEngine($"CFB: Shutting down request {entry.Request.GlobalRequestId}({entry.Request.ConfigurationId}) (nr {entry.Request.NodeRequestId}) failed due to exception: {e}"); if (ExceptionHandling.IsCriticalException(e)) { throw; @@ -284,7 +292,7 @@ public void CleanupForBuild() } catch (Exception e) { - TraceEngine("CFB: Shutting down request {0}({1}) (nr {2}) failed due to exception: {3}", entry.Request.GlobalRequestId, entry.Request.ConfigurationId, entry.Request.NodeRequestId, e.ToString()); + TraceEngine($"CFB: Shutting down request {entry.Request.GlobalRequestId}({entry.Request.ConfigurationId}) (nr {entry.Request.NodeRequestId}) failed due to exception: {e}"); if (ExceptionHandling.IsCriticalException(e)) { throw; @@ -299,7 +307,7 @@ public void CleanupForBuild() foreach (BuildRequestEntry entry in requestsToShutdown) { BuildResult result = entry.Result ?? new BuildResult(entry.Request, new BuildAbortedException()); - TraceEngine("CFB: Request is now {0}({1}) (nr {2}) has been deactivated.", entry.Request.GlobalRequestId, entry.Request.ConfigurationId, entry.Request.NodeRequestId); + TraceEngine($"CFB: Request is now {entry.Request.GlobalRequestId}({entry.Request.ConfigurationId}) (nr {entry.Request.NodeRequestId}) has been deactivated."); RaiseRequestComplete(entry.Request, result); } @@ -329,7 +337,7 @@ public void CleanupForBuild() catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) { // If we caught an exception during cleanup, we need to log that - ErrorUtilities.ThrowInternalError("Failure during engine shutdown. Exception: {0}", e.ToString()); + ErrorUtilities.ThrowInternalError($"Failure during engine shutdown. Exception: {e}"); } finally { @@ -360,11 +368,11 @@ public void SubmitBuildRequest(BuildRequest request) QueueAction( () => { - ErrorUtilities.VerifyThrow(_status != BuildRequestEngineStatus.Shutdown && _status != BuildRequestEngineStatus.Uninitialized, "Engine loop not yet started, status is {0}.", _status.Box()); - TraceEngine("Request {0}({1}) (nr {2}) received and activated.", request.GlobalRequestId, request.ConfigurationId, request.NodeRequestId); + ErrorUtilities.VerifyThrow(_status != BuildRequestEngineStatus.Shutdown && _status != BuildRequestEngineStatus.Uninitialized, $"Engine loop not yet started, status is {_status}."); + TraceEngine($"Request {request.GlobalRequestId}({request.ConfigurationId}) (nr {request.NodeRequestId}) received and activated."); - ErrorUtilities.VerifyThrow(!_requestsByGlobalRequestId.ContainsKey(request.GlobalRequestId), "Request {0} is already known to the engine.", request.GlobalRequestId); - ErrorUtilities.VerifyThrow(_configCache.HasConfiguration(request.ConfigurationId), "Request {0} refers to configuration {1} which is not known to the engine.", request.GlobalRequestId, request.ConfigurationId); + ErrorUtilities.VerifyThrow(!_requestsByGlobalRequestId.ContainsKey(request.GlobalRequestId), $"Request {request.GlobalRequestId} is already known to the engine."); + ErrorUtilities.VerifyThrow(_configCache.HasConfiguration(request.ConfigurationId), $"Request {request.GlobalRequestId} refers to configuration {request.ConfigurationId} which is not known to the engine."); if (request.NodeRequestId == BuildRequest.ResultsTransferNodeRequestId) { @@ -385,7 +393,7 @@ public void SubmitBuildRequest(BuildRequest request) resultToReport.ProjectStateAfterBuild = config.Project; } - TraceEngine("Request {0}({1}) (nr {2}) retrieved results for configuration {3} from node {4} for transfer.", request.GlobalRequestId, request.ConfigurationId, request.NodeRequestId, request.ConfigurationId, _componentHost.BuildParameters.NodeId); + TraceEngine($"Request {request.GlobalRequestId}({request.ConfigurationId}) (nr {request.NodeRequestId}) retrieved results for configuration {request.ConfigurationId} from node {_componentHost.BuildParameters.NodeId} for transfer."); // If this is the inproc node, we've already set the configuration's ResultsNodeId to the correct value in // HandleRequestBlockedOnResultsTransfer, and don't want to set it again, because we actually have less @@ -445,15 +453,15 @@ public void UnblockBuildRequest(BuildRequestUnblocker unblocker) QueueAction( () => { - ErrorUtilities.VerifyThrow(_status != BuildRequestEngineStatus.Shutdown && _status != BuildRequestEngineStatus.Uninitialized, "Engine loop not yet started, status is {0}.", _status.Box()); - ErrorUtilities.VerifyThrow(_requestsByGlobalRequestId.ContainsKey(unblocker.BlockedRequestId), "Request {0} is not known to the engine.", unblocker.BlockedRequestId); + ErrorUtilities.VerifyThrow(_status != BuildRequestEngineStatus.Shutdown && _status != BuildRequestEngineStatus.Uninitialized, $"Engine loop not yet started, status is {_status}."); + ErrorUtilities.VerifyThrow(_requestsByGlobalRequestId.ContainsKey(unblocker.BlockedRequestId), $"Request {unblocker.BlockedRequestId} is not known to the engine."); BuildRequestEntry entry = _requestsByGlobalRequestId[unblocker.BlockedRequestId]; // Are we resuming execution or reporting results? if (unblocker.Result == null) { // We are resuming execution. - TraceEngine("Request {0}({1}) (nr {2}) is now proceeding from current state {3}.", entry.Request.GlobalRequestId, entry.Request.ConfigurationId, entry.Request.NodeRequestId, entry.State); + TraceEngine($"Request {entry.Request.GlobalRequestId}({entry.Request.ConfigurationId}) (nr {entry.Request.NodeRequestId}) is now proceeding from current state {entry.State}."); // UNDONE: (Refactor) This is a bit icky because we still have the concept of blocking on an in-progress request // versus blocking on requests waiting for results. They come to the same thing, and its been rationalized correctly in @@ -474,7 +482,7 @@ public void UnblockBuildRequest(BuildRequestUnblocker unblocker) if (result.NodeRequestId == BuildRequest.ResultsTransferNodeRequestId) { - TraceEngine("Request {0}({1}) (nr {2}) has retrieved the results for configuration {3} and cached them on node {4} (UBR).", entry.Request.GlobalRequestId, entry.Request.ConfigurationId, entry.Request.NodeRequestId, entry.Request.ConfigurationId, _componentHost.BuildParameters.NodeId); + TraceEngine($"Request {entry.Request.GlobalRequestId}({entry.Request.ConfigurationId}) (nr {entry.Request.NodeRequestId}) has retrieved the results for configuration {entry.Request.ConfigurationId} and cached them on node {_componentHost.BuildParameters.NodeId} (UBR)."); IResultsCache resultsCache = (IResultsCache)_componentHost.GetComponent(BuildComponentType.ResultsCache); IConfigCache configCache = (IConfigCache)_componentHost.GetComponent(BuildComponentType.ConfigCache); @@ -501,7 +509,7 @@ public void UnblockBuildRequest(BuildRequestUnblocker unblocker) // PERF: Explicitly check the debug flag here so that we don't pay the cost for getting OverallResult if (_debugDumpState) { - TraceEngine("Request {0}({1}) (nr {2}) is no longer waiting on nr {3} (UBR). Results are {4}.", entry.Request.GlobalRequestId, entry.Request.ConfigurationId, entry.Request.NodeRequestId, result.NodeRequestId, result.OverallResult); + TraceEngine($"Request {entry.Request.GlobalRequestId}({entry.Request.ConfigurationId}) (nr {entry.Request.NodeRequestId}) is no longer waiting on nr {result.NodeRequestId} (UBR). Results are {result.OverallResult}."); } // Update the configuration with targets information, if we received any and didn't already have it. @@ -550,9 +558,9 @@ public void ReportConfigurationResponse(BuildRequestConfigurationResponse respon QueueAction( () => { - ErrorUtilities.VerifyThrow(_status != BuildRequestEngineStatus.Shutdown && _status != BuildRequestEngineStatus.Uninitialized, "Engine loop not yet started, status is {0}.", _status.Box()); + ErrorUtilities.VerifyThrow(_status != BuildRequestEngineStatus.Shutdown && _status != BuildRequestEngineStatus.Uninitialized, $"Engine loop not yet started, status is {_status}."); - TraceEngine("Received configuration response for node config {0}, now global config {1}.", response.NodeConfigurationId, response.GlobalConfigurationId); + TraceEngine($"Received configuration response for node config {response.NodeConfigurationId}, now global config {response.GlobalConfigurationId}."); ErrorUtilities.VerifyThrow(_componentHost != null, "No host object set"); // Remove the unresolved configuration entry from the unresolved cache. @@ -602,11 +610,7 @@ public void ReportConfigurationResponse(BuildRequestConfigurationResponse respon // We have a result, give it back to this request. currentEntry.ReportResult(cacheResponse.Results); - TraceEngine( - "Request {0} (node request {1}) with targets ({2}) satisfied from cache", - request.GlobalRequestId, - request.NodeRequestId, - string.Join(";", request.Targets)); + TraceEngine($"Request {request.GlobalRequestId} (node request {request.NodeRequestId}) with targets ({string.Join(";", request.Targets)}) satisfied from cache"); } else { @@ -661,7 +665,7 @@ public void InitializeComponent(IBuildComponentHost host) /// public void ShutdownComponent() { - ErrorUtilities.VerifyThrow(_status == BuildRequestEngineStatus.Uninitialized, "Cleanup wasn't called, status is {0}", _status); + ErrorUtilities.VerifyThrow(_status == BuildRequestEngineStatus.Uninitialized, $"Cleanup wasn't called, status is {_status}"); _componentHost = null; ChangeStatus(BuildRequestEngineStatus.Shutdown); @@ -674,7 +678,7 @@ public void ShutdownComponent() /// internal static IBuildComponent CreateComponent(BuildComponentType type) { - ErrorUtilities.VerifyThrow(type == BuildComponentType.RequestEngine, "Cannot create component of type {0}", type); + ErrorUtilities.VerifyThrow(type == BuildComponentType.RequestEngine, $"Cannot create component of type {type}"); return new BuildRequestEngine(); } @@ -700,7 +704,7 @@ private void RaiseRequestComplete(BuildRequest request, BuildResult result) RequestCompleteDelegate requestComplete = OnRequestComplete; if (requestComplete != null) { - TraceEngine("RRC: Reporting result for request {0}({1}) (nr {2}).", request.GlobalRequestId, request.ConfigurationId, request.NodeRequestId); + TraceEngine($"RRC: Reporting result for request {request.GlobalRequestId}({request.ConfigurationId}) (nr {request.NodeRequestId})."); requestComplete(request, result); } } @@ -785,13 +789,13 @@ private void EvaluateRequestStates() case BuildRequestEntryState.Active: ErrorUtilities.VerifyThrow(activeEntry == null, "Multiple active requests"); activeEntry = currentEntry; - TraceEngine("ERS: Active request is now {0}({1}) (nr {2}).", currentEntry.Request.GlobalRequestId, currentEntry.Request.ConfigurationId, currentEntry.Request.NodeRequestId); + TraceEngine($"ERS: Active request is now {currentEntry.Request.GlobalRequestId}({currentEntry.Request.ConfigurationId}) (nr {currentEntry.Request.NodeRequestId})."); break; // This request is now complete. case BuildRequestEntryState.Complete: completedEntries.Add(currentEntry); - TraceEngine("ERS: Request {0}({1}) (nr {2}) is marked as complete.", currentEntry.Request.GlobalRequestId, currentEntry.Request.ConfigurationId, currentEntry.Request.NodeRequestId); + TraceEngine($"ERS: Request {currentEntry.Request.GlobalRequestId}({currentEntry.Request.ConfigurationId}) (nr {currentEntry.Request.NodeRequestId}) is marked as complete."); break; // This request is waiting for configurations or results @@ -817,7 +821,7 @@ private void EvaluateRequestStates() // Remove completed requests foreach (BuildRequestEntry completedEntry in completedEntries) { - TraceEngine("ERS: Request {0}({1}) (nr {2}) is being removed from the requests list.", completedEntry.Request.GlobalRequestId, completedEntry.Request.ConfigurationId, completedEntry.Request.NodeRequestId); + TraceEngine($"ERS: Request {completedEntry.Request.GlobalRequestId}({completedEntry.Request.ConfigurationId}) (nr {completedEntry.Request.NodeRequestId}) is being removed from the requests list."); _requests.Remove(completedEntry); _requestsByGlobalRequestId.Remove(completedEntry.Request.GlobalRequestId); } @@ -871,7 +875,7 @@ private void EvaluateRequestStates() completedEntry.Result.ProjectTargets = configuration.ProjectTargets; } - TraceEngine("ERS: Request is now {0}({1}) (nr {2}) has had its builder cleaned up.", completedEntry.Request.GlobalRequestId, completedEntry.Request.ConfigurationId, completedEntry.Request.NodeRequestId); + TraceEngine($"ERS: Request is now {completedEntry.Request.GlobalRequestId}({completedEntry.Request.ConfigurationId}) (nr {completedEntry.Request.NodeRequestId}) has had its builder cleaned up."); RaiseRequestComplete(completedEntry.Request, completedEntry.Result); } } @@ -922,10 +926,7 @@ private void CheckMemoryUsage() ulong memoryInUse = memoryStatus.ullTotalVirtual - memoryStatus.ullAvailVirtual; while ((memoryInUse > memoryUseLimit) || _debugForceCaching) { - TraceEngine( - "Memory usage at {0}, limit is {1}. Caching configurations and results cache and collecting.", - memoryInUse, - memoryUseLimit); + TraceEngine($"Memory usage at {memoryInUse}, limit is {memoryUseLimit}. Caching configurations and results cache and collecting."); IResultsCache resultsCache = _componentHost.GetComponent(BuildComponentType.ResultsCache) as IResultsCache; @@ -950,7 +951,7 @@ private void CheckMemoryUsage() } memoryInUse = memoryStatus.ullTotalVirtual - memoryStatus.ullAvailVirtual; - TraceEngine("Memory usage now at {0}", memoryInUse); + TraceEngine($"Memory usage now at {memoryInUse}"); } } catch (Exception e) when (ExceptionHandling.IsIoRelatedException(e)) @@ -1244,12 +1245,7 @@ private void IssueBuildRequests(BuildRequestEntry issuingEntry, FullyQualifiedBu if (matchingConfig == null) { // Issue the config resolution request - TraceEngine( - "Request {0}({1}) (nr {2}) is waiting on configuration {3} (IBR)", - issuingEntry.Request.GlobalRequestId, - issuingEntry.Request.ConfigurationId, - issuingEntry.Request.NodeRequestId, - request.Config.ConfigurationId); + TraceEngine($"Request {issuingEntry.Request.GlobalRequestId}({issuingEntry.Request.ConfigurationId}) (nr {issuingEntry.Request.NodeRequestId}) is waiting on configuration {request.Config.ConfigurationId} (IBR)"); issuingEntry.WaitForConfiguration(request.Config); } } @@ -1284,11 +1280,7 @@ private void IssueBuildRequests(BuildRequestEntry issuingEntry, FullyQualifiedBu // Log the fact that we handled this from the cache. _nodeLoggingContext.LogRequestHandledFromCache(newRequest, _configCache[newRequest.ConfigurationId], response.Results); - TraceEngine( - "Request {0} (node request {1}) with targets ({2}) satisfied from cache", - newRequest.GlobalRequestId, - newRequest.NodeRequestId, - string.Join(",", request.Targets)); + TraceEngine($"Request {newRequest.GlobalRequestId} (node request {newRequest.NodeRequestId}) with targets ({string.Join(",", request.Targets)}) satisfied from cache"); // Can't report the result directly here, because that could cause the request to go from // Waiting to Ready. @@ -1407,7 +1399,7 @@ private void IssueConfigurationRequest(BuildRequestConfiguration config) ErrorUtilities.VerifyThrow(config.WasGeneratedByNode, "InvalidConfigurationId"); ErrorUtilities.VerifyThrowArgumentNull(config); ErrorUtilities.VerifyThrow(_unresolvedConfigurationsById.ContainsKey(config.ConfigurationId), "NoUnresolvedConfiguration"); - TraceEngine("Issuing configuration request for node config {0}", config.ConfigurationId); + TraceEngine($"Issuing configuration request for node config {config.ConfigurationId}"); RaiseNewConfigurationRequest(config); } @@ -1422,13 +1414,13 @@ private void IssueBuildRequest(BuildRequestBlocker blocker) if (blocker.BuildRequests == null) { // This is the case when we aren't blocking on new requests, but rather an in-progress request which is executing a target for which we need results. - TraceEngine("Blocking global request {0} on global request {1} because it is already executing target {2}", blocker.BlockedRequestId, blocker.BlockingRequestId, blocker.BlockingTarget); + TraceEngine($"Blocking global request {blocker.BlockedRequestId} on global request {blocker.BlockingRequestId} because it is already executing target {blocker.BlockingTarget}"); } else { foreach (BuildRequest blockingRequest in blocker.BuildRequests) { - TraceEngine("Sending node request {0} (configuration {1}) with parent {2} to Build Manager", blockingRequest.NodeRequestId, blockingRequest.ConfigurationId, blocker.BlockedRequestId); + TraceEngine($"Sending node request {blockingRequest.NodeRequestId} (configuration {blockingRequest.ConfigurationId}) with parent {blocker.BlockedRequestId} to Build Manager"); } } @@ -1457,7 +1449,7 @@ private void QueueAction(Action action, bool isLastTask) } catch (Exception e) { - TraceEngine("EL: EXCEPTION caught in engine: {0} - {1}", e.GetType(), e.Message); + TraceEngine($"EL: EXCEPTION caught in engine: {e.GetType()} - {e.Message}"); // Dump all engine exceptions to a temp file // so that we have something to go on in the @@ -1490,121 +1482,69 @@ private void QueueAction(Action action, bool isLastTask) } } -#if FEATURE_WINDOWSINTEROP - private void TraceEngine(string format, ulong arg) - { - if (_debugDumpState) - { - TraceEngine(format, [arg]); - } - } - - private void TraceEngine(string format, ulong arg1, ulong arg2) - { - if (_debugDumpState) - { - TraceEngine(format, [arg1, arg2]); - } - } -#endif - - private void TraceEngine(string format, int arg) - { - if (_debugDumpState) - { - TraceEngine(format, [arg]); - } - } - - private void TraceEngine(string format, int arg1, BuildRequestEngineStatus arg2) + private void TraceEngine(string message) { - if (_debugDumpState) + if (!_debugDumpState) { - TraceEngine(format, [arg1, arg2.Box()]); + return; } - } - private void TraceEngine(string format, int arg1, int arg2) - { - if (_debugDumpState) + lock (s_traceLock) { - TraceEngine(format, [arg1, arg2]); - } - } + try + { + FileUtilities.EnsureDirectoryExists(_debugDumpDirectory); - private void TraceEngine(string format, int arg1, int arg2, int arg3) - { - if (_debugDumpState) - { - TraceEngine(format, [arg1, arg2, arg3]); + using (StreamWriter file = FileUtilities.OpenWrite(_debugDumpFilePath, append: true)) + { + file.WriteLine("{0}({1})-{2}: {3}", Thread.CurrentThread.Name, Environment.CurrentManagedThreadId, DateTime.UtcNow.Ticks, message); + file.Flush(); + } + } + catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) + { + // Trace file failures must never crash the build engine. + // Matches the defensive pattern used by Scheduler.TraceScheduler. + _nodeLoggingContext?.LogCommentFromText(MessageImportance.Low, $"Failed to write to engine trace file: {e}"); + } } } - private void TraceEngine(string format, int arg1, int arg2, string arg3) + private void TraceEngine([InterpolatedStringHandlerArgument("")] ref TraceInterpolatedStringHandler handler) { if (_debugDumpState) { - TraceEngine(format, [arg1, arg2, arg3]); + TraceEngine(handler.GetFormattedText()); } } - private void TraceEngine(string format, int arg1, int arg2, int arg3, string arg4) + /// + /// Interpolated string handler used by + /// to defer string formatting unless tracing is enabled. + /// + [InterpolatedStringHandler] + private ref struct TraceInterpolatedStringHandler { - if (_debugDumpState) - { - TraceEngine(format, [arg1, arg2, arg3, arg4]); - } - } + private StringBuilderHelper _builder; - private void TraceEngine(string format, int arg1, int arg2, int arg3, BuildRequestEntryState arg4) - { - if (_debugDumpState) + public TraceInterpolatedStringHandler(int literalLength, int formattedCount, BuildRequestEngine engine, out bool isEnabled) { - TraceEngine(format, [arg1, arg2, arg3, arg4]); + isEnabled = engine._debugDumpState; + _builder = isEnabled ? new(literalLength) : default; } - } - private void TraceEngine(string format, int arg1, int arg2, int arg3, int arg4, int arg5) - { - if (_debugDumpState) - { - TraceEngine(format, [arg1, arg2, arg3, arg4, arg5]); - } - } + public readonly void AppendLiteral(string value) + => _builder.AppendLiteral(value); - private void TraceEngine(string format, int arg1, int arg2, int arg3, int arg4, BuildResultCode arg5) - { - if (_debugDumpState) - { - TraceEngine(format, [arg1, arg2, arg3, arg4, arg5]); - } - } + public readonly void AppendFormatted(TValue value) + => _builder.AppendFormatted(value); - private void TraceEngine(string format, params object[] stuff) - { - if (_debugDumpState) - { - lock (s_traceLock) - { - try - { - FileUtilities.EnsureDirectoryExists(_debugDumpPath); + public readonly void AppendFormatted(TValue value, string format) + where TValue : IFormattable + => _builder.AppendFormatted(value, format); - 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, Environment.CurrentManagedThreadId, DateTime.UtcNow.Ticks, message); - file.Flush(); - } - } - catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) - { - // Trace file failures must never crash the build engine. - // Matches the defensive pattern used by Scheduler.TraceScheduler. - _nodeLoggingContext?.LogCommentFromText(MessageImportance.Low, $"Failed to write to engine trace file: {e}"); - } - } - } + public string GetFormattedText() + => _builder.GetFormattedText(); } /// diff --git a/src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEntry.cs b/src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEntry.cs index 074b8df2057..cf837871c65 100644 --- a/src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEntry.cs +++ b/src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEntry.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; @@ -223,7 +223,7 @@ public void WaitForBlockingRequest(int blockingGlobalRequestId) { lock (GlobalLock) { - ErrorUtilities.VerifyThrow(State == BuildRequestEntryState.Active, "Must be in Active state to wait for blocking request. Config: {0} State: {1}", RequestConfiguration.ConfigurationId, State); + ErrorUtilities.VerifyThrow(State == BuildRequestEntryState.Active, $"Must be in Active state to wait for blocking request. Config: {RequestConfiguration.ConfigurationId} State: {State}"); _blockingGlobalRequestId = blockingGlobalRequestId; @@ -314,11 +314,9 @@ public void ReportResult(BuildResult result) { ErrorUtilities.VerifyThrowArgumentNull(result); - // PERF: Check the condition and then throw rather than using VerifyThrow to avoid the allocations that happen when boxing the message arguments. - if (!(State == BuildRequestEntryState.Waiting || _outstandingRequests == null)) - { - ErrorUtilities.ThrowInternalError("Entry must be in the Waiting state to report results, or we must have flushed our requests due to an error. Config: {0} State: {1} Requests: {2}", RequestConfiguration.ConfigurationId, State, _outstandingRequests != null); - } + ErrorUtilities.VerifyThrow( + State == BuildRequestEntryState.Waiting || _outstandingRequests == null, + $"Entry must be in the Waiting state to report results, or we must have flushed our requests due to an error. Config: {RequestConfiguration.ConfigurationId} State: {State} Requests: {_outstandingRequests != null}"); // If the matching request is in the issue list, remove it so we don't try to ask for it to be built. if (_requestsToIssue != null) @@ -396,8 +394,8 @@ public void Unblock() { lock (GlobalLock) { - ErrorUtilities.VerifyThrow(State == BuildRequestEntryState.Waiting, "Entry must be in the waiting state to be unblocked. Config: {0} State: {1} Request: {2}", RequestConfiguration.ConfigurationId, State, Request.GlobalRequestId); - ErrorUtilities.VerifyThrow(_blockingGlobalRequestId != BuildRequest.InvalidGlobalRequestId, "Entry must be waiting on another request to be unblocked. Config: {0} Request: {1}", RequestConfiguration.ConfigurationId, Request.GlobalRequestId); + ErrorUtilities.VerifyThrow(State == BuildRequestEntryState.Waiting, $"Entry must be in the waiting state to be unblocked. Config: {RequestConfiguration.ConfigurationId} State: {State} Request: {Request.GlobalRequestId}"); + ErrorUtilities.VerifyThrow(_blockingGlobalRequestId != BuildRequest.InvalidGlobalRequestId, $"Entry must be waiting on another request to be unblocked. Config: {RequestConfiguration.ConfigurationId} Request: {Request.GlobalRequestId}"); _blockingGlobalRequestId = BuildRequest.InvalidGlobalRequestId; @@ -416,7 +414,7 @@ public IDictionary Continue() { ErrorUtilities.VerifyThrow(_unresolvedConfigurations == null, "All configurations must be resolved before Continue may be called."); ErrorUtilities.VerifyThrow(_outstandingRequests == null, "All outstanding requests must have been satisfied."); - ErrorUtilities.VerifyThrow(State == BuildRequestEntryState.Ready, "Entry must be in the Ready state. Config: {0} State: {1}", RequestConfiguration.ConfigurationId, State); + ErrorUtilities.VerifyThrow(State == BuildRequestEntryState.Ready, $"Entry must be in the Ready state. Config: {RequestConfiguration.ConfigurationId} State: {State}"); IDictionary ret = _outstandingResults; _outstandingResults = null; @@ -493,7 +491,7 @@ public void Complete(BuildResult result) // of states. if (result.OverallResult == BuildResultCode.Success) { - ErrorUtilities.VerifyThrow(State == BuildRequestEntryState.Active, "Entry must be active before it can be Completed successfully. Config: {0} State: {1}", RequestConfiguration.ConfigurationId, State); + ErrorUtilities.VerifyThrow(State == BuildRequestEntryState.Active, $"Entry must be active before it can be Completed successfully. Config: {RequestConfiguration.ConfigurationId} State: {State}"); ErrorUtilities.VerifyThrow(_unresolvedConfigurations == null, "Entry must not have any unresolved configurations."); ErrorUtilities.VerifyThrow(_outstandingRequests == null, "Entry must have no outstanding requests."); ErrorUtilities.VerifyThrow(_outstandingResults == null, "Results must be consumed before request may be completed."); @@ -514,13 +512,13 @@ private void WaitForResult(BuildRequest newRequest, bool addToIssueList) { lock (GlobalLock) { - ErrorUtilities.VerifyThrow(State == BuildRequestEntryState.Active || State == BuildRequestEntryState.Waiting, "Must be in Active or Waiting state to wait for results. Config: {0} State: {1}", RequestConfiguration.ConfigurationId, State); + ErrorUtilities.VerifyThrow(State == BuildRequestEntryState.Active || State == BuildRequestEntryState.Waiting, $"Must be in Active or Waiting state to wait for results. Config: {RequestConfiguration.ConfigurationId} State: {State}"); if (newRequest.IsConfigurationResolved) { _outstandingRequests ??= new Dictionary(); - ErrorUtilities.VerifyThrow(!_outstandingRequests.ContainsKey(newRequest.NodeRequestId), "Already waiting for local request {0}", newRequest.NodeRequestId); + ErrorUtilities.VerifyThrow(!_outstandingRequests.ContainsKey(newRequest.NodeRequestId), $"Already waiting for local request {newRequest.NodeRequestId}"); _outstandingRequests.Add(newRequest.NodeRequestId, newRequest); } else diff --git a/src/Build/BackEnd/Components/BuildRequestEngine/IBuildRequestEngine.cs b/src/Build/BackEnd/Components/BuildRequestEngine/IBuildRequestEngine.cs index 75708489039..fac725d8149 100644 --- a/src/Build/BackEnd/Components/BuildRequestEngine/IBuildRequestEngine.cs +++ b/src/Build/BackEnd/Components/BuildRequestEngine/IBuildRequestEngine.cs @@ -86,38 +86,6 @@ internal enum BuildRequestEngineStatus Shutdown } - /// - /// Provides boxed representations of the enumeration values. - /// - /// This class offers pre-boxed objects for each status value to avoid repeated allocations due to boxing - /// when frequently accessing these status values. It also includes an extension method to retrieve the - /// boxed object corresponding to a given . - internal static class BuildRequestEngineStatusBoxes - { - public static readonly object UninitializedBox = BuildRequestEngineStatus.Uninitialized; - public static readonly object IdleBox = BuildRequestEngineStatus.Idle; - public static readonly object ActiveBox = BuildRequestEngineStatus.Active; - public static readonly object WaitingBox = BuildRequestEngineStatus.Waiting; - public static readonly object ShutdownBox = BuildRequestEngineStatus.Shutdown; - - /// - /// Gets the canonical boxed object for the specified . - /// - /// The desired . - /// The boxed . - /// Thrown when is outside the range of - /// defined values. - public static object Box(this BuildRequestEngineStatus status) => status switch - { - BuildRequestEngineStatus.Uninitialized => UninitializedBox, - BuildRequestEngineStatus.Idle => IdleBox, - BuildRequestEngineStatus.Active => ActiveBox, - BuildRequestEngineStatus.Waiting => WaitingBox, - BuildRequestEngineStatus.Shutdown => ShutdownBox, - _ => throw new ArgumentOutOfRangeException(nameof(status)), - }; - } - /// /// Objects implementing this interface may be used by a Node to process build requests /// and generate build results. diff --git a/src/Build/BackEnd/Components/Caching/ConfigCache.cs b/src/Build/BackEnd/Components/Caching/ConfigCache.cs index 64babd9b24e..d92b93a6218 100644 --- a/src/Build/BackEnd/Components/Caching/ConfigCache.cs +++ b/src/Build/BackEnd/Components/Caching/ConfigCache.cs @@ -78,7 +78,7 @@ private void AddConfiguration(BuildRequestConfiguration config, Configurations c if (!configurations.ById.TryAdd(config.ConfigurationId, config)) { - ErrorUtilities.ThrowInternalError("Configuration {0} already cached", config.ConfigurationId); + ErrorUtilities.ThrowInternalError($"Configuration {config.ConfigurationId} already cached"); } _ = configurations.ByMetadata[new ConfigurationMetadata(config)] = config; @@ -338,7 +338,7 @@ public void Translate(ITranslator translator) /// internal static IBuildComponent CreateComponent(BuildComponentType componentType) { - ErrorUtilities.VerifyThrow(componentType == BuildComponentType.ConfigCache, "Cannot create components of type {0}", componentType); + ErrorUtilities.VerifyThrow(componentType == BuildComponentType.ConfigCache, $"Cannot create components of type {componentType}"); return new ConfigCache(); } diff --git a/src/Build/BackEnd/Components/Caching/RegisteredTaskObjectCache.cs b/src/Build/BackEnd/Components/Caching/RegisteredTaskObjectCache.cs index aaf90ecaa5d..b8f4c4a972a 100644 --- a/src/Build/BackEnd/Components/Caching/RegisteredTaskObjectCache.cs +++ b/src/Build/BackEnd/Components/Caching/RegisteredTaskObjectCache.cs @@ -57,7 +57,7 @@ public void Dispose() /// internal static IBuildComponent CreateComponent(BuildComponentType type) { - ErrorUtilities.VerifyThrow(type == BuildComponentType.RegisteredTaskObjectCache, "Cannot create components of type {0}", type); + ErrorUtilities.VerifyThrow(type == BuildComponentType.RegisteredTaskObjectCache, $"Cannot create components of type {type}"); return new RegisteredTaskObjectCache(); } diff --git a/src/Build/BackEnd/Components/Caching/ResultsCache.cs b/src/Build/BackEnd/Components/Caching/ResultsCache.cs index 60b8a0dc12c..e85fd95dddd 100644 --- a/src/Build/BackEnd/Components/Caching/ResultsCache.cs +++ b/src/Build/BackEnd/Components/Caching/ResultsCache.cs @@ -83,7 +83,7 @@ public void AddResult(BuildResult result) // building them again. if (!_resultsByConfiguration.TryAdd(result.ConfigurationId, result)) { - ErrorUtilities.ThrowInternalError("Failed to add result for configuration {0}", result.ConfigurationId); + ErrorUtilities.ThrowInternalError($"Failed to add result for configuration {result.ConfigurationId}"); } } } @@ -299,7 +299,7 @@ public void ShutdownComponent() /// internal static IBuildComponent CreateComponent(BuildComponentType componentType) { - ErrorUtilities.VerifyThrow(componentType == BuildComponentType.ResultsCache, "Cannot create components of type {0}", componentType); + ErrorUtilities.VerifyThrow(componentType == BuildComponentType.ResultsCache, $"Cannot create components of type {componentType}"); return new ResultsCache(); } diff --git a/src/Build/BackEnd/Components/Communications/NodeEndpointInProc.cs b/src/Build/BackEnd/Components/Communications/NodeEndpointInProc.cs index a41eb93edd2..620eb6f4cdd 100644 --- a/src/Build/BackEnd/Components/Communications/NodeEndpointInProc.cs +++ b/src/Build/BackEnd/Components/Communications/NodeEndpointInProc.cs @@ -217,7 +217,7 @@ public void Disconnect() /// The packet to send. public void SendData(INodePacket packet) { - ErrorUtilities.VerifyThrow(_status == LinkStatus.Active, "Cannot send when link status is not active. Current status {0}", _status); + ErrorUtilities.VerifyThrow(_status == LinkStatus.Active, $"Cannot send when link status is not active. Current status {_status}"); if (_mode == EndpointMode.Synchronous) { @@ -309,7 +309,7 @@ private void SetPeerNodeDisconnected() /// private void InternalDisconnect() { - ErrorUtilities.VerifyThrow(_status == LinkStatus.Active, "Endpoint is not connected. Current status {0}", _status); + ErrorUtilities.VerifyThrow(_status == LinkStatus.Active, $"Endpoint is not connected. Current status {_status}"); ChangeLinkStatus(LinkStatus.Inactive); @@ -326,7 +326,7 @@ private void InternalDisconnect() /// The status the node should now be in. private void ChangeLinkStatus(LinkStatus newStatus) { - ErrorUtilities.VerifyThrow(_status != newStatus, "Attempting to change status to existing status {0}.", _status); + ErrorUtilities.VerifyThrow(_status != newStatus, $"Attempting to change status to existing status {_status}."); _status = newStatus; RaiseLinkStatusChanged(_status); } @@ -451,7 +451,7 @@ private void PacketPumpProc() break; default: - ErrorUtilities.ThrowInternalError("waitId {0} out of range.", waitId); + ErrorUtilities.ThrowInternalError($"waitId {waitId} out of range."); break; } } diff --git a/src/Build/BackEnd/Components/Communications/NodeManager.cs b/src/Build/BackEnd/Components/Communications/NodeManager.cs index fb83ceaf5c1..9c8afbdebd6 100644 --- a/src/Build/BackEnd/Components/Communications/NodeManager.cs +++ b/src/Build/BackEnd/Components/Communications/NodeManager.cs @@ -123,7 +123,7 @@ public void SendData(int node, INodePacket packet) { if (!_nodeIdToProvider.TryGetValue(node, out INodeProvider? provider)) { - ErrorUtilities.ThrowInternalError("Node {0} does not have a provider.", node); + ErrorUtilities.ThrowInternalError($"Node {node} does not have a provider."); } else { @@ -287,7 +287,7 @@ public void RoutePacket(int nodeId, INodePacket packet) /// internal static IBuildComponent CreateComponent(BuildComponentType type) { - ErrorUtilities.VerifyThrow(type == BuildComponentType.NodeManager, "Cannot create component of type {0}", type); + ErrorUtilities.VerifyThrow(type == BuildComponentType.NodeManager, $"Cannot create component of type {type}"); return new NodeManager(); } diff --git a/src/Build/BackEnd/Components/Communications/NodeProviderInProc.cs b/src/Build/BackEnd/Components/Communications/NodeProviderInProc.cs index a089b9306ba..4facb734171 100644 --- a/src/Build/BackEnd/Components/Communications/NodeProviderInProc.cs +++ b/src/Build/BackEnd/Components/Communications/NodeProviderInProc.cs @@ -348,7 +348,7 @@ public void Dispose() /// internal static IBuildComponent CreateComponent(BuildComponentType type) { - ErrorUtilities.VerifyThrow(type == BuildComponentType.InProcNodeProvider, "Cannot create component of type {0}", type); + ErrorUtilities.VerifyThrow(type == BuildComponentType.InProcNodeProvider, $"Cannot create component of type {type}"); return new NodeProviderInProc(); } @@ -394,7 +394,7 @@ private bool InstantiateNode(int nodeId, INodePacketFactory factory) int connectionTimeout = CommunicationsUtilities.NodeConnectionTimeout; bool connected = nodeContext._endpointConnectedEvent.WaitOne(connectionTimeout); - ErrorUtilities.VerifyThrow(connected, "In-proc node failed to start up within {0}ms", connectionTimeout); + ErrorUtilities.VerifyThrow(connected, $"In-proc node failed to start up within {connectionTimeout}ms"); return true; } @@ -451,7 +451,7 @@ private void InProcNodeShutdown(NodeEngineShutdownReason reason, Exception e) break; case NodeEngineShutdownReason.ConnectionFailed: - ErrorUtilities.ThrowInternalError("Unexpected shutdown code {0} received.", reason); + ErrorUtilities.ThrowInternalError($"Unexpected shutdown code {reason} received."); break; } } diff --git a/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProc.cs b/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProc.cs index b56d83b19e0..3750059743c 100644 --- a/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProc.cs +++ b/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProc.cs @@ -84,7 +84,7 @@ public IList CreateNodes(int nextNodeId, INodePacketFactory factory, F // we add into _nodeContexts premise of future node and verify that it will not cross limits. if (_nodeContexts.Count + numberOfNodesToCreate > ComponentHost.BuildParameters.MaxNodeCount) { - ErrorUtilities.ThrowInternalError("Exceeded max node count of '{0}', current count is '{1}' ", ComponentHost.BuildParameters.MaxNodeCount, _nodeContexts.Count); + ErrorUtilities.ThrowInternalError($"Exceeded max node count of '{ComponentHost.BuildParameters.MaxNodeCount}', current count is '{_nodeContexts.Count}' "); return new List(); } @@ -135,7 +135,7 @@ void NodeContextCreated(NodeContext context) /// The packet to send. public void SendData(int nodeId, INodePacket packet) { - ErrorUtilities.VerifyThrow(_nodeContexts.ContainsKey(nodeId), "Invalid node id specified: {0}.", nodeId); + ErrorUtilities.VerifyThrow(_nodeContexts.ContainsKey(nodeId), $"Invalid node id specified: {nodeId}."); SendData(_nodeContexts[nodeId], packet); } @@ -198,7 +198,7 @@ public void ShutdownComponent() /// internal static IBuildComponent CreateComponent(BuildComponentType componentType) { - ErrorUtilities.VerifyThrow(componentType == BuildComponentType.OutOfProcNodeProvider, "Factory cannot create components of type {0}", componentType); + ErrorUtilities.VerifyThrow(componentType == BuildComponentType.OutOfProcNodeProvider, $"Factory cannot create components of type {componentType}"); return new NodeProviderOutOfProc(); } diff --git a/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcTaskHost.cs b/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcTaskHost.cs index b77c8bb6f6e..b86f689f60d 100644 --- a/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcTaskHost.cs +++ b/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcTaskHost.cs @@ -206,7 +206,7 @@ public void SendData(int nodeId, INodePacket packet) /// The packet to send. internal void SendData(TaskHostNodeKey nodeKey, INodePacket packet) { - ErrorUtilities.VerifyThrow(_nodeContexts.TryGetValue(nodeKey, out NodeContext context), "Invalid host context specified: {0}.", nodeKey); + ErrorUtilities.VerifyThrow(_nodeContexts.TryGetValue(nodeKey, out NodeContext context), $"Invalid host context specified: {nodeKey}."); SendData(context, packet); } @@ -374,7 +374,7 @@ public void PacketReceived(int node, INodePacket packet) break; default: - ErrorUtilities.ThrowInternalError("PacketReceived: no handler for node {0}, unexpected packet type {1}", node, packet.Type); + ErrorUtilities.ThrowInternalError($"PacketReceived: no handler for node {node}, unexpected packet type {packet.Type}"); break; } } @@ -386,7 +386,7 @@ public void PacketReceived(int node, INodePacket packet) /// internal static IBuildComponent CreateComponent(BuildComponentType componentType) { - ErrorUtilities.VerifyThrow(componentType == BuildComponentType.OutOfProcTaskHostNodeProvider, "Factory cannot create components of type {0}", componentType); + ErrorUtilities.VerifyThrow(componentType == BuildComponentType.OutOfProcTaskHostNodeProvider, $"Factory cannot create components of type {componentType}"); return new NodeProviderOutOfProcTaskHost(); } diff --git a/src/Build/BackEnd/Components/Communications/TaskHostNodeManager.cs b/src/Build/BackEnd/Components/Communications/TaskHostNodeManager.cs index 66c881052b8..0f56d7d755d 100644 --- a/src/Build/BackEnd/Components/Communications/TaskHostNodeManager.cs +++ b/src/Build/BackEnd/Components/Communications/TaskHostNodeManager.cs @@ -176,7 +176,7 @@ public void RoutePacket(int nodeId, INodePacket packet) /// internal static IBuildComponent CreateComponent(BuildComponentType type) { - ErrorUtilities.VerifyThrow(type == BuildComponentType.TaskHostNodeManager, "Cannot create component of type {0}", type); + ErrorUtilities.VerifyThrow(type == BuildComponentType.TaskHostNodeManager, $"Cannot create component of type {type}"); return new TaskHostNodeManager(); } diff --git a/src/Build/BackEnd/Components/Communications/TranslatorExtensions.cs b/src/Build/BackEnd/Components/Communications/TranslatorExtensions.cs index ab2b1bb2da0..15f8934c1a6 100644 --- a/src/Build/BackEnd/Components/Communications/TranslatorExtensions.cs +++ b/src/Build/BackEnd/Components/Communications/TranslatorExtensions.cs @@ -87,7 +87,7 @@ public static T FactoryForDeserializingTypeWithName(this ITranslator translat constructor = type.GetConstructor(BindingFlags.Instance | BindingFlags.NonPublic, null, Type.EmptyTypes, null); ErrorUtilities.VerifyThrow( constructor != null, - "{0} must have a private parameterless constructor", typeName); + $"{typeName} must have a private parameterless constructor"); return constructor; }); diff --git a/src/Build/BackEnd/Components/Logging/LoggingContext.cs b/src/Build/BackEnd/Components/Logging/LoggingContext.cs index a0c565ad491..f08d8845008 100644 --- a/src/Build/BackEnd/Components/Logging/LoggingContext.cs +++ b/src/Build/BackEnd/Components/Logging/LoggingContext.cs @@ -327,8 +327,7 @@ private protected void CheckValidity() { if (!_isValid) { - ErrorUtilities.ThrowInternalError("LoggingContext (type: {0}) was not valid during logging attempt.", - this.GetType()); + ErrorUtilities.ThrowInternalError($"LoggingContext (type: {this.GetType()}) was not valid during logging attempt."); } } } diff --git a/src/Build/BackEnd/Components/Logging/LoggingService.cs b/src/Build/BackEnd/Components/Logging/LoggingService.cs index 04ec5dead94..d9c20035de5 100644 --- a/src/Build/BackEnd/Components/Logging/LoggingService.cs +++ b/src/Build/BackEnd/Components/Logging/LoggingService.cs @@ -1003,11 +1003,9 @@ public void PacketReceived(int node, INodePacket packet) ErrorUtilities.VerifyThrow(packet != null, "packet was null"); // Expected the packet type to be a logging message packet - // PERF: Not using VerifyThrow to avoid allocations for enum.ToString (boxing of NodePacketType) in the non-error case. - if (packet.Type != NodePacketType.LogMessage) - { - ErrorUtilities.ThrowInternalError("Expected packet type \"{0}\" but instead got packet type \"{1}\".", nameof(NodePacketType.LogMessage), packet.Type.ToString()); - } + ErrorUtilities.VerifyThrow( + packet.Type == NodePacketType.LogMessage, + $"""Expected packet type "{nameof(NodePacketType.LogMessage)}" but instead got packet type "{packet.Type}"."""); LogMessagePacket loggingPacket = (LogMessagePacket)packet; InjectNonSerializedData(loggingPacket); @@ -1984,13 +1982,9 @@ private string GetAndVerifyProjectFileFromContext(BuildEventArgs eventArgs, bool BuildEventContext context = eventArgs.BuildEventContext!; _projectFileMap.TryGetValue(context.ProjectContextId, out string projectFile); - // PERF: Not using VerifyThrow to avoid boxing an int in the non-error case. - if (projectFile == null && !allowCacheMiss) - { - ErrorUtilities.ThrowInternalError( - "ContextID {0} should have been in the ID-to-project file mapping but wasn't! Encountered during logging message: '{1}'", - context.ProjectContextId, eventArgs.Message); - } + ErrorUtilities.VerifyThrow( + projectFile != null || allowCacheMiss, + $"ContextID {context.ProjectContextId} should have been in the ID-to-project file mapping but wasn't! Encountered during logging message: '{eventArgs.Message}'"); return projectFile; } diff --git a/src/Build/BackEnd/Components/Logging/LoggingServiceFactory.cs b/src/Build/BackEnd/Components/Logging/LoggingServiceFactory.cs index e4bbb388650..c5767c71553 100644 --- a/src/Build/BackEnd/Components/Logging/LoggingServiceFactory.cs +++ b/src/Build/BackEnd/Components/Logging/LoggingServiceFactory.cs @@ -46,7 +46,7 @@ internal LoggingServiceFactory(LoggerMode mode, int nodeId) /// An instance of a LoggingService as a IBuildComponent public IBuildComponent CreateInstance(BuildComponentType type) { - ErrorUtilities.VerifyThrow(type == BuildComponentType.LoggingService, "Cannot create components of type {0}", type); + ErrorUtilities.VerifyThrow(type == BuildComponentType.LoggingService, $"Cannot create components of type {type}"); IBuildComponent loggingService = (IBuildComponent)LoggingService.CreateLoggingService(_logMode, _nodeId); return loggingService; } diff --git a/src/Build/BackEnd/Components/Logging/LoggingServiceLogMethods.cs b/src/Build/BackEnd/Components/Logging/LoggingServiceLogMethods.cs index 31c9f55eeb0..48f04b93e61 100644 --- a/src/Build/BackEnd/Components/Logging/LoggingServiceLogMethods.cs +++ b/src/Build/BackEnd/Components/Logging/LoggingServiceLogMethods.cs @@ -135,7 +135,7 @@ public void LogErrorFromText(BuildEventContext buildEventContext, string subcate if (buildEvent.ProjectFile == null && buildEventContext.ProjectContextId != BuildEventContext.InvalidProjectContextId) { _projectFileMap.TryGetValue(buildEventContext.ProjectContextId, out string projectFile); - ErrorUtilities.VerifyThrow(projectFile != null, "ContextID {0} should have been in the ID-to-project file mapping but wasn't!", buildEventContext.ProjectContextId); + ErrorUtilities.VerifyThrow(projectFile != null, $"ContextID {buildEventContext.ProjectContextId} should have been in the ID-to-project file mapping but wasn't!"); buildEvent.ProjectFile = projectFile; } @@ -175,7 +175,7 @@ public void LogInvalidProjectFileError(BuildEventContext buildEventContext, Inva if (buildEvent.ProjectFile == null && buildEventContext.ProjectContextId != BuildEventContext.InvalidProjectContextId) { _projectFileMap.TryGetValue(buildEventContext.ProjectContextId, out string projectFile); - ErrorUtilities.VerifyThrow(projectFile != null, "ContextID {0} should have been in the ID-to-project file mapping but wasn't!", buildEventContext.ProjectContextId); + ErrorUtilities.VerifyThrow(projectFile != null, $"ContextID {buildEventContext.ProjectContextId} should have been in the ID-to-project file mapping but wasn't!"); buildEvent.ProjectFile = projectFile; } @@ -327,7 +327,7 @@ public void LogWarningFromText(BuildEventContext buildEventContext, string subca if (buildEvent.ProjectFile == null && buildEventContext.ProjectContextId != BuildEventContext.InvalidProjectContextId) { _projectFileMap.TryGetValue(buildEventContext.ProjectContextId, out string projectFile); - ErrorUtilities.VerifyThrow(projectFile != null, "ContextID {0} should have been in the ID-to-project file mapping but wasn't!", buildEventContext.ProjectContextId); + ErrorUtilities.VerifyThrow(projectFile != null, $"ContextID {buildEventContext.ProjectContextId} should have been in the ID-to-project file mapping but wasn't!"); buildEvent.ProjectFile = projectFile; } @@ -537,11 +537,9 @@ public ProjectStartedEventArgs CreateProjectStarted( { projectContextId = NextProjectId; - // PERF: Not using VerifyThrow to avoid boxing of projectBuildEventContext.ProjectContextId in the non-error case. - if (_projectFileMap.ContainsKey(projectContextId)) - { - ErrorUtilities.ThrowInternalError("ContextID {0} for project {1} should not already be in the ID-to-file mapping!", projectContextId, projectFile); - } + ErrorUtilities.VerifyThrow( + !_projectFileMap.ContainsKey(projectContextId), + $"ContextID {projectContextId} for project {projectFile} should not already be in the ID-to-file mapping!"); _projectFileMap[projectContextId] = projectFile; } @@ -552,7 +550,7 @@ public ProjectStartedEventArgs CreateProjectStarted( { if (!projectFile.Equals(existingProjectFile, StringComparison.OrdinalIgnoreCase)) { - ErrorUtilities.ThrowInternalError("ContextID {0} was already in the ID-to-project file mapping but the project file {1} did not match the provided one {2}!", projectContextId, existingProjectFile, projectFile); + ErrorUtilities.ThrowInternalError($"ContextID {projectContextId} was already in the ID-to-project file mapping but the project file {existingProjectFile} did not match the provided one {projectFile}!"); } } else @@ -562,7 +560,7 @@ public ProjectStartedEventArgs CreateProjectStarted( // So we only need this sanity check for the in-proc node. if (nodeBuildEventContext.NodeId == Scheduler.InProcNodeId) { - ErrorUtilities.ThrowInternalError("ContextID {0} should have been in the ID-to-project file mapping but wasn't!", projectContextId); + ErrorUtilities.ThrowInternalError($"ContextID {projectContextId} should have been in the ID-to-project file mapping but wasn't!"); } _projectFileMap[projectContextId] = projectFile; @@ -619,11 +617,9 @@ public void LogProjectFinished(BuildEventContext projectBuildEventContext, strin // Due to GetAndVerifyProjectFileFromContext validation, these checks break the build. if (!_buildCheckEnabled) { - // PERF: Not using VerifyThrow to avoid boxing of projectBuildEventContext.ProjectContextId in the non-error case. - if (!_projectFileMap.TryRemove(projectBuildEventContext.ProjectContextId, out _)) - { - ErrorUtilities.ThrowInternalError("ContextID {0} for project {1} should be in the ID-to-file mapping!", projectBuildEventContext.ProjectContextId, projectFile); - } + ErrorUtilities.VerifyThrow( + _projectFileMap.TryRemove(projectBuildEventContext.ProjectContextId, out _), + $"ContextID {projectBuildEventContext.ProjectContextId} for project {projectFile} should be in the ID-to-file mapping!"); } } diff --git a/src/Build/BackEnd/Components/ProjectCache/ProjectCacheService.cs b/src/Build/BackEnd/Components/ProjectCache/ProjectCacheService.cs index ba83ecb8c95..fbe371aa2b2 100644 --- a/src/Build/BackEnd/Components/ProjectCache/ProjectCacheService.cs +++ b/src/Build/BackEnd/Components/ProjectCache/ProjectCacheService.cs @@ -620,7 +620,7 @@ private async ValueTask GetCacheResultAsync(BuildRequestData buildR plugin.InitializationException?.Throw(); - ErrorUtilities.VerifyThrow(plugin.PluginInstance != null, "Plugin '{0}' instance is null", plugin.Name); + ErrorUtilities.VerifyThrow(plugin.PluginInstance != null, $"Plugin '{plugin.Name}' instance is null"); MSBuildEventSource.Log.ProjectCacheGetCacheResultStart(plugin.Name, buildRequest.ProjectFullPath, targetNames ?? MSBuildConstants.DefaultTargetsMarker); if (plugin.PluginInstance is ProjectCachePluginBase currentPlugin) @@ -636,7 +636,7 @@ private async ValueTask GetCacheResultAsync(BuildRequestData buildR #pragma warning restore CS0618 // Type or member is obsolete else { - ErrorUtilities.ThrowInternalError("Unknown plugin type", plugin.Name); + ErrorUtilities.ThrowInternalError($"Unknown plugin type: {plugin.Name}"); } if (pluginLogger.HasLoggedErrors || experimentalPluginLogger.HasLoggedErrors || cacheResult.ResultType == CacheResultType.None) @@ -898,7 +898,7 @@ public async Task HandleBuildResultAsync( // Rethrow any initialization exception. plugin.InitializationException?.Throw(); - ErrorUtilities.VerifyThrow(plugin.PluginInstance != null, "Plugin '{0}' instance is null", plugin.Name); + ErrorUtilities.VerifyThrow(plugin.PluginInstance != null, $"Plugin '{plugin.Name}' instance is null"); MSBuildEventSource.Log.ProjectCacheHandleBuildResultStart(plugin.Name, fileAccessContext.ProjectFullPath, targetNames); try diff --git a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTask.cs b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTask.cs index 94c1ee183ac..a1020c2f1aa 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTask.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTask.cs @@ -77,7 +77,7 @@ internal static IntrinsicTask InstantiateTask(ProjectTargetInstanceChild taskIns } else { - ErrorUtilities.ThrowInternalError("Unhandled intrinsic task type {0}", taskInstance.GetType().GetTypeInfo().BaseType); + ErrorUtilities.ThrowInternalError($"Unhandled intrinsic task type {taskInstance.GetType().GetTypeInfo().BaseType}"); return null; } } diff --git a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/IntrinsicTaskFactory.cs b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/IntrinsicTaskFactory.cs index 166552fd23e..863ed5cd6cf 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/IntrinsicTaskFactory.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/IntrinsicTaskFactory.cs @@ -49,7 +49,7 @@ public bool Initialize(string taskName, IDictionary pa { if (!String.Equals(taskName, TaskType.Name, StringComparison.OrdinalIgnoreCase)) { - ErrorUtilities.ThrowInternalError("Unexpected task name {0}. Expected {1}", taskName, TaskType.Name); + ErrorUtilities.ThrowInternalError($"Unexpected task name {taskName}. Expected {TaskType.Name}"); } return true; @@ -84,7 +84,7 @@ public ITask CreateTask(IBuildEngine taskFactoryLoggingHost) return new CallTarget(); } - ErrorUtilities.ThrowInternalError("Unexpected intrinsic task type {0}", TaskType); + ErrorUtilities.ThrowInternalError($"Unexpected intrinsic task type {TaskType}"); return null; } diff --git a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/MSBuild.cs b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/MSBuild.cs index db7e94103fe..2ae330d5c61 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/MSBuild.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/MSBuild.cs @@ -182,7 +182,7 @@ public string SkipNonexistentProjects return "True"; default: - ErrorUtilities.ThrowInternalError("Unexpected case {0}", _skipNonExistentProjects); + ErrorUtilities.ThrowInternalError($"Unexpected case {_skipNonExistentProjects}"); break; } @@ -394,7 +394,7 @@ public async Task ExecuteInternal() } else { - ErrorUtilities.VerifyThrow(skipNonExistProjects == SkipNonExistentProjectsBehavior.Error, "skipNonexistentProjects has unexpected value {0}", skipNonExistProjects); + ErrorUtilities.VerifyThrow(skipNonExistProjects == SkipNonExistentProjectsBehavior.Error, $"skipNonexistentProjects has unexpected value {skipNonExistProjects}"); Log.LogErrorWithCodeFromResources("MSBuild.ProjectFileNotFound", project.ItemSpec); success = false; } diff --git a/src/Build/BackEnd/Components/RequestBuilder/Lookup.cs b/src/Build/BackEnd/Components/RequestBuilder/Lookup.cs index cd7a4178df2..bac55656f31 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/Lookup.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/Lookup.cs @@ -263,7 +263,7 @@ internal Lookup.Scope EnterScope(string description) private void LeaveScope(Lookup.Scope scopeToLeave) { ErrorUtilities.VerifyThrow(_lookupScopes.Count >= 2, "Too many calls to Leave()."); - ErrorUtilities.VerifyThrow(Object.ReferenceEquals(scopeToLeave, _lookupScopes), "Attempting to leave with scope '{0}' but scope '{1}' is on top of the stack.", scopeToLeave.Description, _lookupScopes.Description); + ErrorUtilities.VerifyThrow(Object.ReferenceEquals(scopeToLeave, _lookupScopes), $"Attempting to leave with scope '{scopeToLeave.Description}' but scope '{_lookupScopes.Description}' is on top of the stack."); // Our lookup works by stopping the first time it finds an item group of the appropriate type. // So we can't apply an add directly into the table below because that could create a new group diff --git a/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs b/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs index ad869205157..5666804be0b 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs @@ -231,7 +231,7 @@ public void ContinueRequest() VerifyEntryInReadyState(); _continueResults = _requestEntry.Continue(); - ErrorUtilities.VerifyThrow(_blockType == BlockType.BlockedOnTargetInProgress || _blockType == BlockType.Yielded || (_continueResults != null), "Unexpected null results for request {0} (nr {1})", _requestEntry.Request.GlobalRequestId, _requestEntry.Request.NodeRequestId); + ErrorUtilities.VerifyThrow(_blockType == BlockType.BlockedOnTargetInProgress || _blockType == BlockType.Yielded || (_continueResults != null), $"Unexpected null results for request {_requestEntry.Request.GlobalRequestId} (nr {_requestEntry.Request.NodeRequestId})"); // Setting the continue event will wake up the build thread, which is suspended in StartNewBuildRequests. _continueEvent.Set(); @@ -630,7 +630,7 @@ internal static int WaitWithBuilderThreadStart(WaitHandle[] handles, bool recurs /// internal static IBuildComponent CreateComponent(BuildComponentType type) { - ErrorUtilities.VerifyThrow(type == BuildComponentType.RequestBuilder, "Cannot create components of type {0}", type); + ErrorUtilities.VerifyThrow(type == BuildComponentType.RequestBuilder, $"Cannot create components of type {type}"); return new RequestBuilder(); } @@ -746,7 +746,7 @@ private void SetCommonWorkerThreadParameters() /// private void VerifyEntryInReadyState() { - ErrorUtilities.VerifyThrow(_requestEntry.State == BuildRequestEntryState.Ready, "Entry is not in the Ready state, it is in the {0} state.", _requestEntry.State); + ErrorUtilities.VerifyThrow(_requestEntry.State == BuildRequestEntryState.Ready, $"Entry is not in the Ready state, it is in the {_requestEntry.State} state."); } /// @@ -754,7 +754,7 @@ private void VerifyEntryInReadyState() /// private void VerifyEntryInActiveState() { - ErrorUtilities.VerifyThrow(_requestEntry.State == BuildRequestEntryState.Active, "Entry is not in the Active state, it is in the {0} state.", _requestEntry.State); + ErrorUtilities.VerifyThrow(_requestEntry.State == BuildRequestEntryState.Active, $"Entry is not in the Active state, it is in the {_requestEntry.State} state."); } /// @@ -763,7 +763,7 @@ private void VerifyEntryInActiveState() private void VerifyEntryInActiveOrWaitingState() { ErrorUtilities.VerifyThrow(_requestEntry.State == BuildRequestEntryState.Active || _requestEntry.State == BuildRequestEntryState.Waiting, - "Entry is not in the Active or Waiting state, it is in the {0} state.", _requestEntry.State); + $"Entry is not in the Active or Waiting state, it is in the {_requestEntry.State} state."); } /// @@ -1240,9 +1240,7 @@ private async Task BuildProject() // All of the results should now be on this node. ErrorUtilities.VerifyThrow( _requestEntry.RequestConfiguration.ResultsNodeId == _componentHost.BuildParameters.NodeId, - "Results for configuration {0} were not retrieved from node {1}", - _requestEntry.RequestConfiguration.ConfigurationId, - _requestEntry.RequestConfiguration.ResultsNodeId); + $"Results for configuration {_requestEntry.RequestConfiguration.ConfigurationId} were not retrieved from node {_requestEntry.RequestConfiguration.ResultsNodeId}"); } // Build the targets diff --git a/src/Build/BackEnd/Components/RequestBuilder/TargetBuilder.cs b/src/Build/BackEnd/Components/RequestBuilder/TargetBuilder.cs index d1b8b636f25..3000605bd5b 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/TargetBuilder.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/TargetBuilder.cs @@ -283,7 +283,7 @@ async Task ITargetBuilderCallback.LegacyCallTarget(string[] tar // We push the targets one at a time to emulate the original CallTarget behavior. bool pushed = await PushTargets(targetToPush, currentTargetEntry, callTargetLookup, false, true, TargetBuiltReason.None); - ErrorUtilities.VerifyThrow(pushed, "Failed to push any targets onto the stack. Target: {0} Current Target: {1}", targets[i], currentTargetEntry.Target.Name); + ErrorUtilities.VerifyThrow(pushed, $"Failed to push any targets onto the stack. Target: {targets[i]} Current Target: {currentTargetEntry.Target.Name}"); await ProcessTargetStack(taskBuilder); if (!_cancellationToken.IsCancellationRequested) @@ -397,7 +397,7 @@ void IRequestBuilderCallback.ReleaseCores(int coresToRelease) /// internal static IBuildComponent CreateComponent(BuildComponentType type) { - ErrorUtilities.VerifyThrow(type == BuildComponentType.TargetBuilder, "Cannot create components of type {0}", type); + ErrorUtilities.VerifyThrow(type == BuildComponentType.TargetBuilder, $"Cannot create components of type {type}"); return new TargetBuilder(); } @@ -482,7 +482,7 @@ private async Task ProcessTargetStack(ITaskBuilder taskBuilder) // actually built this target while we were waiting, so that by the time we get here, it's already been finished. In this case, just blow it away. if (!CheckSkipTarget(ref stopProcessingStack, currentTargetEntry)) { - ErrorUtilities.VerifyThrow(!wasActivelyBuilding, "Target {0} was actively building and waited on but we are attempting to build it again.", currentTargetEntry.Name); + ErrorUtilities.VerifyThrow(!wasActivelyBuilding, $"Target {currentTargetEntry.Name} was actively building and waited on but we are attempting to build it again."); // This target is now actively building. _requestEntry.RequestConfiguration.ActivelyBuildingTargets[currentTargetEntry.Name] = _requestEntry.Request.GlobalRequestId; @@ -545,7 +545,7 @@ await PushTargets(errorTargets, currentTargetEntry, currentTargetEntry.Lookup, t break; default: - ErrorUtilities.ThrowInternalError("Unexpected target state {0}", currentTargetEntry.State); + ErrorUtilities.ThrowInternalError($"Unexpected target state {currentTargetEntry.State}"); break; } } diff --git a/src/Build/BackEnd/Components/RequestBuilder/TargetEntry.cs b/src/Build/BackEnd/Components/RequestBuilder/TargetEntry.cs index ea2914858f9..e75df246e82 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/TargetEntry.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/TargetEntry.cs @@ -419,7 +419,7 @@ internal async Task ExecuteTarget(ITaskBuilder taskBuilder, BuildRequestEntry re try { VerifyState(_state, TargetEntryState.Execution); - ErrorUtilities.VerifyThrow(!_isExecuting, "Target {0} is already executing", _target.Name); + ErrorUtilities.VerifyThrow(!_isExecuting, $"Target {_target.Name} is already executing"); _cancellationToken = cancellationToken; _isExecuting = true; @@ -769,7 +769,7 @@ internal void EnterLegacyCallTargetScope(Lookup lookup) /// internal void MarkForError() { - ErrorUtilities.VerifyThrow(_state != TargetEntryState.Completed, "State must not be Completed. State is {0}.", _state); + ErrorUtilities.VerifyThrow(_state != TargetEntryState.Completed, $"State must not be Completed. State is {_state}."); _state = TargetEntryState.ErrorExecution; } @@ -780,9 +780,9 @@ internal void MarkForError() /// internal void MarkForStop() { - ErrorUtilities.VerifyThrow(_state == TargetEntryState.Completed, "State must be Completed. State is {0}.", _state); - ErrorUtilities.VerifyThrow(_targetResult.ResultCode == TargetResultCode.Skipped, "ResultCode must be Skipped. ResultCode is {0}.", _state); - ErrorUtilities.VerifyThrow(_targetResult.WorkUnitResult.ActionCode == WorkUnitActionCode.Continue, "ActionCode must be Continue. ActionCode is {0}.", _state); + ErrorUtilities.VerifyThrow(_state == TargetEntryState.Completed, $"State must be Completed. State is {_state}."); + ErrorUtilities.VerifyThrow(_targetResult.ResultCode == TargetResultCode.Skipped, $"ResultCode must be Skipped. ResultCode is {_targetResult.ResultCode}."); + ErrorUtilities.VerifyThrow(_targetResult.WorkUnitResult.ActionCode == WorkUnitActionCode.Continue, $"ActionCode must be Continue. ActionCode is {_targetResult.WorkUnitResult.ActionCode}."); _targetResult.WorkUnitResult = new WorkUnitResult(_targetResult.WorkUnitResult.ResultCode, WorkUnitActionCode.Stop, _targetResult.WorkUnitResult.Exception); } @@ -886,7 +886,7 @@ private TaskExecutionMode GetTaskExecutionMode(DependencyAnalysisResult analysis /// The expected value private void VerifyState(TargetEntryState actual, TargetEntryState expected) { - ErrorUtilities.VerifyThrow(actual == expected, "Expected state {1}. Got {0}", actual, expected); + ErrorUtilities.VerifyThrow(actual == expected, $"Expected state {expected}. Got {actual}"); } /// diff --git a/src/Build/BackEnd/Components/RequestBuilder/TargetUpToDateChecker.cs b/src/Build/BackEnd/Components/RequestBuilder/TargetUpToDateChecker.cs index 16afcef0c2e..62a3404a29b 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/TargetUpToDateChecker.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/TargetUpToDateChecker.cs @@ -846,7 +846,7 @@ private void SeparateItemVectorsFromDiscreteItems( itemVectorCollection[itemVectorType] = itemVectorPartition; } - ErrorUtilities.VerifyThrow(!itemVectorPartition.ContainsKey(item), "ItemVectorPartition already contains a vector for items with the expression '{0}'", item); + ErrorUtilities.VerifyThrow(!itemVectorPartition.ContainsKey(item), $"ItemVectorPartition already contains a vector for items with the expression '{item}'"); itemVectorPartition[item] = itemVectorContents; ErrorUtilities.VerifyThrow((itemVectorTransforms == null) || (itemVectorCollection.Equals(itemVectorTransforms)) || (itemVectorPartition.Count == 1), diff --git a/src/Build/BackEnd/Components/RequestBuilder/TaskBuilder.cs b/src/Build/BackEnd/Components/RequestBuilder/TaskBuilder.cs index a6c6592bca0..eb8b5dd69c3 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/TaskBuilder.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/TaskBuilder.cs @@ -240,7 +240,7 @@ public void ShutdownComponent() /// internal static IBuildComponent CreateComponent(BuildComponentType type) { - ErrorUtilities.VerifyThrow(type == BuildComponentType.TaskBuilder, "Cannot create components of type {0}", type); + ErrorUtilities.VerifyThrow(type == BuildComponentType.TaskBuilder, $"Cannot create components of type {type}"); return new TaskBuilder(); } diff --git a/src/Build/BackEnd/Components/RequestBuilder/TaskHost.cs b/src/Build/BackEnd/Components/RequestBuilder/TaskHost.cs index 034569b6236..efdb20c22c5 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/TaskHost.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/TaskHost.cs @@ -298,7 +298,7 @@ public bool BuildProjectFilesInParallel(string[] projectFileNames, string[] targ { // Copy results from result.TargetOutputsPerProject to targetOutputsPerProject // We should always have the same number of entries - although an entry might be empty if a project failed. - ErrorUtilities.VerifyThrow(targetOutputsPerProject.Length == result.TargetOutputsPerProject.Count, "{0} != {1}", targetOutputsPerProject.Length, result.TargetOutputsPerProject.Count); + ErrorUtilities.VerifyThrow(targetOutputsPerProject.Length == result.TargetOutputsPerProject.Count, $"{targetOutputsPerProject.Length} != {result.TargetOutputsPerProject.Count}"); for (int i = 0; i < targetOutputsPerProject.Length; i++) { @@ -388,7 +388,7 @@ public void Reacquire() { IRequestBuilderCallback builderCallback = _requestEntry.Builder as IRequestBuilderCallback; ErrorUtilities.VerifyThrow(_yieldThreadId != -1, "Cannot call Reacquire() before Yield()."); - ErrorUtilities.VerifyThrow(_yieldThreadId == Environment.CurrentManagedThreadId, "Cannot call Reacquire() on thread {0} when Yield() was called on thread {1}", Environment.CurrentManagedThreadId, _yieldThreadId); + ErrorUtilities.VerifyThrow(_yieldThreadId == Environment.CurrentManagedThreadId, $"Cannot call Reacquire() on thread {Environment.CurrentManagedThreadId} when Yield() was called on thread {_yieldThreadId}"); MSBuildEventSource.Log.ExecuteTaskYieldStop(_taskLoggingContext.TaskName, _taskLoggingContext.BuildEventContext.TaskId); MSBuildEventSource.Log.ExecuteTaskReacquireStart(_taskLoggingContext.TaskName, _taskLoggingContext.BuildEventContext.TaskId); builderCallback.Reacquire(); @@ -1215,7 +1215,7 @@ private async Task BuildProjectFilesInParallelAsync(string[] skipNonexistentTargets: skipNonexistentTargets); // Even if one of the projects fails to build and therefore has no outputs, it should still have an entry in the results array (albeit with an empty list in it) - ErrorUtilities.VerifyThrow(results.Length == projectFileNames.Length, "{0}!={1}.", results.Length, projectFileNames.Length); + ErrorUtilities.VerifyThrow(results.Length == projectFileNames.Length, $"{results.Length}!={projectFileNames.Length}."); if (returnTargetOutputs) { @@ -1260,7 +1260,7 @@ private async Task BuildProjectFilesInParallelAsync(string[] } } - ErrorUtilities.VerifyThrow(results.Length == projectFileNames.Length || !overallSuccess, "The number of results returned {0} cannot be less than the number of project files {1} unless one of the results indicated failure.", results.Length, projectFileNames.Length); + ErrorUtilities.VerifyThrow(results.Length == projectFileNames.Length || !overallSuccess, $"The number of results returned {results.Length} cannot be less than the number of project files {projectFileNames.Length} unless one of the results indicated failure."); } BuildRequestsSucceeded = overallSuccess; diff --git a/src/Build/BackEnd/Components/Scheduler/SchedulableRequest.cs b/src/Build/BackEnd/Components/Scheduler/SchedulableRequest.cs index 7ac9baffafd..75b0ac9faca 100644 --- a/src/Build/BackEnd/Components/Scheduler/SchedulableRequest.cs +++ b/src/Build/BackEnd/Components/Scheduler/SchedulableRequest.cs @@ -401,11 +401,11 @@ public void ResumeExecution(int nodeId) ErrorUtilities.VerifyThrow(_assignedNodeId == Scheduler.InvalidNodeId || _assignedNodeId == nodeId, "Request must always resume on the same node on which it was started."); VerifyOneOfStates([SchedulableRequestState.Ready, SchedulableRequestState.Unscheduled]); - ErrorUtilities.VerifyThrow((_state == SchedulableRequestState.Ready) || !_schedulingData.IsRequestScheduled(this), "Another instance of request {0} is already scheduled.", _request.GlobalRequestId); - ErrorUtilities.VerifyThrow(!_schedulingData.IsNodeWorking(nodeId), "Cannot resume execution of request {0} because node {1} is already working.", _request.GlobalRequestId, nodeId); + ErrorUtilities.VerifyThrow((_state == SchedulableRequestState.Ready) || !_schedulingData.IsRequestScheduled(this), $"Another instance of request {_request.GlobalRequestId} is already scheduled."); + ErrorUtilities.VerifyThrow(!_schedulingData.IsNodeWorking(nodeId), $"Cannot resume execution of request {_request.GlobalRequestId} because node {nodeId} is already working."); int requiredNodeId = _schedulingData.GetAssignedNodeForRequestConfiguration(_request.ConfigurationId); - ErrorUtilities.VerifyThrow(requiredNodeId == Scheduler.InvalidNodeId || requiredNodeId == nodeId, "Request {0} cannot be assigned to node {1} because its configuration is already assigned to node {2}", _request.GlobalRequestId, nodeId, requiredNodeId); + ErrorUtilities.VerifyThrow(requiredNodeId == Scheduler.InvalidNodeId || requiredNodeId == nodeId, $"Request {_request.GlobalRequestId} cannot be assigned to node {nodeId} because its configuration is already assigned to node {requiredNodeId}"); _assignedNodeId = nodeId; ChangeToState(SchedulableRequestState.Executing); @@ -446,7 +446,7 @@ public void Delete() /// public void VerifyState(SchedulableRequestState requiredState) { - ErrorUtilities.VerifyThrow(_state == requiredState, "Request {0} expected to be in state {1} but state is actually {2}", _request.GlobalRequestId, requiredState, _state); + ErrorUtilities.VerifyThrow(_state == requiredState, $"Request {_request.GlobalRequestId} expected to be in state {requiredState} but state is actually {_state}"); } /// @@ -462,7 +462,7 @@ public void VerifyOneOfStates(SchedulableRequestState[] requiredStates) } } - ErrorUtilities.ThrowInternalError("State {0} is not one of the expected states.", _state); + ErrorUtilities.ThrowInternalError($"State {_state} is not one of the expected states."); } public bool IsProxyBuildRequest() => BuildRequest.IsProxyBuildRequest(); diff --git a/src/Build/BackEnd/Components/Scheduler/Scheduler.cs b/src/Build/BackEnd/Components/Scheduler/Scheduler.cs index 8306de90171..42bc3bf8705 100644 --- a/src/Build/BackEnd/Components/Scheduler/Scheduler.cs +++ b/src/Build/BackEnd/Components/Scheduler/Scheduler.cs @@ -7,11 +7,13 @@ using System.Globalization; using System.IO; using System.Linq; +using System.Runtime.CompilerServices; using System.Text; using System.Threading; using System.Threading.Tasks; using Microsoft.Build.Execution; using Microsoft.Build.Framework; +using Microsoft.Build.Framework.Utilities; using Microsoft.Build.ProjectCache; using Microsoft.Build.Shared; using Microsoft.Build.Shared.FileSystem; @@ -155,9 +157,19 @@ internal bool ForceAffinityOutOfProc => Traits.Instance.InProcNodeDisabled || _componentHost.BuildParameters.DisableInProcNode; /// - /// The path into which debug files will be written. + /// The directory into which debug files will be written. /// - private string _debugDumpPath; + private readonly string _debugDumpDirectory; + + /// + /// The file where we will store the trace file. + /// + private readonly string _debugDumpTraceFilePath; + + /// + /// The file where we will store the state file. + /// + private readonly string _debugDumpStateFilePath; /// /// If MSBUILDCUSTOMSCHEDULER = CustomSchedulerForSQL, the user may also choose to set @@ -211,7 +223,7 @@ public Scheduler() { // Be careful moving these to Traits, changing the timing of reading environment variables has a breaking potential. _debugDumpState = Traits.Instance.DebugScheduler; - _debugDumpPath = FrameworkDebugUtils.DebugPath; + _debugDumpDirectory = FrameworkDebugUtils.DebugPath; _schedulingUnlimitedVariable = Environment.GetEnvironmentVariable("MSBUILDSCHEDULINGUNLIMITED"); _nodeLimitOffset = 0; @@ -252,11 +264,14 @@ public Scheduler() _nodeCoreAllocationWeight = 0; } - if (String.IsNullOrEmpty(_debugDumpPath)) + if (string.IsNullOrEmpty(_debugDumpDirectory)) { - _debugDumpPath = FileUtilities.TempFileDirectory; + _debugDumpDirectory = FileUtilities.TempFileDirectory; } + _debugDumpTraceFilePath = Path.Combine(_debugDumpDirectory, $"SchedulerTrace_{EnvironmentUtilities.CurrentProcessId}.txt"); + _debugDumpStateFilePath = Path.Combine(_debugDumpDirectory, $"SchedulerState_{EnvironmentUtilities.CurrentProcessId}.txt"); + Reset(); } @@ -355,7 +370,7 @@ public IEnumerable ReportRequestBlocked(int nodeId, BuildReque // building a target we want to build. if (blocker.YieldAction != YieldAction.None) { - TraceScheduler("Request {0} on node {1} is performing yield action {2}.", blocker.BlockedRequestId, nodeId, blocker.YieldAction); + TraceScheduler($"Request {blocker.BlockedRequestId} on node {nodeId} is performing yield action {blocker.YieldAction}."); ErrorUtilities.VerifyThrow(string.IsNullOrEmpty(blocker.BlockingTarget), "Blocking target should be null because this is not a request blocking on a target"); HandleYieldAction(parentRequest, blocker); } @@ -376,7 +391,7 @@ public IEnumerable ReportRequestBlocked(int nodeId, BuildReque } catch (SchedulerCircularDependencyException ex) { - TraceScheduler("Circular dependency caused by request {0}({1}) (nr {2}), parent {3}({4}) (nr {5})", ex.Request.GlobalRequestId, ex.Request.ConfigurationId, ex.Request.NodeRequestId, parentRequest.BuildRequest.GlobalRequestId, parentRequest.BuildRequest.ConfigurationId, parentRequest.BuildRequest.NodeRequestId); + TraceScheduler($"Circular dependency caused by request {ex.Request.GlobalRequestId}({ex.Request.ConfigurationId}) (nr {ex.Request.NodeRequestId}), parent {parentRequest.BuildRequest.GlobalRequestId}({parentRequest.BuildRequest.ConfigurationId}) (nr {parentRequest.BuildRequest.NodeRequestId})"); responses.Add(ScheduleResponse.CreateCircularDependencyResponse(nodeId, parentRequest.BuildRequest, ex.Request)); } } @@ -389,7 +404,7 @@ public IEnumerable ReportRequestBlocked(int nodeId, BuildReque } catch (SchedulerCircularDependencyException ex) { - TraceScheduler("Circular dependency caused by request {0}({1}) (nr {2}), parent {3}({4}) (nr {5})", ex.Request.GlobalRequestId, ex.Request.ConfigurationId, ex.Request.NodeRequestId, parentRequest.BuildRequest.GlobalRequestId, parentRequest.BuildRequest.ConfigurationId, parentRequest.BuildRequest.NodeRequestId); + TraceScheduler($"Circular dependency caused by request {ex.Request.GlobalRequestId}({ex.Request.ConfigurationId}) (nr {ex.Request.NodeRequestId}), parent {parentRequest.BuildRequest.GlobalRequestId}({parentRequest.BuildRequest.ConfigurationId}) (nr {parentRequest.BuildRequest.NodeRequestId})"); responses.Add(ScheduleResponse.CreateCircularDependencyResponse(nodeId, parentRequest.BuildRequest, ex.Request)); } @@ -407,7 +422,7 @@ public IEnumerable ReportResult(int nodeId, BuildResult result { _schedulingData.EventTime = DateTime.UtcNow; List responses = new List(); - TraceScheduler("Reporting result from node {0} for request {1}, parent {2}.", nodeId, result.GlobalRequestId, result.ParentGlobalRequestId); + TraceScheduler($"Reporting result from node {nodeId} for request {result.GlobalRequestId}, parent {result.ParentGlobalRequestId}."); RecordResultToCurrentCacheIfConfigNotInOverrideCache(result); if (result.NodeRequestId == BuildRequest.ResultsTransferNodeRequestId) @@ -467,7 +482,7 @@ public IEnumerable ReportResult(int nodeId, BuildResult result { if (unscheduledRequest.BuildRequest.GlobalRequestId == result.GlobalRequestId) { - TraceScheduler("Request {0} (node request {1}) also satisfied by result.", unscheduledRequest.BuildRequest.GlobalRequestId, unscheduledRequest.BuildRequest.NodeRequestId); + TraceScheduler($"Request {unscheduledRequest.BuildRequest.GlobalRequestId} (node request {unscheduledRequest.BuildRequest.NodeRequestId}) also satisfied by result."); BuildResult newResult = new BuildResult(unscheduledRequest.BuildRequest, result, null); // Report results to the parent. @@ -533,7 +548,7 @@ public IEnumerable ReportNodesCreated(IEnumerable no foreach (NodeInfo nodeInfo in nodeInfos) { _availableNodes[nodeInfo.NodeId] = nodeInfo; - TraceScheduler("Node {0} created", nodeInfo.NodeId); + TraceScheduler($"Node {nodeInfo.NodeId} created"); switch (nodeInfo.ProviderType) { @@ -565,7 +580,7 @@ public void ReportBuildAborted(int nodeId) _schedulingData.EventTime = DateTime.UtcNow; // Get the list of build requests currently assigned to the node and report aborted results for them. - TraceScheduler("Build aborted by node {0}", nodeId); + TraceScheduler($"Build aborted by node {nodeId}"); foreach (SchedulableRequest request in _schedulingData.GetScheduledRequestsByNode(nodeId)) { @@ -692,7 +707,7 @@ public void ShutdownComponent() /// internal static IBuildComponent CreateComponent(BuildComponentType componentType) { - ErrorUtilities.VerifyThrow(componentType == BuildComponentType.Scheduler, "Cannot create components of type {0}", componentType); + ErrorUtilities.VerifyThrow(componentType == BuildComponentType.Scheduler, $"Cannot create components of type {componentType}"); return new Scheduler(); } @@ -735,7 +750,7 @@ private void ScheduleUnassignedRequests(List responses) { // Nodes still have work, but we have no requests. Let them proceed and only handle resource requests. HandlePendingResourceRequests(); - TraceScheduler("{0}: Waiting for existing work to proceed.", schedulingTime); + TraceScheduler($"{schedulingTime}: Waiting for existing work to proceed."); } return; @@ -799,25 +814,25 @@ private void ScheduleUnassignedRequests(List responses) if (RequestOrAnyItIsBlockedByCanBeServiced(request)) { DumpSchedulerState(); - ErrorUtilities.ThrowInternalError("Somehow no requests are currently executing, and at least one of the {0} requests blocked by in-progress requests is servicable by a currently existing node, but no circular dependency was detected ...", _schedulingData.BlockedRequestsCount); + ErrorUtilities.ThrowInternalError($"Somehow no requests are currently executing, and at least one of the {_schedulingData.BlockedRequestsCount} requests blocked by in-progress requests is servicable by a currently existing node, but no circular dependency was detected ..."); } } if (!createNodePending) { DumpSchedulerState(); - ErrorUtilities.ThrowInternalError("None of the {0} blocked requests can be serviced by currently existing nodes, but we aren't requesting a new one.", _schedulingData.BlockedRequestsCount); + ErrorUtilities.ThrowInternalError($"None of the {_schedulingData.BlockedRequestsCount} blocked requests can be serviced by currently existing nodes, but we aren't requesting a new one."); } } else if (_schedulingData.ReadyRequestsCount != 0) { DumpSchedulerState(); - ErrorUtilities.ThrowInternalError("Somehow we have {0} requests which are ready to go but we didn't tell the nodes to continue.", _schedulingData.ReadyRequestsCount); + ErrorUtilities.ThrowInternalError($"Somehow we have {_schedulingData.ReadyRequestsCount} requests which are ready to go but we didn't tell the nodes to continue."); } else if (_schedulingData.UnscheduledRequestsCount != 0 && !createNodePending) { DumpSchedulerState(); - ErrorUtilities.ThrowInternalError("Somehow we have {0} unassigned build requests but {1} of our nodes are free and we aren't requesting a new one...", _schedulingData.UnscheduledRequestsCount, idleNodes.Count); + ErrorUtilities.ThrowInternalError($"Somehow we have {_schedulingData.UnscheduledRequestsCount} unassigned build requests but {idleNodes.Count} of our nodes are free and we aren't requesting a new one..."); } } else @@ -825,7 +840,7 @@ private void ScheduleUnassignedRequests(List responses) ErrorUtilities.VerifyThrow(responses.Count > 0, "We failed to request a node to be created."); } - TraceScheduler("Requests scheduled: {0} Unassigned Requests: {1} Blocked Requests: {2} Unblockable Requests: {3} Free Nodes: {4}/{5} Responses: {6}", nodesFreeToDoWorkPriorToScheduling - idleNodes.Count, _schedulingData.UnscheduledRequestsCount, _schedulingData.BlockedRequestsCount, _schedulingData.ReadyRequestsCount, idleNodes.Count, _availableNodes.Count, responses.Count); + TraceScheduler($"Requests scheduled: {nodesFreeToDoWorkPriorToScheduling - idleNodes.Count} Unassigned Requests: {_schedulingData.UnscheduledRequestsCount} Blocked Requests: {_schedulingData.BlockedRequestsCount} Unblockable Requests: {_schedulingData.ReadyRequestsCount} Free Nodes: {idleNodes.Count}/{_availableNodes.Count} Responses: {responses.Count}"); DumpSchedulerState(); } @@ -1087,7 +1102,7 @@ private void AssignUnscheduledRequestsWithConfigurationCountLevelling(List responses, Has // Don't overload the system. if (AtSchedulingLimit()) { - TraceScheduler("System load limit reached, cannot schedule new work. Executing: {0} Yielding: {1} Max Count: {2}", _schedulingData.ExecutingRequestsCount, _schedulingData.YieldingRequestsCount, _componentHost.BuildParameters.MaxNodeCount); + TraceScheduler($"System load limit reached, cannot schedule new work. Executing: {_schedulingData.ExecutingRequestsCount} Yielding: {_schedulingData.YieldingRequestsCount} Max Count: {_componentHost.BuildParameters.MaxNodeCount}"); return; } @@ -1371,7 +1386,7 @@ private void AssignUnscheduledRequestsUsingCustomSchedulerForSQL(List configurationCountLimit) { - TraceScheduler("Chose not to assign request {0} to node {2} because its count of configurations ({3}) exceeds the current limit ({4}).", request.BuildRequest.GlobalRequestId, request.BuildRequest.ConfigurationId, nodeId, configurationCountsByNode[nodeId], configurationCountLimit); + TraceScheduler($"Chose not to assign request {request.BuildRequest.GlobalRequestId} to node {nodeId} because its count of configurations ({configurationCountsByNode[nodeId]}) exceeds the current limit ({configurationCountLimit})."); } } } @@ -1426,7 +1441,7 @@ private void AssignUnscheduledRequestToNode(SchedulableRequest request, int node ErrorUtilities.VerifyThrow(config.ResultsNodeId != InvalidNodeId, "Configuration's results node is not set."); responses.Add(ScheduleResponse.CreateScheduleResponse(nodeId, request.BuildRequest, mustSendConfigurationToNode)); - TraceScheduler("Executing request {0} on node {1} with parent {2}", request.BuildRequest.GlobalRequestId, nodeId, (request.Parent == null) ? -1 : request.Parent.BuildRequest.GlobalRequestId); + TraceScheduler($"Executing request {request.BuildRequest.GlobalRequestId} on node {nodeId} with parent {(request.Parent == null ? -1 : request.Parent.BuildRequest.GlobalRequestId)}"); WarnWhenProxyBuildsGetScheduledOnOutOfProcNode(); @@ -1617,7 +1632,7 @@ private bool CreateNewNodeIfPossible(List responses, IEnumerab ErrorUtilities.VerifyThrow(inProcNodesToCreate == 1, "We should not be trying to create more than one inproc node"); } - TraceScheduler("Requesting creation of new node satisfying affinity {0}", NodeAffinity.InProc); + TraceScheduler($"Requesting creation of new node satisfying affinity {NodeAffinity.InProc}"); responses.Add(ScheduleResponse.CreateNewNodeResponse(NodeAffinity.InProc, inProcNodesToCreate)); // We only want to submit one node creation request at a time -- as part of node creation we recursively re-request the scheduler @@ -1648,7 +1663,7 @@ private bool CreateNewNodeIfPossible(List responses, IEnumerab // If we still want to create them, go ahead if (outOfProcNodesToCreate > 0) { - TraceScheduler("Requesting creation of {0} new node(s) satisfying affinity {1}", outOfProcNodesToCreate, NodeAffinity.OutOfProc); + TraceScheduler($"Requesting creation of {outOfProcNodesToCreate} new node(s) satisfying affinity {NodeAffinity.OutOfProc}"); responses.Add(ScheduleResponse.CreateNewNodeResponse(NodeAffinity.OutOfProc, outOfProcNodesToCreate)); } @@ -1727,7 +1742,7 @@ private void HandleRequestBlockedOnResultsTransfer(SchedulableRequest parentRequ BuildRequestConfiguration configuration = _configCache[parentRequest.BuildRequest.ConfigurationId]; responses.Add(ScheduleResponse.CreateScheduleResponse(configuration.ResultsNodeId, newRequest, false)); - TraceScheduler("Created request {0} (node request {1}) for transfer of configuration {2}'s results from node {3} to node {4}", newRequest.GlobalRequestId, newRequest.NodeRequestId, configuration.ConfigurationId, configuration.ResultsNodeId, parentRequest.AssignedNode); + TraceScheduler($"Created request {newRequest.GlobalRequestId} (node request {newRequest.NodeRequestId}) for transfer of configuration {configuration.ConfigurationId}'s results from node {configuration.ResultsNodeId} to node {parentRequest.AssignedNode}"); // The configuration's results will now be homed at the new location (once they have come back from the // original node.) @@ -1754,7 +1769,7 @@ private void HandleRequestBlockedByNewRequests(SchedulableRequest parentRequest, } int nodeForResults = (parentRequest == null) ? InvalidNodeId : parentRequest.AssignedNode; - TraceScheduler("Received request {0} (node request {1}) with parent {2} from node {3} for project {4} with targets {5}", request.GlobalRequestId, request.NodeRequestId, request.ParentGlobalRequestId, nodeForResults, _configCache![request.ConfigurationId].ProjectFullPath, request.Targets.Count == 0 ? "default" : string.Join(";", request.Targets)); + TraceScheduler($"Received request {request.GlobalRequestId} (node request {request.NodeRequestId}) with parent {request.ParentGlobalRequestId} from node {nodeForResults} for project {_configCache![request.ConfigurationId].ProjectFullPath} with targets {(request.Targets.Count == 0 ? "default" : string.Join(";", request.Targets))}"); // First, determine if we have already built this request and have results for it. If we do, we prepare the responses for it // directly here. We COULD simply report these as blocking the parent request and let the scheduler pick them up later when the parent @@ -1762,7 +1777,7 @@ private void HandleRequestBlockedByNewRequests(SchedulableRequest parentRequest, ScheduleResponse response = TrySatisfyRequestFromCache(nodeForResults, request, skippedResultsDoNotCauseCacheMiss: _componentHost.BuildParameters.SkippedResultsDoNotCauseCacheMiss()); if (response != null) { - TraceScheduler("Request {0} (node request {1}) satisfied from the cache.", request.GlobalRequestId, request.NodeRequestId); + TraceScheduler($"Request {request.GlobalRequestId} (node request {request.NodeRequestId}) satisfied from the cache."); // BuildResult result = (response.Action == ScheduleActionType.Unblock) ? response.Unblocker.Results[0] : response.BuildResult; LogRequestHandledFromCache(request, response.BuildResult); @@ -1819,7 +1834,7 @@ private void HandleRequestBlockedByNewRequests(SchedulableRequest parentRequest, if (affinityMismatch) { - ErrorUtilities.VerifyThrowInternalError( + ErrorUtilities.VerifyThrow( _configCache.HasConfiguration(request.ConfigurationId), "A request should have a configuration if it makes it this far in the build process."); @@ -1883,7 +1898,7 @@ private void ResumeReadyRequestIfAny(int nodeId, List response // We only actually look at the first one, since that is all we need. foreach (SchedulableRequest request in _schedulingData.GetReadyRequestsByNode(nodeId)) { - TraceScheduler("Unblocking request {0} on node {1}", request.BuildRequest.GlobalRequestId, nodeId); + TraceScheduler($"Unblocking request {request.BuildRequest.GlobalRequestId} on node {nodeId}"); // ScheduleResponse response = new ScheduleResponse(nodeId, new BuildRequestUnblocker(request.BuildRequest.GlobalRequestId)); ScheduleResponse response = ScheduleResponse.CreateResumeExecutionResponse(nodeId, request.BuildRequest.GlobalRequestId); @@ -1923,7 +1938,7 @@ private void ResolveRequestFromCacheAndResumeIfPossible(SchedulableRequest reque LogRequestHandledFromCache(request.BuildRequest, response.Unblocker.Result); request.Complete(response.Unblocker.Result); - TraceScheduler("Reporting results for request {0} with parent {1} to node {2} from cache.", request.BuildRequest.GlobalRequestId, request.BuildRequest.ParentGlobalRequestId, response.NodeId); + TraceScheduler($"Reporting results for request {request.BuildRequest.GlobalRequestId} with parent {request.BuildRequest.ParentGlobalRequestId} to node {response.NodeId} from cache."); if (response.NodeId != InvalidNodeId) { responses.Add(response); @@ -1977,7 +1992,7 @@ private void ResumeRequiredWork(List responses) // Don't overload the system. if (AtSchedulingLimit()) { - TraceScheduler("System load limit reached, cannot resume any more work. Executing: {0} Yielding: {1} Max Count: {2}", _schedulingData.ExecutingRequestsCount, _schedulingData.YieldingRequestsCount, _componentHost.BuildParameters.MaxNodeCount); + TraceScheduler($"System load limit reached, cannot resume any more work. Executing: {_schedulingData.ExecutingRequestsCount} Yielding: {_schedulingData.YieldingRequestsCount} Max Count: {_componentHost.BuildParameters.MaxNodeCount}"); return; } @@ -2201,11 +2216,7 @@ private void LogRequestHandledFromCache(BuildRequest request, BuildResult result NodeLoggingContext nodeContext = new NodeLoggingContext(_componentHost.LoggingService, nodeId, true); nodeContext.LogRequestHandledFromCache(request, configuration, result); - TraceScheduler( - "Request {0} (node request {1}) with targets ({2}) satisfied from cache", - request.GlobalRequestId, - request.NodeRequestId, - string.Join(";", request.Targets)); + TraceScheduler($"Request {request.GlobalRequestId} (node request {request.NodeRequestId}) with targets ({string.Join(";", request.Targets)}) satisfied from cache"); } /// @@ -2667,17 +2678,17 @@ private void WriteRecursiveSummary(ILoggingService loggingService, BuildEventCon /// /// Method used for debugging purposes. /// - private void TraceScheduler(string format, params object[] stuff) + private void TraceScheduler(string message) { if (_debugDumpState) { try { - FileUtilities.EnsureDirectoryExists(_debugDumpPath); + FileUtilities.EnsureDirectoryExists(_debugDumpDirectory); - using StreamWriter file = FileUtilities.OpenWrite(string.Format(CultureInfo.CurrentCulture, Path.Combine(_debugDumpPath, "SchedulerTrace_{0}.txt"), EnvironmentUtilities.CurrentProcessId), append: true); + using StreamWriter file = FileUtilities.OpenWrite(_debugDumpTraceFilePath, append: true); file.Write("{0}({1})-{2}: ", Thread.CurrentThread.Name, Environment.CurrentManagedThreadId, _schedulingData.EventTime.Ticks); - file.WriteLine(format, stuff); + file.WriteLine(message); file.Flush(); } catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) @@ -2687,6 +2698,43 @@ private void TraceScheduler(string format, params object[] stuff) } } + private void TraceScheduler([InterpolatedStringHandlerArgument("")] ref TraceInterpolatedStringHandler handler) + { + if (_debugDumpState) + { + TraceScheduler(handler.GetFormattedText()); + } + } + + /// + /// Interpolated string handler used by + /// to defer string formatting unless tracing is enabled. + /// + [InterpolatedStringHandler] + private ref struct TraceInterpolatedStringHandler + { + private StringBuilderHelper _builder; + + public TraceInterpolatedStringHandler(int literalLength, int formattedCount, Scheduler scheduler, out bool isEnabled) + { + isEnabled = scheduler._debugDumpState; + _builder = isEnabled ? new(literalLength) : default; + } + + public readonly void AppendLiteral(string value) + => _builder.AppendLiteral(value); + + public readonly void AppendFormatted(TValue value) + => _builder.AppendFormatted(value); + + public readonly void AppendFormatted(TValue value, string format) + where TValue : IFormattable + => _builder.AppendFormatted(value, format); + + public string GetFormattedText() + => _builder.GetFormattedText(); + } + /// /// Dumps the current state of the scheduler. /// @@ -2698,15 +2746,16 @@ private void DumpSchedulerState() { try { - FileUtilities.EnsureDirectoryExists(_debugDumpPath); + FileUtilities.EnsureDirectoryExists(_debugDumpDirectory); bool shouldWriteHeader = _debugDumpIsFirstWrite; if (shouldWriteHeader) { - shouldWriteHeader = !FileSystems.Default.FileExists(string.Format(CultureInfo.CurrentCulture, Path.Combine(_debugDumpPath, "SchedulerState_{0}.txt"), EnvironmentUtilities.CurrentProcessId)); + shouldWriteHeader = !FileSystems.Default.FileExists(_debugDumpStateFilePath); _debugDumpIsFirstWrite = false; } - using StreamWriter file = FileUtilities.OpenWrite(string.Format(CultureInfo.CurrentCulture, Path.Combine(_debugDumpPath, "SchedulerState_{0}.txt"), EnvironmentUtilities.CurrentProcessId), append: true); + + using StreamWriter file = FileUtilities.OpenWrite(_debugDumpStateFilePath, append: true); if (shouldWriteHeader) @@ -2830,7 +2879,7 @@ private void DumpConfigurations() { try { - using StreamWriter file = FileUtilities.OpenWrite(string.Format(CultureInfo.CurrentCulture, Path.Combine(_debugDumpPath, "SchedulerState_{0}.txt"), EnvironmentUtilities.CurrentProcessId), append: true); + using StreamWriter file = FileUtilities.OpenWrite(_debugDumpStateFilePath, append: true); file.WriteLine("Configurations used during this build"); file.WriteLine("-------------------------------------"); @@ -2870,7 +2919,7 @@ private void DumpRequests() { try { - using StreamWriter file = FileUtilities.OpenWrite(string.Format(CultureInfo.CurrentCulture, Path.Combine(_debugDumpPath, "SchedulerState_{0}.txt"), EnvironmentUtilities.CurrentProcessId), append: true); + using StreamWriter file = FileUtilities.OpenWrite(_debugDumpStateFilePath, append: true); file.WriteLine("Requests used during the build:"); file.WriteLine("-------------------------------"); diff --git a/src/Build/BackEnd/Components/Scheduler/SchedulingData.cs b/src/Build/BackEnd/Components/Scheduler/SchedulingData.cs index 5a9f9bf6dd7..2f87717c143 100644 --- a/src/Build/BackEnd/Components/Scheduler/SchedulingData.cs +++ b/src/Build/BackEnd/Components/Scheduler/SchedulingData.cs @@ -278,7 +278,7 @@ public SchedulableRequest CreateRequest(BuildRequest buildRequest, SchedulableRe if (parent != null) { - ErrorUtilities.VerifyThrow(_buildHierarchy.ContainsKey(parent), "Parent doesn't exist in build hierarchy for request {0}", request.BuildRequest.GlobalRequestId); + ErrorUtilities.VerifyThrow(_buildHierarchy.ContainsKey(parent), $"Parent doesn't exist in build hierarchy for request {request.BuildRequest.GlobalRequestId}"); _buildHierarchy[parent].Add(request); } @@ -330,7 +330,7 @@ public void UpdateFromState(SchedulableRequest request, SchedulableRequestState _scheduledRequestsByNode[request.AssignedNode] = requestsAssignedToNode; } - ErrorUtilities.VerifyThrow(!requestsAssignedToNode.Contains(request), "Request {0} is already scheduled to node {1}", request.BuildRequest.GlobalRequestId, request.AssignedNode); + ErrorUtilities.VerifyThrow(!requestsAssignedToNode.Contains(request), $"Request {request.BuildRequest.GlobalRequestId} is already scheduled to node {request.AssignedNode}"); requestsAssignedToNode.Add(request); // Map the configuration to the node. @@ -354,18 +354,18 @@ public void UpdateFromState(SchedulableRequest request, SchedulableRequestState switch (request.State) { case SchedulableRequestState.Blocked: - ErrorUtilities.VerifyThrow(!_blockedRequests.ContainsKey(request.BuildRequest.GlobalRequestId), "Request with global id {0} is already blocked!"); + ErrorUtilities.VerifyThrow(!_blockedRequests.ContainsKey(request.BuildRequest.GlobalRequestId), $"Request with global id {request.BuildRequest.GlobalRequestId} is already blocked!"); _blockedRequests[request.BuildRequest.GlobalRequestId] = request; break; case SchedulableRequestState.Yielding: - ErrorUtilities.VerifyThrow(!_yieldingRequests.ContainsKey(request.BuildRequest.GlobalRequestId), "Request with global id {0} is already yielded!"); + ErrorUtilities.VerifyThrow(!_yieldingRequests.ContainsKey(request.BuildRequest.GlobalRequestId), $"Request with global id {request.BuildRequest.GlobalRequestId} is already yielded!"); _yieldingRequests[request.BuildRequest.GlobalRequestId] = request; break; case SchedulableRequestState.Completed: - ErrorUtilities.VerifyThrow(_configurationToRequests.ContainsKey(request.BuildRequest.ConfigurationId), "Configuration {0} never had requests assigned to it.", request.BuildRequest.ConfigurationId); - ErrorUtilities.VerifyThrow(_configurationToRequests[request.BuildRequest.ConfigurationId].Count > 0, "Configuration {0} has no requests assigned to it.", request.BuildRequest.ConfigurationId); + ErrorUtilities.VerifyThrow(_configurationToRequests.ContainsKey(request.BuildRequest.ConfigurationId), $"Configuration {request.BuildRequest.ConfigurationId} never had requests assigned to it."); + ErrorUtilities.VerifyThrow(_configurationToRequests[request.BuildRequest.ConfigurationId].Count > 0, $"Configuration {request.BuildRequest.ConfigurationId} has no requests assigned to it."); _configurationToRequests[request.BuildRequest.ConfigurationId].Remove(request); if (_scheduledRequestsByNode.TryGetValue(request.AssignedNode, out var requests)) { @@ -376,8 +376,8 @@ public void UpdateFromState(SchedulableRequest request, SchedulableRequestState break; case SchedulableRequestState.Executing: - ErrorUtilities.VerifyThrow(!_executingRequests.ContainsKey(request.BuildRequest.GlobalRequestId), "Request with global id {0} is already executing!"); - ErrorUtilities.VerifyThrow(!_executingRequestByNode.ContainsKey(request.AssignedNode) || _executingRequestByNode[request.AssignedNode] == null, "Node {0} is currently executing a request.", request.AssignedNode); + ErrorUtilities.VerifyThrow(!_executingRequests.ContainsKey(request.BuildRequest.GlobalRequestId), $"Request with global id {request.BuildRequest.GlobalRequestId} is already executing!"); + ErrorUtilities.VerifyThrow(!_executingRequestByNode.ContainsKey(request.AssignedNode) || _executingRequestByNode[request.AssignedNode] == null, $"Node {request.AssignedNode} is currently executing a request."); _executingRequests[request.BuildRequest.GlobalRequestId] = request; _executingRequestByNode[request.AssignedNode] = request; @@ -390,7 +390,7 @@ public void UpdateFromState(SchedulableRequest request, SchedulableRequestState break; case SchedulableRequestState.Ready: - ErrorUtilities.VerifyThrow(!_readyRequests.ContainsKey(request.BuildRequest.GlobalRequestId), "Request with global id {0} is already ready!"); + ErrorUtilities.VerifyThrow(!_readyRequests.ContainsKey(request.BuildRequest.GlobalRequestId), $"Request with global id {request.BuildRequest.GlobalRequestId} is already ready!"); _readyRequests[request.BuildRequest.GlobalRequestId] = request; HashSet readyRequestsOnNode; if (!_readyRequestsByNode.TryGetValue(request.AssignedNode, out readyRequestsOnNode)) @@ -399,12 +399,12 @@ public void UpdateFromState(SchedulableRequest request, SchedulableRequestState _readyRequestsByNode[request.AssignedNode] = readyRequestsOnNode; } - ErrorUtilities.VerifyThrow(!readyRequestsOnNode.Contains(request), "Request with global id {0} is already marked as ready on node {1}", request.BuildRequest.GlobalRequestId, request.AssignedNode); + ErrorUtilities.VerifyThrow(!readyRequestsOnNode.Contains(request), $"Request with global id {request.BuildRequest.GlobalRequestId} is already marked as ready on node {request.AssignedNode}"); readyRequestsOnNode.Add(request); break; case SchedulableRequestState.Unscheduled: - ErrorUtilities.ThrowInternalError("Request with global id {0} cannot transition to the Unscheduled state", request.BuildRequest.GlobalRequestId); + ErrorUtilities.ThrowInternalError($"Request with global id {request.BuildRequest.GlobalRequestId} cannot transition to the Unscheduled state"); break; } @@ -489,7 +489,7 @@ public SchedulableRequest GetReadyRequest(int globalRequestId) public SchedulableRequest GetScheduledRequest(int globalRequestId) { SchedulableRequest returnValue = InternalGetScheduledRequestByGlobalRequestId(globalRequestId); - ErrorUtilities.VerifyThrow(returnValue != null, "Global Request Id {0} has not been assigned and cannot be retrieved.", globalRequestId); + ErrorUtilities.VerifyThrow(returnValue != null, $"Global Request Id {globalRequestId} has not been assigned and cannot be retrieved."); return returnValue; } @@ -682,8 +682,7 @@ internal void UnassignNodeForRequestConfiguration(int configurationId) { ErrorUtilities.VerifyThrow( GetRequestsAssignedToConfigurationCount(configurationId) == 0, - "Configuration with ID {0} cannot be unassigned from a node, because there are requests scheduled with that configuration.", - configurationId); + $"Configuration with ID {configurationId} cannot be unassigned from a node, because there are requests scheduled with that configuration."); _configurationToNode.Remove(configurationId); } @@ -725,7 +724,7 @@ private void ExpectScheduledRequestState(int globalRequestId, SchedulableRequest SchedulableRequest request = InternalGetScheduledRequestByGlobalRequestId(globalRequestId); if (request == null) { - ErrorUtilities.ThrowInternalError("Request {0} was expected to be in state {1} but is not scheduled at all (it may be unscheduled or may be unknown to the system.)", globalRequestId, state); + ErrorUtilities.ThrowInternalError($"Request {globalRequestId} was expected to be in state {state} but is not scheduled at all (it may be unscheduled or may be unknown to the system.)"); } else { diff --git a/src/Build/BackEnd/Components/SdkResolution/SdkResolverManifest.cs b/src/Build/BackEnd/Components/SdkResolution/SdkResolverManifest.cs index fbd98e38777..e006a0a71e1 100644 --- a/src/Build/BackEnd/Components/SdkResolution/SdkResolverManifest.cs +++ b/src/Build/BackEnd/Components/SdkResolution/SdkResolverManifest.cs @@ -130,7 +130,7 @@ private static SdkResolverManifest ParseSdkResolverElement(XmlReader reader, str } catch (ArgumentException ex) { - ErrorUtilities.ThrowInternalError("A regular expression parsing error occurred while parsing {0}.", ex, filePath); + ErrorUtilities.ThrowInternalError($"A regular expression parsing error occurred while parsing {filePath}.", ex); } break; default: diff --git a/src/Build/BackEnd/Components/SdkResolution/SdkResolverService.cs b/src/Build/BackEnd/Components/SdkResolution/SdkResolverService.cs index 9c0efaef777..bca2922353b 100644 --- a/src/Build/BackEnd/Components/SdkResolution/SdkResolverService.cs +++ b/src/Build/BackEnd/Components/SdkResolution/SdkResolverService.cs @@ -191,7 +191,7 @@ private SdkResult ResolveSdkUsingResolversWithPatternsFirst(int submissionId, Sd } catch (RegexMatchTimeoutException ex) { - ErrorUtilities.ThrowInternalError("Timeout exceeded matching sdk \"{0}\" to from sdk resolver manifest {1}.", ex, sdk.Name, manifest.DisplayName); + ErrorUtilities.ThrowInternalError($"""Timeout exceeded matching sdk "{sdk.Name}" to from sdk resolver manifest {manifest.DisplayName}.""", ex); } } diff --git a/src/Build/BackEnd/Shared/BuildRequestConfiguration.cs b/src/Build/BackEnd/Shared/BuildRequestConfiguration.cs index 8d1dc211da6..3630be4cab1 100644 --- a/src/Build/BackEnd/Shared/BuildRequestConfiguration.cs +++ b/src/Build/BackEnd/Shared/BuildRequestConfiguration.cs @@ -369,7 +369,7 @@ public int ConfigurationId [DebuggerStepThrough] set { - ErrorUtilities.VerifyThrow((_configId == InvalidConfigurationId) || (WasGeneratedByNode && (value > InvalidConfigurationId)), "Configuration ID must be invalid, or it must be less than invalid and the new config must be greater than invalid. It was {0}, the new value was {1}.", _configId, value); + ErrorUtilities.VerifyThrow((_configId == InvalidConfigurationId) || (WasGeneratedByNode && (value > InvalidConfigurationId)), $"Configuration ID must be invalid, or it must be less than invalid and the new config must be greater than invalid. It was {_configId}, the new value was {value}."); _configId = value; } } @@ -462,7 +462,7 @@ internal void LoadProjectIntoConfiguration( int submissionId, int nodeId) { - ErrorUtilities.VerifyThrow(!IsLoaded, "Already loaded the project for this configuration id {0}.", ConfigurationId); + ErrorUtilities.VerifyThrow(!IsLoaded, $"Already loaded the project for this configuration id {ConfigurationId}."); InitializeProject(componentHost.BuildParameters, () => { diff --git a/src/Build/BackEnd/Shared/BuildResult.cs b/src/Build/BackEnd/Shared/BuildResult.cs index e657cfc2e2c..250a163d2e4 100644 --- a/src/Build/BackEnd/Shared/BuildResult.cs +++ b/src/Build/BackEnd/Shared/BuildResult.cs @@ -552,7 +552,7 @@ public void AddResultsForTarget(string target, TargetResult result) if (_resultsByTarget.TryGetValue(target, out TargetResult? targetResult)) { - ErrorUtilities.VerifyThrow(targetResult.ResultCode == TargetResultCode.Skipped, "Items already exist for target {0}.", target); + ErrorUtilities.VerifyThrow(targetResult.ResultCode == TargetResultCode.Skipped, $"Items already exist for target {target}."); } _resultsByTarget[target] = result; diff --git a/src/Build/BuildCheck/Infrastructure/BuildCheckManagerProvider.cs b/src/Build/BuildCheck/Infrastructure/BuildCheckManagerProvider.cs index 89998bad255..18848dc7236 100644 --- a/src/Build/BuildCheck/Infrastructure/BuildCheckManagerProvider.cs +++ b/src/Build/BuildCheck/Infrastructure/BuildCheckManagerProvider.cs @@ -33,7 +33,7 @@ internal sealed class BuildCheckManagerProvider : IBuildCheckManagerProvider internal static IBuildComponent CreateComponent(BuildComponentType type) { - ErrorUtilities.VerifyThrow(type == BuildComponentType.BuildCheckManagerProvider, "Cannot create components of type {0}", type); + ErrorUtilities.VerifyThrow(type == BuildComponentType.BuildCheckManagerProvider, $"Cannot create components of type {type}"); return new BuildCheckManagerProvider(); } diff --git a/src/Build/Construction/ProjectElement.cs b/src/Build/Construction/ProjectElement.cs index 6a4f1eeefda..e568d5c6dd5 100644 --- a/src/Build/Construction/ProjectElement.cs +++ b/src/Build/Construction/ProjectElement.cs @@ -506,7 +506,7 @@ protected internal virtual ProjectElement Clone(ProjectRootElement factory) var clone = CreateNewInstance(factory); if (!clone.GetType().IsEquivalentTo(GetType())) { - ErrorUtilities.ThrowInternalError("{0}.Clone() returned an instance of type {1}.", GetType().Name, clone.GetType().Name); + ErrorUtilities.ThrowInternalError($"{GetType().Name}.Clone() returned an instance of type {clone.GetType().Name}."); } clone.CopyFrom(this); diff --git a/src/Build/Evaluation/ProjectRootElementCache.cs b/src/Build/Evaluation/ProjectRootElementCache.cs index 92c36003026..d0cb124e2e6 100644 --- a/src/Build/Evaluation/ProjectRootElementCache.cs +++ b/src/Build/Evaluation/ProjectRootElementCache.cs @@ -348,9 +348,7 @@ private ProjectRootElement GetOrLoad(string projectFile, OpenProjectRootElement ErrorUtilities.VerifyThrowInternalNull(projectRootElement, "projectRootElement"); ErrorUtilities.VerifyThrow( projectRootElement.FullPath.Equals(projectFile, StringComparison.OrdinalIgnoreCase), - "Got project back with incorrect path. Expected path: {0}, received path: {1}.", - projectFile, - projectRootElement.FullPath); + $"Got project back with incorrect path. Expected path: {projectFile}, received path: {projectRootElement.FullPath}."); // An implicit load will never reset the explicit flag. if (isExplicitlyLoaded) diff --git a/src/Build/Evaluation/SimpleProjectRootElementCache.cs b/src/Build/Evaluation/SimpleProjectRootElementCache.cs index b4e6b80b7b7..2581226b5f0 100644 --- a/src/Build/Evaluation/SimpleProjectRootElementCache.cs +++ b/src/Build/Evaluation/SimpleProjectRootElementCache.cs @@ -65,9 +65,7 @@ private ProjectRootElement GetFromOrAddToCache(string projectFile, OpenProjectRo ErrorUtilities.VerifyThrowInternalNull(rootElement, "projectRootElement"); ErrorUtilities.VerifyThrow( rootElement.FullPath.Equals(key, StringComparison.OrdinalIgnoreCase), - "Got project back with incorrect path. Expected path: {0}, received path: {1}.", - key, - rootElement.FullPath); + $"Got project back with incorrect path. Expected path: {key}, received path: {rootElement.FullPath}."); AddEntry(rootElement); diff --git a/src/Build/Instance/ProjectInstance.cs b/src/Build/Instance/ProjectInstance.cs index 4eb95dc09aa..d6d5083af90 100644 --- a/src/Build/Instance/ProjectInstance.cs +++ b/src/Build/Instance/ProjectInstance.cs @@ -2651,7 +2651,7 @@ internal static ProjectInstance[] LoadSolutionForBuild( ErrorUtilities.VerifyThrowArgumentNull(globalPropertiesInstances); ErrorUtilities.VerifyThrowArgumentLengthIfNotNull(toolsVersion, nameof(toolsVersion)); ErrorUtilities.VerifyThrowArgumentNull(buildParameters); - ErrorUtilities.VerifyThrow(FileUtilities.IsSolutionFilename(projectFile), "Project file {0} is not a solution.", projectFile); + ErrorUtilities.VerifyThrow(FileUtilities.IsSolutionFilename(projectFile), $"Project file {projectFile} is not a solution."); ProjectInstance[] projectInstances = null; @@ -2929,7 +2929,7 @@ internal ProjectTargetInstance AddTarget( VerifyThrowNotImmutable(); ErrorUtilities.VerifyThrowInternalLength(targetName, nameof(targetName)); - ErrorUtilities.VerifyThrow(!_actualTargets.ContainsKey(targetName), "Target {0} already exists.", targetName); + ErrorUtilities.VerifyThrow(!_actualTargets.ContainsKey(targetName), $"Target {targetName} already exists."); ProjectTargetInstance target = new ProjectTargetInstance( targetName, diff --git a/src/Build/Instance/TaskFactories/TaskHostTask.cs b/src/Build/Instance/TaskFactories/TaskHostTask.cs index 715d1416efe..0cc1e8ba1c3 100644 --- a/src/Build/Instance/TaskFactories/TaskHostTask.cs +++ b/src/Build/Instance/TaskFactories/TaskHostTask.cs @@ -738,8 +738,7 @@ private void HandleBuildRequest(TaskHostBuildRequest request) { if (_buildEngine is not IBuildEngine3 engine3) { - ErrorUtilities.ThrowInternalError("HandleBuildRequest requires IBuildEngine3 but _buildEngine is {0}", - _buildEngine?.GetType().Name ?? "null"); + ErrorUtilities.ThrowInternalError($"HandleBuildRequest requires IBuildEngine3 but _buildEngine is {_buildEngine?.GetType().Name ?? "null"}"); return; // unreachable, but satisfies flow analysis } diff --git a/src/Build/Instance/TaskFactoryWrapper.cs b/src/Build/Instance/TaskFactoryWrapper.cs index f2d24afd836..bc488e8e327 100644 --- a/src/Build/Instance/TaskFactoryWrapper.cs +++ b/src/Build/Instance/TaskFactoryWrapper.cs @@ -238,7 +238,7 @@ internal void SetPropertyValue(ITask task, TaskPropertyInfo property, object val } else { - ErrorUtilities.ThrowInternalError("Task does not implement IGeneratedTask and we don't have {0} either.", typeof(ReflectableTaskPropertyInfo).Name); + ErrorUtilities.ThrowInternalError($"Task does not implement IGeneratedTask and we don't have {typeof(ReflectableTaskPropertyInfo)} either."); throw new InternalErrorException(); // unreachable } } diff --git a/src/Build/Instance/TaskRegistry.cs b/src/Build/Instance/TaskRegistry.cs index 24d15831948..be999bd5f67 100644 --- a/src/Build/Instance/TaskRegistry.cs +++ b/src/Build/Instance/TaskRegistry.cs @@ -285,7 +285,7 @@ private static void RegisterTasksFromUsingTaskElement { ErrorUtilities.VerifyThrowInternalNull(directoryOfImportingFile); #if DEBUG - ErrorUtilities.VerifyThrowInternalError(!taskRegistry._isInitialized, "Attempt to modify TaskRegistry after it was initialized."); + ErrorUtilities.VerifyThrow(!taskRegistry._isInitialized, "Attempt to modify TaskRegistry after it was initialized."); #endif if (!ConditionEvaluator.EvaluateCondition( @@ -455,7 +455,7 @@ internal TaskFactoryWrapper GetRegisteredTask( bool isMultiThreadedBuild) { #if DEBUG - ErrorUtilities.VerifyThrowInternalError(_isInitialized, "Attempt to read from TaskRegistry before its initialization was finished."); + ErrorUtilities.VerifyThrow(_isInitialized, "Attempt to read from TaskRegistry before its initialization was finished."); #endif TaskFactoryWrapper taskFactory = null; diff --git a/src/Build/Logging/ParallelLogger/ParallelConsoleLogger.cs b/src/Build/Logging/ParallelLogger/ParallelConsoleLogger.cs index bbc114d14d2..fbeaefe7e15 100644 --- a/src/Build/Logging/ParallelLogger/ParallelConsoleLogger.cs +++ b/src/Build/Logging/ParallelLogger/ParallelConsoleLogger.cs @@ -655,7 +655,7 @@ public override void ProjectFinishedHandler(object sender, ProjectFinishedEventA // Get the project started event so we can use its information to properly display a project finished event ProjectStartedEventMinimumFields startedEvent = _buildEventManager.GetProjectStartedEvent(e.BuildEventContext); - ErrorUtilities.VerifyThrow(startedEvent != null, "Project finished event for {0} received without matching start event", e.ProjectFile); + ErrorUtilities.VerifyThrow(startedEvent != null, $"Project finished event for {e.ProjectFile} received without matching start event"); if (this.showPerfSummary) { diff --git a/src/Build/Resources/AssemblyResources.cs b/src/Build/Resources/AssemblyResources.cs index 0eca428e261..2f3039dac39 100644 --- a/src/Build/Resources/AssemblyResources.cs +++ b/src/Build/Resources/AssemblyResources.cs @@ -82,7 +82,7 @@ private static string GetStringFromEngineResources(string name) resource = s_sharedResources.GetString(name, CultureInfo.CurrentUICulture); } - ErrorUtilities.VerifyThrow(resource != null, "Missing resource '{0}'", name); + ErrorUtilities.VerifyThrow(resource != null, $"Missing resource '{name}'"); return resource; } diff --git a/src/Build/TelemetryInfra/TelemetryCollectorProvider.cs b/src/Build/TelemetryInfra/TelemetryCollectorProvider.cs index 0c5aa8b958c..db78c722ba3 100644 --- a/src/Build/TelemetryInfra/TelemetryCollectorProvider.cs +++ b/src/Build/TelemetryInfra/TelemetryCollectorProvider.cs @@ -28,7 +28,7 @@ internal ITelemetryCollector CreateCollector() internal static IBuildComponent CreateComponent(BuildComponentType type) { - ErrorUtilities.VerifyThrow(type == BuildComponentType.TelemetryCollector, "Cannot create components of type {0}", type); + ErrorUtilities.VerifyThrow(type == BuildComponentType.TelemetryCollector, $"Cannot create components of type {type}"); return new TelemetryCollectorProvider(); } diff --git a/src/Build/Utilities/EngineFileUtilities.cs b/src/Build/Utilities/EngineFileUtilities.cs index 9620015b038..ea5b5141082 100644 --- a/src/Build/Utilities/EngineFileUtilities.cs +++ b/src/Build/Utilities/EngineFileUtilities.cs @@ -207,7 +207,7 @@ private static string[] GetFileList( { if (Traits.Instance.LogExpandedWildcards) { - ErrorUtilities.DebugTraceMessage("Expanding wildcard for file spec {0}", filespecEscaped); + ErrorUtilities.DebugTraceMessage("EngineFileUtilities", $"Expanding wildcard for file spec {filespecEscaped}"); } // Unescape before handing it to the filesystem. diff --git a/src/Framework/ErrorUtilities.cs b/src/Framework/ErrorUtilities.cs index 04bbf855a34..3c03486166d 100644 --- a/src/Framework/ErrorUtilities.cs +++ b/src/Framework/ErrorUtilities.cs @@ -2,99 +2,230 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Diagnostics; using System.Diagnostics.CodeAnalysis; +using System.IO; using System.Runtime.CompilerServices; +using Microsoft.Build.Framework.Utilities; -namespace Microsoft.Build.Framework +namespace Microsoft.Build.Framework; + +// TODO: this should be unified with Shared\ErrorUtilities.cs, but it is hard to untangle everything +// because some of the errors there will use localized resources from different assemblies, +// which won't be referenceable in Framework. + +internal static class FrameworkErrorUtilities { - // TODO: this should be unified with Shared\ErrorUtilities.cs, but it is hard to untangle everything - // because some of the errors there will use localized resources from different assemblies, - // which won't be referenceable in Framework. + private static readonly bool s_enableMSBuildDebugTracing = !string.IsNullOrEmpty(Environment.GetEnvironmentVariable("MSBUILDENABLEDEBUGTRACING")); - internal static class FrameworkErrorUtilities + public static void DebugTraceMessage(string category, string message) { - /// - /// This method should be used in places where one would normally put - /// an "assert". It should be used to validate that our assumptions are - /// true, where false would indicate that there must be a bug in our - /// code somewhere. This should not be used to throw errors based on bad - /// user input or anything that the user did wrong. - /// - /// - /// - internal static void VerifyThrow([DoesNotReturnIf(false)] bool condition, string unformattedMessage) + if (s_enableMSBuildDebugTracing) { - if (!condition) - { - ThrowInternalError(unformattedMessage, innerException: null, args: null); - } + Trace.WriteLine(message, category); } + } - /// - /// Helper to throw an InternalErrorException when the specified parameter is null. - /// This should be used ONLY if this would indicate a bug in MSBuild rather than - /// anything caused by user action. - /// - /// The value of the argument. - /// Parameter that should not be null. - internal static void VerifyThrowInternalNull([NotNull] object? parameter, [CallerArgumentExpression(nameof(parameter))] string? parameterName = null) + public static void DebugTraceMessage(string category, ref DebugTraceInterpolatedStringHandler handler) + { + if (s_enableMSBuildDebugTracing) { - if (parameter is null) - { - ThrowInternalError("{0} unexpectedly null", innerException: null, args: parameterName); - } + Trace.WriteLine(handler.GetFormattedText(), category); } + } + + /// + /// Interpolated string handler used by + /// to defer string formatting unless tracing is enabled. + /// + [InterpolatedStringHandler] + public ref struct DebugTraceInterpolatedStringHandler + { + private StringBuilderHelper _builder; - /// - /// Helper to throw an InternalErrorException when the specified parameter is null or zero length. - /// This should be used ONLY if this would indicate a bug in MSBuild rather than - /// anything caused by user action. - /// - /// The value of the argument. - /// Parameter that should not be null or zero length - internal static void VerifyThrowInternalLength([NotNull] string? parameterValue, [CallerArgumentExpression(nameof(parameterValue))] string? parameterName = null) + public DebugTraceInterpolatedStringHandler(int literalLength, int formattedCount, out bool isEnabled) { - VerifyThrowInternalNull(parameterValue, parameterName); + isEnabled = s_enableMSBuildDebugTracing; + _builder = isEnabled ? new(literalLength) : default; + } + + public readonly void AppendLiteral(string value) + => _builder.AppendLiteral(value); + + public readonly void AppendFormatted(TValue value) + => _builder.AppendFormatted(value); + + public readonly void AppendFormatted(TValue value, string format) + where TValue : IFormattable + => _builder.AppendFormatted(value, format); - if (parameterValue.Length == 0) - { - ThrowInternalError("{0} unexpectedly empty", innerException: null, args: parameterName); - } + public string GetFormattedText() + => _builder.GetFormattedText(); + } + + /// + /// This method should be used in places where one would normally put + /// an "assert". It should be used to validate that our assumptions are + /// true, where false would indicate that there must be a bug in our + /// code somewhere. This should not be used to throw errors based on bad + /// user input or anything that the user did wrong. + /// + public static void VerifyThrow([DoesNotReturnIf(false)] bool condition, string message) + { + if (!condition) + { + ThrowInternalError(message); } + } - /// - /// Throws InternalErrorException. - /// Indicates the code path followed should not have been possible. - /// This is only for situations that would mean that there is a bug in MSBuild itself. - /// - [DoesNotReturn] - internal static void ThrowInternalErrorUnreachable() + public static void VerifyThrow( + [DoesNotReturnIf(false)] bool condition, + [InterpolatedStringHandlerArgument(nameof(condition))] ref IsTrueInterpolatedStringHandler handler) + { + if (!condition) { - throw new InternalErrorException("Unreachable?"); + ThrowInternalError(handler.GetFormattedText()); } + } - /// - /// Throws InternalErrorException. - /// This is only for situations that would mean that there is a bug in MSBuild itself. - /// - [DoesNotReturn] - internal static void ThrowInternalError(string message) + [InterpolatedStringHandler] + public ref struct IsTrueInterpolatedStringHandler + { + private StringBuilderHelper _builder; + + public IsTrueInterpolatedStringHandler(int literalLength, int formattedCount, bool condition, out bool isEnabled) { - throw new InternalErrorException(message); + isEnabled = !condition; + _builder = isEnabled ? new(literalLength) : default; } - /// - /// Throws InternalErrorException. - /// This is only for situations that would mean that there is a bug in MSBuild itself. - /// - [DoesNotReturn] - internal static void ThrowInternalError(string message, Exception? innerException, params object?[]? args) + public readonly void AppendLiteral(string value) + => _builder.AppendLiteral(value); + + public readonly void AppendFormatted(TValue value) + => _builder.AppendFormatted(value); + + public readonly void AppendFormatted(TValue value, string format) + where TValue : IFormattable + => _builder.AppendFormatted(value, format); + + public string GetFormattedText() + => _builder.GetFormattedText(); + } + + /// + /// Helper to throw an InternalErrorException when the specified parameter is null. + /// This should be used ONLY if this would indicate a bug in MSBuild rather than + /// anything caused by user action. + /// + /// The value of the argument. + /// Parameter that should not be null. + public static void VerifyThrowInternalNull( + [NotNull] object? parameter, + [CallerArgumentExpression(nameof(parameter))] string? parameterName = null) + { + if (parameter is null) + { + ThrowInternalError($"{parameterName} unexpectedly null"); + } + } + + /// + /// Helper to throw an InternalErrorException when the specified parameter is null or zero length. + /// This should be used ONLY if this would indicate a bug in MSBuild rather than + /// anything caused by user action. + /// + /// The value of the argument. + /// Parameter that should not be null or zero length + public static void VerifyThrowInternalLength( + [NotNull] string? parameterValue, + [CallerArgumentExpression(nameof(parameterValue))] string? parameterName = null) + { + VerifyThrowInternalNull(parameterValue, parameterName); + + if (parameterValue.Length == 0) + { + ThrowInternalError($"{parameterName} unexpectedly empty"); + } + } + + public static void VerifyThrowInternalLength( + [NotNull] T[]? parameterValue, + [CallerArgumentExpression(nameof(parameterValue))] string? parameterName = null) + { + VerifyThrowInternalNull(parameterValue, parameterName); + + if (parameterValue.Length == 0) + { + ThrowInternalError($"{parameterName} unexpectedly empty"); + } + } + + /// + /// Helper to throw an InternalErrorException when the specified parameter is not a rooted path. + /// This should be used ONLY if this would indicate a bug in MSBuild rather than + /// anything caused by user action. + /// + /// Parameter that should be a rooted path. + public static void VerifyThrowInternalRooted(string value) + { + if (!Path.IsPathRooted(value)) + { + ThrowInternalError($"{value} unexpectedly not a rooted path"); + } + } + + /// + /// Throws InternalErrorException. + /// This is only for situations that would mean that there is a bug in MSBuild itself. + /// + [DoesNotReturn] + public static void ThrowInternalError(string message) + => throw new InternalErrorException(message); + + /// + /// Throws InternalErrorException. + /// This is only for situations that would mean that there is a bug in MSBuild itself. + /// + [DoesNotReturn] + public static void ThrowInternalError(ref UnconditionalInterpolatedStringHandler handler) + => ThrowInternalError(handler.GetFormattedText()); + + /// + /// Throws InternalErrorException. + /// This is only for situations that would mean that there is a bug in MSBuild itself. + /// + [DoesNotReturn] + public static void ThrowInternalError(string message, Exception innerException) + => throw new InternalErrorException(message, innerException); + + /// + /// Throws InternalErrorException. + /// This is only for situations that would mean that there is a bug in MSBuild itself. + /// + [DoesNotReturn] + public static void ThrowInternalError(ref UnconditionalInterpolatedStringHandler handler, Exception innerException) + => ThrowInternalError(handler.GetFormattedText(), innerException); + + /// + /// Throws InternalErrorException. + /// Indicates the code path followed should not have been possible. + /// This is only for situations that would mean that there is a bug in MSBuild itself. + /// + [DoesNotReturn] + public static void ThrowInternalErrorUnreachable() + => ThrowInternalError("Unreachable?"); + + /// + /// Throws InternalErrorException. + /// Indicates the code path followed should not have been possible. + /// This is only for situations that would mean that there is a bug in MSBuild itself. + /// + public static void VerifyThrowInternalErrorUnreachable([DoesNotReturnIf(false)] bool condition) + { + if (!condition) { - throw new InternalErrorException( - args is null ? - message : - string.Format(message, args), - innerException); + ThrowInternalErrorUnreachable(); } } } diff --git a/src/Framework/Utilities/StringBuilderHelper.cs b/src/Framework/Utilities/StringBuilderHelper.cs index 0ecc097cfae..3e89b4c0727 100644 --- a/src/Framework/Utilities/StringBuilderHelper.cs +++ b/src/Framework/Utilities/StringBuilderHelper.cs @@ -2,8 +2,13 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +#if NET +using System.Runtime.CompilerServices; +#endif using System.Text; +#pragma warning disable IDE0038 // Use pattern matching + namespace Microsoft.Build.Framework.Utilities; /// @@ -22,11 +27,69 @@ public readonly void AppendLiteral(string value) => _builder?.Append(value); public readonly void AppendFormatted(T value) - => _builder?.Append(value?.ToString()); + { + if (value is null || _builder is null) + { + return; + } + + // PERF: Intentionally not using pattern matching here to avoid boxing. + // - On .NET, we prefer ISpanFormattable and perform that check below. + // - On .NET Framework, we make a constrained call to IFormattable to avoid boxing, + // which is not possible with pattern matching. + if (value is IFormattable) + { +#if NET + if (value is ISpanFormattable) + { + // This calls the Append overload that takes a StringBuilder.AppendInterpolatedStringHandler, + // which will avoid allocations for value types. + _builder.Append($"{value}"); + return; + } +#endif + + // Make a constrained call through IFormattable to avoid boxing value types. + _builder.Append(((IFormattable)value).ToString(format: null, formatProvider: null)); + } + else + { + _builder.Append(value.ToString()); + } + } + + public readonly void AppendFormatted(T value, string? format) + where T : IFormattable + { + if (value is null || _builder is null) + { + return; + } + +#if NET + if (value is ISpanFormattable) + { + // Use System.Runtime.CompilerServices.DefaultInterpolatedStringHandler + // to do the work of appending 'value' and formatting with 'format'. + // We initialize with a stack-allocated buffer of 64 chars, which would + // only be a significant concern if this code path were running on .NET Framework. + var handler = new DefaultInterpolatedStringHandler( + literalLength: 0, + formattedCount: 1, + provider: null, + initialBuffer: stackalloc char[64]); + + handler.AppendFormatted(value, alignment: 0, format); - public readonly void AppendFormatted(TValue value, string format) - where TValue : IFormattable - => _builder?.Append(value?.ToString(format, formatProvider: null)); + _builder.Append(handler.Text); + + handler.Clear(); + return; + } +#endif + + _builder.Append(value.ToString(format, formatProvider: null)); + } /// /// Returns the formatted string and releases the underlying @@ -38,7 +101,7 @@ public string GetFormattedText() _builder = null; return builder is not null - ? StringBuilderCache.GetStringAndRelease(builder) - : string.Empty; + ? StringBuilderCache.GetStringAndRelease(builder) + : string.Empty; } } diff --git a/src/Framework/Utilities/UnconditionalInterpolatedStringHandler.cs b/src/Framework/Utilities/UnconditionalInterpolatedStringHandler.cs new file mode 100644 index 00000000000..3162f094701 --- /dev/null +++ b/src/Framework/Utilities/UnconditionalInterpolatedStringHandler.cs @@ -0,0 +1,34 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.CompilerServices; + +namespace Microsoft.Build.Framework.Utilities; + +/// +/// Interpolated string handler that handles string formatting unconditionally. +/// +[InterpolatedStringHandler] +internal ref struct UnconditionalInterpolatedStringHandler +{ + private StringBuilderHelper _builder; + + public UnconditionalInterpolatedStringHandler(int literalLength, int formattedCount) + { + _builder = new(literalLength); + } + + public readonly void AppendLiteral(string value) + => _builder.AppendLiteral(value); + + public readonly void AppendFormatted(TValue value) + => _builder.AppendFormatted(value); + + public readonly void AppendFormatted(TValue value, string format) + where TValue : IFormattable + => _builder.AppendFormatted(value, format); + + public string GetFormattedText() + => _builder.GetFormattedText(); +} diff --git a/src/MSBuild/AssemblyResources.cs b/src/MSBuild/AssemblyResources.cs index 041c55cc2b2..65d8eeaff8e 100644 --- a/src/MSBuild/AssemblyResources.cs +++ b/src/MSBuild/AssemblyResources.cs @@ -30,7 +30,7 @@ internal static string GetString(string name) resource = s_sharedResources.GetString(name, CultureInfo.CurrentUICulture); } - ErrorUtilities.VerifyThrow(resource != null, "Missing resource '{0}'", name); + ErrorUtilities.VerifyThrow(resource != null, $"Missing resource '{name}'"); return resource; } diff --git a/src/MSBuild/OutOfProcTaskHostNode.cs b/src/MSBuild/OutOfProcTaskHostNode.cs index b95120ad29d..6b2ffb5f282 100644 --- a/src/MSBuild/OutOfProcTaskHostNode.cs +++ b/src/MSBuild/OutOfProcTaskHostNode.cs @@ -514,9 +514,7 @@ public bool BuildProjectFilesInParallel(string[] projectFileNames, string[] targ ErrorUtilities.VerifyThrow( targetOutputsPerProject is null || projectFileNames.Length == targetOutputsPerProject.Length, - "projectFileNames has {0} entries but targetOutputsPerProject has {1} -- lengths must match.", - projectFileNames.Length, - targetOutputsPerProject?.Length ?? 0); + $"projectFileNames has {projectFileNames.Length} entries but targetOutputsPerProject has {targetOutputsPerProject?.Length ?? 0} -- lengths must match."); bool includeTargetOutputs = targetOutputsPerProject is not null; @@ -943,7 +941,7 @@ private void HandleCallbackResponse(INodePacket packet) { if (packet is not ITaskHostCallbackPacket callbackPacket) { - ErrorUtilities.ThrowInternalError("HandleCallbackResponse called with non-callback packet type: {0}", packet.GetType().Name); + ErrorUtilities.ThrowInternalError($"HandleCallbackResponse called with non-callback packet type: {packet.GetType().Name}"); return; } @@ -965,8 +963,7 @@ private void HandleCallbackResponse(INodePacket packet) // No pending request matched -- this is a protocol bug (duplicate response, // corrupted request ID, or race with shutdown). Crash to surface the issue. - ErrorUtilities.ThrowInternalError("TaskHost received callback response with no pending request. RequestId={0}, Type={1}", - callbackPacket.RequestId, packet.Type); + ErrorUtilities.ThrowInternalError($"TaskHost received callback response with no pending request. RequestId={callbackPacket.RequestId}, Type={packet.Type}"); } /// @@ -1103,7 +1100,7 @@ private TaskExecutionContext CreateTaskContext(TaskHostConfiguration configurati if (!_taskContexts.TryAdd(taskId, context)) { context.Dispose(); - ErrorUtilities.ThrowInternalError("Task ID {0} already exists in TaskHost.", taskId); + ErrorUtilities.ThrowInternalError($"Task ID {taskId} already exists in TaskHost."); } return context; @@ -1172,8 +1169,7 @@ private void HandleTaskHostConfiguration(TaskHostConfiguration taskHostConfigura // Only _activeTaskCount must be zero — blocked tasks (waiting on BuildProjectFile // callbacks) don't prevent accepting a new nested task configuration. ErrorUtilities.VerifyThrow(_activeTaskCount == 0, - "Why are we getting a TaskHostConfiguration packet while a task is actively executing? activeTaskCount={0}", - _activeTaskCount); + $"Why are we getting a TaskHostConfiguration packet while a task is actively executing? activeTaskCount={_activeTaskCount}"); if (_blockedTaskCount > 0) { diff --git a/src/Shared/ErrorUtilities.cs b/src/Shared/ErrorUtilities.cs index 81d609cee19..1fa0ea45f1d 100644 --- a/src/Shared/ErrorUtilities.cs +++ b/src/Shared/ErrorUtilities.cs @@ -3,645 +3,497 @@ using System; using System.Collections.Generic; -using System.Diagnostics; using System.Diagnostics.CodeAnalysis; -using System.Globalization; -using System.IO; using System.Runtime.CompilerServices; using System.Threading; using Microsoft.Build.Framework; +using Microsoft.Build.Framework.Utilities; -#if BUILDINGAPPXTASKS -namespace Microsoft.Build.AppxPackage.Shared -#else -namespace Microsoft.Build.Shared -#endif +namespace Microsoft.Build.Shared; + +/// +/// This class contains methods that are useful for error checking and validation. +/// +internal static class ErrorUtilities { + /// + public static void DebugTraceMessage(string category, string message) + => FrameworkErrorUtilities.DebugTraceMessage(category, message); + + /// + public static void DebugTraceMessage(string category, ref FrameworkErrorUtilities.DebugTraceInterpolatedStringHandler handler) + => FrameworkErrorUtilities.DebugTraceMessage(category, ref handler); + + /// + [DoesNotReturn] + internal static void ThrowInternalError(string message) + => FrameworkErrorUtilities.ThrowInternalError(message); + + /// + [DoesNotReturn] + internal static void ThrowInternalError(ref UnconditionalInterpolatedStringHandler handler) + => FrameworkErrorUtilities.ThrowInternalError(ref handler); + + /// + [DoesNotReturn] + internal static void ThrowInternalError(string message, Exception innerException) + => FrameworkErrorUtilities.ThrowInternalError(message, innerException); + + /// + [DoesNotReturn] + internal static void ThrowInternalError(ref UnconditionalInterpolatedStringHandler handler, Exception innerException) + => FrameworkErrorUtilities.ThrowInternalError(ref handler, innerException); + + /// + [DoesNotReturn] + internal static void ThrowInternalErrorUnreachable() + => FrameworkErrorUtilities.ThrowInternalErrorUnreachable(); + + /// + internal static void VerifyThrowInternalErrorUnreachable([DoesNotReturnIf(false)] bool condition) + => FrameworkErrorUtilities.VerifyThrowInternalErrorUnreachable(condition); + /// - /// This class contains methods that are useful for error checking and validation. + /// Throws InternalErrorException. + /// Indicates the code path followed should not have been possible. + /// This is only for situations that would mean that there is a bug in MSBuild itself. /// - internal static class ErrorUtilities + internal static void ThrowIfTypeDoesNotImplementToString(object param) { - private static readonly bool s_enableMSBuildDebugTracing = !String.IsNullOrEmpty(Environment.GetEnvironmentVariable("MSBUILDENABLEDEBUGTRACING")); - - public static void DebugTraceMessage(string category, string formatstring, params object[]? parameters) - { - if (s_enableMSBuildDebugTracing) - { - if (parameters != null) - { - Trace.WriteLine(String.Format(CultureInfo.CurrentCulture, formatstring, parameters), category); - } - else - { - Trace.WriteLine(formatstring, category); - } - } - } - -#if !BUILDINGAPPXTASKS - - internal static void VerifyThrowInternalError([DoesNotReturnIf(false)] bool condition, string message, params object?[]? args) - { - if (!condition) - { - ThrowInternalError(message, args); - } - } - - /// - /// Throws InternalErrorException. - /// This is only for situations that would mean that there is a bug in MSBuild itself. - /// - [DoesNotReturn] - internal static void ThrowInternalError(string message, params object?[]? args) - { - throw new InternalErrorException(ResourceUtilities.FormatString(message, args)); - } - - /// - /// Throws InternalErrorException. - /// This is only for situations that would mean that there is a bug in MSBuild itself. - /// - [DoesNotReturn] - internal static void ThrowInternalError(string message, Exception? innerException, params object?[]? args) +#if DEBUG + // Check it has a real implementation of ToString() + if (String.Equals(param.GetType().ToString(), param.ToString(), StringComparison.Ordinal)) { - throw new InternalErrorException(ResourceUtilities.FormatString(message, args), innerException); + ThrowInternalError($"This type does not implement ToString() properly {param.GetType().FullName!}"); } +#endif + } - /// - /// Throws InternalErrorException. - /// Indicates the code path followed should not have been possible. - /// This is only for situations that would mean that there is a bug in MSBuild itself. - /// - [DoesNotReturn] - internal static void ThrowInternalErrorUnreachable() - { - throw new InternalErrorException("Unreachable?"); - } + /// + internal static void VerifyThrowInternalNull( + [NotNull] object? parameter, + [CallerArgumentExpression(nameof(parameter))] string? parameterName = null) + => FrameworkErrorUtilities.VerifyThrowInternalNull(parameter, parameterName); - /// - /// Throws InternalErrorException. - /// Indicates the code path followed should not have been possible. - /// This is only for situations that would mean that there is a bug in MSBuild itself. - /// - internal static void VerifyThrowInternalErrorUnreachable([DoesNotReturnIf(false)] bool condition) - { - if (!condition) - { - ThrowInternalErrorUnreachable(); - } - } - - /// - /// Throws InternalErrorException. - /// Indicates the code path followed should not have been possible. - /// This is only for situations that would mean that there is a bug in MSBuild itself. - /// - internal static void ThrowIfTypeDoesNotImplementToString(object param) + /// + /// Helper to throw an InternalErrorException when a lock on the specified object is not already held. + /// This should be used ONLY if this would indicate a bug in MSBuild rather than + /// anything caused by user action. + /// + /// The object that should already have been used as a lock. + internal static void VerifyThrowInternalLockHeld(object locker) + { + if (!Monitor.IsEntered(locker)) { -#if DEBUG - // Check it has a real implementation of ToString() - if (String.Equals(param.GetType().ToString(), param.ToString(), StringComparison.Ordinal)) - { - ThrowInternalError("This type does not implement ToString() properly {0}", param.GetType().FullName!); - } -#endif + ThrowInternalError("Lock should already have been taken"); } + } - /// - /// Helper to throw an InternalErrorException when the specified parameter is null. - /// This should be used ONLY if this would indicate a bug in MSBuild rather than - /// anything caused by user action. - /// - /// The value of the argument. - /// Parameter that should not be null - internal static void VerifyThrowInternalNull([NotNull] object? parameter, [CallerArgumentExpression(nameof(parameter))] string? parameterName = null) - { - if (parameter is null) - { - ThrowInternalError("{0} unexpectedly null", parameterName); - } - } + /// + internal static void VerifyThrowInternalLength( + [NotNull] string? parameterValue, + [CallerArgumentExpression(nameof(parameterValue))] string? parameterName = null) + => FrameworkErrorUtilities.VerifyThrowInternalLength(parameterValue, parameterName); - /// - /// Helper to throw an InternalErrorException when a lock on the specified object is not already held. - /// This should be used ONLY if this would indicate a bug in MSBuild rather than - /// anything caused by user action. - /// - /// The object that should already have been used as a lock. - internal static void VerifyThrowInternalLockHeld(object locker) - { - if (!Monitor.IsEntered(locker)) - { - ThrowInternalError("Lock should already have been taken"); - } - } + internal static void VerifyThrowInternalLength( + [NotNull] T[]? parameterValue, + [CallerArgumentExpression(nameof(parameterValue))] string? parameterName = null) + => FrameworkErrorUtilities.VerifyThrowInternalLength(parameterValue, parameterName); - /// - /// Helper to throw an InternalErrorException when the specified parameter is null or zero length. - /// This should be used ONLY if this would indicate a bug in MSBuild rather than - /// anything caused by user action. - /// - /// The value of the argument. - /// Parameter that should not be null or zero length - internal static void VerifyThrowInternalLength([NotNull] string? parameterValue, [CallerArgumentExpression(nameof(parameterValue))] string? parameterName = null) - { - VerifyThrowInternalNull(parameterValue, parameterName); + /// + internal static void VerifyThrowInternalRooted(string value) + => FrameworkErrorUtilities.VerifyThrowInternalRooted(value); - if (parameterValue.Length == 0) - { - ThrowInternalError("{0} unexpectedly empty", parameterName); - } - } + /// + internal static void VerifyThrow([DoesNotReturnIf(false)] bool condition, string message) + => FrameworkErrorUtilities.VerifyThrow(condition, message); - public static void VerifyThrowInternalLength([NotNull] T[]? parameterValue, [CallerArgumentExpression(nameof(parameterValue))] string? parameterName = null) - { - VerifyThrowInternalNull(parameterValue, parameterName); + /// + internal static void VerifyThrow( + [DoesNotReturnIf(false)] bool condition, + [InterpolatedStringHandlerArgument(nameof(condition))] ref FrameworkErrorUtilities.IsTrueInterpolatedStringHandler handler) + => FrameworkErrorUtilities.VerifyThrow(condition, ref handler); - if (parameterValue.Length == 0) - { - ThrowInternalError("{0} unexpectedly empty", parameterName); - } - } + /// + /// Throws an InvalidOperationException with the specified resource string + /// + /// Resource to use in the exception + /// Formatting args. + [DoesNotReturn] + internal static void ThrowInvalidOperation(string resourceName, params object?[]? args) + { + throw new InvalidOperationException(ResourceUtilities.FormatResourceStringStripCodeAndKeyword(resourceName, args)); + } - /// - /// Helper to throw an InternalErrorException when the specified parameter is not a rooted path. - /// This should be used ONLY if this would indicate a bug in MSBuild rather than - /// anything caused by user action. - /// - /// Parameter that should be a rooted path. - internal static void VerifyThrowInternalRooted(string value) + /// + /// Throws an InvalidOperationException if the given condition is false. + /// + internal static void VerifyThrowInvalidOperation([DoesNotReturnIf(false)] bool condition, string resourceName) + { + ResourceUtilities.VerifyResourceStringExists(resourceName); + if (!condition) { - if (!Path.IsPathRooted(value)) - { - ThrowInternalError("{0} unexpectedly not a rooted path", value); - } + ThrowInvalidOperation(resourceName, null); } + } - /// - /// This method should be used in places where one would normally put - /// an "assert". It should be used to validate that our assumptions are - /// true, where false would indicate that there must be a bug in our - /// code somewhere. This should not be used to throw errors based on bad - /// user input or anything that the user did wrong. - /// - internal static void VerifyThrow([DoesNotReturnIf(false)] bool condition, string unformattedMessage) + /// + /// Overload for one string format argument. + /// + internal static void VerifyThrowInvalidOperation([DoesNotReturnIf(false)] bool condition, string resourceName, object arg0) + { + ResourceUtilities.VerifyResourceStringExists(resourceName); + // PERF NOTE: check the condition here instead of pushing it into + // the ThrowInvalidOperation() method, because that method always + // allocates memory for its variable array of arguments + if (!condition) { - if (!condition) - { - ThrowInternalError(unformattedMessage, null, null); - } + ThrowInvalidOperation(resourceName, arg0); } + } - /// - /// Overload for one string format argument. - /// - internal static void VerifyThrow([DoesNotReturnIf(false)] bool condition, string unformattedMessage, int arg0) + /// + /// Overload for two string format arguments. + /// + internal static void VerifyThrowInvalidOperation([DoesNotReturnIf(false)] bool condition, string resourceName, object arg0, object arg1) + { + ResourceUtilities.VerifyResourceStringExists(resourceName); + // PERF NOTE: check the condition here instead of pushing it into + // the ThrowInvalidOperation() method, because that method always + // allocates memory for its variable array of arguments + if (!condition) { - if (!condition) - { - ThrowInternalError(unformattedMessage, arg0); - } + ThrowInvalidOperation(resourceName, arg0, arg1); } + } - /// - /// Overload for one string format argument. - /// - internal static void VerifyThrow([DoesNotReturnIf(false)] bool condition, string unformattedMessage, object arg0) + /// + /// Overload for three string format arguments. + /// + internal static void VerifyThrowInvalidOperation([DoesNotReturnIf(false)] bool condition, string resourceName, object arg0, object arg1, object arg2) + { + ResourceUtilities.VerifyResourceStringExists(resourceName); + // PERF NOTE: check the condition here instead of pushing it into + // the ThrowInvalidOperation() method, because that method always + // allocates memory for its variable array of arguments + if (!condition) { - if (!condition) - { - ThrowInternalError(unformattedMessage, arg0); - } + ThrowInvalidOperation(resourceName, arg0, arg1, arg2); } + } - /// - /// Overload for two string format arguments. - /// - internal static void VerifyThrow([DoesNotReturnIf(false)] bool condition, string unformattedMessage, int arg0, int arg1) - { - if (!condition) - { - ThrowInternalError(unformattedMessage, arg0, arg1); - } - } + /// + /// Overload for four string format arguments. + /// + internal static void VerifyThrowInvalidOperation([DoesNotReturnIf(false)] bool condition, string resourceName, object arg0, object arg1, object arg2, object arg3) + { + ResourceUtilities.VerifyResourceStringExists(resourceName); - /// - /// Overload for two string format arguments. - /// - internal static void VerifyThrow([DoesNotReturnIf(false)] bool condition, string unformattedMessage, object arg0, object arg1) + // PERF NOTE: check the condition here instead of pushing it into + // the ThrowInvalidOperation() method, because that method always + // allocates memory for its variable array of arguments + if (!condition) { - if (!condition) - { - ThrowInternalError(unformattedMessage, arg0, arg1); - } + ThrowInvalidOperation(resourceName, arg0, arg1, arg2, arg3); } + } - /// - /// Overload for three string format arguments. - /// - internal static void VerifyThrow([DoesNotReturnIf(false)] bool condition, string unformattedMessage, object arg0, object arg1, object arg2) - { - if (!condition) - { - ThrowInternalError(unformattedMessage, arg0, arg1, arg2); - } - } + /// + /// Throws an ArgumentException that can include an inner exception. + /// + /// PERF WARNING: calling a method that takes a variable number of arguments + /// is expensive, because memory is allocated for the array of arguments -- do + /// not call this method repeatedly in performance-critical scenarios + /// + [DoesNotReturn] + internal static void ThrowArgument(string resourceName, params object?[]? args) + { + ThrowArgument(null, resourceName, args); + } - /// - /// Overload for four string format arguments. - /// - internal static void VerifyThrow([DoesNotReturnIf(false)] bool condition, string unformattedMessage, object arg0, object arg1, object arg2, object arg3) - { - if (!condition) - { - ThrowInternalError(unformattedMessage, arg0, arg1, arg2, arg3); - } - } + /// + /// Throws an ArgumentException that can include an inner exception. + /// + /// PERF WARNING: calling a method that takes a variable number of arguments + /// is expensive, because memory is allocated for the array of arguments -- do + /// not call this method repeatedly in performance-critical scenarios + /// + /// + /// This method is thread-safe. + /// + /// Can be null. + /// + /// + [DoesNotReturn] + internal static void ThrowArgument(Exception? innerException, string resourceName, params object?[]? args) + { + throw new ArgumentException(ResourceUtilities.FormatResourceStringStripCodeAndKeyword(resourceName, args), innerException); + } - /// - /// Throws an InvalidOperationException with the specified resource string - /// - /// Resource to use in the exception - /// Formatting args. - [DoesNotReturn] - internal static void ThrowInvalidOperation(string resourceName, params object?[]? args) - { - throw new InvalidOperationException(ResourceUtilities.FormatResourceStringStripCodeAndKeyword(resourceName, args)); - } + /// + /// Throws an ArgumentException if the given condition is false. + /// + internal static void VerifyThrowArgument([DoesNotReturnIf(false)] bool condition, string resourceName) + { + VerifyThrowArgument(condition, null, resourceName); + } - /// - /// Throws an InvalidOperationException if the given condition is false. - /// - internal static void VerifyThrowInvalidOperation([DoesNotReturnIf(false)] bool condition, string resourceName) - { - ResourceUtilities.VerifyResourceStringExists(resourceName); - if (!condition) - { - ThrowInvalidOperation(resourceName, null); - } - } + /// + /// Overload for one string format argument. + /// + internal static void VerifyThrowArgument([DoesNotReturnIf(false)] bool condition, string resourceName, object arg0) + { + VerifyThrowArgument(condition, null, resourceName, arg0); + } - /// - /// Overload for one string format argument. - /// - internal static void VerifyThrowInvalidOperation([DoesNotReturnIf(false)] bool condition, string resourceName, object arg0) - { - ResourceUtilities.VerifyResourceStringExists(resourceName); - // PERF NOTE: check the condition here instead of pushing it into - // the ThrowInvalidOperation() method, because that method always - // allocates memory for its variable array of arguments - if (!condition) - { - ThrowInvalidOperation(resourceName, arg0); - } - } + /// + /// Overload for two string format arguments. + /// + internal static void VerifyThrowArgument([DoesNotReturnIf(false)] bool condition, string resourceName, object arg0, object arg1) + { + VerifyThrowArgument(condition, null, resourceName, arg0, arg1); + } - /// - /// Overload for two string format arguments. - /// - internal static void VerifyThrowInvalidOperation([DoesNotReturnIf(false)] bool condition, string resourceName, object arg0, object arg1) - { - ResourceUtilities.VerifyResourceStringExists(resourceName); - // PERF NOTE: check the condition here instead of pushing it into - // the ThrowInvalidOperation() method, because that method always - // allocates memory for its variable array of arguments - if (!condition) - { - ThrowInvalidOperation(resourceName, arg0, arg1); - } - } + /// + /// Overload for three string format arguments. + /// + internal static void VerifyThrowArgument([DoesNotReturnIf(false)] bool condition, string resourceName, object arg0, object arg1, object arg2) + { + VerifyThrowArgument(condition, null, resourceName, arg0, arg1, arg2); + } - /// - /// Overload for three string format arguments. - /// - internal static void VerifyThrowInvalidOperation([DoesNotReturnIf(false)] bool condition, string resourceName, object arg0, object arg1, object arg2) - { - ResourceUtilities.VerifyResourceStringExists(resourceName); - // PERF NOTE: check the condition here instead of pushing it into - // the ThrowInvalidOperation() method, because that method always - // allocates memory for its variable array of arguments - if (!condition) - { - ThrowInvalidOperation(resourceName, arg0, arg1, arg2); - } - } + /// + /// Overload for four string format arguments. + /// + internal static void VerifyThrowArgument([DoesNotReturnIf(false)] bool condition, string resourceName, object arg0, object arg1, object arg2, object arg3) + { + VerifyThrowArgument(condition, null, resourceName, arg0, arg1, arg2, arg3); + } - /// - /// Overload for four string format arguments. - /// - internal static void VerifyThrowInvalidOperation([DoesNotReturnIf(false)] bool condition, string resourceName, object arg0, object arg1, object arg2, object arg3) + /// + /// Throws an ArgumentException that includes an inner exception, if + /// the given condition is false. + /// + /// + /// Can be null. + /// + internal static void VerifyThrowArgument([DoesNotReturnIf(false)] bool condition, Exception? innerException, string resourceName) + { + ResourceUtilities.VerifyResourceStringExists(resourceName); + if (!condition) { - ResourceUtilities.VerifyResourceStringExists(resourceName); - - // PERF NOTE: check the condition here instead of pushing it into - // the ThrowInvalidOperation() method, because that method always - // allocates memory for its variable array of arguments - if (!condition) - { - ThrowInvalidOperation(resourceName, arg0, arg1, arg2, arg3); - } + ThrowArgument(innerException, resourceName, null); } + } - /// - /// Throws an ArgumentException that can include an inner exception. - /// - /// PERF WARNING: calling a method that takes a variable number of arguments - /// is expensive, because memory is allocated for the array of arguments -- do - /// not call this method repeatedly in performance-critical scenarios - /// - [DoesNotReturn] - internal static void ThrowArgument(string resourceName, params object?[]? args) - { - ThrowArgument(null, resourceName, args); - } + /// + /// Overload for one string format argument. + /// + internal static void VerifyThrowArgument([DoesNotReturnIf(false)] bool condition, Exception? innerException, string resourceName, object arg0) + { + ResourceUtilities.VerifyResourceStringExists(resourceName); - /// - /// Throws an ArgumentException that can include an inner exception. - /// - /// PERF WARNING: calling a method that takes a variable number of arguments - /// is expensive, because memory is allocated for the array of arguments -- do - /// not call this method repeatedly in performance-critical scenarios - /// - /// - /// This method is thread-safe. - /// - /// Can be null. - /// - /// - [DoesNotReturn] - internal static void ThrowArgument(Exception? innerException, string resourceName, params object?[]? args) + if (!condition) { - throw new ArgumentException(ResourceUtilities.FormatResourceStringStripCodeAndKeyword(resourceName, args), innerException); + ThrowArgument(innerException, resourceName, arg0); } + } - /// - /// Throws an ArgumentException if the given condition is false. - /// - internal static void VerifyThrowArgument([DoesNotReturnIf(false)] bool condition, string resourceName) - { - VerifyThrowArgument(condition, null, resourceName); - } + /// + /// Overload for two string format arguments. + /// + internal static void VerifyThrowArgument([DoesNotReturnIf(false)] bool condition, Exception? innerException, string resourceName, object arg0, object arg1) + { + ResourceUtilities.VerifyResourceStringExists(resourceName); - /// - /// Overload for one string format argument. - /// - internal static void VerifyThrowArgument([DoesNotReturnIf(false)] bool condition, string resourceName, object arg0) + if (!condition) { - VerifyThrowArgument(condition, null, resourceName, arg0); + ThrowArgument(innerException, resourceName, arg0, arg1); } + } - /// - /// Overload for two string format arguments. - /// - internal static void VerifyThrowArgument([DoesNotReturnIf(false)] bool condition, string resourceName, object arg0, object arg1) - { - VerifyThrowArgument(condition, null, resourceName, arg0, arg1); - } + /// + /// Overload for three string format arguments. + /// + internal static void VerifyThrowArgument([DoesNotReturnIf(false)] bool condition, Exception? innerException, string resourceName, object arg0, object arg1, object arg2) + { + ResourceUtilities.VerifyResourceStringExists(resourceName); - /// - /// Overload for three string format arguments. - /// - internal static void VerifyThrowArgument([DoesNotReturnIf(false)] bool condition, string resourceName, object arg0, object arg1, object arg2) + if (!condition) { - VerifyThrowArgument(condition, null, resourceName, arg0, arg1, arg2); + ThrowArgument(innerException, resourceName, arg0, arg1, arg2); } + } - /// - /// Overload for four string format arguments. - /// - internal static void VerifyThrowArgument([DoesNotReturnIf(false)] bool condition, string resourceName, object arg0, object arg1, object arg2, object arg3) - { - VerifyThrowArgument(condition, null, resourceName, arg0, arg1, arg2, arg3); - } + /// + /// Overload for four string format arguments. + /// + internal static void VerifyThrowArgument([DoesNotReturnIf(false)] bool condition, Exception? innerException, string resourceName, object arg0, object arg1, object arg2, object arg3) + { + ResourceUtilities.VerifyResourceStringExists(resourceName); - /// - /// Throws an ArgumentException that includes an inner exception, if - /// the given condition is false. - /// - /// - /// Can be null. - /// - internal static void VerifyThrowArgument([DoesNotReturnIf(false)] bool condition, Exception? innerException, string resourceName) + if (!condition) { - ResourceUtilities.VerifyResourceStringExists(resourceName); - if (!condition) - { - ThrowArgument(innerException, resourceName, null); - } + ThrowArgument(innerException, resourceName, arg0, arg1, arg2, arg3); } + } - /// - /// Overload for one string format argument. - /// - internal static void VerifyThrowArgument([DoesNotReturnIf(false)] bool condition, Exception? innerException, string resourceName, object arg0) - { - ResourceUtilities.VerifyResourceStringExists(resourceName); - - if (!condition) - { - ThrowArgument(innerException, resourceName, arg0); - } - } + /// + /// Throws an argument out of range exception. + /// + [DoesNotReturn] + internal static void ThrowArgumentOutOfRange(string? parameterName) + { + throw new ArgumentOutOfRangeException(parameterName); + } - /// - /// Overload for two string format arguments. - /// - internal static void VerifyThrowArgument([DoesNotReturnIf(false)] bool condition, Exception? innerException, string resourceName, object arg0, object arg1) + /// + /// Throws an ArgumentOutOfRangeException using the given parameter name + /// if the condition is false. + /// + internal static void VerifyThrowArgumentOutOfRange([DoesNotReturnIf(false)] bool condition, [CallerArgumentExpression(nameof(condition))] string? parameterName = null) + { + if (!condition) { - ResourceUtilities.VerifyResourceStringExists(resourceName); - - if (!condition) - { - ThrowArgument(innerException, resourceName, arg0, arg1); - } + ThrowArgumentOutOfRange(parameterName); } + } - /// - /// Overload for three string format arguments. - /// - internal static void VerifyThrowArgument([DoesNotReturnIf(false)] bool condition, Exception? innerException, string resourceName, object arg0, object arg1, object arg2) - { - ResourceUtilities.VerifyResourceStringExists(resourceName); - - if (!condition) - { - ThrowArgument(innerException, resourceName, arg0, arg1, arg2); - } - } + /// + /// Throws an ArgumentNullException if the given string parameter is null + /// and ArgumentException if it has zero length. + /// + internal static void VerifyThrowArgumentLength([NotNull] string? parameter, [CallerArgumentExpression(nameof(parameter))] string? parameterName = null) + { + VerifyThrowArgumentNull(parameter, parameterName); - /// - /// Overload for four string format arguments. - /// - internal static void VerifyThrowArgument([DoesNotReturnIf(false)] bool condition, Exception? innerException, string resourceName, object arg0, object arg1, object arg2, object arg3) + if (parameter.Length == 0) { - ResourceUtilities.VerifyResourceStringExists(resourceName); - - if (!condition) - { - ThrowArgument(innerException, resourceName, arg0, arg1, arg2, arg3); - } + ThrowArgumentLength(parameterName); } + } - /// - /// Throws an argument out of range exception. - /// - [DoesNotReturn] - internal static void ThrowArgumentOutOfRange(string? parameterName) - { - throw new ArgumentOutOfRangeException(parameterName); - } + /// + /// Throws an ArgumentNullException if the given collection is null + /// and ArgumentException if it has zero length. + /// + internal static void VerifyThrowArgumentLength([NotNull] IReadOnlyCollection parameter, [CallerArgumentExpression(nameof(parameter))] string? parameterName = null) + { + VerifyThrowArgumentNull(parameter, parameterName); - /// - /// Throws an ArgumentOutOfRangeException using the given parameter name - /// if the condition is false. - /// - internal static void VerifyThrowArgumentOutOfRange([DoesNotReturnIf(false)] bool condition, [CallerArgumentExpression(nameof(condition))] string? parameterName = null) + if (parameter.Count == 0) { - if (!condition) - { - ThrowArgumentOutOfRange(parameterName); - } + ThrowArgumentLength(parameterName); } + } - /// - /// Throws an ArgumentNullException if the given string parameter is null - /// and ArgumentException if it has zero length. - /// - internal static void VerifyThrowArgumentLength([NotNull] string? parameter, [CallerArgumentExpression(nameof(parameter))] string? parameterName = null) + /// + /// Throws an ArgumentException if the given collection is not null but of zero length. + /// + internal static void VerifyThrowArgumentLengthIfNotNull([MaybeNull] IReadOnlyCollection? parameter, [CallerArgumentExpression(nameof(parameter))] string? parameterName = null) + { + if (parameter?.Count == 0) { - VerifyThrowArgumentNull(parameter, parameterName); - - if (parameter.Length == 0) - { - ThrowArgumentLength(parameterName); - } + ThrowArgumentLength(parameterName); } + } - /// - /// Throws an ArgumentNullException if the given collection is null - /// and ArgumentException if it has zero length. - /// - internal static void VerifyThrowArgumentLength([NotNull] IReadOnlyCollection parameter, [CallerArgumentExpression(nameof(parameter))] string? parameterName = null) - { - VerifyThrowArgumentNull(parameter, parameterName); - - if (parameter.Count == 0) - { - ThrowArgumentLength(parameterName); - } - } + [DoesNotReturn] + private static void ThrowArgumentLength(string? parameterName) + { + throw new ArgumentException(ResourceUtilities.FormatResourceStringStripCodeAndKeyword("Shared.ParameterCannotHaveZeroLength", parameterName)); + } - /// - /// Throws an ArgumentException if the given collection is not null but of zero length. - /// - internal static void VerifyThrowArgumentLengthIfNotNull([MaybeNull] IReadOnlyCollection? parameter, [CallerArgumentExpression(nameof(parameter))] string? parameterName = null) - { - if (parameter?.Count == 0) - { - ThrowArgumentLength(parameterName); - } - } + /// + /// Throws an ArgumentNullException if the given string parameter is null + /// and ArgumentException if it has zero length. + /// + internal static void VerifyThrowArgumentInvalidPath([NotNull] string parameter, [CallerArgumentExpression(nameof(parameter))] string? parameterName = null) + { + VerifyThrowArgumentNull(parameter, parameterName); - [DoesNotReturn] - private static void ThrowArgumentLength(string? parameterName) + if (FileUtilities.PathIsInvalid(parameter)) { - throw new ArgumentException(ResourceUtilities.FormatResourceStringStripCodeAndKeyword("Shared.ParameterCannotHaveZeroLength", parameterName)); + ThrowArgument("Shared.ParameterCannotHaveInvalidPathChars", parameterName, parameter); } + } - /// - /// Throws an ArgumentNullException if the given string parameter is null - /// and ArgumentException if it has zero length. - /// - internal static void VerifyThrowArgumentInvalidPath([NotNull] string parameter, [CallerArgumentExpression(nameof(parameter))] string? parameterName = null) + /// + /// Throws an ArgumentException if the string has zero length, unless it is + /// null, in which case no exception is thrown. + /// + internal static void VerifyThrowArgumentLengthIfNotNull(string? parameter, [CallerArgumentExpression(nameof(parameter))] string? parameterName = null) + { + if (parameter?.Length == 0) { - VerifyThrowArgumentNull(parameter, parameterName); - - if (FileUtilities.PathIsInvalid(parameter)) - { - ThrowArgument("Shared.ParameterCannotHaveInvalidPathChars", parameterName, parameter); - } + ThrowArgumentLength(parameterName); } + } - /// - /// Throws an ArgumentException if the string has zero length, unless it is - /// null, in which case no exception is thrown. - /// - internal static void VerifyThrowArgumentLengthIfNotNull(string? parameter, [CallerArgumentExpression(nameof(parameter))] string? parameterName = null) - { - if (parameter?.Length == 0) - { - ThrowArgumentLength(parameterName); - } - } + /// + /// Throws an ArgumentNullException if the given parameter is null. + /// + internal static void VerifyThrowArgumentNull([NotNull] object? parameter, [CallerArgumentExpression(nameof(parameter))] string? parameterName = null) + { + VerifyThrowArgumentNull(parameter, parameterName, "Shared.ParameterCannotBeNull"); + } - /// - /// Throws an ArgumentNullException if the given parameter is null. - /// - internal static void VerifyThrowArgumentNull([NotNull] object? parameter, [CallerArgumentExpression(nameof(parameter))] string? parameterName = null) + /// + /// Throws an ArgumentNullException if the given parameter is null. + /// + internal static void VerifyThrowArgumentNull([NotNull] object? parameter, string? parameterName, string resourceName) + { + ResourceUtilities.VerifyResourceStringExists(resourceName); + if (parameter is null) { - VerifyThrowArgumentNull(parameter, parameterName, "Shared.ParameterCannotBeNull"); + ThrowArgumentNull(parameterName, resourceName); } + } - /// - /// Throws an ArgumentNullException if the given parameter is null. - /// - internal static void VerifyThrowArgumentNull([NotNull] object? parameter, string? parameterName, string resourceName) - { - ResourceUtilities.VerifyResourceStringExists(resourceName); - if (parameter is null) - { - ThrowArgumentNull(parameterName, resourceName); - } - } + [DoesNotReturn] + internal static void ThrowArgumentNull(string? parameterName, string resourceName) + { + // Most ArgumentNullException overloads append its own rather clunky multi-line message. So use the one overload that doesn't. + throw new ArgumentNullException(ResourceUtilities.FormatResourceStringStripCodeAndKeyword(resourceName, parameterName), (Exception?)null); + } - [DoesNotReturn] - internal static void ThrowArgumentNull(string? parameterName, string resourceName) + internal static void VerifyThrowObjectDisposed([DoesNotReturnIf(false)] bool condition, string objectName) + { + if (!condition) { - // Most ArgumentNullException overloads append its own rather clunky multi-line message. So use the one overload that doesn't. - throw new ArgumentNullException(ResourceUtilities.FormatResourceStringStripCodeAndKeyword(resourceName, parameterName), (Exception?)null); + ThrowObjectDisposed(objectName); } + } - internal static void VerifyThrowObjectDisposed([DoesNotReturnIf(false)] bool condition, string objectName) - { - if (!condition) - { - ThrowObjectDisposed(objectName); - } - } + [DoesNotReturn] + internal static void ThrowObjectDisposed(string objectName) + { + throw new ObjectDisposedException(objectName); + } - [DoesNotReturn] - internal static void ThrowObjectDisposed(string objectName) - { - throw new ObjectDisposedException(objectName); - } + /// + /// A utility that verifies the parameters provided to a standard ICollection.CopyTo call. + /// + /// If is null. + /// If falls outside of the bounds . + /// If there is insufficient capacity to copy the collection contents into + /// when starting at . + internal static void VerifyCollectionCopyToArguments( + [NotNull] T[]? array, + string arrayParameterName, + int arrayIndex, + string arrayIndexParameterName, + int requiredCapacity) + { + VerifyThrowArgumentNull(array, arrayParameterName); + VerifyThrowArgumentOutOfRange(arrayIndex >= 0 && arrayIndex < array.Length, arrayIndexParameterName); - /// - /// A utility that verifies the parameters provided to a standard ICollection.CopyTo call. - /// - /// If is null. - /// If falls outside of the bounds . - /// If there is insufficient capacity to copy the collection contents into - /// when starting at . - internal static void VerifyCollectionCopyToArguments( - [NotNull] T[]? array, - string arrayParameterName, - int arrayIndex, - string arrayIndexParameterName, - int requiredCapacity) + int arrayCapacity = array.Length - arrayIndex; + if (requiredCapacity > arrayCapacity) { - VerifyThrowArgumentNull(array, arrayParameterName); - VerifyThrowArgumentOutOfRange(arrayIndex >= 0 && arrayIndex < array.Length, arrayIndexParameterName); - - int arrayCapacity = array.Length - arrayIndex; - if (requiredCapacity > arrayCapacity) - { - throw new ArgumentException( - ResourceUtilities.GetResourceString("Shared.CollectionCopyToFailureProvidedArrayIsTooSmall"), - arrayParameterName); - } + throw new ArgumentException( + ResourceUtilities.GetResourceString("Shared.CollectionCopyToFailureProvidedArrayIsTooSmall"), + arrayParameterName); } -#endif } } diff --git a/src/Shared/LogMessagePacketBase.cs b/src/Shared/LogMessagePacketBase.cs index 17589c57473..4306a91986d 100644 --- a/src/Shared/LogMessagePacketBase.cs +++ b/src/Shared/LogMessagePacketBase.cs @@ -440,7 +440,7 @@ internal void ReadFromStream(ITranslator translator) else { _buildEvent = ReadEventFromStream(_eventType, translator); - ErrorUtilities.VerifyThrow(_buildEvent is not null, "Not Supported LoggingEventType {0}", _eventType.ToString()); + ErrorUtilities.VerifyThrow(_buildEvent is not null, $"Not Supported LoggingEventType {_eventType}"); } _eventType = GetLoggingEventId(_buildEvent); @@ -768,7 +768,7 @@ protected virtual void WriteEventToStream(BuildEventArgs buildEvent, LoggingEven WriteBuildWarningEventToStream((BuildWarningEventArgs)buildEvent, translator); break; default: - ErrorUtilities.ThrowInternalError("Not Supported LoggingEventType {0}", eventType.ToString()); + ErrorUtilities.ThrowInternalError($"Not Supported LoggingEventType {eventType}"); break; } } diff --git a/src/Shared/NodeEndpointOutOfProcBase.cs b/src/Shared/NodeEndpointOutOfProcBase.cs index 7d1c8195028..16828229734 100644 --- a/src/Shared/NodeEndpointOutOfProcBase.cs +++ b/src/Shared/NodeEndpointOutOfProcBase.cs @@ -169,7 +169,7 @@ public LinkStatus LinkStatus /// The factory used to create packets. public void Listen(INodePacketFactory factory) { - ErrorUtilities.VerifyThrow(_status == LinkStatus.Inactive, "Link not inactive. Status is {0}", _status); + ErrorUtilities.VerifyThrow(_status == LinkStatus.Inactive, $"Link not inactive. Status is {_status}"); ErrorUtilities.VerifyThrowArgumentNull(factory, nameof(factory)); _packetFactory = factory; @@ -289,7 +289,7 @@ internal void InternalConstruct(string pipeName = null, byte parentPacketVersion /// The status the node should now be in. protected void ChangeLinkStatus(LinkStatus newStatus) { - ErrorUtilities.VerifyThrow(_status != newStatus, "Attempting to change status to existing status {0}.", _status); + ErrorUtilities.VerifyThrow(_status != newStatus, $"Attempting to change status to existing status {_status}."); CommunicationsUtilities.Trace($"Changing link status from {_status} to {newStatus}"); _status = newStatus; RaiseLinkStatusChanged(_status); @@ -792,7 +792,7 @@ private void RunReadLoop( break; default: - ErrorUtilities.ThrowInternalError("waitId {0} out of range.", waitId); + ErrorUtilities.ThrowInternalError($"waitId {waitId} out of range."); break; } } diff --git a/src/Shared/NodePacketFactory.cs b/src/Shared/NodePacketFactory.cs index 478c88310eb..bb7017de9b7 100644 --- a/src/Shared/NodePacketFactory.cs +++ b/src/Shared/NodePacketFactory.cs @@ -49,11 +49,9 @@ public void UnregisterPacketHandler(NodePacketType packetType) /// public void DeserializeAndRoutePacket(int nodeId, NodePacketType packetType, ITranslator translator) { - // PERF: Not using VerifyThrow to avoid boxing of packetType in the non-error case - if (!_packetFactories.TryGetValue(packetType, out PacketFactoryRecord record)) - { - ErrorUtilities.ThrowInternalError("No packet handler for type {0}", packetType); - } + ErrorUtilities.VerifyThrow( + _packetFactories.TryGetValue(packetType, out PacketFactoryRecord record), + $"No packet handler for type {packetType}"); INodePacket packet = record.DeserializePacket(translator); record.RoutePacket(nodeId, packet); @@ -64,11 +62,9 @@ public void DeserializeAndRoutePacket(int nodeId, NodePacketType packetType, ITr /// public INodePacket DeserializePacket(NodePacketType packetType, ITranslator translator) { - // PERF: Not using VerifyThrow to avoid boxing of packetType in the non-error case - if (!_packetFactories.TryGetValue(packetType, out PacketFactoryRecord record)) - { - ErrorUtilities.ThrowInternalError("No packet handler for type {0}", packetType); - } + ErrorUtilities.VerifyThrow( + _packetFactories.TryGetValue(packetType, out PacketFactoryRecord record), + $"No packet handler for type {packetType}"); return record.DeserializePacket(translator); } @@ -78,11 +74,9 @@ public INodePacket DeserializePacket(NodePacketType packetType, ITranslator tran /// public void RoutePacket(int nodeId, INodePacket packet) { - // PERF: Not using VerifyThrow to avoid boxing of packetType in the non-error case - if (!_packetFactories.TryGetValue(packet.Type, out PacketFactoryRecord record)) - { - ErrorUtilities.ThrowInternalError("No packet handler for type {0}", packet.Type); - } + ErrorUtilities.VerifyThrow( + _packetFactories.TryGetValue(packet.Type, out PacketFactoryRecord record), + $"No packet handler for type {packet.Type}"); record.RoutePacket(nodeId, packet); } diff --git a/src/Shared/ResourceUtilities.cs b/src/Shared/ResourceUtilities.cs index 9d1ccedb342..a609de8b021 100644 --- a/src/Shared/ResourceUtilities.cs +++ b/src/Shared/ResourceUtilities.cs @@ -425,9 +425,7 @@ private static void ValidateArgsIfDebug(object?[] args) if (string.Equals(param.GetType().ToString(), param.ToString(), StringComparison.Ordinal) && param.GetType() != typeof(string)) { - ErrorUtilities.ThrowInternalError( - "Invalid resource parameter type, was {0}", - param.GetType().FullName); + ErrorUtilities.ThrowInternalError($"Invalid resource parameter type, was {param.GetType().FullName}"); } } } diff --git a/src/Shared/TaskLoggingHelper.cs b/src/Shared/TaskLoggingHelper.cs index fcfa634e4c4..5cb603dd344 100644 --- a/src/Shared/TaskLoggingHelper.cs +++ b/src/Shared/TaskLoggingHelper.cs @@ -498,9 +498,9 @@ public void LogMessageFromResources(MessageImportance importance, string message #if DEBUG // Assert that the message does not contain an error code. Only errors and warnings // should have error codes. - string errorCode; - ResourceUtilities.ExtractMessageCode(true /* only msbuild codes */, FormatResourceString(messageResourceName, messageArgs), out errorCode); - ErrorUtilities.VerifyThrow(errorCode == null, errorCode, FormatResourceString(messageResourceName, messageArgs)); + string message = FormatResourceString(messageResourceName, messageArgs); + ResourceUtilities.ExtractMessageCode(msbuildCodeOnly: true, message, out string errorCode); + ErrorUtilities.VerifyThrow(errorCode == null, $"Message has error code: {message}"); #endif } diff --git a/src/Shared/TaskParameter.cs b/src/Shared/TaskParameter.cs index 75cd895b97c..249a20cb164 100644 --- a/src/Shared/TaskParameter.cs +++ b/src/Shared/TaskParameter.cs @@ -115,9 +115,8 @@ public TaskParameter(object wrappedParameter) // It's not null or invalid, so it should be a valid parameter type. ErrorUtilities.VerifyThrow( - TaskParameterTypeVerifier.IsValidInputParameter(wrappedParameterType) || TaskParameterTypeVerifier.IsValidOutputParameter(wrappedParameterType), - "How did we manage to get a task parameter of type {0} that isn't a valid parameter type?", - wrappedParameterType); + TaskParameterTypeVerifier.IsValidInputParameter(wrappedParameterType) || TaskParameterTypeVerifier.IsValidOutputParameter(wrappedParameterType), + $"How did we manage to get a task parameter of type {wrappedParameterType} that isn't a valid parameter type?"); if (wrappedParameterType.IsArray) { diff --git a/src/Shared/UnitTests/ErrorUtilities_Tests.cs b/src/Shared/UnitTests/ErrorUtilities_Tests.cs index 4bd7a10e35d..f0765a5aefc 100644 --- a/src/Shared/UnitTests/ErrorUtilities_Tests.cs +++ b/src/Shared/UnitTests/ErrorUtilities_Tests.cs @@ -5,68 +5,63 @@ using Microsoft.Build.Shared; using Xunit; -#nullable disable +namespace Microsoft.Build.UnitTests; -namespace Microsoft.Build.UnitTests +public sealed class ErrorUtilities_Tests { - public sealed class ErrorUtilities_Tests + [Fact] + public void VerifyThrowFalse() { - [Fact] - public void VerifyThrowFalse() + var ex = Assert.Throws(() => { - try - { - ErrorUtilities.VerifyThrow(false, "msbuild rules"); - } - catch (InternalErrorException e) - { - Assert.Contains("msbuild rules", e.Message); // "exception message" - return; - } + ErrorUtilities.VerifyThrow(false, "msbuild rules"); + }); - Assert.Fail("Should have thrown an exception"); - } + Assert.Contains("msbuild rules", ex.Message); + } - [Fact] - public void VerifyThrowTrue() - { - // This shouldn't throw. - ErrorUtilities.VerifyThrow(true, "msbuild rules"); - } + [Fact] + public void VerifyThrowTrue() + { + // This shouldn't throw. + ErrorUtilities.VerifyThrow(true, "msbuild rules"); + } - [Fact] - public void VerifyThrow0True() - { - // This shouldn't throw. - ErrorUtilities.VerifyThrow(true, "blah"); - } + [Fact] + public void VerifyThrow_InterpolatedString_DoesNotFormat_WhenConditionIsTrue() + { + bool formatted = false; + ErrorUtilities.VerifyThrow(true, $"message {FormatSideEffect(ref formatted)}"); + Assert.False(formatted, "Interpolated string should not have been formatted when condition is true"); + } - [Fact] - public void VerifyThrow1True() + [Fact] + public void VerifyThrow_InterpolatedString_Formats_WhenConditionIsFalse() + { + bool formatted = false; + var ex = Assert.Throws(() => { - // This shouldn't throw. - ErrorUtilities.VerifyThrow(true, "{0}", "a"); - } + ErrorUtilities.VerifyThrow(false, $"error: {FormatSideEffect(ref formatted)}"); + }); - [Fact] - public void VerifyThrow2True() - { - // This shouldn't throw. - ErrorUtilities.VerifyThrow(true, "{0}{1}", "a", "b"); - } + Assert.True(formatted, "Interpolated string should have been formatted when condition is false"); + Assert.Contains("error: formatted", ex.Message); + } - [Fact] - public void VerifyThrow3True() + [Fact] + public void VerifyThrow_InterpolatedString_FormatsMultipleArgs_WhenConditionIsFalse() + { + var ex = Assert.Throws(() => { - // This shouldn't throw. - ErrorUtilities.VerifyThrow(true, "{0}{1}{2}", "a", "b", "c"); - } + ErrorUtilities.VerifyThrow(false, $"a={1} b={2} c={"three"}"); + }); - [Fact] - public void VerifyThrow4True() - { - // This shouldn't throw. - ErrorUtilities.VerifyThrow(true, "{0}{1}{2}{3}", "a", "b", "c", "d"); - } + Assert.Contains("a=1 b=2 c=three", ex.Message); + } + + private static string FormatSideEffect(ref bool formatted) + { + formatted = true; + return "formatted"; } } diff --git a/src/Tasks/AssemblyDependency/GlobalAssemblyCache.cs b/src/Tasks/AssemblyDependency/GlobalAssemblyCache.cs index f124dfec341..ac677c05ca6 100644 --- a/src/Tasks/AssemblyDependency/GlobalAssemblyCache.cs +++ b/src/Tasks/AssemblyDependency/GlobalAssemblyCache.cs @@ -186,7 +186,7 @@ internal static string RetrievePathFromFusionName(string strongName) // net472-only = inherently Windows. CsWin32 types used directly. uint hr = NativeMethods.CreateAssemblyCache(out IAssemblyCache assemblyCache, 0); - ErrorUtilities.VerifyThrow(hr == HRESULT.S_OK, "CreateAssemblyCache failed, hr {0}", hr); + ErrorUtilities.VerifyThrow(hr == HRESULT.S_OK, $"CreateAssemblyCache failed, hr {hr}"); var assemblyInfo = new ASSEMBLY_INFO { cbAssemblyInfo = (uint)Marshal.SizeOf() }; diff --git a/src/Tasks/AssemblyResources.cs b/src/Tasks/AssemblyResources.cs index 1e31b9bafc6..093d643948c 100644 --- a/src/Tasks/AssemblyResources.cs +++ b/src/Tasks/AssemblyResources.cs @@ -24,7 +24,7 @@ internal static string GetString(string name) // NOTE: the ResourceManager.GetString() method is thread-safe string resource = PrimaryResources.GetString(name, CultureInfo.CurrentUICulture) ?? SharedResources.GetString(name, CultureInfo.CurrentUICulture); - ErrorUtilities.VerifyThrow(resource != null, "Missing resource '{0}'", name); + ErrorUtilities.VerifyThrow(resource != null, $"Missing resource '{name}'"); return resource; } @@ -40,7 +40,7 @@ internal static string GetInvariantString(string name) // NOTE: the ResourceManager.GetString() method is thread-safe string resource = PrimaryResources.GetString(name, CultureInfo.InvariantCulture) ?? SharedResources.GetString(name, CultureInfo.InvariantCulture); - ErrorUtilities.VerifyThrow(resource != null, "Missing resource '{0}'", name); + ErrorUtilities.VerifyThrow(resource != null, $"Missing resource '{name}'"); return resource; } diff --git a/src/Tasks/GenerateResource.cs b/src/Tasks/GenerateResource.cs index 75340833833..0824f61d240 100644 --- a/src/Tasks/GenerateResource.cs +++ b/src/Tasks/GenerateResource.cs @@ -2716,8 +2716,7 @@ e is SerializationException || { currentOutputFile = outFileOrDir; ErrorUtilities.VerifyThrow(_readers.Count == 1, - "We have no readers, or we have multiple readers & are ignoring subsequent ones. Num readers: {0}", - _readers.Count); + $"We have no readers, or we have multiple readers & are ignoring subsequent ones. Num readers: {_readers.Count}"); WriteResources(_readers[0], outFileOrDir); } @@ -2726,8 +2725,7 @@ e is SerializationException || try { ErrorUtilities.VerifyThrow(_readers.Count == 1, - "We have no readers, or we have multiple readers & are ignoring subsequent ones. Num readers: {0}", - _readers.Count); + $"We have no readers, or we have multiple readers & are ignoring subsequent ones. Num readers: {_readers.Count}"); CreateStronglyTypedResources(_readers[0], outFileOrDir, inFile, out currentOutputSourceCodeFile); } catch (Exception e) when (ExceptionHandling.IsIoRelatedException(e)) diff --git a/src/Tasks/MSBuild.cs b/src/Tasks/MSBuild.cs index df5ca14e163..e7d34244951 100644 --- a/src/Tasks/MSBuild.cs +++ b/src/Tasks/MSBuild.cs @@ -162,7 +162,7 @@ public string SkipNonexistentProjects return "True"; default: - ErrorUtilities.ThrowInternalError("Unexpected case {0}", _skipNonExistentProjects); + ErrorUtilities.ThrowInternalError($"Unexpected case {_skipNonExistentProjects}"); break; } @@ -352,7 +352,7 @@ public override bool Execute() } else { - ErrorUtilities.VerifyThrow(skipNonExistProjects == SkipNonExistentProjectsBehavior.Error, "skipNonexistentProjects has unexpected value {0}", skipNonExistProjects); + ErrorUtilities.VerifyThrow(skipNonExistProjects == SkipNonExistentProjectsBehavior.Error, $"skipNonexistentProjects has unexpected value {skipNonExistProjects}"); Log.LogErrorWithCodeFromResources("MSBuild.ProjectFileNotFound", project.ItemSpec); success = false; } diff --git a/src/Tasks/XamlTaskFactory/CommandLineToolSwitch.cs b/src/Tasks/XamlTaskFactory/CommandLineToolSwitch.cs index ba932ea9239..576041c585d 100644 --- a/src/Tasks/XamlTaskFactory/CommandLineToolSwitch.cs +++ b/src/Tasks/XamlTaskFactory/CommandLineToolSwitch.cs @@ -240,13 +240,13 @@ public bool BooleanValue { get { - ErrorUtilities.VerifyThrow(Type == CommandLineToolSwitchType.Boolean, "InvalidType", TypeBoolean); + ErrorUtilities.VerifyThrow(Type == CommandLineToolSwitchType.Boolean, $"InvalidType: {TypeBoolean}"); return _booleanValue; } set { - ErrorUtilities.VerifyThrow(Type == CommandLineToolSwitchType.Boolean, "InvalidType", TypeBoolean); + ErrorUtilities.VerifyThrow(Type == CommandLineToolSwitchType.Boolean, $"InvalidType: {TypeBoolean}"); _booleanValue = value; } } @@ -258,13 +258,13 @@ public int Number { get { - ErrorUtilities.VerifyThrow(Type == CommandLineToolSwitchType.Integer, "InvalidType", TypeInteger); + ErrorUtilities.VerifyThrow(Type == CommandLineToolSwitchType.Integer, $"InvalidType: {TypeInteger}"); return _number; } set { - ErrorUtilities.VerifyThrow(Type == CommandLineToolSwitchType.Integer, "InvalidType", TypeInteger); + ErrorUtilities.VerifyThrow(Type == CommandLineToolSwitchType.Integer, $"InvalidType: {TypeInteger}"); _number = value; } } @@ -277,13 +277,13 @@ public string[] StringList { get { - ErrorUtilities.VerifyThrow(Type == CommandLineToolSwitchType.StringArray, "InvalidType", TypeStringArray); + ErrorUtilities.VerifyThrow(Type == CommandLineToolSwitchType.StringArray, $"InvalidType: {TypeStringArray}"); return _stringList; } set { - ErrorUtilities.VerifyThrow(Type == CommandLineToolSwitchType.StringArray, "InvalidType", TypeStringArray); + ErrorUtilities.VerifyThrow(Type == CommandLineToolSwitchType.StringArray, $"InvalidType: {TypeStringArray}"); _stringList = value; } } @@ -296,13 +296,13 @@ public ITaskItem[] TaskItemArray { get { - ErrorUtilities.VerifyThrow(Type == CommandLineToolSwitchType.ITaskItemArray, "InvalidType", TypeITaskItemArray); + ErrorUtilities.VerifyThrow(Type == CommandLineToolSwitchType.ITaskItemArray, $"InvalidType: {TypeITaskItemArray}"); return _taskItemArray; } set { - ErrorUtilities.VerifyThrow(Type == CommandLineToolSwitchType.ITaskItemArray, "InvalidType", TypeITaskItemArray); + ErrorUtilities.VerifyThrow(Type == CommandLineToolSwitchType.ITaskItemArray, $"InvalidType: {TypeITaskItemArray}"); _taskItemArray = value; } } diff --git a/src/Tasks/XamlTaskFactory/TaskGenerator.cs b/src/Tasks/XamlTaskFactory/TaskGenerator.cs index fcd999fede7..2a4dfb31c0e 100644 --- a/src/Tasks/XamlTaskFactory/TaskGenerator.cs +++ b/src/Tasks/XamlTaskFactory/TaskGenerator.cs @@ -576,7 +576,7 @@ private static void GenerateAssignPropertyToString(CodeMemberProperty propertyNa /// private static void GenerateAssignPropertyToValue(CodeMemberProperty propertyName, string property, CodeExpression value) { - ErrorUtilities.VerifyThrow(value != null, "NullValue", property); + ErrorUtilities.VerifyThrow(value != null, $"NullValue: {property}"); CodeAssignStatement setStatement = new CodeAssignStatement(new CodePropertyReferenceExpression(new CodeVariableReferenceExpression(SwitchToAdd), property), value); propertyName.SetStatements.Add(setStatement); } diff --git a/src/Utilities/AssemblyResources.cs b/src/Utilities/AssemblyResources.cs index 3258aecf36d..4995ff40051 100644 --- a/src/Utilities/AssemblyResources.cs +++ b/src/Utilities/AssemblyResources.cs @@ -25,7 +25,7 @@ internal static string GetString(string name) string resource = PrimaryResources.GetString(name, CultureInfo.CurrentUICulture) ?? SharedResources.GetString(name, CultureInfo.CurrentUICulture); - ErrorUtilities.VerifyThrow(resource != null, "Missing resource '{0}'", name); + ErrorUtilities.VerifyThrow(resource != null, $"Missing resource '{name}'"); return resource; } diff --git a/src/Utilities/ToolLocationHelper.cs b/src/Utilities/ToolLocationHelper.cs index d84f3954661..4bfba108508 100644 --- a/src/Utilities/ToolLocationHelper.cs +++ b/src/Utilities/ToolLocationHelper.cs @@ -470,7 +470,7 @@ private static IEnumerable GetTargetPlatformMonikers(string[] string targetPlatformVersionString = targetPlatformVersion.ToString(); - ErrorUtilities.DebugTraceMessage("GetPlatformExtensionSDKLocations", "Calling with TargetPlatformIdentifier:'{0}' and TargetPlatformVersion: '{1}'", targetPlatformIdentifier, targetPlatformVersionString); + ErrorUtilities.DebugTraceMessage("GetPlatformExtensionSDKLocations", $"Calling with TargetPlatformIdentifier:'{targetPlatformIdentifier}' and TargetPlatformVersion: '{targetPlatformVersionString}'"); IEnumerable targetPlatformSDKs = RetrieveTargetPlatformList(diskRoots, extensionDiskRoots, registryRoot); return targetPlatformSDKs @@ -855,17 +855,17 @@ public static string GetPlatformSDKPropsFileLocation(string sdkIdentifier, strin } else { - ErrorUtilities.DebugTraceMessage("GetPlatformSDKPropsFileLocation", "Target platform props file location '{0}' did not exist.", propsFileLocation); + ErrorUtilities.DebugTraceMessage("GetPlatformSDKPropsFileLocation", $"Target platform props file location '{propsFileLocation}' did not exist."); } } else { - ErrorUtilities.DebugTraceMessage("GetPlatformSDKPropsFileLocation", "Could not find root SDK location for SDKI = '{0}', SDKV = '{1}'", sdkIdentifier, sdkVersion); + ErrorUtilities.DebugTraceMessage("GetPlatformSDKPropsFileLocation", $"Could not find root SDK location for SDKI = '{sdkIdentifier}', SDKV = '{sdkVersion}'"); } } catch (Exception e) when (ExceptionHandling.IsIoRelatedException(e)) { - ErrorUtilities.DebugTraceMessage("GetPlatformSDKPropsFileLocation", "Encountered exception trying to get the SDK props file Location : {0}", e.Message); + ErrorUtilities.DebugTraceMessage("GetPlatformSDKPropsFileLocation", $"Encountered exception trying to get the SDK props file Location : {e.Message}"); } return null; @@ -969,7 +969,7 @@ public static string[] GetPlatformOrFrameworkExtensionSdkReferences( if (matchingSdk == null) { - ErrorUtilities.DebugTraceMessage("GetExtensionSdkReferences", "Could not find root SDK for SDKI = '{0}', SDKV = '{1}'", targetSdkIdentifier, targetSdkVersion); + ErrorUtilities.DebugTraceMessage("GetExtensionSdkReferences", $"Could not find root SDK for SDKI = '{targetSdkIdentifier}', SDKV = '{targetSdkVersion}'"); } else { @@ -990,7 +990,7 @@ public static string[] GetPlatformOrFrameworkExtensionSdkReferences( } else { - ErrorUtilities.DebugTraceMessage("GetExtensionSdkReferences", "Could not find matching extension SDK = '{0}'", extensionSdkMoniker); + ErrorUtilities.DebugTraceMessage("GetExtensionSdkReferences", $"Could not find matching extension SDK = '{extensionSdkMoniker}'"); } } @@ -1040,13 +1040,13 @@ private static string[] GetLegacyTargetPlatformReferences(string targetPlatformI if (!FileSystems.Default.DirectoryExists(winmdLocation)) { - ErrorUtilities.DebugTraceMessage("GetLegacyTargetPlatformReferences", "Target platform location '{0}' did not exist", winmdLocation); + ErrorUtilities.DebugTraceMessage("GetLegacyTargetPlatformReferences", $"Target platform location '{winmdLocation}' did not exist"); winmdLocation = null; } } else { - ErrorUtilities.DebugTraceMessage("GetLegacyTargetPlatformReferences", "Could not find root SDK location for TPI = '{0}', TPV = '{1}'", targetPlatformIdentifier, targetPlatformVersion); + ErrorUtilities.DebugTraceMessage("GetLegacyTargetPlatformReferences", $"Could not find root SDK location for TPI = '{targetPlatformIdentifier}', TPV = '{targetPlatformVersion}'"); } if (!string.IsNullOrEmpty(winmdLocation)) @@ -1055,14 +1055,14 @@ private static string[] GetLegacyTargetPlatformReferences(string targetPlatformI if (winmdPaths.Length > 0) { - ErrorUtilities.DebugTraceMessage("GetLegacyTargetPlatformReferences", "Found {0} contract winmds in '{1}'", winmdPaths.Length, winmdLocation); + ErrorUtilities.DebugTraceMessage("GetLegacyTargetPlatformReferences", $"Found {winmdPaths.Length} contract winmds in '{winmdLocation}'"); return winmdPaths; } } } catch (Exception e) when (ExceptionHandling.IsIoRelatedException(e)) { - ErrorUtilities.DebugTraceMessage("GetLegacyTargetPlatformReferences", "Encountered exception trying to gather the platform references: {0}", e.Message); + ErrorUtilities.DebugTraceMessage("GetLegacyTargetPlatformReferences", $"Encountered exception trying to gather the platform references: {e.Message}"); } return []; @@ -1133,7 +1133,7 @@ internal static string[] GetApiContractReferences(IEnumerable apiCo foreach (ApiContract contract in apiContracts) { - ErrorUtilities.DebugTraceMessage("GetApiContractReferences", "Gathering contract references for contract with name '{0}' and version '{1}", contract.Name, contract.Version); + ErrorUtilities.DebugTraceMessage("GetApiContractReferences", $"Gathering contract references for contract with name '{contract.Name}' and version '{contract.Version}'"); string contractPath = Path.Combine(referencesRoot, contract.Name, contract.Version); if (FileSystems.Default.DirectoryExists(contractPath)) @@ -1142,7 +1142,7 @@ internal static string[] GetApiContractReferences(IEnumerable apiCo if (winmdPaths.Length > 0) { - ErrorUtilities.DebugTraceMessage("GetApiContractReferences", "Found {0} contract winmds in '{1}'", winmdPaths.Length, contractPath); + ErrorUtilities.DebugTraceMessage("GetApiContractReferences", $"Found {winmdPaths.Length} contract winmds in '{contractPath}'"); contractWinMDs.AddRange(winmdPaths); } } @@ -1162,12 +1162,12 @@ private static bool TryGetPlatformManifest(TargetPlatformSDK matchingSdk, string { if (!matchingSdk.Platforms.TryGetValue(platformKey, out platformManifestLocation)) { - ErrorUtilities.DebugTraceMessage("GetPlatformManifest", "Target platform location '{0}' did not exist or did not contain Platform.xml", platformManifestLocation); + ErrorUtilities.DebugTraceMessage("GetPlatformManifest", $"Target platform location '{platformManifestLocation}' did not exist or did not contain Platform.xml"); } } else { - ErrorUtilities.DebugTraceMessage("GetPlatformManifest", "Could not find root SDK for '{0}'", platformKey); + ErrorUtilities.DebugTraceMessage("GetPlatformManifest", $"Could not find root SDK for '{platformKey}'"); } if (!string.IsNullOrEmpty(platformManifestLocation)) @@ -1182,7 +1182,7 @@ private static bool TryGetPlatformManifest(TargetPlatformSDK matchingSdk, string } catch (Exception e) when (ExceptionHandling.IsIoRelatedException(e)) { - ErrorUtilities.DebugTraceMessage("GetValueUsingMatchingSDKManifest", "Encountered exception trying to check if SDK is versioned: {0}", e.Message); + ErrorUtilities.DebugTraceMessage("GetValueUsingMatchingSDKManifest", $"Encountered exception trying to check if SDK is versioned: {e.Message}"); } return false; @@ -1539,7 +1539,7 @@ private static bool TryParsePlatformVersion(string platformMoniker, out Version } catch (ArgumentException e) { - ErrorUtilities.DebugTraceMessage("TryParsePlatformVersion", "Cannot create FrameworkName object, Exception:{0}", e.Message); + ErrorUtilities.DebugTraceMessage("TryParsePlatformVersion", $"Cannot create FrameworkName object, Exception:{e.Message}"); } if (framework != null) @@ -2503,7 +2503,7 @@ private static void GatherExtensionSDKListFromDirectory(IEnumerable disk DirectoryInfo rootInfo = new DirectoryInfo(diskRoot); if (!rootInfo.Exists) { - ErrorUtilities.DebugTraceMessage("GatherExtensionSDKListFromDirectory", "DiskRoot '{0}'does not exist, skipping it", diskRoot); + ErrorUtilities.DebugTraceMessage("GatherExtensionSDKListFromDirectory", $"DiskRoot '{diskRoot}' does not exist, skipping it"); continue; } @@ -2524,22 +2524,22 @@ private static void GatherExtensionSDKListFromDirectory(IEnumerable disk internal static void GatherExtensionSDKs(DirectoryInfo extensionSdksDirectory, TargetPlatformSDK targetPlatformSDK) { - ErrorUtilities.DebugTraceMessage("GatherExtensionSDKs", "Found ExtensionsSDK folder '{0}'. ", extensionSdksDirectory.FullName); + ErrorUtilities.DebugTraceMessage("GatherExtensionSDKs", $"Found ExtensionsSDK folder '{extensionSdksDirectory.FullName}'. "); DirectoryInfo[] sdkNameDirectories = extensionSdksDirectory.GetDirectories(); - ErrorUtilities.DebugTraceMessage("GatherExtensionSDKs", "Found '{0}' sdkName directories under '{1}'", sdkNameDirectories.Length, extensionSdksDirectory.FullName); + ErrorUtilities.DebugTraceMessage("GatherExtensionSDKs", $"Found '{sdkNameDirectories.Length}' sdkName directories under '{extensionSdksDirectory.FullName}'"); // For each SDKName under the ExtensionSDKs directory foreach (DirectoryInfo sdkNameFolders in sdkNameDirectories) { DirectoryInfo[] sdkVersionDirectories = sdkNameFolders.GetDirectories(); - ErrorUtilities.DebugTraceMessage("GatherExtensionSDKs", "Found '{0}' sdkVersion directories under '{1}'", sdkVersionDirectories.Length, sdkNameFolders.FullName); + ErrorUtilities.DebugTraceMessage("GatherExtensionSDKs", $"Found '{sdkVersionDirectories.Length}' sdkVersion directories under '{sdkNameFolders.FullName}'"); // For each Version directory under the SDK Name foreach (DirectoryInfo sdkVersionDirectory in sdkVersionDirectories) { // Make sure the version folder parses to a version, anything that cannot parse directly to a version is to be ignored. - ErrorUtilities.DebugTraceMessage("GatherExtensionSDKs", "Parsed sdk version folder '{0}' under '{1}'", sdkVersionDirectory.Name, sdkVersionDirectory.FullName); + ErrorUtilities.DebugTraceMessage("GatherExtensionSDKs", $"Parsed sdk version folder '{sdkVersionDirectory.Name}' under '{sdkVersionDirectory.FullName}'"); if (Version.TryParse(sdkVersionDirectory.Name, out Version _)) { // Create SDK name based on the folder structure. We could open the manifest here and read the display name, but that would @@ -2549,7 +2549,7 @@ internal static void GatherExtensionSDKs(DirectoryInfo extensionSdksDirectory, T // Make sure we have not added the SDK to the list of found SDKs before. if (!targetPlatformSDK.ExtensionSDKs.ContainsKey(SDKKey)) { - ErrorUtilities.DebugTraceMessage("GatherExtensionSDKs", "SDKKey '{0}' was not already found.", SDKKey); + ErrorUtilities.DebugTraceMessage("GatherExtensionSDKs", $"SDKKey '{SDKKey}' was not already found."); string pathToSDKManifest = Path.Combine(sdkVersionDirectory.FullName, "SDKManifest.xml"); if (FileUtilities.FileExistsNoThrow(pathToSDKManifest)) { @@ -2557,17 +2557,17 @@ internal static void GatherExtensionSDKs(DirectoryInfo extensionSdksDirectory, T } else { - ErrorUtilities.DebugTraceMessage("GatherExtensionSDKs", "No SDKManifest.xml files could be found at '{0}'. Not adding sdk", pathToSDKManifest); + ErrorUtilities.DebugTraceMessage("GatherExtensionSDKs", $"No SDKManifest.xml files could be found at '{pathToSDKManifest}'. Not adding sdk"); } } else { - ErrorUtilities.DebugTraceMessage("GatherExtensionSDKs", "SDKKey '{0}' was already found, not adding sdk under '{1}'", SDKKey, sdkVersionDirectory.FullName); + ErrorUtilities.DebugTraceMessage("GatherExtensionSDKs", $"SDKKey '{SDKKey}' was already found, not adding sdk under '{sdkVersionDirectory.FullName}'"); } } else { - ErrorUtilities.DebugTraceMessage("GatherExtensionSDKs", "Failed to parse sdk version folder '{0}' under '{1}'", sdkVersionDirectory.Name, sdkVersionDirectory.FullName); + ErrorUtilities.DebugTraceMessage("GatherExtensionSDKs", $"Failed to parse sdk version folder '{sdkVersionDirectory.Name}' under '{sdkVersionDirectory.FullName}'"); } } } @@ -2583,7 +2583,7 @@ internal static void GatherSDKListFromDirectory(List diskroots, Dictiona DirectoryInfo rootInfo = new DirectoryInfo(diskRoot); if (!rootInfo.Exists) { - ErrorUtilities.DebugTraceMessage("GatherSDKListFromDirectory", "DiskRoot '{0}'does not exist, skipping it", diskRoot); + ErrorUtilities.DebugTraceMessage("GatherSDKListFromDirectory", $"DiskRoot '{diskRoot}' does not exist, skipping it"); continue; } @@ -2594,18 +2594,18 @@ internal static void GatherSDKListFromDirectory(List diskroots, Dictiona if (!rootPathWithIdentifier.Exists) { - ErrorUtilities.DebugTraceMessage("GatherSDKListFromDirectory", "Disk root with Identifier: '{0}' does not exist. ", rootPathWithIdentifier); + ErrorUtilities.DebugTraceMessage("GatherSDKListFromDirectory", $"Disk root with Identifier: '{rootPathWithIdentifier}' does not exist. "); continue; } - ErrorUtilities.DebugTraceMessage("GatherSDKListFromDirectory", "Disk root with Identifier: '{0}' does exist. Enumerating version folders under it. ", rootPathWithIdentifier); + ErrorUtilities.DebugTraceMessage("GatherSDKListFromDirectory", $"Disk root with Identifier: '{rootPathWithIdentifier}' does exist. Enumerating version folders under it. "); // Get a list of subdirectories under the root path and identifier, Ie. c:\Program files\Microsoft SDKs\Windows we should see things like, V8.0, 8.0, 9.0 ect. // Only grab the folders that have a version number (they can start with a v or not). SortedDictionary> versionsInRoot = VersionUtilities.GatherVersionStrings(null, rootPathWithIdentifier.GetDirectories().Select(directory => directory.Name)); - ErrorUtilities.DebugTraceMessage("GatherSDKListFromDirectory", "Found '{0}' version folders under the identifier path '{1}'. ", versionsInRoot.Count, rootPathWithIdentifier); + ErrorUtilities.DebugTraceMessage("GatherSDKListFromDirectory", $"Found '{versionsInRoot.Count}' version folders under the identifier path '{rootPathWithIdentifier}'."); // Go through each of the targetplatform versions under the targetplatform identifier. foreach (KeyValuePair> directoryUnderRoot in versionsInRoot) @@ -2658,7 +2658,7 @@ internal static void GatherSDKListFromDirectory(List diskroots, Dictiona } else { - ErrorUtilities.DebugTraceMessage("GatherSDKListFromDirectory", "Could not find ExtensionsSDK folder '{0}'. ", sdkFolderPath); + ErrorUtilities.DebugTraceMessage("GatherSDKListFromDirectory", $"Could not find ExtensionsSDK folder '{sdkFolderPath}'. "); } } } @@ -2681,7 +2681,7 @@ internal static void GatherSDKsFromRegistryImpl(Dictionary sdkNames = getRegistrySubKeyNames(baseKey, extensionSDKsKey); if (sdkNames == null) { - ErrorUtilities.DebugTraceMessage("GatherSDKsFromRegistryImpl", "Could not find subkeys of '{0}'", extensionSDKsKey); + ErrorUtilities.DebugTraceMessage("GatherSDKsFromRegistryImpl", $"Could not find subkeys of '{extensionSDKsKey}'"); continue; } - ErrorUtilities.DebugTraceMessage("GatherSDKsFromRegistryImpl", "Found subkeys of '{0}'", extensionSDKsKey); + ErrorUtilities.DebugTraceMessage("GatherSDKsFromRegistryImpl", $"Found subkeys of '{extensionSDKsKey}'"); // For each SDK folder under ExtensionSDKs foreach (string sdkName in sdkNames) @@ -2791,14 +2791,14 @@ internal static void GatherSDKsFromRegistryImpl(Dictionary sdkVersions = getRegistrySubKeyNames(baseKey, sdkNameKey); - ErrorUtilities.DebugTraceMessage("GatherSDKsFromRegistryImpl", "Getting subkeys of '{0}'", sdkNameKey); + ErrorUtilities.DebugTraceMessage("GatherSDKsFromRegistryImpl", $"Getting subkeys of '{sdkNameKey}'"); if (sdkVersions == null) { - ErrorUtilities.DebugTraceMessage("GatherSDKsFromRegistryImpl", "Could not find subkeys of '{0}'", sdkNameKey); + ErrorUtilities.DebugTraceMessage("GatherSDKsFromRegistryImpl", $"Could not find subkeys of '{sdkNameKey}'"); continue; } - ErrorUtilities.DebugTraceMessage("GatherSDKsFromRegistryImpl", "Found subkeys of '{0}'", sdkNameKey); + ErrorUtilities.DebugTraceMessage("GatherSDKsFromRegistryImpl", $"Found subkeys of '{sdkNameKey}'"); // For each version registry entry under the SDK Name registry key foreach (string sdkVersion in sdkVersions) @@ -2808,14 +2808,14 @@ internal static void GatherSDKsFromRegistryImpl(Dictionary diskRoots, s if (diskRoots != null && !string.IsNullOrEmpty(directoryRoots)) { string[] splitRoots = directoryRoots.Split(s_diskRootSplitChars, StringSplitOptions.RemoveEmptyEntries); - ErrorUtilities.DebugTraceMessage("ExtractSdkDiskRootsFromEnvironment", "DiskRoots from Registry '{0}'", string.Join(";", splitRoots)); + ErrorUtilities.DebugTraceMessage("ExtractSdkDiskRootsFromEnvironment", $"DiskRoots from Registry '{string.Join(";", splitRoots)}'"); diskRoots.AddRange(splitRoots); } @@ -2951,7 +2951,7 @@ private static List GetTargetPlatformMonikerDiskRoots(string[] diskRoots { if (diskRoots?.Length > 0) { - ErrorUtilities.DebugTraceMessage("GetTargetPlatformMonikerDiskRoots", "Passed in DiskRoots '{0}'", string.Join(";", diskRoots)); + ErrorUtilities.DebugTraceMessage("GetTargetPlatformMonikerDiskRoots", $"Passed in DiskRoots '{string.Join(";", diskRoots)}'"); sdkDiskRoots.AddRange(diskRoots); } else @@ -2961,7 +2961,7 @@ private static List GetTargetPlatformMonikerDiskRoots(string[] diskRoots } } - ErrorUtilities.DebugTraceMessage("GetTargetPlatformMonikerDiskRoots", "Diskroots being used '{0}'", string.Join(";", sdkDiskRoots.ToArray())); + ErrorUtilities.DebugTraceMessage("GetTargetPlatformMonikerDiskRoots", $"Diskroots being used '{string.Join(";", sdkDiskRoots.ToArray())}'"); return sdkDiskRoots; } @@ -2976,11 +2976,11 @@ private static List GetExtensionSdkDiskRoots(string[] diskRoots) ExtractSdkDiskRootsFromEnvironment(sdkDiskRoots, sdkDirectoryRootsFromEnvironment); if (sdkDiskRoots.Count == 0 && diskRoots?.Length > 0) { - ErrorUtilities.DebugTraceMessage("GetMultiPlatformSdkDiskRoots", "Passed in DiskRoots '{0}'", string.Join(";", diskRoots)); + ErrorUtilities.DebugTraceMessage("GetMultiPlatformSdkDiskRoots", $"Passed in DiskRoots '{string.Join(";", diskRoots)}'"); sdkDiskRoots.AddRange(diskRoots); } - ErrorUtilities.DebugTraceMessage("GetMultiPlatformSdkDiskRoots", "Diskroots being used '{0}'", string.Join(";", sdkDiskRoots.ToArray())); + ErrorUtilities.DebugTraceMessage("GetMultiPlatformSdkDiskRoots", $"Diskroots being used '{string.Join(";", sdkDiskRoots.ToArray())}'"); return sdkDiskRoots; } @@ -2990,7 +2990,7 @@ private static List GetExtensionSdkDiskRoots(string[] diskRoots) /// private static string GetTargetPlatformMonikerRegistryRoots(string registryRootLocation) { - ErrorUtilities.DebugTraceMessage("GetTargetPlatformMonikerRegistryRoots", "RegistryRoot passed in '{0}'", registryRootLocation ?? string.Empty); + ErrorUtilities.DebugTraceMessage("GetTargetPlatformMonikerRegistryRoots", $"RegistryRoot passed in '{registryRootLocation ?? string.Empty}'"); string disableRegistryForSDKLookup = Environment.GetEnvironmentVariable("MSBUILDDISABLEREGISTRYFORSDKLOOKUP"); // If we are not disabling the registry for platform sdk lookups then lets look in the default location. @@ -3006,7 +3006,7 @@ private static string GetTargetPlatformMonikerRegistryRoots(string registryRootL registryRoot = @"SOFTWARE\MICROSOFT\Microsoft SDKs\"; } - ErrorUtilities.DebugTraceMessage("GetTargetPlatformMonikerRegistryRoots", "RegistryRoot to be looked under '{0}'", registryRoot); + ErrorUtilities.DebugTraceMessage("GetTargetPlatformMonikerRegistryRoots", $"RegistryRoot to be looked under '{registryRoot}'"); } else { @@ -3031,13 +3031,13 @@ private static void GatherPlatformsForSdk(TargetPlatformSDK sdk) if (platformsRootInfo.Exists) { DirectoryInfo[] platformIdentifiers = platformsRootInfo.GetDirectories(); - ErrorUtilities.DebugTraceMessage("GatherPlatformsForSdk", "Found '{0}' platform identifier directories under '{1}'", platformIdentifiers.Length, platformsRoot); + ErrorUtilities.DebugTraceMessage("GatherPlatformsForSdk", $"Found '{platformIdentifiers.Length}' platform identifier directories under '{platformsRoot}'"); // Iterate through all identifiers foreach (DirectoryInfo platformIdentifier in platformIdentifiers) { DirectoryInfo[] platformVersions = platformIdentifier.GetDirectories(); - ErrorUtilities.DebugTraceMessage("GatherPlatformsForSdk", "Found '{0}' platform version directories under '{1}'", platformVersions.Length, platformIdentifier.FullName); + ErrorUtilities.DebugTraceMessage("GatherPlatformsForSdk", $"Found '{platformVersions.Length}' platform version directories under '{platformIdentifier.FullName}'"); // and all versions under each of those identifiers foreach (DirectoryInfo platformVersion in platformVersions) @@ -3051,7 +3051,7 @@ private static void GatherPlatformsForSdk(TargetPlatformSDK sdk) // make sure we haven't already seen this one somehow if (!sdk.Platforms.ContainsKey(sdkKey)) { - ErrorUtilities.DebugTraceMessage("GatherPlatformsForSdk", "SDKKey '{0}' was not already found.", sdkKey); + ErrorUtilities.DebugTraceMessage("GatherPlatformsForSdk", $"SDKKey '{sdkKey}' was not already found."); string pathToPlatformManifest = Path.Combine(platformVersion.FullName, "Platform.xml"); if (FileUtilities.FileExistsNoThrow(pathToPlatformManifest)) @@ -3060,17 +3060,17 @@ private static void GatherPlatformsForSdk(TargetPlatformSDK sdk) } else { - ErrorUtilities.DebugTraceMessage("GatherPlatformsForSdk", "No Platform.xml could be found at '{0}'. Not adding this platform", pathToPlatformManifest); + ErrorUtilities.DebugTraceMessage("GatherPlatformsForSdk", $"No Platform.xml could be found at '{pathToPlatformManifest}'. Not adding this platform"); } } else { - ErrorUtilities.DebugTraceMessage("GatherPlatformsForSdk", "SDKKey '{0}' was already found, not adding platform under '{1}'", sdkKey, platformVersion.FullName); + ErrorUtilities.DebugTraceMessage("GatherPlatformsForSdk", $"SDKKey '{sdkKey}' was already found, not adding platform under '{platformVersion.FullName}'"); } } else { - ErrorUtilities.DebugTraceMessage("GatherPlatformsForSdk", "Failed to parse platform version folder '{0}' under '{1}'", platformVersion.Name, platformVersion.FullName); + ErrorUtilities.DebugTraceMessage("GatherPlatformsForSdk", $"Failed to parse platform version folder '{platformVersion.Name}' under '{platformVersion.FullName}'"); } } } @@ -3078,7 +3078,7 @@ private static void GatherPlatformsForSdk(TargetPlatformSDK sdk) } catch (Exception e) when (ExceptionHandling.IsIoRelatedException(e)) { - ErrorUtilities.DebugTraceMessage("GatherPlatformsForSdk", "Encountered exception trying to gather platform-specific data: {0}", e.Message); + ErrorUtilities.DebugTraceMessage("GatherPlatformsForSdk", $"Encountered exception trying to gather platform-specific data: {e.Message}"); } } diff --git a/src/Utilities/TrackedDependencies/FileTracker.cs b/src/Utilities/TrackedDependencies/FileTracker.cs index c3ac96f1f15..1adea1aec4d 100644 --- a/src/Utilities/TrackedDependencies/FileTracker.cs +++ b/src/Utilities/TrackedDependencies/FileTracker.cs @@ -560,10 +560,9 @@ private static string GetPath(string filename, DotNetFrameworkArchitecture bitne // Make sure that if someone starts passing the wrong thing to this method we don't silently // eat it and do something possibly unexpected. ErrorUtilities.VerifyThrow( - s_TrackerFilename.Equals(filename, StringComparison.OrdinalIgnoreCase) || - s_FileTrackerFilename.Equals(filename, StringComparison.OrdinalIgnoreCase), - "This method should only be passed s_TrackerFilename or s_FileTrackerFilename, but was passed {0} instead!", - filename); + s_TrackerFilename.Equals(filename, StringComparison.OrdinalIgnoreCase) || + s_FileTrackerFilename.Equals(filename, StringComparison.OrdinalIgnoreCase), + $"This method should only be passed s_TrackerFilename or s_FileTrackerFilename, but was passed {filename} instead!"); // Look for FileTracker.dll/Tracker.exe in the MSBuild tools directory. They may exist elsewhere on disk, // but other copies aren't guaranteed to be compatible with the latest.