refactor: introduce IRepl, move IReplEngine to own file, add guards, remove redundant code, fix code formatting

adamralph · adamralph · commit bd73584ff5ab · 2014-12-21T16:31:14.000Z
diff --git a/src/ScriptCs.Contracts/IRepl.cs b/src/ScriptCs.Contracts/IRepl.cs
@@ -0,0 +1,9 @@
+﻿namespace ScriptCs.Contracts
+{
+    using System.Collections.Generic;
+
+    public interface IRepl : IScriptExecutor
+    {
+        Dictionary<string, IReplCommand> Commands { get; }
+    }
+}
diff --git a/src/ScriptCs.Contracts/IReplCommand.cs b/src/ScriptCs.Contracts/IReplCommand.cs
@@ -6,6 +6,6 @@ public interface IReplCommand
 
         string CommandName { get; }
 
-        object Execute(IScriptExecutor repl, object[] args);
+        object Execute(IRepl repl, object[] args);
     }
 }
diff --git a/src/ScriptCs.Contracts/IReplEngine.cs b/src/ScriptCs.Contracts/IReplEngine.cs
@@ -0,0 +1,9 @@
+﻿using System.Collections.Generic;
+
+namespace ScriptCs.Contracts
+{
+    public interface IReplEngine : IScriptEngine
+    {
+        ICollection<string> GetLocalVariables(ScriptPackSession scriptPackSession);
+    }
+}
diff --git a/src/ScriptCs.Contracts/IScriptEngine.cs b/src/ScriptCs.Contracts/IScriptEngine.cs
@@ -7,14 +7,14 @@ public interface IScriptEngine
         string BaseDirectory { get; set; }
 
         string CacheDirectory { get; set; }
-        
+
         string FileName { get; set; }
-            
-        ScriptResult Execute(string code, string[] scriptArgs, AssemblyReferences references, IEnumerable<string> namespaces, ScriptPackSession scriptPackSession);
-    }
 
-    public interface IReplEngine : IScriptEngine
-    {
-        ICollection<string> GetLocalVariables(ScriptPackSession scriptPackSession);
+        ScriptResult Execute(
+            string code,
+            string[] scriptArgs,
+            AssemblyReferences references,
+            IEnumerable<string> namespaces,
+            ScriptPackSession scriptPackSession);
     }
 }
diff --git a/src/ScriptCs.Contracts/IScriptExecutor.cs b/src/ScriptCs.Contracts/IScriptExecutor.cs
@@ -1,5 +1,4 @@
 ﻿using System.Collections.Generic;
-using System.Collections.ObjectModel;
 using System.Reflection;
 
 namespace ScriptCs.Contracts
diff --git a/src/ScriptCs.Contracts/ScriptCs.Contracts.csproj b/src/ScriptCs.Contracts/ScriptCs.Contracts.csproj
@@ -74,7 +74,9 @@
     <Compile Include="IPackageObject.cs" />
     <Compile Include="IPackageReference.cs" />
     <Compile Include="IReplCommand.cs" />
+    <Compile Include="IReplEngine.cs" />
     <Compile Include="IScriptEngine.cs" />
+    <Compile Include="IRepl.cs" />
     <Compile Include="IScriptExecutor.cs" />
     <Compile Include="IScriptHost.cs" />
     <Compile Include="IScriptHostFactory.cs" />
diff --git a/src/ScriptCs.Core/Repl.cs b/src/ScriptCs.Core/Repl.cs
@@ -8,7 +8,7 @@
 
 namespace ScriptCs
 {
-    public class Repl : ScriptExecutor
+    public class Repl : ScriptExecutor, IRepl
     {
         private readonly string[] _scriptArgs;
 
diff --git a/src/ScriptCs.Core/ReplCommands/AliasCommand.cs b/src/ScriptCs.Core/ReplCommands/AliasCommand.cs
@@ -1,5 +1,4 @@
-﻿using System.IO;
-using System.Linq;
+﻿using System.Linq;
 using ScriptCs.Contracts;
 
 namespace ScriptCs.ReplCommands
@@ -23,32 +22,26 @@ public string CommandName
             get { return "alias"; }
         }
 
-        public object Execute(IScriptExecutor repl, object[] args)
+        public object Execute(IRepl repl, object[] args)
         {
             Guard.AgainstNullArgument("repl", repl);
 
             if (args == null || args.Length != 2)
             {
-                return null;
-            }
-
-            var replInstance = repl as Repl;
-
-            if (replInstance == null)
-            {
+                _console.WriteLine("You must specifiy the command name and alias, e.g. :alias \"clear\" \"cls\"");
                 return null;
             }
 
             var originalCommandName = args[0].ToString();
             var aliasName = args[1].ToString();
 
-            if (replInstance.Commands.Any(x => x.Key.ToLowerInvariant() == aliasName.ToLowerInvariant()))
+            if (repl.Commands.Any(x => x.Key.ToLowerInvariant() == aliasName.ToLowerInvariant()))
             {
                 return null;
             }
 
-            var oldReplCommand = replInstance.Commands[originalCommandName];
-            replInstance.Commands[aliasName] = oldReplCommand;
+            var oldReplCommand = repl.Commands[originalCommandName];
+            repl.Commands[aliasName] = oldReplCommand;
             _console.WriteLine(string.Format("Aliased {0} as {1}", originalCommandName, aliasName));
 
             return null;
diff --git a/src/ScriptCs.Core/ReplCommands/CdCommand.cs b/src/ScriptCs.Core/ReplCommands/CdCommand.cs
@@ -15,7 +15,7 @@ public string CommandName
             get { return "cd"; }
         }
 
-        public object Execute(IScriptExecutor repl, object[] args)
+        public object Execute(IRepl repl, object[] args)
         {
             Guard.AgainstNullArgument("repl", repl);
 
diff --git a/src/ScriptCs.Core/ReplCommands/ClearCommand.cs b/src/ScriptCs.Core/ReplCommands/ClearCommand.cs
@@ -23,7 +23,7 @@ public string CommandName
             get { return "clear"; }
         }
 
-        public object Execute(IScriptExecutor repl, object[] args)
+        public object Execute(IRepl repl, object[] args)
         {
             _console.Clear();
             return null;
diff --git a/src/ScriptCs.Core/ReplCommands/CwdCommand.cs b/src/ScriptCs.Core/ReplCommands/CwdCommand.cs
@@ -24,7 +24,7 @@ public string CommandName
             get { return "cwd"; }
         }
 
-        public object Execute(IScriptExecutor repl, object[] args)
+        public object Execute(IRepl repl, object[] args)
         {
             Guard.AgainstNullArgument("repl", repl);
 
diff --git a/src/ScriptCs.Core/ReplCommands/HelpCommand.cs b/src/ScriptCs.Core/ReplCommands/HelpCommand.cs
@@ -22,16 +22,14 @@ public string CommandName
             get { return "help"; }
         }
 
-        public object Execute(IScriptExecutor repl, object[] args)
+        public object Execute(IRepl repl, object[] args)
         {
-            var typedRepl = repl as Repl;
-            if (typedRepl != null)
+            Guard.AgainstNullArgument("repl", repl);
+
+            _console.WriteLine("The following commands are available in the REPL:");
+            foreach (var command in repl.Commands.Values.OrderBy(x => x.CommandName))
             {
-                _console.WriteLine("The following commands are available in the REPL:");
-                foreach (var command in typedRepl.Commands.Values.OrderBy(x => x.CommandName))
-                {
-                    _console.WriteLine(string.Format(":{0,-15}{1,10}", command.CommandName, command.Description));
-                }
+                _console.WriteLine(string.Format(":{0,-15}{1,10}", command.CommandName, command.Description));
             }
 
             return null;
diff --git a/src/ScriptCs.Core/ReplCommands/InstallCommand.cs b/src/ScriptCs.Core/ReplCommands/InstallCommand.cs
@@ -40,7 +40,7 @@ public string CommandName
             get { return "install"; }
         }
 
-        public object Execute(IScriptExecutor repl, object[] args)
+        public object Execute(IRepl repl, object[] args)
         {
             Guard.AgainstNullArgument("repl", repl);
 
@@ -50,13 +50,12 @@ public object Execute(IScriptExecutor repl, object[] args)
             }
 
             string version = null;
-            var allowPre = false;
             if (args.Length >= 2)
             {
                 version = args[1].ToString();
             }
 
-            allowPre = args.Length >= 3 && args[2].ToString().ToUpperInvariant() == "PRE";
+            var allowPre = args.Length >= 3 && args[2].ToString().ToUpperInvariant() == "PRE";
 
             _logger.InfoFormat("Installing {0}", args[0]);
 
diff --git a/src/ScriptCs.Core/ReplCommands/ReferencesCommand.cs b/src/ScriptCs.Core/ReplCommands/ReferencesCommand.cs
@@ -15,14 +15,13 @@ public string Description
             get { return "Displays a list of assemblies referenced from the REPL context."; }
         }
 
-        public object Execute(IScriptExecutor repl, object[] args)
+        public object Execute(IRepl repl, object[] args)
         {
-            if (repl.References != null)
-            {
-                return repl.References.Assemblies.Select(x => x.FullName).Union(repl.References.PathReferences);
-            }
+            Guard.AgainstNullArgument("repl", repl);
 
-            return null;
+            return repl.References != null
+                ? repl.References.Assemblies.Select(x => x.FullName).Union(repl.References.PathReferences)
+                : null;
         }
     }
 }
diff --git a/src/ScriptCs.Core/ReplCommands/ResetCommand.cs b/src/ScriptCs.Core/ReplCommands/ResetCommand.cs
@@ -14,7 +14,7 @@ public string CommandName
             get { return "reset"; }
         }
 
-        public object Execute(IScriptExecutor repl, object[] args)
+        public object Execute(IRepl repl, object[] args)
         {
             Guard.AgainstNullArgument("repl", repl);
 
diff --git a/src/ScriptCs.Core/ReplCommands/UsingsCommand.cs b/src/ScriptCs.Core/ReplCommands/UsingsCommand.cs
@@ -14,8 +14,10 @@ public string CommandName
             get { return "usings"; }
         }
 
-        public object Execute(IScriptExecutor repl, object[] args)
+        public object Execute(IRepl repl, object[] args)
         {
+            Guard.AgainstNullArgument("repl", repl);
+
             return repl.Namespaces;
         }
     }
diff --git a/src/ScriptCs.Core/ReplCommands/VarsCommand.cs b/src/ScriptCs.Core/ReplCommands/VarsCommand.cs
@@ -14,15 +14,12 @@ public string Description
             get { return "Displays a list of variables defined within the REPL, along with their types and values."; }
         }
 
-        public object Execute(IScriptExecutor repl, object[] args)
+        public object Execute(IRepl repl, object[] args)
         {
-            var replEngine = repl.ScriptEngine as IReplEngine;
-            if (replEngine != null)
-            {
-                return replEngine.GetLocalVariables(repl.ScriptPackSession);
-            }
+            Guard.AgainstNullArgument("repl", repl);
 
-            return null;
+            var replEngine = repl.ScriptEngine as IReplEngine;
+            return replEngine != null ? replEngine.GetLocalVariables(repl.ScriptPackSession) : null;
         }
     }
 }
diff --git a/src/ScriptCs.Core/ScriptHost.cs b/src/ScriptCs.Core/ScriptHost.cs
@@ -1,7 +1,4 @@
-﻿using System;
-using System.Collections.Generic;
-using System.Reflection;
-using ScriptCs.Contracts;
+﻿using ScriptCs.Contracts;
 
 namespace ScriptCs
 {
@@ -11,6 +8,8 @@ public class ScriptHost : IScriptHost
 
         public ScriptHost(IScriptPackManager scriptPackManager, ScriptEnvironment environment)
         {
+            Guard.AgainstNullArgument("scriptPackManager", scriptPackManager);
+
             _scriptPackManager = scriptPackManager;
             Env = environment;
         }
@@ -21,15 +20,5 @@ public T Require<T>() where T : IScriptPackContext
         {
             return _scriptPackManager.Get<T>();
         }
-
-        public void GetVars()
-        {
-            var fields = this.GetType().GetFields(BindingFlags.Instance);
-
-            foreach (var field in fields)
-            {
-                Console.WriteLine(field.Name + " - " + field.GetValue(this));
-            }
-        }
     }
-}
+}
diff --git a/src/ScriptCs.Engine.Mono/MonoScriptEngine.cs b/src/ScriptCs.Engine.Mono/MonoScriptEngine.cs
@@ -12,7 +12,7 @@
 
 namespace ScriptCs.Engine.Mono
 {
-    public class MonoScriptEngine : IScriptEngine, IReplEngine
+    public class MonoScriptEngine : IReplEngine
     {
         private readonly IScriptHostFactory _scriptHostFactory;
 
diff --git a/src/ScriptCs.Engine.Roslyn/RoslynReplEngine.cs b/src/ScriptCs.Engine.Roslyn/RoslynReplEngine.cs
@@ -1,5 +1,4 @@
-﻿using System;
-using System.Collections.Generic;
+﻿using System.Collections.Generic;
 using System.Collections.ObjectModel;
 using System.Linq;
 using System.Reflection;
@@ -63,12 +62,9 @@ protected override ScriptResult Execute(string code, Session session)
         {
             Guard.AgainstNullArgument("session", session);
 
-            if (string.IsNullOrWhiteSpace(FileName) && !IsCompleteSubmission(code))
-            {
-                return ScriptResult.Incomplete;
-            }
-
-            return base.Execute(code, session);
+            return string.IsNullOrWhiteSpace(FileName) && !IsCompleteSubmission(code)
+                ? ScriptResult.Incomplete
+                : base.Execute(code, session);
         }
     }
 }
diff --git a/test/ScriptCs.Core.Tests/ReplCommands/AliasCommandTests.cs b/test/ScriptCs.Core.Tests/ReplCommands/AliasCommandTests.cs
@@ -39,12 +39,12 @@ public void ShouldAliasCommandWithNewName()
                 fs.Setup(x => x.DllCacheFolder).Returns(Path.Combine(currentDir, "cache"));
 
                 var console = new Mock<IConsole>();
-                var executor = new Repl(null, fs.Object, null, null, null, null, null, new List<IReplCommand> {dummyCommand.Object});
+                var executor = new Repl(null, fs.Object, null, null, null, null, null, new List<IReplCommand> { dummyCommand.Object });
 
                 var cmd = new AliasCommand(console.Object);
 
                 // act
-                cmd.Execute(executor, new []{"foo", "bar"});
+                cmd.Execute(executor, new[] { "foo", "bar" });
 
                 // assert
                 executor.Commands.Count.ShouldEqual(2);
diff --git a/test/ScriptCs.Core.Tests/ReplCommands/CdCommandTests.cs b/test/ScriptCs.Core.Tests/ReplCommands/CdCommandTests.cs
@@ -29,17 +29,17 @@ public void ChangesRelativePathBasedOnArg()
             {
                 // arrange
                 var fs = new Mock<IFileSystem>();
-                var executor = new Mock<IScriptExecutor>();
+                var repl = new Mock<IRepl>();
 
                 var tempPath = Path.GetTempPath().TrimEnd(Path.DirectorySeparatorChar);
 
                 fs.Setup(x => x.CurrentDirectory).Returns(Path.Combine(tempPath, "dir"));
-                executor.Setup(x => x.FileSystem).Returns(fs.Object);
+                repl.Setup(x => x.FileSystem).Returns(fs.Object);
 
                 var cmd = new CdCommand();
 
                 // act
-                cmd.Execute(executor.Object, new[] { ".." });
+                cmd.Execute(repl.Object, new[] { ".." });
 
                 // assert
                 fs.VerifySet(x => x.CurrentDirectory = tempPath, Times.Once());
diff --git a/test/ScriptCs.Core.Tests/ReplCommands/ClearCommandTests.cs b/test/ScriptCs.Core.Tests/ReplCommands/ClearCommandTests.cs
@@ -30,7 +30,7 @@ public void CallsConsoleClear()
                 var cmd = new ClearCommand(console.Object);
 
                 // act
-                cmd.Execute(new Mock<IScriptExecutor>().Object, null);
+                cmd.Execute(new Mock<IRepl>().Object, null);
 
                 // assert
                 console.Verify(x => x.Clear(), Times.Once);
diff --git a/test/ScriptCs.Core.Tests/ReplCommands/CwdCommandTests.cs b/test/ScriptCs.Core.Tests/ReplCommands/CwdCommandTests.cs
diff --git a/test/ScriptCs.Core.Tests/ReplCommands/InstallCommandTests.cs b/test/ScriptCs.Core.Tests/ReplCommands/InstallCommandTests.cs
diff --git a/test/ScriptCs.Core.Tests/ReplCommands/ReferencesCommandTests.cs b/test/ScriptCs.Core.Tests/ReplCommands/ReferencesCommandTests.cs
diff --git a/test/ScriptCs.Core.Tests/ReplCommands/ResetCommandTests.cs b/test/ScriptCs.Core.Tests/ReplCommands/ResetCommandTests.cs
diff --git a/test/ScriptCs.Core.Tests/ReplCommands/UsingsCommandTests.cs b/test/ScriptCs.Core.Tests/ReplCommands/UsingsCommandTests.cs
diff --git a/test/ScriptCs.Core.Tests/ReplCommands/VarsCommandTests.cs b/test/ScriptCs.Core.Tests/ReplCommands/VarsCommandTests.cs
diff --git a/test/ScriptCs.Core.Tests/ReplTests.cs b/test/ScriptCs.Core.Tests/ReplTests.cs
diff --git a/test/ScriptCs.Core.Tests/ScriptHostTests.cs b/test/ScriptCs.Core.Tests/ScriptHostTests.cs

Original file line number	Diff line number	Diff line change
`@@ -6,6 +6,6 @@ public interface IReplCommand`
`6`	`6`
`7`	`7`	`string CommandName { get; }`
`8`	`8`
`9`		`- object Execute(IScriptExecutor repl, object[] args);`
	`9`	`+ object Execute(IRepl repl, object[] args);`
`10`	`10`	`}`
`11`	`11`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,4 @@`
`1`	`1`	`using System.Collections.Generic;`
`2`		`-using System.Collections.ObjectModel;`
`3`	`2`	`using System.Reflection;`
`4`	`3`
`5`	`4`	`namespace ScriptCs.Contracts`
Original file line number	Diff line number	Diff line change
`@@ -8,7 +8,7 @@`
`8`	`8`
`9`	`9`	`namespace ScriptCs`
`10`	`10`	`{`
`11`		`- public class Repl : ScriptExecutor`
	`11`	`+ public class Repl : ScriptExecutor, IRepl`
`12`	`12`	`{`
`13`	`13`	`private readonly string[] _scriptArgs;`
`14`	`14`
Original file line number	Diff line number	Diff line change
`@@ -15,7 +15,7 @@ public string CommandName`
`15`	`15`	`get { return "cd"; }`
`16`	`16`	`}`
`17`	`17`
`18`		`- public object Execute(IScriptExecutor repl, object[] args)`
	`18`	`+ public object Execute(IRepl repl, object[] args)`
`19`	`19`	`{`
`20`	`20`	`Guard.AgainstNullArgument("repl", repl);`
`21`	`21`
Original file line number	Diff line number	Diff line change
`@@ -23,7 +23,7 @@ public string CommandName`
`23`	`23`	`get { return "clear"; }`
`24`	`24`	`}`
`25`	`25`
`26`		`- public object Execute(IScriptExecutor repl, object[] args)`
	`26`	`+ public object Execute(IRepl repl, object[] args)`
`27`	`27`	`{`
`28`	`28`	`_console.Clear();`
`29`	`29`	`return null;`