Merge pull request #2016 from alexandear/enable-gocritic

AkihiroSuda · web-flow · commit 439c8baec37e · 2023-12-15T20:22:42.000+09:00
Enable gocritic linter and fix issues
diff --git a/.golangci.yml b/.golangci.yml
@@ -18,7 +18,6 @@
 ---
 run:
   concurrency: 6
-  deadline: 5m
 linters:
   disable-all: true
   enable:
@@ -47,7 +46,7 @@ linters:
   # - gochecknoinits
   # - gocognit
   # - goconst
-  # - gocritic
+  - gocritic
   # - gocyclo
   # - godot
   # - godox
@@ -83,76 +82,29 @@ linters:
   # - wsl
 linters-settings:
   gocritic:
-    enabled-checks:
-    # Diagnostic
-    - appendAssign
-    - argOrder
-    - badCond
-    - caseOrder
-    - codegenComment
-    - commentedOutCode
-    - deprecatedComment
-    - dupArg
-    - dupBranchBody
-    - dupCase
-    - dupSubExpr
-    - exitAfterDefer
-    - flagDeref
-    - flagName
-    - nilValReturn
-    - offBy1
-    - sloppyReassign
-    - weakCond
-    - octalLiteral
-
-    # Performance
+    # See "Tags" section in https://github.com/go-critic/go-critic#usage
+    enabled-tags:
+    - diagnostic
+    - performance
+    - style
+    - opinionated
+    - experimental
+    disabled-checks:
     - appendCombine
-    - equalFold
-    - hugeParam
-    - indexAlloc
-    - rangeExprCopy
+    - sloppyReassign
+    - unlabelStmt
     - rangeValCopy
-
-    # Style
-    - assignOp
-    - boolExprSimplify
-    - captLocal
-    - commentFormatting
-    - commentedOutImport
-    - defaultCaseOrder
-    - docStub
-    - elseif
-    - emptyFallthrough
-    - emptyStringTest
-    - hexLiteral
+    - hugeParam
+    - importShadow
     - ifElseChain
-    - methodExprCall
-    - regexpMust
-    - singleCaseSwitch
-    - sloppyLen
-    - stringXbytes
-    - switchTrue
-    - typeAssertChain
-    - typeSwitchVar
-    - underef
-    - unlabelStmt
-    - unlambda
-    - unslice
-    - valSwap
-    - wrapperFunc
-    - yodaStyleExpr
-
-    # Opinionated
+    - sprintfQuotedString
     - builtinShadow
-    - importShadow
-    - initClause
-    - nestingReduce
-    - paramTypeCombine
-    - ptrToRefParam
-    - typeUnparen
-    - unnamedResult
-    - unnecessaryBlock
+    - filepathJoin
 issues:
+  # Maximum issues count per one linter.
+  max-issues-per-linter: 0
+  # Maximum count of issues with the same text.
+  max-same-issues: 0
   exclude-rules:
   # Allow using Uid, Gid in pkg/osutil.
   - path: "pkg/osutil/"
diff --git a/cmd/limactl/gendoc.go b/cmd/limactl/gendoc.go
@@ -117,7 +117,7 @@ weight: 3
 }
 
 // replaceAll replaces all occurrences of new with old, for all files in dir
-func replaceAll(dir string, old, new string) error {
+func replaceAll(dir, old, new string) error {
 	logrus.Infof("Replacing %q with %q", old, new)
 	return filepath.Walk(dir, func(path string, info fs.FileInfo, err error) error {
 		if err != nil {
@@ -133,7 +133,7 @@ func replaceAll(dir string, old, new string) error {
 		if err != nil {
 			return err
 		}
-		out := bytes.Replace(in, []byte(old), []byte(new), -1)
+		out := bytes.ReplaceAll(in, []byte(old), []byte(new))
 		err = os.WriteFile(path, out, 0o644)
 		if err != nil {
 			return err
diff --git a/pkg/cidata/cidata.go b/pkg/cidata/cidata.go
@@ -437,10 +437,11 @@ func writeCIDataDir(rootPath string, layout []iso9660util.Entry) error {
 		if err != nil {
 			return err
 		}
-		defer f.Close()
 		if _, err := io.Copy(f, e.Reader); err != nil {
+			_ = f.Close()
 			return err
 		}
+		_ = f.Close()
 	}
 
 	return nil
diff --git a/pkg/downloader/downloader.go b/pkg/downloader/downloader.go
@@ -291,7 +291,7 @@ func Cached(remote string, opts ...Opt) (*Result, error) {
 // cacheDirectoryPath returns the cache subdirectory path.
 // - "url" file contains the url
 // - "data" file contains the data
-func cacheDirectoryPath(cacheDir string, remote string) string {
+func cacheDirectoryPath(cacheDir, remote string) string {
 	return filepath.Join(cacheDir, "download", "by-url-sha256", fmt.Sprintf("%x", sha256.Sum256([]byte(remote))))
 }
 
@@ -380,7 +380,7 @@ func Decompressor(ext string) ([]string, bool) {
 	return []string{program, "-d"}, true
 }
 
-func decompressLocal(dst, src, ext string, description string) error {
+func decompressLocal(dst, src, ext, description string) error {
 	command, found := Decompressor(ext)
 	if !found {
 		return fmt.Errorf("decompressLocal: unknown extension %s", ext)
@@ -472,7 +472,7 @@ func validateLocalFileDigest(localPath string, expectedDigest digest.Digest) err
 	return nil
 }
 
-func downloadHTTP(localPath, url string, description string, expectedDigest digest.Digest) error {
+func downloadHTTP(localPath, url, description string, expectedDigest digest.Digest) error {
 	if localPath == "" {
 		return fmt.Errorf("downloadHTTP: got empty localPath")
 	}
diff --git a/pkg/executil/command.go b/pkg/executil/command.go
@@ -16,6 +16,8 @@ type options struct {
 type Opt func(*options) error
 
 // WithContext runs the command with CommandContext.
+//
+//nolint:gocritic // consider `ctx' to be of non-pointer type
 func WithContext(ctx *context.Context) Opt {
 	return func(o *options) error {
 		o.ctx = ctx
diff --git a/pkg/guestagent/guestagent_linux.go b/pkg/guestagent/guestagent_linux.go
@@ -121,8 +121,7 @@ func (a *agent) setWorthCheckingIPTablesRoutine(auditClient *libaudit.AuditClien
 			logrus.Error(err)
 			continue
 		}
-		switch msg.Type {
-		case auparse.AUDIT_NETFILTER_CFG:
+		if msg.Type == auparse.AUDIT_NETFILTER_CFG {
 			a.worthCheckingIPTablesMu.Lock()
 			logrus.Debug("setWorthCheckingIPTablesRoutine(): setting to true")
 			a.worthCheckingIPTables = true
diff --git a/pkg/guestagent/procnettcp/procnettcp_linux.go b/pkg/guestagent/procnettcp/procnettcp_linux.go
@@ -20,11 +20,12 @@ func ParseFiles() ([]Entry, error) {
 			}
 			return res, err
 		}
-		defer r.Close()
 		parsed, err := Parse(r, kind)
 		if err != nil {
+			_ = r.Close()
 			return res, err
 		}
+		_ = r.Close()
 		res = append(res, parsed...)
 	}
 	return res, nil
diff --git a/pkg/hostagent/hostagent.go b/pkg/hostagent/hostagent.go
@@ -665,7 +665,7 @@ func executeSSH(ctx context.Context, sshConfig *ssh.SSHConfig, port int, command
 	return nil
 }
 
-func forwardSSH(ctx context.Context, sshConfig *ssh.SSHConfig, port int, local, remote string, verb string, reverse bool) error {
+func forwardSSH(ctx context.Context, sshConfig *ssh.SSHConfig, port int, local, remote, verb string, reverse bool) error {
 	args := sshConfig.Args()
 	args = append(args,
 		"-T",
@@ -732,7 +732,7 @@ func forwardSSH(ctx context.Context, sshConfig *ssh.SSHConfig, port int, local,
 				}
 			} else {
 				logrus.WithError(err).Warnf("Failed to set up forward from %q (guest) to %q (host)", remote, local)
-				if removeErr := os.RemoveAll(local); err != nil {
+				if removeErr := os.RemoveAll(local); removeErr != nil {
 					logrus.WithError(removeErr).Warnf("Failed to clean up %q (host) after forwarding failed", local)
 				}
 			}
diff --git a/pkg/hostagent/mount.go b/pkg/hostagent/mount.go
@@ -47,10 +47,10 @@ func (a *HostAgent) setupMount(m limayaml.Mount) (*mount, error) {
 	// NOTE: allow_other requires "user_allow_other" in /etc/fuse.conf
 	sshfsOptions := "allow_other"
 	if !*m.SSHFS.Cache {
-		sshfsOptions = sshfsOptions + ",cache=no"
+		sshfsOptions += ",cache=no"
 	}
 	if *m.SSHFS.FollowSymlinks {
-		sshfsOptions = sshfsOptions + ",follow_symlinks"
+		sshfsOptions += ",follow_symlinks"
 	}
 	logrus.Infof("Mounting %q on %q", location, mountPoint)
 
diff --git a/pkg/hostagent/port.go b/pkg/hostagent/port.go
@@ -42,7 +42,7 @@ func hostAddress(rule limayaml.PortForward, guest api.IPPort) string {
 	return host.String()
 }
 
-func (pf *portForwarder) forwardingAddresses(guest api.IPPort, localUnixIP net.IP) (string, string) {
+func (pf *portForwarder) forwardingAddresses(guest api.IPPort, localUnixIP net.IP) (hostAddr, guestAddr string) {
 	if pf.vmType == limayaml.WSL2 {
 		guest.IP = localUnixIP
 		host := api.IPPort{
diff --git a/pkg/hostagent/port_darwin.go b/pkg/hostagent/port_darwin.go
@@ -16,7 +16,7 @@ import (
 )
 
 // forwardTCP is not thread-safe
-func forwardTCP(ctx context.Context, sshConfig *ssh.SSHConfig, port int, local, remote string, verb string) error {
+func forwardTCP(ctx context.Context, sshConfig *ssh.SSHConfig, port int, local, remote, verb string) error {
 	if strings.HasPrefix(local, "/") {
 		return forwardSSH(ctx, sshConfig, port, local, remote, verb, false)
 	}
diff --git a/pkg/hostagent/port_others.go b/pkg/hostagent/port_others.go
@@ -8,7 +8,7 @@ import (
 	"github.com/lima-vm/sshocker/pkg/ssh"
 )
 
-func forwardTCP(ctx context.Context, sshConfig *ssh.SSHConfig, port int, local, remote string, verb string) error {
+func forwardTCP(ctx context.Context, sshConfig *ssh.SSHConfig, port int, local, remote, verb string) error {
 	return forwardSSH(ctx, sshConfig, port, local, remote, verb, false)
 }
 
diff --git a/pkg/hostagent/port_windows.go b/pkg/hostagent/port_windows.go
@@ -7,7 +7,7 @@ import (
 	"github.com/lima-vm/sshocker/pkg/ssh"
 )
 
-func forwardTCP(ctx context.Context, sshConfig *ssh.SSHConfig, port int, local, remote string, verb string) error {
+func forwardTCP(ctx context.Context, sshConfig *ssh.SSHConfig, port int, local, remote, verb string) error {
 	return forwardSSH(ctx, sshConfig, port, local, remote, verb, false)
 }
 
diff --git a/pkg/httpclientutil/httpclientutil.go b/pkg/httpclientutil/httpclientutil.go
@@ -17,7 +17,7 @@ import (
 
 // Get calls HTTP GET and verifies that the status code is 2XX .
 func Get(ctx context.Context, c *http.Client, url string) (*http.Response, error) {
-	req, err := http.NewRequestWithContext(ctx, "GET", url, nil)
+	req, err := http.NewRequestWithContext(ctx, "GET", url, http.NoBody)
 	if err != nil {
 		return nil, err
 	}
diff --git a/pkg/limayaml/defaults.go b/pkg/limayaml/defaults.go
@@ -708,7 +708,7 @@ func executeGuestTemplate(format string) (bytes.Buffer, error) {
 	return bytes.Buffer{}, err
 }
 
-func executeHostTemplate(format string, instDir string) (bytes.Buffer, error) {
+func executeHostTemplate(format, instDir string) (bytes.Buffer, error) {
 	tmpl, err := template.New("").Parse(format)
 	if err == nil {
 		user, _ := osutil.LimaUser(false)
diff --git a/pkg/limayaml/defaults_test.go b/pkg/limayaml/defaults_test.go
@@ -421,16 +421,16 @@ func TestFillDefault(t *testing.T) {
 
 	expect = y
 
-	expect.Provision = append(y.Provision, d.Provision...)
-	expect.Probes = append(y.Probes, d.Probes...)
-	expect.PortForwards = append(y.PortForwards, d.PortForwards...)
-	expect.CopyToHost = append(y.CopyToHost, d.CopyToHost...)
-	expect.Containerd.Archives = append(y.Containerd.Archives, d.Containerd.Archives...)
-	expect.AdditionalDisks = append(y.AdditionalDisks, d.AdditionalDisks...)
+	expect.Provision = append(append([]Provision{}, y.Provision...), d.Provision...)
+	expect.Probes = append(append([]Probe{}, y.Probes...), d.Probes...)
+	expect.PortForwards = append(append([]PortForward{}, y.PortForwards...), d.PortForwards...)
+	expect.CopyToHost = append(append([]CopyToHost{}, y.CopyToHost...), d.CopyToHost...)
+	expect.Containerd.Archives = append(append([]File{}, y.Containerd.Archives...), d.Containerd.Archives...)
+	expect.AdditionalDisks = append(append([]Disk{}, y.AdditionalDisks...), d.AdditionalDisks...)
 
 	// Mounts and Networks start with lowest priority first, so higher priority entries can overwrite
-	expect.Mounts = append(d.Mounts, y.Mounts...)
-	expect.Networks = append(d.Networks, y.Networks...)
+	expect.Mounts = append(append([]Mount{}, d.Mounts...), y.Mounts...)
+	expect.Networks = append(append([]Network{}, d.Networks...), y.Networks...)
 
 	expect.HostResolver.Hosts["default"] = d.HostResolver.Hosts["default"]
 
@@ -585,7 +585,7 @@ func TestFillDefault(t *testing.T) {
 	expect.HostResolver.Hosts["MY.Host"] = d.HostResolver.Hosts["host.lima.internal"]
 
 	// o.Mounts just makes d.Mounts[0] writable because the Location matches
-	expect.Mounts = append(d.Mounts, y.Mounts...)
+	expect.Mounts = append(append([]Mount{}, d.Mounts...), y.Mounts...)
 	expect.Mounts[0].Writable = ptr.Of(true)
 	expect.Mounts[0].SSHFS.Cache = ptr.Of(false)
 	expect.Mounts[0].SSHFS.FollowSymlinks = ptr.Of(true)
diff --git a/pkg/networks/commands.go b/pkg/networks/commands.go
@@ -75,7 +75,7 @@ func (config *YAML) Sock(name string) string {
 
 // VDESock returns a vde socket.
 //
-// Deprecated. Use Sock.
+// Deprecated: Use Sock.
 func (config *YAML) VDESock(name string) string {
 	return filepath.Join(config.Paths.VarRun, fmt.Sprintf("%s.ctl", name))
 }
diff --git a/pkg/networks/config.go b/pkg/networks/config.go
@@ -175,7 +175,7 @@ func Usernet(name string) (bool, error) {
 
 // VDESock returns a vde socket.
 //
-// Deprecated. Use Sock.
+// Deprecated: Use Sock.
 func VDESock(name string) (string, error) {
 	loadCache()
 	if cache.err != nil {
diff --git a/pkg/networks/usernet/config.go b/pkg/networks/usernet/config.go
@@ -29,7 +29,7 @@ func Sock(name string, sockType SockType) (string, error) {
 }
 
 // SockWithDirectory return a usernet socket based on dir, name and sockType
-func SockWithDirectory(dir string, name string, sockType SockType) (string, error) {
+func SockWithDirectory(dir, name string, sockType SockType) (string, error) {
 	if name == "" {
 		name = "default"
 	}
@@ -108,7 +108,7 @@ func Leases(name string) (string, error) {
 	return sockPath, nil
 }
 
-func netmaskToCidr(baseIP net.IP, netMask net.IP) (net.IP, *net.IPNet, error) {
+func netmaskToCidr(baseIP, netMask net.IP) (net.IP, *net.IPNet, error) {
 	size, _ := net.IPMask(netMask.To4()).Size()
 	return net.ParseCIDR(fmt.Sprintf("%s/%d", baseIP.String(), size))
 }
diff --git a/pkg/networks/usernet/dnshosts/dnshosts_test.go b/pkg/networks/usernet/dnshosts/dnshosts_test.go
@@ -194,8 +194,8 @@ func Test_extractZones(t *testing.T) {
 }
 
 var (
-	_ sort.Interface = (recordSorter)(nil)
-	_ sort.Interface = (zoneSorter)(nil)
+	_ sort.Interface = recordSorter(nil)
+	_ sort.Interface = zoneSorter(nil)
 )
 
 type recordSorter []types.Record
@@ -206,12 +206,12 @@ func (r recordSorter) Len() int {
 }
 
 // Less implements sort.Interface
-func (r recordSorter) Less(i int, j int) bool {
+func (r recordSorter) Less(i, j int) bool {
 	return r[i].Name < r[j].Name
 }
 
 // Swap implements sort.Interface
-func (r recordSorter) Swap(i int, j int) {
+func (r recordSorter) Swap(i, j int) {
 	r[i], r[j] = r[j], r[i]
 }
 
@@ -223,11 +223,11 @@ func (z zoneSorter) Len() int {
 }
 
 // Less implements sort.Interface
-func (z zoneSorter) Less(i int, j int) bool {
+func (z zoneSorter) Less(i, j int) bool {
 	return z[i].Name < z[j].Name
 }
 
 // Swap implements sort.Interface
-func (z zoneSorter) Swap(i int, j int) {
+func (z zoneSorter) Swap(i, j int) {
 	z[i], z[j] = z[j], z[i]
 }
diff --git a/pkg/networks/usernet/gvproxy_test.go b/pkg/networks/usernet/gvproxy_test.go
diff --git a/pkg/qemu/entitlementutil/entitlementutil.go b/pkg/qemu/entitlementutil/entitlementutil.go
diff --git a/pkg/qemu/imgutil/imgutil.go b/pkg/qemu/imgutil/imgutil.go
diff --git a/pkg/qemu/qemu.go b/pkg/qemu/qemu.go
diff --git a/pkg/sshutil/sshutil.go b/pkg/sshutil/sshutil.go
diff --git a/pkg/store/disk.go b/pkg/store/disk.go
diff --git a/pkg/textutil/textutil.go b/pkg/textutil/textutil.go
diff --git a/pkg/vz/vm_darwin.go b/pkg/vz/vm_darwin.go

Original file line number	Diff line number	Diff line change
`@@ -437,10 +437,11 @@ func writeCIDataDir(rootPath string, layout []iso9660util.Entry) error {`
`437`	`437`	`if err != nil {`
`438`	`438`	`return err`
`439`	`439`	`}`
`440`		`- defer f.Close()`
`441`	`440`	`if _, err := io.Copy(f, e.Reader); err != nil {`
	`441`	`+ _ = f.Close()`
`442`	`442`	`return err`
`443`	`443`	`}`
	`444`	`+ _ = f.Close()`
`444`	`445`	`}`
`445`	`446`
`446`	`447`	`return nil`
Original file line number	Diff line number	Diff line change
`@@ -291,7 +291,7 @@ func Cached(remote string, opts ...Opt) (*Result, error) {`
`291`	`291`	`// cacheDirectoryPath returns the cache subdirectory path.`
`292`	`292`	`// - "url" file contains the url`
`293`	`293`	`// - "data" file contains the data`
`294`		`-func cacheDirectoryPath(cacheDir string, remote string) string {`
	`294`	`+func cacheDirectoryPath(cacheDir, remote string) string {`
`295`	`295`	`return filepath.Join(cacheDir, "download", "by-url-sha256", fmt.Sprintf("%x", sha256.Sum256([]byte(remote))))`
`296`	`296`	`}`
`297`	`297`
`@@ -380,7 +380,7 @@ func Decompressor(ext string) ([]string, bool) {`
`380`	`380`	`return []string{program, "-d"}, true`
`381`	`381`	`}`
`382`	`382`
`383`		`-func decompressLocal(dst, src, ext string, description string) error {`
	`383`	`+func decompressLocal(dst, src, ext, description string) error {`
`384`	`384`	`command, found := Decompressor(ext)`
`385`	`385`	`if !found {`
`386`	`386`	`return fmt.Errorf("decompressLocal: unknown extension %s", ext)`
`@@ -472,7 +472,7 @@ func validateLocalFileDigest(localPath string, expectedDigest digest.Digest) err`
`472`	`472`	`return nil`
`473`	`473`	`}`
`474`	`474`
`475`		`-func downloadHTTP(localPath, url string, description string, expectedDigest digest.Digest) error {`
	`475`	`+func downloadHTTP(localPath, url, description string, expectedDigest digest.Digest) error {`
`476`	`476`	`if localPath == "" {`
`477`	`477`	`return fmt.Errorf("downloadHTTP: got empty localPath")`
`478`	`478`	`}`
Original file line number	Diff line number	Diff line change
`@@ -20,11 +20,12 @@ func ParseFiles() ([]Entry, error) {`
`20`	`20`	`}`
`21`	`21`	`return res, err`
`22`	`22`	`}`
`23`		`- defer r.Close()`
`24`	`23`	`parsed, err := Parse(r, kind)`
`25`	`24`	`if err != nil {`
	`25`	`+ _ = r.Close()`
`26`	`26`	`return res, err`
`27`	`27`	`}`
	`28`	`+ _ = r.Close()`
`28`	`29`	`res = append(res, parsed...)`
`29`	`30`	`}`
`30`	`31`	`return res, nil`
Original file line number	Diff line number	Diff line change
`@@ -665,7 +665,7 @@ func executeSSH(ctx context.Context, sshConfig *ssh.SSHConfig, port int, command`
`665`	`665`	`return nil`
`666`	`666`	`}`
`667`	`667`
`668`		`-func forwardSSH(ctx context.Context, sshConfig *ssh.SSHConfig, port int, local, remote string, verb string, reverse bool) error {`
	`668`	`+func forwardSSH(ctx context.Context, sshConfig *ssh.SSHConfig, port int, local, remote, verb string, reverse bool) error {`
`669`	`669`	`args := sshConfig.Args()`
`670`	`670`	`args = append(args,`
`671`	`671`	`"-T",`
`@@ -732,7 +732,7 @@ func forwardSSH(ctx context.Context, sshConfig *ssh.SSHConfig, port int, local,`
`732`	`732`	`}`
`733`	`733`	`} else {`
`734`	`734`	`logrus.WithError(err).Warnf("Failed to set up forward from %q (guest) to %q (host)", remote, local)`
`735`		`- if removeErr := os.RemoveAll(local); err != nil {`
	`735`	`+ if removeErr := os.RemoveAll(local); removeErr != nil {`
`736`	`736`	`logrus.WithError(removeErr).Warnf("Failed to clean up %q (host) after forwarding failed", local)`
`737`	`737`	`}`
`738`	`738`	`}`
Original file line number	Diff line number	Diff line change
`@@ -47,10 +47,10 @@ func (a HostAgent) setupMount(m limayaml.Mount) (mount, error) {`
`47`	`47`	`// NOTE: allow_other requires "user_allow_other" in /etc/fuse.conf`
`48`	`48`	`sshfsOptions := "allow_other"`
`49`	`49`	`if !*m.SSHFS.Cache {`
`50`		`- sshfsOptions = sshfsOptions + ",cache=no"`
	`50`	`+ sshfsOptions += ",cache=no"`
`51`	`51`	`}`
`52`	`52`	`if *m.SSHFS.FollowSymlinks {`
`53`		`- sshfsOptions = sshfsOptions + ",follow_symlinks"`
	`53`	`+ sshfsOptions += ",follow_symlinks"`
`54`	`54`	`}`
`55`	`55`	`logrus.Infof("Mounting %q on %q", location, mountPoint)`
`56`	`56`
Original file line number	Diff line number	Diff line change
`@@ -42,7 +42,7 @@ func hostAddress(rule limayaml.PortForward, guest api.IPPort) string {`
`42`	`42`	`return host.String()`
`43`	`43`	`}`
`44`	`44`
`45`		`-func (pf *portForwarder) forwardingAddresses(guest api.IPPort, localUnixIP net.IP) (string, string) {`
	`45`	`+func (pf *portForwarder) forwardingAddresses(guest api.IPPort, localUnixIP net.IP) (hostAddr, guestAddr string) {`
`46`	`46`	`if pf.vmType == limayaml.WSL2 {`
`47`	`47`	`guest.IP = localUnixIP`
`48`	`48`	`host := api.IPPort{`