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

chore: use the nested if's linter to reduce complexity globally #1158

Closed
wants to merge 23 commits into from
Closed
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
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
18 changes: 5 additions & 13 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,17 @@ linters:
- ineffassign
- misspell
- nakedret
- nestif
- staticcheck
- thelper
- typecheck
- stylecheck
- revive
# - testifylint
- typecheck
- tenv
- unconvert
- unparam # Prefer unparam over revive's unused param. It is more thorough in its checking.
- unparam
- unused
- misspell

Expand Down Expand Up @@ -62,10 +64,6 @@ linters-settings:
# 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:
- name: use-any
disabled: true
- name: if-return
disabled: true
- name: max-public-structs
disabled: true
- name: unchecked-type-assertion
Expand Down Expand Up @@ -94,27 +92,21 @@ linters-settings:
disabled: true
- name: empty-lines
disabled: true
- name: unused-receiver # remove later
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
- name: unused-parameter # Disabled in favour of unparam.
- name: unused-parameter
disabled: true
- name: unhandled-error # enable later, currently covered by errcheck
- name: unhandled-error
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The explicit disabling of the unhandled-error rule with specific arguments indicates a targeted approach to ignore unhandled errors from certain functions. While this can be useful for reducing noise from known benign unhandled errors, it's important to ensure that this does not inadvertently mask genuine issues. Consider reviewing these exceptions periodically to ensure they remain valid.

Consider adding a comment explaining the rationale behind each ignored function to ensure future maintainability and understanding of these exceptions.

disabled: true
arguments:
- 'b.WriteString'
Expand Down
2 changes: 1 addition & 1 deletion app/test_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import (
type EmptyAppOptions struct{}

// Get implements AppOptions.
func (EmptyAppOptions) Get(_ string) interface{} {
func (EmptyAppOptions) Get(_ string) any {
return nil
}

Expand Down
2 changes: 1 addition & 1 deletion app/tps_counter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ type writerLogger struct {

var _ log.Logger = (*writerLogger)(nil)

func (wl *writerLogger) Info(msg string, keyVals ...interface{}) {
func (wl *writerLogger) Info(msg string, keyVals ...any) {
wl.mu.Lock()
defer wl.mu.Unlock()

Expand Down
2 changes: 1 addition & 1 deletion cmd/quicksilverd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ func txCommand() *cobra.Command {

// initAppConfig helps to override default appConfig template and configs.
// return "", nil if no custom configuration is required for the application.
func initAppConfig() (string, interface{}) {
func initAppConfig() (string, any) {
customAppTemplate, customAppConfig := servercfg.AppConfig(quicksilverconfig.BaseDenom)

srvCfg, ok := customAppConfig.(servercfg.Config)
Expand Down
2 changes: 1 addition & 1 deletion icq-relayer/pkg/runner/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,7 @@ func doRequest(query Query, logger log.Logger, metrics prommetrics.Metrics) {
// does not contain the Tx object (that we don't use, because the TxProof already contains a byte representation of tx anyway!)
// Note: this function is compatible with 0.34 and 0.37 representations of transactions.
func Tx(client *lensclient.ChainClient, hash []byte) (tmtypes.TxProof, int64, error) {
params := map[string]interface{}{
params := map[string]any{
"hash": hash,
"prove": true,
}
Expand Down
2 changes: 1 addition & 1 deletion server/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ type SupplyConfig struct {

// AppConfig helps to override default appConfig template and configs.
// return "", nil if no custom configuration is required for the application.
func AppConfig(denom string) (customAppTemplate string, customAppConfig interface{}) {
func AppConfig(denom string) (customAppTemplate string, customAppConfig any) {
// Optionally allow the chain developer to overwrite the SDK's default
// server config.
srvCfg := config.DefaultConfig()
Expand Down
2 changes: 1 addition & 1 deletion test/simulation/simtypes/rand.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ func RandLTEBound[T constraints.Integer](r *rand.Rand, upperbound T) T {
return t
}

func RandSelect[T interface{}](r *rand.Rand, args ...T) T {
func RandSelect[T any](r *rand.Rand, args ...T) T {
choice := RandLTBound(r, len(args))
return args[choice]
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func (pa PoolAsset) prettify() poolAssetPretty {

// MarshalYAML returns the YAML representation of a PoolAsset.
// This is assumed to not be called on a stand-alone instance, so it removes the first marshalled line.
func (pa PoolAsset) MarshalYAML() (interface{}, error) {
func (pa PoolAsset) MarshalYAML() (any, error) {
bz, err := yaml.Marshal(pa.prettify())
if err != nil {
return nil, err
Expand Down
2 changes: 1 addition & 1 deletion third-party-chains/osmosis-types/osmomath/decimal.go
Original file line number Diff line number Diff line change
Expand Up @@ -700,7 +700,7 @@ func (d *BigDec) UnmarshalJSON(bz []byte) error {
}

// MarshalYAML returns the YAML representation.
func (d BigDec) MarshalYAML() (interface{}, error) {
func (d BigDec) MarshalYAML() (any, error) {
return d.String(), nil
}

Expand Down
2 changes: 1 addition & 1 deletion third-party-chains/osmosis-types/osmomath/int.go
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ func unmarshalJSON(i *big.Int, bz []byte) error {
}

// MarshalYAML returns the YAML representation.
func (i BigInt) MarshalYAML() (interface{}, error) {
func (i BigInt) MarshalYAML() (any, error) {
return i.String(), nil
}

Expand Down
2 changes: 1 addition & 1 deletion third-party-chains/osmosis-types/osmoutils/cache_ctx.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func ApplyFuncIfNoError(ctx sdk.Context, f func(ctx sdk.Context) error) (err err

// PrintPanicRecoveryError error logs the recoveryError, along with the stacktrace, if it can be parsed.
// If not emits them to stdout.
func PrintPanicRecoveryError(ctx sdk.Context, recoveryError interface{}) {
func PrintPanicRecoveryError(ctx sdk.Context, recoveryError any) {
errStackTrace := string(debug.Stack())
switch e := recoveryError.(type) {
case string:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ func SortSlice[T constraints.Ordered](s []T) {
})
}

func Filter[T interface{}](filter func(T) bool, s []T) []T {
func Filter[T any](filter func(T) bool, s []T) []T {
filteredSlice := []T{}
for _, s := range s {
if filter(s) {
Expand Down
10 changes: 5 additions & 5 deletions third-party-chains/umee-types/leverage/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func (p Params) Validate() error {
return validateDirectLiquidationFee(p.DirectLiquidationFee)
}

func validateLiquidationThreshold(i interface{}) error {
func validateLiquidationThreshold(i any) error {
v, ok := i.(sdk.Dec)
if !ok {
return fmt.Errorf("invalid parameter type: %T", i)
Expand All @@ -108,7 +108,7 @@ func validateLiquidationThreshold(i interface{}) error {
return nil
}

func validateMinimumCloseFactor(i interface{}) error {
func validateMinimumCloseFactor(i any) error {
v, ok := i.(sdk.Dec)
if !ok {
return fmt.Errorf("invalid parameter type: %T", i)
Expand All @@ -124,7 +124,7 @@ func validateMinimumCloseFactor(i interface{}) error {
return nil
}

func validateOracleRewardFactor(i interface{}) error {
func validateOracleRewardFactor(i any) error {
v, ok := i.(sdk.Dec)
if !ok {
return fmt.Errorf("invalid parameter type: %T", i)
Expand All @@ -140,7 +140,7 @@ func validateOracleRewardFactor(i interface{}) error {
return nil
}

func validateSmallLiquidationSize(i interface{}) error {
func validateSmallLiquidationSize(i any) error {
v, ok := i.(sdk.Dec)
if !ok {
return fmt.Errorf("invalid parameter type: %T", i)
Expand All @@ -153,7 +153,7 @@ func validateSmallLiquidationSize(i interface{}) error {
return nil
}

func validateDirectLiquidationFee(i interface{}) error {
func validateDirectLiquidationFee(i any) error {
v, ok := i.(sdk.Dec)
if !ok {
return fmt.Errorf("invalid parameter type: %T", i)
Expand Down
4 changes: 2 additions & 2 deletions utils/sort.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"golang.org/x/exp/constraints"
)

func Keys[V interface{}](in map[string]V) []string {
func Keys[V any](in map[string]V) []string {
out := make([]string, 0)

for k := range in {
Expand All @@ -27,7 +27,7 @@ func SortSlice[T constraints.Ordered](s []T) {
})
}

func Unique[V interface{}](in []V) []V {
func Unique[V any](in []V) []V {
keys := make(map[string]struct{}, len(in))
list := []V{}
for _, entry := range in {
Expand Down
15 changes: 6 additions & 9 deletions wasmbinding/message_plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,22 +38,19 @@ var _ wasmkeeper.Messenger = (*CustomMessenger)(nil)
// DispatchMsg executes on the contractMsg.
func (m *CustomMessenger) DispatchMsg(ctx sdk.Context, contractAddr sdk.AccAddress, contractIBCPortID string, msg wasmvmtypes.CosmosMsg) ([]sdk.Event, [][]byte, error) {
if msg.Custom != nil {
// only handle the happy path where this is really creating / minting / swapping ...
// leave everything else for the wrapped version
var contractMsg bindings.TokenFactoryMsg
if err := json.Unmarshal(msg.Custom, &contractMsg); err != nil {
return nil, nil, sdkioerrors.Wrap(err, "TokenFactoryMsg msg")
}
if contractMsg.CreateDenom != nil {

switch {
case contractMsg.CreateDenom != nil:
return m.createDenom(ctx, contractAddr, contractMsg.CreateDenom)
}
if contractMsg.MintTokens != nil {
case contractMsg.MintTokens != nil:
return m.mintTokens(ctx, contractAddr, contractMsg.MintTokens)
}
if contractMsg.ChangeAdmin != nil {
case contractMsg.ChangeAdmin != nil:
return m.changeAdmin(ctx, contractAddr, contractMsg.ChangeAdmin)
}
if contractMsg.BurnTokens != nil {
case contractMsg.BurnTokens != nil:
Comment on lines +45 to +53
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The refactoring to use a switch statement in the DispatchMsg method for handling different types of TokenFactoryMsg is a positive change that enhances the clarity and maintainability of the code. This approach makes it easier to understand the flow of logic and add new message types in the future. However, ensure that each case in the switch statement properly handles errors and returns meaningful messages to the caller. Additionally, consider adding default case handling to catch any unsupported message types, which could improve error reporting and robustness.

return m.burnTokens(ctx, contractAddr, contractMsg.BurnTokens)
}
}
Expand Down
4 changes: 2 additions & 2 deletions wasmbinding/query_plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func (s *StargateTestSuite) TestStargateQuerier() {
testSetup func()
path string
requestData func() []byte
responseProtoStruct interface{}
responseProtoStruct any
expectedQuerierError bool
expectedUnMarshalError bool
resendRequest bool
Expand Down Expand Up @@ -224,7 +224,7 @@ func (s *StargateTestSuite) TestDeterministicJsonMarshal() {
originalResponsebz []byte
updatedResponsebz []byte
queryPath string
responseProtoStruct interface{}
responseProtoStruct any
expectedProto func() proto.Message
}{
/*
Expand Down
2 changes: 1 addition & 1 deletion wasmbinding/test/custom_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ type ChainResponse struct {
Data []byte `json:"data"`
}

func queryCustom(t *testing.T, ctx sdk.Context, quicksilver *app.Quicksilver, contract sdk.AccAddress, request bindings.QuickSilverQuery, response interface{}) {
func queryCustom(t *testing.T, ctx sdk.Context, quicksilver *app.Quicksilver, contract sdk.AccAddress, request bindings.QuickSilverQuery, response any) {
t.Helper()

msgBz, err := json.Marshal(request)
Expand Down
84 changes: 38 additions & 46 deletions x/airdrop/keeper/claim_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -337,71 +337,63 @@ func (k *Keeper) completeClaim(ctx sdk.Context, cr *types.ClaimRecord, action ty
// after updating the relevant claim record.
func (k *Keeper) getClaimAmountAndUpdateRecord(ctx sdk.Context, cr *types.ClaimRecord, action types.Action) (uint64, error) {
var claimAmount uint64
var err error // Declare err here

// check and initialize ActionsCompleted map
if cr.ActionsCompleted == nil {
cr.ActionsCompleted = make(map[int32]*types.CompletedAction)
}

// The concept here is to intuitively claim all outstanding deposit tiers
// that are below the current deposit claim (improved user experience).
//
// ActionDepositT5: t5amount
// ActionDepositT4: t4amount
// ActionDepositT3: t3amount <-- eg. for T3
// ActionDepositT2: t2amount <-- add to claimAmount if not CompletedAction
// ActionDepositT1: t1amount <-- add to claimAmount if not CompletedAction
//
// For any given deposit action above ActionDepositT1, sum the claimable
// amounts of non completed deposit actions and mark them as complete.
// Then, if no errors occurred, update the ClaimRecord state.

// check for summable ActionDeposit (T2-T5, T1 has nothing below it to sum)
if action > types.ActionDepositT1 && action <= types.ActionDepositT5 {
// check ActionDeposits from T1 to the target tier
// this also ensures that for any completed ActionDeposit tier, all
// tiers below are guaranteed to be completed as well.
for a := types.ActionDepositT1; a <= action; a++ {
if _, exists := cr.ActionsCompleted[int32(a)]; !exists {
// obtain claimable amount per deposit action
claimable, err := k.GetClaimableAmountForAction(ctx, cr.ChainId, cr.Address, a)
if err != nil {
return 0, err
}

// update claim record (transient, not yet written to state)
cr.ActionsCompleted[int32(a)] = &types.CompletedAction{
CompleteTime: ctx.BlockTime(),
ClaimAmount: claimable,
}

// sum total claimable
claimAmount += claimable
}
claimAmount, err = k.processSummableDeposits(ctx, cr, action) // Use '=' instead of ':='
if err != nil {
return 0, err
}
} else {
// obtain claimable amount
claimable, err := k.GetClaimableAmountForAction(ctx, cr.ChainId, cr.Address, action)
claimAmount, err = k.processSingleAction(ctx, cr, action) // Use '=' instead of ':='
if err != nil {
return 0, err
}
}

if err := k.SetClaimRecord(ctx, *cr); err != nil { // This now checks the correct 'err'
return 0, err
}

return claimAmount, nil
}

// set claim amount
claimAmount = claimable
func (k *Keeper) processSummableDeposits(ctx sdk.Context, cr *types.ClaimRecord, action types.Action) (uint64, error) {
var claimAmount uint64
for a := types.ActionDepositT1; a <= action; a++ {
if _, exists := cr.ActionsCompleted[int32(a)]; !exists {
claimable, err := k.GetClaimableAmountForAction(ctx, cr.ChainId, cr.Address, a)
if err != nil {
return 0, err
}

// update claim record
cr.ActionsCompleted[int32(action)] = &types.CompletedAction{
CompleteTime: ctx.BlockTime(),
ClaimAmount: claimAmount,
cr.ActionsCompleted[int32(a)] = &types.CompletedAction{
CompleteTime: ctx.BlockTime(),
ClaimAmount: claimable,
}

claimAmount += claimable
}
}
return claimAmount, nil
}

// set claim record (persistent)
if err := k.SetClaimRecord(ctx, *cr); err != nil {
func (k *Keeper) processSingleAction(ctx sdk.Context, cr *types.ClaimRecord, action types.Action) (uint64, error) {
claimable, err := k.GetClaimableAmountForAction(ctx, cr.ChainId, cr.Address, action)
if err != nil {
return 0, err
}

return claimAmount, nil
cr.ActionsCompleted[int32(action)] = &types.CompletedAction{
CompleteTime: ctx.BlockTime(),
ClaimAmount: claimable,
}

return claimable, nil
}

func (k *Keeper) sendCoins(ctx sdk.Context, cr types.ClaimRecord, amount uint64) (sdk.Coins, error) {
Expand Down
Loading