Merge pull request #20 from cpanato/lints

lints: update based on golangci-lint feedback

.golangci.yml

@@ -13,23 +13,24 @@ issues:
 linters:
   disable-all: true
   enable:
+    - asciicheck
     - bodyclose
     - deadcode
     - depguard
     - dogsled
     - dupl
     - errcheck
-    - gocognit
     - goconst
     - gocritic
     - gocyclo
     - godox
     - gofmt
+    - gofumpt
+    - goheader
     - goimports
     - golint
-    - gomnd
+    - gomodguard
     - goprintffuncname
-    - gosec
     - gosimple
     - govet
     - ineffassign
@@ -39,7 +40,7 @@ linters:
     - nakedret
     - prealloc
     - rowserrcheck
-    - scopelint
+    - sqlclosecheck
     - staticcheck
     - structcheck
     - stylecheck
@@ -49,17 +50,25 @@ linters:
     - unused
     - varcheck
     - whitespace
-
-    # Nits: These are things one might mention, but not block on during review.
-    #       We can consider disabling these checks, if we determine them to be
-    #       a little too nitty and obtrusive.
-    - lll
-    - wsl
-
-    # Disabled
-    # cobra uses globals and inits to set command configuration
-    #- gochecknoglobals
-    #- gochecknoinits
+    # - exhaustive
+    # - exportloopref
+    # - funlen
+    # - gci
+    # - gochecknoglobals
+    # - gochecknoinits
+    # - gocognit
+    # - godot
+    # - goerr113
+    # - gomnd
+    # - gosec
+    # - lll
+    # - nestif
+    # - nlreturn
+    # - noctx
+    # - nolintlint
+    # - scopelint
+    # - testpackage
+    # - wsl
 linters-settings:
   godox:
     keywords:
@@ -112,7 +121,6 @@ linters-settings:
       - emptyFallthrough
       - emptyStringTest
       - hexLiteral
-      - ifElseChain
       - methodExprCall
       - regexpMust
       - singleCaseSwitch
@@ -128,6 +136,7 @@ linters-settings:
       - valSwap
       - wrapperFunc
       - yodaStyleExpr
+      # - ifElseChain
 
       # Opinionated
       - builtinShadow

buoy/commands/check.go

@@ -30,13 +30,15 @@ import (
 )
 
 func addCheckCmd(root *cobra.Command) {
-	var domain string
-	var release string
-	var rulesetFlag string
-	var ruleset git.RulesetType
-	var verbose bool
-
-	var cmd = &cobra.Command{
+	var (
+		domain      string
+		release     string
+		rulesetFlag string
+		ruleset     git.RulesetType
+		verbose     bool
+	)
+
+	cmd := &cobra.Command{
 		Use:   "check go.mod",
 		Short: "Determine if this module has a ref for each dependency for a given release based on a ruleset.",
 		Long: `
@@ -79,12 +81,17 @@ Rulesets,
 		},
 	}
 
-	cmd.Flags().StringVarP(&domain, "domain", "d", "", "domain filter (i.e. knative.dev) [required]")
-	_ = cmd.MarkFlagRequired("domain")
-	cmd.Flags().StringVarP(&release, "release", "r", "", "release should be '<major>.<minor>' (i.e.: 1.23 or v1.23) [required]")
-	_ = cmd.MarkFlagRequired("release")
-	cmd.Flags().StringVar(&rulesetFlag, "ruleset", git.ReleaseOrReleaseBranchRule.String(), fmt.Sprintf("The ruleset to evaluate the dependency refs. Rulesets: [%s]", strings.Join(git.Rulesets(), ", ")))
+	cmd.Flags().StringVarP(&domain,
+		"domain", "d", "", "domain filter (i.e. knative.dev) [required]")
+	cmd.Flags().StringVarP(&release,
+		"release", "r", "", "release should be '<major>.<minor>' (i.e.: 1.23 or v1.23) [required]")
+	cmd.Flags().StringVar(&rulesetFlag,
+		"ruleset", git.ReleaseOrReleaseBranchRule.String(),
+		fmt.Sprintf("The ruleset to evaluate the dependency refs. Rulesets: [%s]", strings.Join(git.Rulesets(), ", ")))
 	cmd.Flags().BoolVarP(&verbose, "verbose", "v", false, "Print verbose output.")
 
+	_ = cmd.MarkFlagRequired("domain")  // nolint: errcheck
+	_ = cmd.MarkFlagRequired("release") // nolint: errcheck
+
 	root.AddCommand(cmd)
 }

buoy/commands/commands.go

@@ -20,7 +20,7 @@ import "github.com/spf13/cobra"
 
 // New creates a new buoy cli command set.
 func New() *cobra.Command {
-	var buoyCmd = &cobra.Command{
+	buoyCmd := &cobra.Command{
 		Use:   "buoy",
 		Short: "Introspect go module dependencies.",
 	}

buoy/commands/exists.go

@@ -33,7 +33,7 @@ func addExistsCmd(root *cobra.Command) {
 		tag     bool
 	)
 
-	var cmd = &cobra.Command{
+	cmd := &cobra.Command{
 		Use:   "exists go.mod",
 		Short: "Determine if the release branch exists for a given module.",
 		Args:  cobra.ExactArgs(1),
@@ -63,9 +63,10 @@ func addExistsCmd(root *cobra.Command) {
 	}
 
 	cmd.Flags().StringVarP(&release, "release", "r", "", "release should be '<major>.<minor>' (i.e.: 1.23 or v1.23) [required]")
-	_ = cmd.MarkFlagRequired("release")
 	cmd.Flags().BoolVarP(&verbose, "verbose", "v", false, "Print verbose output (stderr)")
 	cmd.Flags().BoolVarP(&tag, "next", "t", false, "Print the next release tag (stdout)")
 
+	_ = cmd.MarkFlagRequired("release") // nolint: errcheck
+
 	root.AddCommand(cmd)
 }

buoy/commands/float.go

@@ -34,7 +34,7 @@ func addFloatCmd(root *cobra.Command) {
 		ruleset     git.RulesetType
 	)
 
-	var cmd = &cobra.Command{
+	cmd := &cobra.Command{
 		Use:   "float go.mod",
 		Short: "Find latest versions of dependencies based on a release.",
 		Long: `
@@ -86,8 +86,9 @@ For rulesets that that restrict the selection process, no ref is selected.
 
 	cmd.Flags().StringVarP(&domain, "domain", "d", "knative.dev", "domain filter (i.e. knative.dev) [required]")
 	cmd.Flags().StringVarP(&release, "release", "r", "", "release should be '<major>.<minor>' (i.e.: 1.23 or v1.23) [required]")
-	_ = cmd.MarkFlagRequired("release")
 	cmd.Flags().StringVar(&rulesetFlag, "ruleset", git.AnyRule.String(), fmt.Sprintf("The ruleset to evaluate the dependency refs. Rulesets: [%s]", strings.Join(git.Rulesets(), ", ")))
 
+	_ = cmd.MarkFlagRequired("release") // nolint: errcheck
+
 	root.AddCommand(cmd)
 }

buoy/commands/needs.go

@@ -27,7 +27,7 @@ import (
 func addNeedsCmd(root *cobra.Command) {
 	var domain string
 
-	var cmd = &cobra.Command{
+	cmd := &cobra.Command{
 		Use:   "needs go.mod",
 		Short: "Find dependencies based on a base import domain.",
 		Args:  cobra.MinimumNArgs(1),
@@ -49,7 +49,8 @@ func addNeedsCmd(root *cobra.Command) {
 	}
 
 	cmd.Flags().StringVarP(&domain, "domain", "d", "", "domain filter (i.e. knative.dev) [required]")
-	_ = cmd.MarkFlagRequired("domain")
+
+	_ = cmd.MarkFlagRequired("domain") // nolint: errcheck
 
 	root.AddCommand(cmd)
 }

buoy/pkg/git/git.go

@@ -99,6 +99,8 @@ func (r *Repo) BestRefFor(this semver.Version, ruleset RulesetType) (string, Ref
 		// Look for a release.
 		for _, t := range r.Tags {
 			if sv, ok := normalizeTagVersion(t); ok {
+				// TODO: refactor to check the error
+				// nolint: errcheck
 				v, _ := semver.Make(sv)
 				// Go does not understand how to fetch semver tags with pre or build tags, skip those.
 				if v.Pre != nil || v.Build != nil {
@@ -122,6 +124,8 @@ func (r *Repo) BestRefFor(this semver.Version, ruleset RulesetType) (string, Ref
 		// Look for a release branch.
 		for _, b := range r.Branches {
 			if bv, ok := normalizeBranchVersion(b); ok {
+				// TODO: refactor to check the error
+				// nolint: errcheck
 				v, _ := semver.Make(bv)
 
 				if v.Major == this.Major && v.Minor == this.Minor {
@@ -136,8 +140,7 @@ func (r *Repo) BestRefFor(this semver.Version, ruleset RulesetType) (string, Ref
 		}
 	}
 
-	switch ruleset {
-	case AnyRule:
+	if ruleset == AnyRule {
 		// Look for a Return default branch.
 		return fmt.Sprintf("%s@%s", r.Ref, r.DefaultBranch), DefaultBranchRef
 	}
@@ -175,7 +178,7 @@ func ReleaseBranchVersion(v semver.Version) string {
 // ParseRef takes a go module ref and converts it to the module name and RefType.
 // ParseRef expects ref to be in the form "module@ref".
 // Only release branches and
-func ParseRef(ref string) (string, string, RefType) {
+func ParseRef(ref string) (module, reference string, branchRef RefType) {
 	parts := strings.Split(ref, "@")
 	if len(parts) != 2 {
 		return ref, "", UndefinedRef
@@ -193,5 +196,4 @@ func ParseRef(ref string) (string, string, RefType) {
 
 	// At this point we have to assume it is a branch.
 	return parts[0], parts[1], BranchRef
-
 }

buoy/pkg/git/git_test.go

@@ -34,8 +34,10 @@ func TestGetRepo_BasicOne(t *testing.T) {
 	if r == nil {
 		t.Error("failed to GetRepo: repo is nil")
 	}
-	if want := "master"; r.DefaultBranch != want {
-		t.Errorf("expected default branch to be %q, got %q", want, r.DefaultBranch)
+	// TODO: refactor test (use require pkg)
+	// nolint: staticcheck
+	if r.DefaultBranch != "master" {
+		t.Errorf("expected default branch to be master, got %q", r.DefaultBranch)
 	}
 	if want := 2; len(r.Branches) != want {
 		t.Errorf("expected branch count to be %d, got %d", want, len(r.Branches))

buoy/pkg/git/ruleset.go

@@ -34,8 +34,10 @@ const (
 	InvalidRule
 )
 
-var rulesetTypeString = []string{"Any", "ReleaseOrBranch", "Release", "Branch", "Invalid"}
-var rulesetLookup map[string]RulesetType
+var (
+	rulesetTypeString = []string{"Any", "ReleaseOrBranch", "Release", "Branch", "Invalid"}
+	rulesetLookup     map[string]RulesetType
+)
 
 // init will produce a ruleset lookup map to help with ruleset string conversion.
 func init() {

buoy/pkg/golang/goimport.go

@@ -22,9 +22,9 @@ import (
 	"net/http"
 	"strings"
 
-	"sigs.k8s.io/zeitgeist/buoy/pkg/git"
-
 	"golang.org/x/net/html"
+
+	"sigs.k8s.io/zeitgeist/buoy/pkg/git"
 )
 
 // MetaImport represents the parsed <meta name="go-import"
@@ -33,13 +33,15 @@ type MetaImport struct {
 	Prefix, VCS, RepoRoot string
 }
 
-func (m *MetaImport) OrgRepo() (string, string) {
+func (m *MetaImport) OrgRepo() (org, repo string) {
 	repoRoot := strings.TrimSuffix(m.RepoRoot, ".git")
 	urlParts := strings.Split(repoRoot, "://")
 	parts := strings.Split(urlParts[len(urlParts)-1], "/")
+
 	if len(parts) >= 2 {
 		return parts[len(parts)-2], parts[len(parts)-1]
 	}
+
 	panic("unknown repo root: " + m.RepoRoot)
 }
 
@@ -54,12 +56,12 @@ func metaContent(doc *html.Node, name string) (string, error) {
 					return
 				}
 			}
-
 		}
 		for child := node.FirstChild; child != nil; child = child.NextSibling {
 			crawler(child)
 		}
 	}
+
 	crawler(doc)
 	if meta != nil {
 		for _, attr := range meta.Attr {
@@ -68,6 +70,7 @@ func metaContent(doc *html.Node, name string) (string, error) {
 			}
 		}
 	}
+
 	return "", fmt.Errorf("missing <meta name=%s> in the node tree", name)
 }
 
@@ -78,6 +81,8 @@ func GetMetaImport(url string) (*MetaImport, error) {
 	if err != nil {
 		return nil, err
 	}
+	defer resp.Body.Close()
+
 	doc, err := html.Parse(resp.Body)
 	if err != nil {
 		return nil, err

buoy/pkg/golang/goimport_test.go

@@ -95,7 +95,7 @@ func TestMetaContent(t *testing.T) {
 			meta: "foo",
 			doc: func() *html.Node {
 				body := `<html><head><meta name="foo" content="bar"></head></html>`
-				doc, _ := html.Parse(strings.NewReader(body))
+				doc, _ := html.Parse(strings.NewReader(body)) // nolint: errcheck
 				return doc
 			}(),
 			want: "bar",
@@ -104,7 +104,7 @@ func TestMetaContent(t *testing.T) {
 			meta: "bar",
 			doc: func() *html.Node {
 				body := `<html><head><meta name="foo" content="bar"></head></html>`
-				doc, _ := html.Parse(strings.NewReader(body))
+				doc, _ := html.Parse(strings.NewReader(body)) // nolint: errcheck
 				return doc
 			}(),
 			wantErr: true,
@@ -120,7 +120,7 @@ func TestMetaContent(t *testing.T) {
 					<meta http-equiv="refresh" content="0; url=https://pkg.go.dev/tableflip.dev/buoy/">
 				</head>
 				</html>`
-				doc, _ := html.Parse(strings.NewReader(body))
+				doc, _ := html.Parse(strings.NewReader(body)) // nolint: errcheck
 				return doc
 			}(),
 			want: "tableflip.dev/buoy git https://github.com/n3wscott/buoy",
@@ -142,6 +142,7 @@ func TestMetaContent(t *testing.T) {
 
 func TestGetMetaImport(t *testing.T) {
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// nolint: errcheck
 		w.Write([]byte(`<html>
 		<head>
 			<meta http-equiv="Content-Type" content="text/html; charset=utf-8"/>
@@ -182,7 +183,7 @@ func TestGetMetaImport_InvalidHost(t *testing.T) {
 
 func TestGetMetaImport_MissingGoImport(t *testing.T) {
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		w.Write([]byte(`<html>hi</html>`))
+		w.Write([]byte(`<html>hi</html>`)) // nolint: errcheck
 	}))
 	defer ts.Close()
 

buoy/pkg/gomod/float.go

@@ -35,6 +35,9 @@ func Float(gomod, release, domain string, ruleset git.RulesetType) ([]string, er
 	}
 
 	this, err := semver.ParseTolerant(release)
+	if err != nil {
+		return nil, err
+	}
 
 	refs := make([]string, 0)
 	for _, pkg := range packages {