Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: linter setup and linter issues #480

Merged
merged 1 commit into from
Mar 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 21 additions & 7 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
@@ -1,17 +1,31 @@
name: Lint
# Lint runs golangci-lint over the entire cosmos-sdk repository
# This workflow is run on every pull request and push to master
# The `golangci` will pass without running if no *.{go, mod, sum} files have been changed.
on:
pull_request:
push:
branches:
- master
jobs:
golangci-lint:
golangci:
name: golangci-lint
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: golangci/[email protected]
- uses: actions/[email protected]
with:
# Required: the version of golangci-lint is required and must be specified without patch version: we always use the latest patch version.
version: v1.32
args: --timeout 10m
github-token: ${{ secrets.github_token }}
go-version: 1.17
- uses: technote-space/[email protected]
id: git_diff
with:
PATTERNS: |
**/**.go
go.mod
go.sum
- name: golangci-lint
uses: golangci/golangci-lint-action@v3
with:
version: latest
args: --out-format=tab
skip-go-installation: true
if: env.GIT_DIFF
72 changes: 39 additions & 33 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -1,68 +1,74 @@
run:
tests: false
# timeout for analysis, e.g. 30s, 5m, default is 1m
timeout: 5m

linters:
disable-all: true
enable:
- bodyclose
- deadcode
- depguard
- dogsled
- dupl
- errcheck
# - funlen
# - gochecknoglobals
# - gochecknoinits
# - errcheck
- exportloopref
- goconst
- gocritic
# - gocyclo
# - godox
- gofmt
- goimports
- golint
- gosec
- gosimple
- govet
- ineffassign
- interfacer
# - lll
- misspell
- maligned
- nakedret
- nolintlint
- prealloc
- scopelint
- revive
- staticcheck
- structcheck
- stylecheck
- typecheck
- unconvert
# - unparam
- varcheck
- unparam
- unused
# - whitespace
# - wsl
# - gocognit
- nolintlint
disable:

issues:
exclude-rules:
- path: _test\.go
- text: "Use of weak random number generator"
linters:
- gosec
- linters:
- lll
source: "https://"
- text: "comment on exported var"
linters:
- golint
- text: "don't use an underscore in package name"
linters:
- golint
- text: "ST1003:"
linters:
- stylecheck
# FIXME: Disabled until golangci-lint updates stylecheck with this fix:
# https://github.com/dominikh/go-tools/issues/389
- text: "ST1016:"
linters:
- stylecheck
- path: "migrations"
text: "SA1019:"
linters:
- staticcheck

max-issues-per-linter: 10000
max-same-issues: 10000

linters-settings:
dogsled:
max-blank-identifiers: 3
maligned:
# print struct with more effective memory layout or not, false by default
suggest-new: true
govet:
check-shadowing: true
golint:
min-confidence: 0
misspell:
locale: US
# gocritic:
# enabled-tags:
# - performance
# - style
# - experimental
nolintlint:
allow-unused: false
allow-leading-space: true
require-explanation: false
require-specific: false
4 changes: 2 additions & 2 deletions basic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func TestUnit(t *testing.T) {

expectHash := func(tree *ImmutableTree, hashCount int64) {
// ensure number of new hash calculations is as expected.
hash, count := tree.hashWithCount()
hash, count := tree.root.hashWithCount()
if count != hashCount {
t.Fatalf("Expected %v new hashes, got %v", hashCount, count)
}
Expand All @@ -118,7 +118,7 @@ func TestUnit(t *testing.T) {
return false
})
// ensure that the new hash after nuking is the same as the old.
newHash, _ := tree.hashWithCount()
newHash, _ := tree.root.hashWithCount()
if !bytes.Equal(hash, newHash) {
t.Fatalf("Expected hash %v but got %v after nuking", hash, newHash)
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/iaviewer/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func OpenDB(dir string) (dbm.DB, error) {
return db, nil
}

// nolint: unused,deadcode
// nolint: deadcode
func PrintDBStats(db dbm.DB) {
count := 0
prefix := map[string]int{}
Expand Down
1 change: 1 addition & 0 deletions export.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
const exportBufferSize = 32

// ExportDone is returned by Exporter.Next() when all items have been exported.
// nolint:revive
var ExportDone = errors.New("export is complete") // nolint:golint

// ExportNode contains exported node data.
Expand Down
10 changes: 3 additions & 7 deletions immutable_tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,9 @@ func (t *ImmutableTree) renderNode(node *Node, indent string, depth int, encoder

// recurse on inner node
here := fmt.Sprintf("%s%s", prefix, encoder(node.hash, depth, false))
left := t.renderNode(node.getLeftNode(t), indent, depth+1, encoder)
right := t.renderNode(node.getRightNode(t), indent, depth+1, encoder)
result := append(left, here)
result := t.renderNode(node.getLeftNode(t), indent, depth+1, encoder) // left
result = append(result, here)
result = append(result, right...)
return result
}
Expand Down Expand Up @@ -131,11 +131,6 @@ func (t *ImmutableTree) Hash() []byte {
return hash
}

// hashWithCount returns the root hash and hash count.
func (t *ImmutableTree) hashWithCount() ([]byte, int64) {
return t.root.hashWithCount()
}

// Export returns an iterator that exports tree nodes as ExportNodes. These nodes can be
// imported with MutableTree.Import() to recreate an identical tree.
func (t *ImmutableTree) Export() *Exporter {
Expand Down Expand Up @@ -215,6 +210,7 @@ func (t *ImmutableTree) clone() *ImmutableTree {
}

// nodeSize is like Size, but includes inner nodes too.
//nolint:unused
func (t *ImmutableTree) nodeSize() int {
size := 0
t.root.traverse(t, true, func(n *Node) bool {
Expand Down
2 changes: 1 addition & 1 deletion internal/rand/random.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func (r *Rand) init() {
}

func (r *Rand) reset(seed int64) {
r.rand = mrand.New(mrand.NewSource(seed)) // nolint:gosec // G404: Use of weak random number generator
r.rand = mrand.New(mrand.NewSource(seed))
}

//----------------------------------------
Expand Down
2 changes: 1 addition & 1 deletion mutable_tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ func (tree *MutableTree) recursiveRemove(node *Node, key []byte, orphans *[]*Nod

// node.key < key; we go to the left to find the key:
if bytes.Compare(key, node.key) < 0 {
newLeftHash, newLeftNode, newKey, value := tree.recursiveRemove(node.getLeftNode(tree.ImmutableTree), key, orphans) //nolint:govet
newLeftHash, newLeftNode, newKey, value := tree.recursiveRemove(node.getLeftNode(tree.ImmutableTree), key, orphans)

if len(*orphans) == 0 {
return node.hash, node, nil, value
Expand Down
8 changes: 0 additions & 8 deletions node.go
Original file line number Diff line number Diff line change
Expand Up @@ -465,11 +465,3 @@ func (node *Node) traverseInRange(tree *ImmutableTree, start, end []byte, ascend
}
return stop
}

// Only used in testing...
func (node *Node) lmd(t *ImmutableTree) *Node {
if node.isLeaf() {
return node
}
return node.getLeftNode(t).lmd(t)
}
11 changes: 6 additions & 5 deletions nodedb.go
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,7 @@ func (ndb *nodeDB) traverseOrphansVersion(version int64, fn func(k, v []byte)) {
}

// Traverse all keys.
//nolint:unused
func (ndb *nodeDB) traverse(fn func(key, value []byte)) {
ndb.traverseRange(nil, nil, fn)
}
Expand Down Expand Up @@ -572,6 +573,7 @@ func (ndb *nodeDB) getRoot(version int64) ([]byte, error) {
return ndb.db.Get(ndb.rootKey(version))
}

//nolint:unparam
func (ndb *nodeDB) getRoots() (map[int64][]byte, error) {
roots := map[int64][]byte{}

Expand Down Expand Up @@ -632,6 +634,7 @@ func (ndb *nodeDB) decrVersionReaders(version int64) {

// Utility and test functions

//nolint:unused
func (ndb *nodeDB) leafNodes() []*Node {
leaves := []*Node{}

Expand All @@ -643,6 +646,7 @@ func (ndb *nodeDB) leafNodes() []*Node {
return leaves
}

//nolint:unused
func (ndb *nodeDB) nodes() []*Node {
nodes := []*Node{}

Expand All @@ -652,6 +656,7 @@ func (ndb *nodeDB) nodes() []*Node {
return nodes
}

//nolint:unused
func (ndb *nodeDB) orphans() [][]byte {
orphans := [][]byte{}

Expand All @@ -661,14 +666,10 @@ func (ndb *nodeDB) orphans() [][]byte {
return orphans
}

func (ndb *nodeDB) roots() map[int64][]byte {
roots, _ := ndb.getRoots()
return roots
}

// Not efficient.
// NOTE: DB cannot implement Size() because
// mutations are not always synchronous.
//nolint:unused
func (ndb *nodeDB) size() int {
size := 0
ndb.traverse(func(k, v []byte) {
Expand Down
5 changes: 3 additions & 2 deletions proof_range.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ func (proof *RangeProof) VerifyAbsence(key []byte) error {
case cmp < 0:
return nil // proof ok
case cmp == 0:
return errors.New(fmt.Sprintf("absence disproved via item #%v", i))
return fmt.Errorf("absence disproved via item #%v", i)
default:
// if i == len(proof.Leaves)-1 {
// If last item, check whether
Expand Down Expand Up @@ -225,7 +225,7 @@ func (proof *RangeProof) _computeRootHash() (rootHash []byte, treeEnd bool, err
return nil, false, errors.Wrap(ErrInvalidProof, "no leaves")
}
if len(proof.InnerNodes)+1 != len(proof.Leaves) {
return nil, false, errors.Wrap(ErrInvalidProof, "InnerNodes vs Leaves length mismatch, leaves should be 1 more.")
return nil, false, errors.Wrap(ErrInvalidProof, "InnerNodes vs Leaves length mismatch, leaves should be 1 more.") //nolint:revive
}

// Start from the left path and prove each leaf.
Expand Down Expand Up @@ -372,6 +372,7 @@ func RangeProofFromProto(pbProof *iavlproto.RangeProof) (RangeProof, error) {
// If keyEnd-1 exists, no later leaves will be included.
// If keyStart >= keyEnd and both not nil, panics.
// Limit is never exceeded.
//nolint:unparam
func (t *ImmutableTree) getRangeProof(keyStart, keyEnd []byte, limit int) (proof *RangeProof, keys, values [][]byte, err error) {
if keyStart != nil && keyEnd != nil && bytes.Compare(keyStart, keyEnd) >= 0 {
panic("if keyStart and keyEnd are present, need keyStart < keyEnd.")
Expand Down
29 changes: 29 additions & 0 deletions proof_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package iavl

import (
"bytes"
"sort"
"testing"

proto "github.com/gogo/protobuf/proto"
Expand Down Expand Up @@ -282,3 +283,31 @@ func MutateByteSlice(bytez []byte) []byte {
}
return bytez
}

func sortByteSlices(src [][]byte) [][]byte {
bzz := byteslices(src)
sort.Sort(bzz)
return bzz
}

type byteslices [][]byte

func (bz byteslices) Len() int {
return len(bz)
}

func (bz byteslices) Less(i, j int) bool {
switch bytes.Compare(bz[i], bz[j]) {
case -1:
return true
case 0, 1:
return false
default:
panic("should not happen")
}
}

//nolint:unused
func (bz byteslices) Swap(i, j int) {
bz[j], bz[i] = bz[i], bz[j]
}
7 changes: 7 additions & 0 deletions testutils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,3 +151,10 @@ func benchmarkImmutableAvlTreeWithDB(b *testing.B, db db.DB) {
}
}
}

func (node *Node) lmd(t *ImmutableTree) *Node {
if node.isLeaf() {
return node
}
return node.getLeftNode(t).lmd(t)
}
5 changes: 4 additions & 1 deletion tree_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,10 @@ func TestVersionedRandomTree(t *testing.T) {
}
tree.SaveVersion()
}
require.Equal(versions, len(tree.ndb.roots()), "wrong number of roots")
roots, err := tree.ndb.getRoots()
require.NoError(err)

require.Equal(versions, len(roots), "wrong number of roots")
require.Equal(versions*keysPerVersion, len(tree.ndb.leafNodes()), "wrong number of nodes")

// Before deleting old versions, we should have equal or more nodes in the
Expand Down
Loading