Merge pull request #3770 from thaJeztah/update_golangci_lint_step2

golangci-lint: enable more linters

Author: thaJeztah

.golangci.yml

@@ -3,23 +3,41 @@ linters:
     - bodyclose
     - depguard
     - dogsled
+    - dupword       # Detects duplicate words.
+    - durationcheck
+    - errchkjson
+    - exportloopref # Detects pointers to enclosing loop variables.
+    - gocritic      # Metalinter; detects bugs, performance, and styling issues.
     - gocyclo
-    - gofumpt
+    - gofumpt       # Detects whether code was gofumpt-ed.
     - goimports
-    - gosec
+    - gosec         # Detects security problems.
     - gosimple
     - govet
     - ineffassign
     - lll
     - megacheck
-    - misspell
+    - misspell      # Detects commonly misspelled English words in comments.
     - nakedret
-    - revive
+    - 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.
     - staticcheck
+    - stylecheck    # Replacement for golint
+    - tenv          # Detects using os.Setenv instead of t.Setenv.
+    - thelper       # Detects test helpers without t.Helper().
+    - tparallel     # Detects inappropriate usage of t.Parallel().
     - typecheck
-    - unconvert
+    - unconvert     # Detects unnecessary type conversions.
     - unparam
     - unused
+    - usestdlibvars
+    - vet
+    - wastedassign
 
   disable:
     - errcheck
@@ -40,13 +58,35 @@ linters-settings:
   gocyclo:
     min-complexity: 16
   govet:
-    check-shadowing: false
+    check-shadowing: true
+    settings:
+      shadow:
+        strict: true
   lll:
     line-length: 200
   nakedret:
     command: nakedret
     pattern: ^(?P<path>.*?\\.go):(?P<line>\\d+)\\s*(?P<message>.*)$
 
+  revive:
+    rules:
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#import-shadowing
+      - name: import-shadowing
+        severity: warning
+        disabled: false
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#empty-block
+      - name: empty-block
+        severity: warning
+        disabled: false
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#empty-lines
+      - name: empty-lines
+        severity: warning
+        disabled: false
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#use-any
+      - name: use-any
+        severity: warning
+        disabled: false
+
 issues:
   # The default exclusion rules are a bit too permissive, so copying the relevant ones below
   exclude-use-default: false
@@ -83,7 +123,7 @@ issues:
         - gosec
     # EXC0008
     # TODO: evaluate these and fix where needed: G307: Deferring unsafe method "*os.File" on type "Close" (gosec)
-    - text: "(G104|G307)"
+    - text: "G307"
       linters:
         - gosec
     # EXC0009
@@ -97,10 +137,13 @@ issues:
 
     # G113 Potential uncontrolled memory consumption in Rat.SetString (CVE-2022-23772)
     # only affects gp < 1.16.14. and go < 1.17.7
-    - text: "(G113)"
+    - text: "G113"
+      linters:
+        - gosec
+    # TODO: G104: Errors unhandled. (gosec)
+    - text: "G104"
       linters:
         - gosec
-
     # Looks like the match in "EXC0007" above doesn't catch this one
     # TODO: consider upstreaming this to golangci-lint's default exclusion rules
     - text: "G204: Subprocess launched with a potential tainted input or cmd arguments"
@@ -125,6 +168,15 @@ issues:
       linters:
         - errcheck
         - gosec
+    - text: "ST1000: at least one file in a package should have a package comment"
+      linters:
+        - stylecheck
+
+    # Allow "err" and "ok" vars to shadow existing declarations, otherwise we get too many false positives.
+    - text: '^shadow: declaration of "(err|ok)" shadows declaration'
+      linters:
+        - govet
+
 
   # Maximum issues count per one linter. Set to 0 to disable. Default is 50.
   max-issues-per-linter: 0

cli-plugins/manager/candidate_test.go

@@ -75,13 +75,14 @@ func TestValidateCandidate(t *testing.T) {
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			p, err := newPlugin(tc.c, fakeroot.Commands())
-			if tc.err != "" {
+			switch {
+			case tc.err != "":
 				assert.ErrorContains(t, err, tc.err)
-			} else if tc.invalid != "" {
+			case tc.invalid != "":
 				assert.NilError(t, err)
 				assert.Assert(t, cmp.ErrorType(p.Err, reflect.TypeOf(&pluginError{})))
 				assert.ErrorContains(t, p.Err, tc.invalid)
-			} else {
+			default:
 				assert.NilError(t, err)
 				assert.Equal(t, NamePrefix+p.Name, goodPluginName)
 				assert.Equal(t, p.SchemaVersion, "0.1.0")

cli-plugins/manager/error.go

@@ -43,6 +43,6 @@ func wrapAsPluginError(err error, msg string) error {
 
 // NewPluginError creates a new pluginError, analogous to
 // errors.Errorf.
-func NewPluginError(msg string, args ...interface{}) error {
+func NewPluginError(msg string, args ...any) error {
 	return &pluginError{cause: errors.Errorf(msg, args...)}
 }

cli-plugins/manager/manager_test.go

@@ -46,7 +46,7 @@ func TestListPluginCandidates(t *testing.T) {
 	)
 	defer dir.Remove()
 
-	var dirs []string
+	dirs := make([]string, 0, 6)
 	for _, d := range []string{"plugins1", "nonexistent", "plugins2", "plugins3", "plugins4", "plugins5"} {
 		dirs = append(dirs, dir.Join(d))
 	}

cli/command/checkpoint/formatter.go

@@ -12,8 +12,7 @@ const (
 
 // NewFormat returns a format for use with a checkpoint Context
 func NewFormat(source string) formatter.Format {
-	switch source {
-	case formatter.TableFormatKey:
+	if source == formatter.TableFormatKey {
 		return defaultCheckpointFormat
 	}
 	return formatter.Format(source)

cli/command/cli.go

@@ -394,7 +394,7 @@ func (cli *DockerCli) CurrentContext() string {
 // occur when trying to use it.
 //
 // Refer to [DockerCli.CurrentContext] above for further details.
-func resolveContextName(opts *cliflags.ClientOptions, config *configfile.ConfigFile) string {
+func resolveContextName(opts *cliflags.ClientOptions, cfg *configfile.ConfigFile) string {
 	if opts != nil && opts.Context != "" {
 		return opts.Context
 	}
@@ -407,9 +407,9 @@ func resolveContextName(opts *cliflags.ClientOptions, config *configfile.ConfigF
 	if ctxName := os.Getenv(EnvOverrideContext); ctxName != "" {
 		return ctxName
 	}
-	if config != nil && config.CurrentContext != "" {
+	if cfg != nil && cfg.CurrentContext != "" {
 		// We don't validate if this context exists: errors may occur when trying to use it.
-		return config.CurrentContext
+		return cfg.CurrentContext
 	}
 	return DefaultContextName
 }
@@ -514,7 +514,7 @@ func UserAgent() string {
 }
 
 var defaultStoreEndpoints = []store.NamedTypeGetter{
-	store.EndpointTypeGetter(docker.DockerEndpoint, func() interface{} { return &docker.EndpointMeta{} }),
+	store.EndpointTypeGetter(docker.DockerEndpoint, func() any { return &docker.EndpointMeta{} }),
 }
 
 // RegisterDefaultStoreEndpoints registers a new named endpoint
@@ -528,7 +528,7 @@ func RegisterDefaultStoreEndpoints(ep ...store.NamedTypeGetter) {
 // DefaultContextStoreConfig returns a new store.Config with the default set of endpoints configured.
 func DefaultContextStoreConfig() store.Config {
 	return store.NewConfig(
-		func() interface{} { return &DockerContext{} },
+		func() any { return &DockerContext{} },
 		defaultStoreEndpoints...,
 	)
 }

cli/command/cli_options_test.go

@@ -8,6 +8,7 @@ import (
 )
 
 func contentTrustEnabled(t *testing.T) bool {
+	t.Helper()
 	var cli DockerCli
 	assert.NilError(t, WithContentTrustFromEnv()(&cli))
 	return cli.contentTrust

cli/command/config/formatter.go

@@ -101,7 +101,7 @@ func (c *configContext) Labels() string {
 	if mapLabels == nil {
 		return ""
 	}
-	var joinLabels []string
+	joinLabels := make([]string, 0, len(mapLabels))
 	for k, v := range mapLabels {
 		joinLabels = append(joinLabels, k+"="+v)
 	}

cli/command/config/inspect.go

@@ -48,7 +48,7 @@ func RunConfigInspect(dockerCli command.Cli, opts InspectOptions) error {
 		opts.Format = "pretty"
 	}
 
-	getRef := func(id string) (interface{}, []byte, error) {
+	getRef := func(id string) (any, []byte, error) {
 		return client.ConfigInspectWithRaw(ctx, id)
 	}
 	f := opts.Format

cli/command/container/attach.go

@@ -24,8 +24,8 @@ type AttachOptions struct {
 	DetachKeys string
 }
 
-func inspectContainerAndCheckState(ctx context.Context, cli client.APIClient, args string) (*types.ContainerJSON, error) {
-	c, err := cli.ContainerInspect(ctx, args)
+func inspectContainerAndCheckState(ctx context.Context, apiClient client.APIClient, args string) (*types.ContainerJSON, error) {
+	c, err := apiClient.ContainerInspect(ctx, args)
 	if err != nil {
 		return nil, err
 	}
@@ -45,21 +45,21 @@ func inspectContainerAndCheckState(ctx context.Context, cli client.APIClient, ar
 // NewAttachCommand creates a new cobra.Command for `docker attach`
 func NewAttachCommand(dockerCli command.Cli) *cobra.Command {
 	var opts AttachOptions
-	var container string
+	var ctr string
 
 	cmd := &cobra.Command{
 		Use:   "attach [OPTIONS] CONTAINER",
 		Short: "Attach local standard input, output, and error streams to a running container",
 		Args:  cli.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			container = args[0]
-			return RunAttach(context.Background(), dockerCli, container, &opts)
+			ctr = args[0]
+			return RunAttach(context.Background(), dockerCli, ctr, &opts)
 		},
 		Annotations: map[string]string{
 			"aliases": "docker container attach, docker attach",
 		},
-		ValidArgsFunction: completion.ContainerNames(dockerCli, false, func(container types.Container) bool {
-			return container.State != "paused"
+		ValidArgsFunction: completion.ContainerNames(dockerCli, false, func(ctr types.Container) bool {
+			return ctr.State != "paused"
 		}),
 	}
 
@@ -71,8 +71,8 @@ func NewAttachCommand(dockerCli command.Cli) *cobra.Command {
 }
 
 // RunAttach executes an `attach` command
-func RunAttach(ctx context.Context, dockerCli command.Cli, target string, opts *AttachOptions) error {
-	apiClient := dockerCli.Client()
+func RunAttach(ctx context.Context, dockerCLI command.Cli, target string, opts *AttachOptions) error {
+	apiClient := dockerCLI.Client()
 
 	// request channel to wait for client
 	resultC, errC := apiClient.ContainerWait(ctx, target, "")
@@ -82,11 +82,11 @@ func RunAttach(ctx context.Context, dockerCli command.Cli, target string, opts *
 		return err
 	}
 
-	if err := dockerCli.In().CheckTty(!opts.NoStdin, c.Config.Tty); err != nil {
+	if err := dockerCLI.In().CheckTty(!opts.NoStdin, c.Config.Tty); err != nil {
 		return err
 	}
 
-	detachKeys := dockerCli.ConfigFile().DetachKeys
+	detachKeys := dockerCLI.ConfigFile().DetachKeys
 	if opts.DetachKeys != "" {
 		detachKeys = opts.DetachKeys
 	}
@@ -101,7 +101,7 @@ func RunAttach(ctx context.Context, dockerCli command.Cli, target string, opts *
 
 	var in io.ReadCloser
 	if options.Stdin {
-		in = dockerCli.In()
+		in = dockerCLI.In()
 	}
 
 	if opts.Proxy && !c.Config.Tty {
@@ -129,15 +129,15 @@ func RunAttach(ctx context.Context, dockerCli command.Cli, target string, opts *
 		return err
 	}
 
-	if c.Config.Tty && dockerCli.Out().IsTerminal() {
-		resizeTTY(ctx, dockerCli, target)
+	if c.Config.Tty && dockerCLI.Out().IsTerminal() {
+		resizeTTY(ctx, dockerCLI, target)
 	}
 
 	streamer := hijackedIOStreamer{
-		streams:      dockerCli,
+		streams:      dockerCLI,
 		inputStream:  in,
-		outputStream: dockerCli.Out(),
-		errorStream:  dockerCli.Err(),
+		outputStream: dockerCLI.Out(),
+		errorStream:  dockerCLI.Err(),
 		resp:         resp,
 		tty:          c.Config.Tty,
 		detachKeys:   options.DetachKeys,