diff --git a/dispatcher.go b/dispatcher.go index 7420e91..a2c59f0 100644 --- a/dispatcher.go +++ b/dispatcher.go @@ -296,7 +296,24 @@ func (d *Dispatcher) Execute(args []string) error { if hasHelp { return d.showHelp() } - return fmt.Errorf("unknown command: %s", strings.Join(args, " ")) + + // Nothing but flags was typed, so there is no word to call an unknown + // command. Name the flag instead. No command matched, so there is no + // flag set to draw suggestions from. + if len(nonFlagArgs) == 0 { + for _, arg := range args { + if arg == "--" { + break + } + if strings.HasPrefix(arg, "-") && arg != "-" { + // Report "--bogus", not "--bogus=value". + name, _, _ := strings.Cut(arg, "=") + return &UnknownFlagError{Flag: name} + } + } + } + + return d.unknownCommandError(nonFlagArgs) } // Check if remaining non-flag args form a namespace under the matched command. @@ -344,9 +361,35 @@ func (d *Dispatcher) Execute(args []string) error { fs.disableAutoHelp = true } if err := fs.Parse(allArgs); err != nil { + // Leftover positional arguments are not a flag-parsing problem, and + // saying so only misdirects the reader. + var unexpected *UnexpectedArgsError + if errors.As(err, &unexpected) { + if name, ok := d.mistypedSubCommand(entry, unexpected.Args); ok { + return d.unknownSubCommandError(entry.Path, name) + } + return err + } + + // "unknown flag: --naem" already says which part of the command line + // went wrong; prefixing it adds nothing and pushes the suggestions + // further from the summary. + var unknownFlag *UnknownFlagError + if errors.As(err, &unknownFlag) { + return err + } + return fmt.Errorf("error parsing flags: %w", err) } + // A command that tolerates unknown flags skips Parse's extra-argument check + // entirely, so a mistyped sub-command under one would otherwise run as if + // nothing were wrong. Catch it here, once Parse has had its chance to claim + // those words as flag values. + if name, ok := d.mistypedSubCommand(entry, fs.Args()); ok { + return d.unknownSubCommandError(entry.Path, name) + } + // Execute the command with the parsed flagset and remaining args err := entry.Command.Run(fs, fs.Args()) @@ -363,6 +406,82 @@ func (d *Dispatcher) Run(args []string) error { return d.Execute(args) } +// unknownCommandError builds the error for command words that matched nothing. +// It first walks the words the user typed to find the deepest namespace among +// them, so that "miren debug wibble" blames "wibble" under "debug" rather than +// blaming "debug", which is perfectly valid. +func (d *Dispatcher) unknownCommandError(words []string) error { + parentPath := "" + unknownIdx := 0 + + for i := 1; i < len(words); i++ { + candidate := normalizeCommandPath(strings.Join(words[:i], " ")) + if d.isNamespace(candidate) { + parentPath = candidate + unknownIdx = i + } + } + + name := "" + if unknownIdx < len(words) { + name = words[unknownIdx] + } + + return d.unknownSubCommandError(parentPath, name) +} + +// mistypedSubCommand reports whether leftover arguments under entry can only be +// a misspelled sub-command: the command has children of its own and declares +// nowhere to put arguments. It returns the offending word. +func (d *Dispatcher) mistypedSubCommand(entry *CommandEntry, leftover []string) (string, bool) { + if len(leftover) == 0 { + return "", false + } + + fs := entry.Command.FlagSet() + if fs == nil || fs.PositionalCount() > 0 || fs.restField != nil { + return "", false + } + + children := d.GetDirectChildren(entry.Path) + if len(children) == 0 { + return "", false + } + + known := make(map[string]bool, len(children)) + for _, c := range children { + known[c.Name] = true + } + + for _, word := range leftover { + // "help" is a keyword the dispatcher handles for itself, and a word + // that really names a sub-command was already matched as one. + if word == "help" || known[word] { + continue + } + return word, true + } + + return "", false +} + +// unknownSubCommandError builds the error for a word that should have named a +// sub-command of parentPath but named nothing at all. +func (d *Dispatcher) unknownSubCommandError(parentPath, name string) error { + children := d.GetDirectChildren(parentPath) + candidates := make([]string, 0, len(children)) + for _, c := range children { + candidates = append(candidates, c.Name) + } + + return &UnknownCommandError{ + Program: d.name, + ParentPath: parentPath, + Name: name, + Suggestions: suggestNames(name, candidates), + } +} + // findCommand finds the best matching command for the given arguments func (d *Dispatcher) findCommand(args []string) (*CommandEntry, []string) { // Try progressively shorter command paths diff --git a/dispatcher_test.go b/dispatcher_test.go index ba3e5de..4625d59 100644 --- a/dispatcher_test.go +++ b/dispatcher_test.go @@ -1523,7 +1523,7 @@ func TestDispatcherRejectsExtraArgs(t *testing.T) { err := d.Execute([]string{"app", "services"}) assert.Error(t, err) - assert.Contains(t, err.Error(), "unexpected arguments: [services]") + assert.Contains(t, err.Error(), `unexpected argument "services"`) }) t.Run("nested command with no positional args rejects extra", func(t *testing.T) { @@ -1538,7 +1538,7 @@ func TestDispatcherRejectsExtraArgs(t *testing.T) { err := d.Execute([]string{"app", "services", "extra"}) assert.Error(t, err) - assert.Contains(t, err.Error(), "unexpected arguments: [extra]") + assert.Contains(t, err.Error(), `unexpected argument "extra"`) }) t.Run("command with positional args accepts correct count", func(t *testing.T) { @@ -1572,7 +1572,7 @@ func TestDispatcherRejectsExtraArgs(t *testing.T) { err := d.Execute([]string{"deploy", "production", "extra"}) assert.Error(t, err) - assert.Contains(t, err.Error(), "unexpected arguments: [extra]") + assert.Contains(t, err.Error(), `unexpected argument "extra"`) }) t.Run("command with rest field accepts all args", func(t *testing.T) { diff --git a/errors.go b/errors.go new file mode 100644 index 0000000..6cc2acc --- /dev/null +++ b/errors.go @@ -0,0 +1,94 @@ +package mflags + +import ( + "fmt" + "strings" +) + +// UnknownCommandError reports a command word that matches nothing registered. +// It carries the surrounding context — which program, which parent command, and +// what the user might have meant — so callers can render it themselves rather +// than parsing the message back out of a string. +type UnknownCommandError struct { + // Program is the binary name, e.g. "miren". + Program string + // ParentPath is the command the unknown word appeared under, e.g. "app". + // Empty when the word was typed at the top level. + ParentPath string + // Name is the single word that matched nothing. + Name string + // Suggestions holds close matches, nearest first. May be empty. + Suggestions []string +} + +func (e *UnknownCommandError) Error() string { + helpTarget := strings.TrimSpace(e.Program + " " + e.ParentPath) + + var sections []string + + if e.ParentPath != "" { + sections = append(sections, fmt.Sprintf("unknown command %q for %q", e.Name, helpTarget)) + } else { + sections = append(sections, fmt.Sprintf("unknown command %q", e.Name)) + } + + if len(e.Suggestions) > 0 { + var b strings.Builder + b.WriteString("Did you mean?") + for _, s := range e.Suggestions { + fmt.Fprintf(&b, "\n %s", s) + } + sections = append(sections, b.String()) + } + + if helpTarget != "" { + sections = append(sections, fmt.Sprintf("Run '%s --help' to see available commands.", helpTarget)) + } + + return strings.Join(sections, "\n\n") +} + +// UnknownFlagError reports a flag the command does not define. It unwraps to +// ErrUnknownFlag, so existing errors.Is checks keep working. +type UnknownFlagError struct { + // Flag is the flag as the user typed it, dashes included. + Flag string + // Suggestions holds close long-flag names, dashes included, nearest first. + // Short flags carry no suggestions: a single letter is too little to guess + // from. + Suggestions []string +} + +func (e *UnknownFlagError) Error() string { + sections := []string{"unknown flag: " + e.Flag} + + if len(e.Suggestions) > 0 { + var b strings.Builder + b.WriteString("Did you mean?") + for _, s := range e.Suggestions { + fmt.Fprintf(&b, "\n %s", s) + } + sections = append(sections, b.String()) + } + + return strings.Join(sections, "\n\n") +} + +func (e *UnknownFlagError) Unwrap() error { return ErrUnknownFlag } + +// UnexpectedArgsError reports positional arguments a command has nowhere to put. +type UnexpectedArgsError struct { + Args []string +} + +func (e *UnexpectedArgsError) Error() string { + if len(e.Args) == 1 { + return fmt.Sprintf("unexpected argument %q", e.Args[0]) + } + + quoted := make([]string, 0, len(e.Args)) + for _, a := range e.Args { + quoted = append(quoted, fmt.Sprintf("%q", a)) + } + return "unexpected arguments: " + strings.Join(quoted, " ") +} diff --git a/mflags.go b/mflags.go index 09ec1cc..7b313b7 100644 --- a/mflags.go +++ b/mflags.go @@ -655,8 +655,7 @@ func (f *FlagSet) Parse(arguments []string) error { // Check for unexpected extra arguments when no rest field is defined // Skip validation if allowUnknownFlags is enabled (pass-through mode) if f.restField == nil && !f.allowUnknownFlags && len(f.args) > f.PositionalCount() { - extra := f.args[f.PositionalCount():] - return fmt.Errorf("unexpected arguments: %v", extra) + return &UnexpectedArgsError{Args: f.args[f.PositionalCount():]} } // If we have a rest field, populate it with remaining args after positional ones @@ -709,6 +708,27 @@ func (f *FlagSet) validateRequired() error { return nil } +// unknownLongFlagError reports an undefined long flag, offering the defined +// flags whose names are closest to what was typed. +func (f *FlagSet) unknownLongFlagError(name string) error { + candidates := make([]string, 0, len(f.flags)) + for defined, flag := range f.flags { + // Hidden flags stay out of help output, so advertising one here on a + // near-miss typo would undo that. + if flag.Hidden { + continue + } + candidates = append(candidates, defined) + } + + suggestions := suggestNames(name, candidates) + for i, s := range suggestions { + suggestions[i] = "--" + s + } + + return &UnknownFlagError{Flag: "--" + name, Suggestions: suggestions} +} + func (f *FlagSet) parseLongFlag(name string, args []string, index *int) (bool, error) { var value string hasValue := false @@ -728,7 +748,7 @@ func (f *FlagSet) parseLongFlag(name string, args []string, index *int) (bool, e *index = len(args) - 1 // Skip to end return true, nil } - return false, fmt.Errorf("%w: --%s", ErrUnknownFlag, name) + return false, f.unknownLongFlagError(name) } if flag.Value.IsBool() { @@ -768,7 +788,7 @@ func (f *FlagSet) parseShortFlags(shortFlags string, args []string, index *int) *index = len(args) - 1 // Skip to end return nil } - return fmt.Errorf("%w: -%c", ErrUnknownFlag, r) + return &UnknownFlagError{Flag: fmt.Sprintf("-%c", r)} } if flag.Value.IsBool() { diff --git a/mflags_test.go b/mflags_test.go index 4ebc805..7d8c39e 100644 --- a/mflags_test.go +++ b/mflags_test.go @@ -1092,7 +1092,7 @@ func TestInvalidRestFieldType(t *testing.T) { // Since the rest field is invalid and ignored, extra args should be rejected err = fs.Parse([]string{"arg1", "arg2"}) assert.Error(t, err) - assert.Contains(t, err.Error(), "unexpected arguments") + assert.Contains(t, err.Error(), `unexpected arguments: "arg1" "arg2"`) // The rest field should be ignored since it's not []string assert.Equal(t, "", config.RestField) @@ -1287,7 +1287,7 @@ func TestInvalidPositionTag(t *testing.T) { // so any args should be rejected err = fs.Parse([]string{"value"}) assert.Error(t, err) - assert.Contains(t, err.Error(), "unexpected arguments") + assert.Contains(t, err.Error(), `unexpected argument "value"`) assert.Equal(t, "", config.Item) // Field is ignored due to invalid position } @@ -1307,7 +1307,7 @@ func TestNegativePositionTag(t *testing.T) { // so any args should be rejected err = fs.Parse([]string{"value"}) assert.Error(t, err) - assert.Contains(t, err.Error(), "unexpected arguments") + assert.Contains(t, err.Error(), `unexpected argument "value"`) assert.Equal(t, "", config.Item) // Field is ignored due to negative position } diff --git a/positional_api_test.go b/positional_api_test.go index 6551862..99bc5c0 100644 --- a/positional_api_test.go +++ b/positional_api_test.go @@ -427,7 +427,7 @@ func TestExtraArgumentsRejected(t *testing.T) { err := fs.Parse([]string{"extra"}) assert.Error(t, err) - assert.Contains(t, err.Error(), "unexpected arguments: [extra]") + assert.Contains(t, err.Error(), `unexpected argument "extra"`) }) t.Run("multiple extra args rejected", func(t *testing.T) { @@ -436,7 +436,7 @@ func TestExtraArgumentsRejected(t *testing.T) { err := fs.Parse([]string{"extra1", "extra2", "extra3"}) assert.Error(t, err) - assert.Contains(t, err.Error(), "unexpected arguments: [extra1 extra2 extra3]") + assert.Contains(t, err.Error(), `unexpected arguments: "extra1" "extra2" "extra3"`) }) t.Run("positional args rejects extra args", func(t *testing.T) { @@ -446,7 +446,7 @@ func TestExtraArgumentsRejected(t *testing.T) { err := fs.Parse([]string{"a", "b", "extra"}) assert.Error(t, err) - assert.Contains(t, err.Error(), "unexpected arguments: [extra]") + assert.Contains(t, err.Error(), `unexpected argument "extra"`) }) t.Run("positional with gaps rejects extra args", func(t *testing.T) { @@ -459,7 +459,7 @@ func TestExtraArgumentsRejected(t *testing.T) { // 5 args fill all positions, 6th is extra err := fs.Parse([]string{"a", "b", "c", "d", "e", "extra"}) assert.Error(t, err) - assert.Contains(t, err.Error(), "unexpected arguments: [extra]") + assert.Contains(t, err.Error(), `unexpected argument "extra"`) }) t.Run("rest field accepts all args", func(t *testing.T) { diff --git a/suggest.go b/suggest.go new file mode 100644 index 0000000..31d017f --- /dev/null +++ b/suggest.go @@ -0,0 +1,149 @@ +package mflags + +import ( + "sort" + "strings" +) + +// maxSuggestions caps how many alternatives a "did you mean" block offers. +// Beyond a handful the list stops being a suggestion and starts being help +// output, which the closing "--help" line already points at. +const maxSuggestions = 3 + +// editDistance returns the number of edits between a and b, counting an +// insertion, a deletion, a substitution, or a transposition of two adjacent +// characters as one each. Transpositions matter: swapping two letters is among +// the most common ways to mistype a word, and charging two edits for it would +// put "recieve" as far from "receive" as a word with two genuinely wrong +// letters. +// +// It compares runes rather than bytes so that non-ASCII names cost what they +// look like they cost. +func editDistance(a, b string) int { + ar := []rune(a) + br := []rune(b) + + if len(ar) == 0 { + return len(br) + } + if len(br) == 0 { + return len(ar) + } + + // Three rolling rows of the distance matrix: curr is the row being filled + // in, prev the row above it, and prevPrev the one above that, which only + // the transposition case reaches back for. + prevPrev := make([]int, len(br)+1) + prev := make([]int, len(br)+1) + curr := make([]int, len(br)+1) + + for j := range prev { + prev[j] = j + } + + for i := 1; i <= len(ar); i++ { + curr[0] = i + for j := 1; j <= len(br); j++ { + cost := 1 + if ar[i-1] == br[j-1] { + cost = 0 + } + + best := min(prev[j]+1, min(curr[j-1]+1, prev[j-1]+cost)) + + // The last two characters of each are the same pair, swapped. + if i > 1 && j > 1 && ar[i-1] == br[j-2] && ar[i-2] == br[j-1] { + best = min(best, prevPrev[j-2]+1) + } + + curr[j] = best + } + prevPrev, prev, curr = prev, curr, prevPrev + } + + return prev[len(br)] +} + +// suggestThreshold returns the largest edit distance still worth reporting for +// a word of the given length. Short words get a tight budget because at two +// edits every three-letter word is a neighbour of every other one. +func suggestThreshold(n int) int { + switch { + case n <= 2: + return 0 + case n <= 4: + return 1 + case n <= 8: + return 2 + default: + return 3 + } +} + +// suggestNames returns the candidates closest to unknown, nearest first, or nil +// when nothing is close enough to be worth guessing at. Prefix matches rank +// ahead of pure edit distance: someone who typed "depl" almost certainly meant +// "deploy" even though four insertions is a long way in edit-distance terms. +func suggestNames(unknown string, candidates []string) []string { + if unknown == "" { + return nil + } + + threshold := suggestThreshold(len([]rune(unknown))) + + type scored struct { + name string + prefix bool + dist int + } + + var matches []scored + for _, c := range candidates { + if c == "" { + continue + } + + // A prefix match is strong evidence on its own — an abbreviation or a + // half-typed word — so it bypasses the distance threshold. + if len(c) > len(unknown) && strings.HasPrefix(c, unknown) { + // Rank by how much is left to type, in characters — bytes would + // sort a short non-ASCII completion behind a longer ASCII one. + matches = append(matches, scored{ + name: c, + prefix: true, + dist: len([]rune(c)) - len([]rune(unknown)), + }) + continue + } + + if d := editDistance(unknown, c); d <= threshold { + matches = append(matches, scored{name: c, dist: d}) + } + } + + if len(matches) == 0 { + return nil + } + + // Alphabetical ties keep the output stable; the dispatcher's command map + // iterates in random order. + sort.Slice(matches, func(i, j int) bool { + if matches[i].prefix != matches[j].prefix { + return matches[i].prefix + } + if matches[i].dist != matches[j].dist { + return matches[i].dist < matches[j].dist + } + return matches[i].name < matches[j].name + }) + + if len(matches) > maxSuggestions { + matches = matches[:maxSuggestions] + } + + names := make([]string, 0, len(matches)) + for _, m := range matches { + names = append(names, m.name) + } + return names +} diff --git a/suggest_test.go b/suggest_test.go new file mode 100644 index 0000000..df742f2 --- /dev/null +++ b/suggest_test.go @@ -0,0 +1,95 @@ +package mflags + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestEditDistance(t *testing.T) { + cases := []struct { + a, b string + want int + }{ + {"", "", 0}, + {"", "abc", 3}, + {"abc", "", 3}, + {"deploy", "deploy", 0}, + {"depoy", "deploy", 1}, // deletion + {"deployy", "deploy", 1}, // insertion + {"deploj", "deploy", 1}, // substitution + {"depoly", "deploy", 1}, // transposition + {"naem", "name", 1}, // transposition + {"recieve", "receive", 1}, + {"kitten", "sitting", 3}, + {"café", "cafe", 1}, // one rune, not two bytes + } + + for _, c := range cases { + assert.Equal(t, c.want, editDistance(c.a, c.b), "editDistance(%q, %q)", c.a, c.b) + assert.Equal(t, c.want, editDistance(c.b, c.a), "editDistance(%q, %q)", c.b, c.a) + } +} + +func TestSuggestNames(t *testing.T) { + commands := []string{"app", "deploy", "logs", "rollback", "runner", "server", "upgrade", "version"} + + t.Run("one edit away", func(t *testing.T) { + assert.Equal(t, []string{"deploy"}, suggestNames("depoy", commands)) + }) + + t.Run("transposition", func(t *testing.T) { + assert.Equal(t, []string{"deploy"}, suggestNames("depoly", commands)) + }) + + t.Run("prefix beats distance", func(t *testing.T) { + assert.Equal(t, []string{"deploy"}, suggestNames("depl", commands)) + }) + + t.Run("prefix matches rank ahead of edit-distance matches", func(t *testing.T) { + got := suggestNames("serve", []string{"server", "serve-it", "sever"}) + assert.Equal(t, []string{"server", "serve-it", "sever"}, got) + }) + + t.Run("shorter prefix completion wins", func(t *testing.T) { + assert.Equal(t, []string{"apply", "applesauce"}, suggestNames("appl", []string{"apply", "applesauce"})) + }) + + t.Run("prefix completions rank by characters, not bytes", func(t *testing.T) { + // Past the shared "a", "é" is one character but two bytes, while "bc" + // is two of each. Measured in bytes the two tie and sort + // alphabetically, putting the longer completion first. + got := suggestNames("a", []string{"abc", "aé"}) + assert.Equal(t, []string{"aé", "abc"}, got) + }) + + t.Run("nothing close returns nil", func(t *testing.T) { + assert.Nil(t, suggestNames("zzzzzz", commands)) + }) + + t.Run("short words are not guessed at on distance alone", func(t *testing.T) { + // "vp" is one edit from "up"; at two characters that is too weak a + // signal to guess on. + assert.Nil(t, suggestNames("vp", []string{"up", "down"})) + }) + + t.Run("but a short prefix still counts", func(t *testing.T) { + // "ap" is only two characters, yet it can only be reaching for "app". + assert.Equal(t, []string{"app"}, suggestNames("ap", commands)) + }) + + t.Run("empty input", func(t *testing.T) { + assert.Nil(t, suggestNames("", commands)) + assert.Nil(t, suggestNames("deploy", nil)) + }) + + t.Run("capped at three", func(t *testing.T) { + got := suggestNames("aaa", []string{"aab", "aac", "aad", "aae", "aaf"}) + assert.Len(t, got, maxSuggestions) + }) + + t.Run("ties are alphabetical", func(t *testing.T) { + got := suggestNames("aaa", []string{"aaf", "aab", "aad"}) + assert.Equal(t, []string{"aab", "aad", "aaf"}, got) + }) +} diff --git a/unknown_command_test.go b/unknown_command_test.go new file mode 100644 index 0000000..93bf34e --- /dev/null +++ b/unknown_command_test.go @@ -0,0 +1,291 @@ +package mflags + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// unknownCommandDispatcher builds a dispatcher shaped like a real CLI: a plain +// top-level command, a leaf command that also has children, a section-style +// command that tolerates unknown flags, and a command that takes free-form +// arguments. +func unknownCommandDispatcher() *Dispatcher { + d := NewDispatcher("myapp") + + noArgs := func() *FlagSet { return NewFlagSet("x") } + run := func(fs *FlagSet, args []string) error { return nil } + + d.Dispatch("deploy", NewCommand(noArgs(), run, WithUsage("Deploy an application"))) + d.Dispatch("upgrade", NewCommand(noArgs(), run, WithUsage("Upgrade"))) + + // "app" is a real command that also has sub-commands under it. + d.Dispatch("app", NewCommand(noArgs(), run, WithUsage("Manage applications"))) + d.Dispatch("app list", NewCommand(noArgs(), run, WithUsage("List applications"))) + d.Dispatch("app destroy", NewCommand(noArgs(), run, WithUsage("Destroy an application"))) + + // "runner" stands in for a Section: no flags of its own, but it tolerates + // unknown ones so that global flags may precede it. + section := NewFlagSet("runner") + section.AllowUnknownFlags(true) + d.Dispatch("runner", NewCommand(section, run, WithUsage("Manage runners"))) + d.Dispatch("runner upgrade", NewCommand(noArgs(), run, WithUsage("Upgrade a runner"))) + + // "logs" takes a positional argument, so a stray word is a value, not a typo. + logs := NewFlagSet("logs") + logs.StringPos("app", 0, "", "Application name") + d.Dispatch("logs", NewCommand(logs, run, WithUsage("View logs"))) + d.Dispatch("logs build", NewCommand(noArgs(), run, WithUsage("View build logs"))) + + // "exec" passes everything through. + type execOpts struct { + Args []string `rest:"true"` + } + exec := NewFlagSet("exec") + exec.FromStruct(&execOpts{}) + d.Dispatch("exec", NewCommand(exec, run, WithUsage("Run a command"))) + d.Dispatch("exec shell", NewCommand(noArgs(), run, WithUsage("Open a shell"))) + + return d +} + +func TestUnknownCommandSuggestions(t *testing.T) { + t.Run("top-level typo suggests a top-level command", func(t *testing.T) { + err := unknownCommandDispatcher().Execute([]string{"depoy"}) + require.Error(t, err) + + var uce *UnknownCommandError + require.ErrorAs(t, err, &uce) + assert.Equal(t, "depoy", uce.Name) + assert.Equal(t, "", uce.ParentPath) + assert.Equal(t, []string{"deploy"}, uce.Suggestions) + + assert.Equal(t, `unknown command "depoy" + +Did you mean? + deploy + +Run 'myapp --help' to see available commands.`, err.Error()) + }) + + t.Run("typo under a command with sub-commands", func(t *testing.T) { + err := unknownCommandDispatcher().Execute([]string{"app", "destory"}) + require.Error(t, err) + + var uce *UnknownCommandError + require.ErrorAs(t, err, &uce) + assert.Equal(t, "destory", uce.Name) + assert.Equal(t, "app", uce.ParentPath) + + assert.Equal(t, `unknown command "destory" for "myapp app" + +Did you mean? + destroy + +Run 'myapp app --help' to see available commands.`, err.Error()) + }) + + t.Run("typo under a section errors instead of silently showing help", func(t *testing.T) { + err := unknownCommandDispatcher().Execute([]string{"runner", "upgrde"}) + require.Error(t, err) + + var uce *UnknownCommandError + require.ErrorAs(t, err, &uce) + assert.Equal(t, "upgrde", uce.Name) + assert.Equal(t, "runner", uce.ParentPath) + assert.Equal(t, []string{"upgrade"}, uce.Suggestions) + }) + + t.Run("typo under an unregistered namespace blames the right word", func(t *testing.T) { + d := NewDispatcher("myapp") + run := func(fs *FlagSet, args []string) error { return nil } + // "debug" itself is never registered; it exists only as a prefix. + d.Dispatch("debug entity", NewCommand(NewFlagSet("x"), run, WithUsage("Entities"))) + d.Dispatch("debug etcd", NewCommand(NewFlagSet("x"), run, WithUsage("Etcd"))) + + err := d.Execute([]string{"debug", "entty"}) + require.Error(t, err) + + var uce *UnknownCommandError + require.ErrorAs(t, err, &uce) + assert.Equal(t, "entty", uce.Name, "should blame the bad word, not the valid namespace") + assert.Equal(t, "debug", uce.ParentPath) + assert.Equal(t, []string{"entity"}, uce.Suggestions) + }) + + t.Run("no close match still names the word", func(t *testing.T) { + err := unknownCommandDispatcher().Execute([]string{"zzzzzzzz"}) + require.Error(t, err) + + var uce *UnknownCommandError + require.ErrorAs(t, err, &uce) + assert.Empty(t, uce.Suggestions) + + assert.Equal(t, `unknown command "zzzzzzzz" + +Run 'myapp --help' to see available commands.`, err.Error()) + assert.NotContains(t, err.Error(), "Did you mean") + }) + + t.Run("flag-only input names the flag, not an empty command", func(t *testing.T) { + for _, args := range [][]string{ + {"--bogus"}, + {"-x"}, + {"--bogus", "--other"}, + {"--bogus=value"}, + } { + err := unknownCommandDispatcher().Execute(args) + require.Error(t, err) + + var ufe *UnknownFlagError + require.ErrorAs(t, err, &ufe, "args %q", args) + assert.NotContains(t, err.Error(), `unknown command ""`) + } + }) + + t.Run("flag-only input reports the flag as typed", func(t *testing.T) { + err := unknownCommandDispatcher().Execute([]string{"--bogus=value"}) + require.Error(t, err) + assert.Equal(t, "unknown flag: --bogus", err.Error()) + }) + + t.Run("only the first word is blamed", func(t *testing.T) { + err := unknownCommandDispatcher().Execute([]string{"depoy", "myapp"}) + require.Error(t, err) + + var uce *UnknownCommandError + require.ErrorAs(t, err, &uce) + assert.Equal(t, "depoy", uce.Name) + }) +} + +func TestUnknownCommandDoesNotFireOnValidInput(t *testing.T) { + cases := []struct { + name string + args []string + }{ + {"command with a positional accepts it", []string{"logs", "myapp"}}, + {"rest field accepts arbitrary words", []string{"exec", "echo", "hello"}}, + {"real sub-command still runs", []string{"app", "list"}}, + {"real nested sub-command still runs", []string{"runner", "upgrade"}}, + {"bare command with children shows help", []string{"app"}}, + // The dispatcher handles "help" itself; it is never a mistyped + // sub-command, even under a command that tolerates unknown flags. + {"trailing help keyword", []string{"runner", "help"}}, + {"leading help keyword", []string{"help", "runner"}}, + {"help keyword before a real sub-command", []string{"runner", "help", "upgrade"}}, + // An unknown flag's value is claimed by Parse, not blamed as a word. + {"unknown value flag before a section", []string{"runner", "-C", "prod"}}, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + assert.NoError(t, unknownCommandDispatcher().Execute(c.args)) + }) + } +} + +func TestUnexpectedArgsWithoutSubCommands(t *testing.T) { + // "version" has no children, so there is no sub-command to suggest. The + // message should still avoid both the Go slice syntax and the misleading + // "error parsing flags" prefix. + d := NewDispatcher("myapp") + d.Dispatch("version", NewCommand(NewFlagSet("version"), + func(fs *FlagSet, args []string) error { return nil }, WithUsage("Print the version"))) + + err := d.Execute([]string{"version", "foo"}) + require.Error(t, err) + + var uae *UnexpectedArgsError + require.ErrorAs(t, err, &uae) + assert.Equal(t, `unexpected argument "foo"`, err.Error()) + assert.NotContains(t, err.Error(), "error parsing flags") +} + +func TestUnknownFlagSuggestions(t *testing.T) { + newDispatcher := func() *Dispatcher { + d := NewDispatcher("myapp") + fs := NewFlagSet("deploy") + fs.Bool("verbose", 'v', false, "Verbose output") + fs.String("name", 'n', "", "Application name") + fs.String("namespace", 0, "", "Namespace") + d.Dispatch("deploy", NewCommand(fs, + func(fs *FlagSet, args []string) error { return nil }, WithUsage("Deploy"))) + return d + } + + t.Run("suggests a close long flag", func(t *testing.T) { + err := newDispatcher().Execute([]string{"deploy", "--naem", "x"}) + require.Error(t, err) + + var ufe *UnknownFlagError + require.ErrorAs(t, err, &ufe) + assert.Equal(t, "--naem", ufe.Flag) + assert.Equal(t, []string{"--name"}, ufe.Suggestions) + + assert.Equal(t, `unknown flag: --naem + +Did you mean? + --name`, err.Error()) + }) + + t.Run("still unwraps to ErrUnknownFlag", func(t *testing.T) { + err := newDispatcher().Execute([]string{"deploy", "--naem", "x"}) + assert.ErrorIs(t, err, ErrUnknownFlag) + }) + + t.Run("drops the redundant flag-parsing prefix", func(t *testing.T) { + err := newDispatcher().Execute([]string{"deploy", "--naem", "x"}) + require.Error(t, err) + assert.NotContains(t, err.Error(), "error parsing flags") + }) + + t.Run("nothing close offers no guess", func(t *testing.T) { + err := newDispatcher().Execute([]string{"deploy", "--zzzzzzzz"}) + require.Error(t, err) + assert.Equal(t, "unknown flag: --zzzzzzzz", err.Error()) + }) + + t.Run("short flags carry no suggestions", func(t *testing.T) { + err := newDispatcher().Execute([]string{"deploy", "-q"}) + require.Error(t, err) + + var ufe *UnknownFlagError + require.ErrorAs(t, err, &ufe) + assert.Equal(t, "-q", ufe.Flag) + assert.Empty(t, ufe.Suggestions) + assert.Equal(t, "unknown flag: -q", err.Error()) + }) + + t.Run("hidden flags are never suggested", func(t *testing.T) { + d := NewDispatcher("myapp") + fs := NewFlagSet("deploy") + fs.String("name", 'n', "", "Application name") + fs.String("internal-token", 0, "", "Internal token") + fs.flags["internal-token"].Hidden = true + d.Dispatch("deploy", NewCommand(fs, + func(fs *FlagSet, args []string) error { return nil }, WithUsage("Deploy"))) + + // One edit from the hidden flag, so only its hidden-ness keeps it out. + err := d.Execute([]string{"deploy", "--internal-tokn", "x"}) + require.Error(t, err) + + var ufe *UnknownFlagError + require.ErrorAs(t, err, &ufe) + assert.Empty(t, ufe.Suggestions) + assert.NotContains(t, err.Error(), "internal-token") + }) + + t.Run("a value flag keeps the parse-error prefix", func(t *testing.T) { + d := NewDispatcher("myapp") + fs := NewFlagSet("deploy") + fs.Int("port", 'p', 0, "Port") + d.Dispatch("deploy", NewCommand(fs, + func(fs *FlagSet, args []string) error { return nil }, WithUsage("Deploy"))) + + err := d.Execute([]string{"deploy", "--port", "notanumber"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "error parsing flags") + }) +}