lint pass (chanzuckerberg#246)

lint passSwitch to golangci-lint + clean up findings.

### Summary

This started as a pass to clean up any lint findings, but found that I also needed to switch from gometalinter (which is slow and deprecated) to golangci-lint.

### Test Plan

`make lint test`

### References

* https://github.com/alecthomas/gometalinter
* https://github.com/golangci/golangci-lint

Makefile

@@ -12,12 +12,11 @@ setup: ## setup development dependencies
 	go get -u github.com/gobuffalo/packr/...
 	go install github.com/gobuffalo/packr/packr
 	curl -L https://raw.githubusercontent.com/chanzuckerberg/bff/master/download.sh | sh
+	curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh
 
 lint: ## run the fast go linters
-	gometalinter --vendor --fast ./...
-
-lint-slow: ## run all linters, even the slow ones
-	gometalinter --vendor --deadline 120s ./...
+	golangci-lint run
+.PHONY: lint
 
 packr: ## run the packr tool to generate our static files
 	packr clean -v
@@ -73,4 +72,4 @@ update-golden-files: dep ## update the golden files in testdata
 	go test -v -run TestIntegration ./apply/ -update
 .PHONY: update-golden-files
 
-.PHONY: build clean coverage test install lint lint-slow packr release help setup
+.PHONY: build clean coverage test install packr release help setup

apply/apply_test.go

@@ -88,7 +88,7 @@ func TestApplyTemplateBasicNewDirectory(t *testing.T) {
 	defer os.RemoveAll(d)
 
 	nonexistentDir := getNonExistentDirectoryName()
-	defer dest.RemoveAll(nonexistentDir)
+	defer dest.RemoveAll(nonexistentDir) //nolint
 	path := filepath.Join(nonexistentDir, "bar")
 	overrides := struct{ Foo string }{"foo"}
 
@@ -136,7 +136,8 @@ func TestTouchFile(t *testing.T) {
 	a.NoError(err)
 	defer os.RemoveAll(d2)
 
-	writeFile(fs, "asdf", "jkl")
+	err = writeFile(fs, "asdf", "jkl")
+	a.NoError(err)
 
 	r, e = readFile(fs, "asdf")
 	a.Nil(e)
@@ -156,7 +157,7 @@ func TestTouchFileNonExistentDirectory(t *testing.T) {
 	defer os.RemoveAll(d)
 
 	nonexistentDir := getNonExistentDirectoryName()
-	defer dest.RemoveAll(nonexistentDir)
+	defer dest.RemoveAll(nonexistentDir) //nolint
 	e := touchFile(dest, filepath.Join(nonexistentDir, "foo"))
 	a.Nil(e)
 	r, e := readFile(dest, filepath.Join(nonexistentDir, "foo"))
@@ -400,7 +401,8 @@ func TestCheckToolVersions(t *testing.T) {
 			a.NoError(err)
 			defer os.RemoveAll(d)
 
-			writeFile(fs, ".fogg-version", tc.current)
+			err = writeFile(fs, ".fogg-version", tc.current)
+			a.NoError(err)
 
 			v, _, e := checkToolVersions(fs, tc.tool)
 			a.NoError(e)

apply/golden_file_test.go

@@ -63,7 +63,7 @@ func TestIntegration(t *testing.T) {
 				a.NoError(e)
 				configMode, e := testdataFs.Stat("fogg.json")
 				a.NoError(e)
-				afero.WriteFile(fs, "fogg.json", configContents, configMode.Mode())
+				a.NoError(afero.WriteFile(fs, "fogg.json", configContents, configMode.Mode()))
 
 				conf, e := config.FindAndReadConfig(fs, "fogg.json")
 				a.NoError(e)
@@ -75,7 +75,7 @@ func TestIntegration(t *testing.T) {
 				e = apply.Apply(fs, conf, templates.Templates, true)
 				a.NoError(e)
 
-				afero.Walk(testdataFs, ".", func(path string, info os.FileInfo, err error) error {
+				a.NoError(afero.Walk(testdataFs, ".", func(path string, info os.FileInfo, err error) error {
 					log.Debug("================================================")
 					log.Debug(path)
 					if !info.Mode().IsRegular() {
@@ -104,7 +104,7 @@ func TestIntegration(t *testing.T) {
 						a.Equal(f1, f2)
 					}
 					return nil
-				})
+				}))
 			}
 		})
 	}

config/config_test.go

@@ -282,6 +282,9 @@ func TestFindAndReadConfig(t *testing.T) {
 				return nil, e
 			}
 			_, e = f.Write(v)
+			if e != nil {
+				return nil, e
+			}
 		}
 		return fs, nil
 	}
@@ -297,19 +300,19 @@ func TestFindAndReadConfig(t *testing.T) {
 		"config.json": v1,
 	})
 	a.NoError(e)
-	defer f1.RemoveAll(".")
+	defer f1.RemoveAll(".") //nolint
 
 	f2, e := fs(map[string][]byte{
 		"config.json": v2,
 	})
 	a.NoError(e)
-	defer f2.RemoveAll(".")
+	defer f2.RemoveAll(".") //nolint
 
 	fErr, e := fs(map[string][]byte{
 		"config.json": []byte(`{"version": 7}`),
 	})
 	a.NoError(e)
-	defer fErr.RemoveAll(".")
+	defer fErr.RemoveAll(".") //nolint
 
 	_, e = FindAndReadConfig(f1, "config.json")
 	a.NoError(e)

exp/migrate/migrate.go

@@ -21,7 +21,7 @@ func generatePlan(planPath string) error {
 	cmd.Stdout = os.Stdout
 	err := cmd.Run()
 	if err != nil {
-		errors.Wrap(err, "Could not run make init")
+		return errors.Wrap(err, "Could not run make init")
 	}
 
 	cmd = exec.Command("make", "run")

plan/plan.go

@@ -305,27 +305,6 @@ func resolveExtraVars(vars ...map[string]string) map[string]string {
 	return resolved
 }
 
-func resolveRequired(def *string, override *string) string {
-	if override != nil && *override != "" {
-		return *override
-	}
-	return *def
-}
-
-func resolveRequiredInt(def int64, override *int64) int64 {
-	if override != nil {
-		return *override
-	}
-	return def
-}
-
-func resolveOptionalInt(def *int64, override *int64) *int64 {
-	if override != nil {
-		return override
-	}
-	return def
-}
-
 func resolveAccounts(accounts map[string]v2.Account) map[string]int64 {
 	a := make(map[string]int64)
 	for name, account := range accounts {

plan/plan_quick_test.go

@@ -10,10 +10,6 @@ import (
 	"github.com/chanzuckerberg/fogg/plan"
 )
 
-func add(x, y int) int {
-	return x + y
-}
-
 func TestValidConfigNoPanic(t *testing.T) {
 
 	// return false if valid + panic
@@ -34,7 +30,10 @@ func TestValidConfigNoPanic(t *testing.T) {
 				return true
 			}()
 
-			plan.Eval(conf)
+			_, err = plan.Eval(conf)
+			if err != nil {
+				panic(err)
+			}
 
 		} else {
 			fmt.Println("invalid")

plugins/custom_plugin_test.go

@@ -46,10 +46,10 @@ func TestCustomPluginTar(t *testing.T) {
 	customPlugin.SetTargetPath(plugins.CustomPluginDir)
 	a.Nil(customPlugin.Install(fs, pluginName))
 
-	afero.Walk(fs, "", func(path string, info os.FileInfo, err error) error {
+	a.NoError(afero.Walk(fs, "", func(path string, info os.FileInfo, err error) error {
 		a.Nil(err)
 		return nil
-	})
+	}))
 
 	for _, file := range files {
 		filePath := path.Join(plugins.CustomPluginDir, file)
@@ -95,10 +95,10 @@ func TestCustomPluginTarStripComponents(t *testing.T) {
 	customPlugin.SetTargetPath(plugins.CustomPluginDir)
 	a.Nil(customPlugin.Install(fs, pluginName))
 
-	afero.Walk(fs, "", func(path string, info os.FileInfo, err error) error {
+	a.NoError(afero.Walk(fs, "", func(path string, info os.FileInfo, err error) error {
 		a.Nil(err)
 		return nil
-	})
+	}))
 
 	for idx, file := range expected_files {
 		// files we expect to skip
@@ -146,10 +146,10 @@ func TestCustomPluginZip(t *testing.T) {
 	customPlugin.SetTargetPath(plugins.CustomPluginDir)
 	a.Nil(customPlugin.Install(fs, pluginName))
 
-	afero.Walk(fs, "", func(path string, info os.FileInfo, err error) error {
+	a.NoError(afero.Walk(fs, "", func(path string, info os.FileInfo, err error) error {
 		a.Nil(err)
 		return nil
-	})
+	}))
 
 	for _, file := range files {
 		filePath := path.Join(plugins.CustomPluginDir, file)
@@ -186,10 +186,10 @@ func TestCustomPluginBin(t *testing.T) {
 	customPlugin.SetTargetPath(plugins.CustomPluginDir)
 	a.Nil(customPlugin.Install(fs, pluginName))
 
-	afero.Walk(fs, "", func(path string, info os.FileInfo, err error) error {
+	a.NoError(afero.Walk(fs, "", func(path string, info os.FileInfo, err error) error {
 		a.Nil(err)
 		return nil
-	})
+	}))
 
 	customPluginPath := path.Join(plugins.CustomPluginDir, pluginName)
 	f, err := fs.Open(customPluginPath)