Merge PR #188: Add linting to CI

* Add some linting to gaia

- closes #187

Signed-off-by: Marko Baricevic <[email protected]>

* fail tests

* tests pass

* address PR comments

* some more strictness

Author: tac0turtle

.golangci.yml

@@ -1,17 +1,61 @@
 linters:
-  disable-all: true
   enable:
+    - bodyclose
+    - deadcode
+    - depguard
+    - dogsled
     - errcheck
+    - goconst
+    - gocritic
+    - gofmt
+    - goimports
     - golint
+    - gosec
+    - gosimple
+    - govet
     - ineffassign
-    - unconvert
+    - interfacer
     - misspell
+    - maligned
+    - nakedret
+    - prealloc
+    - scopelint
+    - staticcheck
+    - structcheck
+    - stylecheck
+    - typecheck
+    - unconvert
+    - unused
+    - varcheck
+
+issues:
+  exclude-rules:
+    - text: "Use of weak random number generator"
+      linters:
+        - gosec
+    - text: "ST1003:"
+      linters:
+        - stylecheck
+
 linters-settings:
-  gocyclo:
-    min-complexity: 11
+  dogsled:
+    max-blank-identifiers: 3
+  maligned:
+    # print struct with more effective memory layout or not, false by default
+    suggest-new: true
   errcheck:
-    ignore: fmt:.*,io/ioutil:^Read.*,github.com/spf13/cobra:MarkFlagRequired,github.com/spf13/viper:BindPFlag
+    # report about assignment of errors to blank identifier: `num, _ := strconv.Atoi(numStr)`;
+    # default is false: such cases aren't reported by default.
+    check-blank: true
   golint:
-    min-confidence: 1.1
-run:
-  tests: false
+    # minimal confidence for issues, default is 0.8
+    min-confidence: 0
+  prealloc:
+    # XXX: we don't recommend using this linter before doing performance profiling.
+    # For most programs usage of prealloc will be a premature optimization.
+
+    # Report preallocation suggestions only on simple loops that have no returns/breaks/continues/gotos in them.
+    # True by default.
+    simple: false
+    range-loops: true # Report preallocation suggestions on range loops, true by default
+    for-loops: true # Report preallocation suggestions on for loops, false by default

app/app.go

@@ -34,13 +34,13 @@ import (
 const appName = "GaiaApp"
 
 var (
-	// default home directories for gaiacli
+	// DefaultCLIHome default home directories for gaiacli
 	DefaultCLIHome = os.ExpandEnv("$HOME/.gaiacli")
 
-	// default home directories for gaiad
+	// DefaultNodeHome default home directories for gaiad
 	DefaultNodeHome = os.ExpandEnv("$HOME/.gaiad")
 
-	// The module BasicManager is in charge of setting up basic,
+	// ModuleBasics The module BasicManager is in charge of setting up basic,
 	// non-dependant module elements, such as codec registration
 	// and genesis verification.
 	ModuleBasics = module.NewBasicManager(

app/app_test.go

@@ -17,11 +17,12 @@ import (
 func TestGaiadExport(t *testing.T) {
 	db := db.NewMemDB()
 	gapp := NewGaiaApp(log.NewTMLogger(log.NewSyncWriter(os.Stdout)), db, nil, true, 0)
-	setGenesis(gapp)
+	err := setGenesis(gapp)
+	require.NoError(t, err)
 
 	// Making a new app object with the db, so that initchain hasn't been called
 	newGapp := NewGaiaApp(log.NewTMLogger(log.NewSyncWriter(os.Stdout)), db, nil, true, 0)
-	_, _, err := newGapp.ExportAppStateAndValidators(false, []string{})
+	_, _, err = newGapp.ExportAppStateAndValidators(false, []string{})
 	require.NoError(t, err, "ExportAppStateAndValidators should not have an error")
 }
 

app/benchmarks/txsize_test.go

@@ -31,7 +31,10 @@ func ExampleTxSendSize() {
 	fee := auth.NewStdFee(gas, coins)
 	signBytes := auth.StdSignBytes("example-chain-ID",
 		1, 1, fee, []sdk.Msg{msg1}, "")
-	sig, _ := priv1.Sign(signBytes)
+	sig, err := priv1.Sign(signBytes)
+	if err != nil {
+		return
+	}
 	sigs := []auth.StdSignature{{nil, sig}}
 	tx := auth.NewStdTx([]sdk.Msg{msg1}, fee, sigs, "")
 	fmt.Println(len(cdc.MustMarshalBinaryBare([]sdk.Msg{msg1})))

app/export.go

@@ -60,14 +60,20 @@ func (app *GaiaApp) prepForZeroHeightGenesis(ctx sdk.Context, jailWhiteList []st
 
 	// withdraw all validator commission
 	app.stakingKeeper.IterateValidators(ctx, func(_ int64, val staking.ValidatorI) (stop bool) {
-		_, _ = app.distrKeeper.WithdrawValidatorCommission(ctx, val.GetOperator())
+		_, err := app.distrKeeper.WithdrawValidatorCommission(ctx, val.GetOperator())
+		if err != nil {
+			log.Fatal(err)
+		}
 		return false
 	})
 
 	// withdraw all delegator rewards
 	dels := app.stakingKeeper.GetAllDelegations(ctx)
 	for _, delegation := range dels {
-		_, _ = app.distrKeeper.WithdrawDelegationRewards(ctx, delegation.DelegatorAddress, delegation.ValidatorAddress)
+		_, err := app.distrKeeper.WithdrawDelegationRewards(ctx, delegation.DelegatorAddress, delegation.ValidatorAddress)
+		if err != nil {
+			log.Fatal(err)
+		}
 	}
 
 	// clear validator slash events
@@ -128,7 +134,6 @@ func (app *GaiaApp) prepForZeroHeightGenesis(ctx sdk.Context, jailWhiteList []st
 	iter := sdk.KVStoreReversePrefixIterator(store, staking.ValidatorsKey)
 	counter := int16(0)
 
-	var valConsAddrs []sdk.ConsAddress
 	for ; iter.Valid(); iter.Next() {
 		addr := sdk.ValAddress(iter.Key()[1:])
 		validator, found := app.stakingKeeper.GetValidator(ctx, addr)
@@ -137,7 +142,6 @@ func (app *GaiaApp) prepForZeroHeightGenesis(ctx sdk.Context, jailWhiteList []st
 		}
 
 		validator.UnbondingHeight = 0
-		valConsAddrs = append(valConsAddrs, validator.ConsAddress())
 		if applyWhiteList && !whiteListMap[addr.String()] {
 			validator.Jailed = true
 		}

app/sim_test.go

@@ -252,8 +252,16 @@ func BenchmarkFullAppSimulation(b *testing.B) {
 	config := simapp.NewConfigFromFlags()
 
 	var db dbm.DB
-	dir, _ := ioutil.TempDir("", "goleveldb-app-sim")
-	db, _ = sdk.NewLevelDB("Simulation", dir)
+	dir, err := ioutil.TempDir("", "goleveldb-app-sim")
+	if err != nil {
+		fmt.Println(err)
+		b.Fail()
+	}
+	db, err = sdk.NewLevelDB("Simulation", dir)
+	if err != nil {
+		fmt.Println(err)
+		b.Fail()
+	}
 	defer func() {
 		db.Close()
 		_ = os.RemoveAll(dir)
@@ -310,8 +318,10 @@ func TestFullAppSimulation(t *testing.T) {
 	}
 
 	var db dbm.DB
-	dir, _ := ioutil.TempDir("", "goleveldb-app-sim")
-	db, _ = sdk.NewLevelDB("Simulation", dir)
+	dir, err := ioutil.TempDir("", "goleveldb-app-sim")
+	require.NoError(t, err)
+	db, err = sdk.NewLevelDB("Simulation", dir)
+	require.NoError(t, err)
 
 	defer func() {
 		db.Close()
@@ -364,8 +374,10 @@ func TestAppImportExport(t *testing.T) {
 	}
 
 	var db dbm.DB
-	dir, _ := ioutil.TempDir("", "goleveldb-app-sim")
-	db, _ = sdk.NewLevelDB("Simulation", dir)
+	dir, err := ioutil.TempDir("", "goleveldb-app-sim")
+	require.NoError(t, err)
+	db, err = sdk.NewLevelDB("Simulation", dir)
+	require.NoError(t, err)
 
 	defer func() {
 		db.Close()
@@ -408,8 +420,10 @@ func TestAppImportExport(t *testing.T) {
 	require.NoError(t, err)
 	fmt.Printf("importing genesis...\n")
 
-	newDir, _ := ioutil.TempDir("", "goleveldb-app-sim-2")
-	newDB, _ := sdk.NewLevelDB("Simulation-2", dir)
+	newDir, err := ioutil.TempDir("", "goleveldb-app-sim-2")
+	require.NoError(t, err)
+	newDB, err := sdk.NewLevelDB("Simulation-2", dir)
+	require.NoError(t, err)
 
 	defer func() {
 		newDB.Close()
@@ -480,8 +494,10 @@ func TestAppSimulationAfterImport(t *testing.T) {
 		logger = log.NewNopLogger()
 	}
 
-	dir, _ := ioutil.TempDir("", "goleveldb-app-sim")
-	db, _ := sdk.NewLevelDB("Simulation", dir)
+	dir, err := ioutil.TempDir("", "goleveldb-app-sim")
+	require.NoError(t, err)
+	db, err := sdk.NewLevelDB("Simulation", dir)
+	require.NoError(t, err)
 
 	defer func() {
 		db.Close()
@@ -534,8 +550,10 @@ func TestAppSimulationAfterImport(t *testing.T) {
 
 	fmt.Printf("Importing genesis...\n")
 
-	newDir, _ := ioutil.TempDir("", "goleveldb-app-sim-2")
-	newDB, _ := sdk.NewLevelDB("Simulation-2", dir)
+	newDir, err := ioutil.TempDir("", "goleveldb-app-sim-2")
+	require.NoError(t, err)
+	newDB, err := sdk.NewLevelDB("Simulation-2", dir)
+	require.NoError(t, err)
 
 	defer func() {
 		newDB.Close()
@@ -613,8 +631,16 @@ func BenchmarkInvariants(b *testing.B) {
 	config := simapp.NewConfigFromFlags()
 	config.AllInvariants = false
 
-	dir, _ := ioutil.TempDir("", "goleveldb-app-invariant-bench")
-	db, _ := sdk.NewLevelDB("simulation", dir)
+	dir, err := ioutil.TempDir("", "goleveldb-app-invariant-bench")
+	if err != nil {
+		fmt.Println(err)
+		b.Fail()
+	}
+	db, err := sdk.NewLevelDB("simulation", dir)
+	if err != nil {
+		fmt.Println(err)
+		b.Fail()
+	}
 
 	defer func() {
 		db.Close()
@@ -656,6 +682,7 @@ func BenchmarkInvariants(b *testing.B) {
 	// NOTE: We use the crisis keeper as it has all the invariants registered with
 	// their respective metadata which makes it useful for testing/benchmarking.
 	for _, cr := range gapp.crisisKeeper.Routes() {
+		cr := cr
 		b.Run(fmt.Sprintf("%s/%s", cr.ModuleName, cr.Route), func(b *testing.B) {
 			if res, stop := cr.Invar(ctx); stop {
 				fmt.Printf("broken invariant at block %d of %d\n%s", ctx.BlockHeight()-1, config.NumBlocks, res)

cli_test/test_helpers.go

@@ -45,6 +45,7 @@ const (
 )
 
 var (
+	// nolint:varcheck,deadcode,unused
 	totalCoins = sdk.NewCoins(
 		sdk.NewCoin(fee2Denom, sdk.TokensFromConsensusPower(2000000)),
 		sdk.NewCoin(feeDenom, sdk.TokensFromConsensusPower(2000000)),
@@ -171,7 +172,7 @@ func InitFixtures(t *testing.T) (f *Fixtures) {
 	f.GenTx(keyFoo)
 	f.CollectGenTxs()
 
-	return
+	return f
 }
 
 // Cleanup is meant to be run at the end of a test to clean up an remaining test state
@@ -757,13 +758,15 @@ func WriteToNewTempFile(t *testing.T, s string) *os.File {
 	return fp
 }
 
+//nolint:deadcode,unused
 func marshalStdTx(t *testing.T, stdTx auth.StdTx) []byte {
 	cdc := app.MakeCodec()
 	bz, err := cdc.MarshalBinaryBare(stdTx)
 	require.NoError(t, err)
 	return bz
 }
 
+//nolint:deadcode,unused
 func unmarshalStdTx(t *testing.T, s string) (stdTx auth.StdTx) {
 	cdc := app.MakeCodec()
 	require.Nil(t, cdc.UnmarshalJSON([]byte(s), &stdTx))

cmd/contract_tests/main.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"fmt"
+
 	"github.com/snikch/goodman/hooks"
 	"github.com/snikch/goodman/transaction"
 )

cmd/gaiad/replay.go

@@ -116,7 +116,10 @@ func replayTxs(rootDir string) error {
 		return err
 	}
 	defer func() {
-		_ = proxyApp.Stop()
+		err = proxyApp.Stop()
+		if err != nil {
+			return
+		}
 	}()
 
 	state := tmsm.LoadState(tmDB)

cmd/gaiad/testnet.go

@@ -113,6 +113,7 @@ func InitTestnet(cmd *cobra.Command, config *tmconfig.Config, cdc *codec.Codec,
 	gaiaConfig := srvconfig.DefaultConfig()
 	gaiaConfig.MinGasPrices = minGasPrices
 
+	//nolint:prealloc
 	var (
 		genAccounts []authexported.GenesisAccount
 		genFiles    []string

cmd/gaiadebug/hack.go

@@ -1,7 +1,6 @@
 package main
 
 import (
-	"encoding/base64"
 	"encoding/hex"
 	"fmt"
 	"os"
@@ -14,7 +13,6 @@ import (
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
 	abci "github.com/tendermint/tendermint/abci/types"
-	"github.com/tendermint/tendermint/crypto/ed25519"
 
 	"github.com/tendermint/tendermint/libs/log"
 
@@ -26,7 +24,7 @@ import (
 func runHackCmd(cmd *cobra.Command, args []string) error {
 
 	if len(args) != 1 {
-		return fmt.Errorf("Expected 1 arg")
+		return fmt.Errorf("expected 1 arg")
 	}
 
 	// ".gaiad"
@@ -88,16 +86,11 @@ func runHackCmd(cmd *cobra.Command, args []string) error {
 	}
 }
 
-func base64ToPub(b64 string) ed25519.PubKeyEd25519 {
-	data, _ := base64.StdEncoding.DecodeString(b64)
-	var pubKey ed25519.PubKeyEd25519
-	copy(pubKey[:], data)
-	return pubKey
-
-}
-
 func hexToBytes(h string) []byte {
-	trouble, _ := hex.DecodeString(h)
+	trouble, err := hex.DecodeString(h)
+	if err != nil {
+		return nil
+	}
 	return trouble
 
 }