Improve linting and fix found issues

.golangci.yml

@@ -1,5 +1,16 @@
 # all available settings of specific linters
 linters-settings:
+  gocritic:
+    enabled-tags:
+      - performance
+      - diagnostic
+      - style
+      - experimental
+      - opinionated
+    disabled-checks:
+      - hugeParam
+      - rangeValCopy
+      - unnamedResult
   gofmt:
     simplify: true
   goimports:

cmd/generate-plugin-overview/generate-plugin-overview.go

@@ -55,7 +55,7 @@ tool._
 )
 
 var (
-	githubRepoPattern = regexp.MustCompile(`.*github.com/([^/]+/[^/#]+)`)
+	githubRepoPattern = regexp.MustCompile(`.*github\.com/([^/]+/[^/#]+)`)
 )
 
 func main() {

cmd/validate-krew-manifest/main.go

@@ -32,6 +32,7 @@ import (
 
 	"sigs.k8s.io/krew/internal/index/indexscanner"
 	"sigs.k8s.io/krew/internal/index/validation"
+	"sigs.k8s.io/krew/internal/installation"
 	"sigs.k8s.io/krew/pkg/constants"
 	"sigs.k8s.io/krew/pkg/index"
 )
@@ -83,7 +84,7 @@ func validateManifestFile(path string) error {
 
 	// make sure each platform matches a supported platform
 	for i, p := range p.Spec.Platforms {
-		if os, arch := findAnyMatchingPlatform(p.Selector); os == "" || arch == "" {
+		if env := findAnyMatchingPlatform(p.Selector); env.OS == "" || env.Arch == "" {
 			return errors.Errorf("spec.platform[%d]'s selector (%v) doesn't match any supported platforms", i, p.Selector)
 		}
 	}
@@ -110,18 +111,16 @@ func validateManifestFile(path string) error {
 // isOverlappingPlatformSelectors validates if multiple platforms have selectors
 // that match to a supported <os,arch> pair.
 func isOverlappingPlatformSelectors(platforms []index.Platform) error {
-	for _, v := range allPlatforms() {
-		os, arch := v[0], v[1]
-
+	for _, env := range allPlatforms() {
 		var matchIndex []int
 		for i, p := range platforms {
-			if selectorMatchesOSArch(p.Selector, os, arch) {
+			if selectorMatchesOSArch(p.Selector, env) {
 				matchIndex = append(matchIndex, i)
 			}
 		}
 
 		if len(matchIndex) > 1 {
-			return errors.Errorf("multiple spec.platforms (at indexes %v) have overlapping selectors that select os=%s/arch=%s", matchIndex, os, arch)
+			return errors.Errorf("multiple spec.platforms (at indexes %v) have overlapping selectors that select %s", matchIndex, env)
 		}
 	}
 	return nil
@@ -130,8 +129,8 @@ func isOverlappingPlatformSelectors(platforms []index.Platform) error {
 // installPlatformSpec installs the p to a temporary location on disk to verify
 // by shelling out to external command.
 func installPlatformSpec(manifestFile string, p index.Platform) error {
-	goos, goarch := findAnyMatchingPlatform(p.Selector)
-	if goos == "" || goarch == "" {
+	env := findAnyMatchingPlatform(p.Selector)
+	if env.OS == "" || env.Arch == "" {
 		return errors.Errorf("no supported platform matched platform selector: %+v", p.Selector)
 	}
 
@@ -149,55 +148,55 @@ func installPlatformSpec(manifestFile string, p index.Platform) error {
 	cmd.Stdin = nil
 	cmd.Env = []string{
 		"KREW_ROOT=" + tmpDir,
-		"KREW_OS=" + goos,
-		"KREW_ARCH=" + goarch,
+		"KREW_OS=" + env.OS,
+		"KREW_ARCH=" + env.Arch,
 	}
 	klog.V(2).Infof("installing plugin with: %+v", cmd.Env)
 	cmd.Env = append(cmd.Env, "PATH="+os.Getenv("PATH"))
 
 	b, err := cmd.CombinedOutput()
 	if err != nil {
-		output := strings.Replace(string(b), "\n", "\n\t", -1)
+		output := strings.ReplaceAll(string(b), "\n", "\n\t")
 		return errors.Wrapf(err, "plugin install command failed: %s", output)
 	}
 	return nil
 }
 
 // findAnyMatchingPlatform finds an <os,arch> pair matches to given selector
-func findAnyMatchingPlatform(selector *metav1.LabelSelector) (string, string) {
+func findAnyMatchingPlatform(selector *metav1.LabelSelector) installation.OSArchPair {
 	for _, p := range allPlatforms() {
-		if selectorMatchesOSArch(selector, p[0], p[1]) {
-			klog.V(4).Infof("%s MATCHED <%s,%s>", selector, p[0], p[1])
-			return p[0], p[1]
+		if selectorMatchesOSArch(selector, p) {
+			klog.V(4).Infof("%s MATCHED <%s>", selector, p)
+			return p
 		}
-		klog.V(4).Infof("%s didn't match <%s,%s>", selector, p[0], p[1])
+		klog.V(4).Infof("%s didn't match <%s>", selector, p)
 	}
-	return "", ""
+	return installation.OSArchPair{}
 }
 
-func selectorMatchesOSArch(selector *metav1.LabelSelector, os, arch string) bool {
+func selectorMatchesOSArch(selector *metav1.LabelSelector, env installation.OSArchPair) bool {
 	sel, err := metav1.LabelSelectorAsSelector(selector)
 	if err != nil {
 		// this should've been caught by validation.ValidatePlatform() earlier
 		klog.Warningf("Failed to convert label selector: %+v", selector)
 		return false
 	}
 	return sel.Matches(labels.Set{
-		"os":   os,
-		"arch": arch,
+		"os":   env.OS,
+		"arch": env.Arch,
 	})
 }
 
 // allPlatforms returns all <os,arch> pairs krew is supported on.
-func allPlatforms() [][2]string {
+func allPlatforms() []installation.OSArchPair {
 	// TODO(ahmetb) find a more authoritative source for this list
-	return [][2]string{
-		{"windows", "386"},
-		{"windows", "amd64"},
-		{"linux", "386"},
-		{"linux", "amd64"},
-		{"linux", "arm"},
-		{"darwin", "386"},
-		{"darwin", "amd64"},
+	return []installation.OSArchPair{
+		{OS: "windows", Arch: "386"},
+		{OS: "windows", Arch: "amd64"},
+		{OS: "linux", Arch: "386"},
+		{OS: "linux", Arch: "amd64"},
+		{OS: "linux", Arch: "arm"},
+		{OS: "darwin", Arch: "386"},
+		{OS: "darwin", Arch: "amd64"},
 	}
 }

cmd/validate-krew-manifest/main_test.go

@@ -20,9 +20,9 @@ import (
 	"testing"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/yaml"
 
+	"sigs.k8s.io/krew/internal/installation"
 	"sigs.k8s.io/krew/internal/testutil"
 	"sigs.k8s.io/krew/pkg/index"
 )
@@ -112,55 +112,82 @@ func TestValidateManifestFile(t *testing.T) {
 func Test_selectorMatchesOSArch(t *testing.T) {
 	type args struct {
 		selector *metav1.LabelSelector
-		os       string
-		arch     string
+		env      installation.OSArchPair
 	}
 	tests := []struct {
 		name string
 		args args
 		want bool
 	}{
-		{"label - no match", args{&metav1.LabelSelector{MatchLabels: map[string]string{"os": "darwin"}}, "windows", "amd64"}, false},
-		{"label - match", args{&metav1.LabelSelector{MatchLabels: map[string]string{"os": "darwin"}}, "darwin", "amd64"}, true},
-		{"expression - no match", args{&metav1.LabelSelector{MatchExpressions: []v1.LabelSelectorRequirement{{
-			Key:      "os",
-			Operator: v1.LabelSelectorOpIn,
-			Values:   []string{"darwin", "linux"},
-		}}}, "windows", "amd64"}, false},
-		{"expression - match", args{&metav1.LabelSelector{MatchExpressions: []v1.LabelSelectorRequirement{{
-			Key:      "os",
-			Operator: v1.LabelSelectorOpIn,
-			Values:   []string{"darwin", "linux"},
-		}}}, "darwin", "amd64"}, true},
+		{
+			name: "label - no match",
+			args: args{
+				selector: &metav1.LabelSelector{MatchLabels: map[string]string{"os": "darwin"}},
+				env:      installation.OSArchPair{OS: "windows", Arch: "amd64"},
+			},
+			want: false,
+		},
+		{
+			name: "label - match",
+			args: args{
+				selector: &metav1.LabelSelector{MatchLabels: map[string]string{"os": "darwin"}},
+				env:      installation.OSArchPair{OS: "darwin", Arch: "amd64"},
+			},
+			want: true,
+		},
+		{
+			name: "expression - no match",
+			args: args{
+				selector: &metav1.LabelSelector{MatchExpressions: []metav1.LabelSelectorRequirement{{
+					Key:      "os",
+					Operator: metav1.LabelSelectorOpIn,
+					Values:   []string{"darwin", "linux"},
+				}}},
+				env: installation.OSArchPair{OS: "windows", Arch: "amd64"},
+			},
+			want: false,
+		},
+		{
+			name: "expression - match",
+			args: args{
+				selector: &metav1.LabelSelector{MatchExpressions: []metav1.LabelSelectorRequirement{{
+					Key:      "os",
+					Operator: metav1.LabelSelectorOpIn,
+					Values:   []string{"darwin", "linux"},
+				}}},
+				env: installation.OSArchPair{OS: "darwin", Arch: "amd64"},
+			},
+			want: true,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if got := selectorMatchesOSArch(tt.args.selector, tt.args.os, tt.args.arch); got != tt.want {
+			if got := selectorMatchesOSArch(tt.args.selector, tt.args.env); got != tt.want {
 				t.Errorf("selectorMatchesOSArch() = %v, want %v", got, tt.want)
 			}
 		})
 	}
 }
 
 func Test_findAnyMatchingPlatform(t *testing.T) {
-	s1 := &v1.LabelSelector{MatchLabels: map[string]string{"os": "darwin"}}
-	o1, a1 := findAnyMatchingPlatform(s1)
-	if o1 == "" || a1 == "" {
+	s1 := &metav1.LabelSelector{MatchLabels: map[string]string{"os": "darwin"}}
+	env := findAnyMatchingPlatform(s1)
+	if env.OS == "" || env.Arch == "" {
 		t.Fatalf("with selector %v, expected os/arch", s1)
 	}
 
-	s2 := &v1.LabelSelector{MatchLabels: map[string]string{"os": "non-existing"}}
-	o2, a2 := findAnyMatchingPlatform(s1)
-	if o2 == "" && a2 == "" {
+	s2 := &metav1.LabelSelector{MatchLabels: map[string]string{"os": "non-existing"}}
+	env2 := findAnyMatchingPlatform(s1)
+	if env2.OS == "" && env2.Arch == "" {
 		t.Fatalf("with selector %v, expected os/arch", s2)
 	}
 
-	s3 := &v1.LabelSelector{MatchExpressions: []v1.LabelSelectorRequirement{{
+	s3 := &metav1.LabelSelector{MatchExpressions: []metav1.LabelSelectorRequirement{{
 		Key:      "os",
-		Operator: v1.LabelSelectorOpIn,
+		Operator: metav1.LabelSelectorOpIn,
 		Values:   []string{"darwin", "linux"}}}}
-	o3, a3 := findAnyMatchingPlatform(s3)
-	if o3 == "" || a3 == "" {
+	env3 := findAnyMatchingPlatform(s3)
+	if env3.OS == "" || env3.Arch == "" {
 		t.Fatalf("with selector %v, expected os/arch", s2)
 	}
 }

internal/download/downloader.go

@@ -80,16 +80,16 @@ func extractZIP(targetDir string, read io.ReaderAt, size int64) error {
 			src.Close()
 			return errors.Wrap(err, "can't create file in zip destination dir")
 		}
-		close := func() {
+		closeAll := func() {
 			src.Close()
 			dst.Close()
 		}
 
 		if _, err := io.Copy(dst, src); err != nil {
-			close()
+			closeAll()
 			return errors.Wrap(err, "can't copy content to zip destination file")
 		}
-		close()
+		closeAll()
 	}
 
 	return nil
@@ -142,12 +142,12 @@ func extractTARGZ(targetDir string, at io.ReaderAt, size int64) error {
 			if err != nil {
 				return errors.Wrapf(err, "failed to create file %q", path)
 			}
-			close := func() { f.Close() }
+
 			if _, err := io.Copy(f, tr); err != nil {
-				close()
+				f.Close()
 				return errors.Wrapf(err, "failed to copy %q from tar into file", hdr.Name)
 			}
-			close()
+			f.Close()
 		default:
 			return errors.Errorf("unable to handle file type %d for %q in tar", hdr.Typeflag, hdr.Name)
 		}

internal/download/verifier.go

@@ -39,8 +39,8 @@ type sha256Verifier struct {
 }
 
 // NewSha256Verifier creates a Verifier that tests against the given hash.
-func NewSha256Verifier(hash string) Verifier {
-	raw, _ := hex.DecodeString(hash)
+func NewSha256Verifier(hashed string) Verifier {
+	raw, _ := hex.DecodeString(hashed)
 	return sha256Verifier{
 		Hash:       sha256.New(),
 		wantedHash: raw,

internal/installation/install.go

@@ -186,7 +186,7 @@ func Uninstall(p environment.Paths, name string) error {
 	return errors.Wrapf(err, "could not remove plugin receipt %q", pluginReceiptPath)
 }
 
-func createOrUpdateLink(binDir string, binary string, plugin string) error {
+func createOrUpdateLink(binDir, binary, plugin string) error {
 	dst := filepath.Join(binDir, pluginNameToBin(plugin, IsWindows()))
 
 	if err := removeLink(dst); err != nil {
@@ -238,7 +238,7 @@ func IsWindows() bool {
 // pluginNameToBin creates the name of the symlink file for the plugin name.
 // It converts dashes to underscores.
 func pluginNameToBin(name string, isWindows bool) string {
-	name = strings.Replace(name, "-", "_", -1)
+	name = strings.ReplaceAll(name, "-", "_")
 	name = "kubectl-" + name
 	if isWindows {
 		name += ".exe"
@@ -247,7 +247,7 @@ func pluginNameToBin(name string, isWindows bool) string {
 }
 
 // CleanupStaleKrewInstallations removes the versions that aren't the current version.
-func CleanupStaleKrewInstallations(dir string, currentVersion string) error {
+func CleanupStaleKrewInstallations(dir, currentVersion string) error {
 	ls, err := ioutil.ReadDir(dir)
 	if err != nil {
 		return errors.Wrap(err, "failed to read krew store directory")

internal/installation/install_test.go

@@ -299,8 +299,8 @@ func Test_applyDefaults(t *testing.T) {
 }
 
 func TestCleanupStaleKrewInstallations(t *testing.T) {
-	dir, close := testutil.NewTempDir(t)
-	defer close()
+	dir, cleanup := testutil.NewTempDir(t)
+	defer cleanup()
 
 	testFiles := []string{
 		"dir1/f1.txt",