golangci-lint: enable more linters

.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,