Proto/buildcheck warns promotability (#10668)

* Initial version of evaluation time warnings deferring

* Transfer the warn as error/msg info via event args

* Reflect PR review suggestions

* Accomodate buildcheck build errors happening in the main node

* Tailor the build error case for buildcheck

src/Build.UnitTests/BackEnd/MockLoggingService.cs

@@ -550,6 +550,32 @@ public BuildEventContext LogProjectStarted(
             return new BuildEventContext(0, 0, 0, 0);
         }
 
+        public void LogProjectStarted(ProjectStartedEventArgs args)
+        { }
+
+        public ProjectStartedEventArgs CreateProjectStarted(
+            BuildEventContext nodeBuildEventContext,
+            int submissionId,
+            int configurationId,
+            BuildEventContext parentBuildEventContext,
+            string projectFile,
+            string targetNames,
+            IEnumerable<DictionaryEntry> properties,
+            IEnumerable<DictionaryEntry> items,
+            int evaluationId = BuildEventContext.InvalidEvaluationId,
+            int projectContextId = BuildEventContext.InvalidProjectContextId)
+        {
+            return new ProjectStartedEventArgs(
+                configurationId,
+                message: null,
+                helpKeyword: null,
+                projectFile,
+                targetNames,
+                properties,
+                items,
+                parentBuildEventContext);
+        }
+
         /// <summary>
         /// Logs a project finished event
         /// </summary>

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

@@ -555,6 +555,20 @@ BuildEventContext LogProjectStarted(
             int evaluationId = BuildEventContext.InvalidEvaluationId,
             int projectContextId = BuildEventContext.InvalidProjectContextId);
 
+        void LogProjectStarted(ProjectStartedEventArgs args);
+
+        ProjectStartedEventArgs CreateProjectStarted(
+            BuildEventContext nodeBuildEventContext,
+            int submissionId,
+            int configurationId,
+            BuildEventContext parentBuildEventContext,
+            string projectFile,
+            string targetNames,
+            IEnumerable<DictionaryEntry> properties,
+            IEnumerable<DictionaryEntry> items,
+            int evaluationId = BuildEventContext.InvalidEvaluationId,
+            int projectContextId = BuildEventContext.InvalidProjectContextId);
+
         /// <summary>
         /// Log that the project has finished
         /// </summary>

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

@@ -211,6 +211,11 @@ internal partial class LoggingService : ILoggingService, INodePacketHandler
         /// </summary>
         private readonly ISet<int> _buildSubmissionIdsThatHaveLoggedErrors = new HashSet<int>();
 
+        /// <summary>
+        /// A list of build submission IDs that have logged errors through buildcheck.  If an error is logged outside of a submission, the submission ID is <see cref="BuildEventContext.InvalidSubmissionId"/>.
+        /// </summary>
+        private readonly ISet<int> _buildSubmissionIdsThatHaveLoggedBuildcheckErrors = new HashSet<int>();
+
         /// <summary>
         /// A list of warnings to treat as errors for an associated <see cref="BuildEventContext"/>.  If an empty set, all warnings are treated as errors.
         /// </summary>
@@ -620,6 +625,11 @@ public bool IncludeEvaluationPropertiesAndItemsInEvaluationFinishedEvent
         /// <returns><code>true</code> if the build submission logged an errors, otherwise <code>false</code>.</returns>
         public bool HasBuildSubmissionLoggedErrors(int submissionId)
         {
+            if (_buildSubmissionIdsThatHaveLoggedBuildcheckErrors.Contains(submissionId))
+            {
+                return true;
+            }
+
             // Warnings as errors are not tracked if the user did not specify to do so
             if (WarningsAsErrors == null && _warningsAsErrorsByProject == null)
             {
@@ -730,6 +740,11 @@ public void AddWarningsAsMessages(BuildEventContext buildEventContext, ISet<stri
         /// <param name="codes">Codes to add</param>
         private void AddWarningsAsMessagesOrErrors(ref IDictionary<WarningsConfigKey, ISet<string>> warningsByProject, BuildEventContext buildEventContext, ISet<string> codes)
         {
+            if (codes == null)
+            {
+                return;
+            }
+
             lock (_lockObject)
             {
                 WarningsConfigKey key = GetWarningsConfigKey(buildEventContext);
@@ -1638,14 +1653,25 @@ private void RouteBuildEvent(object loggingEvent)
 
             if (buildEventArgs is BuildErrorEventArgs errorEvent)
             {
-                // Keep track of build submissions that have logged errors.  If there is no build context, add BuildEventContext.InvalidSubmissionId.
-                _buildSubmissionIdsThatHaveLoggedErrors.Add(errorEvent.BuildEventContext?.SubmissionId ?? BuildEventContext.InvalidSubmissionId);
+                int submissionId = errorEvent.BuildEventContext?.SubmissionId ?? BuildEventContext.InvalidSubmissionId;
+
+                if (buildEventArgs is BuildCheckResultError)
+                {
+                    _buildSubmissionIdsThatHaveLoggedBuildcheckErrors.Add(submissionId);
+                }
+                else
+                {
+                    // Keep track of build submissions that have logged errors.  If there is no build context, add BuildEventContext.InvalidSubmissionId.
+                    _buildSubmissionIdsThatHaveLoggedErrors.Add(submissionId);
+                }
             }
 
-            if (buildEventArgs is BuildCheckResultError checkResultError)
+            // If this is BuildCheck-ed build - add the warnings promotability/demotability to the service
+            if (buildEventArgs is ProjectStartedEventArgs projectStartedEvent && this._componentHost.BuildParameters.IsBuildCheckEnabled)
             {
-                // If the specified BuildCheckResultError was issued, an empty ISet<string> signifies that the specified build check warnings should be treated as errors.
-                AddWarningsAsErrors(checkResultError.BuildEventContext, new HashSet<string>());
+                AddWarningsAsErrors(projectStartedEvent.BuildEventContext, projectStartedEvent.WarningsAsErrors);
+                AddWarningsAsMessages(projectStartedEvent.BuildEventContext, projectStartedEvent.WarningsAsMessages);
+                AddWarningsNotAsErrors(projectStartedEvent.BuildEventContext, projectStartedEvent.WarningsNotAsErrors);
             }
 
             if (buildEventArgs is ProjectFinishedEventArgs projectFinishedEvent && projectFinishedEvent.BuildEventContext != null)

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

@@ -497,6 +497,39 @@ public BuildEventContext LogProjectStarted(
             IEnumerable<DictionaryEntry> items,
             int evaluationId = BuildEventContext.InvalidEvaluationId,
             int projectContextId = BuildEventContext.InvalidProjectContextId)
+        {
+            var args = CreateProjectStarted(nodeBuildEventContext,
+                submissionId,
+                configurationId,
+                parentBuildEventContext,
+                projectFile,
+                targetNames,
+                properties,
+                items,
+                evaluationId,
+                projectContextId);
+
+            this.LogProjectStarted(args);
+
+            return args.BuildEventContext;
+        }
+
+        public void LogProjectStarted(ProjectStartedEventArgs buildEvent)
+        {
+            ProcessLoggingEvent(buildEvent);
+        }
+
+        public ProjectStartedEventArgs CreateProjectStarted(
+            BuildEventContext nodeBuildEventContext,
+            int submissionId,
+            int configurationId,
+            BuildEventContext parentBuildEventContext,
+            string projectFile,
+            string targetNames,
+            IEnumerable<DictionaryEntry> properties,
+            IEnumerable<DictionaryEntry> items,
+            int evaluationId = BuildEventContext.InvalidEvaluationId,
+            int projectContextId = BuildEventContext.InvalidProjectContextId)
         {
             ErrorUtilities.VerifyThrow(nodeBuildEventContext != null, "Need a nodeBuildEventContext");
 
@@ -560,9 +593,7 @@ public BuildEventContext LogProjectStarted(
                     buildRequestConfiguration.ToolsVersion);
             buildEvent.BuildEventContext = projectBuildEventContext;
 
-            ProcessLoggingEvent(buildEvent);
-
-            return projectBuildEventContext;
+            return buildEvent;
         }
 
         /// <summary>

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

@@ -57,9 +57,16 @@ internal void LogBuildFinished(bool success)
         /// <param name="requestEntry">The build request entry for this project.</param>
         /// <returns>The BuildEventContext to use for this project.</returns>
         internal ProjectLoggingContext LogProjectStarted(BuildRequestEntry requestEntry)
+        {
+            (ProjectStartedEventArgs arg, ProjectLoggingContext ctx) = CreateProjectLoggingContext(requestEntry);
+            LoggingService.LogProjectStarted(arg);
+            return ctx;
+        }
+
+        internal (ProjectStartedEventArgs, ProjectLoggingContext) CreateProjectLoggingContext(BuildRequestEntry requestEntry)
         {
             ErrorUtilities.VerifyThrow(this.IsValid, "Build not started.");
-            return new ProjectLoggingContext(this, requestEntry);
+            return ProjectLoggingContext.CreateLoggingContext(this, requestEntry);
         }
 
         /// <summary>

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

@@ -71,6 +71,45 @@ internal ProjectLoggingContext(
         {
         }
 
+        /// <summary>
+        /// Creates ProjectLoggingContext, without logging ProjectStartedEventArgs as a side effect.
+        /// The ProjectStartedEventArgs is returned as well - so that it can be later logged explicitly
+        /// </summary>
+        public static (ProjectStartedEventArgs, ProjectLoggingContext) CreateLoggingContext(
+            NodeLoggingContext nodeLoggingContext, BuildRequestEntry requestEntry)
+        {
+            ProjectStartedEventArgs args = CreateProjectStarted(
+                nodeLoggingContext,
+                requestEntry.Request.SubmissionId,
+                requestEntry.Request.ConfigurationId,
+                requestEntry.RequestConfiguration.ProjectFullPath,
+                requestEntry.Request.Targets,
+                requestEntry.RequestConfiguration.ToolsVersion,
+                requestEntry.RequestConfiguration.Project.PropertiesToBuildWith,
+                requestEntry.RequestConfiguration.Project.ItemsToBuildWith,
+                requestEntry.Request.ParentBuildEventContext,
+                requestEntry.RequestConfiguration.Project.EvaluationId,
+                requestEntry.Request.ProjectContextId);
+
+            return (args, new ProjectLoggingContext(nodeLoggingContext, args));
+        }
+
+        private ProjectLoggingContext(
+            NodeLoggingContext nodeLoggingContext,
+            ProjectStartedEventArgs projectStarted)
+        : base(nodeLoggingContext, projectStarted.BuildEventContext)
+        {
+            _projectFullPath = projectStarted.ProjectFile;
+
+            // No need to log a redundant message in the common case
+            if (projectStarted.ToolsVersion != "Current")
+            {
+                LoggingService.LogComment(this.BuildEventContext, MessageImportance.Low, "ToolsVersionInEffectForBuild", projectStarted.ToolsVersion);
+            }
+
+            this.IsValid = true;
+        }
+
         /// <summary>
         /// Constructs a project logging contexts.
         /// </summary>
@@ -122,6 +161,37 @@ private static BuildEventContext CreateInitialContext(
             BuildEventContext parentBuildEventContext,
             int evaluationId,
             int projectContextId)
+        {
+            ProjectStartedEventArgs args = CreateProjectStarted(
+                nodeLoggingContext,
+                submissionId,
+                configurationId,
+                projectFullPath,
+                targets,
+                toolsVersion,
+                projectProperties,
+                projectItems,
+                parentBuildEventContext,
+                evaluationId,
+                projectContextId);
+
+            nodeLoggingContext.LoggingService.LogProjectStarted(args);
+
+            return args.BuildEventContext;
+        }
+
+        private static ProjectStartedEventArgs CreateProjectStarted(
+            NodeLoggingContext nodeLoggingContext,
+            int submissionId,
+            int configurationId,
+            string projectFullPath,
+            List<string> targets,
+            string toolsVersion,
+            PropertyDictionary<ProjectPropertyInstance> projectProperties,
+            IItemDictionary<ProjectItemInstance> projectItems,
+            BuildEventContext parentBuildEventContext,
+            int evaluationId,
+            int projectContextId)
         {
             IEnumerable<DictionaryEntry> properties = null;
             IEnumerable<DictionaryEntry> items = null;
@@ -171,7 +241,7 @@ private static BuildEventContext CreateInitialContext(
                 properties = projectPropertiesToSerialize.Select((ProjectPropertyInstance property) => new DictionaryEntry(property.Name, property.EvaluatedValue));
             }
 
-            return loggingService.LogProjectStarted(
+            return loggingService.CreateProjectStarted(
                 nodeLoggingContext.BuildEventContext,
                 submissionId,
                 configurationId,

src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Concurrent;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Globalization;
 using System.IO;
 using System.Linq;
@@ -1105,11 +1106,11 @@ private async Task<BuildResult> BuildProject()
             ErrorUtilities.VerifyThrow(_targetBuilder != null, "Target builder is null");
 
             // We consider this the entrypoint for the project build for purposes of BuildCheck processing 
-            bool isRestoring = _requestEntry.RequestConfiguration.GlobalProperties[MSBuildConstants.MSBuildIsRestoring] is null;
+            bool isRestoring = _requestEntry.RequestConfiguration.GlobalProperties[MSBuildConstants.MSBuildIsRestoring] is not null;
 
             var buildCheckManager = isRestoring
-                ? (_componentHost.GetComponent(BuildComponentType.BuildCheckManagerProvider) as IBuildCheckManagerProvider)!.Instance
-                : null;
+                ? null
+                : (_componentHost.GetComponent(BuildComponentType.BuildCheckManagerProvider) as IBuildCheckManagerProvider)!.Instance;
 
             buildCheckManager?.SetDataSource(BuildCheckDataSource.BuildExecution);
 
@@ -1154,15 +1155,10 @@ private async Task<BuildResult> BuildProject()
                     _requestEntry.Request.BuildEventContext);
             }
 
-            _projectLoggingContext = _nodeLoggingContext.LogProjectStarted(_requestEntry);
-            buildCheckManager?.StartProjectRequest(
-                _projectLoggingContext.BuildEventContext,
-                _requestEntry.RequestConfiguration.ProjectFullPath);
-
+
             try
             {
-                // Now that the project has started, parse a few known properties which indicate warning codes to treat as errors or messages
-                ConfigureWarningsAsErrorsAndMessages();
+                HandleProjectStarted(buildCheckManager);
 
                 // Make sure to extract known immutable folders from properties and register them for fast up-to-date check
                 ConfigureKnownImmutableFolders();
@@ -1273,6 +1269,31 @@ private void SaveOperatingEnvironment()
             }
         }
 
+        private void HandleProjectStarted(IBuildCheckManager buildCheckManager)
+        {
+            (ProjectStartedEventArgs args, ProjectLoggingContext ctx) = _nodeLoggingContext.CreateProjectLoggingContext(_requestEntry);
+
+            _projectLoggingContext = ctx;
+            ConfigureWarningsAsErrorsAndMessages();
+            ILoggingService loggingService = _projectLoggingContext?.LoggingService;
+            BuildEventContext projectBuildEventContext = _projectLoggingContext?.BuildEventContext;
+
+            // We can set the warning as errors and messages only after the project logging context has been created (as it creates the new ProjectContextId)
+            if (buildCheckManager != null && loggingService != null && projectBuildEventContext != null)
+            {
+                args.WarningsAsErrors = loggingService.GetWarningsAsErrors(projectBuildEventContext).ToHashSet(StringComparer.OrdinalIgnoreCase);
+                args.WarningsAsMessages = loggingService.GetWarningsAsMessages(projectBuildEventContext).ToHashSet(StringComparer.OrdinalIgnoreCase);
+                args.WarningsNotAsErrors = loggingService.GetWarningsNotAsErrors(projectBuildEventContext).ToHashSet(StringComparer.OrdinalIgnoreCase);
+            }
+
+            // We can log the event only after the warning as errors and messages have been set and added
+            loggingService?.LogProjectStarted(args);
+
+            buildCheckManager?.StartProjectRequest(
+                new CheckLoggingContext(_nodeLoggingContext.LoggingService, _projectLoggingContext!.BuildEventContext),
+                _requestEntry.RequestConfiguration.ProjectFullPath);
+        }
+
         /// <summary>
         /// Sets the operationg environment to the initial build environment.
         /// </summary>
@@ -1409,7 +1430,7 @@ private void ConfigureKnownImmutableFolders()
             }
         }
 
-        private ISet<string> ParseWarningCodes(string warnings)
+        private static ISet<string> ParseWarningCodes(string warnings)
         {
             if (String.IsNullOrWhiteSpace(warnings))
             {

src/Build/BuildCheck/Infrastructure/BuildCheckBuildEventHandler.cs

@@ -36,7 +36,7 @@ internal BuildCheckBuildEventHandler(
             { typeof(ProjectEvaluationFinishedEventArgs), (BuildEventArgs e) => HandleProjectEvaluationFinishedEvent((ProjectEvaluationFinishedEventArgs)e) },
             { typeof(ProjectEvaluationStartedEventArgs), (BuildEventArgs e) => HandleProjectEvaluationStartedEvent((ProjectEvaluationStartedEventArgs)e) },
             { typeof(EnvironmentVariableReadEventArgs), (BuildEventArgs e) => HandleEnvironmentVariableReadEvent((EnvironmentVariableReadEventArgs)e) },
-            { typeof(ProjectStartedEventArgs), (BuildEventArgs e) => _buildCheckManager.StartProjectRequest(e.BuildEventContext!, ((ProjectStartedEventArgs)e).ProjectFile!) },
+            { typeof(ProjectStartedEventArgs), (BuildEventArgs e) => HandleProjectStartedRequest((ProjectStartedEventArgs)e) },
             { typeof(ProjectFinishedEventArgs), (BuildEventArgs e) => HandleProjectFinishedRequest((ProjectFinishedEventArgs)e) },
             { typeof(BuildCheckTracingEventArgs), (BuildEventArgs e) => HandleBuildCheckTracingEvent((BuildCheckTracingEventArgs)e) },
             { typeof(BuildCheckAcquisitionEventArgs), (BuildEventArgs e) => HandleBuildCheckAcquisitionEvent((BuildCheckAcquisitionEventArgs)e) },
@@ -98,6 +98,11 @@ private void HandleProjectEvaluationStartedEvent(ProjectEvaluationStartedEventAr
         }
     }
 
+    private void HandleProjectStartedRequest(ProjectStartedEventArgs eventArgs)
+        => _buildCheckManager.StartProjectRequest(
+            _checkContextFactory.CreateCheckContext(eventArgs.BuildEventContext!),
+            eventArgs!.ProjectFile!);
+
     private void HandleProjectFinishedRequest(ProjectFinishedEventArgs eventArgs)
         => _buildCheckManager.EndProjectRequest(
                 _checkContextFactory.CreateCheckContext(eventArgs.BuildEventContext!),