Skip to content

Commit

Permalink
lint: various linting fixs (cosmos#5825)
Browse files Browse the repository at this point in the history
* lint: various linting fixs

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

* more linting

* more linting fixes

* more errchecking

* comment out errcheck for now

* undo error check

* address some comments

* remore require error

* change delete to batch delete

Co-authored-by: Alexander Bezobchuk <[email protected]>
  • Loading branch information
tac0turtle and alexanderbez authored Mar 20, 2020
1 parent 22f377a commit d657513
Show file tree
Hide file tree
Showing 45 changed files with 238 additions and 138 deletions.
1 change: 1 addition & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ linters:
- deadcode
- depguard
- dogsled
# - errcheck
- goconst
- gocritic
- gofmt
Expand Down
4 changes: 2 additions & 2 deletions client/keys/add_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ func Test_runAddCmdBasic(t *testing.T) {
kb, err := keys.NewKeyring(sdk.KeyringServiceName(), viper.GetString(flags.FlagKeyringBackend), kbHome, mockIn)
require.NoError(t, err)
t.Cleanup(func() {
kb.Delete("keyname1", "", false)
kb.Delete("keyname2", "", false)
kb.Delete("keyname1", "", false) // nolint:errcheck
kb.Delete("keyname2", "", false) // nolint:errcheck
})
}
assert.NoError(t, runAddCmd(cmd, []string{"keyname1"}))
Expand Down
4 changes: 2 additions & 2 deletions client/keys/delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ func Test_runDeleteCmd(t *testing.T) {
kb, err := keys.NewKeyring(sdk.KeyringServiceName(), viper.GetString(flags.FlagKeyringBackend), viper.GetString(flags.FlagHome), mockIn)
require.NoError(t, err)
t.Cleanup(func() {
kb.Delete("runDeleteCmd_Key1", "", false)
kb.Delete("runDeleteCmd_Key2", "", false)
kb.Delete("runDeleteCmd_Key1", "", false) // nolint:errcheck
kb.Delete("runDeleteCmd_Key2", "", false) // nolint:errcheck

})
}
Expand Down
2 changes: 1 addition & 1 deletion client/keys/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func Test_runExportCmd(t *testing.T) {
require.NoError(t, err)
if !runningUnattended {
t.Cleanup(func() {
kb.Delete("keyname1", "", false)
kb.Delete("keyname1", "", false) // nolint:errcheck
})
}

Expand Down
2 changes: 1 addition & 1 deletion client/keys/import_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func Test_runImportCmd(t *testing.T) {
kb, err := keys.NewKeyring(sdk.KeyringServiceName(), viper.GetString(flags.FlagKeyringBackend), viper.GetString(flags.FlagHome), mockIn)
require.NoError(t, err)
t.Cleanup(func() {
kb.Delete("keyname1", "", false)
kb.Delete("keyname1", "", false) // nolint:errcheck
})
}

Expand Down
3 changes: 2 additions & 1 deletion client/keys/list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ func Test_runListCmd(t *testing.T) {
require.NoError(t, err)

t.Cleanup(func() {
kb.Delete("something", "", false)
kb.Delete("something", "", false) // nolint:errcheck

})
testData := []struct {
name string
Expand Down
12 changes: 7 additions & 5 deletions crypto/keys/keybase.go
Original file line number Diff line number Diff line change
Expand Up @@ -340,8 +340,7 @@ func (kb dbKeybase) Import(name string, armor string) (err error) {
return
}

kb.db.Set(infoKey(name), infoBytes)
return nil
return kb.db.Set(infoKey(name), infoBytes)
}

// ImportPubKey imports ASCII-armored public keys. Store a new Info object holding
Expand Down Expand Up @@ -388,10 +387,13 @@ func (kb dbKeybase) Delete(name, passphrase string, skipPass bool) error {
}
}

kb.db.DeleteSync(addrKey(info.GetAddress()))
kb.db.DeleteSync(infoKey(name))
batch := kb.db.NewBatch()
defer batch.Close()

return nil
batch.Delete(addrKey(info.GetAddress()))
batch.Delete(infoKey(name))

return batch.WriteSync()
}

// Update changes the passphrase with which an already stored key is
Expand Down
3 changes: 2 additions & 1 deletion crypto/keys/keybase_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,8 @@ func TestSignVerify(t *testing.T) {
// Import a public key
armor, err := cstore.ExportPubKey(n2)
require.Nil(t, err)
cstore.ImportPubKey(n3, armor)
err = cstore.ImportPubKey(n3, armor)
require.NoError(t, err)
i3, err := cstore.Get(n3)
require.NoError(t, err)
require.Equal(t, i3.GetName(), n3)
Expand Down
3 changes: 2 additions & 1 deletion crypto/keys/keyring_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,8 @@ func TestLazySignVerifyKeyRing(t *testing.T) {
// Import a public key
armor, err := kb.ExportPubKey(n2)
require.Nil(t, err)
kb.ImportPubKey(n3, armor)
err = kb.ImportPubKey(n3, armor)
require.NoError(t, err)
i3, err := kb.Get(n3)
require.NoError(t, err)
require.Equal(t, i3.GetName(), n3)
Expand Down
3 changes: 2 additions & 1 deletion crypto/keys/lazy_keybase_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,8 @@ func TestLazySignVerify(t *testing.T) {
// Import a public key
armor, err := kb.ExportPubKey(n2)
require.Nil(t, err)
kb.ImportPubKey(n3, armor)
err = kb.ImportPubKey(n3, armor)
require.NoError(t, err)
i3, err := kb.Get(n3)
require.NoError(t, err)
require.Equal(t, i3.GetName(), n3)
Expand Down
62 changes: 34 additions & 28 deletions store/cachekv/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,13 +231,13 @@ func TestCacheKVMergeIteratorDeletes(t *testing.T) {
// set some items and write them
nItems := 10
for i := 0; i < nItems; i++ {
doOp(st, truth, opSet, i)
doOp(t, st, truth, opSet, i)
}
st.Write()

// delete every other item, starting from 0
for i := 0; i < nItems; i += 2 {
doOp(st, truth, opDel, i)
doOp(t, st, truth, opDel, i)
assertIterateDomainCompare(t, st, truth)
}

Expand All @@ -247,13 +247,13 @@ func TestCacheKVMergeIteratorDeletes(t *testing.T) {

// set some items and write them
for i := 0; i < nItems; i++ {
doOp(st, truth, opSet, i)
doOp(t, st, truth, opSet, i)
}
st.Write()

// delete every other item, starting from 1
for i := 1; i < nItems; i += 2 {
doOp(st, truth, opDel, i)
doOp(t, st, truth, opDel, i)
assertIterateDomainCompare(t, st, truth)
}
}
Expand All @@ -265,27 +265,27 @@ func TestCacheKVMergeIteratorChunks(t *testing.T) {
truth := dbm.NewMemDB()

// sets to the parent
setRange(st, truth, 0, 20)
setRange(st, truth, 40, 60)
setRange(t, st, truth, 0, 20)
setRange(t, st, truth, 40, 60)
st.Write()

// sets to the cache
setRange(st, truth, 20, 40)
setRange(st, truth, 60, 80)
setRange(t, st, truth, 20, 40)
setRange(t, st, truth, 60, 80)
assertIterateDomainCheck(t, st, truth, []keyRange{{0, 80}})

// remove some parents and some cache
deleteRange(st, truth, 15, 25)
deleteRange(t, st, truth, 15, 25)
assertIterateDomainCheck(t, st, truth, []keyRange{{0, 15}, {25, 80}})

// remove some parents and some cache
deleteRange(st, truth, 35, 45)
deleteRange(t, st, truth, 35, 45)
assertIterateDomainCheck(t, st, truth, []keyRange{{0, 15}, {25, 35}, {45, 80}})

// write, add more to the cache, and delete some cache
st.Write()
setRange(st, truth, 38, 42)
deleteRange(st, truth, 40, 43)
setRange(t, st, truth, 38, 42)
deleteRange(t, st, truth, 40, 43)
assertIterateDomainCheck(t, st, truth, []keyRange{{0, 15}, {25, 35}, {38, 40}, {45, 80}})
}

Expand All @@ -295,11 +295,11 @@ func TestCacheKVMergeIteratorRandom(t *testing.T) {

start, end := 25, 975
max := 1000
setRange(st, truth, start, end)
setRange(t, st, truth, start, end)

// do an op, test the iterator
for i := 0; i < 2000; i++ {
doRandomOp(st, truth, max)
doRandomOp(t, st, truth, max)
assertIterateDomainCompare(t, st, truth)
}
}
Expand All @@ -322,48 +322,52 @@ func randInt(n int) int {
}

// useful for replaying a error case if we find one
func doOp(st types.CacheKVStore, truth dbm.DB, op int, args ...int) {
func doOp(t *testing.T, st types.CacheKVStore, truth dbm.DB, op int, args ...int) {
switch op {
case opSet:
k := args[0]
st.Set(keyFmt(k), valFmt(k))
truth.Set(keyFmt(k), valFmt(k))
err := truth.Set(keyFmt(k), valFmt(k))
require.NoError(t, err)
case opSetRange:
start := args[0]
end := args[1]
setRange(st, truth, start, end)
setRange(t, st, truth, start, end)
case opDel:
k := args[0]
st.Delete(keyFmt(k))
truth.Delete(keyFmt(k))
err := truth.Delete(keyFmt(k))
require.NoError(t, err)
case opDelRange:
start := args[0]
end := args[1]
deleteRange(st, truth, start, end)
deleteRange(t, st, truth, start, end)
case opWrite:
st.Write()
}
}

func doRandomOp(st types.CacheKVStore, truth dbm.DB, maxKey int) {
func doRandomOp(t *testing.T, st types.CacheKVStore, truth dbm.DB, maxKey int) {
r := randInt(totalOps)
switch r {
case opSet:
k := randInt(maxKey)
st.Set(keyFmt(k), valFmt(k))
truth.Set(keyFmt(k), valFmt(k))
err := truth.Set(keyFmt(k), valFmt(k))
require.NoError(t, err)
case opSetRange:
start := randInt(maxKey - 2)
end := randInt(maxKey-start) + start
setRange(st, truth, start, end)
setRange(t, st, truth, start, end)
case opDel:
k := randInt(maxKey)
st.Delete(keyFmt(k))
truth.Delete(keyFmt(k))
err := truth.Delete(keyFmt(k))
require.NoError(t, err)
case opDelRange:
start := randInt(maxKey - 2)
end := randInt(maxKey-start) + start
deleteRange(st, truth, start, end)
deleteRange(t, st, truth, start, end)
case opWrite:
st.Write()
}
Expand Down Expand Up @@ -439,17 +443,19 @@ func checkIterators(t *testing.T, itr, itr2 types.Iterator) {

//--------------------------------------------------------

func setRange(st types.KVStore, mem dbm.DB, start, end int) {
func setRange(t *testing.T, st types.KVStore, mem dbm.DB, start, end int) {
for i := start; i < end; i++ {
st.Set(keyFmt(i), valFmt(i))
mem.Set(keyFmt(i), valFmt(i))
err := mem.Set(keyFmt(i), valFmt(i))
require.NoError(t, err)
}
}

func deleteRange(st types.KVStore, mem dbm.DB, start, end int) {
func deleteRange(t *testing.T, st types.KVStore, mem dbm.DB, start, end int) {
for i := start; i < end; i++ {
st.Delete(keyFmt(i))
mem.Delete(keyFmt(i))
err := mem.Delete(keyFmt(i))
require.NoError(t, err)
}
}

Expand Down
4 changes: 2 additions & 2 deletions store/iavl/tree_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ func TestImmutableTreePanics(t *testing.T) {
it := &immutableTree{immTree}
require.Panics(t, func() { it.Set([]byte{}, []byte{}) })
require.Panics(t, func() { it.Remove([]byte{}) })
require.Panics(t, func() { it.SaveVersion() })
require.Panics(t, func() { it.DeleteVersion(int64(1)) })
require.Panics(t, func() { it.SaveVersion() }) // nolint:errcheck
require.Panics(t, func() { it.DeleteVersion(int64(1)) }) // nolint:errcheck
v, _ := it.GetVersioned([]byte{0x01}, 1)
require.Equal(t, int64(-1), v)
v, _ = it.GetVersioned([]byte{0x01}, 0)
Expand Down
6 changes: 4 additions & 2 deletions store/prefix/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,11 @@ func genRandomKVPairs(t *testing.T) []kvpair {

for i := 0; i < 20; i++ {
kvps[i].key = make([]byte, 32)
rand.Read(kvps[i].key)
_, err := rand.Read(kvps[i].key)
require.NoError(t, err)
kvps[i].value = make([]byte, 32)
rand.Read(kvps[i].value)
_, err = rand.Read(kvps[i].value)
require.NoError(t, err)
}

return kvps
Expand Down
10 changes: 6 additions & 4 deletions store/rootmulti/proof_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,8 @@ func TestVerifyMultiStoreQueryProofEmptyStore(t *testing.T) {
iavlStoreKey := types.NewKVStoreKey("iavlStoreKey")

store.MountStoreWithDB(iavlStoreKey, types.StoreTypeIAVL, nil)
store.LoadVersion(0)
err := store.LoadVersion(0)
require.NoError(t, err)
cid := store.Commit() // Commit with empty iavl store.

// Get Proof
Expand All @@ -129,7 +130,7 @@ func TestVerifyMultiStoreQueryProofEmptyStore(t *testing.T) {

// Verify proof.
prt := DefaultProofRuntime()
err := prt.VerifyAbsence(res.Proof, cid.Hash, "/iavlStoreKey/MYKEY")
err = prt.VerifyAbsence(res.Proof, cid.Hash, "/iavlStoreKey/MYKEY")
require.Nil(t, err)

// Verify (bad) proof.
Expand All @@ -145,7 +146,8 @@ func TestVerifyMultiStoreQueryProofAbsence(t *testing.T) {
iavlStoreKey := types.NewKVStoreKey("iavlStoreKey")

store.MountStoreWithDB(iavlStoreKey, types.StoreTypeIAVL, nil)
store.LoadVersion(0)
err := store.LoadVersion(0)
require.NoError(t, err)

iavlStore := store.GetCommitStore(iavlStoreKey).(*iavl.Store)
iavlStore.Set([]byte("MYKEY"), []byte("MYVALUE"))
Expand All @@ -161,7 +163,7 @@ func TestVerifyMultiStoreQueryProofAbsence(t *testing.T) {

// Verify proof.
prt := DefaultProofRuntime()
err := prt.VerifyAbsence(res.Proof, cid.Hash, "/iavlStoreKey/MYABSENTKEY")
err = prt.VerifyAbsence(res.Proof, cid.Hash, "/iavlStoreKey/MYABSENTKEY")
require.Nil(t, err)

// Verify (bad) proof.
Expand Down
5 changes: 4 additions & 1 deletion store/rootmulti/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -691,5 +691,8 @@ func flushCommitInfo(db dbm.DB, version int64, cInfo commitInfo) {

setCommitInfo(batch, version, cInfo)
setLatestVersion(batch, version)
batch.Write()
err := batch.Write()
if err != nil {
panic(fmt.Errorf("error on batch write %w", err))
}
}
9 changes: 5 additions & 4 deletions types/context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,12 @@ func (l MockLogger) With(kvs ...interface{}) log.Logger {
panic("not implemented")
}

func defaultContext(key types.StoreKey) types.Context {
func defaultContext(t *testing.T, key types.StoreKey) types.Context {
db := dbm.NewMemDB()
cms := store.NewCommitMultiStore(db)
cms.MountStoreWithDB(key, types.StoreTypeIAVL, db)
cms.LoadLatestVersion()
err := cms.LoadLatestVersion()
require.NoError(t, err)
ctx := types.NewContext(cms, abci.Header{}, false, log.NewNopLogger())
return ctx
}
Expand All @@ -61,7 +62,7 @@ func TestCacheContext(t *testing.T) {
k2 := []byte("key")
v2 := []byte("value")

ctx := defaultContext(key)
ctx := defaultContext(t, key)
store := ctx.KVStore(key)
store.Set(k1, v1)
require.Equal(t, v1, store.Get(k1))
Expand All @@ -83,7 +84,7 @@ func TestCacheContext(t *testing.T) {

func TestLogContext(t *testing.T) {
key := types.NewKVStoreKey(t.Name())
ctx := defaultContext(key)
ctx := defaultContext(t, key)
logger := NewMockLogger()
ctx = ctx.WithLogger(logger)
ctx.Logger().Debug("debug")
Expand Down
3 changes: 2 additions & 1 deletion types/decimal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,8 @@ func TestDecMarshalJSON(t *testing.T) {
if !tt.wantErr {
assert.Equal(t, tt.want, string(got), "incorrect marshalled value")
unmarshalledDec := NewDec(0)
unmarshalledDec.UnmarshalJSON(got)
err := unmarshalledDec.UnmarshalJSON(got)
assert.NoError(t, err)
assert.Equal(t, tt.d, unmarshalledDec, "incorrect unmarshalled value")
}
})
Expand Down
Loading

0 comments on commit d657513

Please sign in to comment.