Simplify the NopClosers

* Rename
  * `newNopCloser()` → `newReaderNopCloser()`
  * `nopCloser` → `readerNopCloser`
  * `nopCloserWriterTo` → `readerWriterToNopCloser`
  * `nopWriteCloser` → `writerNopCloser`

  to help keep readers and writers straight and because only the
  `Close()` part is a NOP.

* Move `writerNopCloser` to `nop_closer.go` to be with its siblings.

Author: mhagger

pipe/command.go

@@ -74,9 +74,9 @@ func (s *commandStage) Start(
 		// See the long comment in `Pipeline.Start()` for the
 		// explanation of this special case.
 		switch stdin := stdin.(type) {
-		case nopCloser:
+		case readerNopCloser:
 			s.cmd.Stdin = stdin.Reader
-		case nopCloserWriterTo:
+		case readerWriterToNopCloser:
 			s.cmd.Stdin = stdin.Reader
 		default:
 			s.cmd.Stdin = stdin

pipe/iocopier.go

@@ -46,10 +46,10 @@ func (s *ioCopier) Start(_ context.Context, _ Env, r io.ReadCloser) (io.ReadClos
 	go func() {
 		var err error
 
-		// Unwrap nopWriteCloser to see if the underlying writer
+		// Unwrap writerNopCloser to see if the underlying writer
 		// supports ReaderFrom (e.g., for zero-copy network I/O).
 		var dst io.Writer = s.w
-		if nwc, ok := s.w.(nopWriteCloser); ok {
+		if nwc, ok := s.w.(writerNopCloser); ok {
 			dst = nwc.Writer
 		}
 

pipe/iocopier_test.go

@@ -31,7 +31,7 @@ func TestIOCopierPoolBufferUsed(t *testing.T) {
 		pw.Close()
 	}()
 	var warmBuf bytes.Buffer
-	c := newIOCopier(nopWriteCloser{&warmBuf})
+	c := newIOCopier(writerNopCloser{&warmBuf})
 	_, _ = c.Start(context.TODO(), Env{}, pr)
 	_ = c.Wait()
 
@@ -46,7 +46,7 @@ func TestIOCopierPoolBufferUsed(t *testing.T) {
 		pw.Close()
 	}()
 	var buf bytes.Buffer
-	c = newIOCopier(nopWriteCloser{&buf})
+	c = newIOCopier(writerNopCloser{&buf})
 
 	// GC clears sync.Pool, so re-warm it afterward to isolate the
 	// measurement from pool repopulation overhead.
@@ -93,7 +93,7 @@ func (w *readFromWriter) Close() error { return nil }
 
 // TestIOCopierUsesReadFrom verifies that ioCopier dispatches to
 // ReaderFrom when the destination writer supports it, even when
-// wrapped in nopWriteCloser (as happens with WithStdout).
+// wrapped in writerNopCloser (as happens with WithStdout).
 func TestIOCopierUsesReadFrom(t *testing.T) {
 	const payload = "hello readfrom\n"
 
@@ -107,7 +107,7 @@ func TestIOCopierUsesReadFrom(t *testing.T) {
 	}()
 
 	w := &readFromWriter{}
-	c := newIOCopier(nopWriteCloser{w})
+	c := newIOCopier(writerNopCloser{w})
 	_, _ = c.Start(context.TODO(), Env{}, pr)
 	_ = c.Wait()
 
@@ -117,6 +117,6 @@ func TestIOCopierUsesReadFrom(t *testing.T) {
 
 	if !w.readFromCalled.Load() {
 		t.Error("ioCopier did not call ReadFrom on destination; " +
-			"nopWriteCloser may be hiding the ReaderFrom interface")
+			"writerNopCloser may be hiding the ReaderFrom interface")
 	}
 }

pipe/nop_closer.go

@@ -6,29 +6,48 @@ package pipe
 
 import "io"
 
-// newNopCloser returns a ReadCloser with a no-op Close method wrapping
-// the provided io.Reader r.
-// If r implements io.WriterTo, the returned io.ReadCloser will implement io.WriterTo
-// by forwarding calls to r.
-func newNopCloser(r io.Reader) io.ReadCloser {
+// newReaderNopCloser returns a ReadCloser with a no-op Close method,
+// wrapping the provided io.Reader `r`. If `r` implements
+// `io.WriterTo`, the returned `io.ReadCloser` will also implement
+// `io.WriterTo` by forwarding calls to `r`.
+func newReaderNopCloser(r io.Reader) io.ReadCloser {
 	if _, ok := r.(io.WriterTo); ok {
-		return nopCloserWriterTo{r}
+		return readerWriterToNopCloser{r}
 	}
-	return nopCloser{r}
+	return readerNopCloser{r}
 }
 
-type nopCloser struct {
+// readerNopCloser is a ReadCloser that wraps a provided `io.Reader`,
+// but whose `Close()` method does nothing. We don't need to check
+// whether the wrapped reader also implements `io.WriterTo`, since
+// it's always unwrapped before use.
+type readerNopCloser struct {
 	io.Reader
 }
 
-func (nopCloser) Close() error { return nil }
+func (readerNopCloser) Close() error {
+	return nil
+}
 
-type nopCloserWriterTo struct {
+// readerWriterToNopCloser is like `readerNopCloser` except that it
+// also implements `io.WriterTo` by delegating `WriteTo()` to the
+// wrapped `io.Reader` (which must also implement `io.WriterTo`).
+type readerWriterToNopCloser struct {
 	io.Reader
 }
 
-func (nopCloserWriterTo) Close() error { return nil }
+func (readerWriterToNopCloser) Close() error { return nil }
+
+func (r readerWriterToNopCloser) WriteTo(w io.Writer) (n int64, err error) {
+	return r.Reader.(io.WriterTo).WriteTo(w)
+}
+
+// writerNopCloser is a WriteCloser that wraps a provided `io.Writer`,
+// but whose `Close()` method does nothing.
+type writerNopCloser struct {
+	io.Writer
+}
 
-func (c nopCloserWriterTo) WriteTo(w io.Writer) (n int64, err error) {
-	return c.Reader.(io.WriterTo).WriteTo(w)
+func (w writerNopCloser) Close() error {
+	return nil
 }

pipe/pipeline.go

@@ -70,14 +70,6 @@ type Pipeline struct {
 
 var emptyEventHandler = func(_ *Event) {}
 
-type nopWriteCloser struct {
-	io.Writer
-}
-
-func (w nopWriteCloser) Close() error {
-	return nil
-}
-
 type NewPipeFn func(opts ...Option) *Pipeline
 
 // NewPipeline returns a Pipeline struct with all of the `options`
@@ -114,7 +106,7 @@ func WithStdin(stdin io.Reader) Option {
 // WithStdout assigns stdout to the last command in the pipeline.
 func WithStdout(stdout io.Writer) Option {
 	return func(p *Pipeline) {
-		p.stdout = nopWriteCloser{stdout}
+		p.stdout = writerNopCloser{stdout}
 	}
 }
 
@@ -277,7 +269,7 @@ func (p *Pipeline) Start(ctx context.Context) error {
 		// own `nopCloser`, which behaves like `io.NopCloser`, except
 		// that `pipe.CommandStage` knows how to unwrap it before
 		// passing it to `exec.Cmd`.
-		nextStdin = newNopCloser(p.stdin)
+		nextStdin = newReaderNopCloser(p.stdin)
 	}
 
 	for i, s := range p.stages {
@@ -325,7 +317,7 @@ func (p *Pipeline) Start(ctx context.Context) error {
 
 func (p *Pipeline) Output(ctx context.Context) ([]byte, error) {
 	var buf bytes.Buffer
-	p.stdout = nopWriteCloser{&buf}
+	p.stdout = writerNopCloser{&buf}
 	err := p.Run(ctx)
 	return buf.Bytes(), err
 }