Implement better error messages for accessing standardoutput/standard…

…error on iocommands (fix #59).

MedallionShell.Tests/IoCommandTest.cs

@@ -0,0 +1,72 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+using Medallion.Shell.Tests;
+using NUnit.Framework;
+
+namespace MedallionShell.Tests
+{
+    using static UnitTestHelpers;
+
+    public class IOCommandTest
+    {
+        [Test]
+        public void TestStandardOutCannotBeAccessedAfterRedirectingIt()
+        {
+            var output = new List<string>();
+            var command = TestShell.Run(SampleCommand, "argecho", "a");
+            var ioCommand = command.RedirectTo(output);
+
+            var errorMessage = Assert.Throws<InvalidOperationException>(() => ioCommand.StandardOutput.GetHashCode()).Message;
+            errorMessage.ShouldEqual("StandardOutput is unavailable because it is already being piped to System.Collections.Generic.List`1[System.String]");
+
+            Assert.DoesNotThrow(() => command.StandardOutput.GetHashCode());
+
+            Assert.Throws<InvalidOperationException>(() => ioCommand.Result.StandardOutput.GetHashCode())
+                .Message
+                .ShouldEqual(errorMessage);
+            Assert.Throws<ObjectDisposedException>(() => command.Result.StandardOutput.GetHashCode());
+
+            CollectionAssert.AreEqual(new[] { "a" }, output);
+            ioCommand.Result.StandardError.ShouldEqual(command.Result.StandardError).ShouldEqual(string.Empty);
+        }
+
+        [Test]
+        public void TestStandardErrorCannotBeAccessedAfterRedirectingIt()
+        {
+            var output = new List<string>();
+            var command = TestShell.Run(SampleCommand, "argecho", "a");
+            var ioCommand = command.RedirectStandardErrorTo(output);
+
+            var errorMessage = Assert.Throws<InvalidOperationException>(() => ioCommand.StandardError.GetHashCode()).Message;
+            errorMessage.ShouldEqual("StandardError is unavailable because it is already being piped to System.Collections.Generic.List`1[System.String]");
+
+            Assert.DoesNotThrow(() => command.StandardError.GetHashCode());
+
+            Assert.Throws<InvalidOperationException>(() => ioCommand.Result.StandardError.GetHashCode())
+                .Message
+                .ShouldEqual(errorMessage);
+            Assert.Throws<ObjectDisposedException>(() => command.Result.StandardError.GetHashCode());
+
+            Assert.IsEmpty(output);
+            ioCommand.Result.StandardOutput.ShouldEqual(command.Result.StandardOutput).ShouldEqual("a\r\n");
+        }
+
+        [Test]
+        public void TestStandardInputCannotBeAccessedAfterRedirectingIt()
+        {
+            var command = TestShell.Run(SampleCommand, "echo");
+            var ioCommand = command.RedirectFrom(new[] { "a" });
+
+            var errorMessage = Assert.Throws<InvalidOperationException>(() => ioCommand.StandardInput.GetHashCode()).Message;
+            errorMessage.ShouldEqual("StandardInput is unavailable because it is already being piped from System.String[]");
+
+            Assert.DoesNotThrow(() => command.StandardInput.GetHashCode());
+
+            ioCommand.Result.StandardOutput.ShouldEqual(command.Result.StandardOutput).ShouldEqual("a\r\n");
+            ioCommand.Result.StandardError.ShouldEqual(command.Result.StandardError).ShouldEqual(string.Empty);
+        }
+    }
+}

MedallionShell/Command.cs

@@ -136,7 +136,7 @@ public Command RedirectTo(Stream stream)
         {
             Throw.IfNull(stream, nameof(stream));
 
-            return new IoCommand(this, this.StandardOutput.PipeToAsync(stream, leaveStreamOpen: true), ">", stream);
+            return new IOCommand(this, this.StandardOutput.PipeToAsync(stream, leaveStreamOpen: true), StandardIOStream.Out, stream);
         }
 
         /// <summary>
@@ -148,7 +148,7 @@ public Command RedirectStandardErrorTo(Stream stream)
         {
             Throw.IfNull(stream, nameof(stream));
 
-            return new IoCommand(this, this.StandardError.PipeToAsync(stream, leaveStreamOpen: true), "2>", stream);
+            return new IOCommand(this, this.StandardError.PipeToAsync(stream, leaveStreamOpen: true), StandardIOStream.Error, stream);
         }
 
         /// <summary>
@@ -160,7 +160,7 @@ public Command RedirectFrom(Stream stream)
         {
             Throw.IfNull(stream, nameof(stream));
 
-            return new IoCommand(this, this.StandardInput.PipeFromAsync(stream, leaveStreamOpen: true), "<", stream);
+            return new IOCommand(this, this.StandardInput.PipeFromAsync(stream, leaveStreamOpen: true), StandardIOStream.In, stream);
         }
 
         /// <summary>
@@ -172,7 +172,7 @@ public Command RedirectTo(FileInfo file)
         {
             Throw.IfNull(file, nameof(file));
 
-            return new IoCommand(this, this.StandardOutput.PipeToAsync(file), ">", file);
+            return new IOCommand(this, this.StandardOutput.PipeToAsync(file), StandardIOStream.Out, file);
         }
 
         /// <summary>
@@ -184,7 +184,7 @@ public Command RedirectStandardErrorTo(FileInfo file)
         {
             Throw.IfNull(file, nameof(file));
 
-            return new IoCommand(this, this.StandardError.PipeToAsync(file), "2>", file);
+            return new IOCommand(this, this.StandardError.PipeToAsync(file), StandardIOStream.Error, file);
         }
 
         /// <summary>
@@ -196,7 +196,7 @@ public Command RedirectFrom(FileInfo file)
         {
             Throw.IfNull(file, nameof(file));
 
-            return new IoCommand(this, this.StandardInput.PipeFromAsync(file), "<", file);
+            return new IOCommand(this, this.StandardInput.PipeFromAsync(file), StandardIOStream.In, file);
         }
 
         /// <summary>
@@ -208,7 +208,7 @@ public Command RedirectTo(ICollection<string> lines)
         {
             Throw.IfNull(lines, nameof(lines));
 
-            return new IoCommand(this, this.StandardOutput.PipeToAsync(lines), ">", lines.GetType());
+            return new IOCommand(this, this.StandardOutput.PipeToAsync(lines), StandardIOStream.Out, lines.GetType());
         }
 
         /// <summary>
@@ -220,7 +220,7 @@ public Command RedirectStandardErrorTo(ICollection<string> lines)
         {
             Throw.IfNull(lines, nameof(lines));
 
-            return new IoCommand(this, this.StandardError.PipeToAsync(lines), "2>", lines.GetType());
+            return new IOCommand(this, this.StandardError.PipeToAsync(lines), StandardIOStream.Error, lines.GetType());
         }
 
         /// <summary>
@@ -232,7 +232,7 @@ public Command RedirectFrom(IEnumerable<string> lines)
         {
             Throw.IfNull(lines, nameof(lines));
 
-            return new IoCommand(this, this.StandardInput.PipeFromAsync(lines), "<", lines.GetType());
+            return new IOCommand(this, this.StandardInput.PipeFromAsync(lines), StandardIOStream.In, lines.GetType());
         }
 
         /// <summary>
@@ -244,7 +244,7 @@ public Command RedirectTo(ICollection<char> chars)
         {
             Throw.IfNull(chars, nameof(chars));
 
-            return new IoCommand(this, this.StandardOutput.PipeToAsync(chars), ">", chars.GetType());
+            return new IOCommand(this, this.StandardOutput.PipeToAsync(chars), StandardIOStream.Out, chars.GetType());
         }
 
         /// <summary>
@@ -256,7 +256,7 @@ public Command RedirectStandardErrorTo(ICollection<char> chars)
         {
             Throw.IfNull(chars, nameof(chars));
 
-            return new IoCommand(this, this.StandardError.PipeToAsync(chars), "2>", chars.GetType());
+            return new IOCommand(this, this.StandardError.PipeToAsync(chars), StandardIOStream.Error, chars.GetType());
         }
 
         /// <summary>
@@ -268,7 +268,7 @@ public Command RedirectFrom(IEnumerable<char> chars)
         {
             Throw.IfNull(chars, nameof(chars));
 
-            return new IoCommand(this, this.StandardInput.PipeFromAsync(chars), "<", chars.GetType());
+            return new IOCommand(this, this.StandardInput.PipeFromAsync(chars), StandardIOStream.In, chars.GetType());
         }
 
         /// <summary>
@@ -280,7 +280,7 @@ public Command RedirectTo(TextWriter writer)
         {
             Throw.IfNull(writer, nameof(writer));
 
-            return new IoCommand(this, this.StandardOutput.PipeToAsync(writer, leaveWriterOpen: true), ">", writer);
+            return new IOCommand(this, this.StandardOutput.PipeToAsync(writer, leaveWriterOpen: true), StandardIOStream.Out, writer);
         }
 
         /// <summary>
@@ -292,7 +292,7 @@ public Command RedirectStandardErrorTo(TextWriter writer)
         {
             Throw.IfNull(writer, nameof(writer));
 
-            return new IoCommand(this, this.StandardError.PipeToAsync(writer, leaveWriterOpen: true), "2>", writer);
+            return new IOCommand(this, this.StandardError.PipeToAsync(writer, leaveWriterOpen: true), StandardIOStream.Error, writer);
         }
 
         /// <summary>
@@ -304,7 +304,7 @@ public Command RedirectFrom(TextReader reader)
         {
             Throw.IfNull(reader, nameof(reader));
 
-            return new IoCommand(this, this.StandardInput.PipeFromAsync(reader, leaveReaderOpen: true), "<", reader);
+            return new IOCommand(this, this.StandardInput.PipeFromAsync(reader, leaveReaderOpen: true), StandardIOStream.In, reader);
         }
 
         /// <summary>

MedallionShell/CommandResult.cs

@@ -15,30 +15,35 @@ public sealed class CommandResult
         private readonly Lazy<string> standardOutput, standardError;
 
         internal CommandResult(int exitCode, Command command)
+            : this(exitCode, () => command.StandardOutput.ReadToEnd(), () => command.StandardError.ReadToEnd())
+        {
+        }
+
+        internal CommandResult(int exitCode, Func<string> standardOutput, Func<string> standardError)
         {
             this.ExitCode = exitCode;
-            this.standardOutput = new Lazy<string>(() => command.StandardOutput.ReadToEnd());
-            this.standardError = new Lazy<string>(() => command.StandardError.ReadToEnd());
+            this.standardOutput = new Lazy<string>(standardOutput);
+            this.standardError = new Lazy<string>(standardError);
         }
 
         /// <summary>
         /// The exit code of the command's process
         /// </summary>
-        public int ExitCode { get; private set; }
+        public int ExitCode { get; }
 
         /// <summary>
         /// Returns true iff the exit code is 0 (indicating success)
         /// </summary>
-        public bool Success { get { return this.ExitCode == 0; } }
+        public bool Success => this.ExitCode == 0;
 
         /// <summary>
         /// If available, the full standard output text of the command
         /// </summary>
-        public string StandardOutput { get { return this.standardOutput.Value; } }
+        public string StandardOutput => this.standardOutput.Value;
 
         /// <summary>
         /// If available, the full standard error text of the command
         /// </summary>
-        public string StandardError { get { return this.standardError.Value; } }
+        public string StandardError => this.standardError.Value;
     }
 }

MedallionShell/IoCommand.cs

@@ -6,26 +6,46 @@
 
 namespace Medallion.Shell
 {
-    internal sealed class IoCommand : Command
+    internal sealed class IOCommand : Command
     {
         private readonly Command command;
         private readonly Task<CommandResult> task;
+        private readonly StandardIOStream standardIOStream;
         // for toString
-        private readonly string @operator;
         private readonly object sourceOrSink;
 
-        public IoCommand(Command command, Task ioTask, string @operator, object sourceOrSink)
+        public IOCommand(Command command, Task ioTask, StandardIOStream standardIOStream, object sourceOrSink)
         {
             this.command = command;
             this.task = this.CreateTask(ioTask);
-            this.@operator = @operator;
+            this.standardIOStream = standardIOStream;
             this.sourceOrSink = sourceOrSink;
         }
 
         private async Task<CommandResult> CreateTask(Task ioTask)
         {
             await ioTask.ConfigureAwait(false);
-            return await this.command.Task.ConfigureAwait(false);
+            var innerResult = await this.command.Task.ConfigureAwait(false);
+
+            // We wrap the inner command's result so that we can apply our stream availability error
+            // checking (the Ignore() calls). However, we use the inner result's string values since
+            // accessing those consumes the stream and we want both this result and the inner result
+            // to have the value.
+            return new CommandResult(
+                innerResult.ExitCode,
+                standardOutput: () =>
+                {
+                    Ignore(this.StandardOutput);
+                    return innerResult.StandardOutput;
+                },
+                standardError: () =>
+                {
+                    Ignore(this.StandardError);
+                    return innerResult.StandardError;
+                }
+            );
+
+            void Ignore(object ignored) { }
         }
 
         public override System.Diagnostics.Process Process
@@ -41,20 +61,17 @@ public override IReadOnlyList<System.Diagnostics.Process> Processes
         public override int ProcessId => this.command.ProcessId;
         public override IReadOnlyList<int> ProcessIds => this.command.ProcessIds;
 
-        public override Streams.ProcessStreamWriter StandardInput
-        {
-            get { return this.command.StandardInput; }
-        }
+        public override Streams.ProcessStreamWriter StandardInput => this.standardIOStream != StandardIOStream.In
+            ? this.command.StandardInput
+            : throw new InvalidOperationException($"{nameof(this.StandardInput)} is unavailable because it is already being piped from {this.sourceOrSink}");
 
-        public override Streams.ProcessStreamReader StandardOutput
-        {
-            get { return this.command.StandardOutput; }
-        }
+        public override Streams.ProcessStreamReader StandardOutput => this.standardIOStream != StandardIOStream.Out
+            ? this.command.StandardOutput
+            : throw new InvalidOperationException($"{nameof(this.StandardOutput)} is unavailable because it is already being piped to {this.sourceOrSink}");
 
-        public override Streams.ProcessStreamReader StandardError
-        {
-            get { return this.command.StandardError; }
-        }
+        public override Streams.ProcessStreamReader StandardError => this.standardIOStream != StandardIOStream.Error
+            ? this.command.StandardError
+            : throw new InvalidOperationException($"{nameof(this.StandardError)} is unavailable because it is already being piped to {this.sourceOrSink}");
 
         public override Task<CommandResult> Task
         {
@@ -66,11 +83,26 @@ public override void Kill()
             this.command.Kill();
         }
 
-        public override string ToString() => $"{this.command} {this.@operator} {this.sourceOrSink}";
+        public override string ToString() => $"{this.command} {ToString(this.standardIOStream)} {this.sourceOrSink}";
 
         protected override void DisposeInternal()
         {
             this.command.As<IDisposable>().Dispose();
         }
+
+        private static string ToString(StandardIOStream standardIOStream) => standardIOStream switch
+        {
+            StandardIOStream.In => "<",
+            StandardIOStream.Out => ">",
+            StandardIOStream.Error => "2>",
+            _ => throw new InvalidOperationException("should never get here")
+        };
+    }
+
+    internal enum StandardIOStream
+    {
+        In,
+        Out,
+        Error,
     }
 }