Implement new gometalinter capabilities and take care of warnings abo…

…ut them (#464)

* Updated build-tools to 1.5.1

https://github.com/drud/build-tools/releases/tag/1.5.1

* gofmt -s -w updates for dockerutils.go

pkg/dockerutil/dockerutils.go:1::warning: file is not gofmted with -s (gofmt)

* gofmt -s simplification of local_test.go

* gofmt -s simplify utils.go for loop

* gofmt -s simplify testcommon_test.go array definition

* Add gometalinter pragma/comment to ignore utils.go setLetterBytes()

* Add vetshadow to GOMETALINTER_ARGS

* Change assert import to asrt to lose shadow warnings

* Change homedir imports to gohomedir to avoid shadowing

* Fix several shadow warnings, pragma out the others

* Make 'staticrequired' target an alias of 'gometalinter'

* Use gometalinter instead of staticrequired in circleci

.circleci/config.yml

@@ -31,7 +31,7 @@ jobs:
           name: ddev tests
           no_output_timeout: "20m"
 
-      - run: make -s staticrequired
+      - run: make -s gometalinter
 
       - run:
           command: bin/linux/ddev version
@@ -88,7 +88,7 @@ jobs:
           name: ddev tests
           no_output_timeout: "20m"
 
-      - run: make -s staticrequired
+      - run: make -s gometalinter
 
       # Now build using the regular ddev-only technique - this results in a fully clean set of executables.
       # Earlier process updated submodules so now we clean them up to continue.

Makefile

@@ -1,5 +1,7 @@
 # Makefile for a standard golang repo with associated container
 
+GOMETALINTER_ARGS := --vendored-linters --disable-all --enable=gofmt --enable=vet --enable vetshadow --enable=golint --enable=errcheck --enable=staticcheck --enable=ineffassign --enable=varcheck --enable=deadcode --deadline=2m
+
 ##### These variables need to be adjusted in most repositories #####
 
 # This repo's root import path (under GOPATH).
@@ -89,4 +91,4 @@ setup:
 	@mkdir -p .go/src/$(PKG) .go/pkg .go/bin .go/std/linux
 
 # Required static analysis targets used in circleci - these cause fail if they don't work
-staticrequired: gofmt govet golint errcheck staticcheck codecoroner
+staticrequired: gometalinter

build-tools/circleci-config-yaml.example

@@ -16,5 +16,5 @@ stages:
 
       - run: make test
 
-      - run: make -s gofmt golint
+      - run: make -s gometalinter
 

build-tools/makefile_components/base_build_go.mak

@@ -21,7 +21,7 @@ BUILD_BASE_DIR ?= $$PWD
 # Expands SRC_DIRS into the common golang ./dir/... format for "all below"
 SRC_AND_UNDER = $(patsubst %,./%/...,$(SRC_DIRS))
 
-GOMETALINTER_ARGS ?= --vendored-linters --disable=gocyclo --disable=gotype --disable=goconst --disable=gas --deadline=2m
+GOMETALINTER_ARGS ?= --vendored-linters --disable-all --enable=gofmt --enable=vet --enable=golint --enable=errcheck --enable=staticcheck --enable=ineffassign --enable=varcheck --enable=deadcode --deadline=2m
 
 
 COMMIT := $(shell git describe --tags --always --dirty)
@@ -59,8 +59,6 @@ linux darwin windows: $(GOFILES)
 	@$(shell touch $@)
 	@echo $(VERSION) >VERSION.txt
 
-static: govendor gofmt govet lint
-
 govendor:
 	@echo -n "Using govendor to check for missing dependencies and unused dependencies: "
 	@docker run -t --rm -u $(shell id -u):$(shell id -g)                    \

cmd/ddev/cmd/auth_pantheon.go

@@ -5,7 +5,7 @@ import (
 
 	"github.com/drud/ddev/pkg/util"
 	"github.com/drud/go-pantheon/pkg/pantheon"
-	"github.com/mitchellh/go-homedir"
+	gohomedir "github.com/mitchellh/go-homedir"
 	"github.com/spf13/cobra"
 )
 
@@ -22,7 +22,7 @@ var PantheonAuthCommand = &cobra.Command{
 		if len(args) != 1 {
 			util.Failed("Too many arguments detected. Please provide only your Pantheon Machine token., e.g. `ddev auth-pantheon [token]`. See https://pantheon.io/docs/machine-tokens/ for instructions on creating a token.")
 		}
-		userDir, err := homedir.Dir()
+		userDir, err := gohomedir.Dir()
 		util.CheckErr(err)
 		sessionLocation := filepath.Join(userDir, ".ddev", "pantheonconfig.json")
 

cmd/ddev/cmd/describe_test.go

@@ -7,12 +7,12 @@ import (
 	"github.com/drud/ddev/pkg/plugins/platform"
 	"github.com/drud/ddev/pkg/testcommon"
 	"github.com/drud/ddev/pkg/util"
-	"github.com/stretchr/testify/assert"
+	asrt "github.com/stretchr/testify/assert"
 )
 
 // TestDescribeBadArgs ensures the binary behaves as expected when used with invalid arguments or working directories.
 func TestDescribeBadArgs(t *testing.T) {
-	assert := assert.New(t)
+	assert := asrt.New(t)
 
 	// Create a temporary directory and switch to it for the duration of this test.
 	tmpdir := testcommon.CreateTmpDir("badargs")
@@ -41,7 +41,7 @@ func TestDescribeBadArgs(t *testing.T) {
 
 // TestDescribe tests that the describe command works properly when using the binary.
 func TestDescribe(t *testing.T) {
-	assert := assert.New(t)
+	assert := asrt.New(t)
 
 	for _, v := range DevTestSites {
 		// First, try to do a describe from another directory.
@@ -75,7 +75,7 @@ func TestDescribe(t *testing.T) {
 
 // TestDescribeAppFunction performs unit tests on the describeApp function from the working directory.
 func TestDescribeAppFunction(t *testing.T) {
-	assert := assert.New(t)
+	assert := asrt.New(t)
 	for _, v := range DevTestSites {
 		cleanup := v.Chdir()
 
@@ -104,7 +104,7 @@ func TestDescribeAppFunction(t *testing.T) {
 
 // TestDescribeAppUsingSitename performs unit tests on the describeApp function using the sitename as an argument.
 func TestDescribeAppUsingSitename(t *testing.T) {
-	assert := assert.New(t)
+	assert := asrt.New(t)
 
 	// Create a temporary directory and switch to it for the duration of this test.
 	tmpdir := testcommon.CreateTmpDir("describeAppUsingSitename")
@@ -121,7 +121,7 @@ func TestDescribeAppUsingSitename(t *testing.T) {
 
 // TestDescribeAppWithInvalidParams performs unit tests on the describeApp function using a variety of invalid parameters.
 func TestDescribeAppWithInvalidParams(t *testing.T) {
-	assert := assert.New(t)
+	assert := asrt.New(t)
 
 	// Create a temporary directory and switch to it for the duration of this test.
 	tmpdir := testcommon.CreateTmpDir("TestDescribeAppWithInvalidParams")

cmd/ddev/cmd/exec_test.go

@@ -4,14 +4,14 @@ import (
 	"testing"
 
 	"github.com/drud/ddev/pkg/exec"
-	"github.com/stretchr/testify/assert"
+	asrt "github.com/stretchr/testify/assert"
 )
 
 // TestDevExecBadArgs run `ddev exec` without the proper args
 func TestDevExecBadArgs(t *testing.T) {
 	// Change to the first DevTestSite for the duration of this test.
 	defer DevTestSites[0].Chdir()()
-	assert := assert.New(t)
+	assert := asrt.New(t)
 
 	args := []string{"exec"}
 	out, err := exec.RunCommand(DdevBin, args)
@@ -22,7 +22,7 @@ func TestDevExecBadArgs(t *testing.T) {
 // TestDevExec run `ddev exec pwd` with proper args
 func TestDevExec(t *testing.T) {
 
-	assert := assert.New(t)
+	assert := asrt.New(t)
 	for _, v := range DevTestSites {
 		cleanup := v.Chdir()
 

cmd/ddev/cmd/import_test.go

@@ -9,17 +9,17 @@ import (
 
 	"github.com/drud/ddev/pkg/exec"
 	"github.com/drud/ddev/pkg/fileutil"
-	homedir "github.com/mitchellh/go-homedir"
-	"github.com/stretchr/testify/assert"
+	gohomedir "github.com/mitchellh/go-homedir"
+	asrt "github.com/stretchr/testify/assert"
 )
 
 // TestImportTilde tests passing paths to import-files that use ~ to represent home dir.
 func TestImportTilde(t *testing.T) {
-	assert := assert.New(t)
+	assert := asrt.New(t)
 
 	for _, site := range DevTestSites {
 
-		homedir, err := homedir.Dir()
+		homedir, err := gohomedir.Dir()
 		assert.NoError(err)
 		cwd, _ := os.Getwd()
 		testFile := filepath.Join(homedir, "testfile.tar.gz")

cmd/ddev/cmd/list_test.go

@@ -5,11 +5,11 @@ import (
 
 	"github.com/drud/ddev/pkg/exec"
 	"github.com/drud/ddev/pkg/plugins/platform"
-	"github.com/stretchr/testify/assert"
+	asrt "github.com/stretchr/testify/assert"
 )
 
 func TestDevList(t *testing.T) {
-	assert := assert.New(t)
+	assert := asrt.New(t)
 	args := []string{"list"}
 	out, err := exec.RunCommand(DdevBin, args)
 	assert.NoError(err)

cmd/ddev/cmd/logs_test.go

@@ -11,11 +11,11 @@ import (
 	"github.com/drud/ddev/pkg/exec"
 	"github.com/drud/ddev/pkg/testcommon"
 	"github.com/drud/ddev/pkg/util"
-	"github.com/stretchr/testify/assert"
+	asrt "github.com/stretchr/testify/assert"
 )
 
 func TestDevLogsBadArgs(t *testing.T) {
-	assert := assert.New(t)
+	assert := asrt.New(t)
 
 	testDir := testcommon.CreateTmpDir("no-valid-ddev-config")
 
@@ -32,7 +32,7 @@ func TestDevLogsBadArgs(t *testing.T) {
 
 // TestDevLogs tests that the Dev logs functionality is working.
 func TestDevLogs(t *testing.T) {
-	assert := assert.New(t)
+	assert := asrt.New(t)
 
 	for _, v := range DevTestSites {
 		cleanup := v.Chdir()

cmd/ddev/cmd/pull.go

@@ -41,7 +41,7 @@ func appImport(skipConfirmation bool) {
 	if !skipConfirmation {
 		// Unfortunately we cannot use util.Warning here as it automatically adds a newline, which is awkward when dealing with prompts.
 		d := color.New(color.FgYellow)
-		_, err := d.Printf("You're about to delete the current database and files and replace with a fresh import. Would you like to continue (y/N): ")
+		_, err = d.Printf("You're about to delete the current database and files and replace with a fresh import. Would you like to continue (y/N): ")
 		util.CheckErr(err)
 		if !util.AskForConfirmation() {
 			util.Warning("Import cancelled.")

cmd/ddev/cmd/remove_test.go

@@ -4,12 +4,12 @@ import (
 	"testing"
 
 	"github.com/drud/ddev/pkg/exec"
-	"github.com/stretchr/testify/assert"
+	asrt "github.com/stretchr/testify/assert"
 )
 
 // TestDevRestart runs `drud legacy restart` on the test apps
 func TestDevRemove(t *testing.T) {
-	assert := assert.New(t)
+	assert := asrt.New(t)
 
 	// Make sure we have running sites.
 	addSites()

cmd/ddev/cmd/restart_test.go

@@ -6,12 +6,12 @@ import (
 
 	"github.com/drud/ddev/pkg/exec"
 	"github.com/drud/ddev/pkg/plugins/platform"
-	"github.com/stretchr/testify/assert"
+	asrt "github.com/stretchr/testify/assert"
 )
 
 // TestDevRestart runs `drud legacy restart` on the test apps
 func TestDevRestart(t *testing.T) {
-	assert := assert.New(t)
+	assert := asrt.New(t)
 	containerPrefix := "ddev"
 	for _, site := range DevTestSites {
 		cleanup := site.Chdir()

cmd/ddev/cmd/root.go

@@ -62,6 +62,7 @@ var RootCmd = &cobra.Command{
 		}
 
 		if timeToCheckForUpdates {
+			// nolint: vetshadow
 			updateNeeded, updateURL, err := updatecheck.AvailableUpdates("drud", "ddev", version.DdevVersion)
 
 			if err != nil {

cmd/ddev/cmd/root_test.go

@@ -14,7 +14,7 @@ import (
 	"github.com/drud/ddev/pkg/appports"
 	"github.com/drud/ddev/pkg/exec"
 	"github.com/drud/ddev/pkg/plugins/platform"
-	"github.com/stretchr/testify/assert"
+	asrt "github.com/stretchr/testify/assert"
 )
 
 var (
@@ -67,7 +67,7 @@ func TestMain(m *testing.M) {
 }
 
 func TestGetActiveAppRoot(t *testing.T) {
-	assert := assert.New(t)
+	assert := asrt.New(t)
 
 	_, err := platform.GetActiveAppRoot("")
 	assert.Contains(err.Error(), "unable to determine the application for this command")
@@ -90,35 +90,37 @@ func TestGetActiveAppRoot(t *testing.T) {
 
 // TestCreateGlobalDdevDir checks to make sure that ddev will create a ~/.ddev (and updatecheck)
 func TestCreateGlobalDdevDir(t *testing.T) {
+	assert := asrt.New(t)
+
 	tmpDir := testcommon.CreateTmpDir("globalDdevCheck")
 	origHome := os.Getenv("HOME")
 
 	// Make sure that the tmpDir/.ddev and tmpDir/.ddev/.update don't exist before we run ddev.
 	_, err := os.Stat(filepath.Join(tmpDir, ".ddev"))
-	assert.Error(t, err)
-	assert.True(t, os.IsNotExist(err))
+	assert.Error(err)
+	assert.True(os.IsNotExist(err))
 
 	_, err = os.Stat(filepath.Join(tmpDir, ".ddev", ".update"))
-	assert.Error(t, err)
-	assert.True(t, os.IsNotExist(err))
+	assert.Error(err)
+	assert.True(os.IsNotExist(err))
 
 	// Change the homedir temporarily
 	err = os.Setenv("HOME", tmpDir)
-	assert.NoError(t, err)
+	assert.NoError(err)
 
 	args := []string{"list"}
 	_, err = exec.RunCommand(DdevBin, args)
-	assert.NoError(t, err)
+	assert.NoError(err)
 
 	_, err = os.Stat(filepath.Join(tmpDir, ".ddev", ".update"))
-	assert.NoError(t, err)
+	assert.NoError(err)
 
 	// Cleanup our tmp homedir
 	err = os.RemoveAll(tmpDir)
-	assert.NoError(t, err)
+	assert.NoError(err)
 
 	err = os.Setenv("HOME", origHome)
-	assert.NoError(t, err)
+	assert.NoError(err)
 }
 
 // addSites runs `ddev start` on the test apps

cmd/ddev/cmd/sequelpro_test.go

@@ -8,15 +8,15 @@ import (
 	"github.com/drud/ddev/pkg/fileutil"
 	"github.com/drud/ddev/pkg/plugins/platform"
 	"github.com/drud/ddev/pkg/testcommon"
-	"github.com/stretchr/testify/assert"
+	asrt "github.com/stretchr/testify/assert"
 )
 
 // TestSequelproOperation tests basic operation.
 func TestSequelproOperation(t *testing.T) {
 	if !detectSequelpro() {
 		t.SkipNow()
 	}
-	assert := assert.New(t)
+	assert := asrt.New(t)
 	v := DevTestSites[0]
 	cleanup := v.Chdir()
 
@@ -40,7 +40,7 @@ func TestSequelproBadApp(t *testing.T) {
 		t.SkipNow()
 	}
 
-	assert := assert.New(t)
+	assert := asrt.New(t)
 
 	// Create a temporary directory and switch to it for the duration of this test.
 	tmpdir := testcommon.CreateTmpDir("sequelpro_badargs")

cmd/ddev/cmd/version_test.go

@@ -5,11 +5,11 @@ import (
 	"testing"
 
 	"github.com/drud/ddev/pkg/version"
-	"github.com/stretchr/testify/assert"
+	asrt "github.com/stretchr/testify/assert"
 )
 
 func TestVersion(t *testing.T) {
-	assert := assert.New(t)
+	assert := asrt.New(t)
 	v := handleVersionCommand().String()
 	output := strings.TrimSpace(v)
 	assert.Contains(output, version.DdevVersion)

pkg/appimport/appimport.go

@@ -9,7 +9,7 @@ import (
 
 	"path/filepath"
 
-	homedir "github.com/mitchellh/go-homedir"
+	gohomedir "github.com/mitchellh/go-homedir"
 )
 
 // ValidateAsset determines if a given asset matches the required criteria for a given asset type.
@@ -19,7 +19,7 @@ func ValidateAsset(assetPath string, assetType string) (string, error) {
 	extensions := []string{"tar", "gz", "tgz", "zip"}
 
 	// Input provided via prompt or "--flag=value" is not expanded by shell. This will help ensure ~ is expanded to the user home directory.
-	assetPath, err := homedir.Expand(assetPath)
+	assetPath, err := gohomedir.Expand(assetPath)
 	if err != nil {
 		return "", fmt.Errorf(invalidAssetError, err)
 	}