Linting (#181)

.golangci.yml

@@ -1,12 +1,8 @@
 run:
-  concurrency: 4
   timeout: 5m
   issues-exit-code: 1
   tests: true
 
-  skip-dirs:
-    - vendor
-
   modules-download-mode: vendor
 
   # list of build tags, all linters use it. Default is empty list
@@ -15,9 +11,6 @@ run:
 
 # output configuration options
 output:
-  # colored-line-number|line-number|json|tab|checkstyle|code-climate, default is "colored-line-number"
-  format: colored-line-number
-
   # print lines of code with issue, default is true
   print-issued-lines: true
 
@@ -29,15 +22,16 @@ output:
 linters:
   disable-all: true
   enable:
-    - golint
     - gofmt
-    - gosec
+    - gosimple
     - unparam
     - goconst
     - prealloc
     - stylecheck
     - unconvert
+    - unused
     - staticcheck
+    - ineffassign
     - gosec
     - tparallel
     - whitespace
@@ -48,19 +42,47 @@ linters:
     - errname
     - govet
     - predeclared
+    - nestif
     - exhaustive
     - tenv
     - gofumpt
     - forcetypeassert
     - nilerr
     - errcheck
-    # - promlinter this is a very nice linter, but it will most probably break things...
-    # - nestif
+    - bodyclose
+    - goimports
+    - durationcheck
+    - errchkjson
+    - sloglint
+    - dupword
+    - noctx
+    - makezero
+    - nilnil
+    - reassign
+    - sloglint
+    - spancheck
+    - testifylint
+    - wastedassign
+    - rowserrcheck
+    - sqlclosecheck
+    - goprintffuncname
+    - testableexamples
+    - wastedassign
+    - nonamedreturns
+    - perfsprint
+    - dogsled
+    - protogetter
+    - usestdlibvars
+    - testableexamples
   fast: false
 
 issues:
+  exclude-dirs:
+    - vendor
+  max-same-issues: 10
+
   exclude-rules:
     # Exclude some staticcheck messages
     - linters:
         - staticcheck
-      text: "SA1019:"
+      text: "SA1019:"

Makefile

@@ -24,10 +24,10 @@ fmtcheck:
 	@sh -c "'$(CURDIR)/scripts/gofmtcheck.sh'"
 
 lint: fmtcheck
-	$(DOCKER) run --env=GOFLAGS=-mod=vendor --rm -v $(CURDIR):/app -w /app golangci/golangci-lint:v1.54.2 golangci-lint -v run
+	$(DOCKER) run --env=GOFLAGS=-mod=vendor --rm -v $(CURDIR):/app -w /app golangci/golangci-lint:v1.61.0 golangci-lint -v run
 
 deeplint: fmtcheck
-	$(DOCKER) run --env=GOFLAGS=-mod=vendor --rm -v $(CURDIR):/app -w /app golangci/golangci-lint:v1.54.2 golangci-lint run --exclude-use-default=false --enable-all -D dupl --build-tags integration
+	$(DOCKER) run --env=GOFLAGS=-mod=vendor --rm -v $(CURDIR):/app -w /app golangci/golangci-lint:v1.61.0 golangci-lint run --exclude-use-default=false --enable-all -D dupl --build-tags integration
 
 deps:
 	docker container inspect harvester-consul > /dev/null 2>&1 || docker run -d --rm -p 8500:8500 -p 8600:8600/udp --name=harvester-consul consul:1.4.3 agent -server -ui -node=server-1 -bootstrap-expect=1 -client=0.0.0.0  -http-port 8500 -log-level=err

config/config_test.go

@@ -35,9 +35,9 @@ func TestField_Set(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			err := tt.field.Set(tt.args.value, tt.args.version)
 			if tt.wantErr {
-				assert.Error(t, err)
+				require.Error(t, err)
 			} else {
-				assert.NoError(t, err)
+				require.NoError(t, err)
 			}
 		})
 	}
@@ -64,10 +64,10 @@ func TestNew(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			got, err := New(tt.args.cfg, nil)
 			if tt.wantErr {
-				assert.Error(t, err)
+				require.Error(t, err)
 				assert.Nil(t, got)
 			} else {
-				assert.NoError(t, err)
+				require.NoError(t, err)
 				assert.NotNil(t, got)
 				assert.Len(t, got.Fields, 7)
 				assertField(t, got.Fields[0], "Name", "String",
@@ -102,40 +102,40 @@ func TestConfig_Set(t *testing.T) {
 	cfg, err := New(&c, chNotify)
 	require.NoError(t, err)
 	err = cfg.Fields[0].Set("John Doe", 1)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	change := <-chNotify
 	assert.Equal(t, "field [Name] of type [String] changed from [] to [John Doe]", change.String())
 	err = cfg.Fields[1].Set("18", 1)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	change = <-chNotify
 	assert.Equal(t, "field [Age] of type [Int64] changed from [0] to [18]", change.String())
 	err = cfg.Fields[2].Set("99.9", 1)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	change = <-chNotify
 	assert.Equal(t, "field [Balance] of type [Float64] changed from [0.000000] to [99.9]", change.String())
 	err = cfg.Fields[3].Set("true", 1)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	change = <-chNotify
 	assert.Equal(t, "field [HasJob] of type [Bool] changed from [false] to [true]", change.String())
 	err = cfg.Fields[4].Set("6000", 1)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	change = <-chNotify
 	assert.Equal(t, "field [PositionSalary] of type [Int64] changed from [0] to [6000]", change.String())
 	err = cfg.Fields[5].Set("baz", 1)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	change = <-chNotify
 	assert.Equal(t, "field [LevelOneLevelTwoDeepField] of type [String] changed from [] to [baz]", change.String())
 	err = cfg.Fields[6].Set("true", 1)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	change = <-chNotify
 	assert.Equal(t, "field [IsAdult] of type [Bool] changed from [false] to [true]", change.String())
 	assert.Equal(t, "John Doe", c.Name.Get())
 	assert.Equal(t, int64(18), c.Age.Get())
-	assert.Equal(t, 99.9, c.Balance.Get())
-	assert.Equal(t, true, c.HasJob.Get())
+	assert.InDelta(t, 99.9, c.Balance.Get(), 0.01)
+	assert.True(t, c.HasJob.Get())
 	assert.Equal(t, int64(6000), c.Position.Salary.Get())
 	assert.Equal(t, "baz", c.LevelOne.LevelTwo.DeepField.Get())
-	assert.Equal(t, true, c.IsAdult.Get())
+	assert.True(t, c.IsAdult.Get())
 }
 
 type testNestedConfig struct {

config/custom_type_test.go

@@ -1,33 +1,34 @@
 package config_test
 
 import (
-	"fmt"
+	"errors"
 	"sync"
 	"testing"
 
 	"github.com/beatlabs/harvester/config"
 	stdTypes "github.com/beatlabs/harvester/sync"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestCustomField(t *testing.T) {
 	c := &testConfig{}
 	cfg, err := config.New(c, nil)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	err = cfg.Fields[0].Set("expected", 1)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	err = cfg.Fields[1].Set("bar", 1)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.Equal(t, "expected", c.CustomValue.Get())
 	assert.Equal(t, "bar", c.SomeString.Get())
 }
 
 func TestErrorValidationOnCustomField(t *testing.T) {
 	c := &testConfig{}
 	cfg, err := config.New(c, nil)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	err = cfg.Fields[0].Set("not_expected", 1)
-	assert.Error(t, err)
+	require.Error(t, err)
 }
 
 type testConcreteValue struct {
@@ -53,7 +54,7 @@ func (f *testConcreteValue) String() string {
 
 func (f *testConcreteValue) SetString(value string) error {
 	if value != "expected" {
-		return fmt.Errorf("unable to store provided value")
+		return errors.New("unable to store provided value")
 	}
 	f.Set(value)
 	return nil

harvester_integration_test.go

@@ -86,11 +86,11 @@ func Test_harvester_Harvest(t *testing.T) {
 	defer cnl()
 	err = h.Harvest(ctx)
 
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.Equal(t, "Mr. Smith", cfg.Name.Get())
 	assert.Equal(t, int64(99), cfg.Age.Get())
-	assert.Equal(t, 111.1, cfg.Balance.Get())
-	assert.Equal(t, false, cfg.HasJob.Get())
+	assert.InDelta(t, 111.1, cfg.Balance.Get(), 0.01)
+	assert.False(t, cfg.HasJob.Get())
 	assert.Equal(t, 1*time.Second, cfg.FunTime.Get())
 	assert.Equal(t, 2*time.Second, cfg.WorkTime.Get())
 	assert.Equal(t, int64(123), cfg.Foo.Bar.Get())

harvester_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/beatlabs/harvester/sync"
 	"github.com/go-redis/redis/v8"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 const (
@@ -93,10 +94,10 @@ func TestCreateWithConsulAndRedis(t *testing.T) {
 				WithRedisMonitor(tt.args.monitorRedisClient, tt.args.monitoringPollInterval))
 
 			if tt.expectedErr != "" {
-				assert.EqualError(t, err, tt.expectedErr)
+				require.EqualError(t, err, tt.expectedErr)
 				assert.Nil(t, got)
 			} else {
-				assert.NoError(t, err)
+				require.NoError(t, err)
 				assert.NotNil(t, got)
 			}
 		})
@@ -117,7 +118,7 @@ func TestWithNotification(t *testing.T) {
 	for name, tt := range tests {
 		t.Run(name, func(t *testing.T) {
 			got, err := New(tt.args.cfg, tt.args.chNotify)
-			assert.NoError(t, err)
+			require.NoError(t, err)
 			assert.NotNil(t, got)
 		})
 	}
@@ -126,36 +127,35 @@ func TestWithNotification(t *testing.T) {
 func TestCreate_NoConsulOrRedis(t *testing.T) {
 	cfg := &testConfigNoConsul{}
 	got, err := New(cfg, nil)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.NotNil(t, got)
 	ctx, cnl := context.WithCancel(context.Background())
 	defer cnl()
-	err = got.Harvest(ctx)
-	assert.NoError(t, err)
+	require.NoError(t, got.Harvest(ctx))
 	assert.Equal(t, "John Doe", cfg.Name.Get())
 	assert.Equal(t, int64(18), cfg.Age.Get())
-	assert.Equal(t, 99.9, cfg.Balance.Get())
-	assert.Equal(t, true, cfg.HasJob.Get())
+	assert.InDelta(t, 99.9, cfg.Balance.Get(), 0.01)
+	assert.True(t, cfg.HasJob.Get())
 	assert.Equal(t, int64(8000), cfg.Position.Salary.Get())
 	assert.Equal(t, int64(24), cfg.Position.Place.RoomNumber.Get())
 }
 
 func TestCreate_SeedError(t *testing.T) {
 	cfg := &testConfigSeedError{}
 	got, err := New(cfg, nil)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.NotNil(t, got)
 	ctx, cnl := context.WithCancel(context.Background())
 	defer cnl()
 	err = got.Harvest(ctx)
-	assert.Error(t, err)
+	require.Error(t, err)
 }
 
 type testConfig struct {
 	Name    sync.String       `seed:"John Doe" consul:"harvester1/name"`
-	Age     sync.Int64        `seed:"18"  consul:"harvester/age"`
-	Balance sync.Float64      `seed:"99.9"  consul:"harvester/balance"`
-	HasJob  sync.Bool         `seed:"true"  consul:"harvester/has-job"`
+	Age     sync.Int64        `seed:"18" consul:"harvester/age"`
+	Balance sync.Float64      `seed:"99.9" consul:"harvester/balance"`
+	HasJob  sync.Bool         `seed:"true" consul:"harvester/has-job"`
 	FunTime sync.TimeDuration `seed:"1s" consul:"harvester/fun-time"`
 	IsAdult sync.Bool         `seed:"false" redis:"is-adult"`
 }

monitor/consul/watcher.go

@@ -112,7 +112,7 @@ func (w *Watcher) createKeyPlanWithPrefix(key, prefix string, ch chan<- []*chang
 	if err != nil {
 		return nil, err
 	}
-	pl.Handler = func(idx uint64, data interface{}) {
+	pl.Handler = func(_ uint64, data interface{}) {
 		if data == nil {
 			return
 		}
@@ -132,7 +132,7 @@ func (w *Watcher) createKeyPrefixPlan(keyPrefix string, ch chan<- []*change.Chan
 	if err != nil {
 		return nil, err
 	}
-	pl.Handler = func(idx uint64, data interface{}) {
+	pl.Handler = func(_ uint64, data interface{}) {
 		if data == nil {
 			return
 		}

monitor/consul/watcher_integration_test.go

@@ -71,7 +71,7 @@ func TestWatch(t *testing.T) {
 			default:
 				assert.Fail(t, "key invalid", cng.Key())
 			}
-			assert.True(t, cng.Version() > 0)
+			assert.Positive(t, cng.Version())
 		}
 	}
 }

monitor/consul/watcher_test.go

@@ -30,10 +30,10 @@ func TestNew(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			got, err := New(tt.args.addr, "dc", "token", tt.args.timeout, tt.args.ii...)
 			if tt.wantErr {
-				assert.Error(t, err)
+				require.Error(t, err)
 				assert.Nil(t, got)
 			} else {
-				assert.NoError(t, err)
+				require.NoError(t, err)
 				assert.NotNil(t, got)
 			}
 		})
@@ -58,9 +58,9 @@ func TestWatcher_Watch(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			err = w.Watch(tt.args.ctx, tt.args.ch)
 			if tt.wantErr {
-				assert.Error(t, err)
+				require.Error(t, err)
 			} else {
-				assert.NoError(t, err)
+				require.NoError(t, err)
 			}
 		})
 	}