From cdf2b7a7c539c4c8c5bc69fdfb68380cdaec4f56 Mon Sep 17 00:00:00 2001 From: Alessio Treglia Date: Tue, 19 Mar 2019 17:52:43 +0100 Subject: [PATCH] gometalinter -> golangci-lint migration (#3933) {,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. - Fix misleading message in go-mod-cache. - New ci-target to avoid download tools twice. - Run tests with -mod=readonly. Port tools/gometalinter.json to .golangci.yml Update CircleCI config accordingly. Closes: #3896 --- .circleci/config.yml | 44 +++++++++-- .gitignore | 2 +- .golangci.yml | 17 +++++ .../improvements/sdk/Fixed-various-linter | 1 + CONTRIBUTING.md | 2 +- Makefile | 75 ++++++------------- client/keys/types.go | 24 ++++++ client/lcd/certificates.go | 4 +- client/lcd/test_helpers.go | 14 ++-- client/rest/rest.go | 5 +- client/rpc/block.go | 17 ++--- client/rpc/root.go | 9 ++- client/rpc/status.go | 14 ++-- client/rpc/validators.go | 15 ++-- client/tx/broadcast.go | 2 +- client/tx/encode.go | 2 +- client/utils/utils.go | 5 +- contrib/install-golangci-lint.sh | 22 ++++++ go.sum | 1 + scripts/Makefile | 31 +++++--- tools/gometalinter.json | 10 --- x/auth/client/cli/multisign.go | 4 +- x/gov/client/cli/query.go | 14 ++-- x/gov/client/rest/rest.go | 10 ++- 24 files changed, 213 insertions(+), 131 deletions(-) create mode 100644 .golangci.yml create mode 100644 .pending/improvements/sdk/Fixed-various-linter create mode 100644 contrib/install-golangci-lint.sh delete mode 100644 tools/gometalinter.json diff --git a/.circleci/config.yml b/.circleci/config.yml index 303209559250..3fc90c7bf8a5 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -53,6 +53,9 @@ jobs: - run: mkdir -p /tmp/workspace/bin - run: mkdir -p /tmp/workspace/profiles - checkout + - restore_cache: + keys: + - go-mod-v1-{{ checksum "go.sum" }} - run: name: tools command: | @@ -65,6 +68,10 @@ jobs: export PATH="$GOBIN:$PATH" make go-mod-cache make install + - save_cache: + key: go-mod-v1-{{ checksum "go.sum" }} + paths: + - "/go/pkg/mod" - persist_to_workspace: root: /tmp/workspace paths: @@ -79,17 +86,14 @@ jobs: at: /tmp/workspace - checkout - *dependencies - - run: - name: Get metalinter - command: | - export PATH="$GOBIN:$PATH" - make devtools-clean - make devtools + - restore_cache: + keys: + - go-mod-v1-{{ checksum "go.sum" }} - run: name: Lint source command: | export PATH="$GOBIN:$PATH" - make test_lint + make ci-lint integration_tests: <<: *linux_defaults @@ -99,6 +103,9 @@ jobs: at: /tmp/workspace - checkout - *dependencies + - restore_cache: + keys: + - go-mod-v1-{{ checksum "go.sum" }} - run: name: Test cli command: | @@ -113,6 +120,9 @@ jobs: at: /tmp/workspace - checkout - *dependencies + - restore_cache: + keys: + - go-mod-v1-{{ checksum "go.sum" }} - run: name: Test individual module simulations command: | @@ -127,6 +137,9 @@ jobs: at: /tmp/workspace - checkout - *dependencies + - restore_cache: + keys: + - go-mod-v1-{{ checksum "go.sum" }} - run: name: Test full Gaia simulation command: | @@ -141,6 +154,9 @@ jobs: at: /tmp/workspace - checkout - *dependencies + - restore_cache: + keys: + - go-mod-v1-{{ checksum "go.sum" }} - run: name: Test Gaia import/export simulation command: | @@ -155,6 +171,9 @@ jobs: at: /tmp/workspace - checkout - *dependencies + - restore_cache: + keys: + - go-mod-v1-{{ checksum "go.sum" }} - run: name: Test Gaia import/export simulation command: | @@ -169,6 +188,9 @@ jobs: at: /tmp/workspace - checkout - *dependencies + - restore_cache: + keys: + - go-mod-v1-{{ checksum "go.sum" }} - run: name: Test multi-seed Gaia simulation long command: | @@ -184,6 +206,9 @@ jobs: at: /tmp/workspace - checkout - *dependencies + - restore_cache: + keys: + - go-mod-v1-{{ checksum "go.sum" }} - run: name: Test multi-seed Gaia simulation short command: | @@ -200,6 +225,9 @@ jobs: - checkout - *dependencies - run: mkdir -p /tmp/logs + - restore_cache: + keys: + - go-mod-v1-{{ checksum "go.sum" }} - run: name: Run tests command: | @@ -208,7 +236,7 @@ jobs: export GO111MODULE=on for pkg in $(go list ./... | grep -v github.com/cosmos/cosmos-sdk/cmd/gaia/cli_test | grep -v '/simulation' | circleci tests split --split-by=timings); do id=$(echo "$pkg" | sed 's|[/.]|_|g') - go test -timeout 8m -race -coverprofile=/tmp/workspace/profiles/$id.out -covermode=atomic -tags='ledger test_ledger_mock' "$pkg" | tee "/tmp/logs/$id-$RANDOM.log" + go test -mod=readonly -timeout 8m -race -coverprofile=/tmp/workspace/profiles/$id.out -covermode=atomic -tags='ledger test_ledger_mock' "$pkg" | tee "/tmp/logs/$id-$RANDOM.log" done - persist_to_workspace: root: /tmp/workspace diff --git a/.gitignore b/.gitignore index c04df273b7e2..836329e728a4 100644 --- a/.gitignore +++ b/.gitignore @@ -16,7 +16,7 @@ examples/build/* docs/_build docs/tutorial dist -devtools-stamp +tools-stamp # Data - ideally these don't exist baseapp/data/* diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 000000000000..ce8010e7aba6 --- /dev/null +++ b/.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 diff --git a/.pending/improvements/sdk/Fixed-various-linter b/.pending/improvements/sdk/Fixed-various-linter new file mode 100644 index 000000000000..5b6cd8bb208d --- /dev/null +++ b/.pending/improvements/sdk/Fixed-various-linter @@ -0,0 +1 @@ +Fixed various linters warnings in the context of the gometalinter -> golangci-lint migration #3896. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d3ebab274dd9..a6cd69fb45a8 100644 --- a/CONTRIBUTING.md +++ b/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` diff --git a/Makefile b/Makefile index 091c6f82bc69..b0e5b3d11387 100644 --- a/Makefile +++ b/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,7 +59,7 @@ 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 @@ -70,7 +67,7 @@ include scripts/Makefile ######################################## ### CI -ci: devtools install test_cover test_lint test +ci: tools install test_cover lint test ######################################## ### Build/Install @@ -108,37 +105,12 @@ 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 go.sum: tools go.mod - @echo "--> Generating vendor directory via go mod vendor" + @echo "--> Ensure dependencies have not been modified" @go mod verify draw_deps: tools @@ -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/ @@ -166,33 +138,33 @@ godocs: test: test_unit test_cli: build - @go test -p 4 `go list ./cmd/gaia/cli_test/...` -tags=cli_test + @go test -mod=readonly -p 4 `go list ./cmd/gaia/cli_test/...` -tags=cli_test test_ledger: # First test with mock - @go test `go list github.com/cosmos/cosmos-sdk/crypto` -tags='cgo ledger test_ledger_mock' + @go test -mod=readonly `go list github.com/cosmos/cosmos-sdk/crypto` -tags='cgo ledger test_ledger_mock' # Now test with a real device - @go test -v `go list github.com/cosmos/cosmos-sdk/crypto` -tags='cgo ledger' + @go test -mod=readonly -v `go list github.com/cosmos/cosmos-sdk/crypto` -tags='cgo ledger' test_unit: - @VERSION=$(VERSION) go test $(PACKAGES_NOSIMULATION) -tags='ledger test_ledger_mock' + @VERSION=$(VERSION) go test -mod=readonly $(PACKAGES_NOSIMULATION) -tags='ledger test_ledger_mock' test_race: - @VERSION=$(VERSION) go test -race $(PACKAGES_NOSIMULATION) + @VERSION=$(VERSION) go test -mod=readonly -race $(PACKAGES_NOSIMULATION) test_sim_gaia_nondeterminism: @echo "Running nondeterminism test..." - @go test ./cmd/gaia/app -run TestAppStateDeterminism -SimulationEnabled=true -v -timeout 10m + @go test -mod=readonly ./cmd/gaia/app -run TestAppStateDeterminism -SimulationEnabled=true -v -timeout 10m test_sim_gaia_custom_genesis_fast: @echo "Running custom genesis simulation..." @echo "By default, ${HOME}/.gaiad/config/genesis.json will be used." - @go test ./cmd/gaia/app -run TestFullGaiaSimulation -SimulationGenesis=${HOME}/.gaiad/config/genesis.json \ + @go test -mod=readonly ./cmd/gaia/app -run TestFullGaiaSimulation -SimulationGenesis=${HOME}/.gaiad/config/genesis.json \ -SimulationEnabled=true -SimulationNumBlocks=100 -SimulationBlockSize=200 -SimulationCommit=true -SimulationSeed=99 -SimulationPeriod=5 -v -timeout 24h test_sim_gaia_fast: @echo "Running quick Gaia simulation. This may take several minutes..." - @go test ./cmd/gaia/app -run TestFullGaiaSimulation -SimulationEnabled=true -SimulationNumBlocks=100 -SimulationBlockSize=200 -SimulationCommit=true -SimulationSeed=99 -SimulationPeriod=5 -v -timeout 24h + @go test -mod=readonly ./cmd/gaia/app -run TestFullGaiaSimulation -SimulationEnabled=true -SimulationNumBlocks=100 -SimulationBlockSize=200 -SimulationCommit=true -SimulationSeed=99 -SimulationPeriod=5 -v -timeout 24h test_sim_gaia_import_export: @echo "Running Gaia import/export simulation. This may take several minutes..." @@ -216,30 +188,31 @@ SIM_BLOCK_SIZE ?= 200 SIM_COMMIT ?= true test_sim_gaia_benchmark: @echo "Running Gaia benchmark for numBlocks=$(SIM_NUM_BLOCKS), blockSize=$(SIM_BLOCK_SIZE). This may take awhile!" - @go test -benchmem -run=^$$ github.com/cosmos/cosmos-sdk/cmd/gaia/app -bench ^BenchmarkFullGaiaSimulation$$ \ + @go test -mod=readonly -benchmem -run=^$$ github.com/cosmos/cosmos-sdk/cmd/gaia/app -bench ^BenchmarkFullGaiaSimulation$$ \ -SimulationEnabled=true -SimulationNumBlocks=$(SIM_NUM_BLOCKS) -SimulationBlockSize=$(SIM_BLOCK_SIZE) -SimulationCommit=$(SIM_COMMIT) -timeout 24h test_sim_gaia_profile: @echo "Running Gaia benchmark for numBlocks=$(SIM_NUM_BLOCKS), blockSize=$(SIM_BLOCK_SIZE). This may take awhile!" - @go test -benchmem -run=^$$ github.com/cosmos/cosmos-sdk/cmd/gaia/app -bench ^BenchmarkFullGaiaSimulation$$ \ + @go test -mod=readonly -benchmem -run=^$$ github.com/cosmos/cosmos-sdk/cmd/gaia/app -bench ^BenchmarkFullGaiaSimulation$$ \ -SimulationEnabled=true -SimulationNumBlocks=$(SIM_NUM_BLOCKS) -SimulationBlockSize=$(SIM_BLOCK_SIZE) -SimulationCommit=$(SIM_COMMIT) -timeout 24h -cpuprofile cpu.out -memprofile mem.out 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 ci-lint +ci-lint: + 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 benchmark: - @go test -bench=. $(PACKAGES_NOSIMULATION) + @go test -mod=readonly -bench=. $(PACKAGES_NOSIMULATION) ######################################## @@ -292,10 +265,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 diff --git a/client/keys/types.go b/client/keys/types.go index 75ecabe835b3..079ef4962d24 100644 --- a/client/keys/types.go +++ b/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} } diff --git a/client/lcd/certificates.go b/client/lcd/certificates.go index 0ec527e134ac..3260ebfc2dff 100644 --- a/client/lcd/certificates.go +++ b/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 { diff --git a/client/lcd/test_helpers.go b/client/lcd/test_helpers.go index d80c060df49b..3360e0112256 100644 --- a/client/lcd/test_helpers.go +++ b/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) diff --git a/client/rest/rest.go b/client/rest/rest.go index 81d2cc57b933..709ef0d4a6b2 100644 --- a/client/rest/rest.go +++ b/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("could not write response: %v", err) + } return } diff --git a/client/rpc/block.go b/client/rpc/block.go index 127e9c9c8d91..6d6e0a9fd00e 100644 --- a/client/rpc/block.go +++ b/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) diff --git a/client/rpc/root.go b/client/rpc/root.go index 45533e3eaacf..fa82b493fd10 100644 --- a/client/rpc/root.go +++ b/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("could not 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("could not write response: %v", err) + } } } diff --git a/client/rpc/status.go b/client/rpc/status.go index e017fa860c2b..e96889a784e3 100644 --- a/client/rpc/status.go +++ b/client/rpc/status.go @@ -2,6 +2,7 @@ package rpc import ( "fmt" + "log" "net/http" "strconv" @@ -71,8 +72,7 @@ func NodeInfoRequestHandlerFn(cliCtx context.CLIContext) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { status, err := getNodeStatus(cliCtx) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(err.Error())) + rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return } @@ -86,18 +86,18 @@ func NodeSyncingRequestHandlerFn(cliCtx context.CLIContext) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { status, err := getNodeStatus(cliCtx) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(err.Error())) + rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return } syncing := status.SyncInfo.CatchingUp if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(err.Error())) + rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return } - w.Write([]byte(strconv.FormatBool(syncing))) + if _, err := w.Write([]byte(strconv.FormatBool(syncing))); err != nil { + log.Printf("could not write response: %v", err) + } } } diff --git a/client/rpc/validators.go b/client/rpc/validators.go index 90389bf643ed..2e5b0cb73a2e 100644 --- a/client/rpc/validators.go +++ b/client/rpc/validators.go @@ -160,22 +160,19 @@ func ValidatorSetRequestHandlerFn(cliCtx context.CLIContext) http.HandlerFunc { 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 '/validatorsets/{height}'.")) + rest.WriteErrorResponse(w, http.StatusBadRequest, "ERROR: Couldn't parse block height. Assumed format is '/validatorsets/{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 := getValidators(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) @@ -187,15 +184,13 @@ func LatestValidatorSetRequestHandlerFn(cliCtx context.CLIContext) http.HandlerF 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 := getValidators(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) diff --git a/client/tx/broadcast.go b/client/tx/broadcast.go index a0437b1cd1d1..d80475c63840 100644 --- a/client/tx/broadcast.go +++ b/client/tx/broadcast.go @@ -110,7 +110,7 @@ $ gaiacli tx broadcast ./mytxn.json } res, err := cliCtx.BroadcastTx(txBytes) - cliCtx.PrintOutput(res) + cliCtx.PrintOutput(res) // nolint:errcheck return err }, } diff --git a/client/tx/encode.go b/client/tx/encode.go index 75670aade91d..5ea64ea720cc 100644 --- a/client/tx/encode.go +++ b/client/tx/encode.go @@ -97,7 +97,7 @@ If you supply a dash (-) argument in place of an input filename, the command rea txBytesBase64 := base64.StdEncoding.EncodeToString(txBytes) response := txEncodeRespStr(txBytesBase64) - cliCtx.PrintOutput(response) + cliCtx.PrintOutput(response) // nolint:errcheck return nil }, diff --git a/client/utils/utils.go b/client/utils/utils.go index d924d9204254..8bd8671fb58f 100644 --- a/client/utils/utils.go +++ b/client/utils/utils.go @@ -3,10 +3,11 @@ package utils import ( "bytes" "fmt" - "github.com/spf13/viper" "io/ioutil" "os" + "github.com/spf13/viper" + "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/codec" @@ -102,7 +103,7 @@ func CompleteAndBroadcastTxCLI(txBldr authtxb.TxBuilder, cliCtx context.CLIConte // broadcast to a Tendermint node res, err := cliCtx.BroadcastTx(txBytes) - cliCtx.PrintOutput(res) + cliCtx.PrintOutput(res) // nolint:errcheck return err } diff --git a/contrib/install-golangci-lint.sh b/contrib/install-golangci-lint.sh new file mode 100644 index 000000000000..9547f7db7a27 --- /dev/null +++ b/contrib/install-golangci-lint.sh @@ -0,0 +1,22 @@ +#!/bin/bash + +set -euo pipefail + +installer="$(mktemp)" +trap "rm -f ${installer}" EXIT + +GOBIN="${1}" +VERSION="${2}" +HASHSUM="${3}" +CURL="$(which curl)" +GOSUM="$(which gosum)" + +echo "Downloading golangci-lint ${VERSION} installer ..." >&2 +"${CURL}" -sfL "https://raw.githubusercontent.com/golangci/golangci-lint/${VERSION}/install.sh" > "${installer}" + +echo "Checking hashsum ..." >&2 +[ "${HASHSUM}" = "$(${GOSUM} ${installer})" ] +chmod +x "${installer}" + +echo "Launching installer ..." >&2 +exec "${installer}" -d -b "${GOBIN}" "${VERSION}" diff --git a/go.sum b/go.sum index 37765ba9adaf..67f2e4561ffb 100644 --- a/go.sum +++ b/go.sum @@ -155,6 +155,7 @@ golang.org/x/sys v0.0.0-20190222072716-a9d3bda3a223 h1:DH4skfRX4EBpamg7iV4ZlCpbl golang.org/x/sys v0.0.0-20190222072716-a9d3bda3a223/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= +golang.org/x/tools v0.0.0-20190114222345-bf090417da8b h1:qMK98NmNCRVDIYFycQ5yVRkvgDUFfdP8Ip4KqmDEB7g= golang.org/x/tools v0.0.0-20190114222345-bf090417da8b/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM= google.golang.org/genproto v0.0.0-20180817151627-c66870c02cf8 h1:Nw54tB0rB7hY/N0NQvRW8DG4Yk3Q6T9cu9RcFQDu1tc= diff --git a/scripts/Makefile b/scripts/Makefile index 24e12fffef63..d76720d40833 100644 --- a/scripts/Makefile +++ b/scripts/Makefile @@ -17,6 +17,8 @@ endif GOPATH ?= $(shell $(GO) env GOPATH) GITHUBDIR := $(GOPATH)$(FS)src$(FS)github.com +GOLANGCI_LINT_VERSION := v1.15.0 +GOLANGCI_LINT_HASHSUM := ac897cadc180bf0c1a4bf27776c410debad27205b22856b861d41d39d06509cf ### # Functions @@ -33,26 +35,35 @@ cd $(GITHUBDIR)$(FS)$(1)$(FS)$(2) && git fetch origin && git checkout -q $(3) go_install = $(call go_get,$(1),$(2),$(3)) && cd $(GITHUBDIR)$(FS)$(1)$(FS)$(2) && $(GO) install +mkfile_path := $(abspath $(lastword $(MAKEFILE_LIST))) +mkfile_dir := $(shell cd $(shell dirname $(mkfile_path)); pwd) + ### # tools ### all: tools -tools: $(GOPATH)/bin/gometalinter $(GOPATH)/bin/statik $(GOPATH)/bin/goimports $(GOPATH)/bin/gosum -#v2.0.11 -$(GOPATH)/bin/gometalinter: - $(call go_install,alecthomas,gometalinter,v3.0.0) +tools: tools-stamp +tools-stamp: $(GOBIN)/golangci-lint $(GOBIN)/statik $(GOBIN)/goimports $(GOBIN)/gosum $(GOBIN)/sdkch + touch $@ + +$(GOBIN)/golangci-lint: contrib/install-golangci-lint.sh $(GOBIN)/gosum + bash contrib/install-golangci-lint.sh $(GOBIN) $(GOLANGCI_LINT_VERSION) $(GOLANGCI_LINT_HASHSUM) -$(GOPATH)/bin/statik: +$(GOBIN)/statik: $(call go_install,rakyll,statik,v0.1.5) -$(GOPATH)/bin/goimports: +$(GOBIN)/goimports: go get golang.org/x/tools/cmd/goimports@v0.0.0-20190114222345-bf090417da8b -$(GOPATH)/bin/gosum: - go install ./cmd/gosum/ +$(GOBIN)/gosum: + go install -mod=readonly ./cmd/gosum/ + +$(GOBIN)/sdkch: + go install -mod=readonly ./cmd/sdkch/ tools-clean: - cd $(GOPATH)/bin && rm -f gometalinter statik goimports gosum + cd $(GOBIN) && rm -f golangci-lint statik goimports gosum sdkch + rm -f tools-stamp -.PHONY: all tools-clean +.PHONY: all tools tools-clean diff --git a/tools/gometalinter.json b/tools/gometalinter.json deleted file mode 100644 index 0c4b49b3d411..000000000000 --- a/tools/gometalinter.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "Linters": { - "vet": "go tool vet -composites=false :PATH:LINE:MESSAGE" - }, - "Enable": ["golint", "vet", "ineffassign", "misspell"], - "Deadline": "500s", - "Vendor": true, - "Cyclo": 11, - "Exclude": ["/usr/lib/go/src/", "client/lcd/statik/statik.go", "vendor/*"] -} diff --git a/x/auth/client/cli/multisign.go b/x/auth/client/cli/multisign.go index d25983a128b5..ec9613554be5 100644 --- a/x/auth/client/cli/multisign.go +++ b/x/auth/client/cli/multisign.go @@ -105,7 +105,9 @@ func makeMultiSignCmd(cdc *amino.Codec) func(cmd *cobra.Command, args []string) if ok := stdSig.PubKey.VerifyBytes(sigBytes, stdSig.Signature); !ok { return fmt.Errorf("couldn't verify signature") } - multisigSig.AddSignatureFromPubKey(stdSig.Signature, stdSig.PubKey, multisigPub.PubKeys) + if err := multisigSig.AddSignatureFromPubKey(stdSig.Signature, stdSig.PubKey, multisigPub.PubKeys); err != nil { + return err + } } newStdSig := auth.StdSignature{Signature: cdc.MustMarshalBinaryBare(multisigSig), PubKey: multisigPub} diff --git a/x/gov/client/cli/query.go b/x/gov/client/cli/query.go index 0290b6d962e9..9aa52383dc46 100644 --- a/x/gov/client/cli/query.go +++ b/x/gov/client/cli/query.go @@ -43,7 +43,7 @@ $ gaiacli query gov proposal 1 var proposal gov.Proposal cdc.MustUnmarshalJSON(res, &proposal) - return cliCtx.PrintOutput(proposal) + return cliCtx.PrintOutput(proposal) // nolint:errcheck }, } } @@ -118,7 +118,7 @@ $ gaiacli query gov proposals --status (DepositPeriod|VotingPeriod|Passed|Reject return fmt.Errorf("No matching proposals found") } - return cliCtx.PrintOutput(matchingProposals) + return cliCtx.PrintOutput(matchingProposals) // nolint:errcheck }, } @@ -175,17 +175,21 @@ $ gaiacli query gov vote 1 cosmos1skjwj5whet0lpe65qaq4rpq03hjxlwd9nf39lk } var vote gov.Vote - cdc.UnmarshalJSON(res, &vote) + if err := cdc.UnmarshalJSON(res, &vote); err != nil { + return err + } if vote.Empty() { res, err = gcutils.QueryVoteByTxQuery(cdc, cliCtx, params) if err != nil { return err } - cdc.UnmarshalJSON(res, &vote) + if err := cdc.UnmarshalJSON(res, &vote); err != nil { + return err + } } - return cliCtx.PrintOutput(vote) + return cliCtx.PrintOutput(vote) //nolint:errcheck }, } } diff --git a/x/gov/client/rest/rest.go b/x/gov/client/rest/rest.go index 55ba566f19a6..ebd3362aacea 100644 --- a/x/gov/client/rest/rest.go +++ b/x/gov/client/rest/rest.go @@ -346,7 +346,10 @@ func queryDepositHandlerFn(cdc *codec.Codec, cliCtx context.CLIContext) http.Han } var deposit gov.Deposit - cdc.UnmarshalJSON(res, &deposit) + if err := cdc.UnmarshalJSON(res, &deposit); err != nil { + rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error()) + return + } // For an empty deposit, either the proposal does not exist or is inactive in // which case the deposit would be removed from state and should be queried @@ -420,7 +423,10 @@ func queryVoteHandlerFn(cdc *codec.Codec, cliCtx context.CLIContext) http.Handle } var vote gov.Vote - cdc.UnmarshalJSON(res, &vote) + if err := cdc.UnmarshalJSON(res, &vote); err != nil { + rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error()) + return + } // For an empty vote, either the proposal does not exist or is inactive in // which case the vote would be removed from state and should be queried for