perf(cli)!: write compact JSON through a streaming encoder

README.md

@@ -93,8 +93,10 @@ go build -o morphic ./cmd/morphic
 ### CLI
 
 `morphic compile` lowers one OpenAPI 3.x spec into Morphic IR JSON on stdout, and writes
-diagnostics to stderr. `morphic validate` runs the same pipeline over the same spec for the
-diagnostics and the exit code alone, writing no IR anywhere.
+diagnostics to stderr. Stdout is indented for reading; a file written with `-o` is compact, which
+is about half the bytes, unless `--pretty` asks for the indented form. `morphic validate` runs the
+same pipeline over the same spec for the diagnostics and the exit code alone, writing no IR
+anywhere.
 
 ```bash
 morphic compile openapi.yaml                 # IR JSON to stdout
@@ -117,7 +119,8 @@ Help always prints to stdout and exits `0`.
 |---|---|---|
 | `--fail-on error\|warning` | both | Exit non-zero when a diagnostic at or above this severity is emitted (default `error`). |
 | `--skip-validate` | both | Skip the referential-integrity `validate` pass. |
-| `-o <file>` | `compile` | Write IR JSON to `<file>` instead of stdout. |
+| `-o <file>` | `compile` | Write IR JSON to `<file>` instead of stdout, compact rather than indented. |
+| `--pretty` | `compile` | Indent the JSON `-o` writes; stdout is indented either way. |
 | `--explain <json-pointer>` | `compile` | Report what compiling produced at this source coordinate instead of writing the document. |
 
 Diagnostics print one per line as `<severity> <code> <location>: <message>`, where `<location>` is

cmd/morphic/args_test.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"bytes"
+	"encoding/json"
 	"os"
 	"path/filepath"
 	"testing"
@@ -137,7 +138,14 @@ func TestRun_TerminatorAsFlagValue(t *testing.T) {
 	require.Equal(t, 0, code, "stderr: %s", stderr.String())
 	raw, err := os.ReadFile(filepath.Join(dir, "--"))
 	require.NoError(t, err)
-	assert.Contains(t, string(raw), `"name": "Tiny"`,
+	// Decoded rather than matched as text: this is about which argument "--"
+	// became, and -o's JSON is compact while stdout's is indented, so a literal
+	// would pin the formatting of a test that has no stake in it.
+	var doc struct {
+		Name string `json:"name"`
+	}
+	require.NoError(t, json.Unmarshal(raw, &doc))
+	assert.Equal(t, "Tiny", doc.Name,
 		`"--" must be consumed as -o's value, leaving the flags after the spec to parse`)
 }
 

cmd/morphic/command_test.go

@@ -105,7 +105,7 @@ func TestSpecFlags_SharedFlagsAgree(t *testing.T) {
 // asserted from both the constructor and the command-table entry so the two
 // cannot drift.
 var (
-	compileFlagNames  = []string{"o", "fail-on", "skip-validate", "explain"}
+	compileFlagNames  = []string{"o", "fail-on", "skip-validate", "explain", "pretty"}
 	validateFlagNames = []string{"fail-on", "skip-validate"}
 )
 

cmd/morphic/compile.go

@@ -67,7 +67,8 @@ func newCompileCommand() command {
 		summary: "lower an API spec (OpenAPI 3.x) into Morphic IR JSON",
 		usage:   "morphic compile <spec-file> [flags]",
 		description: "Lower an API spec (OpenAPI 3.x) into Morphic IR JSON on stdout, and write\n" +
-			"diagnostics to stderr.\n\n" +
+			"diagnostics to stderr. Output to stdout is indented for reading; a file\n" +
+			"written with -o is compact unless --pretty asks for the indented form.\n\n" +
 			"--explain reports what compiling produced at one source coordinate — the\n" +
 			"type node interned there, the coordinates interned beneath it, and the\n" +
 			"diagnostics stamped at it — instead of writing the document.\n\n" +
@@ -95,6 +96,7 @@ type compileOptions struct {
 	specOptions
 	outPath string
 	explain string
+	pretty  bool
 }
 
 // bindSpecFlags registers the flags every spec-taking command shares onto fs.
@@ -121,6 +123,8 @@ func newCompileFlags() (*flag.FlagSet, *compileOptions) {
 	fs.StringVar(&opts.outPath, "o", "", "write IR JSON to this file instead of stdout")
 	fs.StringVar(&opts.explain, "explain", "",
 		"report what compiling produced at this source pointer instead of writing IR JSON")
+	fs.BoolVar(&opts.pretty, "pretty", false,
+		"indent the IR JSON -o writes; stdout is indented either way")
 
 	return fs, &opts
 }
@@ -201,7 +205,7 @@ func compileSpec(specPath string, opts compileOptions, stdout, stderr io.Writer)
 		explainDocument(stdout, res.Document, res.Diagnostics, opts.explain)
 		return code
 	}
-	if err := writeCompiled(opts.outPath, stdout, res.Document); err != nil {
+	if err := writeCompiled(opts, stdout, res.Document); err != nil {
 		emitf(stderr, "morphic: %v\n", err)
 		return 2
 	}
@@ -303,26 +307,30 @@ func severityRank(s ir.Severity) int {
 	}
 }
 
-// writeCompiled emits doc's pretty IR JSON to outPath, or to stdout when outPath
-// is empty. For a file destination, doc is marshalled in full before any file is
-// touched, so a failed marshal never disturbs a file already there — see
-// marshalDocument — and the bytes are then published atomically by replaceFile.
-func writeCompiled(outPath string, stdout io.Writer, doc *ir.Document) error {
-	if outPath == "" {
-		return writeDocument(stdout, doc)
-	}
-	raw, err := marshalDocument(doc)
-	if err != nil {
-		return err
-	}
-	return replaceFile(outPath, raw)
+// writeCompiled emits doc's IR JSON to opts.outPath, or to stdout when it is
+// empty. Stdout is indented because a person is reading it; a file is compact,
+// which is about half the bytes, unless --pretty asks for the indented form.
+//
+// A file destination is encoded straight into replaceFile's temp file rather
+// than into a slice handed over afterwards. That drops a whole copy of the
+// output and keeps the property the marshal-first order used to provide: a
+// document that will not marshal removes the temp file and leaves whatever is
+// at outPath untouched, because outPath is only ever reached by the rename.
+func writeCompiled(opts compileOptions, stdout io.Writer, doc *ir.Document) error {
+	if opts.outPath == "" {
+		return encodeDocument(stdout, doc, true)
+	}
+	return replaceFile(opts.outPath, func(w io.Writer) error {
+		return encodeDocument(w, doc, opts.pretty)
+	})
 }
 
-// replaceFile writes raw to outPath atomically: the bytes land in a temp file in
-// the destination's own directory — so the publishing rename never crosses a
-// filesystem boundary — and replace outPath only once all of them are on disk.
-// A failed or partial write therefore leaves outPath's previous content intact
-// instead of truncating it.
+// replaceFile writes what fill produces to outPath atomically: fill's bytes land
+// in a temp file in the destination's own directory — so the publishing rename
+// never crosses a filesystem boundary — and replace outPath only once all of
+// them are on disk. A failed or partial write therefore leaves outPath's
+// previous content intact instead of truncating it, and so does a fill that
+// fails halfway through.
 //
 // Publishing by rename replaces the directory entry rather than the bytes behind
 // it, which is what makes the swap atomic and which costs four things a
@@ -354,13 +362,13 @@ func writeCompiled(outPath string, stdout io.Writer, doc *ir.Document) error {
 // property this function exists to provide; making the swap itself survive a
 // crash would need an fsync on the parent directory and is deliberately out of
 // scope.
-func replaceFile(outPath string, raw []byte) error {
+func replaceFile(outPath string, fill func(io.Writer) error) error {
 	perm, replacing, err := destMode(outPath)
 	if err != nil {
 		return err
 	}
 
-	tmp, err := writeTemp(outPath, raw)
+	tmp, err := writeTemp(outPath, fill)
 	if err != nil {
 		return err
 	}
@@ -395,11 +403,11 @@ func destMode(outPath string) (os.FileMode, bool, error) {
 	return info.Mode().Perm(), true, nil
 }
 
-// writeTemp writes raw to a newly created file beside outPath and returns that
+// writeTemp runs fill into a newly created file beside outPath and returns that
 // file's path. Creation is O_EXCL under a random name, so it neither clobbers an
 // existing file nor follows a symlink planted at the name it drew; that makes the
 // unpredictability of the suffix a convenience, not a security boundary.
-func writeTemp(outPath string, raw []byte) (string, error) {
+func writeTemp(outPath string, fill func(io.Writer) error) (string, error) {
 	dir := filepath.Dir(outPath)
 	base := filepath.Base(outPath)
 
@@ -412,7 +420,7 @@ func writeTemp(outPath string, raw []byte) (string, error) {
 		if err != nil {
 			return "", fmt.Errorf("create output %q: %w", outPath, err)
 		}
-		if err := fillTemp(f, tmp, raw); err != nil {
+		if err := fillTemp(f, tmp, fill); err != nil {
 			return "", err
 		}
 		return tmp, nil
@@ -421,17 +429,17 @@ func writeTemp(outPath string, raw []byte) (string, error) {
 		outPath, maxTempAttempts)
 }
 
-// fillTemp writes raw to f, flushes it to the filesystem, and closes it,
+// fillTemp runs fill into f, flushes it to the filesystem, and closes it,
 // removing tmp if any step fails so a failed run leaves no debris beside the
-// destination.
+// destination — including a fill that fails after writing part of its output.
 //
 // The Sync is what lets replaceFile's caller believe the rename publishes
 // durable bytes: without it the rename can expose a file whose contents are
 // still only in the page cache, and a crash before writeback leaves the
 // destination short or empty — the same loss writing through a temp file exists
 // to prevent.
-func fillTemp(f outputFile, tmp string, raw []byte) error {
-	if err := writeRaw(f, raw); err != nil {
+func fillTemp(f outputFile, tmp string, fill func(io.Writer) error) error {
+	if err := fill(f); err != nil {
 		_ = f.Close()
 		_ = os.Remove(tmp)
 		return err
@@ -448,26 +456,50 @@ func fillTemp(f outputFile, tmp string, raw []byte) error {
 	return nil
 }
 
-// marshalDocument renders doc to indented JSON with a trailing newline (the
-// same bytes as irtest.WriteGolden). It is factored out of writeDocument so
-// writeCompiled can marshal a file destination fully in memory before it ever
-// opens — and so truncates — outPath.
-func marshalDocument(doc *ir.Document) ([]byte, error) {
-	raw, err := json.MarshalIndent(doc, "", "  ")
+// destWriter passes writes through to w and records the error w returned. It is
+// what keeps a destination that refused the bytes distinguishable from a
+// document that would not marshal: json.Encoder reports both as one error from
+// Encode, and they are different things to tell a user about.
+type destWriter struct {
+	w   io.Writer
+	err error
+}
+
+func (d *destWriter) Write(p []byte) (int, error) {
+	n, err := d.w.Write(p)
 	if err != nil {
-		return nil, fmt.Errorf("marshal ir document: %w", err)
+		d.err = err
 	}
-	return append(raw, '\n'), nil
+	return n, err
 }
 
-// writeDocument marshals doc to indented JSON with a trailing newline (the same
-// bytes as irtest.WriteGolden) and writes it to w.
-func writeDocument(w io.Writer, doc *ir.Document) error {
-	raw, err := marshalDocument(doc)
-	if err != nil {
-		return err
+// encodeDocument writes doc's IR JSON to w, ending in a newline either way.
+//
+// The two forms take different routes on purpose, and neither is the obvious
+// choice for the other. Indented output goes through json.MarshalIndent — the
+// same bytes irtest.WriteGolden writes — because a json.Encoder with SetIndent
+// indents into a second buffer it keeps for itself, which costs roughly twice
+// the allocation for an identical result. Compact output goes through the
+// encoder precisely because there is no second pass: it hands the marshalled
+// bytes to w directly, where json.Marshal would first copy them into a slice
+// for the caller to write.
+func encodeDocument(w io.Writer, doc *ir.Document, indent bool) error {
+	if indent {
+		raw, err := json.MarshalIndent(doc, "", "  ")
+		if err != nil {
+			return fmt.Errorf("marshal ir document: %w", err)
+		}
+		return writeRaw(w, append(raw, '\n'))
 	}
-	return writeRaw(w, raw)
+
+	dest := &destWriter{w: w}
+	if err := json.NewEncoder(dest).Encode(doc); err != nil {
+		if dest.err != nil {
+			return fmt.Errorf("write ir document: %w", dest.err)
+		}
+		return fmt.Errorf("marshal ir document: %w", err)
+	}
+	return nil
 }
 
 // writeRaw writes raw to w, wrapping any error with context.

cmd/morphic/compile_test.go

@@ -8,6 +8,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -138,3 +139,95 @@ func TestRun_UsageErrors(t *testing.T) {
 		})
 	}
 }
+
+// unsortedSpec declares its schemas out of alphabetical order and its paths in
+// an order of their own, so a document compiled from it exercises both halves
+// of the determinism invariant: maps emitted in sorted-key order and slices in
+// source order.
+const unsortedSpec = `openapi: 3.1.0
+info: {title: Ordered, version: "1"}
+paths:
+  /zebra:
+    get: {operationId: getZebra, responses: {"200": {description: ok}}}
+  /apple:
+    get: {operationId: getApple, responses: {"200": {description: ok}}}
+components:
+  schemas:
+    Zeta: {type: object, properties: {b: {type: string}, a: {type: integer}}}
+    Alpha: {type: string}
+    Mid: {type: array, items: {type: string}}
+`
+
+// compileToFile runs compile over spec with the given extra flags and returns
+// the bytes -o wrote.
+func compileToFile(t *testing.T, spec string, flags ...string) []byte {
+	t.Helper()
+	out := filepath.Join(t.TempDir(), "ir.json")
+	args := append([]string{"compile", spec, "-o", out}, flags...)
+	var stdout, stderr bytes.Buffer
+
+	require.Equal(t, 0, run(args, &stdout, &stderr), "stderr: %s", stderr.String())
+
+	raw, err := os.ReadFile(out)
+	require.NoError(t, err)
+	return raw
+}
+
+// TestRun_FileOutputIsCompact pins the artifact format: -o writes compact JSON,
+// which is one line and a trailing newline, since a raw newline cannot appear
+// inside a JSON string.
+func TestRun_FileOutputIsCompact(t *testing.T) {
+	t.Parallel()
+	spec := writeFile(t, "spec.yaml", testspec.Tiny)
+
+	raw := compileToFile(t, spec)
+
+	assert.Equal(t, 1, bytes.Count(raw, []byte("\n")),
+		"compact JSON is one line plus its trailing newline, got:\n%s", raw)
+	var doc ir.Document
+	require.NoError(t, json.Unmarshal(raw, &doc))
+	assert.Equal(t, "Tiny", doc.Name)
+}
+
+// TestRun_PrettyRestoresIndentedFile pins the escape hatch, and pins it against
+// the bytes that were there before rather than against "looks indented": -o
+// -pretty must write exactly what stdout writes, which is the format -o itself
+// used to write.
+func TestRun_PrettyRestoresIndentedFile(t *testing.T) {
+	t.Parallel()
+	spec := writeFile(t, "spec.yaml", testspec.Tiny)
+
+	raw := compileToFile(t, spec, "-pretty")
+
+	var stdout, stderr bytes.Buffer
+	require.Equal(t, 0, run([]string{"compile", spec}, &stdout, &stderr), "stderr: %s", stderr.String())
+	assert.Empty(t, cmp.Diff(stdout.String(), string(raw)),
+		"-pretty must write the bytes stdout gets")
+}
+
+// TestRun_CompactOutputKeepsDocumentOrder is the determinism check the format
+// change has to survive: compacting the indented artifact must reproduce the
+// compact one byte for byte. Whitespace removal cannot reorder anything, so the
+// two agreeing means the encoder emitted the same map keys in the same sorted
+// order and the same slices in the same source order as the marshaller it
+// replaced. Compiling twice pins that a second run agrees with the first.
+func TestRun_CompactOutputKeepsDocumentOrder(t *testing.T) {
+	t.Parallel()
+	spec := writeFile(t, "spec.yaml", unsortedSpec)
+
+	compact := compileToFile(t, spec)
+	pretty := compileToFile(t, spec, "-pretty")
+
+	var flattened bytes.Buffer
+	require.NoError(t, json.Compact(&flattened, pretty))
+	assert.Empty(t, cmp.Diff(flattened.String()+"\n", string(compact)),
+		"compacting the indented artifact must reproduce the compact one exactly")
+
+	assert.Empty(t, cmp.Diff(string(compact), string(compileToFile(t, spec))),
+		"two runs over one spec must write the same bytes")
+
+	// The assertion above is only worth something if the spec really does force
+	// a reordering, so confirm the emitted key order is not the declared one.
+	assert.Less(t, bytes.Index(compact, []byte("Alpha")), bytes.Index(compact, []byte("Zeta")),
+		"schemas must be emitted in sorted-key order, not declaration order")
+}