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.

Sign up for GitHub

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

refactor: add unparam linter #4333

Merged
merged 10 commits into from
Aug 21, 2023
Merged

refactor: add unparam linter #4333

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
139c023
add unparam linter
faddat Aug 12, 2023
5d2502d
Merge remote-tracking branch 'origin/main' into faddat/unused-parameters
faddat Aug 14, 2023
1fb1355
Merge branch 'main' into faddat/unused-parameters
crodriguezvega Aug 15, 2023
bef98e1
Merge branch 'main' into faddat/unused-parameters
crodriguezvega Aug 15, 2023
d1f90d4
Merge branch 'main' into faddat/unused-parameters
faddat Aug 16, 2023
3e6df02
Merge branch 'main' into faddat/unused-parameters
faddat Aug 20, 2023
1cbbf46
remove unused-param from revive, add comments.
DimitrisJim Aug 21, 2023
429142c
Merge branch 'main' into faddat/unused-parameters
faddat Aug 21, 2023
d9f4e0a
Merge branch 'main' into faddat/unused-parameters
DimitrisJim Aug 21, 2023
6ef52d7
Merge branch 'main' into faddat/unused-parameters
crodriguezvega Aug 21, 2023
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
5 changes: 5 additions & 0 deletions .golangci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ linters:
- typecheck
- tenv
- unconvert
# Prefer unparam over revive's unused param. It is more thorough in its checking.
- unparam
Copy link
Contributor

Choose a reason for hiding this comment

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

you might have context on this. Looking through this configurations history, I see unparam originall existed and then was removed with #1418. Is it maybe the case that revive has similar functionality and we just don't include it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

turns out that, yes, but no :)

Unparam is way better (and different) from revive's version. Unparam will check more deeply to see if it is used anywhere, and then make recommendations based on that. Strongly recommend.

Copy link
Contributor

Choose a reason for hiding this comment

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

gotcha, would be nice to leave it as a comment since I do see a future where this is removed once again by accident.

- unused
- misspell

Expand Down Expand Up @@ -100,6 +102,9 @@ linters-settings:
disabled: true
- name: defer
disabled: true
# Disabled in favour of unparam.
- name: unused-parameter
disabled: true
- name: unhandled-error
disabled: false
arguments:
Expand Down
8 changes: 4 additions & 4 deletions modules/core/02-client/migrations/v7/store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,11 @@ func MigrateStore(ctx sdk.Context, storeKey storetypes.StoreKey, cdc codec.Binar
return err
}

if err := handleTendermintMigration(ctx, store, cdc, clientKeeper); err != nil {
if err := handleTendermintMigration(ctx, store, clientKeeper); err != nil {
return err
}

return handleLocalhostMigration(ctx, store, cdc, clientKeeper)
return handleLocalhostMigration(ctx, store, clientKeeper)
}

// handleSolomachineMigration iterates over the solo machine clients and migrates client state from
Expand Down Expand Up @@ -82,7 +82,7 @@ func handleSolomachineMigration(ctx sdk.Context, store storetypes.KVStore, cdc c

// handlerTendermintMigration asserts that the tendermint client in state can be decoded properly.
// This ensures the upgrading chain properly registered the tendermint client types on the chain codec.
func handleTendermintMigration(ctx sdk.Context, store storetypes.KVStore, cdc codec.BinaryCodec, clientKeeper ClientKeeper) error {
func handleTendermintMigration(ctx sdk.Context, store storetypes.KVStore, clientKeeper ClientKeeper) error {
clients, err := collectClients(ctx, store, exported.Tendermint)
if err != nil {
return err
Expand Down Expand Up @@ -114,7 +114,7 @@ func handleTendermintMigration(ctx sdk.Context, store storetypes.KVStore, cdc co
}

// handleLocalhostMigration removes all client and consensus states associated with the localhost client type.
func handleLocalhostMigration(ctx sdk.Context, store storetypes.KVStore, cdc codec.BinaryCodec, clientKeeper ClientKeeper) error {
func handleLocalhostMigration(ctx sdk.Context, store storetypes.KVStore, clientKeeper ClientKeeper) error {
clients, err := collectClients(ctx, store, Localhost)
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion modules/light-clients/06-solomachine/misbehaviour_handle.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func (ClientState) CheckForMisbehaviour(_ sdk.Context, _ codec.BinaryCodec, _ st
return false
}

func (cs ClientState) verifyMisbehaviour(ctx sdk.Context, cdc codec.BinaryCodec, clientStore storetypes.KVStore, misbehaviour *Misbehaviour) error {
func (cs ClientState) verifyMisbehaviour(cdc codec.BinaryCodec, misbehaviour *Misbehaviour) error {
// NOTE: a check that the misbehaviour message data are not equal is done by
// misbehaviour.ValidateBasic which is called by the 02-client keeper.
// verify first signature
Expand Down
6 changes: 3 additions & 3 deletions modules/light-clients/06-solomachine/update.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,15 @@ import (
func (cs ClientState) VerifyClientMessage(ctx sdk.Context, cdc codec.BinaryCodec, clientStore storetypes.KVStore, clientMsg exported.ClientMessage) error {
switch msg := clientMsg.(type) {
case *Header:
return cs.verifyHeader(ctx, cdc, clientStore, msg)
return cs.verifyHeader(cdc, msg)
case *Misbehaviour:
return cs.verifyMisbehaviour(ctx, cdc, clientStore, msg)
return cs.verifyMisbehaviour(cdc, msg)
default:
return errorsmod.Wrapf(clienttypes.ErrInvalidClientType, "expected type of %T or %T, got type %T", Header{}, Misbehaviour{}, msg)
}
}

func (cs ClientState) verifyHeader(ctx sdk.Context, cdc codec.BinaryCodec, clientStore storetypes.KVStore, header *Header) error {
func (cs ClientState) verifyHeader(cdc codec.BinaryCodec, header *Header) error {
// assert update timestamp is not less than current consensus state timestamp
if header.Timestamp < cs.ConsensusState.Timestamp {
return errorsmod.Wrapf(
Expand Down