Skip to content

Commit

Permalink
feat: improve missing argument error messages (#711)
Browse files Browse the repository at this point in the history 
This PR adds a custom Cobra command validator that outputs useful errors
when positional arguments are missing during command execution. For
example, such an error could look like this:
```
$ hcloud server describe   
hcloud server describe [options] <server>
                                  ^^^^^^
hcloud: expected argument server at position 1
```

Where previously it would look like this:
```
$ hcloud server describe
hcloud: accepts 1 arg(s), received 0
```

Additionally, if there are more arguments provided than necessary, the
following error message will appear:
```
$ hcloud server describe arg1 arg2
hcloud server describe [options] <server>
                                          ^
hcloud: expected exactly 1 positional argument(s), but got 2
```

This is done by parsing the `Use` property of a Cobra command with a
regular expression and then matching the corresponding missing
arguments.

Fixes #700

---------

Co-authored-by: pauhull <22707808+pauhull@users.noreply.github.com>
  • Loading branch information
@phm07
phm07 and phm07 committed Apr 2, 2024
1 parent 373287b commit e7f9e74
Show file tree
Hide file tree
Showing 114 changed files with 270 additions and 120 deletions.
24 changes: 23 additions & 1 deletion CONTRIBUTING.md
View file
Expand Up @@ -139,8 +139,30 @@ Additional Commands:
ssh Spawn an SSH connection for the server
```

### Command validation

Please use the `util.Validate` method to make sure the command is validated against its usage string during runtime.
This can be done by setting the `Args` field of the `cobra.Command` struct to `util.Validate`:

```go
cmd := &cobra.Command{
Use: "my-command [options]",
Args: util.Validate,
Run: func(cmd *cobra.Command, args []string) {
// ...
},
}
```

If you are using base commands like `base.Cmd` or `base.ListCmd` etc., this is already done for you.

**Note:** Your command usage needs to follow the [docopt](http://docopt.org/) syntax for this to work.
If your command uses optional positional arguments or other complicated usage that necessitates to disable
argument count checking, you use `util.ValidateLenient` instead. It will not throw an error if there are
more arguments than expected.

### Generated files

Generated files (that are created by running `go generate`) should be prefixed with `zz_`. This is to group them
Generated files (that are created by running `go generate`) should be prefixed with `zz_`. This is to group them
together in the file list and to easily identify them as generated files. Also, it allows the CI to check if the
generated files are up-to-date.
3 changes: 2 additions & 1 deletion internal/cmd/all/all.go
View file
Expand Up @@ -3,14 +3,15 @@ package all
import (
"github.com/spf13/cobra"

"github.com/hetznercloud/cli/internal/cmd/util"
"github.com/hetznercloud/cli/internal/state"
)

func NewCommand(s state.State) *cobra.Command {
cmd := &cobra.Command{
Use: "all",
Short: "Commands that apply to all resources",
Args: cobra.NoArgs,
Args: util.Validate,
TraverseChildren: true,
DisableFlagsInUseLine: true,
}
Expand Down
2 changes: 1 addition & 1 deletion internal/cmd/base/cmd.go
View file
Expand Up @@ -19,7 +19,7 @@ func (gc *Cmd) CobraCommand(s state.State) *cobra.Command {
cmd := gc.BaseCobraCommand(s.Client())

if cmd.Args == nil {
cmd.Args = cobra.NoArgs
cmd.Args = util.Validate
}

cmd.TraverseChildren = true
Expand Down
2 changes: 1 addition & 1 deletion internal/cmd/base/create.go
View file
Expand Up @@ -27,7 +27,7 @@ func (cc *CreateCmd) CobraCommand(s state.State) *cobra.Command {
output.AddFlag(cmd, output.OptionJSON(), output.OptionYAML())

if cmd.Args == nil {
cmd.Args = cobra.NoArgs
cmd.Args = util.Validate
}

cmd.TraverseChildren = true
Expand Down
5 changes: 2 additions & 3 deletions internal/cmd/base/delete.go
View file
Expand Up @@ -3,7 +3,6 @@ package base
import (
"fmt"
"reflect"
"strings"

"github.com/spf13/cobra"

Expand Down Expand Up @@ -32,9 +31,9 @@ func (dc *DeleteCmd) CobraCommand(s state.State) *cobra.Command {
}

cmd := &cobra.Command{
Use: fmt.Sprintf("delete %s<%s>", opts, strings.ToLower(dc.ResourceNameSingular)),
Use: fmt.Sprintf("delete %s<%s>", opts, util.ToKebabCase(dc.ResourceNameSingular)),
Short: dc.ShortDescription,
Args: cobra.ExactArgs(1),
Args: util.Validate,
ValidArgsFunction: cmpl.SuggestArgs(cmpl.SuggestCandidatesF(dc.NameSuggestions(s.Client()))),
TraverseChildren: true,
DisableFlagsInUseLine: true,
Expand Down
5 changes: 2 additions & 3 deletions internal/cmd/base/describe.go
View file
Expand Up @@ -4,7 +4,6 @@ import (
"fmt"
"os"
"reflect"
"strings"

"github.com/spf13/cobra"

Expand Down Expand Up @@ -33,9 +32,9 @@ type DescribeCmd struct {
// CobraCommand creates a command that can be registered with cobra.
func (dc *DescribeCmd) CobraCommand(s state.State) *cobra.Command {
cmd := &cobra.Command{
Use: fmt.Sprintf("describe [options] <%s>", strings.ToLower(dc.ResourceNameSingular)),
Use: fmt.Sprintf("describe [options] <%s>", util.ToKebabCase(dc.ResourceNameSingular)),
Short: dc.ShortDescription,
Args: cobra.ExactArgs(1),
Args: util.Validate,
ValidArgsFunction: cmpl.SuggestArgs(cmpl.SuggestCandidatesF(dc.NameSuggestions(s.Client()))),
TraverseChildren: true,
DisableFlagsInUseLine: true,
Expand Down
8 changes: 4 additions & 4 deletions internal/cmd/base/labels.go
View file
Expand Up @@ -27,9 +27,9 @@ type LabelCmds struct {
// AddCobraCommand creates a command that can be registered with cobra.
func (lc *LabelCmds) AddCobraCommand(s state.State) *cobra.Command {
cmd := &cobra.Command{
Use: fmt.Sprintf("add-label [--overwrite] <%s> <label>...", strings.ToLower(lc.ResourceNameSingular)),
Use: fmt.Sprintf("add-label [--overwrite] <%s> <label>...", util.ToKebabCase(lc.ResourceNameSingular)),
Short: lc.ShortDescriptionAdd,
Args: cobra.MinimumNArgs(2),
Args: util.Validate,
ValidArgsFunction: cmpl.SuggestArgs(cmpl.SuggestCandidatesF(lc.NameSuggestions(s.Client()))),
TraverseChildren: true,
DisableFlagsInUseLine: true,
Expand Down Expand Up @@ -89,9 +89,9 @@ func validateAddLabel(_ *cobra.Command, args []string) error {
// RemoveCobraCommand creates a command that can be registered with cobra.
func (lc *LabelCmds) RemoveCobraCommand(s state.State) *cobra.Command {
cmd := &cobra.Command{
Use: fmt.Sprintf("remove-label <%s> (--all | <label>...)", strings.ToLower(lc.ResourceNameSingular)),
Use: fmt.Sprintf("remove-label <%s> (--all | <label>...)", util.ToKebabCase(lc.ResourceNameSingular)),
Short: lc.ShortDescriptionRemove,
Args: cobra.MinimumNArgs(1),
Args: util.ValidateLenient,
ValidArgsFunction: cmpl.SuggestArgs(
cmpl.SuggestCandidatesF(lc.NameSuggestions(s.Client())),
cmpl.SuggestCandidatesCtx(func(_ *cobra.Command, args []string) []string {
Expand Down
1 change: 1 addition & 0 deletions internal/cmd/base/list.go
View file
Expand Up @@ -35,6 +35,7 @@ func (lc *ListCmd) CobraCommand(s state.State) *cobra.Command {
fmt.Sprintf("Displays a list of %s.", lc.ResourceNamePlural),
outputColumns,
),
Args: util.Validate,
TraverseChildren: true,
DisableFlagsInUseLine: true,
PreRunE: s.EnsureToken,
Expand Down
5 changes: 2 additions & 3 deletions internal/cmd/base/set_rdns.go
View file
Expand Up @@ -4,7 +4,6 @@ import (
"fmt"
"net"
"reflect"
"strings"

"github.com/spf13/cobra"

Expand All @@ -28,9 +27,9 @@ type SetRdnsCmd struct {
// CobraCommand creates a command that can be registered with cobra.
func (rc *SetRdnsCmd) CobraCommand(s state.State) *cobra.Command {
cmd := &cobra.Command{
Use: fmt.Sprintf("set-rdns [options] --hostname <hostname> <%s>", strings.ToLower(rc.ResourceNameSingular)),
Use: fmt.Sprintf("set-rdns [options] --hostname <hostname> <%s>", util.ToKebabCase(rc.ResourceNameSingular)),
Short: rc.ShortDescription,
Args: cobra.ExactArgs(1),
Args: util.Validate,
ValidArgsFunction: cmpl.SuggestArgs(cmpl.SuggestCandidatesF(rc.NameSuggestions(s.Client()))),
TraverseChildren: true,
DisableFlagsInUseLine: true,
Expand Down
5 changes: 2 additions & 3 deletions internal/cmd/base/update.go
View file
Expand Up @@ -3,7 +3,6 @@ package base
import (
"fmt"
"reflect"
"strings"

"github.com/spf13/cobra"
"github.com/spf13/pflag"
Expand All @@ -28,9 +27,9 @@ type UpdateCmd struct {
// CobraCommand creates a command that can be registered with cobra.
func (uc *UpdateCmd) CobraCommand(s state.State) *cobra.Command {
cmd := &cobra.Command{
Use: fmt.Sprintf("update [options] <%s>", strings.ToLower(uc.ResourceNameSingular)),
Use: fmt.Sprintf("update [options] <%s>", util.ToKebabCase(uc.ResourceNameSingular)),
Short: uc.ShortDescription,
Args: cobra.ExactArgs(1),
Args: util.Validate,
ValidArgsFunction: cmpl.SuggestArgs(cmpl.SuggestCandidatesF(uc.NameSuggestions(s.Client()))),
TraverseChildren: true,
DisableFlagsInUseLine: true,
Expand Down
3 changes: 2 additions & 1 deletion internal/cmd/certificate/certificate.go
View file
Expand Up @@ -3,14 +3,15 @@ package certificate
import (
"github.com/spf13/cobra"

"github.com/hetznercloud/cli/internal/cmd/util"
"github.com/hetznercloud/cli/internal/state"
)

func NewCommand(s state.State) *cobra.Command {
cmd := &cobra.Command{
Use: "certificate",
Short: "Manage certificates",
Args: cobra.NoArgs,
Args: util.Validate,
TraverseChildren: true,
DisableFlagsInUseLine: true,
}
Expand Down
1 change: 0 additions & 1 deletion internal/cmd/certificate/create.go
View file
Expand Up @@ -19,7 +19,6 @@ var CreateCmd = base.CreateCmd{
cmd := &cobra.Command{
Use: "create [options] --name <name> (--type managed --domain <domain> | --type uploaded --cert-file <file> --key-file <file>)",
Short: "Create or upload a Certificate",
Args: cobra.ExactArgs(0),
}

cmd.Flags().String("name", "", "Certificate name (required)")
Expand Down
3 changes: 2 additions & 1 deletion internal/cmd/completion/completion.go
View file
Expand Up @@ -6,6 +6,7 @@ import (

"github.com/spf13/cobra"

"github.com/hetznercloud/cli/internal/cmd/util"
"github.com/hetznercloud/cli/internal/state"
)

Expand Down Expand Up @@ -94,7 +95,7 @@ and source this file from your PowerShell profile.
PS> hcloud completion powershell > hcloud.ps1
`,
Args: cobra.ExactArgs(1),
Args: util.Validate,
ValidArgs: []string{"bash", "fish", "zsh", "powershell"},
DisableFlagsInUseLine: true,
RunE: func(cmd *cobra.Command, args []string) error {
Expand Down
3 changes: 2 additions & 1 deletion internal/cmd/context/active.go
View file
Expand Up @@ -6,14 +6,15 @@ import (

"github.com/spf13/cobra"

"github.com/hetznercloud/cli/internal/cmd/util"
"github.com/hetznercloud/cli/internal/state"
)

func newActiveCommand(s state.State) *cobra.Command {
cmd := &cobra.Command{
Use: "active",
Short: "Show active context",
Args: cobra.NoArgs,
Args: util.Validate,
TraverseChildren: true,
DisableFlagsInUseLine: true,
RunE: state.Wrap(s, runActive),
Expand Down
3 changes: 2 additions & 1 deletion internal/cmd/context/context.go
View file
Expand Up @@ -3,14 +3,15 @@ package context
import (
"github.com/spf13/cobra"

"github.com/hetznercloud/cli/internal/cmd/util"
"github.com/hetznercloud/cli/internal/state"
)

func NewCommand(s state.State) *cobra.Command {
cmd := &cobra.Command{
Use: "context",
Short: "Manage contexts",
Args: cobra.NoArgs,
Args: util.Validate,
TraverseChildren: true,
DisableFlagsInUseLine: true,
}
Expand Down
3 changes: 2 additions & 1 deletion internal/cmd/context/create.go
View file
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/spf13/cobra"
"golang.org/x/term"

"github.com/hetznercloud/cli/internal/cmd/util"
"github.com/hetznercloud/cli/internal/state"
"github.com/hetznercloud/cli/internal/state/config"
)
Expand All @@ -19,7 +20,7 @@ func newCreateCommand(s state.State) *cobra.Command {
cmd := &cobra.Command{
Use: "create <name>",
Short: "Create a new context",
Args: cobra.ExactArgs(1),
Args: util.Validate,
TraverseChildren: true,
DisableFlagsInUseLine: true,
RunE: state.Wrap(s, runCreate),
Expand Down
3 changes: 2 additions & 1 deletion internal/cmd/context/delete.go
View file
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/spf13/cobra"

"github.com/hetznercloud/cli/internal/cmd/cmpl"
"github.com/hetznercloud/cli/internal/cmd/util"
"github.com/hetznercloud/cli/internal/state"
"github.com/hetznercloud/cli/internal/state/config"
)
Expand All @@ -15,7 +16,7 @@ func newDeleteCommand(s state.State) *cobra.Command {
cmd := &cobra.Command{
Use: "delete <context>",
Short: "Delete a context",
Args: cobra.ExactArgs(1),
Args: util.Validate,
ValidArgsFunction: cmpl.SuggestArgs(cmpl.SuggestCandidates(config.ContextNames(s.Config())...)),
TraverseChildren: true,
DisableFlagsInUseLine: true,
Expand Down
2 changes: 1 addition & 1 deletion internal/cmd/context/list.go
View file
Expand Up @@ -30,7 +30,7 @@ func newListCommand(s state.State) *cobra.Command {
"Displays a list of contexts.",
listTableOutput.Columns(),
),
Args: cobra.NoArgs,
Args: util.Validate,
TraverseChildren: true,
DisableFlagsInUseLine: true,
RunE: state.Wrap(s, runList),
Expand Down
3 changes: 2 additions & 1 deletion internal/cmd/context/use.go
View file
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/spf13/cobra"

"github.com/hetznercloud/cli/internal/cmd/cmpl"
"github.com/hetznercloud/cli/internal/cmd/util"
"github.com/hetznercloud/cli/internal/state"
"github.com/hetznercloud/cli/internal/state/config"
)
Expand All @@ -15,7 +16,7 @@ func newUseCommand(s state.State) *cobra.Command {
cmd := &cobra.Command{
Use: "use <context>",
Short: "Use a context",
Args: cobra.ExactArgs(1),
Args: util.Validate,
ValidArgsFunction: cmpl.SuggestArgs(cmpl.SuggestCandidates(config.ContextNames(s.Config())...)),
TraverseChildren: true,
DisableFlagsInUseLine: true,
Expand Down
3 changes: 2 additions & 1 deletion internal/cmd/datacenter/datacenter.go
View file
Expand Up @@ -3,14 +3,15 @@ package datacenter
import (
"github.com/spf13/cobra"

"github.com/hetznercloud/cli/internal/cmd/util"
"github.com/hetznercloud/cli/internal/state"
)

func NewCommand(s state.State) *cobra.Command {
cmd := &cobra.Command{
Use: "datacenter",
Short: "Manage datacenters",
Args: cobra.NoArgs,
Args: util.Validate,
TraverseChildren: true,
DisableFlagsInUseLine: true,
}
Expand Down
1 change: 0 additions & 1 deletion internal/cmd/firewall/add_rule.go
View file
Expand Up @@ -18,7 +18,6 @@ var AddRuleCmd = base.Cmd{
cmd := &cobra.Command{
Use: "add-rule [options] (--direction in --source-ips <ips> | --direction out --destination-ips <ips>) --protocol <icmp|udp|tcp|esp|gre> <firewall>",
Short: "Add a single rule to a firewall",
Args: cobra.ExactArgs(1),
ValidArgsFunction: cmpl.SuggestArgs(cmpl.SuggestCandidatesF(client.Firewall().Names)),
TraverseChildren: true,
DisableFlagsInUseLine: true,
Expand Down
1 change: 0 additions & 1 deletion internal/cmd/firewall/apply_to_resource.go
View file
Expand Up @@ -17,7 +17,6 @@ var ApplyToResourceCmd = base.Cmd{
cmd := &cobra.Command{
Use: "apply-to-resource (--type server --server <server> | --type label_selector --label-selector <label-selector>) <firewall>",
Short: "Applies a Firewall to a single resource",
Args: cobra.ExactArgs(1),
ValidArgsFunction: cmpl.SuggestArgs(cmpl.SuggestCandidatesF(client.Firewall().Names)),
TraverseChildren: true,
DisableFlagsInUseLine: true,
Expand Down
1 change: 0 additions & 1 deletion internal/cmd/firewall/create.go
View file
Expand Up @@ -22,7 +22,6 @@ var CreateCmd = base.CreateCmd{
cmd := &cobra.Command{
Use: "create [options] --name <name>",
Short: "Create a Firewall",
Args: cobra.NoArgs,
}
cmd.Flags().String("name", "", "Name")
cmd.MarkFlagRequired("name")
Expand Down
1 change: 0 additions & 1 deletion internal/cmd/firewall/delete_rule.go
View file
Expand Up @@ -19,7 +19,6 @@ var DeleteRuleCmd = base.Cmd{
cmd := &cobra.Command{
Use: "delete-rule [options] (--direction in --source-ips <ips> | --direction out --destination-ips <ips>) --protocol <icmp|esp|gre|udp|tcp> <firewall>",
Short: "Delete a single rule to a firewall",
Args: cobra.ExactArgs(1),
ValidArgsFunction: cmpl.SuggestArgs(cmpl.SuggestCandidatesF(client.Firewall().Names)),
TraverseChildren: true,
DisableFlagsInUseLine: true,
Expand Down
2 changes: 1 addition & 1 deletion internal/cmd/firewall/firewall.go
View file
Expand Up @@ -11,7 +11,7 @@ func NewCommand(s state.State) *cobra.Command {
cmd := &cobra.Command{
Use: "firewall",
Short: "Manage Firewalls",
Args: cobra.NoArgs,
Args: util.Validate,
TraverseChildren: true,
DisableFlagsInUseLine: true,
}
Expand Down
1 change: 0 additions & 1 deletion internal/cmd/firewall/remove_from_resource.go
View file
Expand Up @@ -17,7 +17,6 @@ var RemoveFromResourceCmd = base.Cmd{
cmd := &cobra.Command{
Use: "remove-from-resource (--type server --server <server> | --type label_selector --label-selector <label-selector>) <firewall>",
Short: "Removes a Firewall from a single resource",
Args: cobra.ExactArgs(1),
ValidArgsFunction: cmpl.SuggestArgs(cmpl.SuggestCandidatesF(client.Firewall().Names)),
TraverseChildren: true,
DisableFlagsInUseLine: true,
Expand Down
1 change: 0 additions & 1 deletion internal/cmd/firewall/replace_rules.go
View file
Expand Up @@ -22,7 +22,6 @@ var ReplaceRulesCmd = base.Cmd{
cmd := &cobra.Command{
Use: "replace-rules --rules-file <file> <firewall>",
Short: "Replaces all rules from a Firewall from a file",
Args: cobra.ExactArgs(1),
ValidArgsFunction: cmpl.SuggestArgs(cmpl.SuggestCandidatesF(client.Firewall().Names)),
TraverseChildren: true,
DisableFlagsInUseLine: true,
Expand Down
1 change: 0 additions & 1 deletion internal/cmd/floatingip/assign.go
View file
Expand Up @@ -16,7 +16,6 @@ var AssignCmd = base.Cmd{
return &cobra.Command{
Use: "assign <floating-ip> <server>",
Short: "Assign a Floating IP to a server",
Args: cobra.ExactArgs(2),
ValidArgsFunction: cmpl.SuggestArgs(
cmpl.SuggestCandidatesF(client.FloatingIP().Names),
cmpl.SuggestCandidatesF(client.Server().Names),
Expand Down

0 comments on commit e7f9e74

Please sign in to comment.