From 3553cc4a5ff3320b92fd41a3625481018ee324af Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Wed, 27 Sep 2023 13:58:27 +0100 Subject: [PATCH] fs: fix option types printing incorrectly for backend flags Before this change backend types were printing incorrectly as the name of the type, not what was defined by the Type() method. This was not working due to not calling the Type() method. However this needed to be defined on a non-pointer type due to the way the options are handled. --- fs/bwtimetable_test.go | 7 +++++-- fs/countsuffix.go | 2 +- fs/countsuffix_test.go | 7 +++++-- fs/cutoffmode.go | 2 +- fs/cutoffmode_test.go | 7 +++++-- fs/dump.go | 2 +- fs/dump_test.go | 7 +++++-- fs/log.go | 4 ++-- fs/log_test.go | 9 ++++++--- fs/parseduration_test.go | 7 +++++-- fs/parsetime_test.go | 7 +++++-- fs/registry.go | 13 ++++++++++++- fs/sizesuffix.go | 2 +- fs/sizesuffix_test.go | 16 ++++++++++++++-- fs/tristate_test.go | 7 +++++-- 15 files changed, 73 insertions(+), 26 deletions(-) diff --git a/fs/bwtimetable_test.go b/fs/bwtimetable_test.go index 2e3f677f6..e51400cab 100644 --- a/fs/bwtimetable_test.go +++ b/fs/bwtimetable_test.go @@ -9,8 +9,11 @@ import ( "github.com/stretchr/testify/require" ) -// Check it satisfies the interface -var _ flagger = (*BwTimetable)(nil) +// Check it satisfies the interfaces +var ( + _ flagger = (*BwTimetable)(nil) + _ flaggerNP = BwTimetable{} +) func TestBwTimetableSet(t *testing.T) { for _, test := range []struct { diff --git a/fs/countsuffix.go b/fs/countsuffix.go index cea3b6329..232a2ff43 100644 --- a/fs/countsuffix.go +++ b/fs/countsuffix.go @@ -154,7 +154,7 @@ func (x *CountSuffix) Set(s string) error { } // Type of the value -func (x *CountSuffix) Type() string { +func (x CountSuffix) Type() string { return "CountSuffix" } diff --git a/fs/countsuffix_test.go b/fs/countsuffix_test.go index e05921c5b..a8635572c 100644 --- a/fs/countsuffix_test.go +++ b/fs/countsuffix_test.go @@ -9,8 +9,11 @@ import ( "github.com/stretchr/testify/require" ) -// Check it satisfies the interface -var _ flagger = (*CountSuffix)(nil) +// Check it satisfies the interfaces +var ( + _ flagger = (*CountSuffix)(nil) + _ flaggerNP = CountSuffix(0) +) func TestCountSuffixString(t *testing.T) { for _, test := range []struct { diff --git a/fs/cutoffmode.go b/fs/cutoffmode.go index 9f6390663..231d1aa44 100644 --- a/fs/cutoffmode.go +++ b/fs/cutoffmode.go @@ -42,7 +42,7 @@ func (m *CutoffMode) Set(s string) error { } // Type of the value -func (m *CutoffMode) Type() string { +func (m CutoffMode) Type() string { return "string" } diff --git a/fs/cutoffmode_test.go b/fs/cutoffmode_test.go index 2b20b71c8..d8eaacd48 100644 --- a/fs/cutoffmode_test.go +++ b/fs/cutoffmode_test.go @@ -9,8 +9,11 @@ import ( "github.com/stretchr/testify/require" ) -// Check it satisfies the interface -var _ flagger = (*CutoffMode)(nil) +// Check it satisfies the interfaces +var ( + _ flagger = (*CutoffMode)(nil) + _ flaggerNP = CutoffMode(0) +) func TestCutoffModeString(t *testing.T) { for _, test := range []struct { diff --git a/fs/dump.go b/fs/dump.go index a15671253..40d7d80c6 100644 --- a/fs/dump.go +++ b/fs/dump.go @@ -86,7 +86,7 @@ func (f *DumpFlags) Set(s string) error { } // Type of the value -func (f *DumpFlags) Type() string { +func (f DumpFlags) Type() string { return "DumpFlags" } diff --git a/fs/dump_test.go b/fs/dump_test.go index ede6125e8..d374672f1 100644 --- a/fs/dump_test.go +++ b/fs/dump_test.go @@ -8,8 +8,11 @@ import ( "github.com/stretchr/testify/assert" ) -// Check it satisfies the interface -var _ flagger = (*DumpFlags)(nil) +// Check it satisfies the interfaces +var ( + _ flagger = (*DumpFlags)(nil) + _ flaggerNP = DumpFlags(0) +) func TestDumpFlagsString(t *testing.T) { assert.Equal(t, "", DumpFlags(0).String()) diff --git a/fs/log.go b/fs/log.go index 3f6fe21a7..c85db3779 100644 --- a/fs/log.go +++ b/fs/log.go @@ -65,8 +65,8 @@ func (l *LogLevel) Set(s string) error { } // Type of the value -func (l *LogLevel) Type() string { - return "string" +func (l LogLevel) Type() string { + return "LogLevel" } // UnmarshalJSON makes sure the value can be parsed as a string or integer in JSON diff --git a/fs/log_test.go b/fs/log_test.go index dcccb8043..e6f012785 100644 --- a/fs/log_test.go +++ b/fs/log_test.go @@ -10,9 +10,12 @@ import ( "github.com/stretchr/testify/require" ) -// Check it satisfies the interface -var _ flagger = (*LogLevel)(nil) -var _ fmt.Stringer = LogValueItem{} +// Check it satisfies the interfac( +var ( + _ flagger = (*LogLevel)(nil) + _ flaggerNP = LogLevel(0) + _ fmt.Stringer = LogValueItem{} +) type withString struct{} diff --git a/fs/parseduration_test.go b/fs/parseduration_test.go index 8361fa3bd..b95e7da8d 100644 --- a/fs/parseduration_test.go +++ b/fs/parseduration_test.go @@ -11,8 +11,11 @@ import ( "github.com/stretchr/testify/require" ) -// Check it satisfies the interface -var _ flagger = (*Duration)(nil) +// Check it satisfies the interfaces +var ( + _ flagger = (*Duration)(nil) + _ flaggerNP = Duration(0) +) func TestParseDuration(t *testing.T) { now := time.Date(2020, 9, 5, 8, 15, 5, 250, time.UTC) diff --git a/fs/parsetime_test.go b/fs/parsetime_test.go index 796217b5b..611b0c79b 100644 --- a/fs/parsetime_test.go +++ b/fs/parsetime_test.go @@ -10,8 +10,11 @@ import ( "github.com/stretchr/testify/require" ) -// Check it satisfies the interface -var _ flagger = (*Time)(nil) +// Check it satisfies the interfaces +var ( + _ flagger = (*Time)(nil) + _ flaggerNP = Time{} +) func TestParseTime(t *testing.T) { now := time.Date(2020, 9, 5, 8, 15, 5, 250, time.UTC) diff --git a/fs/registry.go b/fs/registry.go index 622cf8e87..8bd8cc1a8 100644 --- a/fs/registry.go +++ b/fs/registry.go @@ -207,9 +207,20 @@ func (o *Option) Set(s string) (err error) { return nil } +type typer interface { + Type() string +} + // Type of the value func (o *Option) Type() string { - return reflect.TypeOf(o.GetValue()).Name() + v := o.GetValue() + + // Try to call Type method on non-pointer + if do, ok := v.(typer); ok { + return do.Type() + } + + return reflect.TypeOf(v).Name() } // FlagName for the option diff --git a/fs/sizesuffix.go b/fs/sizesuffix.go index 04056b707..0a2a5661c 100644 --- a/fs/sizesuffix.go +++ b/fs/sizesuffix.go @@ -192,7 +192,7 @@ func (x *SizeSuffix) Set(s string) error { } // Type of the value -func (x *SizeSuffix) Type() string { +func (x SizeSuffix) Type() string { return "SizeSuffix" } diff --git a/fs/sizesuffix_test.go b/fs/sizesuffix_test.go index cb58b807c..fd87811c8 100644 --- a/fs/sizesuffix_test.go +++ b/fs/sizesuffix_test.go @@ -17,8 +17,20 @@ type flagger interface { json.Unmarshaler } -// Check it satisfies the interface -var _ flagger = (*SizeSuffix)(nil) +// Interface which non-pointer flags must satisfy +// +// These are from pflag.Value and need to be non-pointer due the the +// way the backend flags are inserted into the flags. +type flaggerNP interface { + String() string + Type() string +} + +// Check it satisfies the interfaces +var ( + _ flagger = (*SizeSuffix)(nil) + _ flaggerNP = SizeSuffix(0) +) func TestSizeSuffixString(t *testing.T) { for _, test := range []struct { diff --git a/fs/tristate_test.go b/fs/tristate_test.go index fe0822d4b..7e9f11d9b 100644 --- a/fs/tristate_test.go +++ b/fs/tristate_test.go @@ -9,8 +9,11 @@ import ( "github.com/stretchr/testify/require" ) -// Check it satisfies the interface -var _ flagger = (*Tristate)(nil) +// Check it satisfies the interfaces +var ( + _ flagger = (*Tristate)(nil) + _ flaggerNP = Tristate{} +) func TestTristateString(t *testing.T) { for _, test := range []struct {