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>