Improving the log of CombineTargetFrameworkInfoProperties fails with not valid RootElementName

src/Tasks.UnitTests/CombineTargetFrameworkInfoProperties_Tests.cs

@@ -0,0 +1,37 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using Microsoft.Build.Framework;
+using Microsoft.Build.Tasks;
+using Shouldly;
+using Xunit;
+
+namespace Microsoft.Build.UnitTests
+{
+    public sealed class CombineTargetFrameworkInfoProperties_Tests
+    {
+        /// <summary>
+        /// https://github.com/dotnet/msbuild/issues/8320
+        /// </summary>
+        [Theory]
+        [InlineData(null, false, "MSB3991")]
+        [InlineData("", false, "MSB3991")]
+        [InlineData(null, true, "MSB3992")]
+        public void RootElementNameNotValid(string rootElementName, bool UseAttributeForTargetFrameworkInfoPropertyNames, string errorCode)
+        {
+            MockEngine e = new MockEngine();
+            var task = new CombineTargetFrameworkInfoProperties();
+            task.BuildEngine = e;
+            var items = new ITaskItem[]
+            {
+                new TaskItemData("ItemSpec1", null)
+            };
+            task.RootElementName = rootElementName;
+            task.PropertiesAndValues = items;
+            task.UseAttributeForTargetFrameworkInfoPropertyNames = UseAttributeForTargetFrameworkInfoPropertyNames;
+            task.Execute().ShouldBe(false);
+            e.AssertLogContains(errorCode);
+        }
+    }
+}

src/Tasks/CombineTargetFrameworkInfoProperties.cs

@@ -18,7 +18,6 @@ public class CombineTargetFrameworkInfoProperties : TaskExtension
         /// The root element name to use for the generated XML string
         /// </summary>
         public string RootElementName { get; set; }
-
         /// <summary>
         /// Items to include in the XML.  The ItemSpec should be the property name, and it should have Value metadata for its value.
         /// </summary>
@@ -39,16 +38,24 @@ public override bool Execute()
         {
             if (PropertiesAndValues != null)
             {
-                XElement root = UseAttributeForTargetFrameworkInfoPropertyNames ?
-                    new("TargetFramework", new XAttribute("Name", EscapingUtilities.Escape(RootElementName))) :
-                    new(RootElementName);
-
-                foreach (ITaskItem item in PropertiesAndValues)
+                if ((!UseAttributeForTargetFrameworkInfoPropertyNames && string.IsNullOrEmpty(RootElementName)) || (UseAttributeForTargetFrameworkInfoPropertyNames && RootElementName == null))
                 {
-                    root.Add(new XElement(item.ItemSpec, item.GetMetadata("Value")));
+                    string resource = UseAttributeForTargetFrameworkInfoPropertyNames ? "CombineTargetFrameworkInfoProperties.NotNullRootElementName" : "CombineTargetFrameworkInfoProperties.NotNullAndEmptyRootElementName";
+                    Log.LogErrorWithCodeFromResources(resource, nameof(RootElementName), nameof(UseAttributeForTargetFrameworkInfoPropertyNames));
                 }
+                else
+                {
+                    XElement root = UseAttributeForTargetFrameworkInfoPropertyNames ?
+                        new("TargetFramework", new XAttribute("Name", EscapingUtilities.Escape(RootElementName))) :
+                        new(RootElementName);
 
-                Result = root.ToString();
+                    foreach (ITaskItem item in PropertiesAndValues)
+                    {
+                        root.Add(new XElement(item.ItemSpec, item.GetMetadata("Value")));
+                    }
+
+                    Result = root.ToString();
+                }
             }
             return !Log.HasLoggedErrors;
         }

src/Tasks/Resources/Strings.resx

@@ -2985,6 +2985,18 @@
   <data name="ResolveKeySource.PfxUnsupported" xml:space="preserve">
     <value>PFX signing not supported on .NET Core.</value>
   </data>
+
+  <!--
+        MSB3991 - MSB3999   Task: CombineTargetFrameworkInfoProperties
+  -->
+  <data name="CombineTargetFrameworkInfoProperties.NotNullAndEmptyRootElementName">
+    <value>MSB3991: '{0}' is not set or empty. When {1} is false, make sure to set a non-empty value for '{0}'.</value>
+    <comment>{StrBegin="MSB3991: "}</comment>
+  </data>
+  <data name="CombineTargetFrameworkInfoProperties.NotNullRootElementName">
+    <value>MSB3992: '{0}' is not set. When {1} is true, make sure to set a value for '{0}'.</value>
+    <comment>{StrBegin="MSB3992: "}</comment>
+  </data>
   <!--
         The tasks message bucket is: MSB3001 - MSB3999
 
@@ -3071,6 +3083,7 @@
             MSB3961 - MSB3970   Task: GenerateLauncher
             MSB3971 - MSB3980   Task: GetReferenceAssemblyPaths overflow
             MSB3981 - MSB3990   Task: GetCompatiblePlatform
+            MSB3991 - MSB3999   Task: CombineTargetFrameworkInfoProperties
 
             MSB4000 - MSB4200   Portable targets & tasks (vsproject\flavors\portable\msbuild)
             MSB9000 - MSB9900   MSBuild targets files (C++)