@joebigelow / wix-1 / commits / 61d12a71

Handle case where element has text and the target attribute

Fixes 7028

Rob Mensching committed Nov 17, 2022 at 14:10 UTC 61d12a71ca5d5b4e55800950b760331e2513802c
2 files changed +121 -6
src/wix/WixToolset.Converters/WixConverter.cs
+17 -6
@@ -2109,14 +2109,25 @@ namespace WixToolset.Converters
2109
2110 private void ConvertInnerTextToAttribute(XElement element, string attributeName)
2111 {
2112 - if (TryGetInnerText(element, out var text, out var comments) &&
2113 - this.OnInformation(ConverterTestType.InnerTextDeprecated, element, "Using {0} element text is deprecated. Use the '{1}' attribute instead.", element.Name.LocalName, attributeName))
2112 + if (TryGetInnerText(element, out var text, out var comments))
2113 {
2115 - using (var lab = new ConversionLab(element))
2114 + // If the target attribute already exists, error if we have anything more than whitespace.
2115 + var attribute = element.Attribute(attributeName);
2116 + if (attribute != null)
2117 {
2117 - lab.RemoveOrphanTextNodes();
2118 - element.Add(new XAttribute(attributeName, text));
2119 - lab.AddCommentsAsSiblings(comments);
2118 + if (!String.IsNullOrWhiteSpace(text))
2119 + {
2120 + 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);
2121 + }
2122 + }
2123 + else if (this.OnInformation(ConverterTestType.InnerTextDeprecated, element, "Using {0} element text is deprecated. Use the '{1}' attribute instead.", element.Name.LocalName, attributeName))
2124 + {
2125 + using (var lab = new ConversionLab(element))
2126 + {
2127 + lab.RemoveOrphanTextNodes();
2128 + element.Add(new XAttribute(attributeName, text));
2129 + lab.AddCommentsAsSiblings(comments);
2130 + }
2131 }
2132 }
2133 }
src/wix/test/WixToolsetTest.Converters/PropertyFixture.cs
+104
@@ -3,6 +3,7 @@
3 namespace WixToolsetTest.Converters
4 {
5 using System;
6 + using System.Linq;
7 using System.Xml.Linq;
8 using WixInternal.TestSupport;
9 using WixToolset.Converters;
@@ -111,5 +112,108 @@ namespace WixToolsetTest.Converters
112 WixAssert.CompareLineByLine(expected, actual);
113 Assert.Equal(1, errors);
114 }
115 +
116 + [Fact]
117 + public void CanConvertWithValueAndEmptyText()
118 + {
119 + var parse = String.Join(Environment.NewLine,
120 + "<Wix xmlns='http://wixtoolset.org/schemas/v4/wxs'>",
121 + " <Fragment>",
122 + " <Property Id='Prop' Value='123'>",
123 + " </Property>",
124 + " </Fragment>",
125 + "</Wix>");
126 +
127 + var expected = new[]
128 + {
129 + "<Wix xmlns=\"http://wixtoolset.org/schemas/v4/wxs\">",
130 + " <Fragment>",
131 + " <Property Id=\"Prop\" Value=\"123\">",
132 + " </Property>",
133 + " </Fragment>",
134 + "</Wix>",
135 + };
136 +
137 + var document = XDocument.Parse(parse, LoadOptions.PreserveWhitespace | LoadOptions.SetLineInfo);
138 +
139 + var messaging = new MockMessaging();
140 + var converter = new WixConverter(messaging, 2, null, null);
141 + var errors = converter.ConvertDocument(document);
142 +
143 + var actual = UnformattedDocumentLines(document);
144 +
145 + WixAssert.CompareLineByLine(expected, actual);
146 + Assert.Equal(0, errors);
147 + }
148 +
149 + [Fact]
150 + public void CanConvertWithValueAndComment()
151 + {
152 + var parse = String.Join(Environment.NewLine,
153 + "<Wix xmlns='http://wixtoolset.org/schemas/v4/wxs'>",
154 + " <Fragment>",
155 + " <Property Id='Prop' Value='123'>",
156 + " <!-- this is a comment -->",
157 + " </Property>",
158 + " </Fragment>",
159 + "</Wix>");
160 +
161 + var expected = new[]
162 + {
163 + "<Wix xmlns=\"http://wixtoolset.org/schemas/v4/wxs\">",
164 + " <Fragment>",
165 + " <Property Id=\"Prop\" Value=\"123\">",
166 + " <!-- this is a comment -->",
167 + " </Property>",
168 + " </Fragment>",
169 + "</Wix>",
170 + };
171 +
172 + var document = XDocument.Parse(parse, LoadOptions.PreserveWhitespace | LoadOptions.SetLineInfo);
173 +
174 + var messaging = new MockMessaging();
175 + var converter = new WixConverter(messaging, 2, null, null);
176 + var errors = converter.ConvertDocument(document);
177 +
178 + var actual = UnformattedDocumentLines(document);
179 +
180 + WixAssert.CompareLineByLine(expected, actual);
181 + Assert.Equal(0, errors);
182 + }
183 +
184 + [Fact]
185 + public void CanErrorWithValueAndText()
186 + {
187 + var parse = String.Join(Environment.NewLine,
188 + "<Wix xmlns='http://wixtoolset.org/schemas/v4/wxs'>",
189 + " <Fragment>",
190 + " <Property Id='Prop' Value='123'>Not Allowed Value</Property>",
191 + " </Fragment>",
192 + "</Wix>");
193 +
194 + var expected = new[]
195 + {
196 + "<Wix xmlns=\"http://wixtoolset.org/schemas/v4/wxs\">",
197 + " <Fragment>",
198 + " <Property Id=\"Prop\" Value=\"123\">Not Allowed Value</Property>",
199 + " </Fragment>",
200 + "</Wix>",
201 + };
202 +
203 + var document = XDocument.Parse(parse, LoadOptions.PreserveWhitespace | LoadOptions.SetLineInfo);
204 +
205 + var messaging = new MockMessaging();
206 + var converter = new WixConverter(messaging, 2, null, null);
207 + var errors = converter.ConvertDocument(document);
208 +
209 + var actual = UnformattedDocumentLines(document);
210 +
211 + WixAssert.CompareLineByLine(expected, actual);
212 + WixAssert.CompareLineByLine(new[]
213 + {
214 + "Using Property element text is deprecated. Remove the element's text and use only the 'Value' attribute. (InnerTextDeprecated)"
215 + }, messaging.Messages.Select(m => m.ToString()).ToArray());
216 + Assert.Equal(1, errors);
217 + }
218 }
219 }