Skip to content

Commit 1d6cbe1

Browse files
umarcorumarcor
committed
feat: generalize ValidArgs; use it implicitly with any validator (spf13#841)
1 parent 7a64745 commit 1d6cbe1

File tree

5 files changed

+173
-208
lines changed

5 files changed

+173
-208
lines changed

‎args.go

+34-31
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,25 @@ import (
77

88
type PositionalArgs func(cmd *Command, args []string) error
99

10+
// validateArgs returns an error if there are any positional args that are not in
11+
// the `ValidArgs` field of `Command`
12+
func validateArgs(cmd *Command, args []string) error {
13+
if len(cmd.ValidArgs) > 0 {
14+
// Remove any description that may be included in ValidArgs.
15+
// A description is following a tab character.
16+
var validArgs []string
17+
for _, v := range cmd.ValidArgs {
18+
validArgs = append(validArgs, strings.Split(v, "\t")[0])
19+
}
20+
for _, v := range args {
21+
if !stringInSlice(v, validArgs) {
22+
return fmt.Errorf("invalid argument %q for %q%s", v, cmd.CommandPath(), cmd.findSuggestions(args[0]))
23+
}
24+
}
25+
}
26+
return nil
27+
}
28+
1029
// Legacy arg validation has the following behaviour:
1130
// - root commands with no subcommands can take arbitrary arguments
1231
// - root commands with subcommands will do subcommand validity checking
@@ -32,25 +51,6 @@ func NoArgs(cmd *Command, args []string) error {
3251
return nil
3352
}
3453

35-
// OnlyValidArgs returns an error if any args are not in the list of ValidArgs.
36-
func OnlyValidArgs(cmd *Command, args []string) error {
37-
if len(cmd.ValidArgs) > 0 {
38-
// Remove any description that may be included in ValidArgs.
39-
// A description is following a tab character.
40-
var validArgs []string
41-
for _, v := range cmd.ValidArgs {
42-
validArgs = append(validArgs, strings.Split(v, "\t")[0])
43-
}
44-
45-
for _, v := range args {
46-
if !stringInSlice(v, validArgs) {
47-
return fmt.Errorf("invalid argument %q for %q%s", v, cmd.CommandPath(), cmd.findSuggestions(args[0]))
48-
}
49-
}
50-
}
51-
return nil
52-
}
53-
5454
// ArbitraryArgs never returns an error.
5555
func ArbitraryArgs(cmd *Command, args []string) error {
5656
return nil
@@ -86,18 +86,6 @@ func ExactArgs(n int) PositionalArgs {
8686
}
8787
}
8888

89-
// ExactValidArgs returns an error if
90-
// there are not exactly N positional args OR
91-
// there are any positional args that are not in the `ValidArgs` field of `Command`
92-
func ExactValidArgs(n int) PositionalArgs {
93-
return func(cmd *Command, args []string) error {
94-
if err := ExactArgs(n)(cmd, args); err != nil {
95-
return err
96-
}
97-
return OnlyValidArgs(cmd, args)
98-
}
99-
}
100-
10189
// RangeArgs returns an error if the number of args is not within the expected range.
10290
func RangeArgs(min int, max int) PositionalArgs {
10391
return func(cmd *Command, args []string) error {
@@ -119,3 +107,18 @@ func MatchAll(pargs ...PositionalArgs) PositionalArgs {
119107
return nil
120108
}
121109
}
110+
111+
// ExactValidArgs returns an error if there are not exactly N positional args OR
112+
// there are any positional args that are not in the `ValidArgs` field of `Command`
113+
//
114+
// Deprecated: now `ExactArgs` honors `ValidArgs`, when defined and not empty
115+
func ExactValidArgs(n int) PositionalArgs {
116+
return ExactArgs(n)
117+
}
118+
119+
// OnlyValidArgs returns an error if any args are not in the list of `ValidArgs`.
120+
//
121+
// Deprecated: now `ArbitraryArgs` honors `ValidArgs`, when defined and not empty
122+
func OnlyValidArgs(cmd *Command, args []string) error {
123+
return ArbitraryArgs(cmd, args)
124+
}

‎args_test.go

+108-160
Original file line numberDiff line numberDiff line change
@@ -6,188 +6,136 @@ import (
66
"testing"
77
)
88

9-
func getCommand(args PositionalArgs, withValid bool) *Command {
10-
c := &Command{
11-
Use: "c",
12-
Args: args,
13-
Run: emptyRun,
14-
}
15-
if withValid {
16-
c.ValidArgs = []string{"one", "two", "three"}
17-
}
18-
return c
19-
}
20-
21-
func expectSuccess(output string, err error, t *testing.T) {
22-
if output != "" {
23-
t.Errorf("Unexpected output: %v", output)
24-
}
25-
if err != nil {
26-
t.Fatalf("Unexpected error: %v", err)
27-
}
9+
type argsTestcase struct {
10+
exerr string // Expected error key (see map[string][string])
11+
args PositionalArgs // Args validator
12+
wValid bool // Define `ValidArgs` in the command
13+
rargs []string // Runtime args
2814
}
2915

30-
func validWithInvalidArgs(err error, t *testing.T) {
31-
if err == nil {
32-
t.Fatal("Expected an error")
33-
}
34-
got := err.Error()
35-
expected := `invalid argument "a" for "c"`
36-
if got != expected {
37-
t.Errorf("Expected: %q, got: %q", expected, got)
38-
}
16+
var errStrings = map[string]string{
17+
"invalid": `invalid argument "a" for "c"`,
18+
"unknown": `unknown command "one" for "c"`,
19+
"less": "requires at least 2 arg(s), only received 1",
20+
"more": "accepts at most 2 arg(s), received 3",
21+
"notexact": "accepts 2 arg(s), received 3",
22+
"notinrange": "accepts between 2 and 4 arg(s), received 1",
3923
}
4024

41-
func noArgsWithArgs(err error, t *testing.T) {
42-
if err == nil {
43-
t.Fatal("Expected an error")
44-
}
45-
got := err.Error()
46-
expected := `unknown command "illegal" for "c"`
47-
if got != expected {
48-
t.Errorf("Expected: %q, got: %q", expected, got)
49-
}
50-
}
51-
52-
func minimumNArgsWithLessArgs(err error, t *testing.T) {
53-
if err == nil {
54-
t.Fatal("Expected an error")
55-
}
56-
got := err.Error()
57-
expected := "requires at least 2 arg(s), only received 1"
58-
if got != expected {
59-
t.Fatalf("Expected %q, got %q", expected, got)
60-
}
61-
}
62-
63-
func maximumNArgsWithMoreArgs(err error, t *testing.T) {
64-
if err == nil {
65-
t.Fatal("Expected an error")
25+
func (tc *argsTestcase) test(t *testing.T) {
26+
c := &Command{
27+
Use: "c",
28+
Args: tc.args,
29+
Run: emptyRun,
6630
}
67-
got := err.Error()
68-
expected := "accepts at most 2 arg(s), received 3"
69-
if got != expected {
70-
t.Fatalf("Expected %q, got %q", expected, got)
31+
if tc.wValid {
32+
c.ValidArgs = []string{"one", "two", "three"}
7133
}
72-
}
7334

74-
func exactArgsWithInvalidCount(err error, t *testing.T) {
75-
if err == nil {
76-
t.Fatal("Expected an error")
77-
}
78-
got := err.Error()
79-
expected := "accepts 2 arg(s), received 3"
80-
if got != expected {
81-
t.Fatalf("Expected %q, got %q", expected, got)
35+
o, e := executeCommand(c, tc.rargs...)
36+
37+
if len(tc.exerr) > 0 {
38+
// Expect error
39+
if e == nil {
40+
t.Fatal("Expected an error")
41+
}
42+
expected, ok := errStrings[tc.exerr]
43+
if !ok {
44+
t.Errorf(`key "%s" is not found in map "errStrings"`, tc.exerr)
45+
return
46+
}
47+
if got := e.Error(); got != expected {
48+
t.Errorf("Expected: %q, got: %q", expected, got)
49+
}
50+
} else {
51+
// Expect success
52+
if o != "" {
53+
t.Errorf("Unexpected output: %v", o)
54+
}
55+
if e != nil {
56+
t.Fatalf("Unexpected error: %v", e)
57+
}
8258
}
8359
}
8460

85-
func rangeArgsWithInvalidCount(err error, t *testing.T) {
86-
if err == nil {
87-
t.Fatal("Expected an error")
88-
}
89-
got := err.Error()
90-
expected := "accepts between 2 and 4 arg(s), received 1"
91-
if got != expected {
92-
t.Fatalf("Expected %q, got %q", expected, got)
61+
func testArgs(t *testing.T, tests map[string]argsTestcase) {
62+
for name, tc := range tests {
63+
t.Run(name, tc.test)
9364
}
9465
}
9566

96-
func TestNoArgs(t *testing.T) {
97-
c := getCommand(NoArgs, false)
98-
output, err := executeCommand(c)
99-
expectSuccess(output, err, t)
100-
}
101-
102-
func TestNoArgsWithArgs(t *testing.T) {
103-
c := getCommand(NoArgs, false)
104-
_, err := executeCommand(c, "illegal")
105-
noArgsWithArgs(err, t)
106-
}
107-
108-
func TestOnlyValidArgs(t *testing.T) {
109-
c := getCommand(OnlyValidArgs, true)
110-
output, err := executeCommand(c, "one", "two")
111-
expectSuccess(output, err, t)
112-
}
113-
114-
func TestOnlyValidArgsWithInvalidArgs(t *testing.T) {
115-
c := getCommand(OnlyValidArgs, true)
116-
_, err := executeCommand(c, "a")
117-
validWithInvalidArgs(err, t)
67+
func TestArgs_No(t *testing.T) {
68+
testArgs(t, map[string]argsTestcase{
69+
" | ": {"", NoArgs, false, []string{}},
70+
" | Arb": {"unknown", NoArgs, false, []string{"one"}},
71+
"Valid | Valid": {"unknown", NoArgs, true, []string{"one"}},
72+
})
11873
}
119-
120-
func TestArbitraryArgs(t *testing.T) {
121-
c := getCommand(ArbitraryArgs, false)
122-
output, err := executeCommand(c, "a", "b")
123-
expectSuccess(output, err, t)
74+
func TestArgs_Nil(t *testing.T) {
75+
testArgs(t, map[string]argsTestcase{
76+
" | Arb": {"", nil, false, []string{"a", "b"}},
77+
"Valid | Valid": {"", nil, true, []string{"one", "two"}},
78+
"Valid | Invalid": {"invalid", nil, true, []string{"a"}},
79+
})
12480
}
125-
126-
func TestMinimumNArgs(t *testing.T) {
127-
c := getCommand(MinimumNArgs(2), false)
128-
output, err := executeCommand(c, "a", "b", "c")
129-
expectSuccess(output, err, t)
81+
func TestArgs_Arbitrary(t *testing.T) {
82+
testArgs(t, map[string]argsTestcase{
83+
" | Arb": {"", ArbitraryArgs, false, []string{"a", "b"}},
84+
"Valid | Valid": {"", ArbitraryArgs, true, []string{"one", "two"}},
85+
"Valid | Invalid": {"invalid", ArbitraryArgs, true, []string{"a"}},
86+
})
13087
}
131-
132-
func TestMinimumNArgsWithLessArgs(t *testing.T) {
133-
c := getCommand(MinimumNArgs(2), false)
134-
_, err := executeCommand(c, "a")
135-
minimumNArgsWithLessArgs(err, t)
88+
func TestArgs_MinimumN(t *testing.T) {
89+
testArgs(t, map[string]argsTestcase{
90+
" | Arb": {"", MinimumNArgs(2), false, []string{"a", "b", "c"}},
91+
"Valid | Valid": {"", MinimumNArgs(2), true, []string{"one", "three"}},
92+
"Valid | Invalid": {"invalid", MinimumNArgs(2), true, []string{"a", "b"}},
93+
" | Less": {"less", MinimumNArgs(2), false, []string{"a"}},
94+
"Valid | Less": {"less", MinimumNArgs(2), true, []string{"one"}},
95+
"Valid | LessInvalid": {"invalid", MinimumNArgs(2), true, []string{"a"}},
96+
})
13697
}
137-
138-
func TestMaximumNArgs(t *testing.T) {
139-
c := getCommand(MaximumNArgs(3), false)
140-
output, err := executeCommand(c, "a", "b")
141-
expectSuccess(output, err, t)
98+
func TestArgs_MaximumN(t *testing.T) {
99+
testArgs(t, map[string]argsTestcase{
100+
" | Arb": {"", MaximumNArgs(3), false, []string{"a", "b"}},
101+
"Valid | Valid": {"", MaximumNArgs(2), true, []string{"one", "three"}},
102+
"Valid | Invalid": {"invalid", MaximumNArgs(2), true, []string{"a", "b"}},
103+
" | More": {"more", MaximumNArgs(2), false, []string{"a", "b", "c"}},
104+
"Valid | More": {"more", MaximumNArgs(2), true, []string{"one", "three", "two"}},
105+
"Valid | MoreInvalid": {"invalid", MaximumNArgs(2), true, []string{"a", "b", "c"}},
106+
})
142107
}
143-
144-
func TestMaximumNArgsWithMoreArgs(t *testing.T) {
145-
c := getCommand(MaximumNArgs(2), false)
146-
_, err := executeCommand(c, "a", "b", "c")
147-
maximumNArgsWithMoreArgs(err, t)
108+
func TestArgs_Exact(t *testing.T) {
109+
testArgs(t, map[string]argsTestcase{
110+
" | Arb": {"", ExactArgs(3), false, []string{"a", "b", "c"}},
111+
"Valid | Valid": {"", ExactArgs(3), true, []string{"three", "one", "two"}},
112+
"Valid | Invalid": {"invalid", ExactArgs(3), true, []string{"three", "a", "two"}},
113+
" | InvalidCount": {"notexact", ExactArgs(2), false, []string{"a", "b", "c"}},
114+
"Valid | InvalidCount": {"notexact", ExactArgs(2), true, []string{"three", "one", "two"}},
115+
"Valid | InvalidCountInvalid": {"invalid", ExactArgs(2), true, []string{"three", "a", "two"}},
116+
})
148117
}
149-
150-
func TestExactArgs(t *testing.T) {
151-
c := getCommand(ExactArgs(3), false)
152-
output, err := executeCommand(c, "a", "b", "c")
153-
expectSuccess(output, err, t)
118+
func TestArgs_Range(t *testing.T) {
119+
testArgs(t, map[string]argsTestcase{
120+
" | Arb": {"", RangeArgs(2, 4), false, []string{"a", "b", "c"}},
121+
"Valid | Valid": {"", RangeArgs(2, 4), true, []string{"three", "one", "two"}},
122+
"Valid | Invalid": {"invalid", RangeArgs(2, 4), true, []string{"three", "a", "two"}},
123+
" | InvalidCount": {"notinrange", RangeArgs(2, 4), false, []string{"a"}},
124+
"Valid | InvalidCount": {"notinrange", RangeArgs(2, 4), true, []string{"two"}},
125+
"Valid | InvalidCountInvalid": {"invalid", RangeArgs(2, 4), true, []string{"a"}},
126+
})
154127
}
155-
156-
func TestExactArgsWithInvalidCount(t *testing.T) {
157-
c := getCommand(ExactArgs(2), false)
158-
_, err := executeCommand(c, "a", "b", "c")
159-
exactArgsWithInvalidCount(err, t)
128+
func TestArgs_DEPRECATED(t *testing.T) {
129+
testArgs(t, map[string]argsTestcase{
130+
"OnlyValid | Valid | Valid": {"", OnlyValidArgs, true, []string{"one", "two"}},
131+
"OnlyValid | Valid | Invalid": {"invalid", OnlyValidArgs, true, []string{"a"}},
132+
"ExactValid | Valid | Valid": {"", ExactValidArgs(3), true, []string{"two", "three", "one"}},
133+
"ExactValid | Valid | InvalidCount": {"notexact", ExactValidArgs(2), true, []string{"two", "three", "one"}},
134+
"ExactValid | Valid | Invalid": {"invalid", ExactValidArgs(2), true, []string{"two", "a"}},
135+
})
160136
}
161137

162-
func TestExactValidArgs(t *testing.T) {
163-
c := getCommand(ExactValidArgs(3), true)
164-
output, err := executeCommand(c, "three", "one", "two")
165-
expectSuccess(output, err, t)
166-
}
167-
168-
func TestExactValidArgsWithInvalidCount(t *testing.T) {
169-
c := getCommand(ExactValidArgs(2), false)
170-
_, err := executeCommand(c, "three", "one", "two")
171-
exactArgsWithInvalidCount(err, t)
172-
}
173-
174-
func TestExactValidArgsWithInvalidArgs(t *testing.T) {
175-
c := getCommand(ExactValidArgs(3), true)
176-
_, err := executeCommand(c, "three", "a", "two")
177-
validWithInvalidArgs(err, t)
178-
}
179-
180-
func TestRangeArgs(t *testing.T) {
181-
c := getCommand(RangeArgs(2, 4), false)
182-
output, err := executeCommand(c, "a", "b", "c")
183-
expectSuccess(output, err, t)
184-
}
185-
186-
func TestRangeArgsWithInvalidCount(t *testing.T) {
187-
c := getCommand(RangeArgs(2, 4), false)
188-
_, err := executeCommand(c, "a")
189-
rangeArgsWithInvalidCount(err, t)
190-
}
138+
// Takes(No)Args
191139

192140
func TestRootTakesNoArgs(t *testing.T) {
193141
rootCmd := &Command{Use: "root", Run: emptyRun}

‎bash_completions_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ func TestBashCompletions(t *testing.T) {
139139
timesCmd := &Command{
140140
Use: "times [# times] [string to echo]",
141141
SuggestFor: []string{"counts"},
142-
Args: OnlyValidArgs,
142+
Args: ArbitraryArgs,
143143
ValidArgs: []string{"one", "two", "three", "four"},
144144
Short: "Echo anything to the screen more times",
145145
Long: "a slightly useless command for testing.",

0 commit comments

Comments
 (0)