gometalinter -> golangci-lint migration

{,scripts/}Makefile:
- Remove gometalinter, install golangci-lint.
- Remove distinction between tools and devtools.
  Just tools is enough.
- test_lint -> lint
  Migrating away from underscore separated names.
- Remove unnecessary targets.
- Drop tendermint/lint. Incompatbile with golangci-lint
  and no longer necessary anyway.

Port tools/gometalinter.json to .golangci.yml
Update CircleCI config accordingly.

Closes: #3896

.circleci/config.yml

@@ -79,17 +79,11 @@ jobs:
           at: /tmp/workspace
       - checkout
       - *dependencies
-      - run:
-          name: Get metalinter
-          command: |
-            export PATH="$GOBIN:$PATH"
-            make devtools-clean
-            make devtools
       - run:
           name: Lint source
           command: |
             export PATH="$GOBIN:$PATH"
-            make test_lint
+            make lint
 
   integration_tests:
     <<: *linux_defaults

.golangci.yml

@@ -0,0 +1,17 @@
+linters:
+  disable-all: true
+  enable:
+    - errcheck
+    - golint
+    - ineffassign
+    - unconvert
+    - misspell
+linters-settings:
+  gocyclo:
+    min-complexity: 11
+  errcheck:
+    ignore: fmt:.*,io/ioutil:^Read.*,github.com/spf13/cobra:MarkFlagRequired,github.com/spf13/viper:BindPFlag
+  golint:
+    min-confidence: 1.1
+run:
+  tests: false

.pending/improvements/sdk/Fixed-various-linter

@@ -0,0 +1 @@
+Fixed various linters warnings in the context of the gometalinter -> golangci-lint migration #3896.

CONTRIBUTING.md

@@ -165,7 +165,7 @@ only pull requests targeted directly against master.
 ### Development Procedure:
   - the latest state of development is on `develop`
   - `develop` must never fail `make test` or `make test_cli`
-  - `develop` should not fail `make test_lint`
+  - `develop` should not fail `make lint`
   - no --force onto `develop` (except when reverting a broken commit, which should seldom happen)
   - create a development branch either on github.com/cosmos/cosmos-sdk, or your fork (using `git remote add origin`)
   - before submitting a pull request, begin `git rebase` on top of `develop`

Makefile

@@ -4,9 +4,6 @@ VERSION := $(shell echo $(shell git describe --tags) | sed 's/^v//')
 COMMIT := $(shell git log -1 --format='%H')
 CAT := $(if $(filter $(OS),Windows_NT),type,cat)
 LEDGER_ENABLED ?= true
-GOTOOLS = \
-	github.com/alecthomas/gometalinter \
-	github.com/rakyll/statik
 GOBIN ?= $(GOPATH)/bin
 GOSUM := $(shell which gosum)
 
@@ -62,15 +59,15 @@ ldflags := $(strip $(ldflags))
 
 BUILD_FLAGS := -tags "$(build_tags)" -ldflags '$(ldflags)'
 
-all: devtools install test_lint test
+all: tools install lint test
 
 # The below include contains the tools target.
 include scripts/Makefile
 
 ########################################
 ### CI
 
-ci: devtools install test_cover test_lint test
+ci: tools install test_cover lint test
 
 ########################################
 ### Build/Install
@@ -108,31 +105,6 @@ dist:
 ########################################
 ### Tools & dependencies
 
-check_tools:
-	@# https://stackoverflow.com/a/25668869
-	@echo "Found tools: $(foreach tool,$(notdir $(GOTOOLS)),\
-        $(if $(shell which $(tool)),$(tool),$(error "No $(tool) in PATH")))"
-
-update_tools:
-	@echo "--> Updating tools to correct version"
-	$(MAKE) --always-make tools
-
-update_dev_tools:
-	@echo "--> Downloading linters (this may take awhile)"
-	$(GOPATH)/src/github.com/alecthomas/gometalinter/scripts/install.sh -b $(GOBIN)
-	go get -u github.com/tendermint/lint/golint
-
-devtools: devtools-stamp
-devtools-stamp: tools
-	@echo "--> Downloading linters (this may take awhile)"
-	$(GOPATH)/src/github.com/alecthomas/gometalinter/scripts/install.sh -b $(GOBIN)
-	go get github.com/tendermint/lint/golint
-	go install -mod=readonly ./cmd/sdkch
-	touch $@
-
-devtools-clean: tools-clean
-	rm -f devtools-stamp
-
 go-mod-cache: go.sum
 	@echo "--> Download go modules to local cache"
 	@go mod download
@@ -147,7 +119,7 @@ draw_deps: tools
 	@goviz -i github.com/cosmos/cosmos-sdk/cmd/gaia/cmd/gaiad -d 2 | dot -Tpng -o dependency-graph.png
 
 clean:
-	rm -f devtools-stamp snapcraft-local.yaml
+	rm -f snapcraft-local.yaml
 
 distclean: clean
 	rm -rf vendor/
@@ -227,13 +199,13 @@ test_sim_gaia_profile:
 test_cover:
 	@export VERSION=$(VERSION); bash -x tests/test_cover.sh
 
-test_lint:
-	gometalinter --config=tools/gometalinter.json ./...
-	!(gometalinter --exclude /usr/lib/go/src/ --exclude client/lcd/statik/statik.go --exclude 'vendor/*' --disable-all --enable='errcheck' --vendor ./... | grep -v "client/")
+lint: tools
+	golangci-lint run
+	go vet -composites=false -tests=false ./...
 	find . -name '*.go' -type f -not -path "./vendor*" -not -path "*.git*" | xargs gofmt -d -s
 	go mod verify
 
-format:
+format: tools
 	find . -name '*.go' -type f -not -path "./vendor*" -not -path "*.git*" -not -path "./client/lcd/statik/statik.go" | xargs gofmt -w -s
 	find . -name '*.go' -type f -not -path "./vendor*" -not -path "*.git*" -not -path "./client/lcd/statik/statik.go" | xargs misspell -w
 	find . -name '*.go' -type f -not -path "./vendor*" -not -path "*.git*" -not -path "./client/lcd/statik/statik.go" | xargs goimports -w -local github.com/cosmos/cosmos-sdk
@@ -292,10 +264,10 @@ snapcraft-local.yaml: snapcraft-local.yaml.in
 # unless there is a reason not to.
 # https://www.gnu.org/software/make/manual/html_node/Phony-Targets.html
 .PHONY: build install install_debug dist clean distclean \
-check_tools check_dev_tools get_vendor_deps draw_deps test test_cli test_unit \
-test_cover test_lint benchmark devdoc_init devdoc devdoc_save devdoc_update \
+draw_deps test test_cli test_unit \
+test_cover lint benchmark devdoc_init devdoc devdoc_save devdoc_update \
 build-linux build-docker-gaiadnode localnet-start localnet-stop \
 format check-ledger test_sim_gaia_nondeterminism test_sim_modules test_sim_gaia_fast \
 test_sim_gaia_custom_genesis_fast test_sim_gaia_custom_genesis_multi_seed \
-test_sim_gaia_multi_seed test_sim_gaia_import_export update_tools update_dev_tools \
-devtools-clean go-mod-cache
+test_sim_gaia_multi_seed test_sim_gaia_import_export \
+go-mod-cache

client/keys/types.go

@@ -11,6 +11,17 @@ type AddNewKey struct {
 	Index    int    `json:"index,string,omitempty"`
 }
 
+// NewAddNewKey constructs a new AddNewKey request structure.
+func NewAddNewKey(name, password, mnemonic string, account, index int) AddNewKey {
+	return AddNewKey{
+		Name:     name,
+		Password: password,
+		Mnemonic: mnemonic,
+		Account:  account,
+		Index:    index,
+	}
+}
+
 // RecoverKeyBody recovers a key
 type RecoverKey struct {
 	Password string `json:"password"`
@@ -19,13 +30,26 @@ type RecoverKey struct {
 	Index    int    `json:"index,string,omitempty"`
 }
 
+// NewRecoverKey constructs a new RecoverKey request structure.
+func NewRecoverKey(password, mnemonic string, account, index int) RecoverKey {
+	return RecoverKey{Password: password, Mnemonic: mnemonic, Account: account, Index: index}
+}
+
 // UpdateKeyReq requests updating a key
 type UpdateKeyReq struct {
 	OldPassword string `json:"old_password"`
 	NewPassword string `json:"new_password"`
 }
 
+// NewUpdateKeyReq constructs a new UpdateKeyReq structure.
+func NewUpdateKeyReq(old, new string) UpdateKeyReq {
+	return UpdateKeyReq{OldPassword: old, NewPassword: new}
+}
+
 // DeleteKeyReq requests deleting a key
 type DeleteKeyReq struct {
 	Password string `json:"password"`
 }
+
+// NewDeleteKeyReq constructs a new DeleteKeyReq structure.
+func NewDeleteKeyReq(password string) DeleteKeyReq { return DeleteKeyReq{Password: password} }

client/lcd/certificates.go

@@ -145,7 +145,9 @@ func fingerprintForCertificate(certBytes []byte) (string, error) {
 		return "", err
 	}
 	h := sha256.New()
-	h.Write(cert.Raw)
+	if _, err := h.Write(cert.Raw); err != nil {
+		return "", err
+	}
 	fingerprintBytes := h.Sum(nil)
 	var buf bytes.Buffer
 	for i, b := range fingerprintBytes {

client/lcd/test_helpers.go

@@ -227,7 +227,7 @@ func InitializeTestLCD(t *testing.T, nValidators int, initAddrs []sdk.AccAddress
 	genDoc, err := tmtypes.GenesisDocFromFile(genesisFile)
 	require.Nil(t, err)
 	genDoc.Validators = nil
-	genDoc.SaveAs(genesisFile)
+	require.NoError(t, genDoc.SaveAs(genesisFile))
 	genTxs := []json.RawMessage{}
 
 	// append any additional (non-proposing) validators
@@ -337,7 +337,7 @@ func InitializeTestLCD(t *testing.T, nValidators int, initAddrs []sdk.AccAddress
 
 	cleanup = func() {
 		logger.Debug("cleaning up LCD initialization")
-		node.Stop()
+		node.Stop() //nolint:errcheck
 		node.Wait()
 		lcd.Close()
 	}
@@ -394,7 +394,7 @@ func startLCD(logger log.Logger, listenAddr string, cdc *codec.Codec, t *testing
 	if err != nil {
 		return nil, err
 	}
-	go tmrpc.StartHTTPServer(listener, rs.Mux, logger)
+	go tmrpc.StartHTTPServer(listener, rs.Mux, logger) //nolint:errcheck
 	return listener, nil
 }
 
@@ -559,7 +559,7 @@ func getKeys(t *testing.T, port string) []keys.KeyOutput {
 
 // POST /keys Create a new account locally
 func doKeysPost(t *testing.T, port, name, password, mnemonic string, account int, index int) keys.KeyOutput {
-	pk := clientkeys.AddNewKey{name, password, mnemonic, account, index}
+	pk := clientkeys.NewAddNewKey(name, password, mnemonic, account, index)
 	req, err := cdc.MarshalJSON(pk)
 	require.NoError(t, err)
 
@@ -585,7 +585,7 @@ func getKeysSeed(t *testing.T, port string) string {
 
 // POST /keys/{name}/recove Recover a account from a seed
 func doRecoverKey(t *testing.T, port, recoverName, recoverPassword, mnemonic string, account uint32, index uint32) {
-	pk := clientkeys.RecoverKey{recoverPassword, mnemonic, int(account), int(index)}
+	pk := clientkeys.NewRecoverKey(recoverPassword, mnemonic, int(account), int(index))
 	req, err := cdc.MarshalJSON(pk)
 	require.NoError(t, err)
 
@@ -613,7 +613,7 @@ func getKey(t *testing.T, port, name string) keys.KeyOutput {
 
 // PUT /keys/{name} Update the password for this account in the KMS
 func updateKey(t *testing.T, port, name, oldPassword, newPassword string, fail bool) {
-	kr := clientkeys.UpdateKeyReq{oldPassword, newPassword}
+	kr := clientkeys.NewUpdateKeyReq(oldPassword, newPassword)
 	req, err := cdc.MarshalJSON(kr)
 	require.NoError(t, err)
 	keyEndpoint := fmt.Sprintf("/keys/%s", name)
@@ -627,7 +627,7 @@ func updateKey(t *testing.T, port, name, oldPassword, newPassword string, fail b
 
 // DELETE /keys/{name} Remove an account
 func deleteKey(t *testing.T, port, name, password string) {
-	dk := clientkeys.DeleteKeyReq{password}
+	dk := clientkeys.NewDeleteKeyReq(password)
 	req, err := cdc.MarshalJSON(dk)
 	require.NoError(t, err)
 	keyEndpoint := fmt.Sprintf("/keys/%s", name)

client/rest/rest.go

@@ -1,6 +1,7 @@
 package rest
 
 import (
+	"log"
 	"net/http"
 
 	"github.com/cosmos/cosmos-sdk/client"
@@ -67,6 +68,8 @@ func WriteGenerateStdTxResponse(w http.ResponseWriter, cdc *codec.Codec,
 	}
 
 	w.Header().Set("Content-Type", "application/json")
-	w.Write(output)
+	if _, err := w.Write(output); err != nil {
+		log.Printf("couldn't write response: %v", err)
+	}
 	return
 }

client/rpc/block.go

@@ -115,20 +115,19 @@ func BlockRequestHandlerFn(cliCtx context.CLIContext) http.HandlerFunc {
 		vars := mux.Vars(r)
 		height, err := strconv.ParseInt(vars["height"], 10, 64)
 		if err != nil {
-			w.WriteHeader(http.StatusBadRequest)
-			w.Write([]byte("ERROR: Couldn't parse block height. Assumed format is '/block/{height}'."))
+			rest.WriteErrorResponse(w, http.StatusBadRequest,
+				"ERROR: Couldn't parse block height. Assumed format is '/block/{height}'.")
 			return
 		}
 		chainHeight, err := GetChainHeight(cliCtx)
 		if height > chainHeight {
-			w.WriteHeader(http.StatusNotFound)
-			w.Write([]byte("ERROR: Requested block height is bigger then the chain length."))
+			rest.WriteErrorResponse(w, http.StatusNotFound,
+				"ERROR: Requested block height is bigger then the chain length.")
 			return
 		}
 		output, err := getBlock(cliCtx, &height)
 		if err != nil {
-			w.WriteHeader(http.StatusInternalServerError)
-			w.Write([]byte(err.Error()))
+			rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
 			return
 		}
 		rest.PostProcessResponse(w, cdc, output, cliCtx.Indent)
@@ -140,14 +139,12 @@ func LatestBlockRequestHandlerFn(cliCtx context.CLIContext) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		height, err := GetChainHeight(cliCtx)
 		if err != nil {
-			w.WriteHeader(http.StatusInternalServerError)
-			w.Write([]byte(err.Error()))
+			rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
 			return
 		}
 		output, err := getBlock(cliCtx, &height)
 		if err != nil {
-			w.WriteHeader(http.StatusInternalServerError)
-			w.Write([]byte(err.Error()))
+			rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
 			return
 		}
 		rest.PostProcessResponse(w, cdc, output, cliCtx.Indent)

client/rpc/root.go

@@ -2,6 +2,7 @@ package rpc
 
 import (
 	"fmt"
+	"log"
 	"net/http"
 
 	"github.com/gorilla/mux"
@@ -26,7 +27,9 @@ func RegisterRoutes(cliCtx context.CLIContext, r *mux.Router) {
 // cli version REST handler endpoint
 func CLIVersionRequestHandler(w http.ResponseWriter, r *http.Request) {
 	w.Header().Set("Content-Type", "application/json")
-	w.Write([]byte(fmt.Sprintf("{\"version\": \"%s\"}", version.Version)))
+	if _, err := w.Write([]byte(fmt.Sprintf("{\"version\": \"%s\"}", version.Version))); err != nil {
+		log.Printf("couldn't write response: %v", err)
+	}
 }
 
 // connected node version REST handler endpoint
@@ -39,6 +42,8 @@ func NodeVersionRequestHandler(cliCtx context.CLIContext) http.HandlerFunc {
 		}
 
 		w.Header().Set("Content-Type", "application/json")
-		w.Write(version)
+		if _, err := w.Write(version); err != nil {
+			log.Printf("couldn't write response: %v", err)
+		}
 	}
 }