[19.03 backport] Replace gometalinter with Golangci lint [carry 1797]

.golangci.yml

@@ -0,0 +1,83 @@
+linters:
+  enable:
+    - bodyclose
+    - deadcode
+    - dogsled
+    - gocyclo
+    - goimports
+    - golint
+    - gosec
+    - gosimple
+    - govet
+    - ineffassign
+    - interfacer
+    - lll
+    - megacheck
+    - misspell
+    - nakedret
+    - staticcheck
+    - structcheck
+    - typecheck
+    - unconvert
+    - unparam
+    - unused
+    - varcheck
+
+  disable:
+    - errcheck
+
+run:
+  timeout: 5m
+  skip-dirs:
+    - cli/command/stack/kubernetes/api/openapi
+    - cli/command/stack/kubernetes/api/client
+  skip-files:
+    - cli/compose/schema/bindata.go
+    - .*generated.*
+
+linters-settings:
+  gocyclo:
+    min-complexity: 16
+  govet:
+    check-shadowing: false
+  lll:
+    line-length: 200
+  nakedret:
+    command: nakedret
+    pattern: ^(?P<path>.*?\\.go):(?P<line>\\d+)\\s*(?P<message>.*)$
+
+issues:
+  # The default exclusion rules are a bit too permissive, so copying the relevant ones below
+  exclude-use-default: false
+
+  exclude:
+    - parameter .* always receives
+
+  exclude-rules:
+    # These are copied from the default exclude rules, except for "ineffective break statement"
+    # and GoDoc checks.
+    # https://github.com/golangci/golangci-lint/blob/0cc87df732aaf1d5ad9ce9ca538d38d916918b36/pkg/config/config.go#L36
+    - text: "Error return value of .((os\\.)?std(out|err)\\..*|.*Close|.*Flush|os\\.Remove(All)?|.*printf?|os\\.(Un)?Setenv). is not checked"
+      linters:
+        - errcheck
+    - text: "func name will be used as test\\.Test.* by other packages, and that stutters; consider calling this"
+      linters:
+        - golint
+    - text: "G103: Use of unsafe calls should be audited"
+      linters:
+        - gosec
+    - text: "G104: Errors unhandled"
+      linters:
+        - gosec
+    - text: "G204: Subprocess launch(ed with (variable|function call)|ing should be audited)"
+      linters:
+        - gosec
+    - text: "(G301|G302): (Expect directory permissions to be 0750 or less|Expect file permissions to be 0600 or less)"
+      linters:
+        - gosec
+    - text: "G304: Potential file inclusion via variable"
+      linters:
+        - gosec
+    - text: "(G201|G202): SQL string (formatting|concatenation)"
+      linters:
+        - gosec

cli/command/cli_test.go

@@ -152,6 +152,7 @@ func TestInitializeFromClient(t *testing.T) {
 	}
 
 	for _, testcase := range testcases {
+		testcase := testcase
 		t.Run(testcase.doc, func(t *testing.T) {
 			apiclient := &fakeClient{
 				pingFunc: testcase.pingFunc,
@@ -189,6 +190,7 @@ func TestExperimentalCLI(t *testing.T) {
 	}
 
 	for _, testcase := range testcases {
+		testcase := testcase
 		t.Run(testcase.doc, func(t *testing.T) {
 			dir := fs.NewDir(t, testcase.doc, fs.WithFile("config.json", testcase.configfile))
 			defer dir.Remove()
@@ -242,6 +244,7 @@ func TestGetClientWithPassword(t *testing.T) {
 	}
 
 	for _, testcase := range testcases {
+		testcase := testcase
 		t.Run(testcase.doc, func(t *testing.T) {
 			passRetriever := func(_, _ string, _ bool, attempts int) (passphrase string, giveup bool, err error) {
 				// Always return an invalid pass first to test iteration
@@ -294,11 +297,12 @@ func TestNewDockerCliAndOperators(t *testing.T) {
 	inbuf := bytes.NewBuffer([]byte("input"))
 	outbuf := bytes.NewBuffer(nil)
 	errbuf := bytes.NewBuffer(nil)
-	cli.Apply(
+	err = cli.Apply(
 		WithInputStream(ioutil.NopCloser(inbuf)),
 		WithOutputStream(outbuf),
 		WithErrorStream(errbuf),
 	)
+	assert.NilError(t, err)
 	// Check input stream
 	inputStream, err := ioutil.ReadAll(cli.In())
 	assert.NilError(t, err)

cli/command/config/inspect_test.go

@@ -7,10 +7,9 @@ import (
 	"time"
 
 	"github.com/docker/cli/internal/test"
+	. "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
 	"github.com/docker/docker/api/types/swarm"
 	"github.com/pkg/errors"
-	// Import builders to get the builder function as package function
-	. "github.com/docker/cli/internal/test/builders"
 	"gotest.tools/assert"
 	"gotest.tools/golden"
 )

cli/command/config/ls_test.go

@@ -7,11 +7,10 @@ import (
 
 	"github.com/docker/cli/cli/config/configfile"
 	"github.com/docker/cli/internal/test"
+	. "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/swarm"
 	"github.com/pkg/errors"
-	// Import builders to get the builder function as package function
-	. "github.com/docker/cli/internal/test/builders"
 	"gotest.tools/assert"
 	is "gotest.tools/assert/cmp"
 	"gotest.tools/golden"

cli/command/container/attach.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 	"io"
-	"net/http/httputil"
 
 	"github.com/docker/cli/cli"
 	"github.com/docker/cli/cli/command"
@@ -103,10 +102,7 @@ func runAttach(dockerCli command.Cli, opts *attachOptions) error {
 	}
 
 	resp, errAttach := client.ContainerAttach(ctx, opts.container, options)
-	if errAttach != nil && errAttach != httputil.ErrPersistEOF {
-		// ContainerAttach returns an ErrPersistEOF (connection closed)
-		// means server met an error and put it in Hijacked connection
-		// keep the error and read detailed error message from hijacked connection later
+	if errAttach != nil {
 		return errAttach
 	}
 	defer resp.Close()
@@ -142,10 +138,6 @@ func runAttach(dockerCli command.Cli, opts *attachOptions) error {
 		return err
 	}
 
-	if errAttach != nil {
-		return errAttach
-	}
-
 	return getExitStatus(errC, resultC)
 }
 

cli/command/container/create_test.go

@@ -151,6 +151,7 @@ func TestNewCreateCommandWithContentTrustErrors(t *testing.T) {
 		},
 	}
 	for _, tc := range testCases {
+		tc := tc
 		cli := test.NewFakeCli(&fakeClient{
 			createContainerFunc: func(config *container.Config,
 				hostConfig *container.HostConfig,
@@ -209,6 +210,7 @@ func TestNewCreateCommandWithWarnings(t *testing.T) {
 		},
 	}
 	for _, tc := range testCases {
+		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
 			cli := test.NewFakeCli(&fakeClient{
 				createContainerFunc: func(config *container.Config,

cli/command/container/list_test.go

@@ -7,9 +7,8 @@ import (
 
 	"github.com/docker/cli/cli/config/configfile"
 	"github.com/docker/cli/internal/test"
+	. "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
 	"github.com/docker/docker/api/types"
-	// Import builders to get the builder function as package function
-	. "github.com/docker/cli/internal/test/builders"
 	"gotest.tools/assert"
 	"gotest.tools/golden"
 )

cli/command/container/opts_test.go

@@ -65,7 +65,7 @@ func setupRunFlags() (*pflag.FlagSet, *containerOptions) {
 }
 
 func parseMustError(t *testing.T, args string) {
-	_, _, _, err := parseRun(strings.Split(args+" ubuntu bash", " "))
+	_, _, _, err := parseRun(strings.Split(args+" ubuntu bash", " ")) //nolint:dogsled
 	assert.ErrorContains(t, err, "", args)
 }
 
@@ -539,7 +539,7 @@ func TestParseModes(t *testing.T) {
 	}
 
 	// uts ko
-	_, _, _, err = parseRun([]string{"--uts=container:", "img", "cmd"})
+	_, _, _, err = parseRun([]string{"--uts=container:", "img", "cmd"}) //nolint:dogsled
 	assert.ErrorContains(t, err, "--uts: invalid UTS mode")
 
 	// uts ok
@@ -600,7 +600,7 @@ func TestParseRestartPolicy(t *testing.T) {
 
 func TestParseRestartPolicyAutoRemove(t *testing.T) {
 	expected := "Conflicting options: --restart and --rm"
-	_, _, _, err := parseRun([]string{"--rm", "--restart=always", "img", "cmd"})
+	_, _, _, err := parseRun([]string{"--rm", "--restart=always", "img", "cmd"}) //nolint:dogsled
 	if err == nil || err.Error() != expected {
 		t.Fatalf("Expected error %v, but got none", expected)
 	}

cli/command/container/run.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 	"io"
-	"net/http/httputil"
 	"os"
 	"runtime"
 	"strings"
@@ -248,10 +247,7 @@ func attachContainer(
 	}
 
 	resp, errAttach := dockerCli.Client().ContainerAttach(ctx, containerID, options)
-	if errAttach != nil && errAttach != httputil.ErrPersistEOF {
-		// ContainerAttach returns an ErrPersistEOF (connection closed)
-		// means server met an error and put it in Hijacked connection
-		// keep the error and read detailed error message from hijacked connection later
+	if errAttach != nil {
 		return nil, errAttach
 	}
 

cli/command/container/start.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 	"io"
-	"net/http/httputil"
 	"strings"
 
 	"github.com/docker/cli/cli"
@@ -98,10 +97,7 @@ func runStart(dockerCli command.Cli, opts *startOptions) error {
 		}
 
 		resp, errAttach := dockerCli.Client().ContainerAttach(ctx, c.ID, options)
-		if errAttach != nil && errAttach != httputil.ErrPersistEOF {
-			// ContainerAttach return an ErrPersistEOF (connection closed)
-			// means server met an error and already put it in Hijacked connection,
-			// we would keep the error and read the detailed error message from hijacked connection
+		if errAttach != nil {
 			return errAttach
 		}
 		defer resp.Close()
@@ -154,7 +150,7 @@ func runStart(dockerCli command.Cli, opts *startOptions) error {
 			}
 		}
 		if attachErr := <-cErr; attachErr != nil {
-			if _, ok := err.(term.EscapeError); ok {
+			if _, ok := attachErr.(term.EscapeError); ok {
 				// The user entered the detach escape sequence.
 				return nil
 			}

cli/command/container/stats.go

@@ -208,7 +208,9 @@ func runStats(dockerCli command.Cli, opts *statsOptions) error {
 	}
 
 	var err error
-	for range time.Tick(500 * time.Millisecond) {
+	ticker := time.NewTicker(500 * time.Millisecond)
+	defer ticker.Stop()
+	for range ticker.C {
 		cleanScreen()
 		ccstats := []StatsEntry{}
 		cStats.mu.Lock()

cli/command/context/create_test.go

@@ -263,6 +263,7 @@ func TestCreateFromContext(t *testing.T) {
 	cli.SetCurrentContext("dummy")
 
 	for _, c := range cases {
+		c := c
 		t.Run(c.name, func(t *testing.T) {
 			cli.ResetOutputBuffers()
 			err := RunCreate(cli, &CreateOptions{
@@ -339,6 +340,7 @@ func TestCreateFromCurrent(t *testing.T) {
 	cli.SetCurrentContext("original")
 
 	for _, c := range cases {
+		c := c
 		t.Run(c.name, func(t *testing.T) {
 			cli.ResetOutputBuffers()
 			err := RunCreate(cli, &CreateOptions{

cli/command/formatter/container_test.go

@@ -304,7 +304,8 @@ func TestContainerContextWriteWithNoContainers(t *testing.T) {
 	}
 
 	for _, context := range contexts {
-		ContainerWrite(context.context, containers)
+		err := ContainerWrite(context.context, containers)
+		assert.NilError(t, err)
 		assert.Check(t, is.Equal(context.expected, out.String()))
 		// Clean buffer
 		out.Reset()

cli/command/formatter/image_test.go

@@ -348,7 +348,8 @@ func TestImageContextWriteWithNoImage(t *testing.T) {
 	}
 
 	for _, context := range contexts {
-		ImageWrite(context.context, images)
+		err := ImageWrite(context.context, images)
+		assert.NilError(t, err)
 		assert.Check(t, is.Equal(context.expected, out.String()))
 		// Clean buffer
 		out.Reset()

cli/command/idresolver/idresolver_test.go

@@ -1,16 +1,14 @@
 package idresolver
 
 import (
+	"context"
 	"testing"
 
+	. "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
 	"github.com/docker/docker/api/types/swarm"
+	"github.com/pkg/errors"
 	"gotest.tools/assert"
 	is "gotest.tools/assert/cmp"
-	// Import builders to get the builder function as package function
-	"context"
-
-	. "github.com/docker/cli/internal/test/builders"
-	"github.com/pkg/errors"
 )
 
 func TestResolveError(t *testing.T) {

cli/command/image/build.go

@@ -474,7 +474,7 @@ func runBuild(dockerCli command.Cli, options buildOptions) error {
 	// should be just the image ID and we'll print that to stdout.
 	if options.quiet {
 		imageID = fmt.Sprintf("%s", buildBuff)
-		fmt.Fprintf(dockerCli.Out(), imageID)
+		_, _ = fmt.Fprint(dockerCli.Out(), imageID)
 	}
 
 	if options.imageIDFile != "" {

cli/command/image/build/context.go

@@ -232,6 +232,7 @@ func GetContextFromURL(out io.Writer, remoteURL, dockerfileName string) (io.Read
 // 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
 	if resp, err = http.Get(url); err != nil {
 		return nil, err
 	}
@@ -344,7 +345,6 @@ func getDockerfileRelPath(absContextDir, givenDockerfile string) (string, error)
 		absDockerfile, err = filepath.EvalSymlinks(absDockerfile)
 		if err != nil {
 			return "", errors.Errorf("unable to evaluate symlinks in Dockerfile path: %v", err)
-
 		}
 	}