Merge pull request #482 from heejaechang/preserveEncoding

preserve encoding over temporary storage

src/EditorFeatures/Core/Implementation/Workspaces/EditorTextFactoryService.cs

 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
-using System;
 using System.Composition;
 using System.Diagnostics;
 using System.IO;
-using System.Runtime.InteropServices;
 using System.Text;
 using System.Threading;
 using Microsoft.CodeAnalysis.Host;
@@ -34,6 +32,8 @@ internal class EditorTextFactoryService : ITextFactoryService
 
         public SourceText CreateText(Stream stream, Encoding defaultEncoding, CancellationToken cancellationToken = default(CancellationToken))
         {
+            // this API is for a case where user wants us to figure out encoding from the given stream.
+            // if defaultEncoding is given, we will use it if we couldn't figure out encoding used in the stream ourselves.
             Debug.Assert(stream != null);
             Debug.Assert(stream.CanSeek);
             Debug.Assert(stream.CanRead);
@@ -62,6 +62,15 @@ public SourceText CreateText(Stream stream, Encoding defaultEncoding, Cancellati
             }
         }
 
+        public SourceText CreateText(TextReader reader, Encoding encoding, CancellationToken cancellationToken = default(CancellationToken))
+        {
+            // this API is for a case where user just wants to create a source text with explicit encoding.
+            var buffer = CreateTextBuffer(reader, cancellationToken);
+
+            // use the given encoding as it is.
+            return buffer.CurrentSnapshot.AsRoslynText(encoding);
+        }
+
         private ITextBuffer CreateTextBuffer(TextReader reader, CancellationToken cancellationToken = default(CancellationToken))
         {
             return _textBufferFactory.CreateTextBuffer(reader, _unknownContentType);
@@ -72,80 +81,12 @@ private SourceText CreateTextInternal(Stream stream, Encoding encoding, Cancella
             cancellationToken.ThrowIfCancellationRequested();
             stream.Seek(0, SeekOrigin.Begin);
 
-            // Detect text coming from temporary storage
-            var accessor = stream as ISupportDirectMemoryAccess;
-            if (accessor != null)
-            {
-                return CreateTextFromTemporaryStorage(accessor, (int)stream.Length, cancellationToken);
-            }
-
             using (var reader = new StreamReader(stream, encoding, detectEncodingFromByteOrderMarks: true, bufferSize: 1024, leaveOpen: true))
             {
                 var buffer = CreateTextBuffer(reader, cancellationToken);
                 return buffer.CurrentSnapshot.AsRoslynText(reader.CurrentEncoding ?? Encoding.UTF8);
             }
         }
-
-        private unsafe SourceText CreateTextFromTemporaryStorage(ISupportDirectMemoryAccess accessor, int streamLength, CancellationToken cancellationToken)
-        {
-            char* src = (char*)accessor.GetPointer();
-            Debug.Assert(*src == 0xFEFF); // BOM: Unicode, little endian
-            // Skip the BOM when creating the reader
-            using (var reader = new DirectMemoryAccessStreamReader(src + 1, streamLength / sizeof(char) - 1))
-            {
-                var buffer = CreateTextBuffer(reader, cancellationToken);
-                return buffer.CurrentSnapshot.AsRoslynText(Encoding.Unicode);
-            }
-        }
-
-        private unsafe class DirectMemoryAccessStreamReader : TextReader
-        {
-            private char* _position;
-            private readonly char* _end;
-
-            public DirectMemoryAccessStreamReader(char* src, int length)
-            {
-                Debug.Assert(src != null);
-                Debug.Assert(length >= 0);
-                _position = src;
-                _end = _position + length;
-            }
-
-            public override int Read()
-            {
-                if(_position >= _end)
-                {
-                    return -1;
-                }
-
-                return *_position++;
-            }
-
-            public override int Read(char[] buffer, int index, int count)
-            {
-                if (buffer == null)
-                {
-                    throw new ArgumentNullException(nameof(buffer));
-                }
-
-                if (index < 0 || index >= buffer.Length)
-                {
-                    throw new ArgumentOutOfRangeException(nameof(index));
-                }
-
-                if (count < 0 || (index + count) > buffer.Length)
-                {
-                    throw new ArgumentOutOfRangeException(nameof(count));
-                }
-
-                count = Math.Min(count, (int)(_end - _position));
-                if (count > 0)
-                {
-                    Marshal.Copy((IntPtr)_position, buffer, index, count);
-                    _position += count;
-                }
-                return count;
-            }
-        }
     }
 }
+

src/EditorFeatures/Test/Workspaces/TextFactoryTests.cs

@@ -64,7 +64,29 @@ public void TestCreateFromTemporaryStorage()
             // Create a temporary storage location
             using (var temporaryStorage = temporaryStorageService.CreateTemporaryTextStorage(System.Threading.CancellationToken.None))
             {
+                // Write text into it
+                temporaryStorage.WriteTextAsync(text).Wait();
+
+                // Read text back from it
+                var text2 = temporaryStorage.ReadTextAsync().Result;
+
+                Assert.NotSame(text, text2);
+                Assert.Equal(text.ToString(), text2.ToString());
+                Assert.Equal(text2.Encoding, null);
+            }
+        }
 
+        [Fact]
+        public void TestCreateFromTemporaryStorageWithEncoding()
+        {
+            var textFactory = CreateMockTextFactoryService();
+            var temporaryStorageService = new TemporaryStorageServiceFactory.TemporaryStorageService(textFactory);
+
+            var text = Text.SourceText.From("Hello, World!", Encoding.ASCII);
+
+            // Create a temporary storage location
+            using (var temporaryStorage = temporaryStorageService.CreateTemporaryTextStorage(System.Threading.CancellationToken.None))
+            {
                 // Write text into it
                 temporaryStorage.WriteTextAsync(text).Wait();
 
@@ -73,7 +95,7 @@ public void TestCreateFromTemporaryStorage()
 
                 Assert.NotSame(text, text2);
                 Assert.Equal(text.ToString(), text2.ToString());
-                Assert.Equal(text2.Encoding, Encoding.Unicode);
+                Assert.Equal(text2.Encoding, Encoding.ASCII);
             }
         }
 

src/EditorFeatures/Text/Extensions.SnapshotSourceText.cs

@@ -28,8 +28,8 @@ private class SnapshotSourceText : SourceText
             /// </summary>
             internal class ClosedSnapshotSourceText : SnapshotSourceText
             {
-                public ClosedSnapshotSourceText(ITextSnapshot roslynSnapshot, Encoding encoding)
-                    : base(roslynSnapshot, encoding, containerOpt: null)
+                public ClosedSnapshotSourceText(ITextSnapshot roslynSnapshot, Encoding encodingOpt)
+                    : base(roslynSnapshot, encodingOpt, containerOpt: null)
                 {
                 }
             }
@@ -40,29 +40,27 @@ public ClosedSnapshotSourceText(ITextSnapshot roslynSnapshot, Encoding encoding)
             /// The ITextSnapshot backing the SourceText instance
             /// </summary>
             protected readonly ITextSnapshot RoslynSnapshot;
-            private readonly Encoding _encoding;
+            private readonly Encoding _encodingOpt;
             private readonly TextBufferContainer _containerOpt;
             private readonly int _reiteratedVersion;
             private LineInfo _lineInfo;
 
-            private SnapshotSourceText(ITextSnapshot editorSnapshot, Encoding encoding)
+            private SnapshotSourceText(ITextSnapshot editorSnapshot, Encoding encodingOpt)
             {
                 Contract.ThrowIfNull(editorSnapshot);
-                Contract.ThrowIfNull(encoding);
 
                 this.RoslynSnapshot = TextBufferMapper.ToRoslyn(editorSnapshot);
                 _containerOpt = TextBufferContainer.From(editorSnapshot.TextBuffer);
                 _reiteratedVersion = editorSnapshot.Version.ReiteratedVersionNumber;
-                _encoding = encoding;
+                _encodingOpt = encodingOpt;
             }
 
-            public SnapshotSourceText(ITextSnapshot roslynSnapshot, Encoding encoding, TextBufferContainer containerOpt)
+            public SnapshotSourceText(ITextSnapshot roslynSnapshot, Encoding encodingOpt, TextBufferContainer containerOpt)
             {
                 Contract.ThrowIfNull(roslynSnapshot);
-                Contract.ThrowIfNull(encoding);
 
                 this.RoslynSnapshot = roslynSnapshot;
-                _encoding = encoding;
+                _encodingOpt = encodingOpt;
                 _containerOpt = containerOpt;
             }
 
@@ -101,7 +99,7 @@ private static SnapshotSourceText CreateText(ITextSnapshot editorSnapshot)
 
             public override Encoding Encoding
             {
-                get { return _encoding; }
+                get { return _encodingOpt; }
             }
 
             public ITextSnapshot EditorSnapshot

src/Workspaces/Core/Desktop/Workspace/Host/TemporaryStorage/TemporaryStorageServiceFactory.cs

@@ -2,7 +2,9 @@
 
 using System;
 using System.Composition;
+using System.Diagnostics;
 using System.IO;
+using System.Runtime.InteropServices;
 using System.Text;
 using System.Threading;
 using System.Threading.Tasks;
@@ -83,8 +85,10 @@ public SourceText ReadText(CancellationToken cancellationToken)
                     using (Logger.LogBlock(FunctionId.TemporaryStorageServiceFactory_ReadText, cancellationToken))
                     {
                         using (var stream = _memoryMappedInfo.CreateReadableStream())
+                        using (var reader = CreateTextReaderFromTemporaryStorage((ISupportDirectMemoryAccess)stream, (int)stream.Length, cancellationToken))
                         {
-                            return _service._textFactory.CreateText(stream, _encoding, cancellationToken);
+                            // we pass in encoding we got from original source text even if it is null.
+                            return _service._textFactory.CreateText(reader, _encoding, cancellationToken);
                         }
                     }
                 }
@@ -135,6 +139,69 @@ public Task WriteTextAsync(SourceText text, CancellationToken cancellationToken
                     // See commentary in ReadTextAsync for why this is implemented this way.
                     return Task.Factory.StartNew(() => WriteText(text, cancellationToken), cancellationToken, TaskCreationOptions.None, TaskScheduler.Default);
                 }
+
+                private unsafe TextReader CreateTextReaderFromTemporaryStorage(ISupportDirectMemoryAccess accessor, int streamLength, CancellationToken cancellationToken)
+                {
+                    char* src = (char*)accessor.GetPointer();
+
+                    // BOM: Unicode, little endian
+                    // Skip the BOM when creating the reader
+                    Debug.Assert(*src == 0xFEFF);
+
+                    return new DirectMemoryAccessStreamReader(src + 1, streamLength / sizeof(char) - 1);
+                }
+
+                private unsafe class DirectMemoryAccessStreamReader : TextReader
+                {
+                    private char* _position;
+                    private readonly char* _end;
+
+                    public DirectMemoryAccessStreamReader(char* src, int length)
+                    {
+                        Debug.Assert(src != null);
+                        Debug.Assert(length >= 0);
+
+                        _position = src;
+                        _end = _position + length;
+                    }
+
+                    public override int Read()
+                    {
+                        if (_position >= _end)
+                        {
+                            return -1;
+                        }
+
+                        return *_position++;
+                    }
+
+                    public override int Read(char[] buffer, int index, int count)
+                    {
+                        if (buffer == null)
+                        {
+                            throw new ArgumentNullException(nameof(buffer));
+                        }
+
+                        if (index < 0 || index >= buffer.Length)
+                        {
+                            throw new ArgumentOutOfRangeException(nameof(index));
+                        }
+
+                        if (count < 0 || (index + count) > buffer.Length)
+                        {
+                            throw new ArgumentOutOfRangeException(nameof(count));
+                        }
+
+                        count = Math.Min(count, (int)(_end - _position));
+                        if (count > 0)
+                        {
+                            Marshal.Copy((IntPtr)_position, buffer, index, count);
+                            _position += count;
+                        }
+
+                        return count;
+                    }
+                }
             }
 
             private class TemporaryStreamStorage : ITemporaryStreamStorage
@@ -244,3 +311,4 @@ private async Task WriteStreamMaybeAsync(Stream stream, bool useAsync, Cancellat
         }
     }
 }
+

src/Workspaces/Core/Desktop/Workspace/Host/TextFactory/DesktopTextFactoryService.cs

@@ -17,5 +17,12 @@ public SourceText CreateText(Stream stream, Encoding defaultEncoding, Cancellati
             cancellationToken.ThrowIfCancellationRequested();
             return EncodedStringText.Create(stream, defaultEncoding);
         }
+
+        public SourceText CreateText(TextReader reader, Encoding encoding, CancellationToken cancellationToken = default(CancellationToken))
+        {
+            cancellationToken.ThrowIfCancellationRequested();
+            return SourceText.From(reader.ReadToEnd(), encoding);
+        }
     }
 }
+

src/Workspaces/Core/Portable/Workspace/Host/TextFactory/ITextFactoryService.cs

@@ -28,5 +28,16 @@ internal interface ITextFactoryService : IWorkspaceService
         /// </exception>
         /// <exception cref="IOException">An IO error occurred while reading from the stream.</exception>
         SourceText CreateText(Stream stream, Encoding defaultEncoding, CancellationToken cancellationToken = default(CancellationToken));
+
+
+        /// <summary>
+        /// Creates <see cref="SourceText"/> from a reader with given <paramref name="encoding"/>.
+        /// </summary>
+        /// <param name="reader">The <see cref="TextReader"/> to read the text from.</param>
+        /// <param name="encoding">Specifies an encoding for the <see cref="SourceText"/>SourceText. 
+        /// it could be null. but if null is given, it won't be able to calculate checksum</param>
+        /// <param name="cancellationToken">Cancellation token.</param>
+        SourceText CreateText(TextReader reader, Encoding encoding, CancellationToken cancellationToken = default(CancellationToken));
     }
 }
+

src/Workspaces/Core/Portable/Workspace/Host/TextFactory/TextFactoryService.cs

@@ -17,5 +17,12 @@ public SourceText CreateText(Stream stream, Encoding defaultEncoding, Cancellati
             cancellationToken.ThrowIfCancellationRequested();
             return SourceText.From(stream, defaultEncoding);
         }
+
+        public SourceText CreateText(TextReader reader, Encoding encoding, CancellationToken cancellationToken = default(CancellationToken))
+        {
+            cancellationToken.ThrowIfCancellationRequested();
+            return SourceText.From(reader.ReadToEnd(), encoding);
+        }
     }
 }
+

src/Workspaces/CoreTest/WorkspaceServiceTests/TemporaryStorageServiceTests.cs

@@ -18,7 +18,7 @@ namespace Microsoft.CodeAnalysis.UnitTests
 {
     public class TemporaryStorageServiceTests
     {
-        [Fact(Skip = "https://github.com/dotnet/roslyn/issues/380"), Trait(Traits.Feature, Traits.Features.Workspace)]
+        [Fact, Trait(Traits.Feature, Traits.Features.Workspace)]
         public void TestTemporaryStorageText()
         {
             var textFactory = new TextFactoryService();
@@ -79,6 +79,7 @@ private void TestTemporaryStorage(ITemporaryStorageService temporaryStorageServi
 
             Assert.NotSame(text, text2);
             Assert.Equal(text.ToString(), text2.ToString());
+            Assert.Equal(text.Encoding, text2.Encoding);
 
             temporaryStorage.Dispose();
         }
@@ -325,7 +326,7 @@ public void StreamTest3()
             }
         }
 
-        [Fact(Skip = "https://github.com/dotnet/roslyn/issues/353"), Trait(Traits.Feature, Traits.Features.Workspace)]
+        [Fact, Trait(Traits.Feature, Traits.Features.Workspace)]
         public void TestTemporaryStorageTextEncoding()
         {
             var textFactory = new TextFactoryService();
@@ -333,33 +334,15 @@ public void TestTemporaryStorageTextEncoding()
 
             // test normal string
             var text = SourceText.From(new string(' ', 4096) + "public class A {}", Encoding.ASCII);
-            TestTemporaryStorageWithEncoding(service, text);
+            TestTemporaryStorage(service, text);
 
             // test empty string
             text = SourceText.From(string.Empty);
-            TestTemporaryStorageWithEncoding(service, text);
+            TestTemporaryStorage(service, text);
 
             // test large string
             text = SourceText.From(new string(' ', 1024 * 1024) + "public class A {}");
-            TestTemporaryStorageWithEncoding(service, text);
-        }
-
-        private void TestTemporaryStorageWithEncoding(ITemporaryStorageService temporaryStorageService, SourceText text)
-        {
-            // create a temporary storage location
-            var temporaryStorage = temporaryStorageService.CreateTemporaryTextStorage(System.Threading.CancellationToken.None);
-
-            // write text into it
-            temporaryStorage.WriteTextAsync(text).Wait();
-
-            // read text back from it
-            var text2 = temporaryStorage.ReadTextAsync().Result;
-
-            Assert.NotSame(text, text2);
-            Assert.Equal(text.ToString(), text2.ToString());
-            Assert.Equal(text.Encoding, text2.Encoding);
-
-            temporaryStorage.Dispose();
+            TestTemporaryStorage(service, text);
         }
     }
 }