Reduce duplicative messaging in converter

Fixes 6681

2 files changed, 27 insertions, 35 deletions

diff --git a/src/wix/WixToolset.Converters/WixConverter.cs b/src/wix/WixToolset.Converters/WixConverter.cs
index 183a7394..c009042d 100644
--- a/src/wix/WixToolset.Converters/WixConverter.cs
+++ b/src/wix/WixToolset.Converters/WixConverter.cs
@@ -15,7 +15,6 @@ namespace WixToolset.Converters
     using WixToolset.Data.WindowsInstaller;
     using WixToolset.Extensibility.Services;
 
-#pragma warning disable 1591 // TODO: add documentation
     /// <summary>
     /// How to convert CustomTable elements.
     /// </summary>
@@ -295,7 +294,7 @@ namespace WixToolset.Converters
 
         private int SourceVersion { get; set; }
 
-        public XElement XRoot { get; private set; }
+        private XElement XRoot { get; set; }
 
         /// <summary>
         /// Convert a file.
@@ -2085,31 +2084,7 @@ namespace WixToolset.Converters
         /// <returns>Returns true indicating that action should be taken on this error, and false if it should be ignored.</returns>
         private bool OnError(ConverterTestType converterTestType, XObject node, string message, params object[] args)
         {
-            // Ignore the error if explicitly ignored or outside the range of the current operation.
-            if (this.IgnoreErrors.Contains(converterTestType) ||
-                (this.Operation == ConvertOperation.Convert && converterTestType < ConverterTestType.EndIgnoreInConvert) ||
-                (this.Operation == ConvertOperation.Format && converterTestType > ConverterTestType.BeginIgnoreInFormat))
-            {
-                return false;
-            }
-
-            // Increase the message count.
-            this.Messages++;
-
-            var sourceLine = (null == node) ? new SourceLineNumber(this.SourceFile ?? "wix.exe") : new SourceLineNumber(this.SourceFile, ((IXmlLineInfo)node).LineNumber);
-            var warning = this.ErrorsAsWarnings.Contains(converterTestType);
-            
-            var msg = new Message(
-                    sourceLine,
-                    warning ? MessageLevel.Warning : MessageLevel.Error,
-                    (int)converterTestType,
-                    "{0} ({1})",
-                    String.Format(CultureInfo.CurrentCulture, message, args),
-                    converterTestType.ToString());
-
-            this.Messaging.Write(msg);
-
-            return true;
+            return this.OnMessage(MessageLevel.Error, converterTestType, node, message, args);
         }
 
         /// <summary>
@@ -2119,9 +2094,14 @@ namespace WixToolset.Converters
         /// <param name="node">The node that caused the error.</param>
         /// <param name="message">Detailed error message.</param>
         /// <param name="args">Additional formatted string arguments.</param>
-        /// <returns>Returns true indicating that action should be taken on this error, and false if it should be ignored.</returns>
+        /// <returns>Returns true indicating that action should be taken on this message, and false if it should be ignored.</returns>
         private bool OnInformation(ConverterTestType converterTestType, XObject node, string message, params object[] args)
         {
+            return this.OnMessage(MessageLevel.Information, converterTestType, node, message, args);
+        }
+
+        private bool OnMessage(MessageLevel level, ConverterTestType converterTestType, XObject node, string message, params object[] args)
+        {
             // Ignore the error if explicitly ignored or outside the range of the current operation.
             if (this.IgnoreErrors.Contains(converterTestType) ||
                 (this.Operation == ConvertOperation.Convert && converterTestType < ConverterTestType.EndIgnoreInConvert) ||
@@ -2134,14 +2114,19 @@ namespace WixToolset.Converters
             this.Messages++;
 
             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)
+            {
+                prefix = "[Converted] ";
+            }
+            else if (level == MessageLevel.Error && this.ErrorsAsWarnings.Contains(converterTestType))
+            {
+                level = MessageLevel.Warning;
+            }
+
+            var format = prefix + message + $" ({converterTestType})";
 
-            var msg = new Message(
-                    sourceLine,
-                    MessageLevel.Information,
-                    (int)converterTestType,
-                    "[Converted] {0} ({1})",
-                    String.Format(CultureInfo.CurrentCulture, message, args),
-                    converterTestType.ToString());
+            var msg = new Message(sourceLine, level, (int)converterTestType, format, args);
 
             this.Messaging.Write(msg);
 
diff --git a/src/wix/test/WixToolsetTest.Converters/ExePackageFixture.cs b/src/wix/test/WixToolsetTest.Converters/ExePackageFixture.cs
index 0ee8d065..68d234ad 100644
--- a/src/wix/test/WixToolsetTest.Converters/ExePackageFixture.cs
+++ b/src/wix/test/WixToolsetTest.Converters/ExePackageFixture.cs
@@ -3,6 +3,7 @@
 namespace WixToolsetTest.Converters
 {
     using System;
+    using System.Linq;
     using System.Xml.Linq;
     using WixBuildTools.TestSupport;
     using WixToolset.Converters;
@@ -43,6 +44,12 @@ namespace WixToolsetTest.Converters
             Assert.Equal(3, errors);
 
             var actualLines = UnformattedDocumentLines(document);
+            WixAssert.CompareLineByLine(new[]
+            {
+                "[Converted] The ExePackage element InstallCommand attribute has been renamed InstallArguments. (RenameExePackageCommandToArguments)",
+                "[Converted] The ExePackage element RepairCommand attribute has been renamed RepairArguments. (RenameExePackageCommandToArguments)",
+                "[Converted] The ExePackage element UninstallCommand attribute has been renamed UninstallArguments. (RenameExePackageCommandToArguments)",
+            }, messaging.Messages.Select(m => m.ToString()).ToArray());
             WixAssert.CompareLineByLine(expected, actualLines);
         }
     }