diff --git a/args.go b/args.go index b3acfd5ccc..0dad209914 100644 --- a/args.go +++ b/args.go @@ -77,6 +77,11 @@ type Argument interface { Get() any } +type requiredArgument interface { + name() string + required() bool +} + // AnyArguments to differentiate between no arguments(nil) vs aleast one var AnyArguments = []Argument{ &StringArgs{ @@ -89,6 +94,7 @@ type ArgumentBase[T any, C any, VC ValueCreator[T, C]] struct { Value T `json:"value"` // the default value of this argument Destination *T `json:"-"` // the destination point for this argument UsageText string `json:"usageText"` // the usage text to show + Required bool `json:"required"` // whether the argument is required or not Config C `json:"config"` // config for this argument similar to Flag Config value *T @@ -98,18 +104,33 @@ func (a *ArgumentBase[T, C, VC]) HasName(s string) bool { return s == a.Name } +func (a *ArgumentBase[T, C, VC]) name() string { + return a.Name +} + +func (a *ArgumentBase[T, C, VC]) required() bool { + return a.Required +} + func (a *ArgumentBase[T, C, VC]) Usage() string { if a.UsageText != "" { return a.UsageText } - usageFormat := "%[1]s" + usageFormat := "[%[1]s]" + if a.Required { + usageFormat = "%[1]s" + } return fmt.Sprintf(usageFormat, a.Name) } func (a *ArgumentBase[T, C, VC]) Parse(s []string) ([]string, error) { tracef("calling arg%[1] parse with args %[2]", a.Name, s) + if a.Required && len(s) == 0 { + return s, &errRequiredArguments{missingArguments: []string{a.Name}} + } + var vc VC var t T value := vc.Create(a.Value, &t, a.Config) diff --git a/args_test.go b/args_test.go index 7ae337487f..ac3d754a14 100644 --- a/args_test.go +++ b/args_test.go @@ -1,7 +1,9 @@ package cli import ( + "bytes" "context" + "errors" "testing" "time" @@ -422,10 +424,16 @@ func TestArgUsage(t *testing.T) { tests := []struct { name string usage string + required bool expected string }{ { name: "default", + expected: "[ia]", + }, + { + name: "required", + required: true, expected: "ia", }, { @@ -436,7 +444,7 @@ func TestArgUsage(t *testing.T) { } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - arg.UsageText = test.usage + arg.UsageText, arg.Required = test.usage, test.required require.Equal(t, test.expected, arg.Usage()) }) } @@ -552,6 +560,180 @@ func TestSingleOptionalArg(t *testing.T) { } } +func TestSingleRequiredArg(t *testing.T) { + tests := []struct { + name string + args []string + argValue string + exp string + expErr string + }{ + { + name: "no args", + args: []string{"foo"}, + expErr: `Required argument "sa" not set`, + }, + { + name: "no arg with def value", + args: []string{"foo"}, + argValue: "bar", + expErr: `Required argument "sa" not set`, + }, + { + name: "one arg", + args: []string{"foo", "zbar"}, + exp: "zbar", + }, + { + name: "empty string arg", + args: []string{"foo", ""}, + exp: "", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + cmd := buildMinimalTestCommand() + var s1 string + arg := &StringArg{ + Name: "sa", + Value: test.argValue, + Destination: &s1, + Required: true, + } + cmd.Arguments = []Argument{ + arg, + } + + err := cmd.Run(buildTestContext(t), test.args) + r := require.New(t) + if test.expErr != "" { + r.EqualError(err, test.expErr) + return + } + r.NoError(err) + r.Equal(test.exp, s1) + }) + } +} + +func TestMissingRequiredArgDoesNotMutateValue(t *testing.T) { + destination := "unchanged" + arg := &StringArg{ + Name: "sa", + Value: "default", + Destination: &destination, + Required: true, + } + initialValue := arg.Get() + writer := &bytes.Buffer{} + errWriter := &bytes.Buffer{} + cmd := buildMinimalTestCommand() + cmd.Writer = writer + cmd.ErrWriter = errWriter + cmd.Arguments = []Argument{arg} + + err := cmd.Run(buildTestContext(t), []string{"foo"}) + + r := require.New(t) + r.IsType(&errRequiredArguments{}, err) + r.Equal("unchanged", destination) + r.Equal(initialValue, arg.Get()) + r.Contains(errWriter.String(), `Incorrect Usage: Required argument "sa" not set`) + r.Contains(writer.String(), "NAME:") +} + +func TestChainedRequiredArgs(t *testing.T) { + cmd := buildMinimalTestCommand() + cmd.Arguments = []Argument{ + &StringArg{ + Name: "first", + Required: true, + }, + &StringArg{ + Name: "second", + Required: true, + }, + } + + r := require.New(t) + r.EqualError(cmd.Run(buildTestContext(t), []string{"foo"}), `Required arguments "first, second" not set`) + r.EqualError(cmd.Run(buildTestContext(t), []string{"foo", "one"}), `Required argument "second" not set`) + r.NoError(cmd.Run(buildTestContext(t), []string{"foo", "one", "two"})) +} + +func TestRequiredArgAfterOptionalArg(t *testing.T) { + cmd := buildMinimalTestCommand() + cmd.Arguments = []Argument{ + &StringArg{Name: "optional"}, + &StringArg{Name: "required", Required: true}, + } + + r := require.New(t) + r.EqualError(cmd.Run(buildTestContext(t), []string{"foo", "one"}), `Required argument "required" not set`) + r.NoError(cmd.Run(buildTestContext(t), []string{"foo", "one", "two"})) +} + +func TestRequiredArgAfterMultiValueArgUsesRequiredErrorHandling(t *testing.T) { + writer := &bytes.Buffer{} + errWriter := &bytes.Buffer{} + cmd := buildMinimalTestCommand() + cmd.Writer = writer + cmd.ErrWriter = errWriter + cmd.Arguments = []Argument{ + &StringArgs{Name: "rest", Min: 0, Max: -1}, + &StringArg{Name: "required", Required: true}, + } + + err := cmd.Run(buildTestContext(t), []string{"foo", "one", "two"}) + + r := require.New(t) + r.IsType(&errRequiredArguments{}, err) + r.True(cmd.isInError) + r.Contains(errWriter.String(), `Incorrect Usage: Required argument "required" not set`) + r.Contains(writer.String(), "NAME:") +} + +func TestCheckRequiredArgumentsSkipsHelpAndCompletionCommands(t *testing.T) { + tests := []struct { + name string + cmd *Command + }{ + { + name: "built-in help", + cmd: &Command{builtInHelp: true}, + }, + { + name: "completion", + cmd: &Command{isCompletionCommand: true}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + test.cmd.Arguments = []Argument{ + &StringArg{Name: "required", Required: true}, + } + + require.NoError(t, test.cmd.checkRequiredArguments()) + }) + } +} + +func TestRequiredArgWithOnUsageError(t *testing.T) { + expectedErr := errors.New("OnUsageError") + cmd := buildMinimalTestCommand() + cmd.Arguments = []Argument{ + &StringArg{Name: "required", Required: true}, + } + cmd.OnUsageError = func(_ context.Context, _ *Command, err error, _ bool) error { + require.IsType(t, &errRequiredArguments{}, err) + return expectedErr + } + + require.ErrorIs(t, cmd.Run(buildTestContext(t), []string{"foo"}), expectedErr) +} + func TestUnboundedArgs(t *testing.T) { arg := &StringArgs{ Min: 0, diff --git a/command.go b/command.go index 4cd907a558..6f80294755 100644 --- a/command.go +++ b/command.go @@ -454,6 +454,40 @@ func (cmd *Command) checkRequiredFlags() requiredFlagsErr { return nil } +func (cmd *Command) checkRequiredArguments() requiredArgumentsErr { + // The help and completion commands are allowed to run without + // enforcement of required arguments, since they do not invoke user + // actions that depend on those argument values. + if cmd.builtInHelp || cmd.isCompletionCommand { + return nil + } + + tracef("checking for required arguments (cmd=%[1]q)", cmd.Name) + + missingArguments := []string{} + // This count-based precheck relies on required single-value arguments + // being declared before optional or multi-value arguments, as documented. + // Argument.Parse remains the backstop for unsupported orderings. + providedArguments := cmd.Args().Len() + + for index, arg := range cmd.Arguments { + requiredArg, ok := arg.(requiredArgument) + if ok && requiredArg.required() && index >= providedArguments { + missingArguments = append(missingArguments, requiredArg.name()) + } + } + + if len(missingArguments) != 0 { + tracef("found missing required arguments %[1]q (cmd=%[2]q)", missingArguments, cmd.Name) + + return &errRequiredArguments{missingArguments: missingArguments} + } + + tracef("all required arguments set (cmd=%[1]q)", cmd.Name) + + return nil +} + func (cmd *Command) onInvalidFlag(ctx context.Context, name string) { for cmd != nil { if cmd.InvalidFlagAccessHandler != nil { diff --git a/command_run.go b/command_run.go index 8d5907151e..e0fd3f435c 100644 --- a/command_run.go +++ b/command_run.go @@ -343,21 +343,14 @@ func (cmd *Command) run(ctx context.Context, osArgs []string) (_ context.Context } } + var requiredErr error if err := cmd.checkAllRequiredFlags(); err != nil { - cmd.isInError = true - if cmd.OnUsageError != nil { - err = cmd.OnUsageError(ctx, cmd, err, cmd.parent != nil) - } else { - fmt.Fprintf(cmd.Root().ErrWriter, "Incorrect Usage: %s\n\n", err.Error()) - if cmd.parent == nil { - _ = ShowRootCommandHelp(cmd) - } else { - if err := ShowCommandHelp(ctx, cmd.parent, cmd.Name); err != nil { - _ = ShowSubcommandHelp(cmd) - } - } - } - return ctx, err + requiredErr = err + } else if err := cmd.checkRequiredArguments(); err != nil { + requiredErr = err + } + if requiredErr != nil { + return cmd.handleRequiredError(ctx, requiredErr) } // Run the command action. @@ -369,6 +362,9 @@ func (cmd *Command) run(ctx context.Context, osArgs []string) (_ context.Context rargs, err = arg.Parse(rargs) if err != nil { tracef("calling with %[1]v (cmd=%[2]q)", err, cmd.Name) + if _, ok := err.(*errRequiredArguments); ok { + return cmd.handleRequiredError(ctx, err) + } if cmd.OnUsageError != nil { err = cmd.OnUsageError(ctx, cmd, err, cmd.parent != nil) } @@ -388,6 +384,21 @@ func (cmd *Command) run(ctx context.Context, osArgs []string) (_ context.Context return ctx, deferErr } +func (cmd *Command) handleRequiredError(ctx context.Context, err error) (context.Context, error) { + cmd.isInError = true + if cmd.OnUsageError != nil { + err = cmd.OnUsageError(ctx, cmd, err, cmd.parent != nil) + } else { + fmt.Fprintf(cmd.Root().ErrWriter, "Incorrect Usage: %s\n\n", err.Error()) + if cmd.parent == nil { + _ = ShowRootCommandHelp(cmd) + } else if helpErr := ShowCommandHelp(ctx, cmd.parent, cmd.Name); helpErr != nil { + _ = ShowSubcommandHelp(cmd) + } + } + return ctx, err +} + func commandChain(cmd *Command) []*Command { var cmdChain []*Command for p := cmd; p != nil; p = p.parent { diff --git a/docs/v3/examples/arguments/advanced.md b/docs/v3/examples/arguments/advanced.md index 7ab3da9a55..a223679a61 100644 --- a/docs/v3/examples/arguments/advanced.md +++ b/docs/v3/examples/arguments/advanced.md @@ -111,6 +111,48 @@ Some of the basic types arguments supported are This is ok for single value arguments. Any number of these single value arguments can be concatenated in the `Arguments` slice field of `Command`. +Single value arguments are optional by default. If the argument is not provided the default `Value` is used instead. +You can mark a single value argument as *required* by setting the `Required` field to `true`. If a user does not +provide a required argument, they will be shown an error message. + +Required single-value arguments should be declared before optional or multi-value arguments because arguments are +consumed in declaration order. + + +```go +package main + +import ( + "context" + "fmt" + "log" + "os" + + "github.com/urfave/cli/v3" +) + +func main() { + cmd := &cli.Command{ + Arguments: []cli.Argument{ + &cli.IntArg{ + Name: "someint", + Required: true, + }, + }, + Action: func(ctx context.Context, cmd *cli.Command) error { + fmt.Printf("We got %d", cmd.IntArg("someint")) + return nil + }, + } + + if err := cmd.Run(context.Background(), os.Args); err != nil { + log.Fatal(err) + } +} +``` + The library also support multi value arguments for e.g