Repository: urfave/cli
Pull request #1864 (2024-02-11 16:13:07)

Missing arg can cause a panic
<!--
  This template provides some ideas of things to include in your PR description.
  To start, try providing a short summary of your changes in the Title above.
  If a section of the PR template does not apply to this PR, then delete that section.
 -->

## What type of PR is this?

- bug

## What this PR does / why we need it:

<!--
  What goal is this change working towards?
  Provide a bullet pointed summary of how each file was changed.
  Briefly explain any decisions you made with respect to the changes.
  Include anything here that you didn't include in *Release Notes*
  above, such as changes to CI or changes to internal methods.
-->

- In the special case where an arg has `max: 1`, `args.Parse` would panic when arg was omitted.
- Also ensure omitted args retain their `Value`, if provided.

```
--- FAIL: TestSingleOptionalArg (0.00s)
panic: runtime error: index out of range [0] with length 0 [recovered]
        panic: runtime error: index out of range [0] with length 0

goroutine 15 [running]:
testing.tRunner.func1.2({0x163bc20, 0xc00001c660})
        /usr/local/Cellar/go/1.21.4/libexec/src/testing/testing.go:1545 +0x366
testing.tRunner.func1()
        /usr/local/Cellar/go/1.21.4/libexec/src/testing/testing.go:1548 +0x630
panic({0x163bc20?, 0xc00001c660?})
        /usr/local/Cellar/go/1.21.4/libexec/src/runtime/panic.go:920 +0x270
github.com/urfave/cli/v3.(*ArgumentBase[...]).Parse(0x1729d40, {0xc0003381a0, 0x0?, 0x0})
        /Users/josh/src/cli/args.go:136 +0xc88
```

## Testing

Tests added for an omitted arg with `max: 1`, with and without a default `Value`.

## Release Notes

<!--
  If this PR makes user facing changes, please describe them here. This
  description will be copied into the release notes/changelog, whenever the
  next version is released. Keep this section short, and focus on high level
  changes.

  Put your text between the block. To omit notes, use NONE within the block.
-->

```release-note
Optional arguments with arity 1 no longer panic when omitted.
```



Patch:
diff --git a/args.go b/args.go
index f311a93..59b6e4e 100644
--- a/args.go
+++ b/args.go
@@ -133,7 +133,11 @@ func (a *ArgumentBase[T, C, VC]) Parse(s []string) ([]string, error) {
 	}
 
 	if a.Max == 1 && a.Destination != nil {
-		*a.Destination = values[0]
+		if len(values) > 0 {
+			*a.Destination = values[0]
+		} else {
+			*a.Destination = t
+		}
 	}
 	return s[count:], nil
 }


Test patch:
diff --git a/args_test.go b/args_test.go
index 14f8944..e951c8b 100644
--- a/args_test.go
+++ b/args_test.go
@@ -151,3 +151,26 @@ func TestArgsUsage(t *testing.T) {
 		})
 	}
 }
+
+func TestSingleOptionalArg(t *testing.T) {
+	cmd := buildMinimalTestCommand()
+	var s1 string
+	arg := &StringArg{
+		Min:         0,
+		Max:         1,
+		Destination: &s1,
+	}
+	cmd.Arguments = []Argument{
+		arg,
+	}
+
+	require.NoError(t, cmd.Run(context.Background(), []string{"foo"}))
+	require.Equal(t, "", s1)
+
+	arg.Value = "bar"
+	require.NoError(t, cmd.Run(context.Background(), []string{"foo"}))
+	require.Equal(t, "bar", s1)
+
+	require.NoError(t, cmd.Run(context.Background(), []string{"foo", "zbar"}))
+	require.Equal(t, "zbar", s1)
+}