chore: lint code (#114)

* add pebbledb

* use latest pebble

* added a ton of test rigor and fixed linting

* if always false we don't need the param

* really we don't need that

* Update pebble.go

Co-authored-by: Jasmina Malicevic <jasmina.dustinac@gmail.com>

* Update pebble.go

Co-authored-by: Jasmina Malicevic <jasmina.dustinac@gmail.com>

* Update pebble.go

Co-authored-by: Jasmina Malicevic <jasmina.dustinac@gmail.com>

* Update pebble.go

Co-authored-by: Jasmina Malicevic <jasmina.dustinac@gmail.com>

* Update pebble.go

Co-authored-by: Jasmina Malicevic <jasmina.dustinac@gmail.com>

* Update pebble.go

Co-authored-by: Jasmina Malicevic <jasmina.dustinac@gmail.com>

* clean up

* change default ForceSync=1

* pebble

* Update db.go

Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>

* resolve conflicts

* update CHANGELOG.md: Added support for pebbledb v1.0.0

* add unclog entry

* add changelog

* correct a readme file issue

* correct changelog and reove 112's unclog

* update .golangci.yml

* add pebble to the ci test runs

* fix the callt o registerDBCreator

* remove manual edits to changelog so unclog can do its thing

---------

Co-authored-by: Tuan Pham Anh <baabeetaa@gmail.com>
Co-authored-by: Jasmina Malicevic <jasmina.dustinac@gmail.com>
Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>

.changelog/unreleased/chore/114.md

@@ -0,0 +1 @@
+chore: lint code

.golangci.yml

@@ -1,51 +1,115 @@
-linters:
+run:
   tests: true
+  timeout: 10m
+
+linters:
   disable-all: true
   enable:
-    - bodyclose
-    - depguard
-    - dogsled
-    - dupl
+    - exportloopref
     - errcheck
+    - gci
     - goconst
     - gocritic
     - gofumpt
     - gosec
     - gosimple
     - govet
     - ineffassign
-    - lll
     - misspell
     - nakedret
-    - prealloc
     - staticcheck
+    - thelper
+    - typecheck
     - stylecheck
+    - revive
     - typecheck
+    - tenv
     - unconvert
+    # Prefer unparam over revive's unused param. It is more thorough in its checking.
     - unparam
     - unused
-    - nolintlint
+    - misspell
 
 issues:
+  exclude-rules:
+    - text: 'differs only by capitalization to method'
+      linters:
+        - revive
+    - text: 'Use of weak random number generator'
+      linters:
+        - gosec
+    - linters:
+        - staticcheck
+      text: "SA1019:" # silence errors on usage of deprecated funcs
+
+  max-issues-per-linter: 10000
+  max-same-issues: 10000
+
 linters-settings:
   errcheck:
     check-blank: true
-  depguard:
+  gci:
+    sections:
+      - standard # Standard section: captures all standard packages.
+      - default # Default section: contains all imports that could not be matched to another section type.
+      - blank # blank imports
+      - dot # dot imports
+      - prefix(github.com/cometbft/cometbft-db)
+    custom-order: true
+  revive:
+    enable-all-rules: true
+    # Do NOT whine about the following, full explanation found in:
+    # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#description-of-available-rules
     rules:
-      main:
-        files:
-          - $all
-          - "!$test"
-        allow:
-          - $gostd
-          - github.com/cometbft
-          - github.com/syndtr/goleveldb/leveldb
-          - github.com/google/btree
-      test:
-        files:
-          - $test
-        allow:
-          - $gostd
-          - github.com/cometbft
-          - github.com/syndtr/goleveldb/leveldb
-          - github.com/stretchr/testify
+      - name: use-any
+        disabled: true
+      - name: if-return
+        disabled: true
+      - name: max-public-structs
+        disabled: true
+      - name: cognitive-complexity
+        disabled: true
+      - name: argument-limit
+        disabled: true
+      - name: cyclomatic
+        disabled: true
+      - name: file-header
+        disabled: true
+      - name: function-length
+        disabled: true
+      - name: function-result-limit
+        disabled: true
+      - name: line-length-limit
+        disabled: true
+      - name: flag-parameter
+        disabled: true
+      - name: add-constant
+        disabled: true
+      - name: empty-lines
+        disabled: true
+      - name: banned-characters
+        disabled: true
+      - name: deep-exit
+        disabled: true
+      - name: confusing-results
+        disabled: true
+      - name: unused-parameter
+        disabled: true
+      - name: modifies-value-receiver
+        disabled: true
+      - name: early-return
+        disabled: true
+      - name: confusing-naming
+        disabled: true
+      - name: defer
+        disabled: true
+      # Disabled in favour of unparam.
+      - name: unused-parameter
+        disabled: true
+      - name: unhandled-error
+        disabled: false
+        arguments:
+          - 'fmt.Printf'
+          - 'fmt.Print'
+          - 'fmt.Println'
+          - 'myFunction'

Makefile

@@ -45,7 +45,7 @@ test-pebble:
 
 test-all:
 	@echo "--> Running go test"
-	@go test $(PACKAGES) -tags cleveldb,boltdb,rocksdb,grocksdb_clean_link,badgerdb -v
+	@go test $(PACKAGES) -tags cleveldb,boltdb,rocksdb,grocksdb_clean_link,badgerdb,pebbledb -v
 .PHONY: test-all
 
 test-all-with-coverage:

README.md

@@ -16,7 +16,7 @@ applications built on [CometBFT], such as the [Cosmos SDK].
 totally deprecating and removing this library from CometBFT. As such, we do not
 recommend depending on this library for new projects.
 
-### Minimum Go Version
+## Minimum Go Version
 
 Go 1.21+
 

backend_test.go

@@ -12,7 +12,7 @@ import (
 
 // Register a test backend for PrefixDB as well, with some unrelated junk data
 func init() {
-	//nolint: errcheck
+	//nolint: errcheck, revive // probably should check errors?
 	registerDBCreator("prefixdb", func(name, dir string) (DB, error) {
 		mdb := NewMemDB()
 		mdb.Set([]byte("a"), []byte{1})
@@ -22,7 +22,7 @@ func init() {
 		mdb.Set([]byte("u"), []byte{21})
 		mdb.Set([]byte("z"), []byte{26})
 		return NewPrefixDB(mdb, []byte("test/")), nil
-	}, false)
+	})
 }
 
 func cleanupDBDir(dir, name string) {
@@ -33,6 +33,7 @@ func cleanupDBDir(dir, name string) {
 }
 
 func testBackendGetSetDelete(t *testing.T, backend BackendType) {
+	t.Helper()
 	// Default
 	dirname, err := os.MkdirTemp("", fmt.Sprintf("test_backend_%s_", backend))
 	require.Nil(t, err)
@@ -161,6 +162,8 @@ func TestDBIterator(t *testing.T) {
 }
 
 func testDBIterator(t *testing.T, backend BackendType) {
+	t.Helper()
+
 	name := fmt.Sprintf("test_%x", randStr(12))
 	dir := os.TempDir()
 	db, err := NewDB(name, backend, dir)
@@ -317,6 +320,8 @@ func testDBIterator(t *testing.T, backend BackendType) {
 }
 
 func verifyIterator(t *testing.T, itr Iterator, expected []int64, msg string) {
+	t.Helper()
+
 	var list []int64
 	for itr.Valid() {
 		key := itr.Key()
@@ -335,6 +340,8 @@ func TestDBBatch(t *testing.T) {
 }
 
 func testDBBatch(t *testing.T, backend BackendType) {
+	t.Helper()
+
 	name := fmt.Sprintf("test_%x", randStr(12))
 	dir := os.TempDir()
 	db, err := NewDB(name, backend, dir)
@@ -396,8 +403,12 @@ func testDBBatch(t *testing.T, backend BackendType) {
 
 	// it should be possible to close an empty batch, and to re-close a closed batch
 	batch = db.NewBatch()
-	batch.Close()
-	batch.Close()
+	if err := batch.Close(); err != nil {
+		require.NoError(t, err)
+	}
+	if err := batch.Close(); err != nil {
+		require.NoError(t, err)
+	}
 
 	// all other operations on a closed batch should error
 	require.Error(t, batch.Set([]byte("a"), []byte{9}))
@@ -407,6 +418,7 @@ func testDBBatch(t *testing.T, backend BackendType) {
 }
 
 func assertKeyValues(t *testing.T, db DB, expect map[string][]byte) {
+	t.Helper()
 	iter, err := db.Iterator(nil, nil)
 	require.NoError(t, err)
 	defer iter.Close()

badger_db.go

@@ -12,7 +12,7 @@ import (
 	"github.com/dgraph-io/badger/v2"
 )
 
-func init() { registerDBCreator(BadgerDBBackend, badgerDBCreator, true) }
+func init() { registerDBCreator(BadgerDBBackend, badgerDBCreator) }
 
 func badgerDBCreator(dbName, dir string) (DB, error) {
 	return NewBadgerDB(dbName, dir)

boltdb.go

@@ -19,7 +19,7 @@ var (
 func init() {
 	registerDBCreator(BoltDBBackend, func(name, dir string) (DB, error) {
 		return NewBoltDB(name, dir)
-	}, false)
+	})
 }
 
 // BoltDB is a wrapper around etcd's fork of bolt (https://github.com/etcd-io/bbolt).

cleveldb.go

@@ -14,7 +14,7 @@ func init() {
 	dbCreator := func(name string, dir string) (DB, error) {
 		return NewCLevelDB(name, dir)
 	}
-	registerDBCreator(CLevelDBBackend, dbCreator, false)
+	registerDBCreator(CLevelDBBackend, dbCreator)
 }
 
 // CLevelDB uses the C LevelDB database via a Go wrapper.

common_test.go

@@ -11,38 +11,44 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-//----------------------------------------
+// ----------------------------------------
 // Helper functions.
 
 func checkValue(t *testing.T, db DB, key []byte, valueWanted []byte) {
+	t.Helper()
 	valueGot, err := db.Get(key)
 	assert.NoError(t, err)
 	assert.Equal(t, valueWanted, valueGot)
 }
 
 func checkValid(t *testing.T, itr Iterator, expected bool) {
+	t.Helper()
 	valid := itr.Valid()
 	require.Equal(t, expected, valid)
 }
 
 func checkNext(t *testing.T, itr Iterator, expected bool) {
+	t.Helper()
 	itr.Next()
 	// assert.NoError(t, err) TODO: look at fixing this
 	valid := itr.Valid()
 	require.Equal(t, expected, valid)
 }
 
 func checkNextPanics(t *testing.T, itr Iterator) {
+	t.Helper()
 	assert.Panics(t, func() { itr.Next() }, "checkNextPanics expected an error but didn't")
 }
 
 func checkDomain(t *testing.T, itr Iterator, start, end []byte) {
+	t.Helper()
 	ds, de := itr.Domain()
 	assert.Equal(t, start, ds, "checkDomain domain start incorrect")
 	assert.Equal(t, end, de, "checkDomain domain end incorrect")
 }
 
 func checkItem(t *testing.T, itr Iterator, key []byte, value []byte) {
+	t.Helper()
 	v := itr.Value()
 
 	k := itr.Key()
@@ -52,21 +58,25 @@ func checkItem(t *testing.T, itr Iterator, key []byte, value []byte) {
 }
 
 func checkInvalid(t *testing.T, itr Iterator) {
+	t.Helper()
 	checkValid(t, itr, false)
 	checkKeyPanics(t, itr)
 	checkValuePanics(t, itr)
 	checkNextPanics(t, itr)
 }
 
 func checkKeyPanics(t *testing.T, itr Iterator) {
+	t.Helper()
 	assert.Panics(t, func() { itr.Key() }, "checkKeyPanics expected panic but didn't")
 }
 
 func checkValuePanics(t *testing.T, itr Iterator) {
+	t.Helper()
 	assert.Panics(t, func() { itr.Value() })
 }
 
 func newTempDB(t *testing.T, backend BackendType) (db DB, dbDir string) {
+	t.Helper()
 	dirname, err := os.MkdirTemp("", "db_common_test")
 	require.NoError(t, err)
 	db, err = NewDB("testdb", backend, dirname)
@@ -75,6 +85,7 @@ func newTempDB(t *testing.T, backend BackendType) (db DB, dbDir string) {
 }
 
 func benchmarkRangeScans(b *testing.B, db DB, dbSize int64) {
+	b.Helper()
 	b.StopTimer()
 
 	rangeSize := int64(10000)
@@ -83,8 +94,8 @@ func benchmarkRangeScans(b *testing.B, db DB, dbSize int64) {
 	}
 
 	for i := int64(0); i < dbSize; i++ {
-		bytes := int642Bytes(i)
-		err := db.Set(bytes, bytes)
+		int64bytes := int642Bytes(i)
+		err := db.Set(int64bytes, int64bytes)
 		if err != nil {
 			// require.NoError() is very expensive (according to profiler), so check manually
 			b.Fatal(b, err)
@@ -101,12 +112,14 @@ func benchmarkRangeScans(b *testing.B, db DB, dbSize int64) {
 		for ; iter.Valid(); iter.Next() {
 			count++
 		}
-		iter.Close()
+		err = iter.Close()
+		require.NoError(b, err)
 		require.EqualValues(b, rangeSize, count)
 	}
 }
 
 func benchmarkRandomReadsWrites(b *testing.B, db DB) {
+	b.Helper()
 	b.StopTimer()
 
 	// create dummy data

db.go

@@ -46,9 +46,9 @@ type dbCreator func(name string, dir string) (DB, error)
 
 var backends = map[BackendType]dbCreator{}
 
-func registerDBCreator(backend BackendType, creator dbCreator, force bool) {
+func registerDBCreator(backend BackendType, creator dbCreator) {
 	_, ok := backends[backend]
-	if !force && ok {
+	if ok {
 		return
 	}
 	backends[backend] = creator