cleanup Go linter settings (#16736)

.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/

.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:'

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

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()
 }

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")
 	}

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)

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)
 						}

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 {

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)
 	}
 }

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
 }

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

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()

staticcheck.conf

@@ -1 +0,0 @@
-checks = ["all", "-ST1005", "-ST1000", "-SA4000", "-SA9004", "-SA1019", "-SA1008", "-U1000", "-ST1016"]