Merge pull request #5137 from thaJeztah/bump_golangci_lint

update golangci-lint to .v1.59.0 and fix linting issues

.golangci.yml

@@ -44,9 +44,6 @@ linters:
 
 run:
   timeout: 5m
-  skip-files:
-    - cli/compose/schema/bindata.go
-    - .*generated.*
 
 linters-settings:
   depguard:
@@ -58,7 +55,8 @@ linters-settings:
   gocyclo:
     min-complexity: 16
   govet:
-    check-shadowing: true
+    enable:
+      - shadow
     settings:
       shadow:
         strict: true
@@ -94,6 +92,10 @@ issues:
   exclude:
     - parameter .* always receives
 
+  exclude-files:
+    - cli/compose/schema/bindata.go
+    - .*generated.*
+
   exclude-rules:
     # We prefer to use an "exclude-list" so that new "default" exclusions are not
     # automatically inherited. We can decide whether or not to follow upstream

cli-plugins/manager/error_test.go

@@ -2,7 +2,7 @@ package manager
 
 import (
 	"encoding/json"
-	"fmt"
+	"errors"
 	"testing"
 
 	"gotest.tools/v3/assert"
@@ -13,7 +13,7 @@ func TestPluginError(t *testing.T) {
 	err := NewPluginError("new error")
 	assert.Check(t, is.Error(err, "new error"))
 
-	inner := fmt.Errorf("testing")
+	inner := errors.New("testing")
 	err = wrapAsPluginError(inner, "wrapping")
 	assert.Check(t, is.Error(err, "wrapping: testing"))
 	assert.Check(t, is.ErrorIs(err, inner))

cli/command/cli.go

@@ -315,7 +315,7 @@ func newAPIClientFromEndpoint(ep docker.Endpoint, configFile *configfile.ConfigF
 
 func resolveDockerEndpoint(s store.Reader, contextName string) (docker.Endpoint, error) {
 	if s == nil {
-		return docker.Endpoint{}, fmt.Errorf("no context store initialized")
+		return docker.Endpoint{}, errors.New("no context store initialized")
 	}
 	ctxMeta, err := s.GetMetadata(contextName)
 	if err != nil {

cli/command/cli_test.go

@@ -192,7 +192,7 @@ func TestInitializeFromClientHangs(t *testing.T) {
 	ts.Start()
 	defer ts.Close()
 
-	opts := &flags.ClientOptions{Hosts: []string{fmt.Sprintf("unix://%s", socket)}}
+	opts := &flags.ClientOptions{Hosts: []string{"unix://" + socket}}
 	configFile := &configfile.ConfigFile{}
 	apiClient, err := NewAPIClientFromFlags(opts, configFile)
 	assert.NilError(t, err)

cli/command/container/attach.go

@@ -2,7 +2,6 @@ package container
 
 import (
 	"context"
-	"fmt"
 	"io"
 
 	"github.com/docker/cli/cli"
@@ -158,7 +157,7 @@ func getExitStatus(errC <-chan error, resultC <-chan container.WaitResponse) err
 	select {
 	case result := <-resultC:
 		if result.Error != nil {
-			return fmt.Errorf(result.Error.Message)
+			return errors.New(result.Error.Message)
 		}
 		if result.StatusCode != 0 {
 			return cli.StatusError{StatusCode: int(result.StatusCode)}

cli/command/container/attach_test.go

@@ -1,7 +1,6 @@
 package container
 
 import (
-	"fmt"
 	"io"
 	"testing"
 
@@ -79,7 +78,7 @@ func TestNewAttachCommandErrors(t *testing.T) {
 
 func TestGetExitStatus(t *testing.T) {
 	var (
-		expectedErr = fmt.Errorf("unexpected error")
+		expectedErr = errors.New("unexpected error")
 		errC        = make(chan error, 1)
 		resultC     = make(chan container.WaitResponse, 1)
 	)

cli/command/container/create_test.go

@@ -3,7 +3,6 @@ package container
 import (
 	"context"
 	"errors"
-	"fmt"
 	"io"
 	"os"
 	"runtime"
@@ -231,7 +230,7 @@ func TestNewCreateCommandWithContentTrustErrors(t *testing.T) {
 				platform *specs.Platform,
 				containerName string,
 			) (container.CreateResponse, error) {
-				return container.CreateResponse{}, fmt.Errorf("shouldn't try to pull image")
+				return container.CreateResponse{}, errors.New("shouldn't try to pull image")
 			},
 		}, test.EnableContentTrust)
 		fakeCLI.SetNotaryClient(tc.notaryFunc)

cli/command/container/list_test.go

@@ -1,7 +1,7 @@
 package container
 
 import (
-	"fmt"
+	"errors"
 	"io"
 	"testing"
 
@@ -147,7 +147,7 @@ func TestContainerListErrors(t *testing.T) {
 		},
 		{
 			containerListFunc: func(_ container.ListOptions) ([]types.Container, error) {
-				return nil, fmt.Errorf("error listing containers")
+				return nil, errors.New("error listing containers")
 			},
 			expectedError: "error listing containers",
 		},

cli/command/container/opts.go

@@ -574,10 +574,10 @@ func parse(flags *pflag.FlagSet, copts *containerOptions, serverOS string) (*con
 			return nil, errors.Errorf("--health-retries cannot be negative")
 		}
 		if copts.healthStartPeriod < 0 {
-			return nil, fmt.Errorf("--health-start-period cannot be negative")
+			return nil, errors.New("--health-start-period cannot be negative")
 		}
 		if copts.healthStartInterval < 0 {
-			return nil, fmt.Errorf("--health-start-interval cannot be negative")
+			return nil, errors.New("--health-start-interval cannot be negative")
 		}
 
 		healthConfig = &container.HealthConfig{

cli/command/container/opts_test.go

@@ -335,7 +335,7 @@ func TestParseHostname(t *testing.T) {
 	hostnameWithDomain := "--hostname=hostname.domainname"
 	hostnameWithDomainTld := "--hostname=hostname.domainname.tld"
 	for hostname, expectedHostname := range validHostnames {
-		if config, _, _ := mustParse(t, fmt.Sprintf("--hostname=%s", hostname)); config.Hostname != expectedHostname {
+		if config, _, _ := mustParse(t, "--hostname="+hostname); config.Hostname != expectedHostname {
 			t.Fatalf("Expected the config to have 'hostname' as %q, got %q", expectedHostname, config.Hostname)
 		}
 	}

cli/command/container/rm_test.go

@@ -2,7 +2,7 @@ package container
 
 import (
 	"context"
-	"fmt"
+	"errors"
 	"io"
 	"sort"
 	"sync"
@@ -37,7 +37,7 @@ func TestRemoveForce(t *testing.T) {
 					mutex.Unlock()
 
 					if container == "nosuchcontainer" {
-						return errdefs.NotFound(fmt.Errorf("Error: no such container: " + container))
+						return errdefs.NotFound(errors.New("Error: no such container: " + container))
 					}
 					return nil
 				},

cli/command/container/run_test.go

@@ -3,7 +3,6 @@ package container
 import (
 	"context"
 	"errors"
-	"fmt"
 	"io"
 	"net"
 	"os/signal"
@@ -135,7 +134,7 @@ func TestRunCommandWithContentTrustErrors(t *testing.T) {
 				platform *specs.Platform,
 				containerName string,
 			) (container.CreateResponse, error) {
-				return container.CreateResponse{}, fmt.Errorf("shouldn't try to pull image")
+				return container.CreateResponse{}, errors.New("shouldn't try to pull image")
 			},
 		}, test.EnableContentTrust)
 		fakeCLI.SetNotaryClient(tc.notaryFunc)

cli/command/container/stats.go

@@ -218,7 +218,7 @@ func RunStats(ctx context.Context, dockerCLI command.Cli, options *StatsOptions)
 		// with a list of container names/IDs.
 
 		if options.Filters != nil && options.Filters.Len() > 0 {
-			return fmt.Errorf("filtering is not supported when specifying a list of containers")
+			return errors.New("filtering is not supported when specifying a list of containers")
 		}
 
 		// Create the list of containers, and start collecting stats for all

cli/command/context/options.go

@@ -1,7 +1,6 @@
 package context
 
 import (
-	"fmt"
 	"strconv"
 	"strings"
 
@@ -76,7 +75,7 @@ func validateConfig(config map[string]string, allowedKeys map[string]struct{}) e
 	var errs []string
 	for k := range config {
 		if _, ok := allowedKeys[k]; !ok {
-			errs = append(errs, fmt.Sprintf("%s: unrecognized config key", k))
+			errs = append(errs, "unrecognized config key: "+k)
 		}
 	}
 	if len(errs) == 0 {

cli/command/image/build.go

@@ -458,7 +458,7 @@ func rewriteDockerfileFromForContentTrust(ctx context.Context, dockerfile io.Rea
 					return nil, nil, err
 				}
 
-				line = dockerfileFromLinePattern.ReplaceAllLiteralString(line, fmt.Sprintf("FROM %s", reference.FamiliarString(trustedRef)))
+				line = dockerfileFromLinePattern.ReplaceAllLiteralString(line, "FROM "+reference.FamiliarString(trustedRef))
 				resolvedTags = append(resolvedTags, &resolvedTag{
 					digestRef: trustedRef,
 					tagRef:    ref,

cli/command/image/build/context.go

@@ -226,15 +226,15 @@ func GetContextFromURL(out io.Writer, remoteURL, dockerfileName string) (io.Read
 	progressOutput := streamformatter.NewProgressOutput(out)
 
 	// Pass the response body through a progress reader.
-	progReader := progress.NewProgressReader(response.Body, progressOutput, response.ContentLength, "", fmt.Sprintf("Downloading build context from remote url: %s", remoteURL))
+	progReader := progress.NewProgressReader(response.Body, progressOutput, response.ContentLength, "", "Downloading build context from remote url: "+remoteURL)
 
 	return GetContextFromReader(ioutils.NewReadCloserWrapper(progReader, func() error { return response.Body.Close() }), dockerfileName)
 }
 
 // getWithStatusError does an http.Get() and returns an error if the
 // status code is 4xx or 5xx.
 func getWithStatusError(url string) (resp *http.Response, err error) {
-	//#nosec G107 -- Ignore G107: Potential HTTP request made with variable url
+	//nolint:gosec // Ignore G107: Potential HTTP request made with variable url
 	if resp, err = http.Get(url); err != nil {
 		return nil, err
 	}

cli/command/image/pull_test.go

@@ -1,6 +1,7 @@
 package image
 
 import (
+	"errors"
 	"fmt"
 	"io"
 	"strings"
@@ -112,7 +113,7 @@ func TestNewPullCommandWithContentTrustErrors(t *testing.T) {
 	for _, tc := range testCases {
 		cli := test.NewFakeCli(&fakeClient{
 			imagePullFunc: func(ref string, options image.PullOptions) (io.ReadCloser, error) {
-				return io.NopCloser(strings.NewReader("")), fmt.Errorf("shouldn't try to pull image")
+				return io.NopCloser(strings.NewReader("")), errors.New("shouldn't try to pull image")
 			},
 		}, test.EnableContentTrust)
 		cli.SetNotaryClient(tc.notaryFunc)

cli/command/image/remove_test.go

@@ -18,7 +18,7 @@ type notFound struct {
 }
 
 func (n notFound) Error() string {
-	return fmt.Sprintf("Error: No such image: %s", n.imageID)
+	return "Error: No such image: " + n.imageID
 }
 
 func (n notFound) NotFound() {}

cli/command/network/connect.go

@@ -2,14 +2,15 @@ package network
 
 import (
 	"context"
-	"fmt"
+	"errors"
 	"strings"
 
 	"github.com/docker/cli/cli"
 	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/cli/command/completion"
 	"github.com/docker/cli/opts"
 	"github.com/docker/docker/api/types/network"
+	"github.com/docker/docker/client"
 	"github.com/spf13/cobra"
 )
 
@@ -36,14 +37,14 @@ func newConnectCommand(dockerCli command.Cli) *cobra.Command {
 		RunE: func(cmd *cobra.Command, args []string) error {
 			options.network = args[0]
 			options.container = args[1]
-			return runConnect(cmd.Context(), dockerCli, options)
+			return runConnect(cmd.Context(), dockerCli.Client(), options)
 		},
 		ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
 			if len(args) == 0 {
 				return completion.NetworkNames(dockerCli)(cmd, args, toComplete)
 			}
-			network := args[0]
-			return completion.ContainerNames(dockerCli, true, not(isConnected(network)))(cmd, args, toComplete)
+			nw := args[0]
+			return completion.ContainerNames(dockerCli, true, not(isConnected(nw)))(cmd, args, toComplete)
 		},
 	}
 
@@ -57,14 +58,13 @@ func newConnectCommand(dockerCli command.Cli) *cobra.Command {
 	return cmd
 }
 
-func runConnect(ctx context.Context, dockerCli command.Cli, options connectOptions) error {
-	client := dockerCli.Client()
-
+func runConnect(ctx context.Context, apiClient client.NetworkAPIClient, options connectOptions) error {
 	driverOpts, err := convertDriverOpt(options.driverOpts)
 	if err != nil {
 		return err
 	}
-	epConfig := &network.EndpointSettings{
+
+	return apiClient.NetworkConnect(ctx, options.network, options.container, &network.EndpointSettings{
 		IPAMConfig: &network.EndpointIPAMConfig{
 			IPv4Address:  options.ipaddress,
 			IPv6Address:  options.ipv6address,
@@ -73,9 +73,7 @@ func runConnect(ctx context.Context, dockerCli command.Cli, options connectOptio
 		Links:      options.links.GetAll(),
 		Aliases:    options.aliases,
 		DriverOpts: driverOpts,
-	}
-
-	return client.NetworkConnect(ctx, options.network, options.container, epConfig)
+	})
 }
 
 func convertDriverOpt(options []string) (map[string]string, error) {
@@ -85,7 +83,7 @@ func convertDriverOpt(options []string) (map[string]string, error) {
 		// TODO(thaJeztah): we should probably not accept whitespace here (both for key and value).
 		k = strings.TrimSpace(k)
 		if !ok || k == "" {
-			return nil, fmt.Errorf("invalid key/value pair format in driver options")
+			return nil, errors.New("invalid key/value pair format in driver options")
 		}
 		driverOpt[k] = strings.TrimSpace(v)
 	}

cli/command/plugin/create_test.go

@@ -1,7 +1,7 @@
 package plugin
 
 import (
-	"fmt"
+	"errors"
 	"io"
 	"runtime"
 	"testing"
@@ -91,16 +91,14 @@ func TestCreateErrorFromDaemon(t *testing.T) {
 		fs.WithFile("config.json", `{ "Name": "plugin-foo" }`))
 	defer tmpDir.Remove()
 
-	cli := test.NewFakeCli(&fakeClient{
+	cmd := newCreateCommand(test.NewFakeCli(&fakeClient{
 		pluginCreateFunc: func(createContext io.Reader, createOptions types.PluginCreateOptions) error {
-			return fmt.Errorf("Error creating plugin")
+			return errors.New("error creating plugin")
 		},
-	})
-
-	cmd := newCreateCommand(cli)
+	}))
 	cmd.SetArgs([]string{"plugin-foo", tmpDir.Path()})
 	cmd.SetOut(io.Discard)
-	assert.ErrorContains(t, cmd.Execute(), "Error creating plugin")
+	assert.ErrorContains(t, cmd.Execute(), "error creating plugin")
 }
 
 func TestCreatePlugin(t *testing.T) {

cli/command/plugin/disable_test.go

@@ -1,7 +1,7 @@
 package plugin
 
 import (
-	"fmt"
+	"errors"
 	"io"
 	"testing"
 
@@ -27,9 +27,9 @@ func TestPluginDisableErrors(t *testing.T) {
 		},
 		{
 			args:          []string{"plugin-foo"},
-			expectedError: "Error disabling plugin",
+			expectedError: "error disabling plugin",
 			pluginDisableFunc: func(name string, disableOptions types.PluginDisableOptions) error {
-				return fmt.Errorf("Error disabling plugin")
+				return errors.New("error disabling plugin")
 			},
 		},
 	}