Update golangci-lint and fix various things (#323)

* Update golangci-lint and fix various things

* oops

* More things

.github/workflows/golangci-lint.yml

@@ -14,9 +14,9 @@ jobs:
       - uses: actions/checkout@v4
       - uses: actions/setup-go@v4
         with:
-          go-version: '^1.22'
+          go-version: '^1.23'
       - name: golangci-lint
         uses: golangci/golangci-lint-action@v4
         with:
-          version: v1.57.1
+          version: v1.61.0
           args: cli/... discern/... elan/... flair/... grpcutil/... lucidity/... mettle/... purity/... rexclient/... zeal/...

.golangci.yml

@@ -1,8 +1,5 @@
 run:
   timeout: 5m
-  skip-dirs:
-    - plz-out/
-    - tests/
 
 issues:
   exclude:
@@ -18,6 +15,9 @@ issues:
     - unused-parameter # Quite a few of these, some legit, many are fulfilling interfaces
     - redefines-builtin # Not really a big issue
     - empty-block # Only came up once and was a false positive. This should be easily handled by review.
+  exclude-dirs:
+    - plz-out/
+    - tests/
   exclude-rules:
     - path: _test\.go
       linters:
@@ -36,7 +36,7 @@ linters:
     - bodyclose
     - dogsled
     - dupl
-    - exportloopref
+    - copyloopvar
     - gocritic
     - gofmt
     - revive

elan/rpc/gc.go

@@ -20,7 +20,7 @@ import (
 
 func (s *server) List(ctx context.Context, req *ppb.ListRequest) (*ppb.ListResponse, error) {
 	if len(req.Prefix) != 2 {
-		return nil, status.Errorf(codes.InvalidArgument, "Invalid prefix provided: "+req.Prefix)
+		return nil, status.Errorf(codes.InvalidArgument, "Invalid prefix provided: %s", req.Prefix)
 	}
 	var g multierror.Group
 	resp := &ppb.ListResponse{}

elan/rpc/tree.go

@@ -35,7 +35,6 @@ func (s *server) getTree(digest *pb.Digest, stopAtPack bool) ([]*pb.Directory, e
 	var g multierror.Group
 	var mutex sync.Mutex
 	for _, child := range dir.Directories {
-		child := child
 		g.Go(func() error {
 			dirs, err := s.getTree(child.Digest, stopAtPack)
 			mutex.Lock()

flair/rpc/rpc.go

@@ -181,8 +181,6 @@ func (s *server) FindMissingBlobs(ctx context.Context, req *pb.FindMissingBlobsR
 	var g errgroup.Group
 	var mutex sync.Mutex
 	for srv, b := range blobs {
-		srv := srv
-		b := b
 		g.Go(func() error {
 			var mutex2 sync.Mutex
 			found := make(map[string]struct{}, len(b))
@@ -235,8 +233,6 @@ func (s *server) BatchUpdateBlobs(ctx context.Context, req *pb.BatchUpdateBlobsR
 	var mutex sync.Mutex
 	m := make(map[string][]*pb.BatchUpdateBlobsResponse_Response, len(blobs)*s.replicator.Replicas)
 	for srv, rs := range blobs {
-		srv := srv
-		rs := rs
 		g.Go(func() error {
 			return s.replicator.Parallel(srv.Start, func(srv *trie.Server) error {
 				ctx, cancel := context.WithTimeout(ctx, s.timeout)
@@ -297,8 +293,6 @@ func (s *server) BatchReadBlobs(ctx context.Context, req *pb.BatchReadBlobsReque
 	}()
 
 	for srv, d := range blobs {
-		srv := srv
-		d := d
 		g.Go(func() error {
 			return s.replicator.SequentialAck(srv.Start, func(s2 *trie.Server) (bool, error) {
 				ctx, cancel := context.WithTimeout(ctx, s.timeout)
@@ -367,7 +361,7 @@ func (s *server) GetTree(req *pb.GetTreeRequest, srv pb.ContentAddressableStorag
 			} else if len(r.Responses) != 1 {
 				return fmt.Errorf("missing blob in response") // shouldn't happen...
 			} else if s := r.Responses[0].Status; s.Code != int32(codes.OK) {
-				return status.Errorf(codes.Code(s.Code), s.Message)
+				return status.Errorf(codes.Code(s.Code), "%s", s.Message)
 			}
 			resp = r
 			return nil
@@ -610,7 +604,7 @@ func (s *server) forwardMetadata(ctx context.Context) context.Context {
 
 func (s *server) List(ctx context.Context, req *ppb.ListRequest) (*ppb.ListResponse, error) {
 	if len(req.Prefix) != 2 {
-		return nil, status.Errorf(codes.InvalidArgument, "Invalid prefix provided: "+req.Prefix)
+		return nil, status.Errorf(codes.InvalidArgument, "Invalid prefix provided: %s", req.Prefix)
 	}
 	var mutex sync.Mutex
 	resp := &ppb.ListResponse{}
@@ -672,7 +666,7 @@ func (s *server) List(ctx context.Context, req *ppb.ListRequest) (*ppb.ListRespo
 
 func (s *server) Delete(ctx context.Context, req *ppb.DeleteRequest) (*ppb.DeleteResponse, error) {
 	if len(req.Prefix) != 2 {
-		return nil, status.Errorf(codes.InvalidArgument, "Invalid prefix provided: "+req.Prefix)
+		return nil, status.Errorf(codes.InvalidArgument, "Invalid prefix provided: %s", req.Prefix)
 	}
 	return &ppb.DeleteResponse{}, s.replicator.All(req.Prefix, func(srv *trie.Server) error {
 		ctx, cancel := context.WithTimeout(ctx, 10*s.timeout) // Multiply up timeout since this operation can be expensive.

flair/trie/replication_test.go

@@ -158,7 +158,7 @@ func TestErrorCode(t *testing.T) {
 		t.Run(fmt.Sprintf("%s_%s", tc.Output, tc.Inputs), func(t *testing.T) {
 			var merr *multierror.Error
 			for _, input := range tc.Inputs {
-				merr = multierror.Append(merr, status.Errorf(input, input.String()))
+				merr = multierror.Append(merr, status.Errorf(input, "%s", input))
 			}
 			assert.Equal(t, tc.Output, errorCode(merr))
 		})

mettle/worker/download.go

@@ -187,8 +187,6 @@ func (w *worker) downloadAllFiles(files map[sdkdigest.Digest][]fileNode, packs m
 		g.Go(func() error { return w.downloadFiles(fileNodes) })
 	}
 	for dg, paths := range packs {
-		dg := dg
-		paths := paths
 		g.Go(func() error {
 			return w.downloadPack(dg, paths)
 		})

mettle/worker/reporting.go

@@ -173,15 +173,21 @@ func (w *worker) checkLiveConnection() bool {
 func (w *worker) checkConnectivity(check string) {
 	switch check {
 	case "gstatic":
-		if resp, err := http.Get("https://connectivitycheck.gstatic.com/generate_204"); err != nil {
+		resp, err := http.Get("https://connectivitycheck.gstatic.com/generate_204")
+		if err != nil {
 			log.Fatalf("Failed to complete connectivity check: %s", err)
-		} else if resp.StatusCode != http.StatusNoContent {
+		}
+		defer resp.Body.Close()
+		if resp.StatusCode != http.StatusNoContent {
 			log.Fatalf("Connectivity check returned unexpected status: %s", resp.Status)
 		}
 	case "firefox":
-		if resp, err := http.Get("https://detectportal.firefox.com/canonical.html"); err != nil {
+		resp, err := http.Get("https://detectportal.firefox.com/canonical.html")
+		if err != nil {
 			log.Fatalf("Failed to complete connectivity check: %s", err)
-		} else if resp.StatusCode != http.StatusOK {
+		}
+		defer resp.Body.Close()
+		if resp.StatusCode != http.StatusOK {
 			log.Fatalf("Connectivity check returned unexpected status: %s", resp.Status)
 		}
 	case "":

mettle/worker/worker.go

@@ -242,7 +242,7 @@ func runForever(instanceName, requestQueue, responseQueue, name, storage, dir, c
 			// our queues or something else bad happened internally  so we are basically doomed
 			// and should stop.
 			err = fmt.Errorf("Failed to run task: %s", err)
-			w.Report(false, false, false, err.Error())
+			w.Report(false, false, false, "%s", err)
 			return err
 		}
 	}
@@ -533,7 +533,7 @@ func (w *worker) runTask(msg *pubsub.Message) *pb.ExecuteResponse {
 
 // forceShutdown sends any shutdown reports and calls log.Fatal() to shut down the worker
 func (w *worker) forceShutdown(shutdownMsg string) {
-	w.Report(false, false, false, shutdownMsg)
+	w.Report(false, false, false, "%s", shutdownMsg)
 	log.Info("Force shutting down worker")
 	if w.currentMsg != nil {
 		if w.actionDigest != nil {
@@ -901,7 +901,7 @@ func (w *worker) writeUncachedResult(ar *pb.ActionResult, msg string) string {
 	b, _ := proto.Marshal(&bbcas.UncachedActionResult{
 		ActionDigest: w.actionDigest,
 		ExecuteResponse: &pb.ExecuteResponse{
-			Status: status(nil, codes.Unknown, msg),
+			Status: status(nil, codes.Unknown, "%s", msg),
 			Result: ar,
 		},
 	})
@@ -1075,7 +1075,7 @@ func (w *worker) collectOutputs(ar *pb.ActionResult, cmd *pb.Command) error {
 
 // update sends an update on the response channel
 func (w *worker) update(stage pb.ExecutionStage_Value, response *pb.ExecuteResponse) error {
-	w.Report(true, stage == pb.ExecutionStage_EXECUTING, true, stage.String())
+	w.Report(true, stage == pb.ExecutionStage_EXECUTING, true, "%s", stage)
 	body := common.MarshalOperation(stage, w.actionDigest, response, w.metadata)
 	ctx, cancel := context.WithTimeout(context.Background(), 1*time.Minute)
 	defer cancel()

purity/gc/gc.go

@@ -182,7 +182,6 @@ func (c *collector) LoadAllBlobs() error {
 	var g multierror.Group
 	var mutex sync.Mutex
 	for i := 0; i < 16; i++ {
-		i := i
 		g.Go(func() error {
 			for j := 0; j < 16; j++ {
 				ctx, cancel := context.WithTimeout(context.Background(), 30*time.Minute)