Merge pull request #5426 from tmat/ScriptVarsFix

Revisit APIs for accessing script variables

src/Scripting/CSharp/CSharpScriptCompiler.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.Threading;
 using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.CodeAnalysis.Text;
@@ -19,6 +20,8 @@ private CSharpScriptCompiler()
 
         public override DiagnosticFormatter DiagnosticFormatter => CSharpDiagnosticFormatter.Instance;
 
+        public override StringComparer IdentifierComparer => StringComparer.Ordinal;
+
         public override bool IsCompleteSubmission(SyntaxTree tree) => SyntaxFactory.IsCompleteSubmission(tree);
 
         public override SyntaxTree ParseSubmission(SourceText text, CancellationToken cancellationToken) =>

src/Scripting/CSharpTest/InteractiveSessionTests.cs

@@ -26,8 +26,8 @@ public class HostModel
     public class InteractiveSessionTests : TestBase
     {
         private static readonly Assembly s_lazySystemRuntimeAssembly;
-        private static readonly Assembly SystemRuntimeAssembly = s_lazySystemRuntimeAssembly ?? (s_lazySystemRuntimeAssembly = Assembly.Load(new AssemblyName("System.Runtime, Version=4.0.20.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a")));
-        private static readonly Assembly HostAssembly = typeof(InteractiveSessionTests).GetTypeInfo().Assembly;
+        internal static readonly Assembly SystemRuntimeAssembly = s_lazySystemRuntimeAssembly ?? (s_lazySystemRuntimeAssembly = Assembly.Load(new AssemblyName("System.Runtime, Version=4.0.20.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a")));
+        internal static readonly Assembly HostAssembly = typeof(InteractiveSessionTests).GetTypeInfo().Assembly;
 
         // TODO: shouldn't be needed
         private static readonly ScriptOptions OptionsWithFacades = ScriptOptions.Default.AddReferences(SystemRuntimeAssembly);

src/Scripting/CSharpTest/ScriptTests.cs

@@ -9,10 +9,16 @@
 
 #pragma warning disable RS0003 // Do not directly await a Task
 
-namespace Microsoft.CodeAnalysis.Scripting.CSharp.Test
+namespace Microsoft.CodeAnalysis.Scripting.CSharp.UnitTests
 {
     public class ScriptTests : TestBase
     {
+        public class Globals
+        {
+            public int X;
+            public int Y;
+        }
+
         [Fact]
         public void TestCreateScript()
         {
@@ -128,12 +134,6 @@ public void Do()
 , ScriptOptions.Default.WithReferences(MscorlibRef, SystemRef, SystemCoreRef, CSharpRef));
         }
 
-        public class Globals
-        {
-            public int X;
-            public int Y;
-        }
-
         [Fact]
         public async Task TestRunScriptWithGlobals()
         {
@@ -234,14 +234,68 @@ public void TestCreateMethodDelegate()
 #endif
 
         [Fact]
-        public async Task TestGetScriptVariableAfterRunningScript()
+        public async Task ScriptVariables_Chain()
+        {
+            var globals = new Globals { X = 10, Y = 20 };
+
+            var script = 
+                CSharpScript.Create(
+                    "var a = '1';",
+                    options: ScriptOptions.Default.WithReferences(InteractiveSessionTests.SystemRuntimeAssembly),
+                    globalsType: globals.GetType()).
+                ContinueWith("var b = 2u;").
+                ContinueWith("var a = 3m;").
+                ContinueWith("var x = a + b;").
+                ContinueWith("var X = Y;");
+
+            var state = await script.RunAsync(globals);
+
+            AssertEx.Equal(new[] { "a", "b", "a", "x", "X" }, state.Variables.Select(v => v.Name));
+            AssertEx.Equal(new object[] { '1', 2u, 3m, 5m, 20 }, state.Variables.Select(v => v.Value));
+            AssertEx.Equal(new Type[] { typeof(char), typeof(uint), typeof(decimal), typeof(decimal), typeof(int) }, state.Variables.Select(v => v.Type));
+
+            Assert.Equal(3m, state.GetVariable("a").Value);
+            Assert.Equal(2u, state.GetVariable("b").Value);
+            Assert.Equal(5m, state.GetVariable("x").Value);
+            Assert.Equal(20, state.GetVariable("X").Value);
+
+            Assert.Equal(null, state.GetVariable("A"));
+            Assert.Same(state.GetVariable("X"), state.GetVariable("X"));
+        }
+
+        [Fact]
+        public async Task ScriptVariable_SetValue()
+        {
+            var script = CSharpScript.Create("var x = 1;");
+
+            var s1 = await script.RunAsync();
+            s1.GetVariable("x").Value = 2;
+            Assert.Equal(2, s1.GetVariable("x").Value);
+
+            // rerunning the script from the beginning rebuilds the state:
+            var s2 = await s1.Script.RunAsync();
+            Assert.Equal(1, s2.GetVariable("x").Value);
+
+            // continuing preserves the state:
+            var s3 = await s1.ContinueWithAsync("x");
+            Assert.Equal(2, s3.GetVariable("x").Value);
+            Assert.Equal(2, s3.ReturnValue);
+        }
+
+        [Fact]
+        public async Task ScriptVariable_SetValue_Errors()
         {
-            var state = await CSharpScript.RunAsync("int x = 100;");
-            var globals = state.Variables.Names.ToList();
-            Assert.Equal(1, globals.Count);
-            Assert.Equal(true, globals.Contains("x"));
-            Assert.Equal(true, state.Variables.ContainsVariable("x"));
-            Assert.Equal(100, (int)state.Variables["x"].Value);
+            var state = await CSharpScript.RunAsync(@"
+var x = 1;
+readonly var y = 2;
+const int z = 3;
+");
+
+            Assert.Throws<ArgumentException>(() => state.GetVariable("x").Value = "str");
+            Assert.Throws<InvalidOperationException>(() => state.GetVariable("y").Value = "str");
+            Assert.Throws<InvalidOperationException>(() => state.GetVariable("z").Value = "str");
+            Assert.Throws<InvalidOperationException>(() => state.GetVariable("y").Value = 0);
+            Assert.Throws<InvalidOperationException>(() => state.GetVariable("z").Value = 0);
         }
 
         [Fact]

src/Scripting/Core/PublicAPI.Unshipped.txt

@@ -113,21 +113,17 @@ Microsoft.CodeAnalysis.Scripting.ScriptRunner<T>
 Microsoft.CodeAnalysis.Scripting.ScriptState
 Microsoft.CodeAnalysis.Scripting.ScriptState.ContinueWithAsync(string code, Microsoft.CodeAnalysis.Scripting.ScriptOptions options = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task<Microsoft.CodeAnalysis.Scripting.ScriptState<object>>
 Microsoft.CodeAnalysis.Scripting.ScriptState.ContinueWithAsync<TResult>(string code, Microsoft.CodeAnalysis.Scripting.ScriptOptions options = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task<Microsoft.CodeAnalysis.Scripting.ScriptState<TResult>>
+Microsoft.CodeAnalysis.Scripting.ScriptState.GetVariable(string name) -> Microsoft.CodeAnalysis.Scripting.ScriptVariable
 Microsoft.CodeAnalysis.Scripting.ScriptState.ReturnValue.get -> object
 Microsoft.CodeAnalysis.Scripting.ScriptState.Script.get -> Microsoft.CodeAnalysis.Scripting.Script
-Microsoft.CodeAnalysis.Scripting.ScriptState.Variables.get -> Microsoft.CodeAnalysis.Scripting.ScriptVariables
+Microsoft.CodeAnalysis.Scripting.ScriptState.Variables.get -> System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.Scripting.ScriptVariable>
 Microsoft.CodeAnalysis.Scripting.ScriptState<T>
 Microsoft.CodeAnalysis.Scripting.ScriptState<T>.ReturnValue.get -> T
 Microsoft.CodeAnalysis.Scripting.ScriptVariable
 Microsoft.CodeAnalysis.Scripting.ScriptVariable.Name.get -> string
 Microsoft.CodeAnalysis.Scripting.ScriptVariable.Type.get -> System.Type
 Microsoft.CodeAnalysis.Scripting.ScriptVariable.Value.get -> object
-Microsoft.CodeAnalysis.Scripting.ScriptVariables
-Microsoft.CodeAnalysis.Scripting.ScriptVariables.ContainsVariable(string name) -> bool
-Microsoft.CodeAnalysis.Scripting.ScriptVariables.Count.get -> int
-Microsoft.CodeAnalysis.Scripting.ScriptVariables.GetEnumerator() -> System.Collections.Generic.IEnumerator<Microsoft.CodeAnalysis.Scripting.ScriptVariable>
-Microsoft.CodeAnalysis.Scripting.ScriptVariables.Names.get -> System.Collections.Generic.IEnumerable<string>
-Microsoft.CodeAnalysis.Scripting.ScriptVariables.this[string name].get -> Microsoft.CodeAnalysis.Scripting.ScriptVariable
+Microsoft.CodeAnalysis.Scripting.ScriptVariable.Value.set -> void
 abstract Microsoft.CodeAnalysis.Scripting.ObjectFormatter.FormatArrayTypeName(System.Type arrayType, System.Array arrayOpt, Microsoft.CodeAnalysis.Scripting.ObjectFormattingOptions options) -> string
 abstract Microsoft.CodeAnalysis.Scripting.ObjectFormatter.FormatGeneratedTypeName(System.Type type) -> string
 abstract Microsoft.CodeAnalysis.Scripting.ObjectFormatter.FormatLiteral(System.DateTime value) -> string

src/Scripting/Core/ScriptCompiler.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.Text;
 using System.Threading;
 using Microsoft.CodeAnalysis.Text;
@@ -10,6 +11,7 @@ internal abstract class ScriptCompiler
     {
         public abstract Compilation CreateSubmission(Script script);
         public abstract DiagnosticFormatter DiagnosticFormatter { get; }
+        public abstract StringComparer IdentifierComparer { get; }
 
         public abstract SyntaxTree ParseSubmission(SourceText text, CancellationToken cancellationToken);
         public abstract bool IsCompleteSubmission(SyntaxTree tree);

src/Scripting/Core/ScriptExecutionState.cs

@@ -51,8 +51,13 @@ public ScriptExecutionState FreezeAndClone()
             }
         }
 
-        public int Count => _count;
-        public object this[int index] => _submissionStates[index];
+        public int SubmissionStateCount => _count;
+
+        public object GetSubmissionState(int index)
+        {
+            Debug.Assert(index >= 0 && index < _count);
+            return _submissionStates[index];
+        }
 
         internal async Task<TResult> RunSubmissionsAsync<TResult>(
             ImmutableArray<Func<object[], Task>> precedingExecutors,

src/Scripting/Core/ScriptState.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.Collections.Immutable;
 using System.Diagnostics;
 using System.Threading;
 using System.Threading.Tasks;
+using System.Reflection;
+using System.Collections.Generic;
 
 namespace Microsoft.CodeAnalysis.Scripting
 {
@@ -18,6 +22,9 @@ public abstract class ScriptState
 
         internal ScriptExecutionState ExecutionState { get; }
 
+        private ImmutableArray<ScriptVariable> _lazyVariables;
+        private IReadOnlyDictionary<string, int> _lazyVariableMap;
+
         internal ScriptState(ScriptExecutionState executionState, Script script)
         {
             Debug.Assert(executionState != null);
@@ -33,24 +40,83 @@ internal ScriptState(ScriptExecutionState executionState, Script script)
         public object ReturnValue => GetReturnValue();
         internal abstract object GetReturnValue();
 
-        private ScriptVariables _lazyVariables;
-
         /// <summary>
-        /// The global variables accessible to or declared by the script.
+        /// Returns variables defined by the scripts in the declaration order.
         /// </summary>
-        public ScriptVariables Variables
+        public ImmutableArray<ScriptVariable> Variables
         {
             get
             {
                 if (_lazyVariables == null)
                 {
-                    Interlocked.CompareExchange(ref _lazyVariables, new ScriptVariables(ExecutionState), null);
+                    ImmutableInterlocked.InterlockedInitialize(ref _lazyVariables, CreateVariables());
                 }
 
                 return _lazyVariables;
             }
         }
 
+        /// <summary>
+        /// Returns a script variable of the specified name. 
+        /// </summary> 
+        /// <remarks>
+        /// If multiple script variables are defined in the script (in distinct submissions) returns the last one.
+        /// Namve lookup is case sensitive in C# scripts and case insensitive in VB scripts.
+        /// </remarks>
+        /// <returns><see cref="ScriptVariable"/> or null, if no variable of the specified <paramref name="name"/> is defined in the script.</returns>
+        /// <exception cref="ArgumentNullException"><paramref name="name"/> is null.</exception>
+        public ScriptVariable GetVariable(string name)
+        {
+            if (name == null)
+            {
+                throw new ArgumentNullException(nameof(name));
+            }
+
+            int index;
+            return GetVariableMap().TryGetValue(name, out index) ? Variables[index] : null;
+        }
+
+        private ImmutableArray<ScriptVariable> CreateVariables()
+        {
+            var result = ImmutableArray.CreateBuilder<ScriptVariable>();
+
+            var executionState = ExecutionState;
+
+            // Don't include the globals object (slot #0)
+            for (int i = 1; i < executionState.SubmissionStateCount; i++)
+            {
+                var state = executionState.GetSubmissionState(i);
+                Debug.Assert(state != null);
+
+                foreach (var field in state.GetType().GetTypeInfo().DeclaredFields)
+                {
+                    // TODO: synthesized fields of submissions shouldn't be public
+                    if (field.IsPublic && field.Name.Length > 0 && char.IsLetterOrDigit(field.Name[0]))
+                    {
+                        result.Add(new ScriptVariable(state, field));
+                    }
+                }
+            }
+
+            return result.ToImmutable();
+        }
+
+        private IReadOnlyDictionary<string, int> GetVariableMap()
+        {
+            if (_lazyVariableMap == null)
+            {
+                var map = new Dictionary<string, int>(Script.Compiler.IdentifierComparer);
+                for (int i = 0; i < Variables.Length; i++)
+                {
+                    map[Variables[i].Name] = i;
+                }
+
+                _lazyVariableMap = map;
+            }
+
+            return _lazyVariableMap;
+        }
+
         public Task<ScriptState<object>> ContinueWithAsync(string code, ScriptOptions options = null, CancellationToken cancellationToken = default(CancellationToken))
         {
             return ContinueWithAsync<object>(code, options, cancellationToken);

src/Scripting/Core/ScriptVariable.cs

@@ -10,62 +10,58 @@ namespace Microsoft.CodeAnalysis.Scripting
     /// A variable declared by the script.
     /// </summary>
     [DebuggerDisplay("{GetDebuggerDisplay(), nq}")]
-    public class ScriptVariable
+    public sealed class ScriptVariable
     {
         private readonly object _instance;
-        private readonly MemberInfo _member;
+        private readonly FieldInfo _field;
 
-        internal ScriptVariable(object instance, MemberInfo member)
+        internal ScriptVariable(object instance, FieldInfo field)
         {
+            Debug.Assert(instance != null);
+            Debug.Assert(field != null);
+
             _instance = instance;
-            _member = member;
+            _field = field;
         }
 
         /// <summary>
         /// The name of the variable.
         /// </summary>
-        public string Name
-        {
-            get { return _member.Name; }
-        }
+        public string Name => _field.Name;
 
         /// <summary>
         /// The type of the variable.
         /// </summary>
-        public Type Type
-        {
-            get
-            {
-                var field = _member as FieldInfo;
-                if (field != null)
-                {
-                    return field.FieldType;
-                }
-
-                return ((PropertyInfo)_member).PropertyType;
-            }
-        }
+        public Type Type => _field.FieldType;
 
         /// <summary>
         /// The value of the variable after running the script.
         /// </summary>
+        /// <exception cref="InvalidOperationException">Variable is read-only or a constant.</exception>
+        /// <exception cref="ArgumentException">The type of the specified <paramref name="value"/> isn't assignable to the type of the variable.</exception>
         public object Value
         {
             get
             {
-                var field = _member as FieldInfo;
-                if (field != null)
+                return _field.GetValue(_instance);
+            }
+
+            set
+            {
+                if (_field.IsInitOnly)
                 {
-                    return field.GetValue(_instance);
+                    throw new InvalidOperationException(ScriptingResources.CannotSetReadOnlyVariable);
                 }
 
-                return ((PropertyInfo)_member).GetValue(_instance);
+                if (_field.IsLiteral)
+                {
+                    throw new InvalidOperationException(ScriptingResources.CannotSetConstantVariable);
+                }
+
+                _field.SetValue(_instance, value);
             }
         }
 
-        private string GetDebuggerDisplay()
-        {
-            return string.Format("{0}: {1}", this.Name, this.Value?.ToString() ?? "<null>");
-        }
+        private string GetDebuggerDisplay() => $"{Name}: {Value ?? "<null>"}";
     }
 }
\ No newline at end of file

src/Scripting/Core/ScriptVariables.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.Collections;
-using System.Collections.Generic;
-using System.Linq;
-using System.Reflection;
-
-namespace Microsoft.CodeAnalysis.Scripting
-{
-    /// <summary>
-    /// A collection that holds the final state of all global variables used by the script.
-    /// </summary>
-    public class ScriptVariables : IEnumerable<ScriptVariable>, IEnumerable
-    {
-        private readonly Dictionary<string, ScriptVariable> _map;
-
-        internal ScriptVariables(ScriptExecutionState executionState)
-        {
-            _map = CreateVariableMap(executionState);
-        }
-
-        public int Count
-        {
-            get { return _map.Count; }
-        }
-
-        /// <summary>
-        /// Returns the global variable with the specified name.
-        /// </summary>
-        public ScriptVariable this[string name]
-        {
-            get
-            {
-                ScriptVariable global;
-                if (_map.TryGetValue(name, out global))
-                {
-                    return global;
-                }
-                else
-                {
-                    return null;
-                }
-            }
-        }
-
-        /// <summary>
-        /// Determines if a global variable with the specified name exists.
-        /// </summary>
-        public bool ContainsVariable(string name)
-        {
-            return _map.ContainsKey(name);
-        }
-
-        /// <summary>
-        /// A list the global variable names.
-        /// </summary>
-        public IEnumerable<String> Names
-        {
-            get { return _map.Keys; }
-        }
-
-        /// <summary>
-        /// Gets an enumerator over all the variables.
-        /// </summary>
-        public IEnumerator<ScriptVariable> GetEnumerator()
-        {
-            return _map.Values.GetEnumerator();
-        }
-
-        IEnumerator IEnumerable.GetEnumerator()
-        {
-            return this.GetEnumerator();
-        }
-
-        private static Dictionary<string, ScriptVariable> CreateVariableMap(ScriptExecutionState executionState)
-        {
-            var map = new Dictionary<string, ScriptVariable>();
-
-            for (int i = 0; i < executionState.Count; i++)
-            {
-                var state = executionState[i];
-                if (state != null)
-                {
-                    AddVariables(map, state);
-                }
-            }
-
-            return map;
-        }
-
-        private static void AddVariables(Dictionary<string, ScriptVariable> map, object instance)
-        {
-            var typeInfo = instance.GetType().GetTypeInfo();
-
-            foreach (var field in typeInfo.DeclaredFields)
-            {
-                if (field.IsPublic)
-                {
-                    AddVariable(map, instance, field);
-                }
-            }
-
-            foreach (var property in typeInfo.DeclaredProperties)
-            {
-                if (property.GetMethod.IsPublic)
-                {
-                    AddVariable(map, instance, property);
-                }
-            }
-        }
-
-        private static void AddVariable(Dictionary<string, ScriptVariable> map, object instance, MemberInfo member)
-        {
-            if (member.Name.Length > 0 && char.IsLetterOrDigit(member.Name[0]) && !map.ContainsKey(member.Name))
-            {
-                map.Add(member.Name, new ScriptVariable(instance, member));
-            }
-        }
-    }
-}
\ No newline at end of file

src/Scripting/Core/Scripting.csproj

@@ -70,6 +70,11 @@
     <Compile Include="AssemblyLoader\ShadowCopy.cs" />
     <Compile Include="Resolvers\NuGetPackageResolver.cs" />
     <Compile Include="ScriptCompiler.cs" />
+    <Compile Include="ScriptingResources.Designer.cs">
+      <AutoGen>True</AutoGen>
+      <DesignTime>True</DesignTime>
+      <DependentUpon>ScriptingResources.resx</DependentUpon>
+    </Compile>
     <Compile Include="ScriptRunner.cs" />
     <Compile Include="ObjectFormatter.cs" />
     <Compile Include="ObjectFormatter.Formatter.cs" />
@@ -80,12 +85,6 @@
     <Compile Include="MemberDisplayFormat.cs" />
     <Compile Include="ObjectFormattingOptions.cs" />
     <Compile Include="ScriptExecutionState.cs" />
-    <Compile Include="ScriptingResources.Designer.cs">
-      <AutoGen>True</AutoGen>
-      <DesignTime>True</DesignTime>
-      <DependentUpon>ScriptingResources.resx</DependentUpon>
-    </Compile>
-    <Compile Include="ScriptVariables.cs" />
     <Compile Include="ScriptOptions.cs" />
     <Compile Include="ScriptState.cs" />
     <Compile Include="ScriptVariable.cs" />
@@ -108,8 +107,8 @@
   <ItemGroup>
     <EmbeddedResource Include="ScriptingResources.resx">
       <Generator>ResXFileCodeGenerator</Generator>
-      <LastGenOutput>ScriptingResources.Designer.cs</LastGenOutput>
       <SubType>Designer</SubType>
+      <LastGenOutput>ScriptingResources.Designer.cs</LastGenOutput>
     </EmbeddedResource>
   </ItemGroup>
   <ItemGroup>

src/Scripting/Core/ScriptingResources.Designer.cs

@@ -79,6 +79,24 @@ internal class ScriptingResources {
             }
         }
         
+        /// <summary>
+        ///   Looks up a localized string similar to Cannot set a constant variable.
+        /// </summary>
+        internal static string CannotSetConstantVariable {
+            get {
+                return ResourceManager.GetString("CannotSetConstantVariable", resourceCulture);
+            }
+        }
+        
+        /// <summary>
+        ///   Looks up a localized string similar to Cannot set a read-only variable.
+        /// </summary>
+        internal static string CannotSetReadOnlyVariable {
+            get {
+                return ResourceManager.GetString("CannotSetReadOnlyVariable", resourceCulture);
+            }
+        }
+        
         /// <summary>
         ///   Looks up a localized string similar to Can&apos;t assign &apos;{0}&apos; to &apos;{1}&apos;..
         /// </summary>

src/Scripting/Core/ScriptingResources.resx

@@ -140,4 +140,10 @@
   <data name="AtFileLine" xml:space="preserve">
     <value> at {0} : {1}</value>
   </data>
+  <data name="CannotSetReadOnlyVariable" xml:space="preserve">
+    <value>Cannot set a read-only variable</value>
+  </data>
+  <data name="CannotSetConstantVariable" xml:space="preserve">
+    <value>Cannot set a constant variable</value>
+  </data>
 </root>
\ No newline at end of file

src/Scripting/VisualBasic/VisualBasicScriptCompiler.vb

@@ -25,6 +25,12 @@ Namespace Microsoft.CodeAnalysis.Scripting.VisualBasic
             End Get
         End Property
 
+        Public Overrides ReadOnly Property IdentifierComparer As StringComparer
+            Get
+                Return CaseInsensitiveComparison.Comparer
+            End Get
+        End Property
+
         Public Overrides Function IsCompleteSubmission(tree As SyntaxTree) As Boolean
             ' TODO: https://github.com/dotnet/roslyn/issues/5235
             Return True