Enable staticcheck linter

Prevents among others:
* Variables that are appended to but never read from
* Shadowed+dropped errors, most notably in a bunch of filepath.Read
* Invalid xml struct tags
* strings.TrimLeft used as strings.TrimPrefix, which is wrong

.golangci.yml

@@ -18,10 +18,18 @@ linters:
   - deadcode
   - misspell
   - ineffassign
+  - staticcheck
   disable-all: true
 
 issues:
   exclude-rules:
   - linters:
     - golint
     text: ".*should not use dot dot imports"
+  - linters:
+    - staticcheck
+    text: "SA1019: fakectrlruntimeclient.NewFakeClient is deprecated"
+  - linters:
+    - staticcheck
+    # TODO: Is this really supposed to be deprecated? Ref https://github.com/kubernetes/test-infra/issues/14875
+    text: "SA1019: t.*.TrustedOrg is deprecated: TrustedOrg functionality is deprecated and will be removed in January 2020"

config/tests/testgrids/config_test.go

@@ -416,7 +416,7 @@ func TestPresubmitsKubernetesDashboards(t *testing.T) {
 		}
 	}
 	if dashboard == nil {
-		t.Errorf("Missing dashboard: %s", dash)
+		t.Fatalf("Missing dashboard: %s", dash)
 	}
 	testgroups := make(map[string]bool)
 	for _, tab := range dashboard.DashboardTab {

ghproxy/ghcache/partitioner_test.go

@@ -77,7 +77,7 @@ func TestPartitioningRoundTripper(t *testing.T) {
 			defer wg.Done()
 			_, err := partitioningRoundTripper.RoundTrip(request)
 			if err != nil {
-				t.Fatalf("RoundTrip: %v", err)
+				t.Errorf("RoundTrip: %v", err)
 			}
 		}()
 	}

kubetest/aksengine.go

@@ -1188,7 +1188,7 @@ func (c *aksEngineDeployer) DumpClusterLogs(localPath, gcsPath string) error {
 	}
 	if err := logDumperWindows(); err != nil {
 		// don't log error since logDumperWindows failed is expected on non-Windows cluster
-		//errors = append(errors, err.Error())
+		_ = err
 	}
 	if len(errors) != 0 {
 		return fmt.Errorf(strings.Join(errors, "\n"))

kubetest/util/suite.go

@@ -37,7 +37,7 @@ type TestCase struct {
 // A build (column in testgrid) is composed of one or more TestSuites.
 type TestSuite struct {
 	XMLName  xml.Name `xml:"testsuite"`
-	Name     string   `xml:"name,arr"`
+	Name     string   `xml:"name,attr"`
 	Time     float64  `xml:"time,attr"` // seconds
 	Failures int      `xml:"failures,attr"`
 	Tests    int      `xml:"tests,attr"`

label_sync/main.go

@@ -53,8 +53,8 @@ type LabelTarget string
 
 const (
 	prTarget    LabelTarget = "prs"
-	issueTarget             = "issues"
-	bothTarget              = "both"
+	issueTarget LabelTarget = "issues"
+	bothTarget  LabelTarget = "both"
 )
 
 // Label holds declarative data about the label.

linkcheck/links.go

@@ -73,6 +73,9 @@ var (
 
 func newWalkFunc(invalidLink *bool, client *http.Client) filepath.WalkFunc {
 	return func(filePath string, info os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
 		hasSuffix := false
 		for _, suffix := range *fileSuffix {
 			hasSuffix = hasSuffix || strings.HasSuffix(info.Name(), suffix)

prow/clonerefs/run.go

@@ -268,6 +268,9 @@ func addSSHKeys(paths []string) ([]string, []clone.Command, error) {
 		// that are mounted from a secret, so we need to check which
 		// we have
 		if err := filepath.Walk(keyPath, func(path string, info os.FileInfo, err error) error {
+			if err != nil {
+				return err
+			}
 			if strings.HasPrefix(info.Name(), "..") {
 				// kubernetes volumes also include files we
 				// should not look be looking into for keys

prow/cmd/branchprotector/protect_test.go

@@ -125,20 +125,17 @@ func (c fakeClient) GetBranches(org, repo string, onlyProtected bool) ([]github.
 	if !ok {
 		return nil, fmt.Errorf("Unknown repo: %s/%s", org, repo)
 	}
-	var out []github.Branch
 	if onlyProtected {
 		for _, item := range b {
 			if !item.Protected {
 				continue
 			}
-			out = append(out, item)
 		}
 	} else {
 		// when !onlyProtected, github does not set Protected
 		// match that behavior here to ensure we handle this correctly
 		for _, item := range b {
 			item.Protected = false
-			out = append(out, item)
 		}
 	}
 	return b, nil

prow/cmd/deck/main.go

@@ -693,7 +693,7 @@ func initSpyglass(cfg config.Getter, o options, mux *http.ServeMux, ja *jobs.Job
 func initLocalLensHandler(cfg config.Getter, o options, sg *spyglass.Spyglass) error {
 	var localLenses []common.LensWithConfiguration
 	for _, lfc := range cfg().Deck.Spyglass.Lenses {
-		if !strings.HasPrefix(strings.TrimLeft(lfc.RemoteConfig.Endpoint, "http://"), spyglassLocalLensListenerAddr) {
+		if !strings.HasPrefix(strings.TrimPrefix(lfc.RemoteConfig.Endpoint, "http://"), spyglassLocalLensListenerAddr) {
 			continue
 		}
 

prow/cmd/deck/tide.go

@@ -163,13 +163,10 @@ func (ta *tideAgent) filterHiddenPools(pools []tide.Pool) []tide.Pool {
 	filtered := make([]tide.Pool, 0, len(pools))
 	for _, pool := range pools {
 		needsHide := matches(pool.Org+"/"+pool.Repo, ta.hiddenRepos())
-		var ignored []string
 		if needsHide && ta.showHidden {
 			filtered = append(filtered, pool)
 		} else if needsHide == ta.hiddenOnly {
 			filtered = append(filtered, pool)
-		} else {
-			ignored = append(ignored, pool.Org+"/"+pool.Repo)
 		}
 	}
 	return filtered

prow/plank/reconciler_test.go

@@ -182,12 +182,12 @@ func TestAdd(t *testing.T) {
 
 			go func() {
 				if err := mgr.Start(ctx); err != nil {
-					t.Fatalf("failed to start main mgr: %v", err)
+					t.Errorf("failed to start main mgr: %v", err)
 				}
 			}()
 			go func() {
 				if err := buildMgrs["default"].Start(ctx); err != nil {
-					t.Fatalf("failed to start build mgr: %v", err)
+					t.Errorf("failed to start build mgr: %v", err)
 				}
 			}()
 			if err := singnalOrTimout(prowJobInformerStarted); err != nil {

prow/plugins/plugins.go

@@ -299,9 +299,9 @@ func (pa *ConfigAgent) Start(path string, checkUnknownPlugins bool) error {
 	if err := pa.Load(path, checkUnknownPlugins); err != nil {
 		return err
 	}
-	ticker := time.Tick(1 * time.Minute)
+	ticker := time.NewTicker(time.Minute)
 	go func() {
-		for range ticker {
+		for range ticker.C {
 			if err := pa.Load(path, checkUnknownPlugins); err != nil {
 				logrus.WithField("path", path).WithError(err).Error("Error loading plugin config.")
 			}

prow/plugins/yuks/yuks.go

@@ -107,7 +107,7 @@ func handleGenericComment(pc plugins.Agent, e github.GenericCommentEvent) error
 // references with the decimal notation. See https://www.w3.org/TR/html401/charset.html#h-5.3.1
 func escapeMarkdown(s string) string {
 	var b bytes.Buffer
-	for _, r := range []rune(s) {
+	for _, r := range s {
 		// Check for simple characters as they are considered safe, otherwise we escape the rune.
 		c := string(r)
 		if simple.MatchString(c) {

prow/sidecar/censor.go

@@ -90,6 +90,9 @@ func (o Options) censor() error {
 
 	for _, item := range o.GcsOptions.Items {
 		if err := filepath.Walk(item, func(absPath string, info os.FileInfo, err error) error {
+			if err != nil {
+				return err
+			}
 			if info.IsDir() || info.Mode()&os.ModeSymlink == os.ModeSymlink {
 				return nil
 			}
@@ -317,6 +320,9 @@ func archive(srcDir, destArchive string) error {
 	}()
 
 	if err := filepath.Walk(srcDir, func(absPath string, info os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
 		header, err := tar.FileInfoHeader(info, info.Name())
 		if err != nil {
 			return fmt.Errorf("could not create tar header: %w", err)
@@ -440,6 +446,9 @@ func loadSecrets(paths []string) ([][]byte, error) {
 	var secrets [][]byte
 	for _, path := range paths {
 		if err := filepath.Walk(path, func(path string, info os.FileInfo, err error) error {
+			if err != nil {
+				return err
+			}
 			if strings.HasPrefix(info.Name(), "..") {
 				// kubernetes volumes also include files we
 				// should not look be looking into for keys

prow/sidecar/censor_test.go

@@ -93,6 +93,9 @@ const inputDir = "testdata/input"
 func copyTestData(t *testing.T) string {
 	tempDir := t.TempDir()
 	if err := filepath.Walk(inputDir, func(path string, info os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
 		relpath, _ := filepath.Rel(inputDir, path) // this errors when it's not relative, but that's known here
 		dest := filepath.Join(tempDir, relpath)
 		if info.IsDir() {

prow/test/integration/test/horologium_test.go

@@ -73,12 +73,11 @@ func TestLaunchProwJob(t *testing.T) {
 					sort.Slice(pjs.Items, func(i, j int) bool {
 						return pjs.Items[i].Status.StartTime.After(pjs.Items[j].Status.StartTime.Time)
 					})
-					for _, pj := range pjs.Items {
-						if lastRun != nil && pj.CreationTimestamp.Before(lastRun) {
+					if len(pjs.Items) > 0 {
+						if lastRun != nil && pjs.Items[0].CreationTimestamp.Before(lastRun) {
 							return false, nil
 						}
-						res = &pj
-						break
+						res = &pjs.Items[0]
 					}
 					return res != nil, nil
 				}); err != nil {

prow/tide/status.go

@@ -43,8 +43,8 @@ import (
 )
 
 const (
-	statusContext string = "tide"
-	statusInPool         = "In merge pool."
+	statusContext = "tide"
+	statusInPool  = "In merge pool."
 	// statusNotInPool is a format string used when a PR is not in a tide pool.
 	// The '%s' field is populated with the reason why the PR is not in a
 	// tide pool or the empty string if the reason is unknown. See requirementDiff.

prow/tide/tide.go

@@ -101,11 +101,11 @@ type Action string
 // Constants for various actions the controller might take
 const (
 	Wait         Action = "WAIT"
-	Trigger             = "TRIGGER"
-	TriggerBatch        = "TRIGGER_BATCH"
-	Merge               = "MERGE"
-	MergeBatch          = "MERGE_BATCH"
-	PoolBlocked         = "BLOCKED"
+	Trigger      Action = "TRIGGER"
+	TriggerBatch Action = "TRIGGER_BATCH"
+	Merge        Action = "MERGE"
+	MergeBatch   Action = "MERGE_BATCH"
+	PoolBlocked  Action = "BLOCKED"
 )
 
 // recordableActions is the subset of actions that we keep historical record of.

robots/issue-creator/testowner/owner_test.go

@@ -19,6 +19,7 @@ package testowner
 import (
 	"bufio"
 	"bytes"
+	"io"
 	"io/ioutil"
 	"os"
 	"testing"
@@ -146,7 +147,7 @@ func TestReloadingOwnerList(t *testing.T) {
 		time.Sleep(5 * time.Millisecond)
 		// Clear file and reset writing offset
 		tempfile.Truncate(0)
-		tempfile.Seek(0, os.SEEK_SET)
+		tempfile.Seek(0, io.SeekStart)
 		writer.Reset(tempfile)
 		_, err = writer.WriteString(tc.csv)
 		if err != nil {