Merge pull request #39964 from thaJeztah/bump_golangci_lint

bump golangci-lint v1.20.0

builder/dockerfile/mockbackend_test.go

@@ -104,7 +104,7 @@ func (i *mockImage) OperatingSystem() string {
 
 func (i *mockImage) MarshalJSON() ([]byte, error) {
 	type rawImage mockImage
-	return json.Marshal(rawImage(*i))
+	return json.Marshal(rawImage(*i)) //nolint:staticcheck
 }
 
 type mockImageCache struct {

client/container_list.go

@@ -35,7 +35,7 @@ func (cli *Client) ContainerList(ctx context.Context, options types.ContainerLis
 	}
 
 	if options.Filters.Len() > 0 {
-		//lint:ignore SA1019 for old code
+		//nolint:staticcheck // ignore SA1019 for old code
 		filterJSON, err := filters.ToParamWithVersion(cli.version, options.Filters)
 
 		if err != nil {

client/events.go

@@ -90,7 +90,7 @@ func buildEventsQueryParams(cliVersion string, options types.EventsOptions) (url
 	}
 
 	if options.Filters.Len() > 0 {
-		//lint:ignore SA1019 for old code
+		//nolint:staticcheck // ignore SA1019 for old code
 		filterJSON, err := filters.ToParamWithVersion(cliVersion, options.Filters)
 		if err != nil {
 			return nil, err

client/hijack.go

@@ -88,7 +88,7 @@ func (cli *Client) setupHijackConn(ctx context.Context, req *http.Request, proto
 	// Server hijacks the connection, error 'connection closed' expected
 	resp, err := clientconn.Do(req)
 
-	//lint:ignore SA1019 for connecting to old (pre go1.8) daemons
+	//nolint:staticcheck // ignore SA1019 for connecting to old (pre go1.8) daemons
 	if err != httputil.ErrPersistEOF {
 		if err != nil {
 			return nil, err

client/image_list.go

@@ -24,7 +24,7 @@ func (cli *Client) ImageList(ctx context.Context, options types.ImageListOptions
 		}
 	}
 	if optionFilters.Len() > 0 {
-		//lint:ignore SA1019 for old code
+		//nolint:staticcheck // ignore SA1019 for old code
 		filterJSON, err := filters.ToParamWithVersion(cli.version, optionFilters)
 		if err != nil {
 			return images, err

client/network_list.go

@@ -13,7 +13,7 @@ import (
 func (cli *Client) NetworkList(ctx context.Context, options types.NetworkListOptions) ([]types.NetworkResource, error) {
 	query := url.Values{}
 	if options.Filters.Len() > 0 {
-		//lint:ignore SA1019 for old code
+		//nolint:staticcheck // ignore SA1019 for old code
 		filterJSON, err := filters.ToParamWithVersion(cli.version, options.Filters)
 		if err != nil {
 			return nil, err

client/plugin_list.go

@@ -15,7 +15,7 @@ func (cli *Client) PluginList(ctx context.Context, filter filters.Args) (types.P
 	query := url.Values{}
 
 	if filter.Len() > 0 {
-		//lint:ignore SA1019 for old code
+		//nolint:staticcheck // ignore SA1019 for old code
 		filterJSON, err := filters.ToParamWithVersion(cli.version, filter)
 		if err != nil {
 			return plugins, err

client/volume_list.go

@@ -15,7 +15,7 @@ func (cli *Client) VolumeList(ctx context.Context, filter filters.Args) (volumet
 	query := url.Values{}
 
 	if filter.Len() > 0 {
-		//lint:ignore SA1019 for old code
+		//nolint:staticcheck // ignore SA1019 for old code
 		filterJSON, err := filters.ToParamWithVersion(cli.version, filter)
 		if err != nil {
 			return volumes, err

daemon/config/config.go

@@ -493,9 +493,7 @@ func findConfigurationConflicts(config map[string]interface{}, flags *pflag.Flag
 	if len(unknownKeys) > 0 {
 		unknownNamedConflicts := func(f *pflag.Flag) {
 			if namedOption, ok := f.Value.(opts.NamedOption); ok {
-				if _, valid := unknownKeys[namedOption.Name()]; valid {
-					delete(unknownKeys, namedOption.Name())
-				}
+				delete(unknownKeys, namedOption.Name())
 			}
 		}
 		flags.VisitAll(unknownNamedConflicts)

daemon/container_operations.go

@@ -784,9 +784,8 @@ func (daemon *Daemon) connectToNetwork(container *container.Container, idOrName
 		EndpointSettings: endpointConfig,
 		IPAMOperational:  operIPAM,
 	}
-	if _, ok := container.NetworkSettings.Networks[n.ID()]; ok {
-		delete(container.NetworkSettings.Networks, n.ID())
-	}
+
+	delete(container.NetworkSettings.Networks, n.ID())
 
 	if err := daemon.updateEndpointNetworkSettings(container, n, ep); err != nil {
 		return err

daemon/events/testutils/testutils.go

@@ -62,10 +62,9 @@ func Scan(text string) (*events.Message, error) {
 		attrs[kv[0]] = kv[1]
 	}
 
-	tu := time.Unix(t, tn)
 	return &events.Message{
 		Time:     t,
-		TimeNano: tu.UnixNano(),
+		TimeNano: time.Unix(t, tn).UnixNano(),
 		Type:     md["eventType"],
 		Action:   md["action"],
 		Actor: events.Actor{

daemon/graphdriver/aufs/aufs_test.go

@@ -729,9 +729,9 @@ func BenchmarkConcurrentAccess(b *testing.B) {
 
 	numConcurrent := 256
 	// create a bunch of ids
-	var ids []string
+	ids := make([]string, numConcurrent)
 	for i := 0; i < numConcurrent; i++ {
-		ids = append(ids, stringid.GenerateRandomID())
+		ids[i] = stringid.GenerateRandomID()
 	}
 
 	if err := d.Create(ids[0], "", nil); err != nil {
@@ -743,7 +743,7 @@ func BenchmarkConcurrentAccess(b *testing.B) {
 	}
 
 	parent := ids[1]
-	ids = append(ids[2:])
+	ids = ids[2:]
 
 	chErr := make(chan error, numConcurrent)
 	var outerGroup sync.WaitGroup

daemon/graphdriver/aufs/mount.go

@@ -51,7 +51,7 @@ func Unmount(target string) error {
 			continue // try again
 		default:
 			// any other error is fatal
-			break
+			return err
 		}
 	}
 

daemon/graphdriver/btrfs/btrfs.go

@@ -606,7 +606,7 @@ func (d *Driver) parseStorageOpt(storageOpt map[string]string, driver *Driver) e
 
 // Set btrfs storage size
 func (d *Driver) setStorageSize(dir string, driver *Driver) error {
-	if driver.options.size <= 0 {
+	if driver.options.size == 0 {
 		return fmt.Errorf("btrfs: invalid storage size: %s", units.HumanSize(float64(driver.options.size)))
 	}
 	if d.options.minSpace > 0 && driver.options.size < d.options.minSpace {

distribution/push_v2_test.go

@@ -549,7 +549,7 @@ type mockBlobStoreWithCreate struct {
 }
 
 func (blob *mockBlobStoreWithCreate) Create(ctx context.Context, options ...distribution.BlobCreateOption) (distribution.BlobWriter, error) {
-	return nil, errcode.Errors(append([]error{errcode.ErrorCodeUnauthorized.WithMessage("unauthorized")}))
+	return nil, errcode.Errors([]error{errcode.ErrorCodeUnauthorized.WithMessage("unauthorized")})
 }
 
 type mockRepoWithBlob struct {

hack/dockerfile/install/golangci_lint.installer

@@ -1,11 +1,19 @@
 #!/bin/sh
 
-: "${GOLANGCI_LINT_COMMIT=v1.17.1}"
+: "${GOLANGCI_LINT_COMMIT=v1.20.0}"
 
 install_golangci_lint() {
 	echo "Installing golangci-lint version ${GOLANGCI_LINT_COMMIT}"
 	go get -d github.com/golangci/golangci-lint/cmd/golangci-lint
 	cd "$GOPATH/src/github.com/golangci/golangci-lint/" || exit 1
 	git checkout -q "${GOLANGCI_LINT_COMMIT}"
-	go build -buildmode=pie -o "${PREFIX}/golangci-lint" "github.com/golangci/golangci-lint/cmd/golangci-lint"
+
+	version="$(git describe --tags)"
+	commit="$(git rev-parse --short HEAD)"
+	commitDate="$(git show -s --format=%cd)"
+
+	go build \
+		-buildmode=pie \
+		-ldflags "-s -w -X \"main.version=${version}\" -X \"main.commit=${commit}\" -X \"main.date=${commitDate}\"" \
+		-o "${PREFIX}/golangci-lint" "github.com/golangci/golangci-lint/cmd/golangci-lint"
 }

hack/validate/golangci-lint

@@ -6,7 +6,7 @@ SCRIPTDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
 # CI platforms differ, so per-platform GOLANGCI_LINT_OPTS can be set
 # from a platform-specific Dockerfile, otherwise let's just set
 # (somewhat pessimistic) default of 10 minutes.
-: ${GOLANGCI_LINT_OPTS=--deadline=20m}
+: "${GOLANGCI_LINT_OPTS=--timeout=10m}"
 
 [ -n "${TESTDEBUG}" ] && set -x
 
@@ -18,9 +18,10 @@ elif ${PKG_CONFIG} 'libsystemd-journal' 2> /dev/null ; then
 fi
 
 # TODO use --out-format=junit-xml and store artifacts
-GOGC=20 golangci-lint run \
+# shellcheck disable=SC2086
+GOGC=75 golangci-lint run \
 	${GOLANGCI_LINT_OPTS} \
 	--print-resources-usage \
 	--build-tags="${DOCKER_BUILDTAGS}" \
 	--verbose \
-	--config ${SCRIPTDIR}/golangci-lint.yml
+	--config "${SCRIPTDIR}/golangci-lint.yml"

hack/validate/golangci-lint.yml

@@ -1,7 +1,6 @@
 linters:
   enable:
     - deadcode
-    - gofmt
     - goimports
     - golint
     - gosec
@@ -9,7 +8,12 @@ linters:
     - govet
     - ineffassign
     - misspell
+    - staticcheck
+    - structcheck
+    - typecheck
     - unconvert
+    - unused
+    - varcheck
 
   disable:
     - errcheck
@@ -22,28 +26,52 @@ linters:
       - bundles
       - docs
 
-    skip-files:
-      - ".*\\.pb\\.go"
-      - "dockerversion/version_autogen.go"
-      - "api/types/container/container_.*"
-      - "api/types/volume/volume_.*"
-
 linters-settings:
   govet:
     check-shadowing: false
 
 issues:
+  # The default exclusion rules are a bit too permissive, so copying the relevant ones below
+  exclude-use-default: false
+
   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
+
     # Exclude some linters from running on tests files.
     - path: _test\.go
       linters:
         - errcheck
         - gosec
 
-    - text: "G201: SQL string formatting"
+    # Suppress golint complaining about generated types in api/types/
+    - text: "type name will be used as (container|volume)\\.(Container|Volume).* by other packages, and that stutters; consider calling this"
+      path: "api/types/(volume|container)/"
       linters:
-        - gosec
-    - text: "G202: SQL string concatenation"
+        - golint
+    - text: "(G201|G202): SQL string (formatting|concatenation)"
       linters:
         - gosec
     # FIXME temporarily suppress these. See #39928
@@ -51,11 +79,7 @@ issues:
       linters:
         - staticcheck
     # FIXME temporarily suppress these. See #39924
-    - text: "SA1019: h.Xattrs is deprecated: Use PAXRecords instead"
-      linters:
-        - staticcheck
-    # FIXME temporarily suppress these. See #39924
-    - text: "SA1019: hdr.Xattrs is deprecated: Use PAXRecords instead"
+    - text: "SA1019: .*\\.Xattrs is deprecated: Use PAXRecords instead"
       linters:
         - staticcheck
     # FIXME temporarily suppress these. See #39926
@@ -66,3 +90,7 @@ issues:
     - text: "SA1019: httputil.ClientConn is deprecated"
       linters:
         - staticcheck
+    # FIXME temporarily suppress these (related to the ones above)
+    - text: "SA1019: httputil.ErrPersistEOF is deprecated"
+      linters:
+        - staticcheck

pkg/jsonmessage/jsonmessage.go

@@ -139,13 +139,13 @@ type JSONMessage struct {
 	Stream          string        `json:"stream,omitempty"`
 	Status          string        `json:"status,omitempty"`
 	Progress        *JSONProgress `json:"progressDetail,omitempty"`
-	ProgressMessage string        `json:"progress,omitempty"` //deprecated
+	ProgressMessage string        `json:"progress,omitempty"` // deprecated
 	ID              string        `json:"id,omitempty"`
 	From            string        `json:"from,omitempty"`
 	Time            int64         `json:"time,omitempty"`
 	TimeNano        int64         `json:"timeNano,omitempty"`
 	Error           *JSONError    `json:"errorDetail,omitempty"`
-	ErrorMessage    string        `json:"error,omitempty"` //deprecated
+	ErrorMessage    string        `json:"error,omitempty"` // deprecated
 	// Aux contains out-of-band data, such as digests for push signing and image id after building.
 	Aux *json.RawMessage `json:"aux,omitempty"`
 }
@@ -177,7 +177,7 @@ func (jm *JSONMessage) Display(out io.Writer, isTerminal bool) error {
 	if isTerminal && jm.Stream == "" && jm.Progress != nil {
 		clearLine(out)
 		endl = "\r"
-		fmt.Fprintf(out, endl)
+		fmt.Fprint(out, endl)
 	} else if jm.Progress != nil && jm.Progress.String() != "" { //disable progressbar in non-terminal
 		return nil
 	}
@@ -194,7 +194,7 @@ func (jm *JSONMessage) Display(out io.Writer, isTerminal bool) error {
 	}
 	if jm.Progress != nil && isTerminal {
 		fmt.Fprintf(out, "%s %s%s", jm.Status, jm.Progress.String(), endl)
-	} else if jm.ProgressMessage != "" { //deprecated
+	} else if jm.ProgressMessage != "" { // deprecated
 		fmt.Fprintf(out, "%s %s%s", jm.Status, jm.ProgressMessage, endl)
 	} else if jm.Stream != "" {
 		fmt.Fprintf(out, "%s%s", jm.Stream, endl)

pkg/mount/mountinfo_linux.go

@@ -90,7 +90,6 @@ func parseInfoFile(r io.Reader, filter FilterFunc) ([]*Info, error) {
 				   mount propagation flags in fields[6]. The correct
 				   behavior is to ignore any unknown optional fields.
 				*/
-				break
 			}
 		}
 		if i == numFields {