Update linter

.golangci.yml

@@ -19,15 +19,14 @@ linters:
     - gocritic
     - gofmt
     - goimports
-    - golint
     - gosec
     - govet
     - lll
-    - maligned
     - megacheck
     - misspell
     - nakedret
     - prealloc
+    - revive
     - stylecheck
     - unconvert
     - unparam

bin/style

@@ -1,62 +1,59 @@
-#!/usr/bin/env ruby
-
-require 'fileutils'
-require 'open3'
-require 'open-uri'
-
-class Style
-  class << self
-    INSTALLER_URL = 'https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh'
-    EXECUTABLE = 'golangci-lint'
-    VERSION = '1.49.0'
-    VERSION_TAG = "v#{VERSION}"
-
-    def call
-      install unless up_to_date?
-      run(*ARGV)
-    end
-
-    private
-
-    def install_path
-      gopath = ENV.fetch('GOPATH', ENV.fetch('HOME')).split(':').first
-      File.join(gopath, 'bin')
-    end
-
-    def bin_path
-      File.join(install_path, EXECUTABLE)
-    end
-
-    def exist?
-      File.executable?(bin_path)
-    end
-
-    def up_to_date?
-      return false unless exist?
-      out, stat = Open3.capture2(bin_path, '--version')
-      raise("Unable to check version of #{bin_path}") unless stat.success?
-      out.match(/\b#{VERSION}\b/)
-    end
-
-    def install
-      puts "Installing #{EXECUTABLE}..."
-
-      open(INSTALLER_URL) do |installer|
-        Open3.popen2('sh', '-s', '--', '-b', install_path, VERSION_TAG) do |stdin, out, stat|
-          IO.copy_stream(installer, stdin)
-          stdin.close
-          print out.read
-
-          raise("failed to install #{EXECUTABLE} #{VERSION_TAG}") unless stat.value.success?
-        end
-      end
-    end
-
-    def run(*argv)
-      config = ENV.fetch('GOLANGCI_CONFIG', '.golangci.yml')
-      exec(bin_path, 'run', '--config', config, *argv)
-    end
-  end
-end
-
-__FILE__ == $PROGRAM_NAME and Style.call
+#!/bin/sh
+
+# This script is intentionally written to be POSIX compliant to be as portable as possible
+
+set -e
+
+VERSION="1.51.1"
+VERSION_TAG="v${VERSION}"
+
+INSTALLER_URL="github.com/golangci/golangci-lint/cmd/golangci-lint"
+EXECUTABLE="$(basename "${INSTALLER_URL}")"
+
+INSTALL_PATH="${HOME}/.local/share/${EXECUTABLE}/${VERSION}"
+EXECUTABLE_PATH="${INSTALL_PATH}/${EXECUTABLE}" # e.g. $HOME/.local/share/golangci/1.32.0/golangci-lint
+
+GOPATH="${GOPATH:-${HOME}}"
+GOPATH_PRIMARY="${GOPATH%%:*}" # Delete :* from the end, yielding the first path
+BIN_INSTALL_PATH="${GOPATH_PRIMARY}/bin"
+BIN_EXECUTABLE_PATH="${BIN_INSTALL_PATH}/${EXECUTABLE}" # e.g. $HOME/bin/golangci-lint
+
+installed() {
+    [ -x "${EXECUTABLE_PATH}" ]
+}
+
+install() {
+    echo "Installing ${EXECUTABLE} version ${VERSION}" >&2
+
+    mkdir -p "${INSTALL_PATH}"
+    GOBIN="${INSTALL_PATH}" go install "${INSTALLER_URL}@${VERSION_TAG}"
+}
+
+linked() {
+    [ -L "${BIN_EXECUTABLE_PATH}" ] && [ "$(readlink "${BIN_EXECUTABLE_PATH}")" = "${EXECUTABLE_PATH}" ]
+}
+
+link() {
+    mkdir -p "${BIN_INSTALL_PATH}"
+    rm -fv "${BIN_EXECUTABLE_PATH}"
+    ln -sfv "${EXECUTABLE_PATH}" "${BIN_EXECUTABLE_PATH}"
+}
+
+case "$1" in
+    "--installed")
+        installed
+        ;;
+    "--install")
+        installed || install
+        ;;
+    "--linked")
+        installed && linked
+        ;;
+    "--link")
+        (installed || install) && (linked || link)
+        ;;
+    *)
+        installed || install
+        exec "${EXECUTABLE_PATH}" "$@"
+        ;;
+esac

dev.yml

@@ -11,7 +11,7 @@ commands:
     godoc -http=:6060
   test: go test -race ./...
   style:
-    run: bin/style
-    desc: Static verification using gometalinter or autofix issues when possible.
+    run: bin/style run
+    desc: Static verification using golangci or autofix issues when possible.
     syntax:
       optional: --fix

logger/loggable.go

@@ -7,10 +7,10 @@ import (
 )
 
 // This create a private key-space in the Context, meaning that only this package can get or set "contextKey" types
-type contextKey int
+type contextKey struct{}
 
-const (
-	logFieldsKey contextKey = iota
+var (
+	logFieldsKey = contextKey{}
 )
 
 type Loggable interface {

logger/loggable_test.go

@@ -64,7 +64,7 @@ func ExampleWatchingLoggable() {
 func TestEmptyContext(t *testing.T) {
 	ctx := context.Background()
 	// Using a basic type on purpose, disable linter
-	ctx = context.WithValue(ctx, "a", "b") //nolint:golint,staticcheck
+	ctx = context.WithValue(ctx, "a", "b") //nolint:revive,staticcheck
 	// Not showing up in logs
 	checkData(ctx, t, logrus.Fields{"component": "testing"})
 }
@@ -116,19 +116,6 @@ func TestWithField(t *testing.T) {
 	})
 }
 
-func TestLoggableKeyClash(t *testing.T) {
-	ctx := context.Background()
-	ctx = WithField(ctx, "a", "b")
-
-	// logFieldsKey is an int declared as a contextKey, so trying to set an int shouldn't override the contextKey
-	// Using a basic type on purpose, disable linter
-	ctx = context.WithValue(ctx, int(logFieldsKey), "foo") //nolint:golint,staticcheck
-
-	checkData(ctx, t, logrus.Fields{
-		"a": "b",
-	})
-}
-
 func TestChildContext(t *testing.T) {
 	ctx := context.Background()
 	ctx = WithField(ctx, "a", "b")

logrusbugsnag/bugsnag_test.go

@@ -3,7 +3,7 @@ package logrusbugsnag
 import (
 	"bytes"
 	"errors"
-	"io/ioutil"
+	"io"
 	"net/http"
 	"sync"
 	"testing"
@@ -25,15 +25,15 @@ type nilRoundTripper struct{}
 
 func (rt *nilRoundTripper) RoundTrip(_ *http.Request) (*http.Response, error) {
 	return &http.Response{
-		Body:       ioutil.NopCloser(bytes.NewReader(nil)),
+		Body:       io.NopCloser(bytes.NewReader(nil)),
 		StatusCode: http.StatusOK,
 	}, nil
 }
 
 func setup(record bool) {
 	testOnce.Do(func() {
 		l := logrus.New()
-		l.Out = ioutil.Discard
+		l.Out = io.Discard
 
 		bugsnag.Configure(bugsnag.Configuration{
 			APIKey: testAPIKey,
@@ -57,7 +57,7 @@ func BenchmarkHook_Fire(b *testing.B) {
 	setup(false)
 
 	l := logrus.New()
-	l.Out = ioutil.Discard
+	l.Out = io.Discard
 
 	hook, err := NewBugsnagHook(nil)
 	assert.NoError(b, err)
@@ -89,7 +89,7 @@ func TestNewBugsnagHook(t *testing.T) {
 	setup(true)
 
 	l := logrus.New()
-	l.Out = ioutil.Discard
+	l.Out = io.Discard
 
 	hook, err := NewBugsnagHook(nil)
 	assert.NoError(t, err)

profiler/ui.go

@@ -3,7 +3,6 @@ package profiler
 import (
 	"context"
 	"errors"
-	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
 	httppprof "net/http/pprof"
@@ -19,7 +18,7 @@ func pprofUIHandler(w http.ResponseWriter, r *http.Request) {
 	ctx := r.Context()
 	vars := mux.Vars(r)
 
-	f, err := ioutil.TempFile("", "pprof.*.pb")
+	f, err := os.CreateTemp("", "pprof.*.pb")
 	if err != nil {
 		log(ctx, err).Error("error creating temporary file")
 		http.Error(w, err.Error(), http.StatusInternalServerError)

shell/shell_test.go

@@ -5,7 +5,6 @@ import (
 	"context"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"strings"
@@ -27,7 +26,7 @@ func ExampleSupervisor_Wait() {
 	stdin.Write([]byte("foo"))
 	stdin.Close()
 
-	output, _ := ioutil.ReadAll(stdout)
+	output, _ := io.ReadAll(stdout)
 
 	// Must call Wait _after_ interacting with pipes
 	cmd.Wait()
@@ -144,7 +143,7 @@ func TestCommandRunPipe(t *testing.T) {
 	assert.NotEqual(t, 0, c.Process.Pid) // But the process exists
 
 	// Calling ReadAll will wait for the pipe to close, so all the output is there.
-	output, err := ioutil.ReadAll(pipe)
+	output, err := io.ReadAll(pipe)
 	assert.NoError(t, err)
 	assert.Equal(t, []byte("foo"), output)
 
@@ -174,7 +173,7 @@ func TestCommandRunWaitPipeFails(t *testing.T) {
 	assert.True(t, c.ProcessState.Exited())
 
 	// Calling ReadAll will wait for the pipe to close, so all the output is there.
-	_, err = ioutil.ReadAll(pipe)
+	_, err = io.ReadAll(pipe)
 	assert.Error(t, err, "read |0: file already closed")
 }
 

srvutil/metrics_test.go

@@ -5,7 +5,7 @@ import (
 	"bytes"
 	"context"
 	"fmt"
-	"io/ioutil"
+	"io"
 	"net"
 	"net/http"
 	"net/http/httptest"
@@ -70,7 +70,7 @@ func TestRequestMetricsMiddleware(t *testing.T) {
 	res, err := http.DefaultClient.Do(req)
 	assert.NoError(t, err)
 	assert.Equal(t, http.StatusOK, res.StatusCode)
-	body, err := ioutil.ReadAll(res.Body)
+	body, err := io.ReadAll(res.Body)
 	assert.NoError(t, err)
 	assert.Equal(t, "hello world", string(body))
 

srvutil/server_test.go

@@ -4,7 +4,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"net/http"
 	"os"
 	"strings"
@@ -73,7 +72,7 @@ func TestNewServer(t *testing.T) {
 	res, err := http.Get(u)
 	assert.NoError(t, err)
 	assert.Equal(t, http.StatusOK, res.StatusCode)
-	body, err := ioutil.ReadAll(res.Body)
+	body, err := io.ReadAll(res.Body)
 	assert.NoError(t, err)
 	assert.Equal(t, "great success", string(body))
 
@@ -136,7 +135,7 @@ func TestNewServerFromFactory(t *testing.T) {
 	res, err := http.Get(u)
 	assert.NoError(t, err)
 	assert.Equal(t, http.StatusOK, res.StatusCode)
-	body, err := ioutil.ReadAll(res.Body)
+	body, err := io.ReadAll(res.Body)
 	assert.NoError(t, err)
 	assert.Equal(t, "great success", string(body))
 
@@ -214,7 +213,7 @@ func TestStoppableKeepaliveListener_Accept(t *testing.T) {
 		res, err := http.Get(u) // This will block on tb.Dying()
 		assert.NoError(t, err)
 		assert.Equal(t, http.StatusOK, res.StatusCode)
-		body, err := ioutil.ReadAll(res.Body)
+		body, err := io.ReadAll(res.Body)
 		assert.NoError(t, err)
 		assert.Equal(t, "great success", string(body))
 		close(done)

statsd/taggable.go

@@ -9,10 +9,10 @@ import (
 )
 
 // This create a private key-space in the Context, meaning that only this package can get or set "contextKey" types
-type contextKey int
+type contextKey struct{}
 
-const (
-	tagsKey contextKey = iota
+var (
+	tagsKey = contextKey{}
 )
 
 type Tags map[string]interface{}

statsd/taggable_test.go

@@ -91,7 +91,7 @@ func ExampleSelectKeys() {
 func TestEmptyContext(t *testing.T) {
 	ctx := context.Background()
 	// Using a basic type on purpose, disable linter
-	ctx = context.WithValue(ctx, "a", "b") //nolint:golint,staticcheck
+	ctx = context.WithValue(ctx, "a", "b") //nolint:revive,staticcheck
 	// Not showing up in tags
 	assert.Empty(t, getStatsTags(ctx))
 }
@@ -109,19 +109,6 @@ func TestWithTags(t *testing.T) {
 	}, getStatsTags(ctx))
 }
 
-func TestWithTags_keyClash(t *testing.T) {
-	ctx := context.Background()
-	ctx = WithTags(ctx, Tags{"a": "b"})
-
-	// tagsKey is an int declared as a contextKey, so trying to set an int shouldn't override the contextKey
-	// Using a basic type on purpose, disable linter
-	ctx = context.WithValue(ctx, int(tagsKey), "foo") //nolint:golint,staticcheck
-
-	assert.Equal(t, []string{
-		"a:b",
-	}, getStatsTags(ctx))
-}
-
 func TestChildContext(t *testing.T) {
 	ctx := context.Background()
 	ctx = WithTags(ctx, Tags{"a": "b"})