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

5 files changed, 272 insertions, 61 deletions

diff --git a/src/wix/WixToolset.Converters/HashSetExtensions.cs b/src/wix/WixToolset.Converters/HashSetExtensions.cs
new file mode 100644
index 00000000..49b0d4ac
--- /dev/null
+++ b/src/wix/WixToolset.Converters/HashSetExtensions.cs
@@ -0,0 +1,17 @@
+// Copyright (c) .NET Foundation and contributors. All rights reserved. Licensed under the Microsoft Reciprocal License. See LICENSE.TXT file in the project root for full license information.
+
+namespace WixToolset.Converters
+{
+    using System.Collections.Generic;
+
+    internal static class HashSetExtensions
+    {
+        public static void AddRange<T>(this HashSet<T> set, IEnumerable<T> values)
+        {
+            foreach (var value in values)
+            {
+                set.Add(value);
+            }
+        }
+    }
+}
diff --git a/src/wix/WixToolset.Converters/WixConverter.cs b/src/wix/WixToolset.Converters/WixConverter.cs
index de81c876..cf77681c 100644
--- a/src/wix/WixToolset.Converters/WixConverter.cs
+++ b/src/wix/WixToolset.Converters/WixConverter.cs
@@ -11,6 +11,7 @@ namespace WixToolset.Converters
     using System.Text.RegularExpressions;
     using System.Xml;
     using System.Xml.Linq;
+    using System.Xml.XPath;
     using WixToolset.Data;
     using WixToolset.Data.WindowsInstaller;
     using WixToolset.Extensibility.Services;
@@ -272,6 +273,8 @@ namespace WixToolset.Converters
                 { WixConverter.WixLocalizationElementWithoutNamespaceName, this.ConvertWixLocalizationElementWithoutNamespace },
             };
 
+            this.ConversionMessages = new List<Message>();
+
             this.Messaging = messaging;
 
             this.IndentationAmount = indentationAmount;
@@ -285,7 +288,7 @@ namespace WixToolset.Converters
 
         private CustomTableTarget CustomTableSetting { get; }
 
-        private int Messages { get; set; }
+        private List<Message> ConversionMessages { get; }
 
         private HashSet<ConverterTestType> ErrorsAsWarnings { get; set; }
 
@@ -308,54 +311,35 @@ namespace WixToolset.Converters
         /// </summary>
         /// <param name="sourceFile">The file to convert.</param>
         /// <param name="saveConvertedFile">Option to save the converted Messages that are found.</param>
-        /// <returns>The number of Messages found.</returns>
+        /// <returns>The number of conversions found.</returns>
         public int ConvertFile(string sourceFile, bool saveConvertedFile)
         {
-            var document = this.OpenSourceFile(sourceFile);
+            var savedDocument = false;
 
-            if (document is null)
+            if (this.TryOpenSourceFile(sourceFile, out var document))
             {
-                return 1;
-            }
-
-            this.ConvertDocument(document);
+                this.Convert(document);
 
-            // Fix Messages if requested and necessary.
-            if (saveConvertedFile && 0 < this.Messages)
-            {
-                this.SaveDocument(document);
+                // Fix Messages if requested and necessary.
+                if (saveConvertedFile && 0 < this.ConversionMessages.Count)
+                {
+                    savedDocument = this.SaveDocument(document);
+                }
             }
 
-            return this.Messages;
+            return this.ReportMessages(document, savedDocument);
         }
 
         /// <summary>
         /// Convert a document.
         /// </summary>
         /// <param name="document">The document to convert.</param>
-        /// <returns>The number of Messages found.</returns>
+        /// <returns>The number of conversions found.</returns>
         public int ConvertDocument(XDocument document)
         {
-            // Reset the instance info.
-            this.Messages = 0;
-            this.SourceVersion = 0;
-            this.Operation = ConvertOperation.Convert;
-
-            // Remove the declaration.
-            if (null != document.Declaration
-                && this.OnInformation(ConverterTestType.DeclarationPresent, null, "This file contains an XML declaration on the first line."))
-            {
-                document.Declaration = null;
-                TrimLeadingText(document);
-            }
-
-            this.XRoot = document.Root;
-
-            // Start converting the nodes at the top.
-            this.ConvertNodes(document.Nodes(), 0);
-            this.RemoveUnusedNamespaces(document.Root);
+            this.Convert(document);
 
-            return this.Messages;
+            return this.ReportMessages(document, false);
         }
 
         /// <summary>
@@ -366,22 +350,20 @@ namespace WixToolset.Converters
         /// <returns>The number of Messages found.</returns>
         public int FormatFile(string sourceFile, bool saveConvertedFile)
         {
-            var document = this.OpenSourceFile(sourceFile);
+            var savedDocument = false;
 
-            if (document is null)
+            if (this.TryOpenSourceFile(sourceFile, out var document))
             {
-                return 1;
-            }
-
-            this.FormatDocument(document);
+                this.FormatDocument(document);
 
-            // Fix Messages if requested and necessary.
-            if (saveConvertedFile && 0 < this.Messages)
-            {
-                this.SaveDocument(document);
+                // Fix Messages if requested and necessary.
+                if (saveConvertedFile && 0 < this.ConversionMessages.Count)
+                {
+                    savedDocument = this.SaveDocument(document);
+                }
             }
 
-            return this.Messages;
+            return this.ReportMessages(document, savedDocument);
         }
 
         /// <summary>
@@ -391,43 +373,73 @@ namespace WixToolset.Converters
         /// <returns>The number of Messages found.</returns>
         public int FormatDocument(XDocument document)
         {
+            this.Format(document);
+
+            return this.ReportMessages(document, false);
+        }
+
+        private void Convert(XDocument document)
+        {
             // Reset the instance info.
-            this.Messages = 0;
+            this.ConversionMessages.Clear();
             this.SourceVersion = 0;
-            this.Operation = ConvertOperation.Format;
+            this.Operation = ConvertOperation.Convert;
 
             // Remove the declaration.
             if (null != document.Declaration
-                && this.OnInformation(ConverterTestType.DeclarationPresent, null, "This file contains an XML declaration on the first line."))
+                && this.OnInformation(ConverterTestType.DeclarationPresent, document, "This file contains an XML declaration on the first line."))
             {
                 document.Declaration = null;
                 TrimLeadingText(document);
             }
 
+            this.XRoot = document.Root;
+
             // Start converting the nodes at the top.
             this.ConvertNodes(document.Nodes(), 0);
             this.RemoveUnusedNamespaces(document.Root);
+        }
+
+        private void Format(XDocument document)
+        {
+            // Reset the instance info.
+            this.ConversionMessages.Clear();
+            this.SourceVersion = 0;
+            this.Operation = ConvertOperation.Format;
 
-            return this.Messages;
+            // Remove the declaration.
+            if (null != document.Declaration
+                && this.OnInformation(ConverterTestType.DeclarationPresent, document, "This file contains an XML declaration on the first line."))
+            {
+                document.Declaration = null;
+                TrimLeadingText(document);
+            }
+
+            this.XRoot = document.Root;
+
+            // Start converting the nodes at the top.
+            this.ConvertNodes(document.Nodes(), 0);
+            this.RemoveUnusedNamespaces(document.Root);
         }
 
-        private XDocument OpenSourceFile(string sourceFile)
+        private bool TryOpenSourceFile(string sourceFile, out XDocument document)
         {
             this.SourceFile = sourceFile;
 
             try
             {
-                return XDocument.Load(this.SourceFile, LoadOptions.PreserveWhitespace | LoadOptions.SetLineInfo);
+                document = XDocument.Load(this.SourceFile, LoadOptions.PreserveWhitespace | LoadOptions.SetLineInfo);
             }
             catch (XmlException e)
             {
                 this.OnError(ConverterTestType.XmlException, null, "The xml is invalid.  Detail: '{0}'", e.Message);
+                document = null;
             }
 
-            return null;
+            return document != null;
         }
 
-        private void SaveDocument(XDocument document)
+        private bool SaveDocument(XDocument document)
         {
             var ignoreDeclarationError = this.IgnoreErrors.Contains(ConverterTestType.DeclarationPresent);
 
@@ -437,11 +449,15 @@ namespace WixToolset.Converters
                 {
                     document.Save(writer);
                 }
+
+                return true;
             }
             catch (UnauthorizedAccessException)
             {
                 this.OnError(ConverterTestType.UnauthorizedAccessException, null, "Could not write to file.");
             }
+
+            return false;
         }
 
         private void ConvertNodes(IEnumerable<XNode> nodes, int level)
@@ -2129,6 +2145,14 @@ namespace WixToolset.Converters
                         convertedAttribute = new XAttribute(ns.GetName(attribute.Name.LocalName), attribute.Value);
                     }
 
+                    if (convertedAttribute != attribute)
+                    {
+                        foreach (var message in attribute.Annotations<Message>())
+                        {
+                            convertedAttribute.AddAnnotation(message);
+                        }
+                    }
+
                     element.Add(convertedAttribute);
                 }
             }
@@ -2214,6 +2238,110 @@ namespace WixToolset.Converters
             }
         }
 
+        private int ReportMessages(XDocument document, bool saved)
+        {
+            var conversionCount = this.ConversionMessages.Count;
+
+            // If the converted/formatted document was saved, update the source line numbers
+            // in the messages since they will possibly be wrong.
+            if (saved && conversionCount > 0)
+            {
+                var fixedupMessages = new HashSet<Message>();
+
+                // Load the converted document so we can look up new line numbers for
+                // messages.
+                var convertedDocument = XDocument.Load(this.SourceFile, LoadOptions.PreserveWhitespace | LoadOptions.SetLineInfo);
+                var convertedNavigator = convertedDocument.CreateNavigator();
+
+                // Look through all nodes in the document and try to fix up their line numbers.
+                foreach (var elements in document.Descendants())
+                {
+                    var fixedup = this.FixupMessageLineNumbers(elements, convertedNavigator);
+                    fixedupMessages.AddRange(fixedup);
+
+                    // Attributes are not considered nodes so they must be enumerated independently.
+                    if (elements is XElement element)
+                    {
+                        foreach (var attribute in element.Attributes())
+                        {
+                            fixedup = this.FixupMessageLineNumbers(attribute, convertedNavigator);
+                            fixedupMessages.AddRange(fixedup);
+                        }
+                    }
+                }
+
+                // For any messages that couldn't be fixed up, remove their line numbers since they point at lines
+                // that are possibly wrong after the conversion.
+                for (var i = 0; i < this.ConversionMessages.Count; ++i)
+                {
+                    var message = this.ConversionMessages[i];
+
+                    if (!fixedupMessages.Contains(message))
+                    {
+                        this.ConversionMessages[i] = new Message(new SourceLineNumber(this.SourceFile), message.Level, message.Id, message.ResourceNameOrFormat, message.MessageArgs);
+                    }
+                }
+            }
+
+            foreach (var message in this.ConversionMessages)
+            {
+                this.Messaging.Write(message);
+            }
+
+            return conversionCount;
+        }
+
+        private IReadOnlyCollection<Message> FixupMessageLineNumbers(XObject obj, XPathNavigator savedDocument)
+        {
+            var messages = obj.Annotations<Message>().ToList();
+
+            if (messages.Count == 0)
+            {
+                return Array.Empty<Message>();
+            }
+
+            // We can't fix up line numbers based on attributes but we can fix up using their parent element, so do so.
+            if (obj is XAttribute attribute)
+            {
+                obj = attribute.Parent;
+            }
+
+            var fixedupMessages = new List<Message>();
+
+            var modifiedLineNumber = this.GetModifiedNodeLineNumber((XElement)obj, savedDocument);
+
+            foreach (var message in messages)
+            {
+                var fixedupMessage = message;
+
+                // If the line number wasn't modified, the existing message's source line nuber is correct
+                // so skip creating a new one.
+                if (modifiedLineNumber != null)
+                {
+                    var index = this.ConversionMessages.IndexOf(message);
+
+                    fixedupMessage = new Message(modifiedLineNumber, message.Level, message.Id, message.ResourceNameOrFormat, message.MessageArgs);
+
+                    this.ConversionMessages[index] = fixedupMessage;
+                }
+
+                fixedupMessages.Add(fixedupMessage);
+            }
+
+            return fixedupMessages;
+        }
+
+        private SourceLineNumber GetModifiedNodeLineNumber(XElement element, XPathNavigator savedDocument)
+        {
+            var xpathToOld = CalculateXPath(element);
+
+            var newNode = savedDocument.SelectSingleNode(xpathToOld);
+            var newLineNumber = (newNode as IXmlLineInfo).LineNumber;
+
+            var oldLineNumber = (element as IXmlLineInfo)?.LineNumber;
+            return (oldLineNumber != newLineNumber) ? new SourceLineNumber(this.SourceFile, newLineNumber) : null;
+        }
+
         /// <summary>
         /// Output an error message to the console.
         /// </summary>
@@ -2250,10 +2378,8 @@ namespace WixToolset.Converters
                 return false;
             }
 
-            // Increase the message count.
-            this.Messages++;
+            var sourceLine = SourceLineNumberForXmlLineInfo(this.SourceFile, node);
 
-            var sourceLine = (null == node) ? new SourceLineNumber(this.SourceFile ?? "wix.exe") : new SourceLineNumber(this.SourceFile, ((IXmlLineInfo)node).LineNumber);
             var prefix = String.Empty;
             if (level == MessageLevel.Information)
             {
@@ -2268,11 +2394,34 @@ namespace WixToolset.Converters
 
             var msg = new Message(sourceLine, level, (int)converterTestType, format, args);
 
-            this.Messaging.Write(msg);
+            // Add the message as a node annotation so it could be possible to remap source line numbers
+            // to their new locations after converting/formatting (since lines of code could be moved).
+            node?.AddAnnotation(msg);
+            this.ConversionMessages.Add(msg);
 
             return true;
         }
 
+        private static string CalculateXPath(XElement element)
+        {
+            var builder = new StringBuilder();
+
+            while (element != null)
+            {
+                var index = element.Parent?.Elements().TakeWhile(e => e != element).Count() ?? 0;
+                builder.Insert(0, $"/*[{index + 1}]");
+
+                element = element.Parent;
+            }
+
+            return builder.ToString();
+        }
+
+        private static SourceLineNumber SourceLineNumberForXmlLineInfo(string sourceFile, IXmlLineInfo lineInfo)
+        {
+            return (lineInfo?.HasLineInfo() == true) ? new SourceLineNumber(sourceFile, lineInfo.LineNumber) : new SourceLineNumber(sourceFile ?? "wix.exe");
+        }
+
         /// <summary>
         /// Return an identifier based on passed file/directory name
         /// </summary>
@@ -2338,7 +2487,7 @@ namespace WixToolset.Converters
             comments = initialComments;
             var nonCommentNodes = new List<XNode>();
 
-            foreach(var node in nodes)
+            foreach (var node in nodes)
             {
                 if (XmlNodeType.Comment == node.NodeType)
                 {
diff --git a/src/wix/test/WixToolsetTest.Converters/ConverterFixture.cs b/src/wix/test/WixToolsetTest.Converters/ConverterFixture.cs
index d186931b..cd6849d3 100644
--- a/src/wix/test/WixToolsetTest.Converters/ConverterFixture.cs
+++ b/src/wix/test/WixToolsetTest.Converters/ConverterFixture.cs
@@ -3,6 +3,8 @@
 namespace WixToolsetTest.Converters
 {
     using System;
+    using System.IO;
+    using System.Linq;
     using System.Xml.Linq;
     using WixBuildTools.TestSupport;
     using WixToolset.Converters;
@@ -62,7 +64,7 @@ namespace WixToolsetTest.Converters
             var document = XDocument.Parse(parse, LoadOptions.PreserveWhitespace | LoadOptions.SetLineInfo);
 
             var messaging = new MockMessaging();
-            var converter = new WixConverter(messaging, 2, ignoreErrors: new[] { "DeclarationPresent" } );
+            var converter = new WixConverter(messaging, 2, ignoreErrors: new[] { "DeclarationPresent" });
 
             var errors = converter.ConvertDocument(document);
 
@@ -103,6 +105,40 @@ namespace WixToolsetTest.Converters
         }
 
         [Fact]
+        public void CanConvertMainNamespaceFromDisk()
+        {
+            var dataFolder = TestData.Get("TestData", "FixDeclarationAndNamespace");
+
+            var expected = new[]
+            {
+                "<Wix xmlns=\"http://wixtoolset.org/schemas/v4/wxs\">",
+                "  <Fragment />",
+                "</Wix>",
+            };
+
+            using (var fs = new TestDataFolderFileSystem())
+            {
+                fs.Initialize(dataFolder);
+                var path = Path.Combine(fs.BaseFolder, "FixDeclarationAndNamespace.wxs");
+
+                var messaging = new MockMessaging();
+                var converter = new WixConverter(messaging, 2, null, null);
+
+                var errors = converter.ConvertFile(path, true);
+
+                var messages = messaging.Messages.Select(m => $"{Path.GetFileName(m.SourceLineNumbers.FileName)}({m.SourceLineNumbers.LineNumber}) {m.ToString()}").ToArray();
+                WixAssert.CompareLineByLine(new[]
+                {
+                    "FixDeclarationAndNamespace.wxs() [Converted] This file contains an XML declaration on the first line. (DeclarationPresent)",
+                    "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)"
+                }, messages);
+
+                var actual = File.ReadAllLines(path);
+                WixAssert.CompareLineByLine(expected, actual);
+            }
+        }
+
+        [Fact]
         public void CanConvertNamedMainNamespace()
         {
             var parse = String.Join(Environment.NewLine,
diff --git a/src/wix/test/WixToolsetTest.Converters/ConverterIntegrationFixture.cs b/src/wix/test/WixToolsetTest.Converters/ConverterIntegrationFixture.cs
index a8908df7..fdebc6b0 100644
--- a/src/wix/test/WixToolsetTest.Converters/ConverterIntegrationFixture.cs
+++ b/src/wix/test/WixToolsetTest.Converters/ConverterIntegrationFixture.cs
@@ -85,9 +85,14 @@ namespace WixToolsetTest.Converters
 
                 var messaging = new MockMessaging();
                 var converter = new WixConverter(messaging, 4);
-                var errors = converter.ConvertFile(targetFile, true);
-
-                Assert.Single(messaging.Messages.Where(m => m.Id == 5/*WixConverter.ConverterTestType.UnauthorizedAccessException*/));
+                var convertedCount = converter.ConvertFile(targetFile, true);
+
+                var errors = messaging.Messages.Where(m => m.Level == WixToolset.Data.MessageLevel.Error).Select(m => m.ToString()).ToArray();
+                WixAssert.CompareLineByLine(new[]
+                {
+                    "Could not write to file. (UnauthorizedAccessException)"
+                }, errors);
+                Assert.Equal(10, convertedCount);
             }
         }
 
diff --git a/src/wix/test/WixToolsetTest.Converters/TestData/FixDeclarationAndNamespace/FixDeclarationAndNamespace.wxs b/src/wix/test/WixToolsetTest.Converters/TestData/FixDeclarationAndNamespace/FixDeclarationAndNamespace.wxs
new file mode 100644
index 00000000..a8923337
--- /dev/null
+++ b/src/wix/test/WixToolsetTest.Converters/TestData/FixDeclarationAndNamespace/FixDeclarationAndNamespace.wxs
@@ -0,0 +1,4 @@
+<?xml version='1.0' encoding='utf-8'?>
+<Wix xmlns='http://schemas.microsoft.com/wix/2006/wi'>
+  <Fragment />
+</Wix>