Merge pull request #10486 from dotnet/merge/vs17.11-to-main

[automated] Merge branch 'vs17.11' => 'main'

src/Build.UnitTests/BuildEventArgsSerialization_Tests.cs

@@ -896,6 +896,7 @@ public void PropertyInitialValueEventArgs()
                 e => e.HelpKeyword,
                 e => e.SenderName);
         }
+
         [Fact]
         public void ReadingCorruptedStreamThrows()
         {

src/Framework.UnitTests/CustomEventArgSerialization_Tests.cs

@@ -974,5 +974,82 @@ private static void VerifyTaskFinished(TaskFinishedEventArgs genericEvent, TaskF
             newGenericEvent.TaskFile.ShouldBe(genericEvent.TaskFile, StringCompareShould.IgnoreCase); // "Expected TaskFile to Match"
             newGenericEvent.TaskName.ShouldBe(genericEvent.TaskName, StringCompareShould.IgnoreCase); // "Expected TaskName to Match"
         }
+
+
+        [Fact]
+        public void TestTelemetryEventArgs_AllProperties()
+        {
+            // Test using reasonable values
+            TelemetryEventArgs genericEvent = new TelemetryEventArgs { EventName = "Good", Properties = new Dictionary<string, string> { { "Key", "Value" } } };
+            genericEvent.BuildEventContext = new BuildEventContext(5, 4, 3, 2);
+
+            TelemetryEventArgs newGenericEvent = RoundTrip(genericEvent);
+
+            VerifyGenericEventArg(genericEvent, newGenericEvent);
+            VerifyTelemetryEvent(genericEvent, newGenericEvent);
+        }
+
+        [Fact]
+        public void TestTelemetryEventArgs_NullProperties()
+        {
+            // Test using reasonable values
+            TelemetryEventArgs genericEvent = new TelemetryEventArgs { EventName = "Good", Properties = null };
+            genericEvent.BuildEventContext = new BuildEventContext(5, 4, 3, 2);
+
+            TelemetryEventArgs newGenericEvent = RoundTrip(genericEvent);
+
+            // quirk - the properties dict is initialized to an empty dictionary by the default constructor, so it's not _really_ round-trippable.
+            // so we modify the source event for easier comparison here.
+            genericEvent.Properties = new Dictionary<string, string>();
+
+            VerifyGenericEventArg(genericEvent, newGenericEvent);
+            VerifyTelemetryEvent(genericEvent, newGenericEvent);
+        }
+
+        [Fact]
+        public void TestTelemetryEventArgs_NullEventName()
+        {
+            // Test using null event name
+            TelemetryEventArgs genericEvent = new TelemetryEventArgs { EventName = null, Properties = new Dictionary<string, string> { { "Key", "Value" } } };
+            genericEvent.BuildEventContext = new BuildEventContext(5, 4, 3, 2);
+
+            TelemetryEventArgs newGenericEvent = RoundTrip(genericEvent);
+
+            VerifyGenericEventArg(genericEvent, newGenericEvent);
+            VerifyTelemetryEvent(genericEvent, newGenericEvent);
+        }
+
+        [Fact]
+        public void TestTelemetryEventArgs_NullPropertyValue()
+        {
+            // Test using null property value name
+            TelemetryEventArgs genericEvent = new TelemetryEventArgs { EventName = "Good", Properties = new Dictionary<string, string> { { "Key", null } } };
+            genericEvent.BuildEventContext = new BuildEventContext(5, 4, 3, 2);
+
+            TelemetryEventArgs newGenericEvent = RoundTrip(genericEvent);
+
+            VerifyGenericEventArg(genericEvent, newGenericEvent);
+            VerifyTelemetryEvent(genericEvent, newGenericEvent);
+        }
+
+        private T RoundTrip<T>(T original)
+            where T : BuildEventArgs, new()
+        {
+            _stream.Position = 0;
+            original.WriteToStream(_writer);
+            long streamWriteEndPosition = _stream.Position;
+
+            _stream.Position = 0;
+            var actual = new T();
+            actual.CreateFromStream(_reader, _eventArgVersion);
+            _stream.Position.ShouldBe(streamWriteEndPosition); // "Stream End Positions Should Match"
+            return actual;
+        }
+
+        private static void VerifyTelemetryEvent(TelemetryEventArgs expected, TelemetryEventArgs actual)
+        {
+            actual.EventName.ShouldBe(expected.EventName);
+            actual.Properties.ShouldBe(expected.Properties);
+        }
     }
 }

src/Framework/TelemetryEventArgs.cs

@@ -6,8 +6,6 @@
 using System.IO;
 using Microsoft.Build.Shared;
 
-#nullable disable
-
 namespace Microsoft.Build.Framework
 {
     /// <summary>
@@ -19,12 +17,12 @@ public sealed class TelemetryEventArgs : BuildEventArgs
         /// <summary>
         /// Gets or sets the name of the event.
         /// </summary>
-        public string EventName { get; set; }
+        public string? EventName { get; set; }
 
         /// <summary>
         /// Gets or sets a list of properties associated with the event.
         /// </summary>
-        public IDictionary<string, string> Properties { get; set; } = new Dictionary<string, string>();
+        public IDictionary<string, string?> Properties { get; set; } = new Dictionary<string, string?>();
 
         internal override void WriteToStream(BinaryWriter writer)
         {
@@ -34,13 +32,17 @@ internal override void WriteToStream(BinaryWriter writer)
             int count = Properties?.Count ?? 0;
             writer.Write7BitEncodedInt(count);
 
+            if (Properties == null)
+            {
+                return;
+            }
+
             foreach (var kvp in Properties)
             {
                 writer.Write(kvp.Key);
-                writer.Write(kvp.Value);
+                writer.WriteOptionalString(kvp.Value);
             }
         }
-
         internal override void CreateFromStream(BinaryReader reader, int version)
         {
             base.CreateFromStream(reader, version);
@@ -51,7 +53,7 @@ internal override void CreateFromStream(BinaryReader reader, int version)
             for (int i = 0; i < count; i++)
             {
                 string key = reader.ReadString();
-                string value = reader.ReadString();
+                string? value = reader.ReadOptionalString();
                 Properties.Add(key, value);
             }
         }

src/Tasks/CompatibilitySuppressions.xml

@@ -1,10 +1,6 @@
 <?xml version="1.0" encoding="utf-8"?>
 <!-- https://learn.microsoft.com/en-us/dotnet/fundamentals/package-validation/diagnostic-ids -->
 <Suppressions xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema">
-  <!-- For ease of logging the "not supported on Core" message, these tasks are a
-         TaskRequiresFramework on netstandard/netcore. Since the type is sealed there,
-         that shouldn't cause any implementation problems since no one can derive
-         from it and try to call TaskExtension.Log. -->
   <Suppression>
     <DiagnosticId>CP0007</DiagnosticId>
     <Target>T:Microsoft.Build.Tasks.AL</Target>
@@ -71,12 +67,6 @@
     <Left>ref/netstandard2.0/Microsoft.Build.Tasks.Core.dll</Left>
     <Right>ref/net472/Microsoft.Build.Tasks.Core.dll</Right>
   </Suppression>
-
-  <!-- PKV004 for netstandard2.0-supporting TFs that we do not have runtime assemblies for.
-       This is intentional, because you can only use MSBuild in the context of a .NET SDK
-       (on net7.0, as of MSBuild 17.4) or in the context of Visual Studio (net472), but we
-       have previously shipped netstandard2.0 packages, and if you want to support both
-       runtime contexts it still makes sense to target that. -->
   <Suppression>
     <DiagnosticId>PKV004</DiagnosticId>
     <Target>.NETCoreApp,Version=v2.0</Target>
@@ -125,4 +115,4 @@
     <DiagnosticId>PKV004</DiagnosticId>
     <Target>Xamarin.XboxOne,Version=v0.0</Target>
   </Suppression>
-</Suppressions>
+</Suppressions>

src/Utilities/CompatibilitySuppressions.xml

@@ -1,11 +1,6 @@
 <?xml version="1.0" encoding="utf-8"?>
 <!-- https://learn.microsoft.com/en-us/dotnet/fundamentals/package-validation/diagnostic-ids -->
 <Suppressions xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema">
-  <!-- PKV004 for netstandard2.0-supporting TFs that we do not have runtime assemblies for.
-       This is intentional, because you can only use MSBuild in the context of a .NET SDK
-       (on net7.0, as of MSBuild 17.4) or in the context of Visual Studio (net472), but we
-       have previously shipped netstandard2.0 packages, and if you want to support both
-       runtime contexts it still makes sense to target that. -->
   <Suppression>
     <DiagnosticId>PKV004</DiagnosticId>
     <Target>.NETCoreApp,Version=v2.0</Target>
@@ -54,4 +49,4 @@
     <DiagnosticId>PKV004</DiagnosticId>
     <Target>Xamarin.XboxOne,Version=v0.0</Target>
   </Suppression>
-</Suppressions>
+</Suppressions>