Fill holes in BuildEventContext construction and evaluation ID propagation

Directory.Build.props

@@ -92,4 +92,9 @@ ates https://learn.microsoft.com/en-gb/dotnet/fundamentals/syslib-diagnostics/sy
     <EnableNETAnalyzers>true</EnableNETAnalyzers>
   </PropertyGroup>
 
+  <!-- Wire up BannedApiAnalyzers configuration -->
+  <ItemGroup>
+    <AdditionalFiles Include="$(MSBuildThisFileDirectory)eng\BannedSymbols.txt" />
+  </ItemGroup>
+
 </Project>

eng/BannedSymbols.txt

@@ -0,0 +1,7 @@
+// Banned BuildEventContext constructors - use CreateInitial/CreateForSubmission/CreateForNode + WithXxx() builder methods instead.
+// These constructors make it easy to accidentally drop evaluation IDs and other critical context data.
+// See BuildEventContext.cs for the recommended builder pattern.
+M:Microsoft.Build.Framework.BuildEventContext.#ctor(System.Int32,System.Int32,System.Int32,System.Int32);Use BuildEventContext.CreateInitial/CreateForNode + WithXxx() builder methods instead
+M:Microsoft.Build.Framework.BuildEventContext.#ctor(System.Int32,System.Int32,System.Int32,System.Int32,System.Int32);Use BuildEventContext.CreateInitial/CreateForNode + WithXxx() builder methods instead
+M:Microsoft.Build.Framework.BuildEventContext.#ctor(System.Int32,System.Int32,System.Int32,System.Int32,System.Int32,System.Int32);Use BuildEventContext.CreateInitial + WithXxx() builder methods instead
+M:Microsoft.Build.Framework.BuildEventContext.#ctor(System.Int32,System.Int32,System.Int32,System.Int32,System.Int32,System.Int32,System.Int32);Use BuildEventContext.CreateInitial + WithXxx() builder methods instead

src/Build.UnitTests/BackEnd/AssemblyTaskFactory_Tests.cs

@@ -796,7 +796,7 @@ private void SetupTaskFactory(TaskHostParameters factoryParameters, bool explici
                 factoryParameters = factoryParameters.WithTaskHostFactoryExplicitlyRequested(true);
             }
 
-            _loadedType = _taskFactory.InitializeFactory(_loadInfo, "TaskToTestFactories", new Dictionary<string, TaskPropertyInfo>(), string.Empty, factoryParameters, explicitlyLaunchTaskHost, new TestLoggingContext(null!, new BuildEventContext(1, 2, 3, 4)), ElementLocation.Create("NONE"), String.Empty);
+            _loadedType = _taskFactory.InitializeFactory(_loadInfo, "TaskToTestFactories", new Dictionary<string, TaskPropertyInfo>(), string.Empty, factoryParameters, explicitlyLaunchTaskHost, new TestLoggingContext(null!, BuildEventContext.CreateInitial(1, 2).WithEvaluationId(3).WithProjectInstanceId(4)), ElementLocation.Create("NONE"), String.Empty);
             Assert.True(_loadedType.Assembly.Equals(_loadInfo)); // "Expected the AssemblyLoadInfo to be equal"
         }
 

src/Build.UnitTests/BackEnd/BatchingEngine_Tests.cs

@@ -57,7 +57,7 @@ public void GetBuckets()
                 parameters,
                 CreateLookup(itemsByType, properties),
                 MockElementLocation.Instance,
-                new TestLoggingContext(null!, new BuildEventContext(1, 2, 3, 4)));
+                new TestLoggingContext(null!, BuildEventContext.CreateInitial(1, 2).WithEvaluationId(3).WithProjectInstanceId(4)));
 
             Assert.Equal(5, buckets.Count);
 
@@ -74,7 +74,7 @@ public void GetBuckets()
                     Directory.GetCurrentDirectory(),
                     MockElementLocation.Instance,
                     FileSystems.Default,
-                    new TestLoggingContext(null!, new BuildEventContext(1, 2, 3, 4))));
+                    new TestLoggingContext(null!, BuildEventContext.CreateInitial(1, 2).WithEvaluationId(3).WithProjectInstanceId(4))));
                 Assert.Equal("a.doc;b.doc;c.doc;d.doc;e.doc", bucket.Expander.ExpandIntoStringAndUnescape("@(doc)", ExpanderOptions.ExpandItems, MockElementLocation.Instance));
                 Assert.Equal("unittests.foo", bucket.Expander.ExpandIntoStringAndUnescape("$(bogus)$(UNITTESTS)", ExpanderOptions.ExpandPropertiesAndMetadata, MockElementLocation.Instance));
             }
@@ -146,7 +146,7 @@ public void ValidUnqualifiedMetadataReference()
                 parameters,
                 CreateLookup(itemsByType, properties),
                 null,
-                new TestLoggingContext(null!, new BuildEventContext(1, 2, 3, 4)));
+                new TestLoggingContext(null!, BuildEventContext.CreateInitial(1, 2).WithEvaluationId(3).WithProjectInstanceId(4)));
             Assert.Equal(2, buckets.Count);
         }
 
@@ -184,7 +184,7 @@ public void InvalidUnqualifiedMetadataReference()
                     parameters,
                     CreateLookup(itemsByType, properties),
                     MockElementLocation.Instance,
-                    new TestLoggingContext(null!, new BuildEventContext(1, 2, 3, 4)));
+                    new TestLoggingContext(null!, BuildEventContext.CreateInitial(1, 2).WithEvaluationId(3).WithProjectInstanceId(4)));
             });
         }
         /// <summary>
@@ -209,7 +209,7 @@ public void NoItemsConsumed()
                     parameters,
                     CreateLookup(itemsByType, properties),
                     MockElementLocation.Instance,
-                    new TestLoggingContext(null!, new BuildEventContext(1, 2, 3, 4)));
+                    new TestLoggingContext(null!, BuildEventContext.CreateInitial(1, 2).WithEvaluationId(3).WithProjectInstanceId(4)));
             });
         }
         /// <summary>
@@ -239,7 +239,7 @@ public void Regress_Mutation_DuplicateBatchingBucketsAreFoldedTogether()
                 parameters,
                 CreateLookup(itemsByType, properties),
                 null,
-                new TestLoggingContext(null!, new BuildEventContext(1, 2, 3, 4)));
+                new TestLoggingContext(null!, BuildEventContext.CreateInitial(1, 2).WithEvaluationId(3).WithProjectInstanceId(4)));
 
             // If duplicate buckets have been folded correctly, then there will be exactly one bucket here
             // containing both a.foo and b.foo.

src/Build.UnitTests/BackEnd/BuildRequestEngine_Tests.cs

@@ -335,7 +335,7 @@ public void TestEngineShutdownWhileActive()
             BuildRequest request = CreateNewBuildRequest(1, targets);
 
             VerifyEngineStatus(BuildRequestEngineStatus.Uninitialized, true);
-            _engine.InitializeForBuild(new NodeLoggingContext(_host.LoggingService, 0, false));
+            _engine.InitializeForBuild(CreateForTesting(_host));
             // We neeed to get the status changed AutoResetEvent returned to the non-signaled state correctly after each status change for verifying the engine status via waiting for a signal next time.
             // Make sure it returns back to the non-signaled state.
             VerifyEngineStatus(BuildRequestEngineStatus.Idle);
@@ -366,7 +366,7 @@ public void TestSimpleBuildScenario()
             BuildRequest request = CreateNewBuildRequest(1, targets);
 
             VerifyEngineStatus(BuildRequestEngineStatus.Uninitialized, true);
-            _engine.InitializeForBuild(new NodeLoggingContext(_host.LoggingService, 0, false));
+            _engine.InitializeForBuild(CreateForTesting(_host));
             VerifyEngineStatus(BuildRequestEngineStatus.Idle);
 
             _engine.SubmitBuildRequest(request);
@@ -400,7 +400,7 @@ public void TestBuildWithChildren()
 
             // Kick it off
             VerifyEngineStatus(BuildRequestEngineStatus.Uninitialized, true);
-            _engine.InitializeForBuild(new NodeLoggingContext(_host.LoggingService, 0, false));
+            _engine.InitializeForBuild(CreateForTesting(_host));
             VerifyEngineStatus(BuildRequestEngineStatus.Idle);
 
             _engine.SubmitBuildRequest(request);
@@ -455,7 +455,7 @@ public void TestBuildWithNewConfiguration()
 
             // Kick it off
             VerifyEngineStatus(BuildRequestEngineStatus.Uninitialized, true);
-            _engine.InitializeForBuild(new NodeLoggingContext(_host.LoggingService, 0, false));
+            _engine.InitializeForBuild(CreateForTesting(_host));
             VerifyEngineStatus(BuildRequestEngineStatus.Idle);
 
             _engine.SubmitBuildRequest(request);
@@ -501,6 +501,8 @@ public void TestShutdown()
         {
         }
 
+        private NodeLoggingContext CreateForTesting(MockHost host) => new NodeLoggingContext(host.LoggingService, BuildEventContext.Invalid.WithNodeId(0), 0, false);
+
         private BuildRequest CreateNewBuildRequest(int configurationId, string[] targets)
         {
             BuildRequest request = new BuildRequest(1 /* submission id */, _nodeRequestId++, configurationId, targets, null, BuildEventContext.Invalid, null);

src/Build.UnitTests/BackEnd/BuildResult_Tests.cs

@@ -310,7 +310,7 @@ public void TestEnumerator()
         [Fact]
         public void TestTranslation()
         {
-            BuildRequest request = new BuildRequest(1, 1, 2, new string[] { "alpha", "omega" }, null, new BuildEventContext(1, 1, 2, 3, 4, 5), null);
+            BuildRequest request = new BuildRequest(1, 1, 2, new string[] { "alpha", "omega" }, null, BuildEventContext.CreateInitial(1, 1).WithEvaluationId(2).WithProjectInstanceId(3).WithProjectContextId(4).WithTaskId(5), null);
             BuildResult result = new BuildResult(request, new BuildAbortedException());
 
             TaskItem fooTaskItem = new TaskItem("foo", "asdf.proj");
@@ -347,7 +347,7 @@ public void TestTranslation()
         [Fact]
         public void TestTranslationPreservesEvaluationId()
         {
-            BuildRequest request = new(1, 1, 2, ["Build"], null, new BuildEventContext(1, 1, 2, 3, 4, 5), null);
+            BuildRequest request = new(1, 1, 2, ["Build"], null, BuildEventContext.CreateInitial(1, 1).WithProjectInstanceId(2).WithProjectContextId(3).WithTargetId(4).WithTaskId(5), null);
             BuildResult result = new(request, new BuildAbortedException())
             {
                 EvaluationId = 42,

src/Build.UnitTests/BackEnd/EventSourceSink_Tests.cs

@@ -756,7 +756,7 @@ internal sealed class RaiseEventHelper
             /// </summary>
             private static BuildWarningEventArgs s_buildWarning = new BuildWarningEventArgs("SubCategoryForSchemaValidationErrors", "MSB4000", "file", 1, 2, 3, 4, "message", "help", "sender")
             {
-                BuildEventContext = new BuildEventContext(1, 2, 3, 4, 5, 6)
+                BuildEventContext = BuildEventContext.CreateInitial(1, 2).WithEvaluationId(3).WithProjectInstanceId(4).WithProjectContextId(5).WithTaskId(6)
             };
 
             /// <summary>

src/Build.UnitTests/BackEnd/IntrinsicTask_Tests.cs

@@ -3958,7 +3958,7 @@ private static IntrinsicTask CreateIntrinsicTask(string content)
             ProjectInstance projectInstance = project.CreateProjectInstance();
             ProjectTargetInstanceChild targetChild = projectInstance.Targets["t"].Children.First();
 
-            NodeLoggingContext nodeContext = new NodeLoggingContext(new MockLoggingService(), 1, false);
+            NodeLoggingContext nodeContext = new NodeLoggingContext(new MockLoggingService(), BuildEventContext.Invalid.WithNodeId(1), 1, false);
             BuildRequestEntry entry = new BuildRequestEntry(new BuildRequest(1 /* submissionId */, 0, 1, new string[] { "t" }, null, BuildEventContext.Invalid, null), new BuildRequestConfiguration(1, new BuildRequestData("projectFile", new Dictionary<string, string>(), "3.5", Array.Empty<string>(), null), "2.0"), CreateStubTaskEnvironment());
             entry.RequestConfiguration.Project = projectInstance;
             IntrinsicTask task = IntrinsicTask.InstantiateTask(
@@ -3993,7 +3993,7 @@ internal static void AssertItemEvaluationFromTarget(string projectContents, stri
                     var projectInstance = project.CreateProjectInstance();
                     var targetChild = projectInstance.Targets["t"].Children.First();
 
-                    var nodeContext = new NodeLoggingContext(new MockLoggingService(), 1, false);
+                    var nodeContext = new NodeLoggingContext(new MockLoggingService(), BuildEventContext.Invalid.WithNodeId(1), 1, false);
                     var entry = new BuildRequestEntry(new BuildRequest(1 /* submissionId */, 0, 1, new string[] { targetName }, null, BuildEventContext.Invalid, null), new BuildRequestConfiguration(1, new BuildRequestData("projectFile", new Dictionary<string, string>(), "3.5", Array.Empty<string>(), null), "2.0"), CreateStubTaskEnvironment());
                     entry.RequestConfiguration.Project = projectInstance;
                     var task = IntrinsicTask.InstantiateTask(

src/Build.UnitTests/BackEnd/LoggingContext_Tests.cs

@@ -22,13 +22,15 @@ public LoggingContext_Tests(ITestOutputHelper outputHelper)
             _output = outputHelper;
         }
 
+        private NodeLoggingContext CreateNodeLoggingContext(int nodeId, bool isInProc) => new NodeLoggingContext(new MockLoggingService(_output.WriteLine), BuildEventContext.Invalid.WithNodeId(nodeId), nodeId, isInProc);
+
         /// <summary>
         /// A few simple tests for NodeLoggingContexts.
         /// </summary>
         [Fact]
         public void CreateValidNodeLoggingContexts()
         {
-            NodeLoggingContext context = new NodeLoggingContext(new MockLoggingService(_output.WriteLine), 1, true);
+            NodeLoggingContext context = CreateNodeLoggingContext(1, true);
             context.IsInProcNode.ShouldBeTrue();
             context.IsValid.ShouldBeTrue();
 
@@ -37,7 +39,7 @@ public void CreateValidNodeLoggingContexts()
 
             context.BuildEventContext.NodeId.ShouldBe(1);
 
-            NodeLoggingContext context2 = new NodeLoggingContext(new MockLoggingService(_output.WriteLine), 2, false);
+            NodeLoggingContext context2 = CreateNodeLoggingContext(2, false);
             context2.IsInProcNode.ShouldBeFalse();
             context2.IsValid.ShouldBeTrue();
 
@@ -57,14 +59,14 @@ public void InvalidNodeIdOnNodeLoggingContext()
         {
             Assert.Throws<InternalErrorException>(() =>
             {
-                _ = new NodeLoggingContext(new MockLoggingService(), -2, true);
+                _ = CreateNodeLoggingContext(-2, true);
             });
         }
 
         [Fact]
         public void HasLoggedErrors()
         {
-            NodeLoggingContext context = new NodeLoggingContext(new MockLoggingService(_output.WriteLine), 1, true);
+            NodeLoggingContext context = CreateNodeLoggingContext(1, true);
             context.HasLoggedErrors.ShouldBeFalse();
 
             context.LogCommentFromText(Framework.MessageImportance.High, "Test message");

src/Build.UnitTests/BackEnd/LoggingService_Tests.cs

@@ -788,13 +788,15 @@ public void TreatWarningsAsErrorWhenAllSpecified(int loggerMode, int nodeId)
         [Fact]
         public void VerifyWarningsPromotedToErrorsAreCounted()
         {
-            ILoggingService ls = LoggingService.CreateLoggingService(LoggerMode.Synchronous, 1);
+            var submissionId = 1;
+            var nodeId = 1;
+            ILoggingService ls = LoggingService.CreateLoggingService(LoggerMode.Synchronous, nodeId);
             ls.WarningsAsErrors = new HashSet<string>();
             ls.WarningsAsErrors.Add("FOR123");
             BuildWarningEventArgs warningArgs = new("abc", "FOR123", "", 0, 0, 0, 0, "warning message", "keyword", "sender");
-            warningArgs.BuildEventContext = new BuildEventContext(1, 2, BuildEventContext.InvalidProjectContextId, BuildEventContext.InvalidProjectContextId, 5, 6);
+            warningArgs.BuildEventContext = BuildEventContext.Invalid.WithSubmissionId(submissionId).WithNodeId(nodeId);
             ls.LogBuildEvent(warningArgs);
-            ls.HasBuildSubmissionLoggedErrors(1).ShouldBeTrue();
+            ls.HasBuildSubmissionLoggedErrors(submissionId).ShouldBeTrue();
         }
 
         /// <summary>
@@ -952,13 +954,11 @@ private MockLogger GetLoggedEventsWithWarningsAsErrorsOrMessages(
         {
             IBuildComponentHost host = new MockHost();
 
-            BuildEventContext buildEventContext = new BuildEventContext(
-                submissionId: 0,
-                nodeId: 1,
-                projectInstanceId: 2,
-                projectContextId: -1,
-                targetId: -1,
-                taskId: -1);
+            BuildEventContext buildEventContext = BuildEventContext.CreateInitial(0, 1)
+                .WithProjectInstanceId(2)
+                .WithProjectContextId(-1)
+                .WithTargetId(-1)
+                .WithTaskId(-1);
 
             BuildRequestData buildRequestData = new BuildRequestData("projectFile", new Dictionary<string, string>(), "Current", new[] { "Build" }, null);
 
@@ -974,7 +974,9 @@ private MockLogger GetLoggedEventsWithWarningsAsErrorsOrMessages(
 
             loggingService.RegisterLogger(logger);
 
-            BuildEventContext projectStarted = loggingService.LogProjectStarted(buildEventContext, 0, buildEventContext.ProjectInstanceId, BuildEventContext.Invalid, "projectFile", "Build", Enumerable.Empty<DictionaryEntry>(), Enumerable.Empty<DictionaryEntry>());
+            var projectStartedArgs = loggingService.CreateProjectStartedForLocalProject(BuildEventContext.Invalid, buildEventContext.ProjectInstanceId, "projectFile", "Build", null, Enumerable.Empty<DictionaryEntry>(), Enumerable.Empty<DictionaryEntry>(), null);
+            loggingService.LogProjectStarted(projectStartedArgs);
+            BuildEventContext projectStarted = projectStartedArgs.BuildEventContext;
 
             if (warningsAsErrorsForProject != null)
             {