From 61d12a71ca5d5b4e55800950b760331e2513802c Mon Sep 17 00:00:00 2001
From: Rob Mensching <rob@firegiant.com>
Date: Thu, 17 Nov 2022 14:10:50 -0800
Subject: Handle case where element has text and the target attribute

Fixes 7028
---
 src/wix/WixToolset.Converters/WixConverter.cs      |  23 +++--
 .../WixToolsetTest.Converters/PropertyFixture.cs   | 104 +++++++++++++++++++++
 2 files changed, 121 insertions(+), 6 deletions(-)

(limited to 'src')

diff --git a/src/wix/WixToolset.Converters/WixConverter.cs b/src/wix/WixToolset.Converters/WixConverter.cs
index 6c15b022..4e20ff14 100644
--- a/src/wix/WixToolset.Converters/WixConverter.cs
+++ b/src/wix/WixToolset.Converters/WixConverter.cs
@@ -2109,14 +2109,25 @@ namespace WixToolset.Converters
 
         private void ConvertInnerTextToAttribute(XElement element, string attributeName)
         {
-            if (TryGetInnerText(element, out var text, out var comments) &&
-                this.OnInformation(ConverterTestType.InnerTextDeprecated, element, "Using {0} element text is deprecated. Use the '{1}' attribute instead.", element.Name.LocalName, attributeName))
+            if (TryGetInnerText(element, out var text, out var comments))
             {
-                using (var lab = new ConversionLab(element))
+                // If the target attribute already exists, error if we have anything more than whitespace.
+                var attribute = element.Attribute(attributeName);
+                if (attribute != null)
                 {
-                    lab.RemoveOrphanTextNodes();
-                    element.Add(new XAttribute(attributeName, text));
-                    lab.AddCommentsAsSiblings(comments);
+                    if (!String.IsNullOrWhiteSpace(text))
+                    {
+                        this.OnError(ConverterTestType.InnerTextDeprecated, attribute, "Using {0} element text is deprecated. Remove the element's text and use only the '{1}' attribute.", element.Name.LocalName, attributeName);
+                    }
+                }
+                else if (this.OnInformation(ConverterTestType.InnerTextDeprecated, element, "Using {0} element text is deprecated. Use the '{1}' attribute instead.", element.Name.LocalName, attributeName))
+                {
+                    using (var lab = new ConversionLab(element))
+                    {
+                        lab.RemoveOrphanTextNodes();
+                        element.Add(new XAttribute(attributeName, text));
+                        lab.AddCommentsAsSiblings(comments);
+                    }
                 }
             }
         }
diff --git a/src/wix/test/WixToolsetTest.Converters/PropertyFixture.cs b/src/wix/test/WixToolsetTest.Converters/PropertyFixture.cs
index 90c71950..088ead5b 100644
--- a/src/wix/test/WixToolsetTest.Converters/PropertyFixture.cs
+++ b/src/wix/test/WixToolsetTest.Converters/PropertyFixture.cs
@@ -3,6 +3,7 @@
 namespace WixToolsetTest.Converters
 {
     using System;
+    using System.Linq;
     using System.Xml.Linq;
     using WixInternal.TestSupport;
     using WixToolset.Converters;
@@ -111,5 +112,108 @@ namespace WixToolsetTest.Converters
             WixAssert.CompareLineByLine(expected, actual);
             Assert.Equal(1, errors);
         }
+
+        [Fact]
+        public void CanConvertWithValueAndEmptyText()
+        {
+            var parse = String.Join(Environment.NewLine,
+                "<Wix xmlns='http://wixtoolset.org/schemas/v4/wxs'>",
+                "  <Fragment>",
+                "    <Property Id='Prop' Value='123'>",
+                "    </Property>",
+                "  </Fragment>",
+                "</Wix>");
+
+            var expected = new[]
+            {
+                "<Wix xmlns=\"http://wixtoolset.org/schemas/v4/wxs\">",
+                "  <Fragment>",
+                "    <Property Id=\"Prop\" Value=\"123\">",
+                "    </Property>",
+                "  </Fragment>",
+                "</Wix>",
+            };
+
+            var document = XDocument.Parse(parse, LoadOptions.PreserveWhitespace | LoadOptions.SetLineInfo);
+
+            var messaging = new MockMessaging();
+            var converter = new WixConverter(messaging, 2, null, null);
+            var errors = converter.ConvertDocument(document);
+
+            var actual = UnformattedDocumentLines(document);
+
+            WixAssert.CompareLineByLine(expected, actual);
+            Assert.Equal(0, errors);
+        }
+
+        [Fact]
+        public void CanConvertWithValueAndComment()
+        {
+            var parse = String.Join(Environment.NewLine,
+                "<Wix xmlns='http://wixtoolset.org/schemas/v4/wxs'>",
+                "  <Fragment>",
+                "    <Property Id='Prop' Value='123'>",
+                "      <!-- this is a comment -->",
+                "    </Property>",
+                "  </Fragment>",
+                "</Wix>");
+
+            var expected = new[]
+            {
+                "<Wix xmlns=\"http://wixtoolset.org/schemas/v4/wxs\">",
+                "  <Fragment>",
+                "    <Property Id=\"Prop\" Value=\"123\">",
+                "      <!-- this is a comment -->",
+                "    </Property>",
+                "  </Fragment>",
+                "</Wix>",
+            };
+
+            var document = XDocument.Parse(parse, LoadOptions.PreserveWhitespace | LoadOptions.SetLineInfo);
+
+            var messaging = new MockMessaging();
+            var converter = new WixConverter(messaging, 2, null, null);
+            var errors = converter.ConvertDocument(document);
+
+            var actual = UnformattedDocumentLines(document);
+
+            WixAssert.CompareLineByLine(expected, actual);
+            Assert.Equal(0, errors);
+        }
+
+        [Fact]
+        public void CanErrorWithValueAndText()
+        {
+            var parse = String.Join(Environment.NewLine,
+                "<Wix xmlns='http://wixtoolset.org/schemas/v4/wxs'>",
+                "  <Fragment>",
+                "    <Property Id='Prop' Value='123'>Not Allowed Value</Property>",
+                "  </Fragment>",
+                "</Wix>");
+
+            var expected = new[]
+            {
+                "<Wix xmlns=\"http://wixtoolset.org/schemas/v4/wxs\">",
+                "  <Fragment>",
+                "    <Property Id=\"Prop\" Value=\"123\">Not Allowed Value</Property>",
+                "  </Fragment>",
+                "</Wix>",
+            };
+
+            var document = XDocument.Parse(parse, LoadOptions.PreserveWhitespace | LoadOptions.SetLineInfo);
+
+            var messaging = new MockMessaging();
+            var converter = new WixConverter(messaging, 2, null, null);
+            var errors = converter.ConvertDocument(document);
+
+            var actual = UnformattedDocumentLines(document);
+
+            WixAssert.CompareLineByLine(expected, actual);
+            WixAssert.CompareLineByLine(new[]
+            {
+                "Using Property element text is deprecated. Remove the element's text and use only the 'Value' attribute. (InnerTextDeprecated)"
+            }, messaging.Messages.Select(m => m.ToString()).ToArray());
+            Assert.Equal(1, errors);
+        }
     }
 }
-- 
cgit v1.2.3-55-g6feb