From 3423028713b0999056c401fef00a07994d5981da Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Sun, 5 Mar 2023 05:57:35 +0100 Subject: [PATCH] cleanup Go linter settings (#16736) --- .gitignore | 1 + .golangci.yml | 48 ++++++++++++++------------- Makefile | 10 +++--- cmd/admin-handlers-users-race_test.go | 27 ++++++++------- cmd/admin-handlers-users_test.go | 2 +- cmd/erasure-healing_test.go | 4 +-- cmd/local-locker_test.go | 2 +- cmd/untar.go | 2 +- cmd/xl-storage-format-v2.go | 2 +- internal/lsync/lrwmutex_test.go | 4 +-- internal/s3select/csv/reader.go | 4 +-- internal/s3select/json/preader.go | 4 +-- staticcheck.conf | 1 - 13 files changed, 59 insertions(+), 52 deletions(-) delete mode 100644 staticcheck.conf diff --git a/.gitignore b/.gitignore index 8b7ede6b3..476d80b73 100644 --- a/.gitignore +++ b/.gitignore @@ -38,3 +38,4 @@ docs/debugging/s3-check-md5/s3-check-md5 docs/debugging/hash-set/hash-set docs/debugging/healing-bin/healing-bin docs/debugging/inspect/inspect +.bin/ diff --git a/.golangci.yml b/.golangci.yml index 53d567882..c548a0576 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,39 +1,41 @@ linters-settings: gofumpt: - lang-version: "1.18" + lang-version: '1.19' misspell: locale: US + staticcheck: + go: '1.19' + checks: ['all', '-ST1005', '-ST1000', '-SA4000', '-SA9004', '-SA1019', '-SA1008', '-U1000', '-ST1016'] + linters: disable-all: true enable: - - typecheck - - goimports - - misspell - - govet - - revive - - ineffassign - - gomodguard + - durationcheck + - gocritic - gofmt + - gofumpt + - goimports + - gomodguard + - govet + - ineffassign + - misspell + - revive + - staticcheck + - tenv + - typecheck - unconvert - unused - - gocritic - - gofumpt - - tenv - - durationcheck issues: exclude-use-default: false exclude: - - should have a package comment - - error strings should not be capitalized or end with punctuation or a newline - # todo fix these when we get enough time. - - "singleCaseSwitch: should rewrite switch statement to if statement" - - "unlambda: replace" - - "captLocal:" - - "ifElseChain:" - - "elseif:" - -service: - golangci-lint-version: 1.43.0 # use the fixed version to not introduce new linters unexpectedly + - should have a package comment + - error strings should not be capitalized or end with punctuation or a newline + # todo fix these when we get enough time. + - 'singleCaseSwitch: should rewrite switch statement to if statement' + - 'unlambda: replace' + - 'captLocal:' + - 'ifElseChain:' + - 'elseif:' diff --git a/Makefile b/Makefile index 6edbe1be5..525a43988 100644 --- a/Makefile +++ b/Makefile @@ -8,6 +8,10 @@ GOOS := $(shell go env GOOS) VERSION ?= $(shell git describe --tags) TAG ?= "minio/minio:$(VERSION)" +GOLANGCI_VERSION = v1.51.2 +GOLANGCI_DIR = .bin/golangci/$(GOLANGCI_VERSION) +GOLANGCI = $(GOLANGCI_DIR)/golangci-lint + all: build checks: ## check dependencies @@ -19,10 +23,9 @@ help: ## print this help getdeps: ## fetch necessary dependencies @mkdir -p ${GOPATH}/bin - @echo "Installing golangci-lint" && curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(GOPATH)/bin + @echo "Installing golangci-lint" && curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(GOLANGCI_DIR) $(GOLANGCI_VERSION) @echo "Installing msgp" && go install -v github.com/tinylib/msgp@v1.1.7 @echo "Installing stringer" && go install -v golang.org/x/tools/cmd/stringer@latest - @echo "Installing staticcheck" && go install honnef.co/go/tools/cmd/staticcheck@latest crosscompile: ## cross compile minio @(env bash $(PWD)/buildscripts/cross-compile.sh) @@ -35,8 +38,7 @@ check-gen: ## check for updated autogenerated files lint: ## runs golangci-lint suite of linters @echo "Running $@ check" - @${GOPATH}/bin/golangci-lint run --build-tags kqueue --timeout=10m --config ./.golangci.yml - @${GOPATH}/bin/staticcheck --tests=false ./... + @$(GOLANGCI) run --build-tags kqueue --timeout=10m --config ./.golangci.yml check: test test: verifiers build ## builds minio, runs linters, tests diff --git a/cmd/admin-handlers-users-race_test.go b/cmd/admin-handlers-users-race_test.go index fb7438b67..a1d4b8b61 100644 --- a/cmd/admin-handlers-users-race_test.go +++ b/cmd/admin-handlers-users-race_test.go @@ -27,12 +27,12 @@ import ( "context" "fmt" "runtime" - "sync" "testing" "time" "github.com/minio/madmin-go/v2" minio "github.com/minio/minio-go/v7" + "github.com/minio/minio/internal/sync/errgroup" ) func runAllIAMConcurrencyTests(suite *TestSuiteIAM, c *check) { @@ -129,18 +129,21 @@ func (s *TestSuiteIAM) TestDeleteUserRace(c *check) { secretKeys[i] = secretKey } - wg := sync.WaitGroup{} + g := errgroup.Group{} for i := 0; i < userCount; i++ { - wg.Add(1) - go func(i int) { - defer wg.Done() - uClient := s.getUserClient(c, accessKeys[i], secretKeys[i], "") - err := s.adm.RemoveUser(ctx, accessKeys[i]) - if err != nil { - c.Fatalf("unable to remove user: %v", err) + g.Go(func(i int) func() error { + return func() error { + uClient := s.getUserClient(c, accessKeys[i], secretKeys[i], "") + err := s.adm.RemoveUser(ctx, accessKeys[i]) + if err != nil { + return err + } + c.mustNotListObjects(ctx, uClient, bucket) + return nil } - c.mustNotListObjects(ctx, uClient, bucket) - }(i) + }(i), i) + } + if errs := g.Wait(); len(errs) > 0 { + c.Fatalf("unable to remove users: %v", errs) } - wg.Wait() } diff --git a/cmd/admin-handlers-users_test.go b/cmd/admin-handlers-users_test.go index 3e3bce3c7..78f42cdc6 100644 --- a/cmd/admin-handlers-users_test.go +++ b/cmd/admin-handlers-users_test.go @@ -825,7 +825,7 @@ func (s *TestSuiteIAM) TestGroupAddRemove(c *check) { if set.CreateStringSet(groups...).Contains(group) { c.Fatalf("created group still present!") } - groupInfo, err = s.adm.GetGroupDescription(ctx, group) + _, err = s.adm.GetGroupDescription(ctx, group) if err == nil { c.Fatalf("group appears to exist") } diff --git a/cmd/erasure-healing_test.go b/cmd/erasure-healing_test.go index b843acc7f..ff60da72b 100644 --- a/cmd/erasure-healing_test.go +++ b/cmd/erasure-healing_test.go @@ -1621,10 +1621,10 @@ func TestHealLastDataShard(t *testing.T) { } firstGr, err := obj.GetObjectNInfo(ctx, bucket, object, nil, nil, noLock, ObjectOptions{}) - defer firstGr.Close() if err != nil { t.Fatal(err) } + defer firstGr.Close() firstHealedH := sha256.New() _, err = io.Copy(firstHealedH, firstGr) @@ -1651,10 +1651,10 @@ func TestHealLastDataShard(t *testing.T) { } secondGr, err := obj.GetObjectNInfo(ctx, bucket, object, nil, nil, noLock, ObjectOptions{}) - defer secondGr.Close() if err != nil { t.Fatal(err) } + defer secondGr.Close() secondHealedH := sha256.New() _, err = io.Copy(secondHealedH, secondGr) diff --git a/cmd/local-locker_test.go b/cmd/local-locker_test.go index a8f0ea651..06d7d6864 100644 --- a/cmd/local-locker_test.go +++ b/cmd/local-locker_test.go @@ -419,7 +419,7 @@ func Test_localLocker_RUnlock(t *testing.T) { } start = time.Now() for _, lock := range toUnLock { - ok, err := l.RUnlock(nil, lock) + ok, err := l.RUnlock(context.TODO(), lock) if err != nil || !ok { t.Fatal(err) } diff --git a/cmd/untar.go b/cmd/untar.go index f46a51874..49d0b3962 100644 --- a/cmd/untar.go +++ b/cmd/untar.go @@ -243,7 +243,7 @@ func untar(ctx context.Context, r io.Reader, putObject func(reader io.Reader, in rc.Close() <-asyncWriters wg.Done() - //lint:ignore SA6002 we are fine with the tiny alloc + //nolint:staticcheck // SA6002 we are fine with the tiny alloc poolBuf128k.Put(b) }() if err := putObject(&rc, fi, name); err != nil { diff --git a/cmd/xl-storage-format-v2.go b/cmd/xl-storage-format-v2.go index 3d3ce0a3c..8d2dca131 100644 --- a/cmd/xl-storage-format-v2.go +++ b/cmd/xl-storage-format-v2.go @@ -678,7 +678,7 @@ func metaDataPoolGet() []byte { // metaDataPoolPut will put an unused small buffer back into the pool. func metaDataPoolPut(buf []byte) { if cap(buf) >= metaDataReadDefault && cap(buf) < metaDataReadDefault*4 { - //lint:ignore SA6002 we are fine with the tiny alloc + //nolint:staticcheck // SA6002 we are fine with the tiny alloc metaDataPool.Put(buf) } } diff --git a/internal/lsync/lrwmutex_test.go b/internal/lsync/lrwmutex_test.go index 8ad5eda7f..39bcebc4c 100644 --- a/internal/lsync/lrwmutex_test.go +++ b/internal/lsync/lrwmutex_test.go @@ -63,7 +63,7 @@ func testSimpleWriteLock(t *testing.T, duration time.Duration) (locked bool) { lrwm.Unlock() } else { - // fmt.Println("Write lock failed due to timeout") + t.Log("Write lock failed due to timeout") } return } @@ -109,7 +109,7 @@ func testDualWriteLock(t *testing.T, duration time.Duration) (locked bool) { lrwm.Unlock() } else { - // fmt.Println("2nd write lock failed due to timeout") + t.Log("2nd write lock failed due to timeout") } return } diff --git a/internal/s3select/csv/reader.go b/internal/s3select/csv/reader.go index b1afbacb5..e981a2131 100644 --- a/internal/s3select/csv/reader.go +++ b/internal/s3select/csv/reader.go @@ -69,7 +69,7 @@ func (r *Reader) Read(dst sql.Record) (sql.Record, error) { r.err = io.EOF return nil, r.err } - //lint:ignore SA6002 Using pointer would allocate more since we would have to copy slice header before taking a pointer. + //nolint:staticcheck // SA6002 Using pointer would allocate more since we would have to copy slice header before taking a pointer. r.csvDstPool.Put(r.current) r.current = <-item.dst r.err = item.err @@ -269,7 +269,7 @@ func (r *Reader) startReaders(newReader func(io.Reader) *csv.Reader) error { in.err = err } // We don't need the input any more. - //lint:ignore SA6002 Using pointer would allocate more since we would have to copy slice header before taking a pointer. + //nolint:staticcheck // SA6002 Using pointer would allocate more since we would have to copy slice header before taking a pointer. r.bufferPool.Put(in.input) in.input = nil in.dst <- all diff --git a/internal/s3select/json/preader.go b/internal/s3select/json/preader.go index b378ae0d2..fa2cf8481 100644 --- a/internal/s3select/json/preader.go +++ b/internal/s3select/json/preader.go @@ -66,7 +66,7 @@ func (r *PReader) Read(dst sql.Record) (sql.Record, error) { r.err = io.EOF return nil, r.err } - //lint:ignore SA6002 Using pointer would allocate more since we would have to copy slice header before taking a pointer. + //nolint:staticcheck // SA6002 Using pointer would allocate more since we would have to copy slice header before taking a pointer. r.kvDstPool.Put(r.current) r.current = <-item.dst r.err = item.err @@ -204,7 +204,7 @@ func (r *PReader) startReaders() { all = append(all, kvs) } // We don't need the input any more. - //lint:ignore SA6002 Using pointer would allocate more since we would have to copy slice header before taking a pointer. + //nolint:staticcheck // SA6002 Using pointer would allocate more since we would have to copy slice header before taking a pointer. r.bufferPool.Put(in.input) in.input = nil in.err = d.Err() diff --git a/staticcheck.conf b/staticcheck.conf deleted file mode 100644 index fec5f6f33..000000000 --- a/staticcheck.conf +++ /dev/null @@ -1 +0,0 @@ -checks = ["all", "-ST1005", "-ST1000", "-SA4000", "-SA9004", "-SA1019", "-SA1008", "-U1000", "-ST1016"]