chore: enable and fix several revive rules

.golangci.yml

@@ -91,11 +91,35 @@ linters:
     revive:
       enable-all-rules: false
       enable-default-rules: true
+      max-open-files: 2048
       rules:
+        - name: early-return
+          arguments:
+            - "preserve-scope"
+        - name: empty-block
+          disabled: true
         - name: errorf
+        - name: exported
+          disabled: true
         - name: if-return
+        - name: indent-error-flow
+          arguments:
+            - "preserve-scope"
+        - name: redefines-builtin-id
+          disabled: true
+        - name: unused-parameter
+          disabled: true
         - name: unnecessary-format
+        - name: unnecessary-if
+        - name: unnecessary-stmt
         - name: use-errors-new
+        - name: use-fmt-print
+        - name: useless-break
+        - name: superfluous-else
+          arguments:
+            - "preserve-scope"
+        - name: var-naming
+          disabled: true
     staticcheck:
       checks:
         - all
@@ -113,27 +137,6 @@ linters:
       - legacy
       - std-error-handling
     rules:
-      - linters:
-          - revive
-        text: stutters
-      - linters:
-          - revive
-        text: empty-block
-      - linters:
-          - revive
-        text: superfluous-else
-      - linters:
-          - revive
-        text: unused-parameter
-      - linters:
-          - revive
-        text: redefines-builtin-id
-      - linters:
-          - revive
-        text: if-return
-      - linters:
-          - revive
-        text: var-naming
       - linters:
           - staticcheck
         text: "SA1019: .* is deprecated: .*in-toto/attestation"

cache/blobs.go

@@ -341,11 +341,7 @@ func (sr *immutableRef) setBlob(ctx context.Context, desc ocispecs.Descriptor) (
 	sr.queueMediaType(desc.MediaType)
 	sr.queueBlobSize(desc.Size)
 	sr.appendURLs(desc.URLs)
-	if err := sr.commitMetadata(); err != nil {
-		return err
-	}
-
-	return nil
+	return sr.commitMetadata()
 }
 
 func (sr *immutableRef) computeChainMetadata(ctx context.Context, filter map[string]struct{}) error {
@@ -381,11 +377,11 @@ func (sr *immutableRef) computeChainMetadata(ctx context.Context, filter map[str
 			} else {
 				return errors.Errorf("failed to set chain for reference with non-addressable parent %q", sr.layerParent.GetDescription())
 			}
-			if parentBlobChainID := sr.layerParent.getBlobChainID(); parentBlobChainID != "" {
-				blobChainID = parentBlobChainID
-			} else {
+			parentBlobChainID := sr.layerParent.getBlobChainID()
+			if parentBlobChainID == "" {
 				return errors.Errorf("failed to set blobchain for reference with non-addressable parent %q", sr.layerParent.GetDescription())
 			}
+			blobChainID = parentBlobChainID
 		}
 		diffID := sr.getDiffID()
 		chainID = imagespecidentity.ChainID([]digest.Digest{chainID, diffID})
@@ -426,10 +422,7 @@ func (sr *immutableRef) computeChainMetadata(ctx context.Context, filter map[str
 
 	sr.queueChainID(chainID)
 	sr.queueBlobChainID(blobChainID)
-	if err := sr.commitMetadata(); err != nil {
-		return err
-	}
-	return nil
+	return sr.commitMetadata()
 }
 
 func isTypeWindows(sr *immutableRef) bool {

cache/manager.go

@@ -451,20 +451,19 @@ func (cm *cacheManager) getRecord(ctx context.Context, id string, opts ...RefOpt
 			mutable.equalImmutable = &immutableRef{cacheRecord: rec}
 			cm.records[id] = rec
 			return rec, nil
-		} else if IsNotFound(err) {
-			// The equal mutable for this ref is not found, check to see if our snapshot exists
-			if _, statErr := cm.Snapshotter.Stat(ctx, md.getSnapshotID()); statErr != nil {
-				// this ref's snapshot also doesn't exist, just remove this record
-				cm.MetadataStore.Clear(id)
-				return nil, errors.Wrap(errNotFound, id)
-			}
-			// Our snapshot exists, so there may have been a crash while finalizing this ref.
-			// Clear the equal mutable field and continue using this ref.
-			md.clearEqualMutable()
-			md.commitMetadata()
-		} else {
+		} else if !IsNotFound(err) {
 			return nil, err
 		}
+		// The equal mutable for this ref is not found, check to see if our snapshot exists
+		if _, statErr := cm.Snapshotter.Stat(ctx, md.getSnapshotID()); statErr != nil {
+			// this ref's snapshot also doesn't exist, just remove this record
+			cm.MetadataStore.Clear(id)
+			return nil, errors.Wrap(errNotFound, id)
+		}
+		// Our snapshot exists, so there may have been a crash while finalizing this ref.
+		// Clear the equal mutable field and continue using this ref.
+		md.clearEqualMutable()
+		md.commitMetadata()
 	}
 
 	rec := &cacheRecord{

cache/metadata/metadata.go

@@ -115,27 +115,23 @@ func (s *Store) Search(ctx context.Context, index string, prefix bool) ([]*Stora
 		}
 		c := b.Cursor()
 		k, _ := c.Seek([]byte(index))
-		for {
-			if k != nil && strings.HasPrefix(string(k), index) {
-				idx := strings.LastIndex(string(k), "::")
-				if idx == -1 {
-					continue
-				}
-				itemID := string(k[idx+2:])
-				k, _ = c.Next()
-				b := main.Bucket([]byte(itemID))
-				if b == nil {
-					bklog.G(ctx).Errorf("index pointing to missing record %s", itemID)
-					continue
-				}
-				si, err := newStorageItem(itemID, b, s)
-				if err != nil {
-					return err
-				}
-				out = append(out, si)
-			} else {
-				break
+		for k != nil && strings.HasPrefix(string(k), index) {
+			idx := strings.LastIndex(string(k), "::")
+			if idx == -1 {
+				continue
+			}
+			itemID := string(k[idx+2:])
+			k, _ = c.Next()
+			b := main.Bucket([]byte(itemID))
+			if b == nil {
+				bklog.G(ctx).Errorf("index pointing to missing record %s", itemID)
+				continue
 			}
+			si, err := newStorageItem(itemID, b, s)
+			if err != nil {
+				return err
+			}
+			out = append(out, si)
 		}
 		return nil
 	})

cache/refs.go

@@ -1415,10 +1415,7 @@ func (sr *immutableRef) unlazyLayer(ctx context.Context, dhs DescHandlers, pg pr
 	}
 	sr.queueBlobOnly(false)
 	sr.queueSize(sizeUnknown)
-	if err := sr.commitMetadata(); err != nil {
-		return err
-	}
-	return nil
+	return sr.commitMetadata()
 }
 
 func (sr *immutableRef) Release(ctx context.Context) error {

client/client_fileop_test.go

@@ -781,29 +781,26 @@ func testMoveParentDir(t *testing.T, sb integration.Sandbox) {
 			if err == nil && strings.Contains(key, "/wd") {
 				ok = true
 				break
-			} else {
-				ok = false
 			}
+			ok = false
 		}
 		require.True(t, ok)
 
 		for key := range m {
 			if err == nil && strings.Contains(key, "/foo2/bar") {
 				ok = true
 				break
-			} else {
-				ok = false
 			}
+			ok = false
 		}
 		require.True(t, ok)
 
 		for key := range m {
 			if err == nil && strings.Contains(key, "/foo2") {
 				ok = true
 				break
-			} else {
-				ok = false
 			}
+			ok = false
 		}
 		require.True(t, ok)
 	case "busybox:latest":

control/control.go

@@ -338,10 +338,7 @@ func (c *Controller) ListenBuildHistory(req *controlapi.BuildHistoryRequest, srv
 		return err
 	}
 	return c.history.Listen(srv.Context(), req, func(h *controlapi.BuildHistoryEvent) error {
-		if err := srv.Send(h); err != nil {
-			return err
-		}
-		return nil
+		return srv.Send(h)
 	})
 }
 

executor/oci/spec.go

@@ -163,11 +163,11 @@ func GenerateSpec(ctx context.Context, meta executor.Meta, mounts []executor.Mou
 	)
 
 	if cdiManager != nil {
-		if cdiOpts, err := generateCDIOpts(cdiManager, meta.CDIDevices); err == nil {
-			opts = append(opts, cdiOpts...)
-		} else {
+		cdiOpts, err := generateCDIOpts(cdiManager, meta.CDIDevices)
+		if err != nil {
 			return nil, nil, err
 		}
+		opts = append(opts, cdiOpts...)
 	}
 
 	s, err := oci.GenerateSpec(ctx, nil, c, opts...)

exporter/local/export.go

@@ -194,10 +194,7 @@ func (e *localExporterInstance) Export(ctx context.Context, inp *exporter.Source
 
 			progress, closeProgress := NewProgressHandler(ctx, lbl)
 			defer closeProgress()
-			if err := filesync.CopyToCaller(ctx, outputFS, e.id, caller, progress); err != nil {
-				return err
-			}
-			return nil
+			return filesync.CopyToCaller(ctx, outputFS, e.id, caller, progress)
 		}
 	}
 

frontend/dockerfile/builder/caps.go

@@ -25,11 +25,10 @@ func validateCaps(req string) (forward bool, err error) {
 		parts := strings.SplitN(c, "+", 2)
 		if _, ok := enabledCaps[parts[0]]; !ok {
 			err = stack.Enable(grpcerrors.WrapCode(errdefs.NewUnsupportedFrontendCapError(parts[0]), codes.Unimplemented))
-			if strings.Contains(c, "+forward") {
-				forward = true
-			} else {
+			if !strings.Contains(c, "+forward") {
 				return false, err
 			}
+			forward = true
 		}
 	}
 	return

frontend/dockerfile/instructions/commands.go

@@ -370,10 +370,7 @@ type RunCommand struct {
 }
 
 func (c *RunCommand) Expand(expander SingleWordExpander) error {
-	if err := setMountState(c, expander); err != nil {
-		return err
-	}
-	return nil
+	return setMountState(c, expander)
 }
 
 // CmdCommand sets the default command to run in the container on start.

frontend/dockerfile/instructions/commands_runmount.go

@@ -161,9 +161,8 @@ func parseMount(val string, expander SingleWordExpander) (*Mount, error) {
 				if m.Type == MountTypeSecret || m.Type == MountTypeSSH {
 					m.Required = true
 					continue
-				} else {
-					return nil, errors.Errorf("unexpected key '%s' for mount type '%s'", key, m.Type)
 				}
+				return nil, errors.Errorf("unexpected key '%s' for mount type '%s'", key, m.Type)
 			default:
 				// any other option requires a value.
 				return nil, errors.Errorf("invalid field '%s' must be a key=value pair", field)
@@ -212,23 +211,21 @@ func parseMount(val string, expander SingleWordExpander) (*Mount, error) {
 			m.ReadOnly = !rw
 			roAuto = false
 		case "required":
-			if m.Type == MountTypeSecret || m.Type == MountTypeSSH {
-				m.Required, err = strconv.ParseBool(value)
-				if err != nil {
-					return nil, errors.Errorf("invalid value for %s: %s", key, value)
-				}
-			} else {
+			if m.Type != MountTypeSecret && m.Type != MountTypeSSH {
 				return nil, errors.Errorf("unexpected key '%s' for mount type '%s'", key, m.Type)
 			}
+			m.Required, err = strconv.ParseBool(value)
+			if err != nil {
+				return nil, errors.Errorf("invalid value for %s: %s", key, value)
+			}
 		case "size":
-			if m.Type == MountTypeTmpfs {
-				m.SizeLimit, err = units.RAMInBytes(value)
-				if err != nil {
-					return nil, errors.Errorf("invalid value for %s: %s", key, value)
-				}
-			} else {
+			if m.Type != MountTypeTmpfs {
 				return nil, errors.Errorf("unexpected key '%s' for mount type '%s'", key, m.Type)
 			}
+			m.SizeLimit, err = units.RAMInBytes(value)
+			if err != nil {
+				return nil, errors.Errorf("invalid value for %s: %s", key, value)
+			}
 		case "id":
 			m.CacheID = value
 		case "sharing":
@@ -280,11 +277,7 @@ func parseMount(val string, expander SingleWordExpander) (*Mount, error) {
 	}
 
 	if roAuto {
-		if m.Type == MountTypeCache || m.Type == MountTypeTmpfs {
-			m.ReadOnly = false
-		} else {
-			m.ReadOnly = true
-		}
+		m.ReadOnly = m.Type != MountTypeCache && m.Type != MountTypeTmpfs
 	}
 
 	if m.Type == MountTypeSecret {

session/auth/authprovider/authprovider.go

@@ -177,11 +177,10 @@ func (ap *authProvider) tlsConfig(host string) (*tls.Config, error) {
 	if len(c.RootCAs) > 0 {
 		systemPool, err := x509.SystemCertPool()
 		if err != nil {
-			if runtime.GOOS == "windows" {
-				systemPool = x509.NewCertPool()
-			} else {
+			if runtime.GOOS != "windows" {
 				return nil, errors.Wrapf(err, "unable to get system cert pool")
 			}
+			systemPool = x509.NewCertPool()
 		}
 		tc.RootCAs = systemPool
 	}