Handle references to standard directories

Closes 7072

2 files changed, 136 insertions, 45 deletions

diff --git a/src/wix/WixToolset.Converters/WixConverter.cs b/src/wix/WixToolset.Converters/WixConverter.cs
index c7069e5f..61bd0f79 100644
--- a/src/wix/WixToolset.Converters/WixConverter.cs
+++ b/src/wix/WixToolset.Converters/WixConverter.cs
@@ -1094,40 +1094,18 @@ namespace WixToolset.Converters
 
             var id = element.Attribute("Id")?.Value;
 
-            if (id == "TARGETDIR" &&
-                this.OnInformation(ConverterTestType.TargetDirDeprecated, element, "The TARGETDIR directory should not longer be explicitly defined. Remove the Directory element with Id attribute 'TARGETDIR'."))
+            if (id == "TARGETDIR")
             {
-                var parentElement = element.Parent;
-
-                element.Remove();
-
-                if (parentElement.FirstNode is XText text && String.IsNullOrWhiteSpace(text.Value))
-                {
-                    parentElement.FirstNode.Remove();
-                }
-
-                foreach (var child in element.Nodes())
+                if (this.OnInformation(ConverterTestType.TargetDirDeprecated, element, "The TARGETDIR directory should no longer be explicitly defined. Remove the Directory element with Id attribute 'TARGETDIR'."))
                 {
-                    parentElement.Add(child);
-                }
-
-                element.RemoveAll();
-
-                if (parentElement.FirstNode is XText textAgain && String.IsNullOrWhiteSpace(textAgain.Value))
-                {
-                    parentElement.FirstNode.Remove();
+                    RemoveElementKeepChildren(element);
                 }
             }
             else if (id != null &&
                      WindowsInstallerStandard.IsStandardDirectory(id) &&
                      this.OnInformation(ConverterTestType.DefiningStandardDirectoryDeprecated, element, "Standard directories such as '{0}' should no longer be defined using the Directory element. Use the StandardDirectory element instead.", id))
             {
-                element.Name = StandardDirectoryElementName;
-
-                foreach (var attrib in element.Attributes().Where(a => a.Name.LocalName != "Id").ToList())
-                {
-                    attrib.Remove();
-                }
+                RenameElementToStandardDirectory(element);
             }
         }
 
@@ -1135,28 +1113,22 @@ namespace WixToolset.Converters
         {
             var id = element.Attribute("Id")?.Value;
 
-            if (id == "TARGETDIR" &&
-                this.OnInformation(ConverterTestType.TargetDirRefDeprecated, element, "The TARGETDIR directory should not longer be explicitly referenced. Remove the DirectoryRef element with Id attribute 'TARGETDIR'."))
+            if (id != null && WindowsInstallerStandard.IsStandardDirectory(id))
             {
-                var parentElement = element.Parent;
-
-                element.Remove();
-
-                if (parentElement.FirstNode is XText text && String.IsNullOrWhiteSpace(text.Value))
+                if (!element.HasElements)
                 {
-                    parentElement.FirstNode.Remove();
+                    this.OnError(ConverterTestType.EmptyStandardDirectoryRefNotConvertable, element, "Referencing '{0}' directory directly is no longer supported. The DirectoryRef will not be removed but you will probably need to reference a more specific directory.", id);
                 }
-
-                foreach (var child in element.Nodes())
+                else if (id == "TARGETDIR")
                 {
-                    parentElement.Add(child);
+                    if (this.OnInformation(ConverterTestType.StandardDirectoryRefDeprecated, element, "The {0} directory should no longer be explicitly referenced. Remove the DirectoryRef element with Id attribute '{0}'.", id))
+                    {
+                        RemoveElementKeepChildren(element);
+                    }
                 }
-
-                element.RemoveAll();
-
-                if (parentElement.FirstNode is XText textAgain && String.IsNullOrWhiteSpace(textAgain.Value))
+                else if (this.OnInformation(ConverterTestType.StandardDirectoryRefDeprecated, element, "The standard directory '{0}' should no longer be directly referenced. Use the StandardDirectory element instead.", id))
                 {
-                    parentElement.FirstNode.Remove();
+                    RenameElementToStandardDirectory(element);
                 }
             }
         }
@@ -2725,6 +2697,40 @@ namespace WixToolset.Converters
             return value;
         }
 
+        private static void RemoveElementKeepChildren(XElement element)
+        {
+            var parentElement = element.Parent;
+
+            element.Remove();
+
+            if (parentElement.FirstNode is XText text && String.IsNullOrWhiteSpace(text.Value))
+            {
+                parentElement.FirstNode.Remove();
+            }
+
+            foreach (var child in element.Nodes())
+            {
+                parentElement.Add(child);
+            }
+
+            element.RemoveAll();
+
+            if (parentElement.FirstNode is XText textAgain && String.IsNullOrWhiteSpace(textAgain.Value))
+            {
+                parentElement.FirstNode.Remove();
+            }
+        }
+
+        private static void RenameElementToStandardDirectory(XElement element)
+        {
+            element.Name = StandardDirectoryElementName;
+
+            foreach (var attrib in element.Attributes().Where(a => a.Name.LocalName != "Id").ToList())
+            {
+                attrib.Remove();
+            }
+        }
+
         private static bool TryGetInnerText(XElement element, out string value, out List<XNode> comments)
         {
             return TryGetInnerText(element, out value, out comments, new List<XNode>());
@@ -3138,7 +3144,7 @@ namespace WixToolset.Converters
             SoftwareTagTypeObsolete,
 
             /// <summary>
-            /// TARGETDIR directory should not longer be explicitly defined.
+            /// TARGETDIR directory should no longer be explicitly defined.
             /// </summary>
             TargetDirDeprecated,
 
@@ -3178,9 +3184,14 @@ namespace WixToolset.Converters
             CustomActionIdsIncludePlatformSuffix,
 
             /// <summary>
-            /// The TARGETDIR directory should not longer be explicitly referenced.
+            /// The {0} directory should no longer be explicitly referenced. Remove the DirectoryRef element with Id attribute '{0}'.
+            /// </summary>
+            StandardDirectoryRefDeprecated,
+
+            /// <summary>
+            /// Referencing '{0}' directory directly is no longer supported. The DirectoryRef will not be removed but you will probably need to reference a more specific directory.
             /// </summary>
-            TargetDirRefDeprecated,
+            EmptyStandardDirectoryRefNotConvertable,
         }
     }
 }
diff --git a/src/wix/test/WixToolsetTest.Converters/DirectoryFixture.cs b/src/wix/test/WixToolsetTest.Converters/DirectoryFixture.cs
index 6f233509..e992bf30 100644
--- a/src/wix/test/WixToolsetTest.Converters/DirectoryFixture.cs
+++ b/src/wix/test/WixToolsetTest.Converters/DirectoryFixture.cs
@@ -3,6 +3,7 @@
 namespace WixToolsetTest.Converters
 {
     using System;
+    using System.Linq;
     using System.Xml.Linq;
     using WixInternal.TestSupport;
     using WixToolset.Converters;
@@ -130,6 +131,42 @@ namespace WixToolsetTest.Converters
         }
 
         [Fact]
+        public void FixStandardDirectoryRef()
+        {
+            var parse = String.Join(Environment.NewLine,
+                "<?xml version=\"1.0\" encoding=\"utf-16\"?>",
+                "<Wix xmlns='http://schemas.microsoft.com/wix/2006/wi'>",
+                "  <Fragment>",
+                "    <DirectoryRef Id='ProgramFilesFolder'>",
+                "      <Directory Id='ChildFolder' Name='Child' />",
+                "    </DirectoryRef>",
+                "  </Fragment>",
+                "</Wix>");
+
+            var expected = new[]
+            {
+                "<Wix xmlns=\"http://wixtoolset.org/schemas/v4/wxs\">",
+                "  <Fragment>",
+                "    <StandardDirectory Id=\"ProgramFilesFolder\">",
+                "      <Directory Id=\"ChildFolder\" Name=\"Child\" />",
+                "    </StandardDirectory>",
+                "  </Fragment>",
+                "</Wix>"
+            };
+
+            var document = XDocument.Parse(parse, LoadOptions.PreserveWhitespace | LoadOptions.SetLineInfo);
+
+            var messaging = new MockMessaging();
+            var converter = new WixConverter(messaging, 2, null, null);
+
+            var errors = converter.ConvertDocument(document);
+
+            var actualLines = UnformattedDocumentLines(document);
+            WixAssert.CompareLineByLine(expected, actualLines);
+            Assert.Equal(3, errors);
+        }
+
+        [Fact]
         public void RemoveTargetDirRefAndFixStandardDirectory()
         {
             var parse = String.Join(Environment.NewLine,
@@ -166,5 +203,48 @@ namespace WixToolsetTest.Converters
             WixAssert.CompareLineByLine(expected, actualLines);
             Assert.Equal(4, errors);
         }
+
+        [Fact]
+        public void ErrorOnEmptyStandardDirectoryRef()
+        {
+            var parse = String.Join(Environment.NewLine,
+                "<Wix xmlns='http://schemas.microsoft.com/wix/2006/wi'>",
+                "  <Fragment>",
+                "    <DirectoryRef Id='TARGETDIR' />",
+                "    <DirectoryRef Id='ProgramFilesFolder' />",
+                "    <DirectoryRef Id='DesktopFolder' />",
+                "  </Fragment>",
+                "</Wix>");
+
+            var expected = new[]
+            {
+                "<Wix xmlns=\"http://wixtoolset.org/schemas/v4/wxs\">",
+                "  <Fragment>",
+                "    <DirectoryRef Id=\"TARGETDIR\" />",
+                "    <DirectoryRef Id=\"ProgramFilesFolder\" />",
+                "    <DirectoryRef Id=\"DesktopFolder\" />",
+                "  </Fragment>",
+                "</Wix>"
+            };
+
+            var document = XDocument.Parse(parse, LoadOptions.PreserveWhitespace | LoadOptions.SetLineInfo);
+
+            var messaging = new MockMessaging();
+            var converter = new WixConverter(messaging, 2, null, null);
+
+            var errors = converter.ConvertDocument(document);
+
+            WixAssert.CompareLineByLine(new[]
+            {
+                "[Converted] The namespace 'http://schemas.microsoft.com/wix/2006/wi' is out of date. It must be 'http://wixtoolset.org/schemas/v4/wxs'. (XmlnsValueWrong)",
+                "Referencing 'TARGETDIR' directory directly is no longer supported. The DirectoryRef will not be removed but you will probably need to reference a more specific directory. (EmptyStandardDirectoryRefNotConvertable)",
+                "Referencing 'ProgramFilesFolder' directory directly is no longer supported. The DirectoryRef will not be removed but you will probably need to reference a more specific directory. (EmptyStandardDirectoryRefNotConvertable)",
+                "Referencing 'DesktopFolder' directory directly is no longer supported. The DirectoryRef will not be removed but you will probably need to reference a more specific directory. (EmptyStandardDirectoryRefNotConvertable)"
+            }, messaging.Messages.Select(m => m.ToString()).ToArray());
+
+            var actualLines = UnformattedDocumentLines(document);
+            WixAssert.CompareLineByLine(expected, actualLines);
+            Assert.Equal(4, errors);
+        }
     }
 }