Merge pull request #13997 from Luap99/gocritic

enable gocritic linter

.golangci.yml

@@ -46,7 +46,6 @@ linters:
     - lll
     - unconvert
     - errcheck
-    - gocritic
     - gosec
     - maligned
     - gomoddirectives

cmd/podman/common/completion.go

@@ -327,7 +327,7 @@ func suffixCompSlice(suf string, slice []string) []string {
 		if len(split) > 1 {
 			slice[i] = split[0] + suf + "\t" + split[1]
 		} else {
-			slice[i] = slice[i] + suf
+			slice[i] += suf
 		}
 	}
 	return slice
@@ -647,7 +647,10 @@ func AutocompleteInspect(cmd *cobra.Command, args []string, toComplete string) (
 	pods, _ := getPods(cmd, toComplete, completeDefault)
 	networks, _ := getNetworks(cmd, toComplete, completeDefault)
 	volumes, _ := getVolumes(cmd, toComplete)
-	suggestions := append(containers, images...)
+
+	suggestions := make([]string, 0, len(containers)+len(images)+len(pods)+len(networks)+len(volumes))
+	suggestions = append(suggestions, containers...)
+	suggestions = append(suggestions, images...)
 	suggestions = append(suggestions, pods...)
 	suggestions = append(suggestions, networks...)
 	suggestions = append(suggestions, volumes...)
@@ -961,6 +964,8 @@ func AutocompleteFormat(o interface{}) func(cmd *cobra.Command, args []string, t
 	// this function provides shell completion for go templates
 	return func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
 		// autocomplete json when nothing or json is typed
+		// gocritic complains about the argument order but this is correct in this case
+		//nolint:gocritic
 		if strings.HasPrefix("json", toComplete) {
 			return []string{"json"}, cobra.ShellCompDirectiveNoFileComp
 		}

cmd/podman/common/create.go

@@ -896,7 +896,7 @@ func DefineCreateFlags(cmd *cobra.Command, cf *entities.ContainerCreateOptions,
 		)
 		_ = cmd.RegisterFlagCompletionFunc(memorySwappinessFlagName, completion.AutocompleteNone)
 	}
-	//anyone can use these
+	// anyone can use these
 	cpusFlagName := "cpus"
 	createFlags.Float64Var(
 		&cf.CPUS,

cmd/podman/completion/completion.go

@@ -31,7 +31,7 @@ var (
 		Example: `podman completion bash
   podman completion zsh -f _podman
   podman completion fish --no-desc`,
-		//don't show this command to users
+		// don't show this command to users
 		Hidden: true,
 	}
 )

cmd/podman/images/scp.go

@@ -121,13 +121,8 @@ func scp(cmd *cobra.Command, args []string) (finalErr error) {
 			return err
 		}
 		if flipConnections { // the order of cliConnections matters, we need to flip both arrays since the args are parsed separately sometimes.
-			connect := cliConnections[0]
-			cliConnections[0] = cliConnections[1]
-			cliConnections[1] = connect
-
-			loc := locations[0]
-			locations[0] = locations[1]
-			locations[1] = loc
+			cliConnections[0], cliConnections[1] = cliConnections[1], cliConnections[0]
+			locations[0], locations[1] = locations[1], locations[0]
 		}
 		dest = *locations[1]
 	case len(locations) == 1:

cmd/podman/pods/create.go

@@ -224,7 +224,8 @@ func create(cmd *cobra.Command, args []string) error {
 	}
 	sort.Ints(vals)
 	for ind, core := range vals {
-		if core > int(cpuSet) {
+		switch {
+		case core > int(cpuSet):
 			if copy == "" {
 				copy = "0-" + strconv.Itoa(int(cpuSet))
 				infraOptions.CPUSetCPUs = copy
@@ -233,9 +234,9 @@ func create(cmd *cobra.Command, args []string) error {
 				infraOptions.CPUSetCPUs = copy
 				break
 			}
-		} else if ind != 0 {
+		case ind != 0:
 			copy += "," + strconv.Itoa(core)
-		} else {
+		default:
 			copy = "" + strconv.Itoa(core)
 		}
 	}

cmd/podman/secrets/create.go

@@ -62,13 +62,14 @@ func create(cmd *cobra.Command, args []string) error {
 	path := args[1]
 
 	var reader io.Reader
-	if env {
+	switch {
+	case env:
 		envValue := os.Getenv(path)
 		if envValue == "" {
 			return errors.Errorf("cannot create store secret data: environment variable %s is not set", path)
 		}
 		reader = strings.NewReader(envValue)
-	} else if path == "-" || path == "/dev/stdin" {
+	case path == "-" || path == "/dev/stdin":
 		stat, err := os.Stdin.Stat()
 		if err != nil {
 			return err
@@ -77,7 +78,7 @@ func create(cmd *cobra.Command, args []string) error {
 			return errors.New("if `-` is used, data must be passed into stdin")
 		}
 		reader = os.Stdin
-	} else {
+	default:
 		file, err := os.Open(path)
 		if err != nil {
 			return err

cmd/podman/system/migrate.go

@@ -69,6 +69,10 @@ func migrate(cmd *cobra.Command, args []string) {
 	err = engine.Migrate(registry.Context(), cmd.Flags(), registry.PodmanConfig(), migrateOptions)
 	if err != nil {
 		fmt.Println(err)
+
+		// FIXME change this to return the error like other commands
+		// defer will never run on os.Exit()
+		//nolint:gocritic
 		os.Exit(define.ExecErrorCodeGeneric)
 	}
 	os.Exit(0)

cmd/podman/system/renumber.go

@@ -56,6 +56,9 @@ func renumber(cmd *cobra.Command, args []string) {
 	err = engine.Renumber(registry.Context(), cmd.Flags(), registry.PodmanConfig())
 	if err != nil {
 		fmt.Println(err)
+		// FIXME change this to return the error like other commands
+		// defer will never run on os.Exit()
+		//nolint:gocritic
 		os.Exit(define.ExecErrorCodeGeneric)
 	}
 	os.Exit(0)

cmd/podman/system/reset.go

@@ -95,6 +95,9 @@ func reset(cmd *cobra.Command, args []string) {
 
 	if err := engine.Reset(registry.Context()); err != nil {
 		logrus.Error(err)
+		// FIXME change this to return the error like other commands
+		// defer will never run on os.Exit()
+		//nolint:gocritic
 		os.Exit(define.ExecErrorCodeGeneric)
 	}
 	os.Exit(0)

cmd/podman/utils/alias.go

@@ -37,8 +37,7 @@ func AliasFlags(f *pflag.FlagSet, name string) pflag.NormalizedName {
 
 // TimeoutAliasFlags is a function to handle backwards compatibility with old timeout flags
 func TimeoutAliasFlags(f *pflag.FlagSet, name string) pflag.NormalizedName {
-	switch name {
-	case "timeout":
+	if name == "timeout" {
 		name = "time"
 	}
 	return pflag.NormalizedName(name)

libpod/container.go

@@ -1180,7 +1180,7 @@ func (c *Container) Umask() string {
 	return c.config.Umask
 }
 
-//Secrets return the secrets in the container
+// Secrets return the secrets in the container
 func (c *Container) Secrets() []*ContainerSecret {
 	return c.config.Secrets
 }

libpod/container_api.go

@@ -229,8 +229,7 @@ func (c *Container) Kill(signal uint) error {
 // This function returns when the attach finishes. It does not hold the lock for
 // the duration of its runtime, only using it at the beginning to verify state.
 func (c *Container) Attach(streams *define.AttachStreams, keys string, resize <-chan define.TerminalSize) error {
-	switch c.LogDriver() {
-	case define.PassthroughLogging:
+	if c.LogDriver() == define.PassthroughLogging {
 		return errors.Wrapf(define.ErrNoLogs, "this container is using the 'passthrough' log driver, cannot attach")
 	}
 	if !c.batched {

libpod/container_exec.go

@@ -817,16 +817,16 @@ func (c *Container) Exec(config *ExecConfig, streams *define.AttachStreams, resi
 // Please be careful when using this function since it might temporarily unlock
 // the container when os.RemoveAll($bundlePath) fails with ENOTEMPTY or EBUSY
 // errors.
-func (c *Container) cleanupExecBundle(sessionID string) (Err error) {
+func (c *Container) cleanupExecBundle(sessionID string) (err error) {
 	path := c.execBundlePath(sessionID)
 	for attempts := 0; attempts < 50; attempts++ {
-		Err = os.RemoveAll(path)
-		if Err == nil || os.IsNotExist(Err) {
+		err = os.RemoveAll(path)
+		if err == nil || os.IsNotExist(err) {
 			return nil
 		}
-		if pathErr, ok := Err.(*os.PathError); ok {
-			Err = pathErr.Err
-			if errors.Cause(Err) == unix.ENOTEMPTY || errors.Cause(Err) == unix.EBUSY {
+		if pathErr, ok := err.(*os.PathError); ok {
+			err = pathErr.Err
+			if errors.Cause(err) == unix.ENOTEMPTY || errors.Cause(err) == unix.EBUSY {
 				// give other processes a chance to use the container
 				if !c.batched {
 					if err := c.save(); err != nil {

libpod/container_internal_linux.go

@@ -505,8 +505,7 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) {
 			}
 
 			for _, o := range namedVol.Options {
-				switch o {
-				case "U":
+				if o == "U" {
 					if err := c.ChangeHostPathOwnership(mountPoint, true, int(hostUID), int(hostGID)); err != nil {
 						return nil, err
 					}
@@ -596,8 +595,7 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) {
 
 		// Check overlay volume options
 		for _, o := range overlayVol.Options {
-			switch o {
-			case "U":
+			if o == "U" {
 				if err := c.ChangeHostPathOwnership(overlayVol.Source, true, int(hostUID), int(hostGID)); err != nil {
 					return nil, err
 				}
@@ -2144,11 +2142,9 @@ func (c *Container) makeBindMounts() error {
 				return err
 			}
 		}
-	} else {
-		if !c.config.UseImageHosts && c.state.BindMounts["/etc/hosts"] == "" {
-			if err := c.createHosts(); err != nil {
-				return errors.Wrapf(err, "error creating hosts file for container %s", c.ID())
-			}
+	} else if !c.config.UseImageHosts && c.state.BindMounts["/etc/hosts"] == "" {
+		if err := c.createHosts(); err != nil {
+			return errors.Wrapf(err, "error creating hosts file for container %s", c.ID())
 		}
 	}
 
@@ -2267,7 +2263,7 @@ rootless=%d
 			base := "/run/secrets"
 			if secret.Target != "" {
 				secretFileName = secret.Target
-				//If absolute path for target given remove base.
+				// If absolute path for target given remove base.
 				if filepath.IsAbs(secretFileName) {
 					base = ""
 				}
@@ -2351,21 +2347,21 @@ func (c *Container) generateResolvConf() (string, error) {
 		return "", errors.Wrapf(err, "error parsing host resolv.conf")
 	}
 
-	dns := make([]net.IP, 0, len(c.runtime.config.Containers.DNSServers))
+	dns := make([]net.IP, 0, len(c.runtime.config.Containers.DNSServers)+len(c.config.DNSServer))
 	for _, i := range c.runtime.config.Containers.DNSServers {
 		result := net.ParseIP(i)
 		if result == nil {
 			return "", errors.Wrapf(define.ErrInvalidArg, "invalid IP address %s", i)
 		}
 		dns = append(dns, result)
 	}
-	dnsServers := append(dns, c.config.DNSServer...)
+	dns = append(dns, c.config.DNSServer...)
 	// If the user provided dns, it trumps all; then dns masq; then resolv.conf
 	var search []string
 	switch {
-	case len(dnsServers) > 0:
+	case len(dns) > 0:
 		// We store DNS servers as net.IP, so need to convert to string
-		for _, server := range dnsServers {
+		for _, server := range dns {
 			nameservers = append(nameservers, server.String())
 		}
 	default:
@@ -2890,11 +2886,11 @@ func (c *Container) generateUserPasswdEntry(addedUID int) (string, error) {
 
 func (c *Container) passwdEntry(username string, uid, gid, name, homeDir string) string {
 	s := c.config.PasswdEntry
-	s = strings.Replace(s, "$USERNAME", username, -1)
-	s = strings.Replace(s, "$UID", uid, -1)
-	s = strings.Replace(s, "$GID", gid, -1)
-	s = strings.Replace(s, "$NAME", name, -1)
-	s = strings.Replace(s, "$HOME", homeDir, -1)
+	s = strings.ReplaceAll(s, "$USERNAME", username)
+	s = strings.ReplaceAll(s, "$UID", uid)
+	s = strings.ReplaceAll(s, "$GID", gid)
+	s = strings.ReplaceAll(s, "$NAME", name)
+	s = strings.ReplaceAll(s, "$HOME", homeDir)
 	return s + "\n"
 }
 

libpod/container_stat_linux.go

@@ -94,15 +94,16 @@ func (c *Container) stat(containerMountPoint string, containerPath string) (*def
 		}
 	}
 
-	if statInfo.IsSymlink {
+	switch {
+	case statInfo.IsSymlink:
 		// Symlinks are already evaluated and always relative to the
 		// container's mount point.
 		absContainerPath = statInfo.ImmediateTarget
-	} else if strings.HasPrefix(resolvedPath, containerMountPoint) {
+	case strings.HasPrefix(resolvedPath, containerMountPoint):
 		// If the path is on the container's mount point, strip it off.
 		absContainerPath = strings.TrimPrefix(resolvedPath, containerMountPoint)
 		absContainerPath = filepath.Join("/", absContainerPath)
-	} else {
+	default:
 		// No symlink and not on the container's mount point, so let's
 		// move it back to the original input.  It must have evaluated
 		// to a volume or bind mount but we cannot return host paths.

libpod/define/container_inspect.go

@@ -100,7 +100,7 @@ type InspectRestartPolicy struct {
 // InspectLogConfig holds information about a container's configured log driver
 type InspectLogConfig struct {
 	Type   string            `json:"Type"`
-	Config map[string]string `json:"Config"` //idk type, TODO
+	Config map[string]string `json:"Config"`
 	// Path specifies a path to the log file
 	Path string `json:"Path"`
 	// Tag specifies a custom log tag for the container
@@ -680,7 +680,7 @@ type InspectContainerData struct {
 	SizeRootFs      int64                       `json:"SizeRootFs,omitempty"`
 	Mounts          []InspectMount              `json:"Mounts"`
 	Dependencies    []string                    `json:"Dependencies"`
-	NetworkSettings *InspectNetworkSettings     `json:"NetworkSettings"` //TODO
+	NetworkSettings *InspectNetworkSettings     `json:"NetworkSettings"`
 	Namespace       string                      `json:"Namespace"`
 	IsInfra         bool                        `json:"IsInfra"`
 	Config          *InspectContainerConfig     `json:"Config"`

libpod/define/info.go

@@ -12,7 +12,7 @@ type Info struct {
 	Version    Version                `json:"version"`
 }
 
-//HostInfo describes the libpod host
+// HostInfo describes the libpod host
 type SecurityInfo struct {
 	AppArmorEnabled     bool   `json:"apparmorEnabled"`
 	DefaultCapabilities string `json:"capabilities"`