Migrate from staticcheck to golangci-lint

Also fix issues reported by the current configuration of golangci-lint.
yyuuttaaoo · Sep 16, 2019 · 4e6e3de · 4e6e3de
1 parent e8e539a
commit 4e6e3de
Show file tree

Hide file tree

Showing 44 changed files with 561 additions and 529 deletions.
diff --git a/.golangci.yaml b/.golangci.yaml
@@ -0,0 +1,27 @@
+run:
+  deadline: 5m
+
+  skip-dirs:
+    - internal
+
+issues:
+  exclude-rules:
+    - path: _test\.go
+      linters:
+        - bodyclose
+        - goconst
+        - gosec
+        - scopelint
+    - path: testing/.+\.go
+      linters:
+        - gosec
+
+linters:
+  enable-all: true
+  disable:
+    - dupl
+    - errcheck
+    - funlen
+    - gochecknoglobals
+    - goconst
+    - lll
diff --git a/Makefile b/Makefile
@@ -1,6 +1,6 @@
 .PHONY: \
 	all \
-	staticcheck \
+	lint \
 	fmt \
 	fmtcheck \
 	pretest \
@@ -9,9 +9,9 @@
 
 all: test
 
-staticcheck:
-	GO111MODULE=off go get honnef.co/go/tools/cmd/staticcheck
-	staticcheck ./...
+lint:
+	cd /tmp && GO111MODULE=on go get github.com/golangci/golangci-lint/cmd/golangci-lint@latest
+	golangci-lint run
 
 fmtcheck:
 	if [ -z "$${SKIP_FMT_CHECK}" ]; then [ -z "$$(gofumpt -s -d . | tee /dev/stderr)" ]; fi
@@ -23,7 +23,7 @@ fmt:
 testdeps:
 	go mod download
 
-pretest: staticcheck fmtcheck
+pretest: lint fmtcheck
 
 gotest:
 	go test -race -vet all ./...

diff --git a/README.md b/README.md
@@ -118,7 +118,7 @@ All development commands can be seen in the [Makefile](Makefile).
 
 Commited code must pass:
 
-* [staticcheck](https://staticcheck.io/)
+* [golangci-lint](integration_unix_test.go)
 * [gofumpt](https://github.com/mvdan/gofumpt)
 * [go test](https://golang.org/cmd/go/#hdr-Test_packages)
 

diff --git a/auth.go b/auth.go
@@ -12,6 +12,7 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
+	"net/http"
 	"os"
 	"path"
 	"strings"
@@ -219,7 +220,7 @@ func (c *Client) AuthCheck(conf *AuthConfiguration) (AuthStatus, error) {
 	if conf == nil {
 		return authStatus, errors.New("conf is nil")
 	}
-	resp, err := c.do("POST", "/auth", doOptions{data: conf})
+	resp, err := c.do(http.MethodPost, "/auth", doOptions{data: conf})
 	if err != nil {
 		return authStatus, err
 	}

diff --git a/auth_test.go b/auth_test.go
@@ -24,8 +24,8 @@ func TestAuthConfigurationSearchPath(t *testing.T) {
 	}{
 		{"", "", []string{}},
 		{"", "home", []string{path.Join("home", ".docker", "plaintext-passwords.json"), path.Join("home", ".docker", "config.json"), path.Join("home", ".dockercfg")}},
-		{"docker_config", "", []string{path.Join("docker_config", "plaintext-passwords.json"),path.Join("docker_config", "config.json") }},
-		{"a", "b", []string{path.Join("a", "plaintext-passwords.json"),path.Join("a", "config.json"),path.Join("b", ".docker", "plaintext-passwords.json"), path.Join("b", ".docker", "config.json"), path.Join("b", ".dockercfg")}},
+		{"docker_config", "", []string{path.Join("docker_config", "plaintext-passwords.json"), path.Join("docker_config", "config.json")}},
+		{"a", "b", []string{path.Join("a", "plaintext-passwords.json"), path.Join("a", "config.json"), path.Join("b", ".docker", "plaintext-passwords.json"), path.Join("b", ".docker", "config.json"), path.Join("b", ".dockercfg")}},
 	}
 	for _, tt := range testData {
 		tt := tt

diff --git a/client.go b/client.go
@@ -400,7 +400,7 @@ func (c *Client) Ping() error {
 // See https://goo.gl/wYfgY1 for more details.
 func (c *Client) PingWithContext(ctx context.Context) error {
 	path := "/_ping"
-	resp, err := c.do("GET", path, doOptions{context: ctx})
+	resp, err := c.do(http.MethodGet, path, doOptions{context: ctx})
 	if err != nil {
 		return err
 	}
@@ -412,7 +412,7 @@ func (c *Client) PingWithContext(ctx context.Context) error {
 }
 
 func (c *Client) getServerAPIVersionString() (version string, err error) {
-	resp, err := c.do("GET", "/version", doOptions{})
+	resp, err := c.do(http.MethodGet, "/version", doOptions{})
 	if err != nil {
 		return "", err
 	}
@@ -468,7 +468,7 @@ func (c *Client) do(method, path string, doOptions doOptions) (*http.Response, e
 	req.Header.Set("User-Agent", userAgent)
 	if doOptions.data != nil {
 		req.Header.Set("Content-Type", "application/json")
-	} else if method == "POST" {
+	} else if method == http.MethodPost {
 		req.Header.Set("Content-Type", "plain/text")
 	}
 
@@ -523,7 +523,7 @@ func chooseError(ctx context.Context, err error) error {
 }
 
 func (c *Client) stream(method, path string, streamOptions streamOptions) error {
-	if (method == "POST" || method == "PUT") && streamOptions.in == nil {
+	if (method == http.MethodPost || method == http.MethodPut) && streamOptions.in == nil {
 		streamOptions.in = bytes.NewReader(nil)
 	}
 	if path != "/version" && !c.SkipServerVersionCheck && c.expectedAPIVersion == nil {
@@ -532,11 +532,11 @@ func (c *Client) stream(method, path string, streamOptions streamOptions) error
 			return err
 		}
 	}
-	return c.streamUrl(method, c.getURL(path), streamOptions)
+	return c.streamURL(method, c.getURL(path), streamOptions)
 }
 
-func (c *Client) streamUrl(method, url string, streamOptions streamOptions) error {
-	if (method == "POST" || method == "PUT") && streamOptions.in == nil {
+func (c *Client) streamURL(method, url string, streamOptions streamOptions) error {
+	if (method == http.MethodPost || method == http.MethodPut) && streamOptions.in == nil {
 		streamOptions.in = bytes.NewReader(nil)
 	}
 	if !c.SkipServerVersionCheck && c.expectedAPIVersion == nil {
@@ -550,7 +550,7 @@ func (c *Client) streamUrl(method, url string, streamOptions streamOptions) erro
 		return err
 	}
 	req.Header.Set("User-Agent", userAgent)
-	if method == "POST" {
+	if method == http.MethodPost {
 		req.Header.Set("Content-Type", "plain/text")
 	}
 	for key, val := range streamOptions.headers {
@@ -609,18 +609,19 @@ func (c *Client) streamUrl(method, url string, streamOptions streamOptions) erro
 
 			return chooseError(subCtx, err)
 		}
+		defer resp.Body.Close()
 	} else {
 		if resp, err = c.HTTPClient.Do(req.WithContext(subCtx)); err != nil {
 			if strings.Contains(err.Error(), "connection refused") {
 				return ErrConnectionRefused
 			}
 			return chooseError(subCtx, err)
 		}
+		defer resp.Body.Close()
 		if streamOptions.reqSent != nil {
 			close(streamOptions.reqSent)
 		}
 	}
-	defer resp.Body.Close()
 	if resp.StatusCode < 200 || resp.StatusCode >= 400 {
 		return newError(resp)
 	}
@@ -782,6 +783,7 @@ func (c *Client) hijack(method, path string, hijackOptions hijackOptions) (Close
 		//lint:ignore SA1019 this is needed here
 		clientconn := httputil.NewClientConn(dial, nil)
 		defer clientconn.Close()
+		//nolint:bodyclose
 		clientconn.Do(req)
 		if hijackOptions.success != nil {
 			hijackOptions.success <- struct{}{}
@@ -889,16 +891,14 @@ func (c *Client) pathVersionCheck(basepath, queryStr string, requiredAPIVersion
 	if c.requestedAPIVersion != nil {
 		if c.requestedAPIVersion.GreaterThanOrEqualTo(requiredAPIVersion) {
 			return fmt.Sprintf("%s/v%s%s?%s", urlStr, c.requestedAPIVersion, basepath, queryStr), nil
-		} else {
-			return "", fmt.Errorf("API %s requires version %s, requested version %s is insufficient",
-				basepath, requiredAPIVersion, c.requestedAPIVersion)
 		}
+		return "", fmt.Errorf("API %s requires version %s, requested version %s is insufficient",
+			basepath, requiredAPIVersion, c.requestedAPIVersion)
 	}
 	if requiredAPIVersion != nil {
 		return fmt.Sprintf("%s/v%s%s?%s", urlStr, requiredAPIVersion, basepath, queryStr), nil
-	} else {
-		return fmt.Sprintf("%s%s?%s", urlStr, basepath, queryStr), nil
 	}
+	return fmt.Sprintf("%s%s?%s", urlStr, basepath, queryStr), nil
 }
 
 // getFakeNativeURL returns the URL needed to make an HTTP request over a UNIX
@@ -928,7 +928,7 @@ func queryStringVersion(opts interface{}) (string, APIVersion) {
 	if value.Kind() != reflect.Struct {
 		return "", nil
 	}
-	var apiVersion APIVersion = nil
+	var apiVersion APIVersion
 	items := url.Values(map[string][]string{})
 	for i := 0; i < value.NumField(); i++ {
 		field := value.Type().Field(i)
@@ -1008,7 +1008,7 @@ func addQueryStringValue(items url.Values, key string, v reflect.Value) bool {
 		if vLen > 0 {
 			for i := 0; i < vLen; i++ {
 				if addQueryStringValue(items, key, v.Index(i)) {
-					valuesAdded += 1
+					valuesAdded++
 				}
 			}
 		}

diff --git a/client_stress_test.go b/client_stress_test.go
@@ -90,17 +90,17 @@ func TestClientDoConcurrentStress(t *testing.T) {
 			waiters := make(chan CloseWaiter, n)
 			for i := 0; i < n; i++ {
 				path := fmt.Sprintf("/%05d", i)
-				paths = append(paths, "GET"+path)
-				paths = append(paths, "POST"+path)
+				paths = append(paths, http.MethodGet+path)
+				paths = append(paths, http.MethodPost+path)
 				paths = append(paths, "HEAD"+path)
 				wg.Add(1)
 				go func() {
 					defer wg.Done()
-					_, clientErr := client.do("GET", path, doOptions{})
+					_, clientErr := client.do(http.MethodGet, path, doOptions{})
 					if clientErr != nil {
 						errsCh <- clientErr
 					}
-					clientErr = client.stream("POST", path, streamOptions{})
+					clientErr = client.stream(http.MethodPost, path, streamOptions{})
 					if clientErr != nil {
 						errsCh <- clientErr
 					}

diff --git a/client_test.go b/client_test.go
@@ -533,7 +533,7 @@ func TestClientStreamTimeoutNotHit(t *testing.T) {
 		t.Fatal(err)
 	}
 	var w bytes.Buffer
-	err = client.stream("POST", "/image/create", streamOptions{
+	err = client.stream(http.MethodPost, "/image/create", streamOptions{
 		setRawTerminal:    true,
 		stdout:            &w,
 		inactivityTimeout: 300 * time.Millisecond,
@@ -565,7 +565,7 @@ func TestClientStreamInactivityTimeout(t *testing.T) {
 		t.Fatal(err)
 	}
 	var w bytes.Buffer
-	err = client.stream("POST", "/image/create", streamOptions{
+	err = client.stream(http.MethodPost, "/image/create", streamOptions{
 		setRawTerminal:    true,
 		stdout:            &w,
 		inactivityTimeout: 100 * time.Millisecond,
@@ -601,7 +601,7 @@ func TestClientStreamContextDeadline(t *testing.T) {
 	var w bytes.Buffer
 	ctx, cancel := context.WithTimeout(context.Background(), 400*time.Millisecond)
 	defer cancel()
-	err = client.stream("POST", "/image/create", streamOptions{
+	err = client.stream(http.MethodPost, "/image/create", streamOptions{
 		setRawTerminal: true,
 		stdout:         &w,
 		context:        ctx,
@@ -640,7 +640,7 @@ func TestClientStreamContextCancel(t *testing.T) {
 		time.Sleep(200 * time.Millisecond)
 		cancel()
 	}()
-	err = client.stream("POST", "/image/create", streamOptions{
+	err = client.stream(http.MethodPost, "/image/create", streamOptions{
 		setRawTerminal: true,
 		stdout:         &w,
 		context:        ctx,
@@ -698,7 +698,7 @@ func TestClientStreamJSONDecode(t *testing.T) {
 		t.Fatal(err)
 	}
 	var w bytes.Buffer
-	err = client.stream("POST", "/image/create", streamOptions{
+	err = client.stream(http.MethodPost, "/image/create", streamOptions{
 		stdout:         &w,
 		useJSONDecoder: true,
 	})
@@ -754,7 +754,7 @@ func TestClientStreamJSONDecodeWithTerminal(t *testing.T) {
 		t.Fatal(err)
 	}
 	var w terminalBuffer
-	err = client.stream("POST", "/image/create", streamOptions{
+	err = client.stream(http.MethodPost, "/image/create", streamOptions{
 		stdout:         &w,
 		useJSONDecoder: true,
 	})
@@ -785,7 +785,7 @@ func TestClientDoContextDeadline(t *testing.T) {
 	}
 	ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
 	defer cancel()
-	_, err = client.do("POST", "/image/create", doOptions{
+	_, err = client.do(http.MethodPost, "/image/create", doOptions{
 		context: ctx,
 	})
 	if err != context.DeadlineExceeded {
@@ -808,7 +808,7 @@ func TestClientDoContextCancel(t *testing.T) {
 		time.Sleep(100 * time.Millisecond)
 		cancel()
 	}()
-	_, err = client.do("POST", "/image/create", doOptions{
+	_, err = client.do(http.MethodPost, "/image/create", doOptions{
 		context: ctx,
 	})
 	if err != context.Canceled {
@@ -838,7 +838,7 @@ func TestClientStreamTimeoutNativeClient(t *testing.T) {
 		t.Fatal(err)
 	}
 	var w bytes.Buffer
-	err = client.stream("POST", "/image/create", streamOptions{
+	err = client.stream(http.MethodPost, "/image/create", streamOptions{
 		setRawTerminal:    true,
 		stdout:            &w,
 		inactivityTimeout: 50 * time.Millisecond,
@@ -865,7 +865,7 @@ func TestClientStreamJSONDecoderFailingOutputWriter(t *testing.T) {
 		t.Fatal(err)
 	}
 	var w eofWriter
-	err = client.stream("POST", "/image/create", streamOptions{
+	err = client.stream(http.MethodPost, "/image/create", streamOptions{
 		setRawTerminal: true,
 		useJSONDecoder: true,
 		stdout:         &w,

diff --git a/client_unix_test.go b/client_unix_test.go
@@ -40,7 +40,7 @@ func TestNewTSLAPIClientUnixEndpoint(t *testing.T) {
 	if client.endpoint != endpoint {
 		t.Errorf("Expected endpoint %s. Got %s.", endpoint, client.endpoint)
 	}
-	rsp, err := client.do("GET", "/", doOptions{})
+	rsp, err := client.do(http.MethodGet, "/", doOptions{})
 	if err != nil {
 		t.Fatal(err)
 	}