From b60e7cbe4e1564384aeb96d79a51f6103f70745d Mon Sep 17 00:00:00 2001 From: sergioperezcheco Date: Sat, 3 Oct 2026 12:54:29 +0800 Subject: [PATCH 1/3] fix: propagate validator errors from empty bool sources Co-Authored-By: OpenAI GPT-6.1-Sol Signed-off-by: sergioperezcheco --- flag_empty_bool_validator_test.go | 80 +++++++++++++++++++++++++++++++ flag_impl.go | 17 ++++--- 2 files changed, 88 insertions(+), 9 deletions(-) create mode 100644 flag_empty_bool_validator_test.go diff --git a/flag_empty_bool_validator_test.go b/flag_empty_bool_validator_test.go new file mode 100644 index 0000000000..640fd3c07b --- /dev/null +++ b/flag_empty_bool_validator_test.go @@ -0,0 +1,80 @@ +package cli_test + +import ( + "context" + "errors" + "io" + "os" + "path/filepath" + "strings" + "testing" + + cli "github.com/urfave/cli/v3" +) + +func TestBoolFlagEmptySourceRunsValidator(t *testing.T) { + for _, sourceKind := range []string{"env", "file"} { + for _, sourceValue := range []string{"", "false", "true"} { + for _, override := range []bool{false, true} { + t.Run(sourceKind+"/"+sourceValue+"/override="+map[bool]string{false: "no", true: "yes"}[override], func(t *testing.T) { + var sources cli.ValueSourceChain + if sourceKind == "env" { + t.Setenv("CLI_BOOL_VALIDATOR_TEST", sourceValue) + sources = cli.EnvVars("CLI_BOOL_VALIDATOR_TEST") + } else { + path := filepath.Join(t.TempDir(), "enabled") + if err := os.WriteFile(path, []byte(sourceValue), 0o600); err != nil { + t.Fatal(err) + } + sources = cli.Files(path) + } + calls := 0 + actionRan := false + flagActionRan := false + destination := true + rejected := errors.New("enabled must be true") + flag := &cli.BoolFlag{ + Name: "enabled", Value: true, Destination: &destination, Sources: sources, + Validator: func(value bool) error { + calls++ + if !value { + return rejected + } + return nil + }, + Action: func(context.Context, *cli.Command, bool) error { flagActionRan = true; return nil }, + } + cmd := &cli.Command{ + Name: "test", Writer: io.Discard, ErrWriter: io.Discard, + Flags: []cli.Flag{flag}, + Action: func(context.Context, *cli.Command) error { actionRan = true; return nil }, + } + args := []string{"test"} + if override { + args = append(args, "--enabled=true") + } + err := cmd.Run(context.Background(), args) + wantError := sourceValue != "true" && !override + if wantError { + if err == nil || !strings.Contains(err.Error(), rejected.Error()) { + t.Errorf("Run() error = %v, want validation error", err) + } + if actionRan || flagActionRan { + t.Errorf("actions ran after rejected source: command=%v flag=%v", actionRan, flagActionRan) + } + } else { + if err != nil { + t.Fatal(err) + } + if !actionRan || !flagActionRan || !destination || !flag.IsSet() { + t.Errorf("accepted source: action=%v flagAction=%v destination=%v IsSet=%v", actionRan, flagActionRan, destination, flag.IsSet()) + } + } + if calls != 1 { + t.Errorf("validator calls = %d, want 1", calls) + } + }) + } + } + } +} diff --git a/flag_impl.go b/flag_impl.go index 410f7f0875..56941e77e4 100644 --- a/flag_impl.go +++ b/flag_impl.go @@ -155,15 +155,14 @@ func (f *FlagBase[T, C, V]) PostParse() error { continue } - if val != "" || kind == reflect.String { - if err := f.Set(f.Name, val); err != nil { - return fmt.Errorf( - "could not parse %[1]q as %[2]T value from %[3]s for flag %[4]s: %[5]s", - val, f.Value, source, f.Name, err, - ) - } - } else { - _ = f.Set(f.Name, "false") + if val == "" && kind == reflect.Bool { + val = "false" + } + if err := f.Set(f.Name, val); err != nil { + return fmt.Errorf( + "could not parse %[1]q as %[2]T value from %[3]s for flag %[4]s: %[5]s", + val, f.Value, source, f.Name, err, + ) } f.hasBeenSet = true From 9f822480a35f57a61d93f1e38ae6d7345f15e7dd Mon Sep 17 00:00:00 2001 From: sergioperezcheco Date: Sat, 3 Oct 2026 19:06:54 +0800 Subject: [PATCH 2/3] test: list bool source validator variants explicitly Co-Authored-By: OpenAI GPT-6.1-Sol Signed-off-by: sergioperezcheco --- flag_empty_bool_validator_test.go | 137 +++++++++++++++++------------- 1 file changed, 76 insertions(+), 61 deletions(-) diff --git a/flag_empty_bool_validator_test.go b/flag_empty_bool_validator_test.go index 640fd3c07b..9b7247979b 100644 --- a/flag_empty_bool_validator_test.go +++ b/flag_empty_bool_validator_test.go @@ -13,68 +13,83 @@ import ( ) func TestBoolFlagEmptySourceRunsValidator(t *testing.T) { - for _, sourceKind := range []string{"env", "file"} { - for _, sourceValue := range []string{"", "false", "true"} { - for _, override := range []bool{false, true} { - t.Run(sourceKind+"/"+sourceValue+"/override="+map[bool]string{false: "no", true: "yes"}[override], func(t *testing.T) { - var sources cli.ValueSourceChain - if sourceKind == "env" { - t.Setenv("CLI_BOOL_VALIDATOR_TEST", sourceValue) - sources = cli.EnvVars("CLI_BOOL_VALIDATOR_TEST") - } else { - path := filepath.Join(t.TempDir(), "enabled") - if err := os.WriteFile(path, []byte(sourceValue), 0o600); err != nil { - t.Fatal(err) - } - sources = cli.Files(path) - } - calls := 0 - actionRan := false - flagActionRan := false - destination := true - rejected := errors.New("enabled must be true") - flag := &cli.BoolFlag{ - Name: "enabled", Value: true, Destination: &destination, Sources: sources, - Validator: func(value bool) error { - calls++ - if !value { - return rejected - } - return nil - }, - Action: func(context.Context, *cli.Command, bool) error { flagActionRan = true; return nil }, - } - cmd := &cli.Command{ - Name: "test", Writer: io.Discard, ErrWriter: io.Discard, - Flags: []cli.Flag{flag}, - Action: func(context.Context, *cli.Command) error { actionRan = true; return nil }, - } - args := []string{"test"} - if override { - args = append(args, "--enabled=true") - } - err := cmd.Run(context.Background(), args) - wantError := sourceValue != "true" && !override - if wantError { - if err == nil || !strings.Contains(err.Error(), rejected.Error()) { - t.Errorf("Run() error = %v, want validation error", err) - } - if actionRan || flagActionRan { - t.Errorf("actions ran after rejected source: command=%v flag=%v", actionRan, flagActionRan) - } - } else { - if err != nil { - t.Fatal(err) - } - if !actionRan || !flagActionRan || !destination || !flag.IsSet() { - t.Errorf("accepted source: action=%v flagAction=%v destination=%v IsSet=%v", actionRan, flagActionRan, destination, flag.IsSet()) - } - } - if calls != 1 { - t.Errorf("validator calls = %d, want 1", calls) + tests := []struct { + name string + sourceKind string + sourceValue string + override bool + wantError bool + }{ + {name: "env/empty", sourceKind: "env", sourceValue: "", wantError: true}, + {name: "env/empty/override", sourceKind: "env", sourceValue: "", override: true}, + {name: "env/false", sourceKind: "env", sourceValue: "false", wantError: true}, + {name: "env/false/override", sourceKind: "env", sourceValue: "false", override: true}, + {name: "env/true", sourceKind: "env", sourceValue: "true"}, + {name: "env/true/override", sourceKind: "env", sourceValue: "true", override: true}, + {name: "file/empty", sourceKind: "file", sourceValue: "", wantError: true}, + {name: "file/empty/override", sourceKind: "file", sourceValue: "", override: true}, + {name: "file/false", sourceKind: "file", sourceValue: "false", wantError: true}, + {name: "file/false/override", sourceKind: "file", sourceValue: "false", override: true}, + {name: "file/true", sourceKind: "file", sourceValue: "true"}, + {name: "file/true/override", sourceKind: "file", sourceValue: "true", override: true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var sources cli.ValueSourceChain + if tt.sourceKind == "env" { + t.Setenv("CLI_BOOL_VALIDATOR_TEST", tt.sourceValue) + sources = cli.EnvVars("CLI_BOOL_VALIDATOR_TEST") + } else { + path := filepath.Join(t.TempDir(), "enabled") + if err := os.WriteFile(path, []byte(tt.sourceValue), 0o600); err != nil { + t.Fatal(err) + } + sources = cli.Files(path) + } + calls := 0 + actionRan := false + flagActionRan := false + destination := true + rejected := errors.New("enabled must be true") + flag := &cli.BoolFlag{ + Name: "enabled", Value: true, Destination: &destination, Sources: sources, + Validator: func(value bool) error { + calls++ + if !value { + return rejected } - }) + return nil + }, + Action: func(context.Context, *cli.Command, bool) error { flagActionRan = true; return nil }, + } + cmd := &cli.Command{ + Name: "test", Writer: io.Discard, ErrWriter: io.Discard, + Flags: []cli.Flag{flag}, + Action: func(context.Context, *cli.Command) error { actionRan = true; return nil }, + } + args := []string{"test"} + if tt.override { + args = append(args, "--enabled=true") + } + err := cmd.Run(context.Background(), args) + if tt.wantError { + if err == nil || !strings.Contains(err.Error(), rejected.Error()) { + t.Errorf("Run() error = %v, want validation error", err) + } + if actionRan || flagActionRan { + t.Errorf("actions ran after rejected source: command=%v flag=%v", actionRan, flagActionRan) + } + } else { + if err != nil { + t.Fatal(err) + } + if !actionRan || !flagActionRan || !destination || !flag.IsSet() { + t.Errorf("accepted source: action=%v flagAction=%v destination=%v IsSet=%v", actionRan, flagActionRan, destination, flag.IsSet()) + } + } + if calls != 1 { + t.Errorf("validator calls = %d, want 1", calls) } - } + }) } } From 271990f7332f4473aeea0ebe2566ab2b4e7cda5c Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Wed, 7 Oct 2026 23:41:47 +0800 Subject: [PATCH 3/3] test: cover empty bool source validation in TestFlagsFromEmptyEnv Only the empty env and empty file cases of TestBoolFlagEmptySourceRunsValidator fail without the fix. The other ten already pass on main, and a file source goes through the same PostParse loop as an env var. Replace the file with one case in TestFlagsFromEmptyEnv, next to the existing "an empty bool still reads as false" case. Assisted-by: claude:claude-opus-5-5 Signed-off-by: Eng Zer Jun --- flag_empty_bool_validator_test.go | 95 ------------------------------- flag_test.go | 11 ++++ 2 files changed, 11 insertions(+), 95 deletions(-) delete mode 100644 flag_empty_bool_validator_test.go diff --git a/flag_empty_bool_validator_test.go b/flag_empty_bool_validator_test.go deleted file mode 100644 index 9b7247979b..0000000000 --- a/flag_empty_bool_validator_test.go +++ /dev/null @@ -1,95 +0,0 @@ -package cli_test - -import ( - "context" - "errors" - "io" - "os" - "path/filepath" - "strings" - "testing" - - cli "github.com/urfave/cli/v3" -) - -func TestBoolFlagEmptySourceRunsValidator(t *testing.T) { - tests := []struct { - name string - sourceKind string - sourceValue string - override bool - wantError bool - }{ - {name: "env/empty", sourceKind: "env", sourceValue: "", wantError: true}, - {name: "env/empty/override", sourceKind: "env", sourceValue: "", override: true}, - {name: "env/false", sourceKind: "env", sourceValue: "false", wantError: true}, - {name: "env/false/override", sourceKind: "env", sourceValue: "false", override: true}, - {name: "env/true", sourceKind: "env", sourceValue: "true"}, - {name: "env/true/override", sourceKind: "env", sourceValue: "true", override: true}, - {name: "file/empty", sourceKind: "file", sourceValue: "", wantError: true}, - {name: "file/empty/override", sourceKind: "file", sourceValue: "", override: true}, - {name: "file/false", sourceKind: "file", sourceValue: "false", wantError: true}, - {name: "file/false/override", sourceKind: "file", sourceValue: "false", override: true}, - {name: "file/true", sourceKind: "file", sourceValue: "true"}, - {name: "file/true/override", sourceKind: "file", sourceValue: "true", override: true}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - var sources cli.ValueSourceChain - if tt.sourceKind == "env" { - t.Setenv("CLI_BOOL_VALIDATOR_TEST", tt.sourceValue) - sources = cli.EnvVars("CLI_BOOL_VALIDATOR_TEST") - } else { - path := filepath.Join(t.TempDir(), "enabled") - if err := os.WriteFile(path, []byte(tt.sourceValue), 0o600); err != nil { - t.Fatal(err) - } - sources = cli.Files(path) - } - calls := 0 - actionRan := false - flagActionRan := false - destination := true - rejected := errors.New("enabled must be true") - flag := &cli.BoolFlag{ - Name: "enabled", Value: true, Destination: &destination, Sources: sources, - Validator: func(value bool) error { - calls++ - if !value { - return rejected - } - return nil - }, - Action: func(context.Context, *cli.Command, bool) error { flagActionRan = true; return nil }, - } - cmd := &cli.Command{ - Name: "test", Writer: io.Discard, ErrWriter: io.Discard, - Flags: []cli.Flag{flag}, - Action: func(context.Context, *cli.Command) error { actionRan = true; return nil }, - } - args := []string{"test"} - if tt.override { - args = append(args, "--enabled=true") - } - err := cmd.Run(context.Background(), args) - if tt.wantError { - if err == nil || !strings.Contains(err.Error(), rejected.Error()) { - t.Errorf("Run() error = %v, want validation error", err) - } - if actionRan || flagActionRan { - t.Errorf("actions ran after rejected source: command=%v flag=%v", actionRan, flagActionRan) - } - } else { - if err != nil { - t.Fatal(err) - } - if !actionRan || !flagActionRan || !destination || !flag.IsSet() { - t.Errorf("accepted source: action=%v flagAction=%v destination=%v IsSet=%v", actionRan, flagActionRan, destination, flag.IsSet()) - } - } - if calls != 1 { - t.Errorf("validator calls = %d, want 1", calls) - } - }) - } -} diff --git a/flag_test.go b/flag_test.go index 9dca7de7b8..a699465d3c 100644 --- a/flag_test.go +++ b/flag_test.go @@ -734,6 +734,17 @@ func TestFlagsFromEmptyEnv(t *testing.T) { wantValue: false, wantIsSet: true, }, + { + name: "an empty bool still runs the validator", + env: map[string]string{"DEBUG": ""}, + fl: &BoolFlag{Name: "debug", Value: true, Sources: EnvVars("DEBUG"), Validator: func(b bool) error { + if !b { + return errors.New("debug must be true") + } + return nil + }}, + errContains: "debug must be true", + }, } for _, tc := range testCases {