@joebigelow / wix-1 / commits / 217182f8

Update converted source line numbers when conversion is saved

The conversion process has a good chance of changing the shape of .wxs source file. Therefore, the old line numbers may be wrong and should be replaced with the line numbers from the converted file when it is saved. Fixes 6969

Rob Mensching committed Oct 25, 2022 at 07:01 UTC 217182f85fa737d20fb5846f395cabfa599bf1c6

5 files changed +272 -61

src/wix/WixToolset.Converters/HashSetExtensions.cs new

+17

@@ -0,0 +1,17 @@

2 +

3 +namespace WixToolset.Converters

4 +{

5 + using System.Collections.Generic;

6 +

7 + internal static class HashSetExtensions

8 + {

9 + public static void AddRange<T>(this HashSet<T> set, IEnumerable<T> values)

10 + {

11 + foreach (var value in values)

12 + {

13 + set.Add(value);

14 + }

15 + }

16 + }

17 +}

src/wix/WixToolset.Converters/WixConverter.cs

+206 -57

@@ -11,6 +11,7 @@ namespace WixToolset.Converters

11 using System.Text.RegularExpressions;

12 using System.Xml;

13 using System.Xml.Linq;

14 + using System.Xml.XPath;

15 using WixToolset.Data;

16 using WixToolset.Data.WindowsInstaller;

17 using WixToolset.Extensibility.Services;

@@ -272,6 +273,8 @@ namespace WixToolset.Converters

273 { WixConverter.WixLocalizationElementWithoutNamespaceName, this.ConvertWixLocalizationElementWithoutNamespace },

274 };

275

276 + this.ConversionMessages = new List<Message>();

277 +

278 this.Messaging = messaging;

279

280 this.IndentationAmount = indentationAmount;

@@ -285,7 +288,7 @@ namespace WixToolset.Converters

288

289 private CustomTableTarget CustomTableSetting { get; }

290

288 - private int Messages { get; set; }

291 + private List<Message> ConversionMessages { get; }

292

293 private HashSet<ConverterTestType> ErrorsAsWarnings { get; set; }

294

@@ -308,54 +311,35 @@ namespace WixToolset.Converters

311 /// </summary>

312 /// <param name="sourceFile">The file to convert.</param>

313 /// <param name="saveConvertedFile">Option to save the converted Messages that are found.</param>

311 - /// <returns>The number of Messages found.</returns>

314 + /// <returns>The number of conversions found.</returns>

315 public int ConvertFile(string sourceFile, bool saveConvertedFile)

316 {

314 - var document = this.OpenSourceFile(sourceFile);

317 + var savedDocument = false;

318

316 - if (document is null)

319 + if (this.TryOpenSourceFile(sourceFile, out var document))

320 {

318 - return 1;

319 - }

320 -

321 - this.ConvertDocument(document);

321 + this.Convert(document);

322

323 - // Fix Messages if requested and necessary.

324 - if (saveConvertedFile && 0 < this.Messages)

325 - {

326 - this.SaveDocument(document);

323 + // Fix Messages if requested and necessary.

324 + if (saveConvertedFile && 0 < this.ConversionMessages.Count)

325 + {

326 + savedDocument = this.SaveDocument(document);

327 + }

328 }

329

329 - return this.Messages;

330 + return this.ReportMessages(document, savedDocument);

331 }

332

333 /// <summary>

334 /// Convert a document.

335 /// </summary>

336 /// <param name="document">The document to convert.</param>

336 - /// <returns>The number of Messages found.</returns>

337 + /// <returns>The number of conversions found.</returns>

338 public int ConvertDocument(XDocument document)

339 {

339 - // Reset the instance info.

340 - this.Messages = 0;

341 - this.SourceVersion = 0;

342 - this.Operation = ConvertOperation.Convert;

343 -

344 - // Remove the declaration.

345 - if (null != document.Declaration

346 - && this.OnInformation(ConverterTestType.DeclarationPresent, null, "This file contains an XML declaration on the first line."))

347 - {

348 - document.Declaration = null;

349 - TrimLeadingText(document);

350 - }

351 -

352 - this.XRoot = document.Root;

353 -

354 - // Start converting the nodes at the top.

355 - this.ConvertNodes(document.Nodes(), 0);

356 - this.RemoveUnusedNamespaces(document.Root);

340 + this.Convert(document);

341

358 - return this.Messages;

342 + return this.ReportMessages(document, false);

343 }

344

345 /// <summary>

@@ -366,22 +350,20 @@ namespace WixToolset.Converters

350 /// <returns>The number of Messages found.</returns>

351 public int FormatFile(string sourceFile, bool saveConvertedFile)

352 {

369 - var document = this.OpenSourceFile(sourceFile);

353 + var savedDocument = false;

354

371 - if (document is null)

355 + if (this.TryOpenSourceFile(sourceFile, out var document))

356 {

373 - return 1;

374 - }

375 -

376 - this.FormatDocument(document);

357 + this.FormatDocument(document);

358

378 - // Fix Messages if requested and necessary.

379 - if (saveConvertedFile && 0 < this.Messages)

380 - {

381 - this.SaveDocument(document);

359 + // Fix Messages if requested and necessary.

360 + if (saveConvertedFile && 0 < this.ConversionMessages.Count)

361 + {

362 + savedDocument = this.SaveDocument(document);

363 + }

364 }

365

384 - return this.Messages;

366 + return this.ReportMessages(document, savedDocument);

367 }

368

369 /// <summary>

@@ -390,44 +372,74 @@ namespace WixToolset.Converters

372 /// <param name="document">The document to format.</param>

373 /// <returns>The number of Messages found.</returns>

374 public int FormatDocument(XDocument document)

375 + {

376 + this.Format(document);

377 +

378 + return this.ReportMessages(document, false);

379 + }

380 +

381 + private void Convert(XDocument document)

382 {

383 // Reset the instance info.

395 - this.Messages = 0;

384 + this.ConversionMessages.Clear();

385 this.SourceVersion = 0;

397 - this.Operation = ConvertOperation.Format;

386 + this.Operation = ConvertOperation.Convert;

387

388 // Remove the declaration.

389 if (null != document.Declaration

401 - && this.OnInformation(ConverterTestType.DeclarationPresent, null, "This file contains an XML declaration on the first line."))

390 + && this.OnInformation(ConverterTestType.DeclarationPresent, document, "This file contains an XML declaration on the first line."))

391 {

392 document.Declaration = null;

393 TrimLeadingText(document);

394 }

395

396 + this.XRoot = document.Root;

397 +

398 // Start converting the nodes at the top.

399 this.ConvertNodes(document.Nodes(), 0);

400 this.RemoveUnusedNamespaces(document.Root);

401 + }

402 +

403 + private void Format(XDocument document)

404 + {

405 + // Reset the instance info.

406 + this.ConversionMessages.Clear();

407 + this.SourceVersion = 0;

408 + this.Operation = ConvertOperation.Format;

409

411 - return this.Messages;

410 + // Remove the declaration.

411 + if (null != document.Declaration

412 + && this.OnInformation(ConverterTestType.DeclarationPresent, document, "This file contains an XML declaration on the first line."))

413 + {

414 + document.Declaration = null;

415 + TrimLeadingText(document);

416 + }

417 +

418 + this.XRoot = document.Root;

419 +

420 + // Start converting the nodes at the top.

421 + this.ConvertNodes(document.Nodes(), 0);

422 + this.RemoveUnusedNamespaces(document.Root);

423 }

424

414 - private XDocument OpenSourceFile(string sourceFile)

425 + private bool TryOpenSourceFile(string sourceFile, out XDocument document)

426 {

427 this.SourceFile = sourceFile;

428

429 try

430 {

420 - return XDocument.Load(this.SourceFile, LoadOptions.PreserveWhitespace | LoadOptions.SetLineInfo);

431 + document = XDocument.Load(this.SourceFile, LoadOptions.PreserveWhitespace | LoadOptions.SetLineInfo);

432 }

433 catch (XmlException e)

434 {

435 this.OnError(ConverterTestType.XmlException, null, "The xml is invalid. Detail: '{0}'", e.Message);

436 + document = null;

437 }

438

427 - return null;

439 + return document != null;

440 }

441

430 - private void SaveDocument(XDocument document)

442 + private bool SaveDocument(XDocument document)

443 {

444 var ignoreDeclarationError = this.IgnoreErrors.Contains(ConverterTestType.DeclarationPresent);

445

@@ -437,11 +449,15 @@ namespace WixToolset.Converters

449 {

450 document.Save(writer);

451 }

452 +

453 + return true;

454 }

455 catch (UnauthorizedAccessException)

456 {

457 this.OnError(ConverterTestType.UnauthorizedAccessException, null, "Could not write to file.");

458 }

459 +

460 + return false;

461 }

462

463 private void ConvertNodes(IEnumerable<XNode> nodes, int level)

@@ -2129,6 +2145,14 @@ namespace WixToolset.Converters

2145 convertedAttribute = new XAttribute(ns.GetName(attribute.Name.LocalName), attribute.Value);

2146 }

2147

2148 + if (convertedAttribute != attribute)

2149 + {

2150 + foreach (var message in attribute.Annotations<Message>())

2151 + {

2152 + convertedAttribute.AddAnnotation(message);

2153 + }

2154 + }

2155 +

2156 element.Add(convertedAttribute);

2157 }

2158 }

@@ -2214,6 +2238,110 @@ namespace WixToolset.Converters

2238 }

2239 }

2240

2241 + private int ReportMessages(XDocument document, bool saved)

2242 + {

2243 + var conversionCount = this.ConversionMessages.Count;

2244 +

2245 + // If the converted/formatted document was saved, update the source line numbers

2246 + // in the messages since they will possibly be wrong.

2247 + if (saved && conversionCount > 0)

2248 + {

2249 + var fixedupMessages = new HashSet<Message>();

2250 +

2251 + // Load the converted document so we can look up new line numbers for

2252 + // messages.

2253 + var convertedDocument = XDocument.Load(this.SourceFile, LoadOptions.PreserveWhitespace | LoadOptions.SetLineInfo);

2254 + var convertedNavigator = convertedDocument.CreateNavigator();

2255 +

2256 + // Look through all nodes in the document and try to fix up their line numbers.

2257 + foreach (var elements in document.Descendants())

2258 + {

2259 + var fixedup = this.FixupMessageLineNumbers(elements, convertedNavigator);

2260 + fixedupMessages.AddRange(fixedup);

2261 +

2262 + // Attributes are not considered nodes so they must be enumerated independently.

2263 + if (elements is XElement element)

2264 + {

2265 + foreach (var attribute in element.Attributes())

2266 + {

2267 + fixedup = this.FixupMessageLineNumbers(attribute, convertedNavigator);

2268 + fixedupMessages.AddRange(fixedup);

2269 + }

2270 + }

2271 + }

2272 +

2273 + // For any messages that couldn't be fixed up, remove their line numbers since they point at lines

2274 + // that are possibly wrong after the conversion.

2275 + for (var i = 0; i < this.ConversionMessages.Count; ++i)

2276 + {

2277 + var message = this.ConversionMessages[i];

2278 +

2279 + if (!fixedupMessages.Contains(message))

2280 + {

2281 + this.ConversionMessages[i] = new Message(new SourceLineNumber(this.SourceFile), message.Level, message.Id, message.ResourceNameOrFormat, message.MessageArgs);

2282 + }

2283 + }

2284 + }

2285 +

2286 + foreach (var message in this.ConversionMessages)

2287 + {

2288 + this.Messaging.Write(message);

2289 + }

2290 +

2291 + return conversionCount;

2292 + }

2293 +

2294 + private IReadOnlyCollection<Message> FixupMessageLineNumbers(XObject obj, XPathNavigator savedDocument)

2295 + {

2296 + var messages = obj.Annotations<Message>().ToList();

2297 +

2298 + if (messages.Count == 0)

2299 + {

2300 + return Array.Empty<Message>();

2301 + }

2302 +

2303 + // We can't fix up line numbers based on attributes but we can fix up using their parent element, so do so.

2304 + if (obj is XAttribute attribute)

2305 + {

2306 + obj = attribute.Parent;

2307 + }

2308 +

2309 + var fixedupMessages = new List<Message>();

2310 +

2311 + var modifiedLineNumber = this.GetModifiedNodeLineNumber((XElement)obj, savedDocument);

2312 +

2313 + foreach (var message in messages)

2314 + {

2315 + var fixedupMessage = message;

2316 +

2317 + // If the line number wasn't modified, the existing message's source line nuber is correct

2318 + // so skip creating a new one.

2319 + if (modifiedLineNumber != null)

2320 + {

2321 + var index = this.ConversionMessages.IndexOf(message);

2322 +

2323 + fixedupMessage = new Message(modifiedLineNumber, message.Level, message.Id, message.ResourceNameOrFormat, message.MessageArgs);

2324 +

2325 + this.ConversionMessages[index] = fixedupMessage;

2326 + }

2327 +

2328 + fixedupMessages.Add(fixedupMessage);

2329 + }

2330 +

2331 + return fixedupMessages;

2332 + }

2333 +

2334 + private SourceLineNumber GetModifiedNodeLineNumber(XElement element, XPathNavigator savedDocument)

2335 + {

2336 + var xpathToOld = CalculateXPath(element);

2337 +

2338 + var newNode = savedDocument.SelectSingleNode(xpathToOld);

2339 + var newLineNumber = (newNode as IXmlLineInfo).LineNumber;

2340 +

2341 + var oldLineNumber = (element as IXmlLineInfo)?.LineNumber;

2342 + return (oldLineNumber != newLineNumber) ? new SourceLineNumber(this.SourceFile, newLineNumber) : null;

2343 + }

2344 +

2345 /// <summary>

2346 /// Output an error message to the console.

2347 /// </summary>

@@ -2250,10 +2378,8 @@ namespace WixToolset.Converters

2378 return false;

2379 }

2380

2253 - // Increase the message count.

2254 - this.Messages++;

2381 + var sourceLine = SourceLineNumberForXmlLineInfo(this.SourceFile, node);

2382

2256 - var sourceLine = (null == node) ? new SourceLineNumber(this.SourceFile ?? "wix.exe") : new SourceLineNumber(this.SourceFile, ((IXmlLineInfo)node).LineNumber);

2383 var prefix = String.Empty;

2384 if (level == MessageLevel.Information)

2385 {

@@ -2268,11 +2394,34 @@ namespace WixToolset.Converters

2394

2395 var msg = new Message(sourceLine, level, (int)converterTestType, format, args);

2396

2271 - this.Messaging.Write(msg);

2397 + // Add the message as a node annotation so it could be possible to remap source line numbers

2398 + // to their new locations after converting/formatting (since lines of code could be moved).

2399 + node?.AddAnnotation(msg);

2400 + this.ConversionMessages.Add(msg);

2401

2402 return true;

2403 }

2404

2405 + private static string CalculateXPath(XElement element)

2406 + {

2407 + var builder = new StringBuilder();

2408 +

2409 + while (element != null)

2410 + {

2411 + var index = element.Parent?.Elements().TakeWhile(e => e != element).Count() ?? 0;

2412 + builder.Insert(0, $"/*[{index + 1}]");

2413 +

2414 + element = element.Parent;

2415 + }

2416 +

2417 + return builder.ToString();

2418 + }

2419 +

2420 + private static SourceLineNumber SourceLineNumberForXmlLineInfo(string sourceFile, IXmlLineInfo lineInfo)

2421 + {

2422 + return (lineInfo?.HasLineInfo() == true) ? new SourceLineNumber(sourceFile, lineInfo.LineNumber) : new SourceLineNumber(sourceFile ?? "wix.exe");

2423 + }

2424 +

2425 /// <summary>

2426 /// Return an identifier based on passed file/directory name

2427 /// </summary>

@@ -2338,7 +2487,7 @@ namespace WixToolset.Converters

2487 comments = initialComments;

2488 var nonCommentNodes = new List<XNode>();

2489

2341 - foreach(var node in nodes)

2490 + foreach (var node in nodes)

2491 {

2492 if (XmlNodeType.Comment == node.NodeType)

2493 {

src/wix/test/WixToolsetTest.Converters/ConverterFixture.cs

+37 -1

@@ -3,6 +3,8 @@

3 namespace WixToolsetTest.Converters

4 {

5 using System;

6 + using System.IO;

7 + using System.Linq;

8 using System.Xml.Linq;

9 using WixBuildTools.TestSupport;

10 using WixToolset.Converters;

@@ -62,7 +64,7 @@ namespace WixToolsetTest.Converters

64 var document = XDocument.Parse(parse, LoadOptions.PreserveWhitespace | LoadOptions.SetLineInfo);

66 var messaging = new MockMessaging();

65 - var converter = new WixConverter(messaging, 2, ignoreErrors: new[] { "DeclarationPresent" } );

67 + var converter = new WixConverter(messaging, 2, ignoreErrors: new[] { "DeclarationPresent" });

69 var errors = converter.ConvertDocument(document);

@@ -102,6 +104,40 @@ namespace WixToolsetTest.Converters

104 WixAssert.CompareLineByLine(expected, actual);

105 }

106

107 + [Fact]

108 + public void CanConvertMainNamespaceFromDisk()

109 + {

110 + var dataFolder = TestData.Get("TestData", "FixDeclarationAndNamespace");

111 +

112 + var expected = new[]

113 + {

114 + "<Wix xmlns=\"http://wixtoolset.org/schemas/v4/wxs\">",

115 + " <Fragment />",

116 + "</Wix>",

117 + };

118 +

119 + using (var fs = new TestDataFolderFileSystem())

120 + {

121 + fs.Initialize(dataFolder);

122 + var path = Path.Combine(fs.BaseFolder, "FixDeclarationAndNamespace.wxs");

123 +

124 + var messaging = new MockMessaging();

125 + var converter = new WixConverter(messaging, 2, null, null);

126 +

127 + var errors = converter.ConvertFile(path, true);

128 +

129 + var messages = messaging.Messages.Select(m => $"{Path.GetFileName(m.SourceLineNumbers.FileName)}({m.SourceLineNumbers.LineNumber}) {m.ToString()}").ToArray();

130 + WixAssert.CompareLineByLine(new[]

131 + {

132 + "FixDeclarationAndNamespace.wxs() [Converted] This file contains an XML declaration on the first line. (DeclarationPresent)",

133 + "FixDeclarationAndNamespace.wxs(1) [Converted] The namespace 'http://schemas.microsoft.com/wix/2006/wi' is out of date. It must be 'http://wixtoolset.org/schemas/v4/wxs'. (XmlnsValueWrong)"

134 + }, messages);

135 +

136 + var actual = File.ReadAllLines(path);

137 + WixAssert.CompareLineByLine(expected, actual);

138 + }

139 + }

140 +

141 [Fact]

142 public void CanConvertNamedMainNamespace()

143 {

src/wix/test/WixToolsetTest.Converters/ConverterIntegrationFixture.cs

+8 -3

@@ -85,9 +85,14 @@ namespace WixToolsetTest.Converters

86 var messaging = new MockMessaging();

87 var converter = new WixConverter(messaging, 4);

88 - var errors = converter.ConvertFile(targetFile, true);

89 -

90 - Assert.Single(messaging.Messages.Where(m => m.Id == 5/*WixConverter.ConverterTestType.UnauthorizedAccessException*/));

88 + var convertedCount = converter.ConvertFile(targetFile, true);

89 +

90 + var errors = messaging.Messages.Where(m => m.Level == WixToolset.Data.MessageLevel.Error).Select(m => m.ToString()).ToArray();

91 + WixAssert.CompareLineByLine(new[]

92 + {

93 + "Could not write to file. (UnauthorizedAccessException)"

94 + }, errors);

95 + Assert.Equal(10, convertedCount);

96 }

97 }

src/wix/test/WixToolsetTest.Converters/TestData/FixDeclarationAndNamespace/FixDeclarationAndNamespace.wxs new

@@ -0,0 +1,4 @@

1 +<?xml version='1.0' encoding='utf-8'?>

2 +<Wix xmlns='http://schemas.microsoft.com/wix/2006/wi'>

3 + <Fragment />

4 +</Wix>