tools: Add unparam linter

unparam detects unused parameters in functions, and a parameter to
a function which only ever takes on one value. The latter is an
indication that more tests are required.

There are many nolints in this PR, as I believe that writing tests
to fix alot of these situations is out of scope for this PR / it
will be changed in future commits. There are some nolints for
when we have to comply to normal api's.

CHANGELOG.md

@@ -41,6 +41,7 @@ FEATURES
   * unconvert
   * ineffassign
   * errcheck
+  * unparam
 * [server] Default config now creates a profiler at port 6060, and increase p2p send/recv rates
 * [tests] Add WaitForNextNBlocksTM helper method
 * [types] Switches internal representation of Int/Uint/Rat to use pointers

Makefile

@@ -108,7 +108,7 @@ test_cover:
 	@bash tests/test_cover.sh
 
 test_lint:
-	gometalinter.v2 --disable-all --enable='golint' --enable='misspell' --enable='unconvert' --enable='ineffassign' --linter='vet:go vet -composites=false:PATH:LINE:MESSAGE' --enable='vet' --deadline=500s --vendor ./...
+	gometalinter.v2 --disable-all --enable='golint' --enable='misspell' --enable='unparam' --enable='unconvert' --enable='ineffassign' --linter='vet:go vet -composites=false:PATH:LINE:MESSAGE' --enable='vet' --deadline=500s --vendor ./...
 	!(gometalinter.v2 --disable-all --enable='errcheck' --vendor ./... | grep -v "client/")
 	find . -name '*.go' -type f -not -path "./vendor*" -not -path "*.git*" | xargs gofmt -d -s
 

client/lcd/root.go

@@ -73,14 +73,14 @@ func createHandler(cdc *wire.Codec) http.Handler {
 
 	// TODO make more functional? aka r = keys.RegisterRoutes(r)
 	r.HandleFunc("/version", CLIVersionRequestHandler).Methods("GET")
-	r.HandleFunc("/node_version", NodeVersionRequestHandler(cdc, ctx)).Methods("GET")
+	r.HandleFunc("/node_version", NodeVersionRequestHandler(ctx)).Methods("GET")
 	keys.RegisterRoutes(r)
 	rpc.RegisterRoutes(ctx, r)
 	tx.RegisterRoutes(ctx, r, cdc)
 	auth.RegisterRoutes(ctx, r, cdc, "acc")
 	bank.RegisterRoutes(ctx, r, cdc, kb)
 	ibc.RegisterRoutes(ctx, r, cdc, kb)
 	stake.RegisterRoutes(ctx, r, cdc, kb)
-	gov.RegisterRoutes(ctx, r, cdc, kb)
+	gov.RegisterRoutes(ctx, r, cdc)
 	return r
 }

client/lcd/test_helpers.go

@@ -132,7 +132,7 @@ func InitializeTestLCD(t *testing.T, nValidators int, initAddrs []sdk.Address) (
 	for _, gdValidator := range genDoc.Validators {
 		pk := gdValidator.PubKey
 		validatorsPKs = append(validatorsPKs, pk) // append keys for output
-		appGenTx, _, _, err := gapp.GaiaAppGenTxNF(cdc, pk, pk.Address(), "test_val1", true)
+		appGenTx, _, _, err := gapp.GaiaAppGenTxNF(cdc, pk, pk.Address(), "test_val1")
 		require.NoError(t, err)
 		appGenTxs = append(appGenTxs, appGenTx)
 	}

client/lcd/version.go

@@ -6,7 +6,6 @@ import (
 
 	"github.com/cosmos/cosmos-sdk/client/context"
 	"github.com/cosmos/cosmos-sdk/version"
-	"github.com/cosmos/cosmos-sdk/wire"
 )
 
 // cli version REST handler endpoint
@@ -16,7 +15,7 @@ func CLIVersionRequestHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 // connected node version REST handler endpoint
-func NodeVersionRequestHandler(cdc *wire.Codec, ctx context.CoreContext) http.HandlerFunc {
+func NodeVersionRequestHandler(ctx context.CoreContext) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		version, err := ctx.Query("/app/version")
 		if err != nil {

cmd/gaia/app/app.go

@@ -129,6 +129,7 @@ func (app *GaiaApp) BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock) ab
 }
 
 // application updates every end block
+// nolint: unparam
 func (app *GaiaApp) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) abci.ResponseEndBlock {
 	validatorUpdates := stake.EndBlocker(ctx, app.stakeKeeper)
 

cmd/gaia/app/genesis.go

@@ -103,12 +103,12 @@ func GaiaAppGenTx(cdc *wire.Codec, pk crypto.PubKey, genTxConfig config.GenTx) (
 
 	cliPrint = json.RawMessage(bz)
 
-	appGenTx, _, validator, err = GaiaAppGenTxNF(cdc, pk, addr, genTxConfig.Name, genTxConfig.Overwrite)
+	appGenTx, _, validator, err = GaiaAppGenTxNF(cdc, pk, addr, genTxConfig.Name)
 	return
 }
 
 // Generate a gaia genesis transaction without flags
-func GaiaAppGenTxNF(cdc *wire.Codec, pk crypto.PubKey, addr sdk.Address, name string, overwrite bool) (
+func GaiaAppGenTxNF(cdc *wire.Codec, pk crypto.PubKey, addr sdk.Address, name string) (
 	appGenTx, cliPrint json.RawMessage, validator tmtypes.GenesisValidator, err error) {
 
 	var bz []byte

cmd/gaia/cmd/gaiadebug/hack.go

@@ -210,6 +210,7 @@ func (app *GaiaApp) BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock) ab
 }
 
 // application updates every end block
+// nolint: unparam
 func (app *GaiaApp) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) abci.ResponseEndBlock {
 	validatorUpdates := stake.EndBlocker(ctx, app.stakeKeeper)
 

examples/basecoin/app/app.go

@@ -120,6 +120,7 @@ func (app *BasecoinApp) BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock
 }
 
 // application updates every end block
+// nolint: unparam
 func (app *BasecoinApp) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) abci.ResponseEndBlock {
 	validatorUpdates := stake.EndBlocker(ctx, app.stakeKeeper)
 

examples/democoin/app/app.go

@@ -117,6 +117,7 @@ func MakeCodec() *wire.Codec {
 }
 
 // custom logic for democoin initialization
+// nolint: unparam
 func (app *DemocoinApp) initChainerFn(coolKeeper cool.Keeper, powKeeper pow.Keeper) sdk.InitChainer {
 	return func(ctx sdk.Context, req abci.RequestInitChain) abci.ResponseInitChain {
 		stateJSON := req.AppStateBytes

examples/democoin/x/simplestake/errors.go

@@ -32,6 +32,7 @@ func ErrEmptyStake(codespace sdk.CodespaceType) sdk.Error {
 // -----------------------------
 // Helpers
 
+// nolint: unparam
 func newError(codespace sdk.CodespaceType, code sdk.CodeType, msg string) sdk.Error {
 	return sdk.NewError(codespace, code, msg)
 }

examples/democoin/x/simplestake/handler.go

@@ -7,26 +7,26 @@ import (
 // NewHandler returns a handler for "simplestake" type messages.
 func NewHandler(k Keeper) sdk.Handler {
 	return func(ctx sdk.Context, msg sdk.Msg) sdk.Result {
-		switch msg := msg.(type) {
+		switch msg.(type) {
 		case MsgBond:
-			return handleMsgBond(ctx, k, msg)
+			return handleMsgBond()
 		case MsgUnbond:
-			return handleMsgUnbond(ctx, k, msg)
+			return handleMsgUnbond()
 		default:
 			return sdk.ErrUnknownRequest("No match for message type.").Result()
 		}
 	}
 }
 
-func handleMsgBond(ctx sdk.Context, k Keeper, msg MsgBond) sdk.Result {
+func handleMsgBond() sdk.Result {
 	// Removed ValidatorSet from result because it does not get used.
 	// TODO: Implement correct bond/unbond handling
 	return sdk.Result{
 		Code: sdk.ABCICodeOK,
 	}
 }
 
-func handleMsgUnbond(ctx sdk.Context, k Keeper, msg MsgUnbond) sdk.Result {
+func handleMsgUnbond() sdk.Result {
 	return sdk.Result{
 		Code: sdk.ABCICodeOK,
 	}

server/init.go

@@ -226,7 +226,7 @@ func initWithConfig(cdc *wire.Codec, appInit AppInit, config *cfg.Config, initCo
 	var persistentPeers string
 
 	if initConfig.GenTxs {
-		validators, appGenTxs, persistentPeers, err = processGenTxs(initConfig.GenTxsDir, cdc, appInit)
+		validators, appGenTxs, persistentPeers, err = processGenTxs(initConfig.GenTxsDir, cdc)
 		if err != nil {
 			return
 		}
@@ -263,7 +263,7 @@ func initWithConfig(cdc *wire.Codec, appInit AppInit, config *cfg.Config, initCo
 }
 
 // append a genesis-piece
-func processGenTxs(genTxsDir string, cdc *wire.Codec, appInit AppInit) (
+func processGenTxs(genTxsDir string, cdc *wire.Codec) (
 	validators []tmtypes.GenesisValidator, appGenTxs []json.RawMessage, persistentPeers string, err error) {
 
 	var fos []os.FileInfo

server/mock/store.go

@@ -115,6 +115,6 @@ func (kv kvStore) ReverseSubspaceIterator(prefix []byte) sdk.Iterator {
 	panic("not implemented")
 }
 
-func NewCommitMultiStore(db dbm.DB) sdk.CommitMultiStore {
+func NewCommitMultiStore() sdk.CommitMultiStore {
 	return multiStore{kv: make(map[sdk.StoreKey]kvStore)}
 }

server/mock/store_test.go

@@ -12,7 +12,7 @@ import (
 
 func TestStore(t *testing.T) {
 	db := dbm.NewMemDB()
-	cms := NewCommitMultiStore(db)
+	cms := NewCommitMultiStore()
 
 	key := sdk.NewKVStoreKey("test")
 	cms.MountStoreWithDB(key, sdk.StoreTypeIAVL, db)

store/iavlstore.go

@@ -46,6 +46,8 @@ type iavlStore struct {
 }
 
 // CONTRACT: tree should be fully loaded.
+// TODO: use more numHistory's, so the below nolint can be removed
+// nolint: unparam
 func newIAVLStore(tree *iavl.VersionedTree, numHistory int64) *iavlStore {
 	st := &iavlStore{
 		tree:       tree,

tools/Makefile

@@ -11,6 +11,7 @@ UNCONVERT = github.com/mdempsky/unconvert
 INEFFASSIGN = github.com/gordonklaus/ineffassign
 MISSPELL = github.com/client9/misspell/cmd/misspell
 ERRCHECK = github.com/kisielk/errcheck
+UNPARAM = mvdan.cc/unparam
 
 DEP_CHECK := $(shell command -v dep 2> /dev/null)
 GOLINT_CHECK := $(shell command -v golint 2> /dev/null)
@@ -19,6 +20,7 @@ UNCONVERT_CHECK := $(shell command -v unconvert 2> /dev/null)
 INEFFASSIGN_CHECK := $(shell command -v ineffassign 2> /dev/null)
 MISSPELL_CHECK := $(shell command -v misspell 2> /dev/null)
 ERRCHECK_CHECK := $(shell command -v errcheck 2> /dev/null)
+UNPARAM_CHECK := $(shell command -v unparam 2> /dev/null)
 
 check_tools:
 ifndef DEP_CHECK
@@ -51,11 +53,16 @@ ifndef MISSPELL_CHECK
 else
 	@echo "Found misspell in path."
 endif
-ifndef MISSPELL_CHECK
+ifndef ERRCHECK_CHECK
 	@echo "No errcheck in path.  Install with 'make get_tools'."
 else
 	@echo "Found errcheck in path."
 endif
+ifndef UNPARAM_CHECK
+	@echo "No unparam in path.  Install with 'make get_tools'."
+else
+	@echo "Found unparam in path."
+endif
 
 get_tools:
 ifdef DEP_CHECK
@@ -95,11 +102,17 @@ else
 	go get -v $(MISSPELL)
 endif
 ifdef ERRCHECK_CHECK
-	@echo "misspell is already installed.  Run 'make update_tools' to update."
+	@echo "errcheck is already installed.  Run 'make update_tools' to update."
 else
-	@echo "Installing misspell"
+	@echo "Installing errcheck"
 	go get -v $(ERRCHECK)
 endif
+ifdef UNPARAM_CHECK
+	@echo "unparam is already installed.  Run 'make update_tools' to update."
+else
+	@echo "Installing unparam"
+	go get -v $(UNPARAM)
+endif
 
 update_tools:
 	@echo "Updating dep"
@@ -116,6 +129,8 @@ update_tools:
 	go get -u -v $(MISSPELL)
 	@echo "Updating errcheck"
 	go get -u -v $(ERRCHECK)
+	@echo "Updating unparam"
+	go get -u -v $(UNPARAM)
 
 # To avoid unintended conflicts with file names, always add to .PHONY
 # unless there is a reason not to.

x/auth/mock/simulate_block.go

@@ -72,7 +72,7 @@ func incrementAllSequenceNumbers(initSeqNums []int64) {
 }
 
 // check a transaction result
-func SignCheck(t *testing.T, app *baseapp.BaseApp, msgs []sdk.Msg, accnums []int64, seq []int64, priv ...crypto.PrivKeyEd25519) sdk.Result {
+func SignCheck(app *baseapp.BaseApp, msgs []sdk.Msg, accnums []int64, seq []int64, priv ...crypto.PrivKeyEd25519) sdk.Result {
 	tx := GenTx(msgs, accnums, seq, priv...)
 	res := app.Check(tx)
 	return res

x/gov/client/rest/rest.go

@@ -11,7 +11,6 @@ import (
 	"github.com/cosmos/cosmos-sdk/x/gov"
 	"github.com/gorilla/mux"
 	"github.com/pkg/errors"
-	"github.com/tendermint/go-crypto/keys"
 )
 
 // REST Variable names
@@ -20,19 +19,20 @@ const (
 	RestProposalID = "proposalID"
 	RestDepositer  = "depositer"
 	RestVoter      = "voter"
+	storeName      = "gov"
 )
 
 // RegisterRoutes - Central function to define routes that get registered by the main application
-func RegisterRoutes(ctx context.CoreContext, r *mux.Router, cdc *wire.Codec, kb keys.Keybase) {
-	r.HandleFunc("/gov/proposals", postProposalHandlerFn(cdc, kb, ctx)).Methods("POST")
-	r.HandleFunc(fmt.Sprintf("/gov/proposals/{%s}/deposits", RestProposalID), depositHandlerFn(cdc, kb, ctx)).Methods("POST")
-	r.HandleFunc(fmt.Sprintf("/gov/proposals/{%s}/votes", RestProposalID), voteHandlerFn(cdc, kb, ctx)).Methods("POST")
+func RegisterRoutes(ctx context.CoreContext, r *mux.Router, cdc *wire.Codec) {
+	r.HandleFunc("/gov/proposals", postProposalHandlerFn(cdc, ctx)).Methods("POST")
+	r.HandleFunc(fmt.Sprintf("/gov/proposals/{%s}/deposits", RestProposalID), depositHandlerFn(cdc, ctx)).Methods("POST")
+	r.HandleFunc(fmt.Sprintf("/gov/proposals/{%s}/votes", RestProposalID), voteHandlerFn(cdc, ctx)).Methods("POST")
 
-	r.HandleFunc(fmt.Sprintf("/gov/proposals/{%s}", RestProposalID), queryProposalHandlerFn("gov", cdc, kb, ctx)).Methods("GET")
-	r.HandleFunc(fmt.Sprintf("/gov/proposals/{%s}/deposits/{%s}", RestProposalID, RestDepositer), queryDepositHandlerFn("gov", cdc, kb, ctx)).Methods("GET")
-	r.HandleFunc(fmt.Sprintf("/gov/proposals/{%s}/votes/{%s}", RestProposalID, RestVoter), queryVoteHandlerFn("gov", cdc, kb, ctx)).Methods("GET")
+	r.HandleFunc(fmt.Sprintf("/gov/proposals/{%s}", RestProposalID), queryProposalHandlerFn(cdc)).Methods("GET")
+	r.HandleFunc(fmt.Sprintf("/gov/proposals/{%s}/deposits/{%s}", RestProposalID, RestDepositer), queryDepositHandlerFn(cdc)).Methods("GET")
+	r.HandleFunc(fmt.Sprintf("/gov/proposals/{%s}/votes/{%s}", RestProposalID, RestVoter), queryVoteHandlerFn(cdc)).Methods("GET")
 
-	r.HandleFunc("/gov/proposals", queryProposalsWithParameterFn("gov", cdc, kb, ctx)).Methods("GET")
+	r.HandleFunc("/gov/proposals", queryProposalsWithParameterFn(cdc)).Methods("GET")
 }
 
 type postProposalReq struct {
@@ -56,7 +56,7 @@ type voteReq struct {
 	Option  string  `json:"option"` //  option from OptionSet chosen by the voter
 }
 
-func postProposalHandlerFn(cdc *wire.Codec, kb keys.Keybase, ctx context.CoreContext) http.HandlerFunc {
+func postProposalHandlerFn(cdc *wire.Codec, ctx context.CoreContext) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		var req postProposalReq
 		err := buildReq(w, r, &req)
@@ -93,7 +93,7 @@ func postProposalHandlerFn(cdc *wire.Codec, kb keys.Keybase, ctx context.CoreCon
 	}
 }
 
-func depositHandlerFn(cdc *wire.Codec, kb keys.Keybase, ctx context.CoreContext) http.HandlerFunc {
+func depositHandlerFn(cdc *wire.Codec, ctx context.CoreContext) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		vars := mux.Vars(r)
 		strProposalID := vars[RestProposalID]
@@ -141,7 +141,7 @@ func depositHandlerFn(cdc *wire.Codec, kb keys.Keybase, ctx context.CoreContext)
 	}
 }
 
-func voteHandlerFn(cdc *wire.Codec, kb keys.Keybase, ctx context.CoreContext) http.HandlerFunc {
+func voteHandlerFn(cdc *wire.Codec, ctx context.CoreContext) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		vars := mux.Vars(r)
 		strProposalID := vars[RestProposalID]
@@ -195,7 +195,7 @@ func voteHandlerFn(cdc *wire.Codec, kb keys.Keybase, ctx context.CoreContext) ht
 	}
 }
 
-func queryProposalHandlerFn(storeName string, cdc *wire.Codec, kb keys.Keybase, ctx context.CoreContext) http.HandlerFunc {
+func queryProposalHandlerFn(cdc *wire.Codec) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		vars := mux.Vars(r)
 		strProposalID := vars[RestProposalID]
@@ -236,7 +236,7 @@ func queryProposalHandlerFn(storeName string, cdc *wire.Codec, kb keys.Keybase,
 	}
 }
 
-func queryDepositHandlerFn(storeName string, cdc *wire.Codec, kb keys.Keybase, ctx context.CoreContext) http.HandlerFunc {
+func queryDepositHandlerFn(cdc *wire.Codec) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		vars := mux.Vars(r)
 		strProposalID := vars[RestProposalID]
@@ -302,7 +302,7 @@ func queryDepositHandlerFn(storeName string, cdc *wire.Codec, kb keys.Keybase, c
 	}
 }
 
-func queryVoteHandlerFn(storeName string, cdc *wire.Codec, kb keys.Keybase, ctx context.CoreContext) http.HandlerFunc {
+func queryVoteHandlerFn(cdc *wire.Codec) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		vars := mux.Vars(r)
 		strProposalID := vars[RestProposalID]
@@ -369,7 +369,7 @@ func queryVoteHandlerFn(storeName string, cdc *wire.Codec, kb keys.Keybase, ctx
 	}
 }
 
-func queryProposalsWithParameterFn(storeName string, cdc *wire.Codec, kb keys.Keybase, ctx context.CoreContext) http.HandlerFunc {
+func queryProposalsWithParameterFn(cdc *wire.Codec) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		bechVoterAddr := r.URL.Query().Get(RestVoter)
 		bechDepositerAddr := r.URL.Query().Get(RestDepositer)

x/gov/handler.go

@@ -114,7 +114,7 @@ func EndBlocker(ctx sdk.Context, keeper Keeper) (tags sdk.Tags, nonVotingVals []
 	for shouldPopActiveProposalQueue(ctx, keeper) {
 		activeProposal := keeper.ActiveProposalQueuePop(ctx)
 
-		if ctx.BlockHeight() >= activeProposal.GetVotingStartBlock()+keeper.GetVotingProcedure(ctx).VotingPeriod {
+		if ctx.BlockHeight() >= activeProposal.GetVotingStartBlock()+keeper.GetVotingProcedure().VotingPeriod {
 			passes, nonVotingVals = tally(ctx, keeper, activeProposal)
 			proposalIDBytes := keeper.cdc.MustMarshalBinaryBare(activeProposal.GetProposalID())
 			if passes {
@@ -136,7 +136,7 @@ func EndBlocker(ctx sdk.Context, keeper Keeper) (tags sdk.Tags, nonVotingVals []
 	return tags, nonVotingVals
 }
 func shouldPopInactiveProposalQueue(ctx sdk.Context, keeper Keeper) bool {
-	depositProcedure := keeper.GetDepositProcedure(ctx)
+	depositProcedure := keeper.GetDepositProcedure()
 	peekProposal := keeper.InactiveProposalQueuePeek(ctx)
 
 	if peekProposal == nil {
@@ -150,7 +150,7 @@ func shouldPopInactiveProposalQueue(ctx sdk.Context, keeper Keeper) bool {
 }
 
 func shouldPopActiveProposalQueue(ctx sdk.Context, keeper Keeper) bool {
-	votingProcedure := keeper.GetVotingProcedure(ctx)
+	votingProcedure := keeper.GetVotingProcedure()
 	peekProposal := keeper.ActiveProposalQueuePeek(ctx)
 
 	if peekProposal == nil {