From 697164e89fd275ffdd56363bc2934365c3c3b0fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tam=C3=A1s=20Gul=C3=A1csi?= Date: Wed, 27 Nov 2024 09:00:17 +0100 Subject: [PATCH 1/5] Fix ErrDuplicateFlags when no env var parsing is requested --- flag_set_test.go | 13 +++++++++++++ parse.go | 27 +++++++++++++++------------ parse_test.go | 28 ++++++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 12 deletions(-) diff --git a/flag_set_test.go b/flag_set_test.go index 928c6d7..e96c5f3 100644 --- a/flag_set_test.go +++ b/flag_set_test.go @@ -295,6 +295,19 @@ func TestFlagSet_invalid(t *testing.T) { _ = fs.Bool('a', "apple", "this should panic") }) + t.Run("same fold short name", func(t *testing.T) { + defer func() { + if x := recover(); x == nil { + t.Logf("have not expected panic (%v)", x) + } else { + t.Errorf("want peace, have panic") + } + }() + fs := ff.NewFlagSet(t.Name()) + _ = fs.Bool('a', "alpha", "this should be OK") + _ = fs.Bool('A', "apple", "this should be OK") + }) + t.Run("duplicate long name", func(t *testing.T) { defer func() { if x := recover(); x == nil { diff --git a/parse.go b/parse.go index bdfb2c7..0b8a03c 100644 --- a/parse.go +++ b/parse.go @@ -38,20 +38,23 @@ func parse(fs Flags, args []string, options ...Option) error { option(&pc) } - // Index valid flags by env var key, to support .env config files (below). - env2flag := map[string]Flag{} - { - if err := fs.WalkFlags(func(f Flag) error { - for _, name := range getNameStrings(f) { - key := getEnvVarKey(name, pc.envVarPrefix) - if existing, ok := env2flag[key]; ok { - return fmt.Errorf("%s: %w (%s)", getNameString(f), ErrDuplicateFlag, getNameString(existing)) + var env2flag map[string]Flag + if pc.envVarEnabled { + // Index valid flags by env var key, to support .env config files (below). + env2flag = map[string]Flag{} + { + if err := fs.WalkFlags(func(f Flag) error { + for _, name := range getNameStrings(f) { + key := getEnvVarKey(name, pc.envVarPrefix) + if existing, ok := env2flag[key]; ok { + return fmt.Errorf("%s: %w (%s)", getNameString(f), ErrDuplicateFlag, getNameString(existing)) + } + env2flag[key] = f } - env2flag[key] = f + return nil + }); err != nil { + return err } - return nil - }); err != nil { - return err } } diff --git a/parse_test.go b/parse_test.go index 62f9b3e..578276d 100644 --- a/parse_test.go +++ b/parse_test.go @@ -2,6 +2,7 @@ package ff_test import ( "embed" + "errors" "flag" "os" "path/filepath" @@ -428,3 +429,30 @@ func TestParse_stdfs(t *testing.T) { t.Errorf("foo: want %q, have %q", want, have) } } + +func TestParse_shortSameCase(t *testing.T) { + t.Parallel() + + newFS := func() *ff.FlagSet { + fs := ff.NewFlagSet(t.Name()) + _ = fs.String('a', "apple", "", "foo string") + _ = fs.String('A', "apricot", "", "FOO string") + return fs + } + args := []string{"-a", "-A"} + + t.Run("no env", func(t *testing.T) { + t.Parallel() + if err := ff.Parse(newFS(), args); err != nil { + t.Error(err) + } + }) + t.Run("WithEnv", func(t *testing.T) { + t.Parallel() + if err := ff.Parse(newFS(), args, ff.WithEnvVars()); err == nil { + t.Error("wanted ErrDuplicateFlage, got nil") + } else if !errors.Is(err, ff.ErrDuplicateFlag) { + t.Errorf("wanted ErrDuplicateFlage, got %+v", err) + } + }) +} From d6d7939bf9c12f7b438c3e60c92e1c44f674a734 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tam=C3=A1s=20Gul=C3=A1csi?= Date: Wed, 27 Nov 2024 17:12:46 +0000 Subject: [PATCH 2/5] Update parse_test.go Co-authored-by: ccoVeille <3875889+ccoVeille@users.noreply.github.com> --- parse_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parse_test.go b/parse_test.go index 578276d..07b9090 100644 --- a/parse_test.go +++ b/parse_test.go @@ -452,7 +452,7 @@ func TestParse_shortSameCase(t *testing.T) { if err := ff.Parse(newFS(), args, ff.WithEnvVars()); err == nil { t.Error("wanted ErrDuplicateFlage, got nil") } else if !errors.Is(err, ff.ErrDuplicateFlag) { - t.Errorf("wanted ErrDuplicateFlage, got %+v", err) + t.Errorf("wanted ErrDuplicateFlag, got %+v", err) } }) } From d20f2827d0ff9a26c18351d5cac57dc5ebf3ef92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tam=C3=A1s=20Gul=C3=A1csi?= Date: Wed, 27 Nov 2024 18:15:18 +0100 Subject: [PATCH 3/5] TestParse_shortSameCase: fix typo, accept simplification suggestion --- parse_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/parse_test.go b/parse_test.go index 578276d..ff4a29a 100644 --- a/parse_test.go +++ b/parse_test.go @@ -449,10 +449,10 @@ func TestParse_shortSameCase(t *testing.T) { }) t.Run("WithEnv", func(t *testing.T) { t.Parallel() - if err := ff.Parse(newFS(), args, ff.WithEnvVars()); err == nil { - t.Error("wanted ErrDuplicateFlage, got nil") - } else if !errors.Is(err, ff.ErrDuplicateFlag) { - t.Errorf("wanted ErrDuplicateFlage, got %+v", err) + if err := ff.Parse( + newFS(), args, ff.WithEnvVars(), + ); err == nil || !errors.Is(err, ff.ErrDuplicateFlag) { + t.Errorf("wanted ErrDuplicateFlag, got %+v", err) } }) } From 41ae7d9d47bed07442cbb7d17da8cfbe76ed7d70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tam=C3=A1s=20Gul=C3=A1csi?= Date: Wed, 27 Nov 2024 19:50:44 +0000 Subject: [PATCH 4/5] Update parse_test.go It is enough. Co-authored-by: ccoVeille <3875889+ccoVeille@users.noreply.github.com> --- parse_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parse_test.go b/parse_test.go index ff4a29a..70750ba 100644 --- a/parse_test.go +++ b/parse_test.go @@ -451,7 +451,7 @@ func TestParse_shortSameCase(t *testing.T) { t.Parallel() if err := ff.Parse( newFS(), args, ff.WithEnvVars(), - ); err == nil || !errors.Is(err, ff.ErrDuplicateFlag) { + ); !errors.Is(err, ff.ErrDuplicateFlag) { t.Errorf("wanted ErrDuplicateFlag, got %+v", err) } }) From 3dc1dba24f3f93faa9729cd9f1b881fa62c46e1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gul=C3=A1csi=20Tam=C3=A1s?= Date: Fri, 31 Jan 2025 07:37:17 +0100 Subject: [PATCH 5/5] fftest: *.env implies WithEnvVars --- fftest/test_case.go | 1 + 1 file changed, 1 insertion(+) diff --git a/fftest/test_case.go b/fftest/test_case.go index 6a7bde9..0760579 100644 --- a/fftest/test_case.go +++ b/fftest/test_case.go @@ -59,6 +59,7 @@ func (tc *ParseTest) Run(t *testing.T) { parseFunc = fftoml.Parse case ".env": parseFunc = ffenv.Parse + opts = append(opts, ff.WithEnvVars()) default: parseFunc = ff.PlainParser }