applied code review feedback

src/Scripting/CSharpTest/ScriptTests.cs

@@ -11,6 +11,7 @@
 using Xunit;
 using System.IO;
 using System.Globalization;
+using System.Text;
 
 namespace Microsoft.CodeAnalysis.CSharp.Scripting.UnitTests
 {
@@ -692,13 +693,13 @@ public async Task ScriptOptionsConfiguredForDebuggingResultInPdbEmitted()
         {
             try
             {
-                var opts = ScriptOptions.Default.WithDebugInformation();
+                var opts = ScriptOptions.Default.WithEmitDebugInformation(true);
                 var script = await CSharpScript.RunAsync("throw new System.Exception();", opts);
             }
             catch (Exception ex)
             {
                 // line information is only available when PDBs have been emitted
-                Assert.Contains("at Submission#0.<<Initialize>>d__0.MoveNext() in :line 1", ex.StackTrace);
+                Assert.Equal("at Submission#0.<<Initialize>>d__0.MoveNext() in :line 1", GetStackTraceLine(ex, 0));
             }
         }
 
@@ -712,7 +713,7 @@ public async Task ScriptOptionsNotConfiguredForDebuggingResultInPdbNotEmitted()
             catch (Exception ex)
             {
                 // line information is only available when PDBs have been emitted
-                Assert.DoesNotContain("at Submission#0.<<Initialize>>d__0.MoveNext() in :line 1", ex.StackTrace);
+                Assert.Equal("at Submission#0.<<Initialize>>d__0.MoveNext()", GetStackTraceLine(ex, 0));
             }
         }
 
@@ -768,5 +769,18 @@ public override Stream OpenRead(string resolvedPath)
                 return stream;
             }
         }
+
+        private string GetStackTraceLine(Exception ex, int index)
+        {
+            if (ex == null || ex.StackTrace == null) return null;
+
+            var stackTrace = ex.StackTrace?.Split(new[] { Environment.NewLine }, StringSplitOptions.None);
+            if (stackTrace.Length >= index)
+            {
+                return stackTrace[index].Trim();
+            }
+
+            return null;
+        }
     }
 }

src/Scripting/Core/Hosting/CommandLine/CommandLineRunner.cs

@@ -149,7 +149,9 @@ private static ScriptOptions GetScriptOptions(CommandLineArguments arguments, st
                 references: ImmutableArray.CreateRange(resolvedReferences),
                 namespaces: CommandLineHelpers.GetImports(arguments),
                 metadataResolver: metadataResolver,
-                sourceResolver: sourceResolver);
+                sourceResolver: sourceResolver,
+                emitDebugInformation: false,
+                fileEncoding: null);
         }
 
         internal static MetadataReferenceResolver GetMetadataReferenceResolver(CommandLineArguments arguments, TouchedFileLogger loggerOpt)

src/Scripting/Core/PublicAPI.Unshipped.txt

 Microsoft.CodeAnalysis.Scripting.ScriptOptions.EmitDebugInformation.get -> bool
 Microsoft.CodeAnalysis.Scripting.ScriptOptions.FileEncoding.get -> System.Text.Encoding
-Microsoft.CodeAnalysis.Scripting.ScriptOptions.WithDebugInformation() -> Microsoft.CodeAnalysis.Scripting.ScriptOptions
+Microsoft.CodeAnalysis.Scripting.ScriptOptions.WithEmitDebugInformation(bool emitDebugInformation) -> Microsoft.CodeAnalysis.Scripting.ScriptOptions
 Microsoft.CodeAnalysis.Scripting.ScriptOptions.WithFileEncoding(System.Text.Encoding encoding) -> Microsoft.CodeAnalysis.Scripting.ScriptOptions
\ No newline at end of file

src/Scripting/Core/Script.cs

@@ -325,7 +325,7 @@ internal override ImmutableArray<Diagnostic> CommonCompile(CancellationToken can
         {
             if (_lazyExecutor == null)
             {
-                Interlocked.CompareExchange(ref _lazyExecutor, Builder.CreateExecutor<T>(Compiler, GetCompilation(), Options, cancellationToken), null);
+                Interlocked.CompareExchange(ref _lazyExecutor, Builder.CreateExecutor<T>(Compiler, GetCompilation(), Options.EmitDebugInformation, cancellationToken), null);
             }
 
             return _lazyExecutor;

src/Scripting/Core/ScriptBuilder.cs

@@ -66,7 +66,7 @@ public int GenerateSubmissionId(out string assemblyName, out string typeName)
         }
 
         /// <exception cref="CompilationErrorException">Compilation has errors.</exception>
-        internal Func<object[], Task<T>> CreateExecutor<T>(ScriptCompiler compiler, Compilation compilation, ScriptOptions scriptOptions, CancellationToken cancellationToken)
+        internal Func<object[], Task<T>> CreateExecutor<T>(ScriptCompiler compiler, Compilation compilation, bool emitDebugInformation, CancellationToken cancellationToken)
         {
             var diagnostics = DiagnosticBag.GetInstance();
             try
@@ -76,7 +76,7 @@ public int GenerateSubmissionId(out string assemblyName, out string typeName)
                 ThrowIfAnyCompilationErrors(diagnostics, compiler.DiagnosticFormatter);
                 diagnostics.Clear();
 
-                var executor = Build<T>(compilation, diagnostics, scriptOptions.EmitDebugInformation, cancellationToken);
+                var executor = Build<T>(compilation, diagnostics, emitDebugInformation, cancellationToken);
 
                 // emit can fail due to compilation errors or because there is nothing to emit:
                 ThrowIfAnyCompilationErrors(diagnostics, compiler.DiagnosticFormatter);

src/Scripting/Core/ScriptOptions.cs

@@ -25,7 +25,9 @@ public sealed class ScriptOptions
             references: GetDefaultMetadataReferences(),
             namespaces: ImmutableArray<string>.Empty,
             metadataResolver: RuntimeMetadataReferenceResolver.Default,
-            sourceResolver: SourceFileResolver.Default);
+            sourceResolver: SourceFileResolver.Default,
+            emitDebugInformation: false,
+            fileEncoding: null);
 
         private static ImmutableArray<MetadataReference> GetDefaultMetadataReferences()
         {
@@ -100,7 +102,7 @@ private static ImmutableArray<MetadataReference> GetDefaultMetadataReferences()
         /// <summary>
         /// Specifies the encoding to be used when debugging scripts loaded from a file, or that will be saved to a file for debugging purposes.
         /// </summary>
-        public Encoding FileEncoding { get; private set; } = Encoding.UTF8;
+        public Encoding FileEncoding { get; private set; }
 
         /// <summary>
         /// The path to the script source if it originated from a file, empty otherwise.
@@ -112,7 +114,9 @@ private static ImmutableArray<MetadataReference> GetDefaultMetadataReferences()
             ImmutableArray<MetadataReference> references,
             ImmutableArray<string> namespaces,
             MetadataReferenceResolver metadataResolver,
-            SourceReferenceResolver sourceResolver)
+            SourceReferenceResolver sourceResolver,
+            bool emitDebugInformation,
+            Encoding fileEncoding)
         {
             Debug.Assert(filePath != null);
             Debug.Assert(!references.IsDefault);
@@ -125,6 +129,8 @@ private static ImmutableArray<MetadataReference> GetDefaultMetadataReferences()
             Imports = namespaces;
             MetadataResolver = metadataResolver;
             SourceResolver = sourceResolver;
+            EmitDebugInformation = emitDebugInformation;
+            FileEncoding = fileEncoding;
         }
 
         private ScriptOptions(ScriptOptions other)
@@ -132,10 +138,10 @@ private ScriptOptions(ScriptOptions other)
                    references: other.MetadataReferences,
                    namespaces: other.Imports,
                    metadataResolver: other.MetadataResolver,
-                   sourceResolver: other.SourceResolver)
+                   sourceResolver: other.SourceResolver,
+                   emitDebugInformation: other.EmitDebugInformation,
+                   fileEncoding: other.FileEncoding)
         {
-            EmitDebugInformation = other.EmitDebugInformation;
-            FileEncoding = other.FileEncoding;
         }
 
         // a reference to an assembly should by default be equivalent to #r, which applies recursive global alias:
@@ -299,13 +305,13 @@ private static MetadataReference CreateReferenceFromAssembly(Assembly assembly)
         /// <summary>
         /// Creates a new <see cref="ScriptOptions"/> with debugging information enabled.
         /// </summary>
-        public ScriptOptions WithDebugInformation() =>
-            new ScriptOptions(this) { EmitDebugInformation = true };
+        public ScriptOptions WithEmitDebugInformation(bool emitDebugInformation) =>
+            emitDebugInformation == EmitDebugInformation ? this : new ScriptOptions(this) { EmitDebugInformation = emitDebugInformation };
 
         /// <summary>
         /// Creates a new <see cref="ScriptOptions"/> with specified <see cref="FileEncoding"/>.
         /// </summary>
-        public ScriptOptions WithFileEncoding(Encoding encoding) =>
-            new ScriptOptions(this) { FileEncoding = encoding };
+        public ScriptOptions WithFileEncoding(Encoding encoding) => 
+            encoding == FileEncoding ? this : new ScriptOptions(this) { FileEncoding = encoding };
     }
 }

src/Scripting/CoreTest/ScriptOptionsTests.cs

@@ -4,6 +4,7 @@
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Reflection;
+using System.Text;
 using Microsoft.CodeAnalysis.Emit;
 using Microsoft.CodeAnalysis.Scripting.Hosting;
 using Roslyn.Test.Utilities;
@@ -150,11 +151,34 @@ public void WithImports_Errors()
             options.WithImports(".blah");
         }
 
+        [Theory]
+        [InlineData(true)]
+        [InlineData(false)]
+        public void WithEmitDebugInformation_SetsEmitDebugInformation(bool emitDebugInformation)
+        {
+            var options = ScriptOptions.Default.WithEmitDebugInformation(emitDebugInformation);
+            Assert.Equal(emitDebugInformation, options.EmitDebugInformation);
+        }
+
+        [Fact]
+        public void WithEmitDebugInformation_SameValueTwice_DoesNotCreateNewInstance()
+        {
+            var options = ScriptOptions.Default.WithEmitDebugInformation(true);
+            Assert.Same(options, options.WithEmitDebugInformation(true));
+        }
+
+        [Fact]
+        public void WithFileEncoding_SetsWithFileEncoding()
+        {
+            var options = ScriptOptions.Default.WithFileEncoding(Encoding.ASCII);
+            Assert.Equal(Encoding.ASCII, options.FileEncoding);
+        }
+
         [Fact]
-        public void WithDebugInformation_SetsEmitDebugInformation()
+        public void WithFileEncoding_SameValueTwice_DoesNotCreateNewInstance()
         {
-            var options = ScriptOptions.Default.WithDebugInformation();
-            Assert.Equal(true, options.EmitDebugInformation);
+            var options = ScriptOptions.Default.WithFileEncoding(Encoding.ASCII);
+            Assert.Same(options, options.WithFileEncoding(Encoding.ASCII));
         }
     }
 }