Log TaskParameterEvent for scalar parameters

documentation/wiki/ChangeWaves.md

@@ -23,6 +23,9 @@ A wave of features is set to "rotate out" (i.e. become standard functionality) t
 
 ## Current Rotation of Change Waves
 
+### 17.12
+- [Log TaskParameterEvent for scalar parameters](https://github.com/dotnet/msbuild/pull/9908)
+
 ### 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`
 - [Warning on serialization custom events by default in .NET framework](https://github.com/dotnet/msbuild/pull/9318)
@@ -36,14 +39,12 @@ A wave of features is set to "rotate out" (i.e. become standard functionality) t
 - [Introduce [MSBuild]::StableStringHash overloads](https://github.com/dotnet/msbuild/issues/9519)
 - [Keep the encoding of standard output & error consistent with the console code page for ToolTask](https://github.com/dotnet/msbuild/pull/9539)
 
-
 ### 17.8
 - [[RAR] Don't do I/O on SDK-provided references](https://github.com/dotnet/msbuild/pull/8688)
 - [Delete destination file before copy](https://github.com/dotnet/msbuild/pull/8685)
 - [Moving from SHA1 to SHA256 for Hash task](https://github.com/dotnet/msbuild/pull/8812)
 - [Deprecating custom derived BuildEventArgs](https://github.com/dotnet/msbuild/pull/8917) - 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`
 
-
 ### 17.6
 - [Parse invalid property under target](https://github.com/dotnet/msbuild/pull/8190)
 - [Eliminate project string cache](https://github.com/dotnet/msbuild/pull/7965)

src/Build.UnitTests/BackEnd/TaskExecutionHost_Tests.cs

@@ -18,6 +18,7 @@
 using Microsoft.Build.Execution;
 using Microsoft.Build.Framework;
 using Microsoft.Build.Shared;
+using Shouldly;
 using Xunit;
 using InvalidProjectFileException = Microsoft.Build.Exceptions.InvalidProjectFileException;
 using TaskItem = Microsoft.Build.Execution.ProjectItemInstance.TaskItem;
@@ -1048,6 +1049,36 @@ public void TestTaskDictionaryOutputItems()
                 """);
             ml.AssertLogContains("a=b");
         }
+
+        [Fact]
+        public void TestTaskParameterLogging()
+        {
+            string customTaskPath = Assembly.GetExecutingAssembly().Location;
+            MockLogger ml = ObjectModelHelpers.BuildProjectExpectSuccess($"""
+                    <Project>
+                        <UsingTask TaskName=`TaskThatReturnsDictionaryTaskItem` AssemblyFile=`{customTaskPath}`/>
+                        <ItemGroup>
+                            <MyItem Include="item1"/>
+                            <MyItem Include="item2"/>
+                        </ItemGroup>
+                        <Target Name=`Build`>
+                           <TaskThatReturnsDictionaryTaskItem Key="a" Value="b" AdditionalParameters="@(MyItem)" />
+                        </Target>
+                    </Project>
+                """);
+
+            // Each parameter should be logged as TaskParameterEvent.
+            ml.TaskParameterEvents.Count.ShouldBe(3);
+            IList<string> messages = ml.TaskParameterEvents.Select(e => e.Message).ToList();
+            messages.ShouldContain($"{ItemGroupLoggingHelper.TaskParameterPrefix}Key=a");
+            messages.ShouldContain($"{ItemGroupLoggingHelper.TaskParameterPrefix}Value=b");
+            messages.ShouldContain($"{ItemGroupLoggingHelper.TaskParameterPrefix}\n    AdditionalParameters=\n        item1\n        item2");
+
+            // Parameters should not be logged as messages.
+            messages = ml.BuildMessageEvents.Select(e => e.Message).ToList();
+            messages.ShouldNotContain(m => m.StartsWith(ItemGroupLoggingHelper.TaskParameterPrefix));
+        }
+
         #endregion
 
         #region ITestTaskHost Members

src/Build.UnitTests/BackEnd/TaskThatReturnsDictionaryTaskItem.cs

@@ -17,6 +17,8 @@ public sealed class TaskThatReturnsDictionaryTaskItem : Utilities.Task
     public string Key { get; set; }
     public string Value { get; set; }
 
+    public ITaskItem[] AdditionalParameters { get; set; }
+
     public override bool Execute()
     {
         var metaValue = new MinimalDictionary<string, string>

src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs

@@ -1266,30 +1266,6 @@ private bool InitializeTaskVectorParameter(
             return success;
         }
 
-        /// <summary>
-        /// Variation to handle arrays, to help with logging the parameters.
-        /// </summary>
-        /// <remarks>
-        /// Logging currently enabled only by an env var.
-        /// </remarks>
-        private bool InternalSetTaskParameter(TaskPropertyInfo parameter, IList parameterValue)
-        {
-            if (LogTaskInputs &&
-                !_taskLoggingContext.LoggingService.OnlyLogCriticalEvents &&
-                parameterValue.Count > 0 &&
-                parameter.Log)
-            {
-                ItemGroupLoggingHelper.LogTaskParameter(
-                    _taskLoggingContext,
-                    TaskParameterMessageKind.TaskInput,
-                    parameter.Name,
-                    parameterValue,
-                    parameter.LogItemMetadata);
-            }
-
-            return InternalSetTaskParameter(parameter, (object)parameterValue);
-        }
-
         private static readonly string TaskParameterFormatString = ItemGroupLoggingHelper.TaskParameterPrefix + "{0}={1}";
 
         /// <summary>
@@ -1303,14 +1279,31 @@ private bool InternalSetTaskParameter(
 
             if (LogTaskInputs && !_taskLoggingContext.LoggingService.OnlyLogCriticalEvents)
             {
-                // If the type is a list, we already logged the parameters
-                if (!(parameterValue is IList))
+                IList parameterValueAsList = parameterValue as IList;
+                bool legacyBehavior = !ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_12);
+
+                // Legacy textual logging for parameters that are not lists.
+                if (legacyBehavior && parameterValueAsList == null)
                 {
                     _taskLoggingContext.LogCommentFromText(
-                        MessageImportance.Low,
-                        TaskParameterFormatString,
-                        parameter.Name,
-                        ItemGroupLoggingHelper.GetStringFromParameterValue(parameterValue));
+                       MessageImportance.Low,
+                       TaskParameterFormatString,
+                       parameter.Name,
+                       ItemGroupLoggingHelper.GetStringFromParameterValue(parameterValue));
+                }
+
+                if (parameter.Log)
+                {
+                    // Structured logging for all parameters that have logging enabled and are not empty lists.
+                    if (parameterValueAsList?.Count > 0 || (parameterValueAsList == null && !legacyBehavior))
+                    {
+                        ItemGroupLoggingHelper.LogTaskParameter(
+                            _taskLoggingContext,
+                            TaskParameterMessageKind.TaskInput,
+                            parameter.Name,
+                            parameterValueAsList ?? new object[] { parameterValue },
+                            parameter.LogItemMetadata);
+                    }
                 }
             }
 

src/Framework/ChangeWaves.cs

@@ -29,7 +29,8 @@ internal static class ChangeWaves
         internal static readonly Version Wave17_6 = new Version(17, 6);
         internal static readonly Version Wave17_8 = new Version(17, 8);
         internal static readonly Version Wave17_10 = new Version(17, 10);
-        internal static readonly Version[] AllWaves = { Wave17_4, Wave17_6, Wave17_8, Wave17_10 };
+        internal static readonly Version Wave17_12 = new Version(17, 12);
+        internal static readonly Version[] AllWaves = { Wave17_4, Wave17_6, Wave17_8, Wave17_10, Wave17_12 };
 
         /// <summary>
         /// Special value indicating that all features behind all Change Waves should be enabled.

src/UnitTests.Shared/MockLogger.cs

@@ -123,6 +123,11 @@ public sealed class MockLogger : ILogger
         /// </summary>
         public List<TaskFinishedEventArgs> TaskFinishedEvents { get; } = new List<TaskFinishedEventArgs>();
 
+        /// <summary>
+        /// List of TaskParameter events
+        /// </summary>
+        public List<TaskParameterEventArgs> TaskParameterEvents { get; } = new List<TaskParameterEventArgs>();
+
         /// <summary>
         /// List of BuildMessage events
         /// </summary>
@@ -362,6 +367,11 @@ public void LoggerEventHandler(object sender, BuildEventArgs eventArgs)
                             TaskFinishedEvents.Add(taskFinishedEventArgs);
                             break;
                         }
+                    case TaskParameterEventArgs taskParameterEventArgs:
+                        {
+                            TaskParameterEvents.Add(taskParameterEventArgs);
+                            break;
+                        }
                     case BuildMessageEventArgs buildMessageEventArgs:
                         {
                             BuildMessageEvents.Add(buildMessageEventArgs);