Bugfixes 991528 and 1038018 Respecting encoding settings

Bugfix 991528 "[CodePlex] Persisting Documents with MSBuildWorkspace.TryApplyChanges doesn't respect existing encoding"

Pass the CodePage MSBuild property to our MSBuildWorkspace instead of defaulting to the system default encoding. If the CodePage is 0 or missing, we default to UTF8.

Bugfix 1038018 "Inline rename causes unrelated edits in files with unicode characters"

The TextBufferTextFactory now prefers encodings in the following order:
     - UTF8/Unicode/BigEndianUnicode if the byte order mark is present
     - The specified default codepage/encoding in the project file
     - UTF8 without a byte order mark
     - The system default encoding (changeset 1348078)

Src/Workspaces/CSharp/MSBuild/CSharpProjectFileLoader.CSharpProjectFile.cs

@@ -111,6 +111,7 @@ private ProjectFileInfo CreateProjectFileInfo(CSharpCompilerInputs compilerInput
                     assemblyName,
                     compilerInputs.CompilationOptions,
                     compilerInputs.ParseOptions,
+                    compilerInputs.CodePage,
                     docs,
                     additionalDocs,
                     this.GetProjectReferences(executedProject),
@@ -271,6 +272,7 @@ private class CSharpCompilerInputs :
                 internal bool Initialized { get; private set; }
                 internal CSharpParseOptions ParseOptions { get; private set; }
                 internal CSharpCompilationOptions CompilationOptions { get; private set; }
+                internal int CodePage { get; private set; }
                 internal IEnumerable<MSB.Framework.ITaskItem> Sources { get; private set; }
                 internal IEnumerable<MSB.Framework.ITaskItem> References { get; private set; }
                 internal IEnumerable<MSB.Framework.ITaskItem> AnalyzerReferences { get; private set; }
@@ -447,7 +449,7 @@ public bool SetCheckForOverflowUnderflow(bool checkForOverflowUnderflow)
 
                 public bool SetCodePage(int codePage)
                 {
-                    // ??
+                    this.CodePage = codePage;
                     return true;
                 }
 

Src/Workspaces/Core/Workspace/MSBuild/MSBuildWorkspace.cs

@@ -644,9 +644,7 @@ private async Task<ProjectId> LoadProjectAsync(string projectFilePath, IProjectF
             var docFileInfos = projectFileInfo.Documents.ToImmutableArrayOrEmpty();
             CheckDocuments(docFileInfos, projectFilePath, projectId);
 
-            // TODO: is there a way how to specify encoding to msbuild? csc.exe has /codepage command line option.
-            // For now use auto-detection. (bug 941489).
-            Encoding defaultEncoding = null;
+            Encoding defaultEncoding = GetDefaultEncoding(projectFileInfo.CodePage);
 
             var docs = new List<DocumentInfo>();
             foreach (var docFileInfo in docFileInfos)
@@ -713,13 +711,32 @@ private async Task<ProjectId> LoadProjectAsync(string projectFilePath, IProjectF
                     resolvedReferences.ProjectReferences,
                     metadataReferences,
                     analyzerReferences: projectFileInfo.AnalyzerReferences,
-                    additionalDocuments: additonalDocs, 
+                    additionalDocuments: additonalDocs,
                     isSubmission: false,
                     hostObjectType: null));
 
             return projectId;
         }
 
+        private static Encoding GetDefaultEncoding(int codePage)
+        {
+            // If no CodePage was specified in the project file, then the FileTextLoader will 
+            // attempt to use UTF8 before falling back on Encoding.Default.
+            if (codePage == 0)
+            {
+                return null;
+            }
+
+            try
+            {
+                return Encoding.GetEncoding(codePage);
+            }
+            catch (ArgumentOutOfRangeException)
+            {
+                return null;
+            }
+        }
+
         private static readonly char[] DirectorySplitChars = new char[] { Path.DirectorySeparatorChar };
 
         private static ImmutableArray<string> GetDocumentFolders(string logicalPath)
@@ -955,7 +972,7 @@ private void AddDocumentCore(DocumentId documentId, IEnumerable<string> folders,
                 var relativePath = folders != null ? Path.Combine(Path.Combine(folders.ToArray()), fileName) : fileName;
 
                 var fullPath = GetAbsolutePath(relativePath, Path.GetDirectoryName(project.FilePath));
-                var encoding = (text != null) ? text.Encoding : Encoding.UTF8;
+                var encoding = (text != null) ? text.Encoding : null;
 
                 var documentInfo = DocumentInfo.Create(documentId, fileName, folders, sourceCodeKind, new FileTextLoader(fullPath, encoding), fullPath, encoding, isGenerated: false);
 

Src/Workspaces/Core/Workspace/MSBuild/ProjectFile/ProjectFileInfo.cs

@@ -37,6 +37,11 @@ internal sealed class ProjectFileInfo
         /// </summary>
         public ParseOptions ParseOptions { get; private set; }
 
+        /// <summary>
+        /// The codepage for this project.
+        /// </summary>
+        public int CodePage { get; private set; }
+
         /// <summary>
         /// The source documents.
         /// </summary>
@@ -68,6 +73,7 @@ internal sealed class ProjectFileInfo
             string assemblyName,
             CompilationOptions compilationOptions,
             ParseOptions parseOptions,
+            int codePage,
             IEnumerable<DocumentFileInfo> documents,
             IEnumerable<DocumentFileInfo> additionalDocuments,
             IEnumerable<ProjectFileReference> projectReferences,
@@ -79,6 +85,7 @@ internal sealed class ProjectFileInfo
             this.AssemblyName = assemblyName;
             this.CompilationOptions = compilationOptions;
             this.ParseOptions = parseOptions;
+            this.CodePage = codePage;
             this.Documents = documents.ToImmutableReadOnlyListOrEmpty();
             this.AdditionalDocuments = additionalDocuments.ToImmutableReadOnlyListOrEmpty();
             this.ProjectReferences = projectReferences.ToImmutableReadOnlyListOrEmpty();

Src/Workspaces/Core/WorkspacesResources.Designer.cs

@@ -348,6 +348,15 @@ internal class WorkspacesResources {
             }
         }
         
+        /// <summary>
+        ///   Looks up a localized string similar to Invalid CodePage value: {0}.
+        /// </summary>
+        internal static string InvalidCodePage {
+            get {
+                return ResourceManager.GetString("InvalidCodePage", resourceCulture);
+            }
+        }
+        
         /// <summary>
         ///   Looks up a localized string similar to Destination type must be a {0}..
         /// </summary>

Src/Workspaces/Core/WorkspacesResources.resx

@@ -360,4 +360,7 @@
   <data name="RemovedHeader" xml:space="preserve">
     <value>Removed:</value>
   </data>
+  <data name="InvalidCodePage" xml:space="preserve">
+    <value>Invalid CodePage value: {0}</value>
+  </data>
 </root>
\ No newline at end of file

Src/Workspaces/CoreTest/ServicesTest.csproj

@@ -274,6 +274,9 @@
   <ItemGroup>
     <None Include="packages.config" />
   </ItemGroup>
+  <ItemGroup>
+    <EmbeddedResource Include="TestFiles\Encoding.csproj" />
+  </ItemGroup>
   <ImportGroup Label="Targets">
     <Import Project="..\..\Tools\Microsoft.CodeAnalysis.Toolset.Open\Targets\VSL.Imports.targets" />
     <Import Project="..\..\..\packages\StyleCop.MSBuild.4.7.48.2\build\StyleCop.MSBuild.Targets" Condition="Exists('..\..\..\packages\StyleCop.MSBuild.4.7.48.2\build\StyleCop.MSBuild.Targets')" />

Src/Workspaces/CoreTest/TestFiles/Encoding.csproj

+<?xml version="1.0" encoding="utf-8"?>
+<Project ToolsVersion="14.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" Condition="Exists('$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props')" />
+  <PropertyGroup>
+    <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
+    <OutDir>..\..\..\..\Binaries\$(Configuration)\</OutDir>
+    <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
+    <ProjectGuid>0457d095-9f74-415c-b461-dd08d4503a34</ProjectGuid>
+    <OutputType>Library</OutputType>
+    <AppDesignerFolder>Properties</AppDesignerFolder>
+    <RootNamespace>ClassLibrary1</RootNamespace>
+    <AssemblyName>ClassLibrary1</AssemblyName>
+    <TargetFrameworkVersion>v4.5</TargetFrameworkVersion>
+    <FileAlignment>512</FileAlignment>
+    <CodePage>ReplaceMe</CodePage>
+    <DebugSymbols>true</DebugSymbols>
+    <DebugType>full</DebugType>
+    <Optimize>false</Optimize>
+    <OutputPath>bin\Debug\</OutputPath>
+    <DefineConstants>DEBUG;TRACE</DefineConstants>
+    <ErrorReport>prompt</ErrorReport>
+    <WarningLevel>4</WarningLevel>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="class1.cs" />
+  </ItemGroup>
+  <Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />
+</Project>
\ No newline at end of file

Src/Workspaces/CoreTest/WorkspaceTests/MSBuildWorkspaceTests.cs

@@ -5,6 +5,7 @@
 using System.Collections.Immutable;
 using System.IO;
 using System.Linq;
+using System.Text;
 using System.Threading;
 using System.Xml.Linq;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
@@ -2194,5 +2195,88 @@ public void TestOpenSolution_WithDuplicatedGuidsBecomeCircularReferential()
                 var c = p.GetCompilationAsync().Result;
             }
         }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.Workspace)]
+        [WorkItem(991528)]
+        public void MSBuildProjectShouldHandleCodePageProperty()
+        {
+            var files = new FileSet(new Dictionary<string, object>
+            {
+                { "Encoding.csproj", GetResourceText("Encoding.csproj").Replace("<CodePage>ReplaceMe</CodePage>", "<CodePage>1254</CodePage>") },
+                { "class1.cs", "//“" }
+            });
+
+            CreateFiles(files);
+
+            var projPath = Path.Combine(this.SolutionDirectory.Path, "Encoding.csproj");
+            var project = MSBuildWorkspace.Create().OpenProjectAsync(projPath).Result;
+
+            var text = project.Documents.First(d => d.Name == "class1.cs").GetTextAsync().Result;
+            Assert.Equal(Encoding.GetEncoding(1254), text.Encoding);
+
+            // The smart quote (“) in class1.cs shows up as "“" in codepage 1254. Do a sanity
+            // check here to make sure this file hasn't been corrupted in a way that would
+            // impact subsequent asserts.
+            Assert.Equal("//“".Length, 5);
+            Assert.Equal("//“".Length, text.Length);
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.Workspace)]
+        [WorkItem(991528)]
+        public void MSBuildProjectShouldHandleInvalidCodePageProperty()
+        {
+            var files = new FileSet(new Dictionary<string, object>
+            {
+                { "Encoding.csproj", GetResourceText("Encoding.csproj").Replace("<CodePage>ReplaceMe</CodePage>", "<CodePage>-1</CodePage>") },
+                { "class1.cs", "//“" }
+            });
+
+            CreateFiles(files);
+
+            var projPath = Path.Combine(this.SolutionDirectory.Path, "Encoding.csproj");
+
+            var project = MSBuildWorkspace.Create().OpenProjectAsync(projPath).Result;
+
+            Assert.Equal(new UTF8Encoding(encoderShouldEmitUTF8Identifier: false, throwOnInvalidBytes: true), project.Documents.First(d => d.Name == "class1.cs").GetTextAsync().Result.Encoding);
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.Workspace)]
+        [WorkItem(991528)]
+        public void MSBuildProjectShouldHandleInvalidCodePageProperty2()
+        {
+            var files = new FileSet(new Dictionary<string, object>
+            {
+                { "Encoding.csproj", GetResourceText("Encoding.csproj").Replace("<CodePage>ReplaceMe</CodePage>", "<CodePage>Broken</CodePage>") },
+                { "class1.cs", "//“" }
+            });
+
+            CreateFiles(files);
+
+            var projPath = Path.Combine(this.SolutionDirectory.Path, "Encoding.csproj");
+
+            var project = MSBuildWorkspace.Create().OpenProjectAsync(projPath).Result;
+
+            Assert.Equal(new UTF8Encoding(encoderShouldEmitUTF8Identifier: false, throwOnInvalidBytes: true), project.Documents.First(d => d.Name == "class1.cs").GetTextAsync().Result.Encoding);
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.Workspace)]
+        [WorkItem(991528)]
+        public void MSBuildProjectShouldHandleDefaultCodePageProperty()
+        {
+            var files = new FileSet(new Dictionary<string, object>
+            {
+                { "Encoding.csproj", GetResourceText("Encoding.csproj").Replace("<CodePage>ReplaceMe</CodePage>", string.Empty) },
+                { "class1.cs", "//“" }
+            });
+
+            CreateFiles(files);
+
+            var projPath = Path.Combine(this.SolutionDirectory.Path, "Encoding.csproj");
+            var project = MSBuildWorkspace.Create().OpenProjectAsync(projPath).Result;
+
+            var text = project.Documents.First(d => d.Name == "class1.cs").GetTextAsync().Result;
+            Assert.Equal(new UTF8Encoding(encoderShouldEmitUTF8Identifier: false, throwOnInvalidBytes: true), text.Encoding);
+            Assert.Equal("//“", text.ToString());
+        }
     }
 }

Src/Workspaces/VisualBasic/MSBuild/VisualBasicProjectFileLoader.vb

@@ -85,6 +85,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
                     compilerInputs.CompilationOptions,
                     compilerInputs.ParseOptions.WithPreprocessorSymbols(AddPredefinedPreprocessorSymbols(
                         compilerInputs.CompilationOptions.OutputKind, compilerInputs.ParseOptions.PreprocessorSymbols)),
+                    compilerInputs.CodePage,
                     Me.GetDocuments(compilerInputs.Sources, executedProject),
                     Me.GetDocuments(compilerInputs.AdditionalFiles, executedProject),
                     Me.GetProjectReferences(executedProject),
@@ -263,6 +264,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
                 Private _initialized As Boolean
                 Private _parseOptions As VisualBasicParseOptions
                 Private _compilationOptions As VisualBasicCompilationOptions
+                Private _codePage As Integer
                 Private _sources As IEnumerable(Of MSB.Framework.ITaskItem)
                 Private _additionalFiles As IEnumerable(Of MSB.Framework.ITaskItem)
                 Private _references As IEnumerable(Of MSB.Framework.ITaskItem)
@@ -311,6 +313,12 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
                     End Get
                 End Property
 
+                Public ReadOnly Property CodePage As Integer
+                    Get
+                        Return Me._codePage
+                    End Get
+                End Property
+
                 Public ReadOnly Property References As IEnumerable(Of MSB.Framework.ITaskItem)
                     Get
                         Return Me._references
@@ -415,6 +423,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
                 End Function
 
                 Public Function SetCodePage(codePage As Integer) As Boolean Implements Microsoft.Build.Tasks.Hosting.IVbcHostObject.SetCodePage
+                    Me._codePage = codePage
                     Return True
                 End Function