Merge pull request #1024 from Luap99/linters

enable unparam, exportloopref and revive linters

.golangci.yml

@@ -13,6 +13,9 @@ linters:
   enable:
     - dupl
     - gocritic
+    - unparam
+    - exportloopref
+    - revive
 linters-settings:
   errcheck:
     check-type-assertions: true

libimage/load.go

@@ -88,6 +88,8 @@ func (r *Runtime) Load(ctx context.Context, path string, options *LoadOptions) (
 	}
 
 	// Give a decent error message if nothing above worked.
+	// we want the colon here for the multiline error
+	//nolint:revive
 	loadError := fmt.Errorf("payload does not match any of the supported image formats:")
 	for _, err := range loadErrors {
 		loadError = fmt.Errorf("%v\n * %v", loadError, err)

libimage/runtime.go

@@ -232,16 +232,15 @@ func (r *Runtime) LookupImage(name string, options *LookupImageOptions) (*Image,
 		}
 		logrus.Debugf("Found image %q in local containers storage (%s)", name, storageRef.StringWithinTransport())
 		return r.storageToImage(img, storageRef), "", nil
-	} else {
-		// Docker compat: strip off the tag iff name is tagged and digested
-		// (e.g., fedora:latest@sha256...).  In that case, the tag is stripped
-		// off and entirely ignored.  The digest is the sole source of truth.
-		normalizedName, err := normalizeTaggedDigestedString(name)
-		if err != nil {
-			return nil, "", err
-		}
-		name = normalizedName
 	}
+	// Docker compat: strip off the tag iff name is tagged and digested
+	// (e.g., fedora:latest@sha256...).  In that case, the tag is stripped
+	// off and entirely ignored.  The digest is the sole source of truth.
+	normalizedName, err := normalizeTaggedDigestedString(name)
+	if err != nil {
+		return nil, "", err
+	}
+	name = normalizedName
 
 	byDigest := false
 	originalName := name

libnetwork/cni/config.go

@@ -96,7 +96,7 @@ func (n *cniNetwork) networkCreate(newNetwork *types.Network, defaultNet bool) (
 	newNetwork.ID = getNetworkIDFromName(newNetwork.Name)
 
 	// when we do not have ipam we must disable dns
-	internalutil.IpamNoneDisableDns(newNetwork)
+	internalutil.IpamNoneDisableDNS(newNetwork)
 
 	// FIXME: Should this be a hard error?
 	if newNetwork.DNSEnabled && newNetwork.Internal && hasDNSNamePlugin(n.cniPluginDirs) {

libnetwork/cni/run.go

@@ -106,7 +106,7 @@ func (n *cniNetwork) Setup(namespacePath string, options types.SetupOptions) (ma
 }
 
 // CNIResultToStatus convert the cni result to status block
-// nolint:golint
+// nolint:golint,revive
 func CNIResultToStatus(res cnitypes.Result) (types.StatusBlock, error) {
 	result := types.StatusBlock{}
 	cniResult, err := types040.GetResult(res)

libnetwork/etchosts/hosts_test.go

@@ -296,9 +296,8 @@ func TestNew(t *testing.T) {
 			if tt.wantErrString != "" {
 				assert.ErrorContains(t, err, tt.wantErrString)
 				return
-			} else {
-				assert.NoError(t, err, "New() failed")
 			}
+			assert.NoError(t, err, "New() failed")
 
 			content, err := ioutil.ReadFile(targetFile)
 			assert.NoErrorf(t, err, "failed to read target host file: %v", err)
@@ -362,9 +361,8 @@ func TestAdd(t *testing.T) {
 			if tt.wantErrString != "" {
 				assert.ErrorContains(t, err, tt.wantErrString)
 				return
-			} else {
-				assert.NoError(t, err, "Add() failed")
 			}
+			assert.NoError(t, err, "Add() failed")
 
 			content, err := ioutil.ReadFile(hostFile)
 			assert.NoErrorf(t, err, "failed to read host file: %v", err)
@@ -459,9 +457,8 @@ func TestAddIfExists(t *testing.T) {
 			if tt.wantErrString != "" {
 				assert.ErrorContains(t, err, tt.wantErrString)
 				return
-			} else {
-				assert.NoError(t, err, "AddIfExists() failed")
 			}
+			assert.NoError(t, err, "AddIfExists() failed")
 
 			content, err := ioutil.ReadFile(hostFile)
 			assert.NoErrorf(t, err, "failed to read host file: %v", err)

libnetwork/internal/util/create.go

@@ -41,7 +41,7 @@ func CommonNetworkCreate(n NetUtil, network *types.Network) error {
 	return nil
 }
 
-func IpamNoneDisableDns(network *types.Network) {
+func IpamNoneDisableDNS(network *types.Network) {
 	if network.IPAMOptions[types.Driver] == types.NoneIPAMDriver {
 		logrus.Debugf("dns disabled for network %q because ipam driver is set to none", network.Name)
 		network.DNSEnabled = false

libnetwork/netavark/config.go

@@ -121,7 +121,7 @@ func (n *netavarkNetwork) networkCreate(newNetwork *types.Network, defaultNet bo
 	}
 
 	// when we do not have ipam we must disable dns
-	internalutil.IpamNoneDisableDns(newNetwork)
+	internalutil.IpamNoneDisableDNS(newNetwork)
 
 	// add gateway when not internal or dns enabled
 	addGateway := !newNetwork.Internal || newNetwork.DNSEnabled

libnetwork/network/interface.go

@@ -46,6 +46,9 @@ const (
 //   1. read ${graphroot}/defaultNetworkBackend
 //   2. find netavark binary (if not installed use CNI)
 //   3. check containers, images and CNI networks and if there are some we have an existing install and should continue to use CNI
+//
+// revive does not like the name because the package is already called network
+//nolint:revive
 func NetworkBackend(store storage.Store, conf *config.Config, syslog bool) (types.NetworkBackend, types.ContainerNetwork, error) {
 	backend := types.NetworkBackend(conf.Network.NetworkBackend)
 	if backend == "" {

pkg/apparmor/apparmor_linux.go

@@ -251,19 +251,17 @@ func CheckProfileAndLoadDefault(name string) (string, error) {
 	if unshare.IsRootless() {
 		if name != "" {
 			return "", errors.Wrapf(ErrApparmorRootless, "cannot load AppArmor profile %q", name)
-		} else {
-			logrus.Debug("Skipping loading default AppArmor profile (rootless mode)")
-			return "", nil
 		}
+		logrus.Debug("Skipping loading default AppArmor profile (rootless mode)")
+		return "", nil
 	}
 
 	// Check if AppArmor is disabled and error out if a profile is to be set.
 	if !runcaa.IsEnabled() {
 		if name == "" {
 			return "", nil
-		} else {
-			return "", errors.Errorf("profile %q specified but AppArmor is disabled on the host", name)
 		}
+		return "", errors.Errorf("profile %q specified but AppArmor is disabled on the host", name)
 	}
 
 	if name == "" {

pkg/auth/auth.go

@@ -26,8 +26,8 @@ func GetDefaultAuthFile() string {
 	if authfile := os.Getenv("REGISTRY_AUTH_FILE"); authfile != "" {
 		return authfile
 	}
-	if auth_env := os.Getenv("DOCKER_CONFIG"); auth_env != "" {
-		return filepath.Join(auth_env, "config.json")
+	if authEnv := os.Getenv("DOCKER_CONFIG"); authEnv != "" {
+		return filepath.Join(authEnv, "config.json")
 	}
 	return ""
 }
@@ -313,7 +313,7 @@ func Logout(systemContext *types.SystemContext, opts *LogoutOptions, args []stri
 			fmt.Printf("Not logged into %s with current tool. Existing credentials were established via docker login. Please use docker logout instead.\n", key)
 			return nil
 		}
-		return errors.Errorf("Not logged into %s\n", key)
+		return errors.Errorf("not logged into %s", key)
 	default:
 		return errors.Wrapf(err, "logging out of %q", key)
 	}

pkg/config/config.go

@@ -553,6 +553,9 @@ type SecretConfig struct {
 }
 
 // ConfigMapConfig represents the "configmap" TOML config table
+//
+// revive does not like the name because the package is already called config
+//nolint:revive
 type ConfigMapConfig struct {
 	// Driver specifies the configmap driver to use.
 	// Current valid value:
@@ -1215,14 +1218,14 @@ func (c *Config) ActiveDestination() (uri, identity string, err error) {
 // FindHelperBinary will search the given binary name in the configured directories.
 // If searchPATH is set to true it will also search in $PATH.
 func (c *Config) FindHelperBinary(name string, searchPATH bool) (string, error) {
-	dir_list := c.Engine.HelperBinariesDir
+	dirList := c.Engine.HelperBinariesDir
 
 	// If set, search this directory first. This is used in testing.
 	if dir, found := os.LookupEnv("CONTAINERS_HELPER_BINARY_DIR"); found {
-		dir_list = append([]string{dir}, dir_list...)
+		dirList = append([]string{dir}, dirList...)
 	}
 
-	for _, path := range dir_list {
+	for _, path := range dirList {
 		fullpath := filepath.Join(path, name)
 		if fi, err := os.Stat(fullpath); err == nil && fi.Mode().IsRegular() {
 			return fullpath, nil

pkg/configmaps/configmaps.go

@@ -79,6 +79,9 @@ type ConfigMap struct {
 // ConfigMapsDriver interfaces with the configMaps data store.
 // The driver stores the actual bytes of configMap data, as opposed to
 // the configMap metadata.
+//
+// revive does not like the name because the package is already called configmaps
+//nolint:revive
 type ConfigMapsDriver interface {
 	// List lists all configMap ids in the configMaps data store
 	List() ([]string, error)
@@ -272,9 +275,8 @@ func getDriver(name string, opts map[string]string) (ConfigMapsDriver, error) {
 	if name == "file" {
 		if path, ok := opts["path"]; ok {
 			return filedriver.NewDriver(path)
-		} else {
-			return nil, errors.Wrap(errInvalidDriverOpt, "need path for filedriver")
 		}
+		return nil, errors.Wrap(errInvalidDriverOpt, "need path for filedriver")
 	}
 	return nil, errInvalidDriver
 }

pkg/configmaps/configmapsdb.go

@@ -31,9 +31,8 @@ func (s *ConfigMapManager) loadDB() error {
 			// the db cache will show no entries anyway.
 			// The file will be created later on a store()
 			return nil
-		} else {
-			return err
 		}
+		return err
 	}
 
 	// We check if the file has been modified after the last time it was loaded into the cache.

pkg/configmaps/filedriver/filedriver.go

@@ -134,9 +134,8 @@ func (d *Driver) getAllData() (map[string][]byte, error) {
 		if os.IsNotExist(err) {
 			// the file will be created later on a store()
 			return make(map[string][]byte), nil
-		} else {
-			return nil, err
 		}
+		return nil, err
 	}
 
 	file, err := os.Open(d.configMapsDataFilePath)

pkg/filters/filters.go

@@ -36,6 +36,9 @@ func ComputeUntilTimestamp(filterValues []string) (time.Time, error) {
 //
 // Please refer to https://github.com/containers/podman/issues/6899 for some
 // background.
+//
+// revive does not like the name because the package is already called filters
+//nolint:revive
 func FiltersFromRequest(r *http.Request) ([]string, error) {
 	var (
 		compatFilters map[string]map[string]bool

pkg/machine/machine.go

@@ -9,6 +9,8 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
+// TODO: change name to MachineMarker since package is already called machine
+//nolint:revive
 type MachineMarker struct {
 	Enabled bool
 	Type    string
@@ -54,6 +56,8 @@ func IsPodmanMachine() bool {
 	return GetMachineMarker().Enabled
 }
 
+// TODO: change name to HostType since package is already called machine
+//nolint:revive
 func MachineHostType() string {
 	return GetMachineMarker().Type
 }

pkg/retry/retry.go

@@ -17,12 +17,17 @@ import (
 )
 
 // RetryOptions defines the option to retry
+// revive does not like the name because the package is already called retry
+//nolint:revive
 type RetryOptions struct {
 	MaxRetry int           // The number of times to possibly retry
 	Delay    time.Duration // The delay to use between retries, if set
 }
 
 // RetryIfNecessary retries the operation in exponential backoff with the retryOptions
+//
+// revive does not like the name because the package is already called retry
+//nolint:revive
 func RetryIfNecessary(ctx context.Context, operation func() error, retryOptions *RetryOptions) error {
 	err := operation()
 	for attempt := 0; err != nil && isRetryable(err) && attempt < retryOptions.MaxRetry; attempt++ {

pkg/seccomp/filter.go

@@ -130,7 +130,7 @@ func matchSyscall(filter *libseccomp.ScmpFilter, call *Syscall) error {
 				return errors.Wrapf(err, "create seccomp syscall condition for syscall %s", call.Name)
 			}
 
-			argCounts[cond.Index] += 1
+			argCounts[cond.Index]++
 
 			conditions = append(conditions, newCond)
 		}

pkg/secrets/filedriver/filedriver.go

@@ -134,9 +134,8 @@ func (d *Driver) getAllData() (map[string][]byte, error) {
 		if os.IsNotExist(err) {
 			// the file will be created later on a store()
 			return make(map[string][]byte), nil
-		} else {
-			return nil, err
 		}
+		return nil, err
 	}
 
 	file, err := os.Open(d.secretsDataFilePath)

pkg/secrets/secrets.go

@@ -53,6 +53,9 @@ var secretsFile = "secrets.json"
 var secretNameRegexp = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9_.-]*$`)
 
 // SecretsManager holds information on handling secrets
+//
+// revive does not like the name because the package is already called secrets
+//nolint:revive
 type SecretsManager struct {
 	// secretsPath is the path to the db file where secrets are stored
 	secretsDBPath string
@@ -82,6 +85,9 @@ type Secret struct {
 // The driver stores the actual bytes of secret data, as opposed to
 // the secret metadata.
 // Currently only the unencrypted filedriver is implemented.
+//
+// revive does not like the name because the package is already called secrets
+//nolint:revive
 type SecretsDriver interface {
 	// List lists all secret ids in the secrets data store
 	List() ([]string, error)
@@ -276,9 +282,8 @@ func getDriver(name string, opts map[string]string) (SecretsDriver, error) {
 	case "file":
 		if path, ok := opts["path"]; ok {
 			return filedriver.NewDriver(path)
-		} else {
-			return nil, errors.Wrap(errInvalidDriverOpt, "need path for filedriver")
 		}
+		return nil, errors.Wrap(errInvalidDriverOpt, "need path for filedriver")
 	case "pass":
 		return passdriver.NewDriver(opts)
 	case "shell":

pkg/secrets/secretsdb.go

@@ -31,9 +31,8 @@ func (s *SecretsManager) loadDB() error {
 			// the db cache will show no entries anyway.
 			// The file will be created later on a store()
 			return nil
-		} else {
-			return err
 		}
+		return err
 	}
 
 	// We check if the file has been modified after the last time it was loaded into the cache.

pkg/sysinfo/nummem_linux.go

@@ -12,6 +12,8 @@ import (
 // NUMANodeCount queries the system for the count of Memory Nodes available
 // for use to this process.
 func NUMANodeCount() int {
+	// this is the correct flag name (not defined in the unix package)
+	//nolint:revive
 	MPOL_F_MEMS_ALLOWED := (1 << 2)
 	var mask [1024 / 64]uintptr
 	_, _, err := unix.RawSyscall6(unix.SYS_GET_MEMPOLICY, 0, uintptr(unsafe.Pointer(&mask[0])), uintptr(len(mask)*8), 0, uintptr(MPOL_F_MEMS_ALLOWED), 0)