Support preserving formatting when opening a project

src/XMakeBuildEngine/Construction/ProjectRootElement.cs

@@ -58,6 +58,8 @@ public class ProjectRootElement : ProjectElementContainer
         /// </summary>
         private static readonly ProjectRootElementCache.OpenProjectRootElement s_openLoaderDelegate = OpenLoader;
 
+        private static readonly ProjectRootElementCache.OpenProjectRootElement s_openLoaderPreserveWhitespaceDelegate = OpenLoaderPreserveWhitespace;
+
         /// <summary>
         /// The default encoding to use / assume for a new project.
         /// </summary>
@@ -200,7 +202,8 @@ private ProjectRootElement(ProjectRootElementCache projectRootElementCache)
         /// Assumes path is already normalized.
         /// May throw InvalidProjectFileException.
         /// </summary>
-        private ProjectRootElement(string path, ProjectRootElementCache projectRootElementCache, BuildEventContext buildEventContext)
+        private ProjectRootElement(string path, ProjectRootElementCache projectRootElementCache, BuildEventContext buildEventContext,
+            bool preserveWhitespace)
             : base()
         {
             ErrorUtilities.VerifyThrowArgumentLength(path, "path");
@@ -214,7 +217,7 @@ private ProjectRootElement(string path, ProjectRootElementCache projectRootEleme
             _versionOnDisk = _version;
             _timeLastChangedUtc = DateTime.UtcNow;
 
-            XmlDocumentWithLocation document = LoadDocument(path);
+            XmlDocumentWithLocation document = LoadDocument(path, preserveWhitespace);
 
             ProjectParser.Parse(document, this);
 
@@ -995,13 +998,19 @@ public static ProjectRootElement Open(string path)
         /// May throw InvalidProjectFileException.
         /// </summary>
         public static ProjectRootElement Open(string path, ProjectCollection projectCollection)
+        {
+            return Open(path, projectCollection,
+                preserveWhitespace: false);
+        }
+
+        public static ProjectRootElement Open(string path, ProjectCollection projectCollection, bool preserveWhitespace)
         {
             ErrorUtilities.VerifyThrowArgumentLength(path, "path");
             ErrorUtilities.VerifyThrowArgumentNull(projectCollection, "projectCollection");
 
             path = FileUtilities.NormalizePath(path);
 
-            return Open(path, projectCollection.ProjectRootElementCache, true /*Is explicitly loaded*/);
+            return Open(path, projectCollection.ProjectRootElementCache, true /*Is explicitly loaded*/, preserveWhitespace);
         }
 
         /// <summary>
@@ -1735,11 +1744,14 @@ internal static ProjectRootElement Create(ProjectRootElementCache projectRootEle
         /// Uses the specified project root element cache.
         /// May throw InvalidProjectFileException.
         /// </summary>
-        internal static ProjectRootElement Open(string path, ProjectRootElementCache projectRootElementCache, bool isExplicitlyLoaded)
+        internal static ProjectRootElement Open(string path, ProjectRootElementCache projectRootElementCache, bool isExplicitlyLoaded,
+            bool preserveWhitespace)
         {
             ErrorUtilities.VerifyThrowInternalRooted(path);
 
-            ProjectRootElement projectRootElement = projectRootElementCache.Get(path, s_openLoaderDelegate, isExplicitlyLoaded);
+            ProjectRootElement projectRootElement = projectRootElementCache.Get(path,
+                preserveWhitespace ? s_openLoaderPreserveWhitespaceDelegate : s_openLoaderDelegate,
+                isExplicitlyLoaded);
 
             return projectRootElement;
         }
@@ -1771,7 +1783,8 @@ internal static ProjectRootElement OpenProjectOrSolution(string fullPath, IDicti
 
             ProjectRootElement projectRootElement = projectRootElementCache.Get(
                 fullPath,
-                (path, cache) => CreateProjectFromPath(path, globalProperties, toolsVersion, loggingService, cache, buildEventContext),
+                (path, cache) => CreateProjectFromPath(path, globalProperties, toolsVersion, loggingService, cache, buildEventContext,
+                                    preserveWhitespace: false),
                 isExplicitlyLoaded);
 
             return projectRootElement;
@@ -1869,11 +1882,24 @@ protected override ProjectElement CreateNewInstance(ProjectRootElement owner)
         /// <param name="path">The path to the file to load.</param>
         /// <param name="projectRootElementCache">The cache to load the PRE into.</param>
         private static ProjectRootElement OpenLoader(string path, ProjectRootElementCache projectRootElementCache)
+        {
+            return OpenLoader(path, projectRootElementCache,
+                preserveWhitespace: false);
+        }
+
+        private static ProjectRootElement OpenLoaderPreserveWhitespace(string path, ProjectRootElementCache projectRootElementCache)
+        {
+            return OpenLoader(path, projectRootElementCache,
+                preserveWhitespace: true);
+        }
+
+        private static ProjectRootElement OpenLoader(string path, ProjectRootElementCache projectRootElementCache, bool preserveWhitespace)
         {
             return new ProjectRootElement(
                 path,
                 projectRootElementCache,
-                new BuildEventContext(0, BuildEventContext.InvalidNodeId, BuildEventContext.InvalidProjectContextId, BuildEventContext.InvalidTaskId));
+                new BuildEventContext(0, BuildEventContext.InvalidNodeId, BuildEventContext.InvalidProjectContextId, BuildEventContext.InvalidTaskId),
+                preserveWhitespace);
         }
 
         /// <summary>
@@ -1890,7 +1916,8 @@ private static ProjectRootElement CreateProjectFromPath
                 string toolsVersion,
                 ILoggingService loggingService,
                 ProjectRootElementCache projectRootElementCache,
-                BuildEventContext buildEventContext
+                BuildEventContext buildEventContext,
+                bool preserveWhitespace
             )
         {
             ErrorUtilities.VerifyThrowInternalRooted(projectFile);
@@ -1903,7 +1930,7 @@ BuildEventContext buildEventContext
                 }
 
                 // OK it's a regular project file, load it normally.
-                return new ProjectRootElement(projectFile, projectRootElementCache, buildEventContext);
+                return new ProjectRootElement(projectFile, projectRootElementCache, buildEventContext, preserveWhitespace);
             }
             catch (InvalidProjectFileException)
             {
@@ -1924,11 +1951,12 @@ BuildEventContext buildEventContext
         /// Does NOT add to the ProjectRootElementCache. Caller should add after verifying subsequent MSBuild parsing succeeds.
         /// </summary>
         /// <param name="fullPath">The full path to the document to load.</param>
-        private XmlDocumentWithLocation LoadDocument(string fullPath)
+        private XmlDocumentWithLocation LoadDocument(string fullPath, bool preserveWhitespace)
         {
             ErrorUtilities.VerifyThrowInternalRooted(fullPath);
 
             XmlDocumentWithLocation document = new XmlDocumentWithLocation();
+            document.PreserveWhitespace = preserveWhitespace;
 #if (!STANDALONEBUILD)
             using (new CodeMarkerStartEnd(CodeMarkerEvent.perfMSBuildProjectLoadFromFileBegin, CodeMarkerEvent.perfMSBuildProjectLoadFromFileEnd))
 #endif

src/XMakeBuildEngine/Definition/Toolset.cs

@@ -1011,7 +1011,9 @@ private void LoadAndRegisterFromTasksFile(string searchPath, string[] defaultTas
                     }
                     else
                     {
-                        projectRootElement = ProjectRootElement.Open(defaultTasksFile, projectRootElementCache, false /*The tasks file is not a explicitly loaded file*/);
+                        projectRootElement = ProjectRootElement.Open(defaultTasksFile, projectRootElementCache,
+                            false /*The tasks file is not a explicitly loaded file*/,
+                            preserveWhitespace: false);
                     }
 
                     foreach (ProjectElement elementXml in projectRootElement.Children)

src/XMakeBuildEngine/Evaluation/ProjectRootElementCache.cs

@@ -227,6 +227,8 @@ internal ProjectRootElement Get(string projectFile, OpenProjectRootElement openP
                             // use: it checks the file content as well as the timestamp. That's better than completely disabling
                             // the cache as we get test coverage of the rest of the cache code.
                             XmlDocument document = new XmlDocument();
+                            //  TODO: does PreserveWhitespace need to be an option here?
+                            //document.PreserveWhitespace = true;
                             using (XmlTextReader xtr = new XmlTextReader(projectRootElement.FullPath))
                             {
                                 xtr.DtdProcessing = DtdProcessing.Ignore;

src/XMakeBuildEngine/UnitTests/Evaluation/Preprocessor_Tests.cs

@@ -832,5 +832,74 @@ public void ProjectMetadata()
 
             Helpers.VerifyAssertLineByLine(expected, writer.ToString());
         }
+
+        //  TODO: Test preprocessor formatting (project.SaveLogicalProject)
+        //  TODO: Test loading project from string instead of file
+        //        ProjectRootElement xml = ProjectRootElement.Create(XmlReader.Create(new StringReader(content)));
+        //  TODO: Add test for whether single quotes get changed to double quotes
+        //  TODO: Rename preserveWhitespace parameters (and variable names, etc) to preserveFormatting
+        //  TODO: If preserveFormatting is true, don't change single quotes to double quotes
+
+        //  TODO: Not sure this actually belongs in the preprocessor tests
+
+        [Fact]
+        public void ProjectCommentFormatting()
+        {
+            string content = ObjectModelHelpers.CleanupFileContents(@"
+<Project DefaultTargets=`Build` ToolsVersion=`msbuilddefaulttoolsversion` xmlns=`msbuildnamespace`>
+  <ItemGroup>
+    <ProjectReference Include=`..\CLREXE\CLREXE.vcxproj`><!-- Comment -->
+      <Project>{3699f81b-2d03-46c5-abd7-e88a4c946f28}</Project>
+    </ProjectReference>
+  </ItemGroup>
+</Project>");
+
+            VerifyWhitespacePreserved(content);
+        }
+
+        [Fact]
+        public void ProjectQuoteFormatting()
+        {
+            string content = ObjectModelHelpers.CleanupFileContents(@"
+<Project DefaultTargets='Build' ToolsVersion='msbuilddefaulttoolsversion' xmlns='msbuildnamespace'>
+  <ItemGroup>
+    <ProjectReference Include='..\CLREXE\CLREXE.vcxproj'>
+      <Project>{3699f81b-2d03-46c5-abd7-e88a4c946f28}</Project>
+    </ProjectReference>
+  </ItemGroup>
+</Project>");
+
+            VerifyWhitespacePreserved(content);
+        }
+
+        void VerifyWhitespacePreserved(string projectContents)
+        {
+            string directory = null;
+
+            try
+            {
+                directory = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString());
+                Directory.CreateDirectory(directory);
+
+                string file = Path.Combine(directory, "test.proj");
+                File.WriteAllText(file, projectContents);
+
+                ProjectRootElement xml = ProjectRootElement.Open(file, ProjectCollection.GlobalProjectCollection,
+                    preserveWhitespace: true);
+                Project project = new Project(xml);
+                StringWriter writer = new StringWriter();
+                project.Save(writer);
+
+                string expected = @"<?xml version=""1.0"" encoding=""utf-16""?>" +
+                    projectContents;
+                string actual = writer.ToString();
+
+                Helpers.VerifyAssertLineByLine(expected, actual);
+            }
+            finally
+            {
+                Directory.Delete(directory, true);
+            }
+        }
     }
 }