From 7b96743b37dc8fe42edfdde61fe84de89738e78d Mon Sep 17 00:00:00 2001 From: Sotirios Mantziaris Date: Mon, 14 Mar 2022 09:21:56 +0200 Subject: [PATCH] Linter improvements --- .golangci.yml | 66 ++++++++++++++++++++++ Makefile | 4 +- config/config.go | 11 +++- config/parser.go | 6 +- harvester_integration_test.go | 1 + harvester_test.go | 1 - log/consul.go | 1 + monitor/consul/watcher_integration_test.go | 1 + monitor/redis/watcher.go | 6 +- monitor/redis/watcher_test.go | 1 - seed/consul/getter.go | 2 +- seed/consul/getter_integration_test.go | 1 + seed/redis/getter_integration_test.go | 1 + seed/seed.go | 2 +- 14 files changed, 91 insertions(+), 13 deletions(-) create mode 100644 .golangci.yml diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 00000000..de06c13c --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,66 @@ +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 + build-tags: + - integration + +# 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 + + # print linter name in the end of issue text, default is true + print-linter-name: true + + uniq-by-line: false + +linters: + disable-all: true + enable: + - golint + - gofmt + - gosec + - unparam + - goconst + - prealloc + - stylecheck + - unconvert + - staticcheck + - gosec + - tparallel + - whitespace + - revive + - godot + - errorlint + - gocritic + - errname + - govet + - predeclared + - exhaustive + - tenv + - gofumpt + - forcetypeassert + - nilerr + - errcheck + # - promlinter this is a very nice linter, but it will most probably break things... + # - nestif + fast: false + +issues: + exclude-rules: + # Exclude some staticcheck messages + - linters: + - staticcheck + text: "SA1019:" \ No newline at end of file diff --git a/Makefile b/Makefile index a6902f87..fc7f9058 100644 --- a/Makefile +++ b/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.28.1 golangci-lint run --enable golint,gofmt,unparam,goconst,prealloc,stylecheck,unconvert --exclude-use-default=false --deadline=5m --build-tags integration + $(DOCKER) run --env=GOFLAGS=-mod=vendor --rm -v $(CURDIR):/app -w /app golangci/golangci-lint:v1.44.2 golangci-lint -v run deeplint: fmtcheck - docker run --env=GOFLAGS=-mod=vendor --rm -v $(CURDIR):/app -w /app golangci/golangci-lint:v1.28.1 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.44.2 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 diff --git a/config/config.go b/config/config.go index ebbe27cf..9b299852 100644 --- a/config/config.go +++ b/config/config.go @@ -58,12 +58,17 @@ type Field struct { } // newField constructor. -func newField(prefix string, fld reflect.StructField, val reflect.Value, chNotify chan<- ChangeNotification) *Field { +func newField(prefix string, fld reflect.StructField, val reflect.Value, chNotify chan<- ChangeNotification) (*Field, error) { + sf, ok := val.Addr().Interface().(CfgType) + if !ok { + return nil, errors.New("failed to type assert to CfgType") + } + f := &Field{ name: prefix + fld.Name, tp: fld.Type.Name(), version: 0, - structField: val.Addr().Interface().(CfgType), + structField: sf, sources: make(map[Source]string), chNotify: chNotify, } @@ -75,7 +80,7 @@ func newField(prefix string, fld reflect.StructField, val reflect.Value, chNotif } } - return f + return f, nil } // Name getter. diff --git a/config/parser.go b/config/parser.go index 8378caf5..20a0e1ee 100644 --- a/config/parser.go +++ b/config/parser.go @@ -57,13 +57,17 @@ func (p *parser) getFields(prefix string, tp reflect.Type, val reflect.Value, ch return nil, err } ff = append(ff, nested...) + case typeInvalid: } } return ff, nil } func (p *parser) createField(prefix string, f reflect.StructField, val reflect.Value, chNotify chan<- ChangeNotification) (*Field, error) { - fld := newField(prefix, f, val, chNotify) + fld, err := newField(prefix, f, val, chNotify) + if err != nil { + return nil, err + } value, ok := fld.Sources()[SourceConsul] if ok { diff --git a/harvester_integration_test.go b/harvester_integration_test.go index a2d105dd..858a2800 100644 --- a/harvester_integration_test.go +++ b/harvester_integration_test.go @@ -1,3 +1,4 @@ +//go:build integration // +build integration package harvester diff --git a/harvester_test.go b/harvester_test.go index 27f23f72..8cd5b631 100644 --- a/harvester_test.go +++ b/harvester_test.go @@ -164,7 +164,6 @@ func TestWithConsulFolderPrefixMonitor(t *testing.T) { assert.Equal(t, test.ExpectedKeyLocation, filepath.Join(builder.monitorConsulCfg.folderPrefix, "key1")) }) } - } func TestCreate_NoConsulOrRedis(t *testing.T) { diff --git a/log/consul.go b/log/consul.go index 838853a1..54e8b144 100644 --- a/log/consul.go +++ b/log/consul.go @@ -30,6 +30,7 @@ func (l consul) Log(level hclog.Level, msg string, args ...interface{}) { warnf(msg, args) case hclog.Error: errorf(msg, args) + case hclog.Off: } } diff --git a/monitor/consul/watcher_integration_test.go b/monitor/consul/watcher_integration_test.go index 7e822d61..84cc85ca 100644 --- a/monitor/consul/watcher_integration_test.go +++ b/monitor/consul/watcher_integration_test.go @@ -1,3 +1,4 @@ +//go:build integration // +build integration package consul diff --git a/monitor/redis/watcher.go b/monitor/redis/watcher.go index e7ab1b91..fa061d90 100644 --- a/monitor/redis/watcher.go +++ b/monitor/redis/watcher.go @@ -3,7 +3,7 @@ package redis import ( "context" - "crypto/md5" + "crypto/md5" //nolint:gosec "encoding/hex" "errors" "time" @@ -79,7 +79,7 @@ func (w *Watcher) getValues(ctx context.Context, ch chan<- []*change.Change) { continue } if strCmd.Err() != nil { - if strCmd.Err() != redis.Nil { + if !errors.Is(strCmd.Err(), redis.Nil) { log.Errorf("failed to get value for key %s: %s", key, strCmd.Err()) } continue @@ -115,6 +115,6 @@ func (w *Watcher) getValues(ctx context.Context, ch chan<- []*change.Change) { } func (w *Watcher) hash(value string) string { - hash := md5.Sum([]byte(value)) + hash := md5.Sum([]byte(value)) // nolint:gosec return hex.EncodeToString(hash[:]) } diff --git a/monitor/redis/watcher_test.go b/monitor/redis/watcher_test.go index dfe6e18b..255af7f4 100644 --- a/monitor/redis/watcher_test.go +++ b/monitor/redis/watcher_test.go @@ -212,7 +212,6 @@ func (c *clientStub) Get(_ context.Context, key string) *redis.StringCmd { } return redis.NewStringResult("", redis.Nil) - } func (c *clientStub) rollInternalRedisState() { diff --git a/seed/consul/getter.go b/seed/consul/getter.go index 36f7ba00..cfc3f499 100644 --- a/seed/consul/getter.go +++ b/seed/consul/getter.go @@ -15,7 +15,7 @@ type Getter struct { token string } -// New constructor. Timeout is set to 60s when 0 is provided +// New constructor. Timeout is set to 60s when 0 is provided. func New(addr, dc, token string, timeout time.Duration) (*Getter, error) { if addr == "" { return nil, errors.New("address is empty") diff --git a/seed/consul/getter_integration_test.go b/seed/consul/getter_integration_test.go index 197d72b5..4f2f8d84 100644 --- a/seed/consul/getter_integration_test.go +++ b/seed/consul/getter_integration_test.go @@ -1,3 +1,4 @@ +//go:build integration // +build integration package consul diff --git a/seed/redis/getter_integration_test.go b/seed/redis/getter_integration_test.go index 2dc323cf..537785d1 100644 --- a/seed/redis/getter_integration_test.go +++ b/seed/redis/getter_integration_test.go @@ -1,3 +1,4 @@ +//go:build integration // +build integration package redis diff --git a/seed/seed.go b/seed/seed.go index ad4ffa92..2a9bc5d7 100644 --- a/seed/seed.go +++ b/seed/seed.go @@ -163,7 +163,7 @@ func (s *Seeder) Seed(cfg *config.Config) error { // the parsing won't stop, and we make sure we try to parse every flag passed when running the command. for _, arg := range os.Args[1:] { if err := flagSet.Parse([]string{arg}); err != nil { - // Simply log errors that can happen, such as parsing unexpected flags. We want this to be silent + // Simply log errors that can happen, such as parsing unexpected flags. We want this to be silent, // and we won't want to stop the execution. log.Errorf("could not parse flagSet: %v", err) }