test(cmd/morphic): assert one usage block per misuse

cmd/morphic/command.go

@@ -0,0 +1,44 @@
+package main
+
+import (
+	"flag"
+	"io"
+)
+
+// command describes one morphic subcommand: how it is invoked, how it is
+// documented, and how it runs. Dispatch and help rendering both read this
+// table, so a new subcommand becomes reachable and documented in one edit.
+type command struct {
+	// name is the word typed after "morphic".
+	name string
+	// summary is the one-line description shown in the root command list.
+	summary string
+	// usage is the invocation synopsis, e.g. "morphic compile <spec-file> [flags]".
+	usage string
+	// description is the paragraph shown above the flag table in command help.
+	description string
+	// flagSet returns a fresh FlagSet with this command's flags defined. Help
+	// rendering and argument parsing share it, so the flag table printed by
+	// PrintDefaults cannot drift from what Parse accepts.
+	flagSet func() *flag.FlagSet
+	// run executes the command with the subcommand word already removed from
+	// args, and returns the process exit code.
+	run func(args []string, stdout, stderr io.Writer) int
+}
+
+// commands is the subcommand table. Adding a subcommand means adding one entry.
+var commands = []command{newCompileCommand()}
+
+// lookup resolves a subcommand by name. The empty name never resolves, so an
+// empty argv element cannot select a command.
+func lookup(name string) (command, bool) {
+	if name == "" {
+		return command{}, false
+	}
+	for _, c := range commands {
+		if c.name == name {
+			return c, true
+		}
+	}
+	return command{}, false
+}

cmd/morphic/command_test.go

@@ -0,0 +1,41 @@
+package main
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestLookup_KnownAndUnknown(t *testing.T) {
+	t.Parallel()
+
+	c, ok := lookup("compile")
+	require.True(t, ok, "compile must be in the command table")
+	assert.Equal(t, "compile", c.name)
+	assert.NotEmpty(t, c.summary, "every command needs a summary for the root help list")
+	assert.NotEmpty(t, c.usage)
+	assert.NotEmpty(t, c.description)
+	require.NotNil(t, c.flagSet)
+	require.NotNil(t, c.run)
+
+	_, ok = lookup("bogus")
+	assert.False(t, ok)
+
+	_, ok = lookup("")
+	assert.False(t, ok, "the empty name must never resolve")
+}
+
+func TestNewCompileFlags_DefinesEveryFlag(t *testing.T) {
+	t.Parallel()
+
+	fs, opts := newCompileFlags()
+	require.NotNil(t, opts)
+
+	for _, name := range []string{"o", "fail-on", "skip-validate"} {
+		assert.NotNil(t, fs.Lookup(name), "flag %q must be defined", name)
+	}
+	assert.Equal(t, "error", opts.failOn, "default --fail-on")
+	assert.Empty(t, opts.outPath)
+	assert.False(t, opts.skipValidate)
+}

cmd/morphic/compile.go

@@ -3,6 +3,7 @@ package main
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"flag"
 	"fmt"
 	"io"
@@ -23,51 +24,106 @@ var newEngine = engine.New
 // not fail after a successful write on the platforms Morphic targets.
 var openOutput = func(path string) (io.WriteCloser, error) { return os.Create(path) }
 
-// runCompile implements the `compile` subcommand: lower one spec file to IR JSON,
-// render its diagnostics to stderr, and return the process exit code.
-func runCompile(args []string, stdout, stderr io.Writer) int {
+// newCompileCommand builds compile's command-table entry. It is a function,
+// not a package-level var, because its run field refers to runCompile, and
+// runCompile itself needs this same metadata to render help and usage text;
+// a var holding that cycle back to itself would be a Go initialization
+// cycle, while two package-level functions may freely refer to each other.
+func newCompileCommand() command {
+	return command{
+		name:    "compile",
+		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.",
+		flagSet: func() *flag.FlagSet {
+			fs, _ := newCompileFlags()
+			return fs
+		},
+		run: runCompile,
+	}
+}
+
+// compileOptions holds the values compile's flags parse into.
+type compileOptions struct {
+	outPath      string
+	failOn       string
+	skipValidate bool
+}
+
+// newCompileFlags returns compile's FlagSet and the options its flags write
+// into. The FlagSet prints nothing on its own: parse failures and help requests
+// come back as errors from Parse, so the CLI renders exactly one text for them.
+func newCompileFlags() (*flag.FlagSet, *compileOptions) {
 	fs := flag.NewFlagSet("compile", flag.ContinueOnError)
-	fs.SetOutput(stderr)
-	outPath := fs.String("o", "", "write IR JSON to this file instead of stdout")
-	failOn := fs.String("fail-on", "error",
+	fs.SetOutput(io.Discard)
+	fs.Usage = func() {}
+
+	var opts compileOptions
+	fs.StringVar(&opts.outPath, "o", "", "write IR JSON to this file instead of stdout")
+	fs.StringVar(&opts.failOn, "fail-on", "error",
 		"fail (exit 1) on diagnostics at or above this severity: error|warning")
-	skipValidate := fs.Bool("skip-validate", false, "skip the referential-integrity validate pass")
+	fs.BoolVar(&opts.skipValidate, "skip-validate", false,
+		"skip the referential-integrity validate pass")
+
+	return fs, &opts
+}
+
+// runCompile implements the `compile` subcommand: lower one spec file to IR
+// JSON, render its diagnostics to stderr, and return the process exit code.
+func runCompile(args []string, stdout, stderr io.Writer) int {
+	fs, opts := newCompileFlags()
 
 	positional, err := parseArgs(fs, args)
+	if errors.Is(err, flag.ErrHelp) {
+		writeCommandHelp(stdout, newCompileCommand())
+		return 0
+	}
 	if err != nil {
-		printUsage(stderr)
-		return 2
+		return compileUsageError(stderr, "%v", err)
 	}
-	if *failOn != "error" && *failOn != "warning" {
-		emitf(stderr, "morphic: invalid --fail-on %q (want error or warning)\n", *failOn)
-		printUsage(stderr)
-		return 2
+	if opts.failOn != "error" && opts.failOn != "warning" {
+		return compileUsageError(stderr, "invalid --fail-on %q (want error or warning)", opts.failOn)
 	}
 	if len(positional) != 1 {
-		emitf(stderr, "morphic: compile requires exactly one spec file\n")
-		printUsage(stderr)
-		return 2
+		return compileUsageError(stderr, "compile requires exactly one spec file")
 	}
 
+	return compileSpec(positional[0], *opts, stdout, stderr)
+}
+
+// compileUsageError reports a misuse of compile: one reason line, one short
+// usage pointer, exit 2. It never touches stdout.
+func compileUsageError(stderr io.Writer, format string, args ...any) int {
+	emitf(stderr, "morphic: "+format+"\n", args...)
+	writeCommandUsage(stderr, newCompileCommand())
+	return 2
+}
+
+// compileSpec runs the pipeline over specPath, writes the IR document and its
+// diagnostics, and returns the process exit code.
+func compileSpec(specPath string, opts compileOptions, stdout, stderr io.Writer) int {
 	eng, err := newEngine()
 	if err != nil {
 		emitf(stderr, "morphic: %v\n", err)
 		return 2
 	}
-	res, err := eng.Run(context.Background(), positional[0], engine.RunOptions{SkipValidate: *skipValidate})
+
+	res, err := eng.Run(context.Background(), specPath, engine.RunOptions{SkipValidate: opts.skipValidate})
 	if err != nil {
 		emitf(stderr, "morphic: %v\n", err)
 		return 2
 	}
+
 	renderDiagnostics(stderr, res)
 	if res.Document == nil {
 		return 1
 	}
-	if err := writeCompiled(*outPath, stdout, res.Document); err != nil {
+	if err := writeCompiled(opts.outPath, stdout, res.Document); err != nil {
 		emitf(stderr, "morphic: %v\n", err)
 		return 2
 	}
-	return exitCodeFor(res.Diagnostics, *failOn)
+	return exitCodeFor(res.Diagnostics, opts.failOn)
 }
 
 // parseArgs binds fs and collects positional arguments, tolerating flags that
@@ -78,7 +134,9 @@ func parseArgs(fs *flag.FlagSet, args []string) ([]string, error) {
 	rest := args
 	for {
 		if err := fs.Parse(rest); err != nil {
-			return nil, fmt.Errorf("parse flags: %w", err)
+			// Returned verbatim, not wrapped: this error is rendered straight to
+			// the user, and the flag package's messages already name the flag.
+			return nil, err
 		}
 		rest = fs.Args()
 		if len(rest) == 0 {

cmd/morphic/compile_test.go

@@ -83,9 +83,34 @@ components:
 
 func TestRun_UsageErrors(t *testing.T) {
 	t.Parallel()
-	var stdout, stderr bytes.Buffer
-	assert.Equal(t, 2, run(nil, &stdout, &stderr))
-	assert.Equal(t, 2, run([]string{"bogus"}, &stdout, &stderr))
-	assert.Equal(t, 2, run([]string{"compile", "x.yaml", "--fail-on", "hint"}, &stdout, &stderr))
-	assert.True(t, strings.Contains(stderr.String(), "usage"))
+
+	spec := writeFile(t, "spec.yaml", tinySpec)
+	tests := []struct {
+		name   string
+		args   []string
+		reason string
+	}{
+		{"unknown command", []string{"bogus"}, `unknown command "bogus"`},
+		{"help of unknown command", []string{"help", "bogus"}, `unknown command "bogus"`},
+		{"help of unknown command with help flag", []string{"help", "bogus", "--help"}, `unknown command "bogus"`},
+		{"help with extra args", []string{"help", "compile", "extra"}, "help accepts at most one command"},
+		{"help with extra args and help flag", []string{"help", "a", "b", "--help"}, "help accepts at most one command"},
+		{"unknown flag", []string{"compile", spec, "--bogus"}, "flag provided but not defined: -bogus"},
+		{"no spec file", []string{"compile"}, "compile requires exactly one spec file"},
+		{"two spec files", []string{"compile", spec, spec}, "compile requires exactly one spec file"},
+		{"bad fail-on", []string{"compile", spec, "--fail-on", "hint"}, `invalid --fail-on "hint"`},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			var stdout, stderr bytes.Buffer
+
+			assert.Equal(t, 2, run(tt.args, &stdout, &stderr))
+			assert.Empty(t, stdout.String(), "usage errors must never write to stdout")
+			assert.Contains(t, stderr.String(), tt.reason)
+			assert.Equal(t, 1, strings.Count(stderr.String(), "usage:"),
+				"exactly one usage block per misuse, got:\n%s", stderr.String())
+		})
+	}
 }

cmd/morphic/edgecases_test.go

@@ -67,7 +67,7 @@ func TestMain_ExitCode(t *testing.T) {
 
 	var got int
 	osExit = func(code int) { got = code }
-	os.Args = []string{"morphic"} // no subcommand → usage → exit 2
+	os.Args = []string{"morphic", "bogus"} // unknown command → usage → exit 2
 
 	main()
 
@@ -109,38 +109,6 @@ func TestRunParse_NilDocumentReturnsOne(t *testing.T) {
 	assert.Contains(t, stderr.String(), "openapi/unsupported-version")
 }
 
-func TestRunParse_UnknownFlagIsUsageError(t *testing.T) {
-	t.Parallel()
-	spec := writeFile(t, "spec.yaml", tinySpec)
-	var stdout, stderr bytes.Buffer
-
-	code := run([]string{"compile", spec, "--bogus"}, &stdout, &stderr)
-
-	assert.Equal(t, 2, code)
-	assert.Contains(t, stderr.String(), "usage")
-}
-
-func TestRunParse_WrongPositionalCount(t *testing.T) {
-	t.Parallel()
-	spec := writeFile(t, "spec.yaml", tinySpec)
-	tests := []struct {
-		name string
-		args []string
-	}{
-		{"no spec file", []string{"compile"}},
-		{"two spec files", []string{"compile", spec, spec}},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-			var stdout, stderr bytes.Buffer
-			code := run(tt.args, &stdout, &stderr)
-			assert.Equal(t, 2, code)
-			assert.Contains(t, stderr.String(), "requires exactly one spec file")
-		})
-	}
-}
-
 func TestRunParse_SkipValidateToStdout(t *testing.T) {
 	t.Parallel()
 	spec := writeFile(t, "spec.yaml", tinySpec)