chore: turbo lint (#368)

* update linter * begin lint * linting * finalize * timeout * revert * revert * properly use %q * regenerate proto * fix test --------- Co-authored-by: aljo242 <[email protected]>
quicksilver-zone · Mar 30, 2023 · 00907da · 00907da
1 parent 8f9ddd9
commit 00907da
Show file tree

Hide file tree

Showing 227 changed files with 3,326 additions and 3,015 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -1,38 +1,152 @@
 run:
-  tests: false
+  tests: true
   # timeout for analysis, e.g. 30s, 5m, default is 1m
-  timeout: 5m
+  timeout: 10m
   skip-dirs:
     - osmosis-types/*
 
 linters:
   disable-all: true
   enable:
+    # Check for pass []any as any in variadic func(...any).
+      # Rare case but saved me from debugging a few times.
+    - asasalint
+    # I prefer plane ASCII identifiers.
+    # Symbol `∆` instead of `delta` looks cool but no thanks.
+    - asciicheck
+    # Checks for dangerous unicode character sequences.
+    # Super rare but why not to be a bit paranoid?
+    - bidichk
     - bodyclose
+    # Check whether the function uses a non-inherited context.
+    - contextcheck
+    # Check for two durations multiplied together.
+    - durationcheck
     - depguard
     - dogsled
     - errcheck
+    # Checks `Err-` prefix for var and `-Error` suffix for error type.
+    - errname
+    # Suggests to use `%w` for error-wrapping.
+    - errorlint
+    # Checks for pointers to enclosing loop variables.
+    - exportloopref
     - goconst
     - gocritic
+    # Forces to put `.` at the end of the comment. Code is poetry.
+    - godot
+    # Might not be that important, but I prefer to keep all of them.
+    # `gofumpt` is amazing, kudos to Daniel Marti https://github.com/mvdan/gofumpt
+    - gofmt
     - gofumpt
-    - revive
+    - goimports
     - gosec
     - gosimple
     - govet
     - ineffassign
     - misspell
     - nakedret
-    - exportloopref
+    # Finds sending HTTP request without context.Context.
+    - noctx
+    # Finds slices that could potentially be pre-allocated.
+    # Small performance win + cleaner code.
+    - prealloc
+    # Finds shadowing of Go's predeclared identifiers.
+    # I hear a lot of complaints from junior developers.
+    # But after some time they find it very useful.
+    - predeclared
+    # Lint your Prometheus metrics name.
+    - promlinter
+    # Checks that package variables are not reassigned.
+    # Super rare case but can catch bad things (like `io.EOF = nil`)
+    - reassign
+    - revive
     - staticcheck
     - stylecheck
+    # Checks that package variables are not reassigned.
+    # Super rare case but can catch bad things (like `io.EOF = nil`)
+    - reassign
     - typecheck
+    # Test-related checks. All of them are good.
+    - tenv
+    - testableexamples
+    - thelper
+    - tparallel
     - unconvert
     - unused
     - unparam
-    - misspell
-    - nolintlint
+    # Detect the possibility to use variables/constants from stdlib.
+    - usestdlibvars
+    # Finds wasted assignment statements.
+    - wastedassign
 
 exclude-rules:
   - linters:
       - nolintlint
-    text: "should be written without leading space"
+    text:
+    - "should be written without leading space"
+
+issues:
+  exclude-rules:
+    # Exclude some linters from running on tests files.
+    - path: _test\.go
+      linters:
+        - gocyclo
+        - errcheck
+        - dupl
+        - gosec
+
+linters-settings:
+  # I'm biased and I'm enabling more than 100 checks
+  # Might be too much for you. See https://go-critic.com/overview.html
+  gocritic:
+    enabled-tags:
+      - diagnostic
+      - experimental
+      - opinionated
+      - performance
+      - style
+    disabled-checks:
+      - hugeParam
+      - rangeExprCopy
+      - rangeValCopy
+      - timeCmpSimplify
+      - unlabelStmt
+      - unnamedResult
+      - tooManyResultsChecker
+      - whyNoLint
+      - commentedOutCode
+
+  gosec:
+    excludes:
+      - G306
+
+  errcheck:
+    # Report `a := b.(MyStruct)` when `a, ok := ...` should be.
+    check-type-assertions: true # Default: false
+
+
+    # Function to skip.
+    exclude-functions:
+      - io/ioutil.ReadFile
+      - io.Copy(*bytes.Buffer)
+      - io.Copy(os.Stdout)
+
+  govet:
+    disable:
+      - fieldalignment # I'm ok to waste some bytes
+
+
+  nakedret:
+    # No naked returns, ever.
+    max-func-lines: 1 # Default: 30
+
+  tagliatelle:
+    case:
+      rules:
+        json: snake # why it's not a `snake` by default?!
+        yaml: snake # why it's not a `snake` by default?!
+        xml: camel
+        bson: camel
+        avro: snake
+        mapstructure: kebab
diff --git a/app/app.go b/app/app.go
@@ -54,15 +54,15 @@ func Init() {
 }
 
 const (
-	// Name defines the application binary name
+	// Name defines the application binary name.
 	Name = "quicksilverd"
 )
 
 var (
-	// DefaultNodeHome default home directories for the application daemon
+	// DefaultNodeHome default home directories for the application daemon.
 	DefaultNodeHome string
 
-	// module accounts that are allowed to receive tokens
+	// module accounts that are allowed to receive tokens.
 	allowedReceivingModAcc = map[string]bool{
 		distrtypes.ModuleName:             true,
 		interchainstakingtypes.ModuleName: true,
@@ -227,16 +227,16 @@ func NewQuicksilver(
 	go func() {
 		// Unfortunately golangci-lint is so pedantic
 		// so we have to ignore this error explicitly.
-		_ = app.tpsCounter.start(context.Background())
+		_ = app.tpsCounter.start(context.Background()) //nolint:errcheck
 	}()
 
 	return app
 }
 
-// Name returns the name of the App
+// Name returns the name of the App.
 func (app *Quicksilver) Name() string { return app.BaseApp.Name() }
 
-// BeginBlocker updates every begin block
+// BeginBlocker updates every begin block.
 func (app *Quicksilver) BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock) abci.ResponseBeginBlock {
 	if ctx.ChainID() == "quicksilver-2" && ctx.BlockHeight() == 235001 {
 		zone, found := app.InterchainstakingKeeper.GetZone(ctx, "stargaze-1")
@@ -249,12 +249,12 @@ func (app *Quicksilver) BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock
 	return app.mm.BeginBlock(ctx, req)
 }
 
-// EndBlocker updates every end block
+// EndBlocker updates every end block.
 func (app *Quicksilver) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) abci.ResponseEndBlock {
 	return app.mm.EndBlock(ctx, req)
 }
 
-// We are intentionally decomposing the DeliverTx method so as to calculate the transactions per second.
+// DeliverTx calls BaseApp.DeliverTx and calculates transactions per second.
 func (app *Quicksilver) DeliverTx(req abci.RequestDeliverTx) (res abci.ResponseDeliverTx) {
 	defer func() {
 		// TODO: Record the count along with the code and or reason so as to display
@@ -269,7 +269,7 @@ func (app *Quicksilver) DeliverTx(req abci.RequestDeliverTx) (res abci.ResponseD
 	return app.BaseApp.DeliverTx(req)
 }
 
-// InitChainer updates at chain initialization
+// InitChainer updates at chain initialization.
 func (app *Quicksilver) InitChainer(ctx sdk.Context, req abci.RequestInitChain) abci.ResponseInitChain {
 	var genesisState GenesisState
 	if err := json.Unmarshal(req.AppStateBytes, &genesisState); err != nil {
@@ -279,7 +279,7 @@ func (app *Quicksilver) InitChainer(ctx sdk.Context, req abci.RequestInitChain)
 	return app.mm.InitGenesis(ctx, app.appCodec, genesisState)
 }
 
-// LoadHeight loads state at a particular height
+// LoadHeight loads state at a particular height.
 func (app *Quicksilver) LoadHeight(height int64) error {
 	return app.LoadVersion(height)
 }
@@ -321,7 +321,7 @@ func (app *Quicksilver) AppCodec() codec.Codec {
 	return app.appCodec
 }
 
-// InterfaceRegistry returns Quicksilver's InterfaceRegistry
+// InterfaceRegistry returns Quicksilver's InterfaceRegistry.
 func (app *Quicksilver) InterfaceRegistry() types.InterfaceRegistry {
 	return app.interfaceRegistry
 }
@@ -334,7 +334,7 @@ func (app *Quicksilver) GetSubspace(moduleName string) paramstypes.Subspace {
 	return subspace
 }
 
-// SimulationManager implements the SimulationApp interface
+// SimulationManager implements the SimulationApp interface.
 func (app *Quicksilver) SimulationManager() *module.SimulationManager {
 	return app.sm
 }
@@ -400,7 +400,7 @@ func (app *Quicksilver) GetTxConfig() client.TxConfig {
 	return cfg.TxConfig
 }
 
-// GetMaccPerms returns a copy of the module account permissions
+// GetMaccPerms returns a copy of the module account permissions.
 func GetMaccPerms() map[string][]string {
 	dupMaccPerms := make(map[string][]string)
 	for k, v := range maccPerms {
@@ -421,9 +421,9 @@ func GetWasmOpts(appOpts servertypes.AppOptions) []wasm.Option {
 	return wasmOpts
 }
 
-const (
+// PROPOSALS
 
-	// PROPOSALS
+const (
 	ProposalsEnabled = "true"
 	// If set to non-empty string it must be comma-separated list of values that are all a subset
 	// of "EnableAllProposals" (takes precedence over ProposalsEnabled)

diff --git a/app/app_test.go b/app/app_test.go
@@ -23,7 +23,8 @@ import (
 
 func TestQuicksilverExport(t *testing.T) {
 	privVal := mock.NewPV()
-	pubKey, _ := privVal.GetPubKey()
+	pubKey, err := privVal.GetPubKey()
+	require.NoError(t, err)
 
 	// create validator set with single validator
 	validator := tmtypes.NewValidator(pubKey, 1)

diff --git a/app/export.go b/app/export.go
@@ -50,9 +50,9 @@ func (app *Quicksilver) ExportAppStateAndValidators(
 	}, nil
 }
 
-// prepare for fresh start at zero height
+// prepForZeroHeightGenesis prepares for fresh start at zero height
 // NOTE zero height genesis is a temporary feature which will be deprecated
-// in favor of export at a block height
+// in favor of export at a block height.
 func (app *Quicksilver) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs []string) error {
 	applyAllowedAddrs := false
 
@@ -79,7 +79,7 @@ func (app *Quicksilver) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAdd
 	// withdraw all validator commission
 	// withdraw all validator commission
 	app.StakingKeeper.IterateValidators(ctx, func(_ int64, val stakingtypes.ValidatorI) (stop bool) {
-		_, _ = app.DistrKeeper.WithdrawValidatorCommission(ctx, val.GetOperator())
+		_, _ = app.DistrKeeper.WithdrawValidatorCommission(ctx, val.GetOperator()) //nolint
 		return false
 	})
 	// withdraw all delegator rewards
@@ -94,7 +94,10 @@ func (app *Quicksilver) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAdd
 		if err != nil {
 			return err
 		}
-		_, _ = app.DistrKeeper.WithdrawDelegationRewards(ctx, delAddr, valAddr)
+		_, err = app.DistrKeeper.WithdrawDelegationRewards(ctx, delAddr, valAddr)
+		if err != nil {
+			return err
+		}
 	}
 
 	// clear validator slash events

diff --git a/app/modules.go b/app/modules.go
@@ -106,7 +106,7 @@ var (
 		wasm.AppModuleBasic{},
 	)
 
-	// module account permissions
+	// module account permissions.
 	maccPerms = map[string][]string{
 		authtypes.FeeCollectorName:                 nil,
 		distrtypes.ModuleName:                      nil,
@@ -174,7 +174,7 @@ func appModules(
 }
 
 // simulationModules returns modules for simulation manager
-// define the order of the modules for deterministic simulations
+// define the order of the modules for deterministic simulations.
 func simulationModules(
 	app *Quicksilver,
 	encodingConfig EncodingConfig,
@@ -263,7 +263,7 @@ Interchain Security Requirements:
 thus, staking.EndBlock must be executed before provider.EndBlock;
 - creating a new consumer chain requires the following order,
 CreateChildClient(), staking.EndBlock, provider.EndBlock;
-thus, gov.EndBlock must be executed before staking.EndBlock
+thus, gov.EndBlock must be executed before staking.EndBlock.
 */
 func orderEndBlockers() []string {
 	return []string{

diff --git a/app/test_helpers.go b/app/test_helpers.go
@@ -27,10 +27,10 @@ import (
 	dbm "github.com/tendermint/tm-db"
 )
 
-// EmptyAppOptions is a stub implementing AppOptions
+// EmptyAppOptions is a stub implementing AppOptions.
 type EmptyAppOptions struct{}
 
-// Get implements AppOptions
+// Get implements AppOptions.
 func (ao EmptyAppOptions) Get(_ string) interface{} {
 	return nil
 }
@@ -56,8 +56,11 @@ var DefaultConsensusParams = &abci.ConsensusParams{
 
 // Setup initializes a new Quicksilver. A Nop logger is set in Quicksilver.
 func Setup(t *testing.T, isCheckTx bool) *Quicksilver {
+	t.Helper()
+
 	privVal := mock.NewPV()
-	pubKey, _ := privVal.GetPubKey()
+	pubKey, err := privVal.GetPubKey()
+	require.NoError(t, err)
 
 	// create validator set with single validator
 	validator := tmtypes.NewValidator(pubKey, 1)
@@ -115,13 +118,15 @@ func Setup(t *testing.T, isCheckTx bool) *Quicksilver {
 }
 
 func GetAppWithContext(t *testing.T, init bool) (*Quicksilver, sdk.Context) {
+	t.Helper()
+
 	app := Setup(t, !init)
 	ctx := app.BaseApp.NewContext(false, tmproto.Header{Height: 1, ChainID: "mercury-1", Time: time.Now().UTC()})
 	return app, ctx
 }
 
-// SetupTestingApp initializes the IBC-go testing application
-func SetupTestingApp() (ibctesting.TestingApp, map[string]json.RawMessage) {
+// SetupTestingApp initializes the IBC-go testing application.
+func SetupTestingApp() (testApp ibctesting.TestingApp, genesisState map[string]json.RawMessage) {
 	db := dbm.NewMemDB()
 	app := NewQuicksilver(
 		log.NewNopLogger(),
@@ -146,6 +151,8 @@ func GenesisStateWithValSet(t *testing.T,
 	valSet *tmtypes.ValidatorSet, genAccs []authtypes.GenesisAccount,
 	balances ...banktypes.Balance,
 ) GenesisState {
+	t.Helper()
+
 	// set genesis accounts
 	authGenesis := authtypes.NewGenesisState(authtypes.DefaultParams(), genAccs)
 	genesisState[authtypes.ModuleName] = app.AppCodec().MustMarshalJSON(authGenesis)