From f0c4ccd6cd7527b198aecc6d50b3ffd6d06e36d1 Mon Sep 17 00:00:00 2001 From: Oleksandr Redko Date: Thu, 30 Nov 2023 17:45:20 +0200 Subject: [PATCH] fix: gocritic lint issues --- .golangci.yaml | 12 ++++++- internal/encoding/dotenv/map_utils.go | 2 +- internal/encoding/ini/codec.go | 6 ++-- internal/encoding/ini/map_utils.go | 2 +- internal/encoding/javaproperties/map_utils.go | 2 +- logger.go | 1 + overrides_test.go | 6 ++-- remote/remote.go | 2 +- viper.go | 17 ++++++---- viper_test.go | 33 ++++++++++--------- 10 files changed, 49 insertions(+), 34 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index fa28eb3..c961c47 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -7,6 +7,16 @@ linters-settings: - standard - default - prefix(github.com/spf13/viper) + gocritic: + # Enable multiple checks by tags. See "Tags" section in https://github.com/go-critic/go-critic#usage. + enabled-tags: + - diagnostic + - experimental + - opinionated + - performance + - style + disabled-checks: + - importShadow golint: min-confidence: 0 goimports: @@ -22,6 +32,7 @@ linters: - exhaustive - exportloopref - gci + - gocritic - godot - gofmt - gofumpt @@ -63,7 +74,6 @@ linters: # - gochecknoinits # - gocognit # - goconst - # - gocritic # - gocyclo # - gosec # - gosimple diff --git a/internal/encoding/dotenv/map_utils.go b/internal/encoding/dotenv/map_utils.go index 4901d7c..8bfe0a9 100644 --- a/internal/encoding/dotenv/map_utils.go +++ b/internal/encoding/dotenv/map_utils.go @@ -9,7 +9,7 @@ import ( // flattenAndMergeMap recursively flattens the given map into a new map // Code is based on the function with the same name in the main package. // TODO: move it to a common place. -func flattenAndMergeMap(shadow map[string]any, m map[string]any, prefix string, delimiter string) map[string]any { +func flattenAndMergeMap(shadow, m map[string]any, prefix, delimiter string) map[string]any { if shadow != nil && prefix != "" && shadow[prefix] != nil { // prefix is shadowed => nothing more to flatten return shadow diff --git a/internal/encoding/ini/codec.go b/internal/encoding/ini/codec.go index d91cf59..597f744 100644 --- a/internal/encoding/ini/codec.go +++ b/internal/encoding/ini/codec.go @@ -19,7 +19,7 @@ type Codec struct { LoadOptions LoadOptions } -func (c Codec) Encode(v map[string]any) ([]byte, error) { +func (c *Codec) Encode(v map[string]any) ([]byte, error) { cfg := ini.Empty() ini.PrettyFormat = false @@ -62,7 +62,7 @@ func (c Codec) Encode(v map[string]any) ([]byte, error) { return buf.Bytes(), nil } -func (c Codec) Decode(b []byte, v map[string]any) error { +func (c *Codec) Decode(b []byte, v map[string]any) error { cfg := ini.Empty(c.LoadOptions) err := cfg.Append(b) @@ -90,7 +90,7 @@ func (c Codec) Decode(b []byte, v map[string]any) error { return nil } -func (c Codec) keyDelimiter() string { +func (c *Codec) keyDelimiter() string { if c.KeyDelimiter == "" { return "." } diff --git a/internal/encoding/ini/map_utils.go b/internal/encoding/ini/map_utils.go index 06e703f..490ab59 100644 --- a/internal/encoding/ini/map_utils.go +++ b/internal/encoding/ini/map_utils.go @@ -42,7 +42,7 @@ func deepSearch(m map[string]any, path []string) map[string]any { // flattenAndMergeMap recursively flattens the given map into a new map // Code is based on the function with the same name in the main package. // TODO: move it to a common place. -func flattenAndMergeMap(shadow map[string]any, m map[string]any, prefix string, delimiter string) map[string]any { +func flattenAndMergeMap(shadow, m map[string]any, prefix, delimiter string) map[string]any { if shadow != nil && prefix != "" && shadow[prefix] != nil { // prefix is shadowed => nothing more to flatten return shadow diff --git a/internal/encoding/javaproperties/map_utils.go b/internal/encoding/javaproperties/map_utils.go index 7c7e78a..6e1aff2 100644 --- a/internal/encoding/javaproperties/map_utils.go +++ b/internal/encoding/javaproperties/map_utils.go @@ -42,7 +42,7 @@ func deepSearch(m map[string]any, path []string) map[string]any { // flattenAndMergeMap recursively flattens the given map into a new map // Code is based on the function with the same name in the main package. // TODO: move it to a common place. -func flattenAndMergeMap(shadow map[string]any, m map[string]any, prefix string, delimiter string) map[string]any { +func flattenAndMergeMap(shadow, m map[string]any, prefix, delimiter string) map[string]any { if shadow != nil && prefix != "" && shadow[prefix] != nil { // prefix is shadowed => nothing more to flatten return shadow diff --git a/logger.go b/logger.go index 8938053..c25c7a0 100644 --- a/logger.go +++ b/logger.go @@ -55,6 +55,7 @@ func (n *discardHandler) Enabled(_ context.Context, _ slog.Level) bool { return false } +//nolint:gocritic // hugeParam: _ is heavy (288 bytes); consider passing it by pointer func (n *discardHandler) Handle(_ context.Context, _ slog.Record) error { return nil } diff --git a/overrides_test.go b/overrides_test.go index a8f4cc1..42da3ba 100644 --- a/overrides_test.go +++ b/overrides_test.go @@ -97,7 +97,7 @@ func overrideFromLayer(l layer, assert *assert.Assertions, firstPath string, fir v := New() firstKeys := strings.Split(firstPath, v.keyDelim) if assert == nil || - len(firstKeys) == 0 || len(firstKeys[0]) == 0 { + len(firstKeys) == 0 || firstKeys[0] == "" { return v } @@ -115,7 +115,7 @@ func overrideFromLayer(l layer, assert *assert.Assertions, firstPath string, fir // Override and check new value secondKeys := strings.Split(secondPath, v.keyDelim) - if len(secondKeys) == 0 || len(secondKeys[0]) == 0 { + if len(secondKeys) == 0 || secondKeys[0] == "" { return v } v.Set(secondPath, secondValue) @@ -129,7 +129,7 @@ func overrideFromLayer(l layer, assert *assert.Assertions, firstPath string, fir // configuration map of the given layer, and that the final value equals the one given. func deepCheckValue(assert *assert.Assertions, v *Viper, l layer, keys []string, value any) { if assert == nil || v == nil || - len(keys) == 0 || len(keys[0]) == 0 { + len(keys) == 0 || keys[0] == "" { return } diff --git a/remote/remote.go b/remote/remote.go index d61f880..4a89e42 100644 --- a/remote/remote.go +++ b/remote/remote.go @@ -44,7 +44,7 @@ func (rc remoteConfigProvider) Watch(rp viper.RemoteProvider) (io.Reader, error) return bytes.NewReader(resp), nil } -func (rc remoteConfigProvider) WatchChannel(rp viper.RemoteProvider) (<-chan *viper.RemoteResponse, chan bool) { +func (rc remoteConfigProvider) WatchChannel(rp viper.RemoteProvider) (responseCh <-chan *viper.RemoteResponse, quitCh chan bool) { cm, err := getConfigManager(rp) if err != nil { return nil, nil diff --git a/viper.go b/viper.go index 53ded8a..4bff7e8 100644 --- a/viper.go +++ b/viper.go @@ -345,7 +345,7 @@ func (v *Viper) resetEncoding() { } { - codec := ini.Codec{ + codec := &ini.Codec{ KeyDelimiter: v.keyDelim, LoadOptions: v.iniLoadOptions, } @@ -957,7 +957,8 @@ func (v *Viper) Sub(key string) *Viper { } if reflect.TypeOf(data).Kind() == reflect.Map { - subv.parents = append(v.parents, strings.ToLower(key)) + subv.parents = append([]string(nil), v.parents...) + subv.parents = append(subv.parents, strings.ToLower(key)) subv.automaticEnvApplied = v.automaticEnvApplied subv.envPrefix = v.envPrefix subv.envKeyReplacer = v.envKeyReplacer @@ -1434,7 +1435,7 @@ func readAsCSV(val string) ([]string, error) { func stringToStringConv(val string) any { val = strings.Trim(val, "[]") // An empty string would cause an empty map - if len(val) == 0 { + if val == "" { return map[string]any{} } r := csv.NewReader(strings.NewReader(val)) @@ -1458,7 +1459,7 @@ func stringToStringConv(val string) any { func stringToIntConv(val string) any { val = strings.Trim(val, "[]") // An empty string would cause an empty map - if len(val) == 0 { + if val == "" { return map[string]any{} } ss := strings.Split(val, ",") @@ -1506,13 +1507,13 @@ func (v *Viper) SetEnvKeyReplacer(r *strings.Replacer) { // RegisterAlias creates an alias that provides another accessor for the same key. // This enables one to change a name without breaking the application. -func RegisterAlias(alias string, key string) { v.RegisterAlias(alias, key) } +func RegisterAlias(alias, key string) { v.RegisterAlias(alias, key) } -func (v *Viper) RegisterAlias(alias string, key string) { +func (v *Viper) RegisterAlias(alias, key string) { v.registerAlias(alias, strings.ToLower(key)) } -func (v *Viper) registerAlias(alias string, key string) { +func (v *Viper) registerAlias(alias, key string) { alias = strings.ToLower(alias) if alias != key && alias != v.realKey(key) { _, exists := v.aliases[alias] @@ -2181,6 +2182,8 @@ func (v *Viper) SetConfigPermissions(perm os.FileMode) { } // IniLoadOptions sets the load options for ini parsing. +// +//nolint:gocritic // hugeParam: in is heavy (114 bytes); consider passing it by pointer func IniLoadOptions(in ini.LoadOptions) Option { return optionFunc(func(v *Viper) { v.iniLoadOptions = in diff --git a/viper_test.go b/viper_test.go index ca579eb..0d7e6f3 100644 --- a/viper_test.go +++ b/viper_test.go @@ -234,17 +234,15 @@ func initIni() { } // initDirs makes directories for testing. -func initDirs(t *testing.T) (string, string) { - var ( - testDirs = []string{`a a`, `b`, `C_`} - config = `improbable` - ) +func initDirs(t *testing.T) (root, config string) { + testDirs := []string{`a a`, `b`, `C_`} + config = `improbable` if runtime.GOOS != "windows" { testDirs = append(testDirs, `d\d`) } - root := t.TempDir() + root = t.TempDir() for _, dir := range testDirs { innerDir := filepath.Join(root, dir) @@ -428,7 +426,7 @@ func TestReadInConfig(t *testing.T) { file, err := fs.Create(testutil.AbsFilePath(t, "/etc/viper/config.yaml")) require.NoError(t, err) - _, err = file.Write([]byte(`key: value`)) + _, err = file.WriteString(`key: value`) require.NoError(t, err) file.Close() @@ -453,7 +451,7 @@ func TestReadInConfig(t *testing.T) { file, err := fs.Create(testutil.AbsFilePath(t, "/etc/viper/config.yaml")) require.NoError(t, err) - _, err = file.Write([]byte(`key: value`)) + _, err = file.WriteString(`key: value`) require.NoError(t, err) file.Close() @@ -936,7 +934,8 @@ func TestUnmarshalWithDecoderOptions(t *testing.T) { if raw == "" { return m, nil } - return m, json.Unmarshal([]byte(raw), &m) + err := json.Unmarshal([]byte(raw), &m) + return m, err }, )) @@ -2343,12 +2342,12 @@ func doTestCaseInsensitive(t *testing.T, typ, config string) { assert.Equal(t, 5, cast.ToInt(Get("ef.lm.p.q"))) } -func newViperWithConfigFile(t *testing.T) (*Viper, string) { +func newViperWithConfigFile(t *testing.T) (v *Viper, configFile string) { watchDir := t.TempDir() - configFile := path.Join(watchDir, "config.yaml") + configFile = path.Join(watchDir, "config.yaml") err := os.WriteFile(configFile, []byte("foo: bar\n"), 0o640) require.NoError(t, err) - v := New() + v = New() v.SetConfigFile(configFile) err = v.ReadInConfig() require.NoError(t, err) @@ -2356,8 +2355,8 @@ func newViperWithConfigFile(t *testing.T) (*Viper, string) { return v, configFile } -func newViperWithSymlinkedConfigFile(t *testing.T) (*Viper, string, string) { - watchDir := t.TempDir() +func newViperWithSymlinkedConfigFile(t *testing.T) (v *Viper, watchDir, configFile string) { + watchDir = t.TempDir() dataDir1 := path.Join(watchDir, "data1") err := os.Mkdir(dataDir1, 0o777) require.NoError(t, err) @@ -2368,11 +2367,11 @@ func newViperWithSymlinkedConfigFile(t *testing.T) (*Viper, string, string) { // now, symlink the tm `data1` dir to `data` in the baseDir os.Symlink(dataDir1, path.Join(watchDir, "data")) // and link the `/datadir1/config.yaml` to `/config.yaml` - configFile := path.Join(watchDir, "config.yaml") + configFile = path.Join(watchDir, "config.yaml") os.Symlink(path.Join(watchDir, "data", "config.yaml"), configFile) t.Logf("Config file location: %s\n", path.Join(watchDir, "config.yaml")) // init Viper - v := New() + v = New() v.SetConfigFile(configFile) err = v.ReadInConfig() require.NoError(t, err) @@ -2612,6 +2611,8 @@ func BenchmarkGetBoolFromMap(b *testing.B) { } // Skip some tests on Windows that kept failing when Windows was added to the CI as a target. +// +//nolint:gocritic // sloppyTestFuncName func skipWindows(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("Skip test on Windows")