From 6650375893797acf4e4008c524563c5f2aac2cb8 Mon Sep 17 00:00:00 2001 From: Tim Voronov Date: Thu, 25 Jun 2026 02:15:19 -0400 Subject: [PATCH 1/2] Remove redundant `make vet` usage, update validation error message, and add .PHONY targets to Makefile. --- pkg/repl/output.go | 72 +++++++++++++++++ pkg/repl/output_test.go | 166 ++++++++++++++++++++++++++++++++++++++++ pkg/repl/repl.go | 3 +- 3 files changed, 239 insertions(+), 2 deletions(-) create mode 100644 pkg/repl/output.go create mode 100644 pkg/repl/output_test.go diff --git a/pkg/repl/output.go b/pkg/repl/output.go new file mode 100644 index 0000000..03a373e --- /dev/null +++ b/pkg/repl/output.go @@ -0,0 +1,72 @@ +package repl + +import ( + "errors" + "io" +) + +func writeResult(dst io.Writer, src io.ReadCloser) (err error) { + defer func() { + if closeErr := src.Close(); closeErr != nil { + if err == nil { + err = closeErr + } else { + err = errors.Join(err, closeErr) + } + } + }() + + var buf [32 * 1024]byte + var wrote bool + var last byte + + for { + n, readErr := src.Read(buf[:]) + + if n > 0 { + if err := writeAll(dst, buf[:n]); err != nil { + return err + } + + wrote = true + last = buf[n-1] + } + + if readErr == nil { + continue + } + + if errors.Is(readErr, io.EOF) { + break + } + + return readErr + } + + if wrote && last != '\n' { + return writeAll(dst, []byte("\n")) + } + + return nil +} + +// writeAll writes the entire data slice to the given writer. +// It returns io.ErrShortWrite if dst.Write returns n=0 without an error, +// indicating that the writer is not making any progress. +func writeAll(dst io.Writer, data []byte) error { + for len(data) > 0 { + n, err := dst.Write(data) + + if err != nil { + return err + } + + if n <= 0 { + return io.ErrShortWrite + } + + data = data[n:] + } + + return nil +} diff --git a/pkg/repl/output_test.go b/pkg/repl/output_test.go new file mode 100644 index 0000000..6e410b3 --- /dev/null +++ b/pkg/repl/output_test.go @@ -0,0 +1,166 @@ +package repl + +import ( + "bytes" + "errors" + "io" + "strings" + "testing" + "testing/iotest" +) + +func TestWriteResultAddsTrailingNewline(t *testing.T) { + var out bytes.Buffer + + err := writeResult(&out, io.NopCloser(strings.NewReader("1"))) + + if err != nil { + t.Fatal(err) + } + + if out.String() != "1\n" { + t.Fatalf("expected output %q, got %q", "1\n", out.String()) + } +} + +func TestWriteResultPreservesExistingTrailingNewline(t *testing.T) { + var out bytes.Buffer + + err := writeResult(&out, io.NopCloser(strings.NewReader("1\n"))) + + if err != nil { + t.Fatal(err) + } + + if out.String() != "1\n" { + t.Fatalf("expected output %q, got %q", "1\n", out.String()) + } +} + +func TestWriteResultKeepsEmptyOutputEmpty(t *testing.T) { + var out bytes.Buffer + + err := writeResult(&out, io.NopCloser(strings.NewReader(""))) + + if err != nil { + t.Fatal(err) + } + + if out.String() != "" { + t.Fatalf("expected empty output, got %q", out.String()) + } +} + +func TestWriteResultPropagatesReadError(t *testing.T) { + readErr := errors.New("read failed") + var out bytes.Buffer + + err := writeResult(&out, io.NopCloser(iotest.ErrReader(readErr))) + + if !errors.Is(err, readErr) { + t.Fatalf("expected read error %v, got %v", readErr, err) + } +} + +func TestWriteResultPropagatesWriteError(t *testing.T) { + writeErr := errors.New("write failed") + + err := writeResult(writerFunc(func([]byte) (int, error) { + return 0, writeErr + }), io.NopCloser(strings.NewReader("1"))) + + if !errors.Is(err, writeErr) { + t.Fatalf("expected write error %v, got %v", writeErr, err) + } +} + +func TestWriteResultClosesSource(t *testing.T) { + var out bytes.Buffer + src := &trackingReadCloser{Reader: strings.NewReader("1")} + + err := writeResult(&out, src) + + if err != nil { + t.Fatal(err) + } + + if !src.closed { + t.Fatal("expected source to be closed") + } +} + +type trackingReadCloser struct { + *strings.Reader + closed bool +} + +func (rc *trackingReadCloser) Close() error { + rc.closed = true + + return nil +} + +type writerFunc func([]byte) (int, error) + +func (fn writerFunc) Write(data []byte) (int, error) { + return fn(data) +} + +func TestWriteResult_CloseError(t *testing.T) { + t.Run("join errors when both fail", func(t *testing.T) { + mainErr := errors.New("main error") + closeErr := errors.New("close error") + src := &errorCloseReader{Reader: strings.NewReader("data"), closeErr: closeErr} + + err := writeResult(writerFunc(func(p []byte) (int, error) { + return 0, mainErr + }), src) + + if err == nil { + t.Fatal("expected error, got nil") + } + if !errors.Is(err, mainErr) { + t.Errorf("expected main error, got %v", err) + } + if !errors.Is(err, closeErr) { + t.Errorf("expected close error, got %v", err) + } + }) + + t.Run("return close error when main succeeds", func(t *testing.T) { + closeErr := errors.New("close error") + src := &errorCloseReader{Reader: strings.NewReader("data"), closeErr: closeErr} + + var buf bytes.Buffer + err := writeResult(&buf, src) + + if err == nil { + t.Fatal("expected error, got nil") + } + if !errors.Is(err, closeErr) { + t.Errorf("expected close error, got %v", err) + } + if buf.String() != "data\n" { + t.Errorf("expected data\\n, got %q", buf.String()) + } + }) +} + +func TestWriteAll_ZeroWrite(t *testing.T) { + err := writeAll(writerFunc(func(p []byte) (int, error) { + return 0, nil + }), []byte("data")) + + if !errors.Is(err, io.ErrShortWrite) { + t.Errorf("expected io.ErrShortWrite, got %v", err) + } +} + +type errorCloseReader struct { + io.Reader + closeErr error +} + +func (e *errorCloseReader) Close() error { + return e.closeErr +} diff --git a/pkg/repl/repl.go b/pkg/repl/repl.go index 9831492..a389194 100644 --- a/pkg/repl/repl.go +++ b/pkg/repl/repl.go @@ -3,7 +3,6 @@ package repl import ( "context" "fmt" - "io" "os" "strings" @@ -99,7 +98,7 @@ func Start(ctx context.Context, opts runtime.Options, params map[string]interfac continue } - _, err = io.Copy(os.Stdout, out) + err = writeResult(os.Stdout, out) if err != nil { fmt.Fprintln(os.Stderr, err) From 240012d3046a362274599dbc7993af5577a5f195 Mon Sep 17 00:00:00 2001 From: Tim Voronov Date: Thu, 25 Jun 2026 15:43:18 -0400 Subject: [PATCH 2/2] Refactor tests to use blank identifier for unused `[]byte` parameter in writer function --- pkg/repl/output_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/repl/output_test.go b/pkg/repl/output_test.go index 6e410b3..ab27589 100644 --- a/pkg/repl/output_test.go +++ b/pkg/repl/output_test.go @@ -112,7 +112,7 @@ func TestWriteResult_CloseError(t *testing.T) { closeErr := errors.New("close error") src := &errorCloseReader{Reader: strings.NewReader("data"), closeErr: closeErr} - err := writeResult(writerFunc(func(p []byte) (int, error) { + err := writeResult(writerFunc(func(_ []byte) (int, error) { return 0, mainErr }), src) @@ -147,7 +147,7 @@ func TestWriteResult_CloseError(t *testing.T) { } func TestWriteAll_ZeroWrite(t *testing.T) { - err := writeAll(writerFunc(func(p []byte) (int, error) { + err := writeAll(writerFunc(func(_ []byte) (int, error) { return 0, nil }), []byte("data"))