Merge pull request moby#50082 from mmorel-35/go-critic

fix go-critic linter

Author: thaJeztah

.golangci.yml

@@ -27,6 +27,7 @@ linters:
     - fatcontext                # Detects nested contexts in loops and function literals.
     - forbidigo
     - gocheckcompilerdirectives # Detects invalid go compiler directive comments (//go:).
+    - gocritic                  # Detects for bugs, performance and style issues.
     - gosec                     # Detects security problems.
     - govet
     - iface                     # Detects incorrect use of interfaces. Currently only used for "identical" interfaces in the same package.
@@ -103,6 +104,56 @@ linters:
           msg: Add a wrapper to nlwrap.Handle for EINTR handling and update the list in .golangci.yml.
       analyze-types: true
 
+    gocritic:
+      disabled-checks:
+        - appendAssign
+        - appendCombine
+        - assignOp
+        - builtinShadow
+        - builtinShadowDecl
+        - captLocal
+        - commentedOutCode
+        - deferInLoop
+        - dupImport
+        - dupSubExpr
+        - elseif
+        - emptyFallthrough
+        - equalFold
+        - evalOrder
+        - exitAfterDefer
+        - exposedSyncMutex
+        - filepathJoin
+        - hexLiteral
+        - hugeParam
+        - ifElseChain
+        - importShadow
+        - indexAlloc
+        - methodExprCall
+        - nestingReduce
+        - nilValReturn
+        - octalLiteral
+        - paramTypeCombine
+        - preferStringWriter
+        - ptrToRefParam
+        - rangeValCopy
+        - redundantSprint
+        - regexpMust
+        - regexpSimplify
+        - singleCaseSwitch
+        - sloppyReassign
+        - stringXbytes
+        - typeAssertChain
+        - typeDefFirst
+        - typeUnparen
+        - uncheckedInlineErr
+        - unlambda
+        - unnamedResult
+        - unnecessaryDefer
+        - unslice
+        - valSwap
+        - whyNoLint
+      enable-all: true
+
     gosec:
       excludes:
         - G104 # G104: Errors unhandled; (TODO: reduce unhandled errors, or explicitly ignore)

api/server/httputils/form_test.go

@@ -30,7 +30,7 @@ func TestBoolValue(t *testing.T) {
 	for c, e := range cases {
 		v := url.Values{}
 		v.Set("test", c)
-		r, _ := http.NewRequest(http.MethodPost, "", nil)
+		r, _ := http.NewRequest(http.MethodPost, "", http.NoBody)
 		r.Form = v
 
 		a := BoolValue(r, "test")
@@ -41,14 +41,14 @@ func TestBoolValue(t *testing.T) {
 }
 
 func TestBoolValueOrDefault(t *testing.T) {
-	r, _ := http.NewRequest(http.MethodGet, "", nil)
+	r, _ := http.NewRequest(http.MethodGet, "", http.NoBody)
 	if !BoolValueOrDefault(r, "queryparam", true) {
 		t.Fatal("Expected to get true default value, got false")
 	}
 
 	v := url.Values{}
 	v.Set("param", "")
-	r, _ = http.NewRequest(http.MethodGet, "", nil)
+	r, _ = http.NewRequest(http.MethodGet, "", http.NoBody)
 	r.Form = v
 	if BoolValueOrDefault(r, "param", true) {
 		t.Fatal("Expected not to get true")
@@ -66,7 +66,7 @@ func TestInt64ValueOrZero(t *testing.T) {
 	for c, e := range cases {
 		v := url.Values{}
 		v.Set("test", c)
-		r, _ := http.NewRequest(http.MethodPost, "", nil)
+		r, _ := http.NewRequest(http.MethodPost, "", http.NoBody)
 		r.Form = v
 
 		a := Int64ValueOrZero(r, "test")
@@ -86,7 +86,7 @@ func TestInt64ValueOrDefault(t *testing.T) {
 	for c, e := range cases {
 		v := url.Values{}
 		v.Set("test", c)
-		r, _ := http.NewRequest(http.MethodPost, "", nil)
+		r, _ := http.NewRequest(http.MethodPost, "", http.NoBody)
 		r.Form = v
 
 		a, err := Int64ValueOrDefault(r, "test", -1)
@@ -102,7 +102,7 @@ func TestInt64ValueOrDefault(t *testing.T) {
 func TestInt64ValueOrDefaultWithError(t *testing.T) {
 	v := url.Values{}
 	v.Set("test", "invalid")
-	r, _ := http.NewRequest(http.MethodPost, "", nil)
+	r, _ := http.NewRequest(http.MethodPost, "", http.NoBody)
 	r.Form = v
 
 	_, err := Int64ValueOrDefault(r, "test", -1)
@@ -150,7 +150,7 @@ func TestUint32Value(t *testing.T) {
 	}
 	for _, tc := range tests {
 		t.Run(tc.value, func(t *testing.T) {
-			r, _ := http.NewRequest(http.MethodPost, "", nil)
+			r, _ := http.NewRequest(http.MethodPost, "", http.NoBody)
 			r.Form = url.Values{}
 			if tc.value != valueNotSet {
 				r.Form.Set("field", tc.value)

api/server/httputils/httputils_test.go

@@ -33,7 +33,7 @@ func TestJsonContentType(t *testing.T) {
 
 func TestReadJSON(t *testing.T) {
 	t.Run("nil body", func(t *testing.T) {
-		req, err := http.NewRequest(http.MethodPost, "https://example.com/some/path", nil)
+		req, err := http.NewRequest(http.MethodPost, "https://example.com/some/path", http.NoBody)
 		if err != nil {
 			t.Error(err)
 		}

api/server/middleware/version_test.go

@@ -79,7 +79,7 @@ func TestVersionMiddlewareVersion(t *testing.T) {
 	assert.NilError(t, err)
 	h := m.WrapHandler(handler)
 
-	req, _ := http.NewRequest(http.MethodGet, "/containers/json", nil)
+	req, _ := http.NewRequest(http.MethodGet, "/containers/json", http.NoBody)
 	resp := httptest.NewRecorder()
 	ctx := context.Background()
 
@@ -121,15 +121,15 @@ func TestVersionMiddlewareVersion(t *testing.T) {
 func TestVersionMiddlewareWithErrorsReturnsHeaders(t *testing.T) {
 	handler := func(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
 		v := httputils.VersionFromContext(ctx)
-		assert.Check(t, len(v) != 0)
+		assert.Check(t, v != "")
 		return nil
 	}
 
 	m, err := NewVersionMiddleware("1.2.3", api.DefaultVersion, api.MinSupportedAPIVersion)
 	assert.NilError(t, err)
 	h := m.WrapHandler(handler)
 
-	req, _ := http.NewRequest(http.MethodGet, "/containers/json", nil)
+	req, _ := http.NewRequest(http.MethodGet, "/containers/json", http.NoBody)
 	resp := httptest.NewRecorder()
 	ctx := context.Background()
 

api/server/router/image/image_routes.go

@@ -113,7 +113,7 @@ func (ir *imageRouter) postImagesCreate(ctx context.Context, w http.ResponseWrit
 			return errdefs.InvalidParameter(err)
 		}
 
-		if len(comment) == 0 {
+		if comment == "" {
 			comment = "Imported from " + src
 		}
 

api/server/router/volume/volume_routes_test.go

@@ -22,7 +22,7 @@ import (
 func callGetVolume(v *volumeRouter, name string) (*httptest.ResponseRecorder, error) {
 	ctx := context.WithValue(context.Background(), httputils.APIVersionKey{}, clusterVolumesVersion)
 	vars := map[string]string{"name": name}
-	req := httptest.NewRequest(http.MethodGet, fmt.Sprintf("/volumes/%s", name), nil)
+	req := httptest.NewRequest(http.MethodGet, fmt.Sprintf("/volumes/%s", name), http.NoBody)
 	resp := httptest.NewRecorder()
 
 	err := v.getVolumeByName(ctx, resp, req, vars)
@@ -32,7 +32,7 @@ func callGetVolume(v *volumeRouter, name string) (*httptest.ResponseRecorder, er
 func callListVolumes(v *volumeRouter) (*httptest.ResponseRecorder, error) {
 	ctx := context.WithValue(context.Background(), httputils.APIVersionKey{}, clusterVolumesVersion)
 	vars := map[string]string{}
-	req := httptest.NewRequest(http.MethodGet, "/volumes", nil)
+	req := httptest.NewRequest(http.MethodGet, "/volumes", http.NoBody)
 	resp := httptest.NewRecorder()
 
 	err := v.getVolumesList(ctx, resp, req, vars)
@@ -428,7 +428,7 @@ func TestVolumeRemove(t *testing.T) {
 	}
 
 	ctx := context.WithValue(context.Background(), httputils.APIVersionKey{}, clusterVolumesVersion)
-	req := httptest.NewRequest(http.MethodDelete, "/volumes/vol1", nil)
+	req := httptest.NewRequest(http.MethodDelete, "/volumes/vol1", http.NoBody)
 	resp := httptest.NewRecorder()
 
 	err := v.deleteVolumes(ctx, resp, req, map[string]string{"name": "vol1"})
@@ -455,7 +455,7 @@ func TestVolumeRemoveSwarm(t *testing.T) {
 	}
 
 	ctx := context.WithValue(context.Background(), httputils.APIVersionKey{}, clusterVolumesVersion)
-	req := httptest.NewRequest(http.MethodDelete, "/volumes/vol1", nil)
+	req := httptest.NewRequest(http.MethodDelete, "/volumes/vol1", http.NoBody)
 	resp := httptest.NewRecorder()
 
 	err := v.deleteVolumes(ctx, resp, req, map[string]string{"name": "vol1"})
@@ -472,7 +472,7 @@ func TestVolumeRemoveNotFoundNoSwarm(t *testing.T) {
 	}
 
 	ctx := context.WithValue(context.Background(), httputils.APIVersionKey{}, clusterVolumesVersion)
-	req := httptest.NewRequest(http.MethodDelete, "/volumes/vol1", nil)
+	req := httptest.NewRequest(http.MethodDelete, "/volumes/vol1", http.NoBody)
 	resp := httptest.NewRecorder()
 
 	err := v.deleteVolumes(ctx, resp, req, map[string]string{"name": "vol1"})
@@ -489,7 +489,7 @@ func TestVolumeRemoveNotFoundNoManager(t *testing.T) {
 	}
 
 	ctx := context.WithValue(context.Background(), httputils.APIVersionKey{}, clusterVolumesVersion)
-	req := httptest.NewRequest(http.MethodDelete, "/volumes/vol1", nil)
+	req := httptest.NewRequest(http.MethodDelete, "/volumes/vol1", http.NoBody)
 	resp := httptest.NewRecorder()
 
 	err := v.deleteVolumes(ctx, resp, req, map[string]string{"name": "vol1"})
@@ -513,7 +513,7 @@ func TestVolumeRemoveFoundNoSwarm(t *testing.T) {
 	}
 
 	ctx := context.WithValue(context.Background(), httputils.APIVersionKey{}, clusterVolumesVersion)
-	req := httptest.NewRequest(http.MethodDelete, "/volumes/vol1", nil)
+	req := httptest.NewRequest(http.MethodDelete, "/volumes/vol1", http.NoBody)
 	resp := httptest.NewRecorder()
 
 	err := v.deleteVolumes(ctx, resp, req, map[string]string{"name": "vol1"})
@@ -536,7 +536,7 @@ func TestVolumeRemoveNoSwarmInUse(t *testing.T) {
 	}
 
 	ctx := context.WithValue(context.Background(), httputils.APIVersionKey{}, clusterVolumesVersion)
-	req := httptest.NewRequest(http.MethodDelete, "/volumes/inuse", nil)
+	req := httptest.NewRequest(http.MethodDelete, "/volumes/inuse", http.NoBody)
 	resp := httptest.NewRecorder()
 
 	err := v.deleteVolumes(ctx, resp, req, map[string]string{"name": "inuse"})
@@ -564,7 +564,7 @@ func TestVolumeRemoveSwarmForce(t *testing.T) {
 	}
 
 	ctx := context.WithValue(context.Background(), httputils.APIVersionKey{}, clusterVolumesVersion)
-	req := httptest.NewRequest(http.MethodDelete, "/volumes/vol1", nil)
+	req := httptest.NewRequest(http.MethodDelete, "/volumes/vol1", http.NoBody)
 	resp := httptest.NewRecorder()
 
 	err := v.deleteVolumes(ctx, resp, req, map[string]string{"name": "vol1"})
@@ -573,7 +573,7 @@ func TestVolumeRemoveSwarmForce(t *testing.T) {
 	assert.Assert(t, cerrdefs.IsConflict(err))
 
 	ctx = context.WithValue(context.Background(), httputils.APIVersionKey{}, clusterVolumesVersion)
-	req = httptest.NewRequest(http.MethodDelete, "/volumes/vol1?force=1", nil)
+	req = httptest.NewRequest(http.MethodDelete, "/volumes/vol1?force=1", http.NoBody)
 	resp = httptest.NewRecorder()
 
 	err = v.deleteVolumes(ctx, resp, req, map[string]string{"name": "vol1"})

api/server/server_test.go

@@ -21,7 +21,7 @@ func TestMiddlewares(t *testing.T) {
 	}
 	srv.UseMiddleware(*m)
 
-	req, _ := http.NewRequest(http.MethodGet, "/containers/json", nil)
+	req, _ := http.NewRequest(http.MethodGet, "/containers/json", http.NoBody)
 	resp := httptest.NewRecorder()
 	ctx := context.Background()
 

api/types/events/events.go

@@ -112,11 +112,14 @@ type Actor struct {
 
 // Message represents the information an event contains
 type Message struct {
-	// Deprecated information from JSONMessage.
+	// Deprecated: use Action instead.
+	// Information from JSONMessage.
 	// With data only in container events.
-	Status string `json:"status,omitempty"` // Deprecated: use Action instead.
-	ID     string `json:"id,omitempty"`     // Deprecated: use Actor.ID instead.
-	From   string `json:"from,omitempty"`   // Deprecated: use Actor.Attributes["image"] instead.
+	Status string `json:"status,omitempty"`
+	// Deprecated: use Actor.ID instead.
+	ID string `json:"id,omitempty"`
+	// Deprecated: use Actor.Attributes["image"] instead.
+	From string `json:"from,omitempty"`
 
 	Type   Type
 	Action Action

builder/builder-next/builder.go

@@ -368,7 +368,7 @@ func (b *Builder) Build(ctx context.Context, opt backend.BuildConfig) (*builder.
 	ulimits, err := toBuildkitUlimits(opt.Options.Ulimits)
 	if err != nil {
 		return nil, err
-	} else if len(ulimits) > 0 {
+	} else if ulimits != "" {
 		frontendAttrs["ulimit"] = ulimits
 	}
 

builder/dockerfile/imageprobe.go

@@ -60,7 +60,7 @@ func (c *imageProber) Probe(parentID string, runConfig *container.Config, platfo
 	if err != nil {
 		return "", err
 	}
-	if len(cacheID) == 0 {
+	if cacheID == "" {
 		log.G(context.TODO()).Debugf("[BUILDER] Cache miss: %s", runConfig.Cmd)
 		c.cacheBusted = true
 		return "", nil