From 7202ba12bcb2bfb01f202019ca9f77e8271f45c8 Mon Sep 17 00:00:00 2001 From: Arik Kfir Date: Fri, 14 Jun 2024 19:39:37 +0300 Subject: [PATCH] bug(flags): bool flags still fail validation despite default value This change completes the previous fix for default values of flags, by ensuring that bool flags also don't ignore their default value. This fixes an issue where bool flags did not get their default value applied, and thus were considered missing, despite having a valid default value. --- flag_set.go | 15 +++++++-------- flag_set_test.go | 20 ++++++++++++++++++++ 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/flag_set.go b/flag_set.go index b1dc393..f777baa 100644 --- a/flag_set.go +++ b/flag_set.go @@ -366,19 +366,18 @@ func (fs *flagSet) apply(envVars map[string]string, args []string) error { // By definition, for the same name - all flags have the same "HasValue" value, so it should be safe to just // take it from the first one if mfd.HasValue { - - // Set the field's default value so it's marked as "applied" (and thus the "required" validation will ignore it) - if mfd.DefaultValue != "" { - if err := mfd.setValue(mfd.DefaultValue); err != nil { - return fmt.Errorf("failed applying default value for flag '%s': %w", mfd.Name, err) - } - } stdFs.Func(mfd.Name, "", func(v string) error { return mfd.setValue(v) }) - } else { stdFs.BoolFunc(mfd.Name, "", func(string) error { return mfd.setValue("true") }) } + // Set the field's default value so it's marked as "applied" (and thus the "required" validation will ignore it) + if mfd.DefaultValue != "" { + if err := mfd.setValue(mfd.DefaultValue); err != nil { + return fmt.Errorf("failed applying default value for flag '%s': %w", mfd.Name, err) + } + } + // Set the value to the flag's corresponding environment variable, if one was given // Important this is done here, so it overrides the default value set earlier if v, found := envVars[*mfd.EnvVarName]; found { diff --git a/flag_set_test.go b/flag_set_test.go index b69c41f..63153c5 100644 --- a/flag_set_test.go +++ b/flag_set_test.go @@ -978,6 +978,26 @@ func TestFlagSetApply(t *testing.T) { args: []string{"--my-field1=VVV1"}, expectedError: `^required flag is missing: --my-field2$`, }, + "bool flag default value is considered": { + config: &struct { + F1 bool `name:"my-field1" required:"true"` + }{F1: false}, + envVars: map[string]string{}, + args: []string{}, + expectedConfig: &struct { + F1 bool `name:"my-field1" required:"true"` + }{F1: false}, + }, + "bool flag default value overridden by arg": { + config: &struct { + F1 bool `name:"my-field1" required:"true"` + }{F1: false}, + envVars: map[string]string{}, + args: []string{"--my-field1"}, + expectedConfig: &struct { + F1 bool `name:"my-field1" required:"true"` + }{F1: true}, + }, } for name, tc := range testCases { tc := tc