Merge pull request #701 from google/lint-fixes

Update to quiet the linter

.github/workflows/golangci-lint.yml

@@ -22,7 +22,7 @@ jobs:
       - uses: golangci/golangci-lint-action@v3
         with:
           # Required: the version of golangci-lint is required and must be specified without patch version: we always use the latest patch version.
-          version: v1.41
+          version: v1.50
 
           # Optional: show only new issues if it's a pull request. The default value is `false`.
           only-new-issues: true

.golangci.yml

@@ -9,6 +9,11 @@ run:
 linters-settings:
   govet:
     check-shadowing: true
+    disable:
+      composites # same as exhaustruct below
+  asasalint:
+    exclude:
+      - glog\.Infof
 
 linters:
   presets:
@@ -31,6 +36,7 @@ linters:
     - errcheck # handled by gosec, lots of false posi
     - exhaustive # this false-positives for switches with a default
     - exhaustivestruct # too noisy, labelling fields is not my jam
+    - exhaustruct # above, renamed
     - forbidigo # exclude non prod tools
     - forcetypeassert # too many at the moment
     - funlen    # My tests will be as long as they need to be thanks

Makefile

@@ -90,10 +90,6 @@ GOSEC = $(GOBIN)/gosec
 $(GOSEC):
 	go install github.com/securego/gosec/v2/cmd/gosec@latest
 
-GOLANGCILINT = $(GOBIN)/golangci-lint
-$(GOLANGCILINT):
-	go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest
-
 
 .PHONY: clean covclean crossclean depclean veryclean
 clean: covclean crossclean
@@ -106,10 +102,12 @@ depclean:
 	rm -f .d/*  .*dep-stamp
 veryclean: clean depclean
 
+GOLANGCILINT_VERSION=v1.50.1
 # lint
 .PHONY: lint
-lint:  $(GOFILES) $(GOGENFILES) $(GOTESTFILES) | $(GOLANGCILINT)
-	$(GOLANGCILINT) run ./...
+lint:  $(GOFILES) $(GOGENFILES) $(GOTESTFILES)
+	mkdir -p $(HOME)/.cache/golangci-lint/$(GOLANGCILINT_VERSION)
+	podman run --rm -v $(shell pwd):/app -v $(HOME)/.cache/golangci-lint/$(GOLANGCILINT_VERSION):/root/.cache -w /app golangci/golangci-lint:$(GOLANGCILINT_VERSION) golangci-lint run -v
 
 branch := $(shell git rev-parse --abbrev-ref HEAD)
 version := $(shell git describe --tags --always --dirty)

cmd/mdot/main.go

@@ -6,11 +6,11 @@ Command mdot turns an mtail program AST into a graphviz graph on standard output
 
 To use, run it like (assuming your shell is in the same directory as this file)
 
-  go run github.com/google/mtail/cmd/mdot --prog ../../examples/dhcpd.mtail | xdot -
+	go run github.com/google/mtail/cmd/mdot --prog ../../examples/dhcpd.mtail | xdot -
 
 or
 
-  go run github.com/google/mtail/cmd/mdot --prog ../../examples/dhcpd.mtail --http_port 8080
+	go run github.com/google/mtail/cmd/mdot --prog ../../examples/dhcpd.mtail --http_port 8080
 
 to view the dot output visit http://localhost:8080
 

internal/exporter/export.go

@@ -179,7 +179,7 @@ func (e *Exporter) writeSocketMetrics(c io.Writer, f formatter, exportTotal *exp
 			if err == nil {
 				exportSuccess.Add(1)
 			} else {
-				return errors.Errorf("write error: %s\n", err)
+				return errors.Errorf("write error: %s", err)
 			}
 		}
 		m.RUnlock()

internal/exporter/graphite_test.go

@@ -5,7 +5,7 @@ package exporter
 
 import (
 	"context"
-	"io/ioutil"
+	"io"
 	"net/http"
 	"net/http/httptest"
 	"sync"
@@ -59,7 +59,7 @@ func TestHandleGraphite(t *testing.T) {
 			if response.Code != 200 {
 				t.Errorf("response code not 200: %d", response.Code)
 			}
-			b, err := ioutil.ReadAll(response.Body)
+			b, err := io.ReadAll(response.Body)
 			if err != nil {
 				t.Errorf("failed to read response %s", err)
 			}

internal/exporter/json_test.go

@@ -5,7 +5,7 @@ package exporter
 
 import (
 	"context"
-	"io/ioutil"
+	"io"
 	"math"
 	"net/http"
 	"net/http/httptest"
@@ -153,7 +153,7 @@ func TestHandleJSON(t *testing.T) {
 			if response.Code != 200 {
 				t.Errorf("response code not 200: %d", response.Code)
 			}
-			b, err := ioutil.ReadAll(response.Body)
+			b, err := io.ReadAll(response.Body)
 			if err != nil {
 				t.Errorf("failed to read response: %s", err)
 			}

internal/exporter/varz_test.go

@@ -5,7 +5,7 @@ package exporter
 
 import (
 	"context"
-	"io/ioutil"
+	"io"
 	"net/http"
 	"net/http/httptest"
 	"sync"
@@ -86,7 +86,7 @@ func TestHandleVarz(t *testing.T) {
 			if response.Code != 200 {
 				t.Errorf("response code not 200: %d", response.Code)
 			}
-			b, err := ioutil.ReadAll(response.Body)
+			b, err := io.ReadAll(response.Body)
 			if err != nil {
 				t.Errorf("failed to read response: %s", err)
 			}

internal/metrics/store.go

@@ -40,7 +40,7 @@ func (s *Store) Add(m *Metric) error {
 		t := s.Metrics[m.Name][0].Kind
 		if m.Kind != t {
 			s.searchMu.RUnlock()
-			return errors.Errorf("Metric %s has different kind %v to existing %v.", m.Name, m.Kind, t)
+			return errors.Errorf("metric %s has different kind %v to existing %v", m.Name, m.Kind, t)
 		}
 
 		// To avoid duplicate metrics:

internal/metrics/store_test.go

@@ -63,11 +63,9 @@ func TestDuplicateMetric(t *testing.T) {
 	}
 }
 
-/* A program can add a metric with the same name and
-   of different type.
-   Prometheus behavior in this case is undefined.
-   @see https://github.com/google/mtail/issues/130
-*/
+// A program can add a metric with the same name and of different type.
+// Prometheus behavior in this case is undefined.  @see
+// https://github.com/google/mtail/issues/130
 func TestAddMetricDifferentType(t *testing.T) {
 	expected := 2
 	s := NewStore()

internal/mtail/compile_only_integration_test.go

@@ -5,7 +5,7 @@ package mtail_test
 
 import (
 	"context"
-	"io/ioutil"
+	"os"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -19,7 +19,7 @@ func TestBadProgramFailsCompilation(t *testing.T) {
 	testutil.SkipIfShort(t)
 	progDir := testutil.TestTempDir(t)
 
-	err := ioutil.WriteFile(filepath.Join(progDir, "bad.mtail"), []byte("asdfasdf\n"), 0o666)
+	err := os.WriteFile(filepath.Join(progDir, "bad.mtail"), []byte("asdfasdf\n"), 0o666)
 	testutil.FatalIfErr(t, err)
 
 	ctx := context.Background()

internal/runtime/runtime.go

@@ -11,7 +11,6 @@ import (
 	"crypto/sha256"
 	"expvar"
 	"io"
-	"io/ioutil"
 	"os"
 	"os/signal"
 	"path/filepath"
@@ -58,7 +57,7 @@ func (r *Runtime) LoadAllPrograms() error {
 	}
 	switch {
 	case s.IsDir():
-		fis, rerr := ioutil.ReadDir(r.programPath)
+		dirents, rerr := os.ReadDir(r.programPath)
 		if rerr != nil {
 			return errors.Wrapf(rerr, "Failed to list programs in %q", r.programPath)
 		}
@@ -70,19 +69,19 @@ func (r *Runtime) LoadAllPrograms() error {
 			markDeleted[name] = struct{}{}
 		}
 		r.handleMu.RUnlock()
-		for _, fi := range fis {
-			if fi.IsDir() {
+		for _, dirent := range dirents {
+			if dirent.IsDir() {
 				continue
 			}
-			err = r.LoadProgram(filepath.Join(r.programPath, fi.Name()))
+			err = r.LoadProgram(filepath.Join(r.programPath, dirent.Name()))
 			if err != nil {
 				if r.errorsAbort {
 					return err
 				}
 				glog.Warning(err)
 			}
-			glog.Infof("unmarking %s", filepath.Base(fi.Name()))
-			delete(markDeleted, filepath.Base(fi.Name()))
+			glog.Infof("unmarking %s", filepath.Base(dirent.Name()))
+			delete(markDeleted, filepath.Base(dirent.Name()))
 		}
 		for name := range markDeleted {
 			glog.Infof("unloading %s", name)
@@ -163,7 +162,7 @@ func (r *Runtime) CompileAndRun(name string, input io.Reader) error {
 	}
 	if obj == nil {
 		ProgLoadErrors.Add(name, 1)
-		return errors.Errorf("Internal error: Compilation failed for %s: No program returned, but no errors.", name)
+		return errors.Errorf("internal error: compilation failed for %s: no program returned, but no errors", name)
 	}
 	v := vm.New(name, obj, r.syslogUseCurrentYear, r.overrideLocation, r.logRuntimeErrors, r.trace)
 

internal/tailer/logstream/filestream_unix_test.go

@@ -123,6 +123,7 @@ func TestFileStreamOpenFailure(t *testing.T) {
 
 	name := filepath.Join(tmpDir, "log")
 	f, err := os.OpenFile(name, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0)
+	//nolint:staticcheck // test code
 	defer f.Close()
 
 	testutil.FatalIfErr(t, err)

internal/tailer/tail_unix_test.go

@@ -45,6 +45,7 @@ func TestTailerOpenRetries(t *testing.T) {
 	testutil.FatalIfErr(t, ta.PollLogStreamsForCompletion())
 	glog.Info("openfile")
 	f, err := os.OpenFile(logfile, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0)
+	//nolint:staticcheck // test code
 	defer f.Close()
 	testutil.FatalIfErr(t, err)
 	testutil.FatalIfErr(t, ta.PollLogPatterns())

internal/testutil/fs.go

@@ -4,7 +4,6 @@
 package testutil
 
 import (
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"testing"
@@ -13,7 +12,7 @@ import (
 // TestTempDir creates a temporary directory for use during tests, returning the pathname.
 func TestTempDir(tb testing.TB) string {
 	tb.Helper()
-	name, err := ioutil.TempDir("", "mtail-test")
+	name, err := os.MkdirTemp("", "mtail-test")
 	if err != nil {
 		tb.Fatal(err)
 	}