Merge pull request #6086 from thaJeztah/golangci_tweaks

thaJeztah · web-flow · commit 4be9afb8018e · 2025-05-22T07:57:53.000+02:00
golangci-lint: enable more linters, and some minor linting fixes
diff --git a/.golangci.yml b/.golangci.yml
@@ -26,39 +26,49 @@ formatters:
 
 linters:
   enable:
+    - asasalint                 # Detects "[]any" used as argument for variadic "func(...any)".
     - bodyclose
-    - copyloopvar   # Detects places where loop variables are copied.
+    - copyloopvar               # Detects places where loop variables are copied.
     - depguard
-    - dogsled
-    - dupword       # Detects duplicate words.
-    - durationcheck
+    - dogsled                   # Detects assignments with too many blank identifiers.
+    - dupword                   # Detects duplicate words.
+    - durationcheck             # Detect cases where two time.Duration values are being multiplied in possibly erroneous ways.
     - errcheck
-    - errchkjson
+    - errchkjson                # Detects unsupported types passed to json encoding functions and reports if checks for the returned error can be omitted.
+    - exhaustive                # Detects missing options in enum switch statements.
+    - exptostd                  # Detects functions from golang.org/x/exp/ that can be replaced by std functions.
+    - fatcontext                # Detects nested contexts in loops and function literals.
     - forbidigo
-    - gocritic      # Metalinter; detects bugs, performance, and styling issues.
+    - gocheckcompilerdirectives # Detects invalid go compiler directive comments (//go:).
+    - gocritic                  # Metalinter; detects bugs, performance, and styling issues.
     - gocyclo
-    - gosec         # Detects security problems.
+    - gosec                     # Detects security problems.
     - govet
+    - iface                     # Detects incorrect use of interfaces. Currently only used for "identical" interfaces in the same package.
+    - importas                  # Enforces consistent import aliases.
     - ineffassign
-    - importas      # Enforces consistent import aliases.
-    - misspell      # Detects commonly misspelled English words in comments.
-    - nakedret      # Detects uses of naked returns.
-    - nilerr        # Detects code that returns nil even if it checks that the error is not nil.
-    - nolintlint    # Detects ill-formed or insufficient nolint directives.
-    - perfsprint    # Detects fmt.Sprintf uses that can be replaced with a faster alternative.
-    - prealloc      # Detects slice declarations that could potentially be pre-allocated.
-    - predeclared   # Detects code that shadows one of Go's predeclared identifiers
-    - reassign
-    - revive        # Metalinter; drop-in replacement for golint.
+    - makezero                  # Finds slice declarations with non-zero initial length.
+    - mirror                    # Detects wrong mirror patterns of bytes/strings usage.
+    - misspell                  # Detects commonly misspelled English words in comments.
+    - nakedret                  # Detects uses of naked returns.
+    - nilnesserr                # Detects returning nil errors. It combines the features of nilness and nilerr,
+    - nosprintfhostport         # Detects misuse of Sprintf to construct a host with port in a URL.
+    - nolintlint                # Detects ill-formed or insufficient nolint directives.
+    - perfsprint                # Detects fmt.Sprintf uses that can be replaced with a faster alternative.
+    - prealloc                  # Detects slice declarations that could potentially be pre-allocated.
+    - predeclared               # Detects code that shadows one of Go's predeclared identifiers
+    - reassign                  # Detects reassigning a top-level variable in another package.
+    - revive                    # Metalinter; drop-in replacement for golint.
+    - spancheck                 # Detects mistakes with OpenTelemetry/Census spans.
     - staticcheck
-    - thelper       # Detects test helpers without t.Helper().
-    - tparallel     # Detects inappropriate usage of t.Parallel().
-    - unconvert     # Detects unnecessary type conversions.
+    - thelper                   # Detects test helpers without t.Helper().
+    - tparallel                 # Detects inappropriate usage of t.Parallel().
+    - unconvert                 # Detects unnecessary type conversions.
     - unparam
     - unused
-    - usestdlibvars
-    - usetesting    # Reports uses of functions with replacement inside the testing package.
-    - wastedassign
+    - usestdlibvars             # Detects the possibility to use variables/constants from the Go standard library.
+    - usetesting                # Reports uses of functions with replacement inside the testing package.
+    - wastedassign              # Detects wasted assignment statements.
 
   disable:
     - errcheck
@@ -132,9 +142,7 @@ linters:
     staticcheck:
       checks:
         - all
-        - -QF1008 # Omit embedded fields from selector expression
-        - -ST1020 # The documentation of an exported function should start with the function’s name
-        - -ST1022 # The documentation of an exported variable or constant should start with variable’s name
+        - -QF1008 # Omit embedded fields from selector expression; https://staticcheck.dev/docs/checks/#QF1008
 
     revive:
       rules:
@@ -159,8 +167,8 @@ linters:
     # (unlike the "include" option), the "exclude" option does not take exclusion
     # ID's.
     #
-    # These exclusion patterns are copied from the default excluses at:
-    # https://github.com/golangci/golangci-lint/blob/v1.44.0/pkg/config/issues.go#L10-L104
+    # These exclusion patterns are copied from the default excludes at:
+    # https://github.com/golangci/golangci-lint/blob/v1.61.0/pkg/config/issues.go#L11-L104
     #
     # The default list of exclusions can be found at:
     # https://golangci-lint.run/usage/false-positives/#default-exclusions
@@ -207,6 +215,12 @@ linters:
         linters:
           - govet
 
+      # Ignore for cli/command/formatter/tabwriter, which is forked from go stdlib, so we want to align with it.
+      - text: '^(ST1020|ST1022): comment on exported'
+        path: "cli/command/formatter/tabwriter"
+        linters:
+          - staticcheck
+
     # Log a warning if an exclusion rule is unused.
     # Default: false
     warn-unused: true
diff --git a/cli-plugins/manager/manager.go b/cli-plugins/manager/manager.go
@@ -81,7 +81,7 @@ func addPluginCandidatesFromDir(res map[string][]string, d string) {
 		return
 	}
 	for _, dentry := range dentries {
-		switch dentry.Type() & os.ModeType {
+		switch dentry.Type() & os.ModeType { //nolint:exhaustive,nolintlint // no need to include all possible file-modes in this list
 		case 0, os.ModeSymlink:
 			// Regular file or symlink, keep going
 		default:
diff --git a/cli/command/container/formatter_stats_test.go b/cli/command/container/formatter_stats_test.go
@@ -148,7 +148,7 @@ container2  --  --
 `,
 		},
 	}
-	stats := []StatsEntry{
+	entries := []StatsEntry{
 		{
 			Container:        "container1",
 			CPUPercentage:    20,
@@ -181,7 +181,7 @@ container2  --  --
 		t.Run(string(tc.context.Format), func(t *testing.T) {
 			var out bytes.Buffer
 			tc.context.Output = &out
-			err := statsFormatWrite(tc.context, stats, "windows", false)
+			err := statsFormatWrite(tc.context, entries, "windows", false)
 			if err != nil {
 				assert.Error(t, err, tc.expected)
 			} else {
@@ -273,45 +273,46 @@ func TestContainerStatsContextWriteWithNoStatsWindows(t *testing.T) {
 }
 
 func TestContainerStatsContextWriteTrunc(t *testing.T) {
-	var out bytes.Buffer
-
-	contexts := []struct {
+	tests := []struct {
+		doc      string
 		context  formatter.Context
 		trunc    bool
 		expected string
 	}{
 		{
-			formatter.Context{
+			doc: "non-truncated",
+			context: formatter.Context{
 				Format: "{{.ID}}",
-				Output: &out,
 			},
-			false,
-			"b95a83497c9161c9b444e3d70e1a9dfba0c1840d41720e146a95a08ebf938afc\n",
+			expected: "b95a83497c9161c9b444e3d70e1a9dfba0c1840d41720e146a95a08ebf938afc\n",
 		},
 		{
-			formatter.Context{
+			doc: "truncated",
+			context: formatter.Context{
 				Format: "{{.ID}}",
-				Output: &out,
 			},
-			true,
-			"b95a83497c91\n",
+			trunc:    true,
+			expected: "b95a83497c91\n",
 		},
 	}
 
-	for _, context := range contexts {
-		statsFormatWrite(context.context, []StatsEntry{{ID: "b95a83497c9161c9b444e3d70e1a9dfba0c1840d41720e146a95a08ebf938afc"}}, "linux", context.trunc)
-		assert.Check(t, is.Equal(context.expected, out.String()))
-		// Clean buffer
-		out.Reset()
+	for _, tc := range tests {
+		t.Run(tc.doc, func(t *testing.T) {
+			var out bytes.Buffer
+			tc.context.Output = &out
+			err := statsFormatWrite(tc.context, []StatsEntry{{ID: "b95a83497c9161c9b444e3d70e1a9dfba0c1840d41720e146a95a08ebf938afc"}}, "linux", tc.trunc)
+			assert.NilError(t, err)
+			assert.Check(t, is.Equal(tc.expected, out.String()))
+		})
 	}
 }
 
 func BenchmarkStatsFormat(b *testing.B) {
 	b.ReportAllocs()
-	stats := genStats()
+	entries := genStats()
 
 	for i := 0; i < b.N; i++ {
-		for _, s := range stats {
+		for _, s := range entries {
 			_ = s.CPUPerc()
 			_ = s.MemUsage()
 			_ = s.MemPerc()
@@ -334,9 +335,9 @@ func genStats() []statsContext {
 		NetworkTx:        987.654321,
 		PidsCurrent:      123456789,
 	}}
-	stats := make([]statsContext, 100)
-	for i := 0; i < 100; i++ {
-		stats = append(stats, entry)
+	entries := make([]statsContext, 0, 100)
+	for range 100 {
+		entries = append(entries, entry)
 	}
-	return stats
+	return entries
 }
diff --git a/cli/command/formatter/displayutils.go b/cli/command/formatter/displayutils.go
@@ -20,6 +20,8 @@ func charWidth(r rune) int {
 	switch width.LookupRune(r).Kind() {
 	case width.EastAsianWide, width.EastAsianFullwidth:
 		return 2
+	case width.Neutral, width.EastAsianAmbiguous, width.EastAsianNarrow, width.EastAsianHalfwidth:
+		return 1
 	default:
 		return 1
 	}
diff --git a/cli/command/swarm/ca_test.go b/cli/command/swarm/ca_test.go
@@ -69,7 +69,7 @@ func writeFile(data string) (string, error) {
 	if err != nil {
 		return "", err
 	}
-	_, err = tmpfile.Write([]byte(data))
+	_, err = tmpfile.WriteString(data)
 	if err != nil {
 		return "", err
 	}
diff --git a/cli/command/swarm/unlock.go b/cli/command/swarm/unlock.go
@@ -50,19 +50,18 @@ func runUnlock(ctx context.Context, dockerCli command.Cli) error {
 		return errors.New("Error: This node is not part of a swarm")
 	case swarm.LocalNodeStateLocked:
 		break
-	default:
+	case swarm.LocalNodeStatePending, swarm.LocalNodeStateActive, swarm.LocalNodeStateError:
 		return errors.New("Error: swarm is not locked")
 	}
 
 	key, err := readKey(dockerCli.In(), "Enter unlock key: ")
 	if err != nil {
 		return err
 	}
-	req := swarm.UnlockRequest{
-		UnlockKey: key,
-	}
 
-	return client.SwarmUnlock(ctx, req)
+	return client.SwarmUnlock(ctx, swarm.UnlockRequest{
+		UnlockKey: key,
+	})
 }
 
 func readKey(in *streams.In, prompt string) (string, error) {
diff --git a/dockerfiles/Dockerfile.lint b/dockerfiles/Dockerfile.lint
@@ -2,7 +2,7 @@
 
 ARG GO_VERSION=1.24.3
 ARG ALPINE_VERSION=3.21
-ARG GOLANGCI_LINT_VERSION=v2.1.2
+ARG GOLANGCI_LINT_VERSION=v2.1.5
 
 FROM golangci/golangci-lint:${GOLANGCI_LINT_VERSION}-alpine AS golangci-lint
 
diff --git a/e2e/cli-plugins/socket_test.go b/e2e/cli-plugins/socket_test.go
@@ -58,7 +58,7 @@ func TestPluginSocketBackwardsCompatible(t *testing.T) {
 
 			ptmx, err := pty.Start(command)
 			assert.NilError(t, err, "failed to launch command with fake TTY")
-			_, _ = ptmx.Write([]byte("hello!"))
+			_, _ = ptmx.WriteString("hello!")
 
 			done := make(chan error)
 			go func() {
diff --git a/opts/hosts_test.go b/opts/hosts_test.go
@@ -1,7 +1,7 @@
 package opts
 
 import (
-	"fmt"
+	"net"
 	"testing"
 
 	"gotest.tools/v3/assert"
@@ -25,13 +25,13 @@ func TestParseHost(t *testing.T) {
 		"fd://something":           "fd://something",
 		"tcp://host:":              "tcp://host:" + defaultHTTPPort,
 		"tcp://":                   defaultTCPHost,
-		"tcp://:2375":              fmt.Sprintf("tcp://%s:%s", defaultHTTPHost, defaultHTTPPort),
-		"tcp://:2376":              fmt.Sprintf("tcp://%s:%s", defaultHTTPHost, defaultTLSHTTPPort),
+		"tcp://:2375":              "tcp://" + net.JoinHostPort(defaultHTTPHost, defaultHTTPPort),
+		"tcp://:2376":              "tcp://" + net.JoinHostPort(defaultHTTPHost, defaultTLSHTTPPort),
 		"tcp://0.0.0.0:8080":       "tcp://0.0.0.0:8080",
 		"tcp://192.168.0.0:12000":  "tcp://192.168.0.0:12000",
 		"tcp://192.168:8080":       "tcp://192.168:8080",
 		"tcp://0.0.0.0:1234567890": "tcp://0.0.0.0:1234567890", // yeah it's valid :P
-		" tcp://:7777/path ":       fmt.Sprintf("tcp://%s:7777/path", defaultHTTPHost),
+		" tcp://:7777/path ":       "tcp://" + net.JoinHostPort(defaultHTTPHost, "7777") + "/path",
 		"tcp://docker.com:2375":    "tcp://docker.com:2375",
 		"unix://":                  "unix://" + defaultUnixSocket,
 		"unix://path/to/socket":    "unix://path/to/socket",
@@ -70,11 +70,11 @@ func TestParseDockerDaemonHost(t *testing.T) {
 		"[::1]:5555/path":             "tcp://[::1]:5555/path",
 		"[0:0:0:0:0:0:0:1]:":          "tcp://[0:0:0:0:0:0:0:1]:2375",
 		"[0:0:0:0:0:0:0:1]:5555/path": "tcp://[0:0:0:0:0:0:0:1]:5555/path",
-		":6666":                       fmt.Sprintf("tcp://%s:6666", defaultHTTPHost),
-		":6666/path":                  fmt.Sprintf("tcp://%s:6666/path", defaultHTTPHost),
+		":6666":                       "tcp://" + net.JoinHostPort(defaultHTTPHost, "6666"),
+		":6666/path":                  "tcp://" + net.JoinHostPort(defaultHTTPHost, "6666") + "/path",
 		"tcp://":                      defaultTCPHost,
-		"tcp://:7777":                 fmt.Sprintf("tcp://%s:7777", defaultHTTPHost),
-		"tcp://:7777/path":            fmt.Sprintf("tcp://%s:7777/path", defaultHTTPHost),
+		"tcp://:7777":                 "tcp://" + net.JoinHostPort(defaultHTTPHost, "7777"),
+		"tcp://:7777/path":            "tcp://" + net.JoinHostPort(defaultHTTPHost, "7777") + "/path",
 		"unix:///run/docker.sock":     "unix:///run/docker.sock",
 		"unix://":                     "unix://" + defaultUnixSocket,
 		"fd://":                       "fd://",
@@ -96,16 +96,16 @@ func TestParseDockerDaemonHost(t *testing.T) {
 }
 
 func TestParseTCP(t *testing.T) {
-	defaultHTTPHost := "tcp://127.0.0.1:2376"
+	const defaultHost = "tcp://127.0.0.1:2376"
 	invalids := map[string]string{
 		"tcp:a.b.c.d":          "",
 		"tcp:a.b.c.d/path":     "",
 		"udp://127.0.0.1":      "invalid proto, expected tcp: udp://127.0.0.1",
 		"udp://127.0.0.1:2375": "invalid proto, expected tcp: udp://127.0.0.1:2375",
 	}
 	valids := map[string]string{
-		"":                            defaultHTTPHost,
-		"tcp://":                      defaultHTTPHost,
+		"":                            defaultHost,
+		"tcp://":                      defaultHost,
 		"0.0.0.1:":                    "tcp://0.0.0.1:2376",
 		"0.0.0.1:5555":                "tcp://0.0.0.1:5555",
 		"0.0.0.1:5555/path":           "tcp://0.0.0.1:5555/path",
@@ -124,12 +124,12 @@ func TestParseTCP(t *testing.T) {
 		"localhost:5555/path":         "tcp://localhost:5555/path",
 	}
 	for invalidAddr, expectedError := range invalids {
-		if addr, err := ParseTCPAddr(invalidAddr, defaultHTTPHost); err == nil || expectedError != "" && err.Error() != expectedError {
+		if addr, err := ParseTCPAddr(invalidAddr, defaultHost); err == nil || expectedError != "" && err.Error() != expectedError {
 			t.Errorf("tcp %v address expected error %v return, got %s and addr %v", invalidAddr, expectedError, err, addr)
 		}
 	}
 	for validAddr, expectedAddr := range valids {
-		if addr, err := ParseTCPAddr(validAddr, defaultHTTPHost); err != nil || addr != expectedAddr {
+		if addr, err := ParseTCPAddr(validAddr, defaultHost); err != nil || addr != expectedAddr {
 			t.Errorf("%v -> expected %v, got %v and addr %v", validAddr, expectedAddr, err, addr)
 		}
 	}

Original file line number	Diff line number	Diff line change
`@@ -81,7 +81,7 @@ func addPluginCandidatesFromDir(res map[string][]string, d string) {`
`81`	`81`	`return`
`82`	`82`	`}`
`83`	`83`	`for _, dentry := range dentries {`
`84`		`- switch dentry.Type() & os.ModeType {`
	`84`	`+ switch dentry.Type() & os.ModeType { //nolint:exhaustive,nolintlint // no need to include all possible file-modes in this list`
`85`	`85`	`case 0, os.ModeSymlink:`
`86`	`86`	`// Regular file or symlink, keep going`
`87`	`87`	`default:`
Original file line number	Diff line number	Diff line change
`@@ -20,6 +20,8 @@ func charWidth(r rune) int {`
`20`	`20`	`switch width.LookupRune(r).Kind() {`
`21`	`21`	`case width.EastAsianWide, width.EastAsianFullwidth:`
`22`	`22`	`return 2`
	`23`	`+ case width.Neutral, width.EastAsianAmbiguous, width.EastAsianNarrow, width.EastAsianHalfwidth:`
	`24`	`+ return 1`
`23`	`25`	`default:`
`24`	`26`	`return 1`
`25`	`27`	`}`
Original file line number	Diff line number	Diff line change
`@@ -69,7 +69,7 @@ func writeFile(data string) (string, error) {`
`69`	`69`	`if err != nil {`
`70`	`70`	`return "", err`
`71`	`71`	`}`
`72`		`- _, err = tmpfile.Write([]byte(data))`
	`72`	`+ _, err = tmpfile.WriteString(data)`
`73`	`73`	`if err != nil {`
`74`	`74`	`return "", err`
`75`	`75`	`}`