Add ParameterName and PropertyName to TaskParameterEventArgs (#10130)

Contributes to #9881 ### Context When `TaskParameterEventArgs` represents an output parameter of a task, it currently does not capture the name of the parameter. This information is generally useful as multiple output parameters can be assigned to the same item, and it's important for the BuildCheck infrastructure to provide a consistent object model to build analyzers. Additionally, output parameters that are assigned to properties are currently logged as textual log messages `Output Property: {0}={1}` which miss the name of the parameter as well. ### Changes Made Added two new properties on the `TaskParameterEventArgs` class, including the requisite serialization support. Updated logging to populate the properties with the name of the task parameter and the name of the build property, respectively, and updated BuildCheck infra to expose it to analyzers subscribed to task parameter events. All task parameters are now logged as the structured `TaskParameterEventArgs` and the kind of parameter (input, output to property, output to item) can be determined by examining the args. ### Testing - Tweaked existing unit test and updated the relevant BuildCheck test. - Verified that when using the standard console logger, the resulting textual log is identical. - Verified that binlog viewer without the corresponding changes displays the forward-compat warning about unknown data. ### Notes - Complementary binlog viewer PR: KirillOsenkov/MSBuildStructuredLog#780 - The change is behind a changewave because it is technically a breaking change for third party loggers. Co-authored-by: Rainer Sigwald <raines@microsoft.com>
dotnet · May 23, 2024 · 147ecad · 147ecad
1 parent 1d47d30
commit 147ecad
Show file tree

Hide file tree

Showing 26 changed files with 209 additions and 24 deletions.
diff --git a/documentation/wiki/ChangeWaves.md b/documentation/wiki/ChangeWaves.md
@@ -27,6 +27,7 @@ A wave of features is set to "rotate out" (i.e. become standard functionality) t
 - [Log TaskParameterEvent for scalar parameters](https://github.com/dotnet/msbuild/pull/9908)
 - [Convert.ToString during a property evaluation uses the InvariantCulture for all types](https://github.com/dotnet/msbuild/pull/9874)
 - [Fix oversharing of build results in ResultsCache](https://github.com/dotnet/msbuild/pull/9987)
+- [Add ParameterName and PropertyName to TaskParameterEventArgs](https://github.com/dotnet/msbuild/pull/10130)
 
 ### 17.10
 - [AppDomain configuration is serialized without using BinFmt](https://github.com/dotnet/msbuild/pull/9320) - feature can be opted out only if [BinaryFormatter](https://learn.microsoft.com/en-us/dotnet/api/system.runtime.serialization.formatters.binary.binaryformatter) is allowed at runtime by editing `MSBuild.runtimeconfig.json`

diff --git a/src/Build.UnitTests/BuildEventArgsSerialization_Tests.cs b/src/Build.UnitTests/BuildEventArgsSerialization_Tests.cs
@@ -650,12 +650,14 @@ public void RoundtripTaskParameterEventArgs()
                 new TaskItemData("ItemSpec1", null),
                 new TaskItemData("ItemSpec2", Enumerable.Range(1,3).ToDictionary(i => i.ToString(), i => i.ToString() + "value"))
             };
-            var args = new TaskParameterEventArgs(TaskParameterMessageKind.TaskOutput, "ItemName", items, true, DateTime.MinValue);
+            var args = new TaskParameterEventArgs(TaskParameterMessageKind.TaskOutput, "ParameterName1", "PropertyName1", "ItemName1", items, true, DateTime.MinValue);
             args.LineNumber = 265;
             args.ColumnNumber = 6;
 
             Roundtrip(args,
                 e => e.Kind.ToString(),
+                e => e.ParameterName,
+                e => e.PropertyName,
                 e => e.ItemType,
                 e => e.LogItemMetadata.ToString(),
                 e => e.LineNumber.ToString(),

diff --git a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/ItemGroupIntrinsicTask.cs b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/ItemGroupIntrinsicTask.cs
@@ -221,6 +221,8 @@ private void ExecuteAdd(ProjectItemGroupTaskItemInstance child, ItemBucket bucke
                 ItemGroupLoggingHelper.LogTaskParameter(
                     LoggingContext,
                     TaskParameterMessageKind.AddItem,
+                    parameterName: null,
+                    propertyName: null,
                     child.ItemType,
                     itemsToAdd,
                     logItemMetadata: true,
@@ -265,6 +267,8 @@ private void ExecuteRemove(ProjectItemGroupTaskItemInstance child, ItemBucket bu
                     ItemGroupLoggingHelper.LogTaskParameter(
                         LoggingContext,
                         TaskParameterMessageKind.RemoveItem,
+                        parameterName: null,
+                        propertyName: null,
                         child.ItemType,
                         itemsToRemove,
                         logItemMetadata: true,

diff --git a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/ItemGroupLoggingHelper.cs b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/ItemGroupLoggingHelper.cs
@@ -10,6 +10,7 @@
 #endif
 using Microsoft.Build.BackEnd.Logging;
 using Microsoft.Build.Collections;
+using Microsoft.Build.Execution;
 using Microsoft.Build.Framework;
 using Microsoft.Build.Shared;
 
@@ -35,6 +36,7 @@ internal static class ItemGroupLoggingHelper
         internal static string ItemGroupIncludeLogMessagePrefix = ResourceUtilities.GetResourceString("ItemGroupIncludeLogMessagePrefix");
         internal static string ItemGroupRemoveLogMessage = ResourceUtilities.GetResourceString("ItemGroupRemoveLogMessage");
         internal static string OutputItemParameterMessagePrefix = ResourceUtilities.GetResourceString("OutputItemParameterMessagePrefix");
+        internal static string OutputPropertyLogMessagePrefix = ResourceUtilities.GetResourceString("OutputPropertyLogMessagePrefix");
         internal static string TaskParameterPrefix = ResourceUtilities.GetResourceString("TaskParameterPrefix");
         internal static string SkipTargetUpToDateInputs = ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword("SkipTargetUpToDateInputs", string.Empty);
         internal static string SkipTargetUpToDateOutputs = ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword("SkipTargetUpToDateOutputs", string.Empty);
@@ -255,6 +257,8 @@ private static void AppendStringFromParameterValue(ReuseableStringBuilder sb, ob
         internal static void LogTaskParameter(
             LoggingContext loggingContext,
             TaskParameterMessageKind messageKind,
+            string parameterName,
+            string propertyName,
             string itemType,
             IList items,
             bool logItemMetadata,
@@ -263,6 +267,8 @@ internal static void LogTaskParameter(
             var args = CreateTaskParameterEventArgs(
                 loggingContext.BuildEventContext,
                 messageKind,
+                parameterName,
+                propertyName,
                 itemType,
                 items,
                 logItemMetadata,
@@ -276,6 +282,8 @@ internal static void LogTaskParameter(
         internal static TaskParameterEventArgs CreateTaskParameterEventArgs(
             BuildEventContext buildEventContext,
             TaskParameterMessageKind messageKind,
+            string parameterName,
+            string propertyName,
             string itemType,
             IList items,
             bool logItemMetadata,
@@ -290,6 +298,8 @@ internal static TaskParameterEventArgs CreateTaskParameterEventArgs(
 
             var args = new TaskParameterEventArgs(
                 messageKind,
+                parameterName,
+                propertyName,
                 itemType,
                 items,
                 logItemMetadata,
@@ -355,26 +365,23 @@ private static void CreateItemsSnapshot(ref IList items)
 #endif
 
         internal static string GetTaskParameterText(TaskParameterEventArgs args)
-            => GetTaskParameterText(args.Kind, args.ItemType, args.Items, args.LogItemMetadata);
-
-        internal static string GetTaskParameterText(TaskParameterMessageKind messageKind, string itemType, IList items, bool logItemMetadata)
         {
-            var resourceText = messageKind switch
+            var resourceText = args.Kind switch
             {
                 TaskParameterMessageKind.AddItem => ItemGroupIncludeLogMessagePrefix,
                 TaskParameterMessageKind.RemoveItem => ItemGroupRemoveLogMessage,
                 TaskParameterMessageKind.TaskInput => TaskParameterPrefix,
-                TaskParameterMessageKind.TaskOutput => OutputItemParameterMessagePrefix,
+                TaskParameterMessageKind.TaskOutput => args.PropertyName is null ? OutputItemParameterMessagePrefix : OutputPropertyLogMessagePrefix,
                 TaskParameterMessageKind.SkippedTargetInputs => SkipTargetUpToDateInputs,
                 TaskParameterMessageKind.SkippedTargetOutputs => SkipTargetUpToDateOutputs,
-                _ => throw new NotImplementedException($"Unsupported {nameof(TaskParameterMessageKind)} value: {messageKind}")
+                _ => throw new NotImplementedException($"Unsupported {nameof(TaskParameterMessageKind)} value: {args.Kind}")
             };
 
             var itemGroupText = GetParameterText(
                 resourceText,
-                itemType,
-                items,
-                logItemMetadata);
+                args.PropertyName ?? args.ItemType,
+                args.Items,
+                args.LogItemMetadata);
             return itemGroupText;
         }
     }

diff --git a/src/Build/BackEnd/Components/RequestBuilder/TargetUpToDateChecker.cs b/src/Build/BackEnd/Components/RequestBuilder/TargetUpToDateChecker.cs
@@ -368,6 +368,8 @@ private void LogUniqueInputsAndOutputs()
             var args = ItemGroupLoggingHelper.CreateTaskParameterEventArgs(
                 _buildEventContext,
                 TaskParameterMessageKind.SkippedTargetInputs,
+                parameterName: null,
+                propertyName: null,
                 itemType: null,
                 _uniqueTargetInputs.Keys.ToArray(),
                 logItemMetadata: false,
@@ -377,6 +379,8 @@ private void LogUniqueInputsAndOutputs()
             args = ItemGroupLoggingHelper.CreateTaskParameterEventArgs(
                 _buildEventContext,
                 TaskParameterMessageKind.SkippedTargetOutputs,
+                parameterName: null,
+                propertyName: null,
                 itemType: null,
                 _uniqueTargetOutputs.Keys.ToArray(),
                 logItemMetadata: false,

diff --git a/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs b/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs
@@ -1317,10 +1317,14 @@ private bool InternalSetTaskParameter(
                     // Structured logging for all parameters that have logging enabled and are not empty lists.
                     if (parameterValueAsList?.Count > 0 || (parameterValueAsList == null && !legacyBehavior))
                     {
+                        // Note: We're setting TaskParameterEventArgs.ItemType to parameter name for backward compatibility with
+                        // older loggers and binlog viewers.
                         ItemGroupLoggingHelper.LogTaskParameter(
                             _taskLoggingContext,
                             TaskParameterMessageKind.TaskInput,
-                            parameter.Name,
+                            parameterName: parameter.Name,
+                            propertyName: null,
+                            itemType: parameter.Name,
                             parameterValueAsList ?? new object[] { parameterValue },
                             parameter.LogItemMetadata);
                     }
@@ -1429,7 +1433,9 @@ static IEnumerable<KeyValuePair<string, string>> EnumerateMetadata(IDictionary c
                         ItemGroupLoggingHelper.LogTaskParameter(
                             _taskLoggingContext,
                             TaskParameterMessageKind.TaskOutput,
-                            outputTargetName,
+                            parameterName: parameter.Name,
+                            propertyName: null,
+                            itemType: outputTargetName,
                             outputs,
                             parameter.LogItemMetadata);
                     }
@@ -1470,7 +1476,23 @@ static IEnumerable<KeyValuePair<string, string>> EnumerateMetadata(IDictionary c
                         var outputString = joinedOutputs.ToString();
                         if (LogTaskInputs && !_taskLoggingContext.LoggingService.OnlyLogCriticalEvents)
                         {
-                            _taskLoggingContext.LogComment(MessageImportance.Low, "OutputPropertyLogMessage", outputTargetName, outputString);
+                            if (ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_12))
+                            {
+                                // Note: We're setting TaskParameterEventArgs.ItemType to property name for backward compatibility with
+                                // older loggers and binlog viewers.
+                                ItemGroupLoggingHelper.LogTaskParameter(
+                                    _taskLoggingContext,
+                                    TaskParameterMessageKind.TaskOutput,
+                                    parameterName: parameter.Name,
+                                    propertyName: outputTargetName,
+                                    itemType: outputTargetName,
+                                    new object[] { outputString },
+                                    parameter.LogItemMetadata);
+                            }
+                            else
+                            {
+                                _taskLoggingContext.LogComment(MessageImportance.Low, "OutputPropertyLogMessage", outputTargetName, outputString);
+                            }
                         }
 
                         _batchBucket.Lookup.SetProperty(ProjectPropertyInstance.Create(outputTargetName, outputString, parameterLocation, _projectInstance.IsImmutable));
@@ -1505,7 +1527,9 @@ private void GatherArrayStringAndValueOutputs(bool outputTargetIsItem, string ou
                         ItemGroupLoggingHelper.LogTaskParameter(
                             _taskLoggingContext,
                             TaskParameterMessageKind.TaskOutput,
-                            outputTargetName,
+                            parameterName: parameter.Name,
+                            propertyName: null,
+                            itemType: outputTargetName,
                             outputs,
                             parameter.LogItemMetadata);
                     }
@@ -1539,7 +1563,23 @@ private void GatherArrayStringAndValueOutputs(bool outputTargetIsItem, string ou
                         var outputString = joinedOutputs.ToString();
                         if (LogTaskInputs && !_taskLoggingContext.LoggingService.OnlyLogCriticalEvents)
                         {
-                            _taskLoggingContext.LogComment(MessageImportance.Low, "OutputPropertyLogMessage", outputTargetName, outputString);
+                            if (ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_12))
+                            {
+                                // Note: We're setting TaskParameterEventArgs.ItemType to property name for backward compatibility with
+                                // older loggers and binlog viewers.
+                                ItemGroupLoggingHelper.LogTaskParameter(
+                                    _taskLoggingContext,
+                                    TaskParameterMessageKind.TaskOutput,
+                                    parameterName: parameter.Name,
+                                    propertyName: outputTargetName,
+                                    itemType: outputTargetName,
+                                    new object[] { outputString },
+                                    parameter.LogItemMetadata);
+                            }
+                            else
+                            {
+                                _taskLoggingContext.LogComment(MessageImportance.Low, "OutputPropertyLogMessage", outputTargetName, outputString);
+                            }
                         }
 
                         _batchBucket.Lookup.SetProperty(ProjectPropertyInstance.Create(outputTargetName, outputString, parameterLocation, _projectInstance.IsImmutable));

diff --git a/src/Build/BuildCheck/Infrastructure/BuildEventsProcessor.cs b/src/Build/BuildCheck/Infrastructure/BuildEventsProcessor.cs
@@ -151,7 +151,7 @@ internal void ProcessTaskParameterEventArgs(
         {
             // Add the parameter name and value to the matching entry in _tasksBeingExecuted. Parameters come typed as IList
             // but it's more natural to pass them as scalar values so we unwrap one-element lists.
-            string parameterName = taskParameterEventArgs.ItemType;
+            string parameterName = taskParameterEventArgs.ParameterName;
             object? parameterValue = taskParameterEventArgs.Items?.Count switch
             {
                 1 => taskParameterEventArgs.Items[0],

diff --git a/src/Build/Logging/BinaryLogger/BinaryLogger.cs b/src/Build/Logging/BinaryLogger/BinaryLogger.cs
@@ -71,14 +71,16 @@ public sealed class BinaryLogger : ILogger
         //   - GeneratedFileUsedEventArgs exposed for brief period of time (so let's continue with 20)
         // version 20:
         //   - TaskStartedEventArgs: Added TaskAssemblyLocation property
+        // version 21:
+        //   - TaskParameterEventArgs: Added ParameterName and PropertyName properties
 
         // This should be never changed.
         // The minimum version of the binary log reader that can read log of above version.
         internal const int ForwardCompatibilityMinimalVersion = 18;
 
         // The current version of the binary log representation.
         // Changes with each update of the binary log format.
-        internal const int FileFormatVersion = 20;
+        internal const int FileFormatVersion = 21;
 
         // The minimum version of the binary log reader that can read log of above version.
         // This should be changed only when the binary log format is changed in a way that would prevent it from being

diff --git a/src/Build/Logging/BinaryLogger/BuildEventArgsReader.cs b/src/Build/Logging/BinaryLogger/BuildEventArgsReader.cs
@@ -1022,10 +1022,15 @@ private BuildEventArgs ReadTaskParameterEventArgs()
             var kind = (TaskParameterMessageKind)ReadInt32();
             var itemType = ReadDeduplicatedString();
             var items = ReadTaskItemList() as IList;
+            var (parameterName, propertyName) = _fileFormatVersion >= 21
+                ? (ReadDeduplicatedString(), ReadDeduplicatedString())
+                : (null, null);
 
             var e = ItemGroupLoggingHelper.CreateTaskParameterEventArgs(
                 fields.BuildEventContext,
                 kind,
+                parameterName,
+                propertyName,
                 itemType,
                 items,
                 logItemMetadata: true,

diff --git a/src/Build/Logging/BinaryLogger/BuildEventArgsWriter.cs b/src/Build/Logging/BinaryLogger/BuildEventArgsWriter.cs
@@ -571,6 +571,8 @@ private BinaryLogRecordKind Write(TaskParameterEventArgs e)
             Write((int)e.Kind);
             WriteDeduplicatedString(e.ItemType);
             WriteTaskItemList(e.Items, e.LogItemMetadata);
+            WriteDeduplicatedString(e.ParameterName);
+            WriteDeduplicatedString(e.PropertyName);
             if (e.Kind == TaskParameterMessageKind.AddItem
                || e.Kind == TaskParameterMessageKind.TaskOutput)
             {

diff --git a/src/Build/Resources/Strings.resx b/src/Build/Resources/Strings.resx
@@ -1102,6 +1102,9 @@
   <data name="OutputItemParameterMessagePrefix" xml:space="preserve">
     <value>Output Item(s): </value>
   </data>
+  <data name="OutputPropertyLogMessagePrefix" xml:space="preserve">
+    <value>Output Property: </value>
+  </data>
   <data name="OutputPropertyLogMessage" xml:space="preserve">
     <value>Output Property: {0}={1}</value>
   </data>

diff --git a/src/Build/Resources/xlf/Strings.cs.xlf b/src/Build/Resources/xlf/Strings.cs.xlf
diff --git a/src/Build/Resources/xlf/Strings.de.xlf b/src/Build/Resources/xlf/Strings.de.xlf
diff --git a/src/Build/Resources/xlf/Strings.es.xlf b/src/Build/Resources/xlf/Strings.es.xlf
diff --git a/src/Build/Resources/xlf/Strings.fr.xlf b/src/Build/Resources/xlf/Strings.fr.xlf
diff --git a/src/Build/Resources/xlf/Strings.it.xlf b/src/Build/Resources/xlf/Strings.it.xlf