cleanup (#100)

.github/workflows/test.yml

@@ -26,7 +26,7 @@ jobs:
         run: go test -v -shuffle=on -count=10 -race ./...
         working-directory: ${{ matrix.module }}
       - name: Test
-        run: go test -v ./...
+        run: go test -shuffle=on -v ./...
         working-directory: ${{ matrix.module }}
   all:
     if: ${{ always() }}

.golangci.yml

@@ -134,14 +134,13 @@ linters:
 
 issues:
   exclude-rules:
-    - text: 'shadow: declaration of "(err|ctx)" shadows declaration at'
-      linters:
-        - govet
     - path: _test\.go
       linters:
         - cyclop
         - forcetypeassert
         - funlen
+        - gochecknoglobals
+        - gochecknoinits
         - goconst
         - goerr113
         - wrapcheck

config_test.go

@@ -150,8 +150,8 @@ func TestConfig_Unmarshal(t *testing.T) {
 		},
 	}
 
-	for i := range testcases {
-		testcase := testcases[i]
+	for _, testcase := range testcases {
+		testcase := testcase
 
 		t.Run(testcase.description, func(t *testing.T) {
 			t.Parallel()

internal/maps/merge_test.go

@@ -22,14 +22,14 @@ func TestMerge(t *testing.T) {
 		{
 			description: "nil source",
 			src:         nil,
-			dst:         make(map[string]any),
-			expected:    make(map[string]any),
+			dst:         map[string]any{},
+			expected:    map[string]any{},
 		},
 		{
 			description: "empty",
-			src:         make(map[string]any),
-			dst:         make(map[string]any),
-			expected:    make(map[string]any),
+			src:         map[string]any{},
+			dst:         map[string]any{},
+			expected:    map[string]any{},
 		},
 		{
 			description: "no key conflict",

provider/appconfig/appconfig.go

@@ -15,6 +15,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"log/slog"
+	"reflect"
 	"sync/atomic"
 	"time"
 
@@ -70,14 +71,14 @@ func New(application, environment, profile string, opts ...Option) *AppConfig {
 	if option.pollInterval <= 0 {
 		option.pollInterval = time.Minute
 	}
-	if option.awsConfig == nil {
+	if reflect.ValueOf(option.awsConfig).IsZero() {
 		awsConfig, err := config.LoadDefaultConfig(context.Background())
 		if err != nil {
 			panic(fmt.Sprintf("cannot load AWS default config: %v", err))
 		}
-		option.awsConfig = &awsConfig
+		option.awsConfig = awsConfig
 	}
-	option.AppConfig.client = appconfigdata.NewFromConfig(*option.awsConfig)
+	option.AppConfig.client = appconfigdata.NewFromConfig(option.awsConfig)
 
 	return &option.AppConfig
 }
@@ -97,21 +98,7 @@ func (a *AppConfig) Load() (map[string]any, error) {
 		a.token.Store(output.InitialConfigurationToken)
 	}
 
-	input := &appconfigdata.GetLatestConfigurationInput{
-		ConfigurationToken: a.token.Load(),
-	}
-	output, err := a.client.GetLatestConfiguration(context.Background(), input)
-	if err != nil {
-		return nil, fmt.Errorf("get latest configuration: %w", err)
-	}
-	a.token.Store(output.NextPollConfigurationToken)
-
-	var out map[string]any
-	if err := a.unmarshal(output.Configuration, &out); err != nil {
-		return nil, fmt.Errorf("unmarshal: %w", err)
-	}
-
-	return out, nil
+	return a.load(context.Background())
 }
 
 func (a *AppConfig) Watch(ctx context.Context, onChange func(map[string]any)) error {
@@ -121,10 +108,7 @@ func (a *AppConfig) Watch(ctx context.Context, onChange func(map[string]any)) er
 	for {
 		select {
 		case <-ticker.C:
-			input := &appconfigdata.GetLatestConfigurationInput{
-				ConfigurationToken: a.token.Load(),
-			}
-			output, err := a.client.GetLatestConfiguration(ctx, input)
+			out, err := a.load(ctx)
 			if err != nil {
 				a.logger.WarnContext(
 					ctx, "Error when reloading from AWS AppConfig",
@@ -136,34 +120,40 @@ func (a *AppConfig) Watch(ctx context.Context, onChange func(map[string]any)) er
 
 				continue
 			}
-			a.token.Store(output.NextPollConfigurationToken)
 
-			if len(output.Configuration) == 0 {
-				// It may return empty configuration data
-				// if the client already has the latest version.
-				continue
+			if out != nil {
+				onChange(out)
 			}
-
-			var out map[string]any
-			if err := a.unmarshal(output.Configuration, &out); err != nil {
-				a.logger.WarnContext(
-					ctx, "Error when unmarshalling config from AWS AppConfig",
-					"application", a.application,
-					"environment", a.environment,
-					"profile", a.profile,
-					"error", err,
-				)
-
-				continue
-			}
-
-			onChange(out)
 		case <-ctx.Done():
 			return nil
 		}
 	}
 }
 
+func (a *AppConfig) load(ctx context.Context) (map[string]any, error) {
+	input := &appconfigdata.GetLatestConfigurationInput{
+		ConfigurationToken: a.token.Load(),
+	}
+	output, err := a.client.GetLatestConfiguration(ctx, input)
+	if err != nil {
+		return nil, fmt.Errorf("get latest configuration: %w", err)
+	}
+	a.token.Store(output.NextPollConfigurationToken)
+
+	if len(output.Configuration) == 0 {
+		// It may return empty configuration data
+		// if the client already has the latest version.
+		return nil, nil //nolint:nilnil // Use nil to indicate no change
+	}
+
+	var out map[string]any
+	if e := a.unmarshal(output.Configuration, &out); e != nil {
+		return nil, fmt.Errorf("unmarshal: %w", e)
+	}
+
+	return out, nil
+}
+
 func (a *AppConfig) String() string {
 	return "appConfig:" + a.application + "-" + a.environment + "-" + a.profile
 }

provider/appconfig/appconfig_test.go

@@ -55,8 +55,8 @@ func TestAppConfig_New_panic(t *testing.T) {
 		},
 	}
 
-	for i := range testcases {
-		testcase := testcases[i]
+	for _, testcase := range testcases {
+		testcase := testcase
 
 		t.Run(testcase.description, func(t *testing.T) {
 			t.Parallel()
@@ -187,8 +187,8 @@ func TestAppConfig_Load(t *testing.T) {
 		},
 	}
 
-	for i := range testcases {
-		testcase := testcases[i]
+	for _, testcase := range testcases {
+		testcase := testcase
 
 		t.Run(testcase.description, func(t *testing.T) {
 			t.Parallel()
@@ -211,7 +211,7 @@ func TestAppConfig_Load(t *testing.T) {
 
 			loader := appconfig.New(
 				"app", "env", "profiler",
-				appconfig.WithAWSConfig(&cfg),
+				appconfig.WithAWSConfig(cfg),
 				appconfig.WithUnmarshal(testcase.unmarshal),
 			)
 			values, err := loader.Load()
@@ -226,6 +226,8 @@ func TestAppConfig_Load(t *testing.T) {
 }
 
 func TestAppConfig_Watch(t *testing.T) {
+	t.Parallel()
+
 	testcases := []struct {
 		description string
 		middleware  func(
@@ -294,7 +296,8 @@ func TestAppConfig_Watch(t *testing.T) {
 			},
 			log: `level=WARN msg="Error when reloading from AWS AppConfig"` +
 				` application=app environment=env profile=profiler` +
-				` error="operation error AppConfigData: GetLatestConfiguration, get latest configuration error"` + "\n",
+				` error="get latest configuration: operation error AppConfigData: GetLatestConfiguration,` +
+				` get latest configuration error"` + "\n",
 		},
 		{
 			description: "unmarshal error",
@@ -318,15 +321,17 @@ func TestAppConfig_Watch(t *testing.T) {
 			unmarshal: func([]byte, any) error {
 				return errors.New("unmarshal error")
 			},
-			log: `level=WARN msg="Error when unmarshalling config from AWS AppConfig"` +
-				` application=app environment=env profile=profiler error="unmarshal error"` + "\n",
+			log: `level=WARN msg="Error when reloading from AWS AppConfig"` +
+				` application=app environment=env profile=profiler error="unmarshal: unmarshal error"` + "\n",
 		},
 	}
 
-	for i := range testcases {
-		testcase := testcases[i]
+	for _, testcase := range testcases {
+		testcase := testcase
 
 		t.Run(testcase.description, func(t *testing.T) {
+			t.Parallel()
+
 			cfg, err := config.LoadDefaultConfig(
 				context.Background(),
 				config.WithAPIOptions([]func(*middleware.Stack) error{
@@ -366,7 +371,7 @@ func TestAppConfig_Watch(t *testing.T) {
 			buf := new(buffer)
 			loader := appconfig.New(
 				"app", "env", "profiler",
-				appconfig.WithAWSConfig(&cfg),
+				appconfig.WithAWSConfig(cfg),
 				appconfig.WithPollInterval(100*time.Millisecond),
 				appconfig.WithLogHandler(logHandler(buf)),
 				appconfig.WithUnmarshal(testcase.unmarshal),

provider/appconfig/benchmark_test.go

@@ -65,7 +65,7 @@ func BenchmarkLoad(b *testing.B) {
 	)
 	assert.NoError(b, err)
 
-	loader := appconfig.New("app", "env", "profiler", appconfig.WithAWSConfig(&cfg))
+	loader := appconfig.New("app", "env", "profiler", appconfig.WithAWSConfig(cfg))
 	b.ResetTimer()
 
 	var values map[string]any

provider/appconfig/option.go

@@ -13,7 +13,7 @@ import (
 // WithAWSConfig provides the AWS Config for the AWS SDK.
 //
 // By default, it loads the default AWS Config.
-func WithAWSConfig(awsConfig *aws.Config) Option {
+func WithAWSConfig(awsConfig aws.Config) Option {
 	return func(options *options) {
 		options.awsConfig = awsConfig
 	}
@@ -22,9 +22,9 @@ func WithAWSConfig(awsConfig *aws.Config) Option {
 // WithPollInterval provides the interval for polling the configuration.
 //
 // The default interval is 1 minute.
-func WithPollInterval(pollInterval time.Duration) Option {
+func WithPollInterval(interval time.Duration) Option {
 	return func(options *options) {
-		options.pollInterval = pollInterval
+		options.pollInterval = interval
 	}
 }
 
@@ -55,6 +55,6 @@ type (
 	options struct {
 		AppConfig
 
-		awsConfig *aws.Config
+		awsConfig aws.Config
 	}
 )

provider/env/env_test.go

@@ -45,8 +45,8 @@ func TestEnv_Load(t *testing.T) {
 	t.Setenv("P.D", ".")
 	t.Setenv("P.N", "")
 
-	for i := range testcases {
-		testcase := testcases[i]
+	for _, testcase := range testcases {
+		testcase := testcase
 
 		t.Run(testcase.description, func(t *testing.T) {
 			values, err := env.New(testcase.opts...).Load()
@@ -75,8 +75,8 @@ func TestEnv_String(t *testing.T) {
 		},
 	}
 
-	for i := range testcases {
-		testcase := testcases[i]
+	for _, testcase := range testcases {
+		testcase := testcase
 
 		t.Run(testcase.description, func(t *testing.T) {
 			t.Parallel()

provider/file/file_test.go

@@ -1,8 +1,6 @@
 // Copyright (c) 2024 The konf authors
 // Use of this source code is governed by a MIT license found in the LICENSE file.
 
-//go:build !race
-
 package file_test
 
 import (
@@ -59,8 +57,8 @@ func TestFile_Load(t *testing.T) {
 		},
 	}
 
-	for i := range testcases {
-		testcase := testcases[i]
+	for _, testcase := range testcases {
+		testcase := testcase
 
 		t.Run(testcase.description, func(t *testing.T) {
 			t.Parallel()

provider/file/watch.go

@@ -19,16 +19,16 @@ func (f File) Watch(ctx context.Context, onChange func(map[string]any)) error {
 		return fmt.Errorf("create file watcher for %s: %w", f.path, err)
 	}
 	defer func() {
-		if err := watcher.Close(); err != nil {
-			f.logger.WarnContext(ctx, "Error when closing file watcher.", "file", f.path, "error", err)
+		if e := watcher.Close(); e != nil {
+			f.logger.WarnContext(ctx, "Error when closing file watcher.", "file", f.path, "error", e)
 		}
 	}()
 
 	// Although only a single file is being watched, fsnotify has to watch
 	// the whole parent directory to pick up all events such as symlink changes.
 	dir, _ := filepath.Split(f.path)
-	if err := watcher.Add(dir); err != nil {
-		return fmt.Errorf("watch dir %s: %w", dir, err)
+	if e := watcher.Add(dir); e != nil {
+		return fmt.Errorf("watch dir %s: %w", dir, e)
 	}
 
 	// Resolve symlinks and save the original path so that changes to symlinks

provider/file/watch_test.go

@@ -18,6 +18,8 @@ import (
 )
 
 func TestFile_Watch(t *testing.T) {
+	t.Parallel()
+
 	testcases := []struct {
 		description string
 		action      func(string) error
@@ -43,10 +45,12 @@ func TestFile_Watch(t *testing.T) {
 		},
 	}
 
-	for i := range testcases {
-		testcase := testcases[i]
+	for _, testcase := range testcases {
+		testcase := testcase
 
 		t.Run(testcase.description, func(t *testing.T) {
+			t.Parallel()
+
 			tmpFile := filepath.Join(t.TempDir(), "watch.json")
 			assert.NoError(t, os.WriteFile(tmpFile, []byte(`{"p": {"k": "v"}}`), 0o600))
 

provider/flag/flag.go

@@ -56,7 +56,9 @@ func New(konf konf, opts ...Option) Flag {
 		option.delimiter = "."
 	}
 	if option.set == nil {
-		flag.Parse()
+		if !flag.Parsed() {
+			flag.Parse()
+		}
 		option.set = flag.CommandLine
 	}