chore(linting): lint all files based on latest setting (#1171)

* lint: nakedret

* lint: thelper

* lint: autofix

* lint: test

* lint: autofix

* lint: unused

* lint: unconvert

* lint: stylecheck

* lint: staticcheck

* lint: ineffassign

* lint: govet

* lint: gosimple

* lint: gosec

* lint: gofumpt

* lint: gocritic

* lint: goconst

* lint: errcheck

* lint: nolintlint

* fix

.github/workflows/lint.yml

@@ -30,7 +30,7 @@ jobs:
       - uses: golangci/golangci-lint-action@v3
         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.51
+          version: v1.55
           args: --timeout 10m
           github-token: ${{ secrets.GITHUB_TOKEN }}
         if: env.GIT_DIFF

.golangci.yml

@@ -1,37 +1,45 @@
 run:
-  tests: false
-#   # timeout for analysis, e.g. 30s, 5m, default is 1m
-#   timeout: 5m
+  tests: true
+  timeout: 15m
+  sort-results: true
+  allow-parallel-runners: true
+  exclude-dir: testutil/testdata
+  skip-files:
+    - server/grpc/gogoreflection/fix_registration.go
+    - "fix_registration.go"
+    - ".*\\.pb\\.go$"
+    - ".*\\.pb\\.gw\\.go$"
+    - ".*\\.pulsar\\.go$"
+    - crypto/keys/secp256k1/internal/*
+  build-tags:
+    - ledger
+    - goleveldb
+    - test_ledger_mock
 
 linters:
-  disable-all: false
+  disable-all: true
   enable:
-    - gofmt
+    - errcheck
+    - dogsled
+    - exportloopref
     - goconst
-    - goimports
-    - golint
+    - gocritic
+    - gci
+    - gofumpt
     - gosec
     - gosimple
     - govet
     - ineffassign
     - misspell
     - nakedret
-    - prealloc
-    - scopelint
+    - nolintlint
     - staticcheck
+    - revive
     - stylecheck
     - typecheck
+    - thelper
     - unconvert
     - unused
-    - unparam
-    - misspell
-  disable:
-    - gocritic
-    - maligned
-    - errcheck
-    - interfacer
-    - wsl
-    - nolintlint
 
 issues:
   exclude-rules:
@@ -52,16 +60,108 @@ issues:
     - text: "SA1019: collection."
       linters:
         - staticcheck
+    - path: "legacy"
+      text: "SA1019:"
+      linters:
+        - staticcheck
+    - text: "SA1019: codec.NewAminoCodec is deprecated" # TODO remove once migration path is set out
+      linters:
+        - staticcheck
+    - text: "SA1019: legacybech32.MustMarshalPubKey" # TODO remove once ready to remove from the sdk
+      linters:
+        - staticcheck
+    - text: "SA1019: legacybech32.MarshalPubKey" # TODO remove once ready to remove from the sdk
+      linters:
+        - staticcheck
+    - text: "SA1019: legacybech32.UnmarshalPubKey" # TODO remove once ready to remove from the sdk
+      linters:
+        - staticcheck
+    - text: "SA1019: params.SendEnabled is deprecated" # TODO remove once ready to remove from the sdk
+      linters:
+        - staticcheck
+    - text: "SA1019: \"github.com/golang/protobuf/proto\" is deprecated" # TODO remove once ready to remove from the sdk
+      linters:
+        - staticcheck
+    - text: "SA1019: vote.Option is deprecated" # TODO remove once ready to remove from the sdk
+      linters:
+        - staticcheck
+    - text: "SA1019: types.QueryNextAccountNumberRequest is deprecated" # TODO remove once ready to remove from the sdk
+      linters:
+        - staticcheck
+    - text: "leading space"
+      linters:
+        - nolintlint
+    - path: _test\.go
+      linters:
+        - goconst
   max-issues-per-linter: 10000
   max-same-issues: 10000
 
 linters-settings:
+  gci:
+    custom-order: true
+    sections:
+      - standard # Standard section: captures all standard packages.
+      - default # Default section: contains all imports that could not be matched to another section type.
+      - prefix(github.com/Finschia/ostracon)
+      - prefix(github.com/Finschia/finschia-sdk)
+  revive:
+    rules:
+      - name: redefines-builtin-id
+        disabled: true
+
+  gosec:
+    includes:
+      #  - G101 # Look for hard coded credentials
+      - G102 # Bind to all interfaces
+      - G103 # Audit the use of unsafe block
+      - G104 # Audit errors not checked
+      - G106 # Audit the use of ssh.InsecureIgnoreHostKey
+      - G107 # Url provided to HTTP request as taint input
+      - G108 # Profiling endpoint automatically exposed on /debug/pprof
+      - G109 # Potential Integer overflow made by strconv.Atoi result conversion to int16/32
+      - G110 # Potential DoS vulnerability via decompression bomb
+      - G111 # Potential directory traversal
+      - G112 # Potential slowloris attack
+      - G113 # Usage of Rat.SetString in math/big with an overflow (CVE-2022-23772)
+      - G114 # Use of net/http serve function that has no support for setting timeouts
+      - G201 # SQL query construction using format string
+      - G202 # SQL query construction using string concatenation
+      - G203 # Use of unescaped data in HTML templates
+      - G204 # Audit use of command execution
+      - G301 # Poor file permissions used when creating a directory
+      - G302 # Poor file permissions used with chmod
+      - G303 # Creating tempfile using a predictable path
+      - G304 # File path provided as taint input
+      - G305 # File traversal when extracting zip/tar archive
+      - G306 # Poor file permissions used when writing to a new file
+      - G307 # Deferring a method which returns an error
+      - G401 # Detect the usage of DES, RC4, MD5 or SHA1
+      - G402 # Look for bad TLS connection settings
+      - G403 # Ensure minimum RSA key length of 2048 bits
+      - G404 # Insecure random number source (rand)
+      - G501 # Import blocklist: crypto/md5
+      - G502 # Import blocklist: crypto/des
+      - G503 # Import blocklist: crypto/rc4
+      - G504 # Import blocklist: net/http/cgi
+      - G505 # Import blocklist: crypto/sha1
+      - G601 # Implicit memory aliasing of items from a range statement
+  misspell:
+    locale: US
+  gofumpt:
+    extra-rules: true
   dogsled:
-    max-blank-identifiers: 3
+    max-blank-identifiers: 6
   maligned:
-    # print struct with more effective memory layout or not, false by default
     suggest-new: true
   nolintlint:
     allow-unused: false
     require-explanation: false
-    require-specific: false
+    require-specific: false
+  gosimple:
+    checks: ["all"]
+  gocritic:
+    disabled-checks:
+      - regexpMust
+      - appendAssign
+      - ifElseChain

baseapp/abci.go

@@ -12,23 +12,23 @@ import (
 	"time"
 
 	"github.com/gogo/protobuf/proto"
+	abci "github.com/tendermint/tendermint/abci/types"
 	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
 	"google.golang.org/grpc/codes"
 	grpcstatus "google.golang.org/grpc/status"
 
-	abci "github.com/tendermint/tendermint/abci/types"
+	ocabci "github.com/Finschia/ostracon/abci/types"
 
 	"github.com/Finschia/finschia-sdk/codec"
 	snapshottypes "github.com/Finschia/finschia-sdk/snapshots/types"
 	"github.com/Finschia/finschia-sdk/telemetry"
 	sdk "github.com/Finschia/finschia-sdk/types"
 	sdkerrors "github.com/Finschia/finschia-sdk/types/errors"
-	ocabci "github.com/Finschia/ostracon/abci/types"
 )
 
 // InitChain implements the ABCI interface. It runs the initialization logic
 // directly on the CommitMultiStore.
-func (app *BaseApp) InitChain(req abci.RequestInitChain) (res abci.ResponseInitChain) {
+func (app *BaseApp) InitChain(req abci.RequestInitChain) abci.ResponseInitChain {
 	// On a new chain, we consider the init chain block height as 0, even though
 	// req.InitialHeight is 1 by default.
 	initHeader := tmproto.Header{ChainID: req.ChainId, Time: req.Time}
@@ -56,13 +56,13 @@ func (app *BaseApp) InitChain(req abci.RequestInitChain) (res abci.ResponseInitC
 	}
 
 	if app.initChainer == nil {
-		return
+		return abci.ResponseInitChain{}
 	}
 
 	// add block gas meter for any genesis transactions (allow infinite gas)
 	app.deliverState.ctx = app.deliverState.ctx.WithBlockGasMeter(sdk.NewInfiniteGasMeter())
 
-	res = app.initChainer(app.deliverState.ctx, req)
+	res := app.initChainer(app.deliverState.ctx, req)
 
 	// sanity check
 	if len(req.Validators) > 0 {
@@ -925,7 +925,6 @@ func splitPath(requestPath string) (path []string) {
 // createQueryContext creates a new sdk.Context for a query, taking as args
 // the block height and whether the query needs a proof or not.
 func (app *BaseApp) createQueryContextWithCheckState() sdk.Context {
-
 	cacheMS := app.checkState.CacheMultiStore()
 
 	// branch the commit-multistore for safety

baseapp/abci_test.go

@@ -140,7 +140,8 @@ func TestBaseAppCreateQueryContext(t *testing.T) {
 	db := dbm.NewMemDB()
 	name := t.Name()
 	app := NewBaseApp(name, logger, db, nil)
-	app.init()
+	err := app.init()
+	require.NoError(t, err)
 
 	app.BeginBlock(ocabci.RequestBeginBlock{Header: tmproto.Header{Height: 1}})
 	app.Commit()
@@ -190,7 +191,8 @@ func TestBaseAppBeginBlockConsensusParams(t *testing.T) {
 			},
 		},
 	})
-	app.init()
+	err := app.init()
+	require.NoError(t, err)
 
 	// set block params
 	app.BeginBlock(ocabci.RequestBeginBlock{Header: tmproto.Header{Height: 1}})
@@ -218,7 +220,10 @@ func (ps *paramStore) Set(_ sdk.Context, key []byte, value interface{}) {
 		panic(err)
 	}
 
-	ps.db.Set(key, bz)
+	err = ps.db.Set(key, bz)
+	if err != nil {
+		panic(err)
+	}
 }
 
 func (ps *paramStore) Has(_ sdk.Context, key []byte) bool {

baseapp/baseapp.go

@@ -8,14 +8,13 @@ import (
 	"sync"
 
 	"github.com/gogo/protobuf/proto"
-
 	abci "github.com/tendermint/tendermint/abci/types"
 	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
+	dbm "github.com/tendermint/tm-db"
 
 	ocabci "github.com/Finschia/ostracon/abci/types"
 	"github.com/Finschia/ostracon/crypto/tmhash"
 	"github.com/Finschia/ostracon/libs/log"
-	dbm "github.com/tendermint/tm-db"
 
 	"github.com/Finschia/finschia-sdk/codec/types"
 	"github.com/Finschia/finschia-sdk/server/config"
@@ -667,7 +666,6 @@ func (app *BaseApp) anteTx(ctx sdk.Context, txBytes []byte, tx sdk.Tx, simulate
 	anteCtx, msCache := app.cacheTxContext(ctx, txBytes)
 	anteCtx = anteCtx.WithEventManager(sdk.NewEventManager())
 	newCtx, err := app.anteHandler(anteCtx, tx, simulate)
-
 	if err != nil {
 		return newCtx, err
 	}

baseapp/baseapp_test.go

@@ -63,6 +63,7 @@ func aminoTxEncoder() sdk.TxEncoder {
 
 // simple one store baseapp
 func setupBaseApp(t *testing.T, options ...func(*BaseApp)) *BaseApp {
+	t.Helper()
 	app := newBaseApp(t.Name(), options...)
 	require.Equal(t, t.Name(), app.Name())
 
@@ -132,6 +133,7 @@ func TestLoadVersionPruning(t *testing.T) {
 }
 
 func testLoadVersionHelper(t *testing.T, app *BaseApp, expectedHeight int64, expectedID sdk.CommitID) {
+	t.Helper()
 	lastHeight := app.LastBlockHeight()
 	lastID := app.LastCommitID()
 	require.Equal(t, expectedHeight, lastHeight)
@@ -163,13 +165,6 @@ func TestGetMaximumBlockGas(t *testing.T) {
 }
 
 func TestListSnapshots(t *testing.T) {
-	type setupConfig struct {
-		blocks            uint64
-		blockTxs          int
-		snapshotInterval  uint64
-		snapshotKeepEvery uint32
-	}
-
 	app, _ := setupBaseAppWithSnapshots(t, 2, 5)
 
 	expected := abci.ResponseListSnapshots{Snapshots: []*abci.Snapshot{
@@ -222,7 +217,7 @@ func TestSetChanCheckTxSize(t *testing.T) {
 	logger := defaultLogger()
 	db := dbm.NewMemDB()
 
-	var size = uint(100)
+	size := uint(100)
 
 	app := NewBaseApp(t.Name(), logger, db, nil, SetChanCheckTxSize(size))
 	require.Equal(t, int(size), cap(app.chCheckTx))

baseapp/block_gas_test.go

@@ -142,7 +142,7 @@ func TestBaseApp_BlockGas(t *testing.T) {
 	}
 }
 
-func createTestTx(txConfig client.TxConfig, txBuilder client.TxBuilder, privs []cryptotypes.PrivKey, accNums []uint64, accSeqs []uint64, chainID string) (xauthsigning.Tx, []byte, error) {
+func createTestTx(txConfig client.TxConfig, txBuilder client.TxBuilder, privs []cryptotypes.PrivKey, accNums, accSeqs []uint64, chainID string) (xauthsigning.Tx, []byte, error) {
 	// First round: we gather all the signer infos. We use the "set empty
 	// signature" hack to do that.
 	var sigsV2 []signing.SignatureV2