Merge PR #1438: Tools: Add errcheck linter

This linter ensures that all errors are checked.
This is disabled in the client directories, since its not needed on
those writes

CHANGELOG.md

@@ -40,6 +40,7 @@ FEATURES
   * go vet -composites=false
   * unconvert
   * ineffassign
+  * errcheck
 * [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

@@ -109,6 +109,7 @@ test_cover:
 
 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='errcheck' --vendor ./... | grep -v "client/")
 	find . -name '*.go' -type f -not -path "./vendor*" -not -path "*.git*" | xargs gofmt -d -s
 
 benchmark:

baseapp/baseapp.go

@@ -164,13 +164,19 @@ func (app *BaseApp) Router() Router { return app.router }
 
 // load latest application version
 func (app *BaseApp) LoadLatestVersion(mainKey sdk.StoreKey) error {
-	app.cms.LoadLatestVersion()
+	err := app.cms.LoadLatestVersion()
+	if err != nil {
+		return err
+	}
 	return app.initFromStore(mainKey)
 }
 
 // load application version
 func (app *BaseApp) LoadVersion(version int64, mainKey sdk.StoreKey) error {
-	app.cms.LoadVersion(version)
+	err := app.cms.LoadVersion(version)
+	if err != nil {
+		return err
+	}
 	return app.initFromStore(mainKey)
 }
 

baseapp/helpers.go

@@ -13,12 +13,20 @@ func RunForever(app abci.Application) {
 	srv, err := server.NewServer("0.0.0.0:26658", "socket", app)
 	if err != nil {
 		cmn.Exit(err.Error())
+		return
+	}
+	err = srv.Start()
+	if err != nil {
+		cmn.Exit(err.Error())
+		return
 	}
-	srv.Start()
 
 	// Wait forever
 	cmn.TrapSignal(func() {
 		// Cleanup
-		srv.Stop()
+		err := srv.Stop()
+		if err != nil {
+			cmn.Exit(err.Error())
+		}
 	})
 }

cmd/gaia/cmd/gaiacli/main.go

@@ -142,5 +142,9 @@ func main() {
 
 	// prepare and add flags
 	executor := cli.PrepareMainCmd(rootCmd, "GA", app.DefaultCLIHome)
-	executor.Execute()
+	err := executor.Execute()
+	if err != nil {
+		// handle with #870
+		panic(err)
+	}
 }

cmd/gaia/cmd/gaiad/main.go

@@ -31,7 +31,11 @@ func main() {
 
 	// prepare and add flags
 	executor := cli.PrepareBaseCmd(rootCmd, "GA", app.DefaultNodeHome)
-	executor.Execute()
+	err := executor.Execute()
+	if err != nil {
+		// handle with #870
+		panic(err)
+	}
 }
 
 func newApp(logger log.Logger, db dbm.DB) abci.Application {

examples/basecoin/cmd/basecli/main.go

@@ -80,5 +80,9 @@ func main() {
 
 	// prepare and add flags
 	executor := cli.PrepareMainCmd(rootCmd, "BC", os.ExpandEnv("$HOME/.basecli"))
-	executor.Execute()
+	err := executor.Execute()
+	if err != nil {
+		// handle with #870
+		panic(err)
+	}
 }

examples/basecoin/cmd/basecoind/main.go

@@ -33,7 +33,11 @@ func main() {
 	// prepare and add flags
 	rootDir := os.ExpandEnv("$HOME/.basecoind")
 	executor := cli.PrepareBaseCmd(rootCmd, "BC", rootDir)
-	executor.Execute()
+	err := executor.Execute()
+	if err != nil {
+		// handle with #870
+		panic(err)
+	}
 }
 
 func newApp(logger log.Logger, db dbm.DB) abci.Application {

examples/democoin/cmd/democli/main.go

@@ -92,5 +92,9 @@ func main() {
 
 	// prepare and add flags
 	executor := cli.PrepareMainCmd(rootCmd, "BC", os.ExpandEnv("$HOME/.democli"))
-	executor.Execute()
+	err := executor.Execute()
+	if err != nil {
+		// handle with #870
+		panic(err)
+	}
 }

examples/democoin/cmd/democoind/main.go

@@ -72,5 +72,9 @@ func main() {
 	// prepare and add flags
 	rootDir := os.ExpandEnv("$HOME/.democoind")
 	executor := cli.PrepareBaseCmd(rootCmd, "BC", rootDir)
-	executor.Execute()
+	err := executor.Execute()
+	if err != nil {
+		// handle with #870
+		panic(err)
+	}
 }

examples/kvstore/main.go

@@ -55,12 +55,18 @@ func main() {
 		fmt.Println(err)
 		os.Exit(1)
 	}
-	srv.Start()
+	err = srv.Start()
+	if err != nil {
+		cmn.Exit(err.Error())
+	}
 
 	// Wait forever
 	cmn.TrapSignal(func() {
 		// Cleanup
-		srv.Stop()
+		err = srv.Stop()
+		if err != nil {
+			cmn.Exit(err.Error())
+		}
 	})
 	return
 }

server/init.go

@@ -88,7 +88,7 @@ func GenTxCmd(ctx *Context, cdc *wire.Codec, appInit AppInit) *cobra.Command {
 				viper.GetBool(FlagOWK),
 				ip,
 			}
-			cliPrint, genTxFile, err := gentxWithConfig(ctx, cdc, appInit, config, genTxConfig)
+			cliPrint, genTxFile, err := gentxWithConfig(cdc, appInit, config, genTxConfig)
 			if err != nil {
 				return err
 			}
@@ -112,7 +112,7 @@ func GenTxCmd(ctx *Context, cdc *wire.Codec, appInit AppInit) *cobra.Command {
 	return cmd
 }
 
-func gentxWithConfig(ctx *Context, cdc *wire.Codec, appInit AppInit, config *cfg.Config, genTxConfig serverconfig.GenTx) (
+func gentxWithConfig(cdc *wire.Codec, appInit AppInit, config *cfg.Config, genTxConfig serverconfig.GenTx) (
 	cliPrint json.RawMessage, genTxFile json.RawMessage, err error) {
 	nodeKey, err := p2p.LoadOrGenNodeKey(config.NodeKeyFile())
 	if err != nil {
@@ -169,7 +169,7 @@ func InitCmd(ctx *Context, cdc *wire.Codec, appInit AppInit) *cobra.Command {
 				viper.GetBool(FlagOverwrite),
 			}
 
-			chainID, nodeID, appMessage, err := initWithConfig(ctx, cdc, appInit, config, initConfig)
+			chainID, nodeID, appMessage, err := initWithConfig(cdc, appInit, config, initConfig)
 			if err != nil {
 				return err
 			}
@@ -200,7 +200,7 @@ func InitCmd(ctx *Context, cdc *wire.Codec, appInit AppInit) *cobra.Command {
 	return cmd
 }
 
-func initWithConfig(ctx *Context, cdc *wire.Codec, appInit AppInit, config *cfg.Config, initConfig InitConfig) (
+func initWithConfig(cdc *wire.Codec, appInit AppInit, config *cfg.Config, initConfig InitConfig) (
 	chainID string, nodeID string, appMessage json.RawMessage, err error) {
 	nodeKey, err := p2p.LoadOrGenNodeKey(config.NodeKeyFile())
 	if err != nil {

server/mock/helpers.go

@@ -1,6 +1,7 @@
 package mock
 
 import (
+	"fmt"
 	"io/ioutil"
 	"os"
 
@@ -19,7 +20,10 @@ func SetupApp() (abci.Application, func(), error) {
 	}
 
 	cleanup := func() {
-		os.RemoveAll(rootDir)
+		err := os.RemoveAll(rootDir)
+		if err != nil {
+			fmt.Printf("could not delete %s, had error %s\n", rootDir, err.Error())
+		}
 	}
 
 	app, err := NewApp(rootDir, logger)

server/start.go

@@ -58,12 +58,18 @@ func startStandAlone(ctx *Context, appCreator AppCreator) error {
 		return errors.Errorf("error creating listener: %v\n", err)
 	}
 	svr.SetLogger(ctx.Logger.With("module", "abci-server"))
-	svr.Start()
+	err = svr.Start()
+	if err != nil {
+		cmn.Exit(err.Error())
+	}
 
 	// Wait forever
 	cmn.TrapSignal(func() {
 		// Cleanup
-		svr.Stop()
+		err = svr.Stop()
+		if err != nil {
+			cmn.Exit(err.Error())
+		}
 	})
 	return nil
 }

server/test_helpers.go

@@ -21,7 +21,16 @@ func FreeTCPAddr() (addr, port string, err error) {
 	if err != nil {
 		return "", "", err
 	}
-	defer l.Close()
+
+	closer := func() {
+		err := l.Close()
+		if err != nil {
+			// TODO: Handle with #870
+			panic(err)
+		}
+	}
+
+	defer closer()
 
 	portI := l.Addr().(*net.TCPAddr).Port
 	port = fmt.Sprintf("%d", portI)
@@ -36,7 +45,11 @@ func setupViper(t *testing.T) func() {
 	require.Nil(t, err)
 	viper.Set(cli.HomeFlag, rootDir)
 	return func() {
-		os.RemoveAll(rootDir)
+		err := os.RemoveAll(rootDir)
+		if err != nil {
+			// TODO: Handle with #870
+			panic(err)
+		}
 	}
 }
 

server/testnet.go

@@ -9,11 +9,12 @@ import (
 
 	gc "github.com/cosmos/cosmos-sdk/server/config"
 
+	"os"
+
 	"github.com/cosmos/cosmos-sdk/wire"
 	"github.com/spf13/viper"
 	cfg "github.com/tendermint/tendermint/config"
 	cmn "github.com/tendermint/tmlibs/common"
-	"os"
 )
 
 var (
@@ -42,7 +43,7 @@ Example:
 	`,
 		RunE: func(_ *cobra.Command, _ []string) error {
 			config := ctx.Config
-			err := testnetWithConfig(config, ctx, cdc, appInit)
+			err := testnetWithConfig(config, cdc, appInit)
 			return err
 		},
 	}
@@ -58,7 +59,7 @@ Example:
 	return cmd
 }
 
-func testnetWithConfig(config *cfg.Config, ctx *Context, cdc *wire.Codec, appInit AppInit) error {
+func testnetWithConfig(config *cfg.Config, cdc *wire.Codec, appInit AppInit) error {
 
 	outDir := viper.GetString(outputDir)
 	// Generate private key, node ID, initial transaction
@@ -104,7 +105,7 @@ func testnetWithConfig(config *cfg.Config, ctx *Context, cdc *wire.Codec, appIni
 		}
 
 		// Run `init gen-tx` and generate initial transactions
-		cliPrint, genTxFile, err := gentxWithConfig(ctx, cdc, appInit, config, genTxConfig)
+		cliPrint, genTxFile, err := gentxWithConfig(cdc, appInit, config, genTxConfig)
 		if err != nil {
 			return err
 		}
@@ -154,7 +155,7 @@ func testnetWithConfig(config *cfg.Config, ctx *Context, cdc *wire.Codec, appIni
 		config.SetRoot(nodeDir)
 
 		// Run `init` and generate genesis.json and config.toml
-		_, _, _, err := initWithConfig(ctx, cdc, appInit, config, initConfig)
+		_, _, _, err := initWithConfig(cdc, appInit, config, initConfig)
 		if err != nil {
 			return err
 		}

server/util.go

@@ -69,7 +69,11 @@ func PersistentPreRunEFn(context *Context) func(*cobra.Command, []string) error
 // If a new config is created, change some of the default tendermint settings
 func interceptLoadConfig() (conf *cfg.Config, err error) {
 	tmpConf := cfg.DefaultConfig()
-	viper.Unmarshal(tmpConf)
+	err = viper.Unmarshal(tmpConf)
+	if err != nil {
+		// TODO: Handle with #870
+		panic(err)
+	}
 	rootDir := tmpConf.RootDir
 	configFilePath := filepath.Join(rootDir, "config/config.toml")
 	// Intercept only if the file doesn't already exist

store/iavlstore.go

@@ -67,7 +67,11 @@ func (st *iavlStore) Commit() CommitID {
 	// Release an old version of history
 	if st.numHistory > 0 && (st.numHistory < st.tree.Version64()) {
 		toRelease := version - st.numHistory
-		st.tree.DeleteVersion(toRelease)
+		err := st.tree.DeleteVersion(toRelease)
+		if err != nil {
+			// TODO: Handle with #870
+			panic(err)
+		}
 	}
 
 	return CommitID{

store/rootmultistore.go

@@ -343,7 +343,11 @@ func (si storeInfo) Hash() []byte {
 	// include them via the keys.
 	bz, _ := cdc.MarshalBinary(si.Core) // Does not error
 	hasher := ripemd160.New()
-	hasher.Write(bz)
+	_, err := hasher.Write(bz)
+	if err != nil {
+		// TODO: Handle with #870
+		panic(err)
+	}
 	return hasher.Sum(nil)
 }
 

tests/util.go

@@ -101,7 +101,10 @@ func waitForHeight(height int64, url string) {
 		if err != nil {
 			panic(err)
 		}
-		res.Body.Close()
+		err = res.Body.Close()
+		if err != nil {
+			panic(err)
+		}
 
 		var resultBlock ctypes.ResultBlock
 		err = cdc.UnmarshalJSON(body, &resultBlock)
@@ -136,7 +139,10 @@ func WaitForStart(port string) {
 
 		// waiting for server to start ...
 		if res.StatusCode != http.StatusOK {
-			res.Body.Close()
+			err = res.Body.Close()
+			if err != nil {
+				panic(err)
+			}
 			return
 		}
 	}

tools/Makefile

@@ -10,6 +10,7 @@ GOMETALINTER = gopkg.in/alecthomas/gometalinter.v2
 UNCONVERT = github.com/mdempsky/unconvert
 INEFFASSIGN = github.com/gordonklaus/ineffassign
 MISSPELL = github.com/client9/misspell/cmd/misspell
+ERRCHECK = github.com/kisielk/errcheck
 
 DEP_CHECK := $(shell command -v dep 2> /dev/null)
 GOLINT_CHECK := $(shell command -v golint 2> /dev/null)
@@ -17,6 +18,7 @@ GOMETALINTER_CHECK := $(shell command -v gometalinter.v2 2> /dev/null)
 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)
 
 check_tools:
 ifndef DEP_CHECK
@@ -49,6 +51,11 @@ ifndef MISSPELL_CHECK
 else
 	@echo "Found misspell in path."
 endif
+ifndef MISSPELL_CHECK
+	@echo "No errcheck in path.  Install with 'make get_tools'."
+else
+	@echo "Found errcheck in path."
+endif
 
 get_tools:
 ifdef DEP_CHECK
@@ -87,6 +94,12 @@ else
 	@echo "Installing misspell"
 	go get -v $(MISSPELL)
 endif
+ifdef ERRCHECK_CHECK
+	@echo "misspell is already installed.  Run 'make update_tools' to update."
+else
+	@echo "Installing misspell"
+	go get -v $(ERRCHECK)
+endif
 
 update_tools:
 	@echo "Updating dep"
@@ -101,6 +114,8 @@ update_tools:
 	go get -u -v $(INEFFASSIGN)
 	@echo "Updating misspell"
 	go get -u -v $(MISSPELL)
+	@echo "Updating errcheck"
+	go get -u -v $(ERRCHECK)
 
 # To avoid unintended conflicts with file names, always add to .PHONY
 # unless there is a reason not to.

types/lib/linear.go

@@ -233,7 +233,11 @@ func (m Linear) Flush(ptr interface{}, fn func() bool) {
 
 	var i uint64
 	for i = top; i < length; i++ {
-		m.Get(i, ptr)
+		err := m.Get(i, ptr)
+		if err != nil {
+			// TODO: Handle with #870
+			panic(err)
+		}
 		m.Delete(i)
 		if fn() {
 			break

x/auth/ante.go

@@ -140,8 +140,11 @@ func processSig(
 		return nil, sdk.ErrInvalidSequence(
 			fmt.Sprintf("Invalid sequence. Got %d, expected %d", sig.Sequence, seq)).Result()
 	}
-	acc.SetSequence(seq + 1)
-
+	err := acc.SetSequence(seq + 1)
+	if err != nil {
+		// Handle w/ #870
+		panic(err)
+	}
 	// If pubkey is not known for account,
 	// set it from the StdSignature.
 	pubKey := acc.GetPubKey()
@@ -154,7 +157,7 @@ func processSig(
 			return nil, sdk.ErrInvalidPubKey(
 				fmt.Sprintf("PubKey does not match Signer address %v", addr)).Result()
 		}
-		err := acc.SetPubKey(pubKey)
+		err = acc.SetPubKey(pubKey)
 		if err != nil {
 			return nil, sdk.ErrInternal("setting PubKey on signer's account").Result()
 		}
@@ -181,7 +184,11 @@ func deductFees(acc Account, fee StdFee) (Account, sdk.Result) {
 		errMsg := fmt.Sprintf("%s < %s", coins, feeAmount)
 		return nil, sdk.ErrInsufficientFunds(errMsg).Result()
 	}
-	acc.SetCoins(newCoins)
+	err := acc.SetCoins(newCoins)
+	if err != nil {
+		// Handle w/ #870
+		panic(err)
+	}
 	return acc, sdk.Result{}
 }
 

x/auth/mapper.go

@@ -39,14 +39,26 @@ func NewAccountMapper(cdc *wire.Codec, key sdk.StoreKey, proto Account) AccountM
 // Implaements sdk.AccountMapper.
 func (am AccountMapper) NewAccountWithAddress(ctx sdk.Context, addr sdk.Address) Account {
 	acc := am.clonePrototype()
-	acc.SetAddress(addr)
-	acc.SetAccountNumber(am.GetNextAccountNumber(ctx))
+	err := acc.SetAddress(addr)
+	if err != nil {
+		// Handle w/ #870
+		panic(err)
+	}
+	err = acc.SetAccountNumber(am.GetNextAccountNumber(ctx))
+	if err != nil {
+		// Handle w/ #870
+		panic(err)
+	}
 	return acc
 }
 
 // New Account
 func (am AccountMapper) NewAccount(ctx sdk.Context, acc Account) Account {
-	acc.SetAccountNumber(am.GetNextAccountNumber(ctx))
+	err := acc.SetAccountNumber(am.GetNextAccountNumber(ctx))
+	if err != nil {
+		// TODO: Handle with #870
+		panic(err)
+	}
 	return acc
 }
 
@@ -114,7 +126,11 @@ func (am AccountMapper) setSequence(ctx sdk.Context, addr sdk.Address, newSequen
 	if acc == nil {
 		return sdk.ErrUnknownAddress(addr.String())
 	}
-	acc.SetSequence(newSequence)
+	err := acc.SetSequence(newSequence)
+	if err != nil {
+		// Handle w/ #870
+		panic(err)
+	}
 	am.SetAccount(ctx, acc)
 	return nil
 }

x/auth/mock/app.go

@@ -77,7 +77,11 @@ func (app *App) InitChainer(ctx sdk.Context, _ abci.RequestInitChain) abci.Respo
 	// load the accounts
 	for _, genacc := range app.GenesisAccounts {
 		acc := app.AccountMapper.NewAccountWithAddress(ctx, genacc.GetAddress())
-		acc.SetCoins(genacc.GetCoins())
+		err := acc.SetCoins(genacc.GetCoins())
+		if err != nil {
+			// TODO: Handle with #870
+			panic(err)
+		}
 		app.AccountMapper.SetAccount(ctx, acc)
 	}
 

x/bank/keeper.go

@@ -131,7 +131,11 @@ func setCoins(ctx sdk.Context, am auth.AccountMapper, addr sdk.Address, amt sdk.
 	if acc == nil {
 		acc = am.NewAccountWithAddress(ctx, addr)
 	}
-	acc.SetCoins(amt)
+	err := acc.SetCoins(amt)
+	if err != nil {
+		// Handle w/ #870
+		panic(err)
+	}
 	am.SetAccount(ctx, acc)
 	return nil
 }

x/gov/genesis.go

@@ -24,7 +24,11 @@ func DefaultGenesisState() GenesisState {
 
 // InitGenesis - store genesis parameters
 func InitGenesis(ctx sdk.Context, k Keeper, data GenesisState) {
-	k.setInitialProposalID(ctx, data.StartingProposalID)
+	err := k.setInitialProposalID(ctx, data.StartingProposalID)
+	if err != nil {
+		// TODO: Handle this with #870
+		panic(err)
+	}
 }
 
 // WriteGenesis - output genesis parameters

x/slashing/test_common.go

@@ -66,10 +66,11 @@ func createTestInput(t *testing.T) (sdk.Context, bank.Keeper, stake.Keeper, Keep
 	genesis.Pool.LooseTokens = initCoins.MulRaw(int64(len(addrs))).Int64()
 	stake.InitGenesis(ctx, sk, genesis)
 	for _, addr := range addrs {
-		ck.AddCoins(ctx, addr, sdk.Coins{
+		_, _, err = ck.AddCoins(ctx, addr, sdk.Coins{
 			{sk.GetParams(ctx).BondDenom, initCoins},
 		})
 	}
+	require.Nil(t, err)
 	keeper := NewKeeper(cdc, keySlashing, sk, DefaultCodespace)
 	return ctx, ck, sk, keeper
 }

x/stake/keeper/delegation.go

@@ -295,7 +295,10 @@ func (k Keeper) CompleteUnbonding(ctx sdk.Context, delegatorAddr, validatorAddr
 		return types.ErrNotMature(k.Codespace(), "unbonding", "unit-time", ubd.MinTime, ctxTime)
 	}
 
-	k.coinKeeper.AddCoins(ctx, ubd.DelegatorAddr, sdk.Coins{ubd.Balance})
+	_, _, err := k.coinKeeper.AddCoins(ctx, ubd.DelegatorAddr, sdk.Coins{ubd.Balance})
+	if err != nil {
+		return err
+	}
 	k.RemoveUnbondingDelegation(ctx, ubd)
 	return nil
 }

x/stake/keeper/test_common.go

@@ -114,9 +114,10 @@ func CreateTestInput(t *testing.T, isCheckTx bool, initCoins int64) (sdk.Context
 	// fill all the addresses with some coins, set the loose pool tokens simultaneously
 	for _, addr := range Addrs {
 		pool := keeper.GetPool(ctx)
-		ck.AddCoins(ctx, addr, sdk.Coins{
+		_, _, err := ck.AddCoins(ctx, addr, sdk.Coins{
 			{keeper.GetParams(ctx).BondDenom, sdk.NewInt(initCoins)},
 		})
+		require.Nil(t, err)
 		pool.LooseTokens += initCoins
 		keeper.SetPool(ctx, pool)
 	}