diff --git a/cmd/morphic/command.go b/cmd/morphic/command.go new file mode 100644 index 0000000..e74fe7c --- /dev/null +++ b/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 [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 +} diff --git a/cmd/morphic/command_test.go b/cmd/morphic/command_test.go new file mode 100644 index 0000000..9efa70c --- /dev/null +++ b/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) +} diff --git a/cmd/morphic/compile.go b/cmd/morphic/compile.go index dc24015..f4038d5 100644 --- a/cmd/morphic/compile.go +++ b/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 [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 { diff --git a/cmd/morphic/compile_test.go b/cmd/morphic/compile_test.go index d833785..f0df260 100644 --- a/cmd/morphic/compile_test.go +++ b/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()) + }) + } } diff --git a/cmd/morphic/edgecases_test.go b/cmd/morphic/edgecases_test.go index dd7d6b8..9e42a10 100644 --- a/cmd/morphic/edgecases_test.go +++ b/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) diff --git a/cmd/morphic/help.go b/cmd/morphic/help.go new file mode 100644 index 0000000..9be5108 --- /dev/null +++ b/cmd/morphic/help.go @@ -0,0 +1,84 @@ +package main + +import "io" + +// rootDescription is the one-paragraph summary shown in root help. +const rootDescription = "morphic lowers an API spec into Morphic IR." + +// isHelpFlag reports whether arg asks for help rather than naming work to do. +func isHelpFlag(arg string) bool { + return arg == "-h" || arg == "--help" || arg == "-help" +} + +// writeRootHelp writes the top-level help text to w: the synopsis, what morphic +// is, the command list built from the command table, and how to go deeper. +func writeRootHelp(w io.Writer) { + emitf(w, "usage:\n morphic [flags]\n\n%s\n\ncommands:\n", rootDescription) + for _, c := range commands { + emitf(w, " %-9s %s\n", c.name, c.summary) + } + emitf(w, "\nrun \"morphic help \" for command details.\n") +} + +// writeCommandHelp writes c's full help text to w: synopsis, description, and +// the flag table rendered from c's own FlagSet. +func writeCommandHelp(w io.Writer, c command) { + emitf(w, "usage:\n %s\n\n%s\n\nflags:\n", c.usage, c.description) + fs := c.flagSet() + fs.SetOutput(w) + fs.PrintDefaults() +} + +// writeCommandUsage writes the short pointer shown after a misuse of c: the +// synopsis and where the details live, never the whole flag table. +func writeCommandUsage(w io.Writer, c command) { + emitf(w, "usage:\n %s\nrun \"morphic help %s\" for details.\n", c.usage, c.name) +} + +// filterHelpTokens returns args with every help-flag token removed. help +// takes only a bare positional command name and defines no flags of its own, +// so a help-flag token can never be a legitimate value for it — stripping +// these tokens first lets the argument-count and lookup logic in runHelp run +// on whatever command name, if any, remains. This filtering approach is safe +// here specifically because help has no flags; runCompile must keep detecting +// help via errors.Is(err, flag.ErrHelp) instead of pre-scanning argv. +func filterHelpTokens(args []string) []string { + names := make([]string, 0, len(args)) + for _, arg := range args { + if isHelpFlag(arg) { + continue + } + names = append(names, arg) + } + return names +} + +// runHelp implements the `help` subcommand: bare `help` prints root help, +// `help ` prints that command's help, and anything else is misuse. +// Help-flag tokens (`-h`, `--help`, `-help`) are stripped from args before the +// argument-count check runs, so `help compile --help` prints compile help +// rather than being conflated with `help --help`, and `help bogus --help` is +// still rejected as misuse rather than silently returning root help — a help +// flag no longer masks a mistyped or extra command name. +func runHelp(args []string, stdout, stderr io.Writer) int { + names := filterHelpTokens(args) + if len(names) == 0 { + writeRootHelp(stdout) + return 0 + } + if len(names) > 1 { + emitf(stderr, "morphic: help accepts at most one command\n") + writeRootHelp(stderr) + return 2 + } + + c, ok := lookup(names[0]) + if !ok { + emitf(stderr, "morphic: unknown command %q\n", names[0]) + writeRootHelp(stderr) + return 2 + } + + writeCommandHelp(stdout, c) + return 0 +} diff --git a/cmd/morphic/help_test.go b/cmd/morphic/help_test.go new file mode 100644 index 0000000..73d9c22 --- /dev/null +++ b/cmd/morphic/help_test.go @@ -0,0 +1,127 @@ +package main + +import ( + "bytes" + "flag" + "os" + "path/filepath" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestRun_HelpForms(t *testing.T) { + t.Parallel() + spec := writeFile(t, "spec.yaml", tinySpec) + + tests := []struct { + name string + args []string + }{ + {"no arguments", nil}, + {"root -h", []string{"-h"}}, + {"root --help", []string{"--help"}}, + {"root -help", []string{"-help"}}, + {"root help word", []string{"help"}}, + {"help compile", []string{"help", "compile"}}, + {"help -h", []string{"help", "-h"}}, + {"help --help", []string{"help", "--help"}}, + {"help compile --help", []string{"help", "compile", "--help"}}, + {"help compile -h", []string{"help", "compile", "-h"}}, + {"compile -h", []string{"compile", "-h"}}, + {"compile --help", []string{"compile", "--help"}}, + {"compile spec --help", []string{"compile", spec, "--help"}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + var stdout, stderr bytes.Buffer + + assert.Equal(t, 0, run(tt.args, &stdout, &stderr)) + assert.Contains(t, stdout.String(), "usage:") + assert.Empty(t, stderr.String(), "help must never write to stderr") + }) + } +} + +// TestRun_HelpFlagAsFlagValue pins the property the help design relies on: +// help is detected via errors.Is(err, flag.ErrHelp) from flag.Parse, never by +// pre-scanning argv for "--help". So "-o --help" must consume "--help" as the +// value of -o and compile normally, not print help. This guards against a +// future refactor of runCompile that pre-scans args and would keep full +// statement coverage while silently breaking that distinction. Not run in +// parallel: it changes the process working directory so -o's relative value +// resolves to a file literally named "--help". +func TestRun_HelpFlagAsFlagValue(t *testing.T) { + dir := t.TempDir() + prevWD, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Chdir(dir)) + t.Cleanup(func() { require.NoError(t, os.Chdir(prevWD)) }) + + spec := writeFile(t, "spec.yaml", tinySpec) + var stdout, stderr bytes.Buffer + + code := run([]string{"compile", "-o", "--help", spec}, &stdout, &stderr) + + require.Equal(t, 0, code, "stderr: %s", stderr.String()) + assert.NotContains(t, stdout.String(), "usage:", + "--help must be consumed as -o's value, not treated as a help request") + raw, err := os.ReadFile(filepath.Join(dir, "--help")) + require.NoError(t, err) + assert.Contains(t, string(raw), `"name": "Tiny"`) +} + +func TestRun_HelpFormsAgree(t *testing.T) { + t.Parallel() + + forms := [][]string{ + {"compile", "--help"}, + {"compile", "-h"}, + {"help", "compile"}, + {"help", "compile", "--help"}, + {"help", "compile", "-h"}, + } + + var want string + for i, args := range forms { + var stdout, stderr bytes.Buffer + require.Equal(t, 0, run(args, &stdout, &stderr), "stderr: %s", stderr.String()) + if i == 0 { + want = stdout.String() + require.NotEmpty(t, want) + continue + } + assert.Empty(t, cmp.Diff(want, stdout.String()), "help text differs for %v", args) + } +} + +func TestRun_CompileHelpListsEveryFlag(t *testing.T) { + t.Parallel() + + var stdout, stderr bytes.Buffer + require.Equal(t, 0, run([]string{"help", "compile"}, &stdout, &stderr)) + + got := stdout.String() + fs, _ := newCompileFlags() + fs.VisitAll(func(f *flag.Flag) { + assert.Contains(t, got, f.Name, "compile help must document -%s", f.Name) + }) +} + +func TestRootHelp_ListsEveryCommand(t *testing.T) { + t.Parallel() + + var stdout, stderr bytes.Buffer + require.Equal(t, 0, run(nil, &stdout, &stderr)) + + got := stdout.String() + require.NotEmpty(t, commands, "the command table must not be empty") + for _, c := range commands { + assert.Contains(t, got, c.name) + assert.Contains(t, got, c.summary) + } +} diff --git a/cmd/morphic/main.go b/cmd/morphic/main.go index c4ef282..0391d4e 100644 --- a/cmd/morphic/main.go +++ b/cmd/morphic/main.go @@ -22,18 +22,21 @@ func main() { // run dispatches subcommands and returns the process exit code. It exists so // tests can drive the CLI without a subprocess; only main calls os.Exit. func run(args []string, stdout, stderr io.Writer) int { - if len(args) == 0 { - printUsage(stderr) - return 2 + if len(args) == 0 || isHelpFlag(args[0]) { + writeRootHelp(stdout) + return 0 + } + if args[0] == "help" { + return runHelp(args[1:], stdout, stderr) } - switch args[0] { - case "compile": - return runCompile(args[1:], stdout, stderr) - default: + + c, ok := lookup(args[0]) + if !ok { emitf(stderr, "morphic: unknown command %q\n", args[0]) - printUsage(stderr) + writeRootHelp(stderr) return 2 } + return c.run(args[1:], stdout, stderr) } // emitf writes a formatted line to w. Write errors on a human-facing stream are @@ -41,13 +44,3 @@ func run(args []string, stdout, stderr io.Writer) int { func emitf(w io.Writer, format string, args ...any) { _, _ = fmt.Fprintf(w, format, args...) } - -// printUsage writes the usage text to w. -func printUsage(w io.Writer) { - emitf(w, "%s\n", usage) -} - -const usage = `usage: - morphic compile [-o out.json] [--fail-on error|warning] [--skip-validate] - -compile lowers an API spec (OpenAPI 3.x) into Morphic IR JSON.`