Skip to content

Commit

Permalink
style: Improve code quality with new linters (#1414)
Browse files Browse the repository at this point in the history
* Use golangci-lint v1.59

* Bump golangci-lint ci from 1.55 to 1.59

* Add useful linters

* Apply copyloopvar

* Apply errchkjson, tenv

* Apply `wastedassign`

* Apply `errorlint`

(cherry picked from commit d60e44a)

# Conflicts:
#	.golangci.yml
#	CHANGELOG.md
#	Makefile
#	server/oc_cmds_test.go
#	server/util_test.go
#	x/foundation/client/testutil/tx.go
  • Loading branch information
tkxkd0159 authored and mergify[bot] committed Jun 18, 2024
1 parent 2350c1a commit 8aa19f3
Show file tree
Hide file tree
Showing 193 changed files with 315 additions and 628 deletions.
62 changes: 62 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
run:
<<<<<<< HEAD
tests: false
# # timeout for analysis, e.g. 30s, 5m, default is 1m
# timeout: 5m
Expand Down Expand Up @@ -31,6 +32,63 @@ linters:
- nolintlint

issues:
=======
tests: true
timeout: 15m
allow-parallel-runners: true
build-tags:
- ledger
- goleveldb
- test_ledger_mock

output:
sort-results: true

linters:
disable-all: true
enable:
- errcheck
- gosimple
- govet
- ineffassign
- staticcheck
- unused
- dogsled
- gosec
- gci
- gofumpt
- goconst
- gocritic
- nakedret
- nolintlint
- revive
- misspell
- stylecheck
- typecheck
- thelper
- unconvert
- asasalint
- asciicheck
- bidichk
- bodyclose
- copyloopvar
- errchkjson
- errorlint
- tenv
- wastedassign
- fatcontext

issues:
exclude-dirs:
- "testdata$"
exclude-files:
- server/grpc/gogoreflection/fix_registration.go
- "fix_registration.go"
- ".*\\.pb\\.go$"
- ".*\\.pb\\.gw\\.go$"
- ".*\\.pulsar\\.go$"
- crypto/keys/secp256k1/internal/*
>>>>>>> d60e44aa6 (style: Improve code quality with new linters (#1414))
exclude-rules:
- text: "Use of weak random number generator"
linters:
Expand Down Expand Up @@ -59,7 +117,11 @@ issues:

linters-settings:
dogsled:
<<<<<<< HEAD
max-blank-identifiers: 3
=======
max-blank-identifiers: 6
>>>>>>> d60e44aa6 (style: Improve code quality with new linters (#1414))
nolintlint:
allow-unused: false
require-explanation: false
Expand Down
31 changes: 31 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,37 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (x/fswap) [\#1415](https://github.com/Finschia/finschia-sdk/pull/1415) add more testcases for fswap module

### Bug Fixes
<<<<<<< HEAD
=======
* chore(deps) [\#1141](https://github.com/Finschia/finschia-sdk/pull/1141) Bump github.com/cosmos/ledger-cosmos-go from 0.12.2 to 0.13.2 to fix ledger signing issue
* (x/auth, x/slashing) [\#1179](https://github.com/Finschia/finschia-sdk/pull/1179) modify missing changes of converting to tendermint
* (x/auth) [#1274](https://github.com/Finschia/finschia-sdk/pull/1274) `ModuleAccount.Validate` now reports a nil `.BaseAccount` instead of panicking.
* (x/collection) [\#1276](https://github.com/Finschia/finschia-sdk/pull/1276) eliminates potential risk for Insufficient Sanity Check of tokenID in Genesis
* (x/foundation) [\#1277](https://github.com/Finschia/finschia-sdk/pull/1277) add init logic of foundation module accounts to InitGenesis in order to eliminate potential panic
* (x/collection, x/token) [\#1288](https://github.com/Finschia/finschia-sdk/pull/1288) use accAddress to compare in validatebasic function in collection & token modules
* (x/collection) [\#1268](https://github.com/Finschia/finschia-sdk/pull/1268) export x/collection params into genesis
* (x/collection) [\#1294](https://github.com/Finschia/finschia-sdk/pull/1294) reject NFT coins on FT APIs
* (sec) [\#1302](https://github.com/Finschia/finschia-sdk/pull/1302) remove map iteration non-determinism with keys + sorting
* (client) [\#1303](https://github.com/Finschia/finschia-sdk/pull/1303) fix possible overflow in BuildUnsignedTx
* (types) [\#1299](https://github.com/Finschia/finschia-sdk/pull/1299) add missing nil checks
* (x/staking) [\#1301](https://github.com/Finschia/finschia-sdk/pull/1301) Use bytes instead of string comparison in delete validator queue (backport cosmos/cosmos-sdk#12303)
* (x/gov) [\#1304](https://github.com/Finschia/finschia-sdk/pull/1304) fetch a failed proposal tally from proposal.FinalTallyResult in the gprc query
* (x/staking) [\#1306](https://github.com/Finschia/finschia-sdk/pull/1306) add validation for potential slashing evasion during re-delegation
* (client/keys) [#1312](https://github.com/Finschia/finschia-sdk/pull/1312) ignore error when key not found in `keys delete`
* (store) [\#1310](https://github.com/Finschia/finschia-sdk/pull/1310) fix app-hash mismatch if upgrade migration commit is interrupted(backport cosmos/cosmos-sdk#13530)
* (types) [\#1313](https://github.com/Finschia/finschia-sdk/pull/1313) fix correctly coalesce coins even with repeated denominations(backport cosmos/cosmos-sdk#13265)
* (x/crypto) [\#1316](https://github.com/Finschia/finschia-sdk/pull/1316) error if incorrect ledger public key (backport cosmos/cosmos-sdk#14460, cosmos/cosmos-sdk#19691)
* (x/auth) [#1319](https://github.com/Finschia/finschia-sdk/pull/1319) prevent signing from wrong key in multisig
* (x/mint, x/slashing) [\#1323](https://github.com/Finschia/finschia-sdk/pull/1323) add missing nil check for params validation
* (x/server) [\#1337](https://github.com/Finschia/finschia-sdk/pull/1337) fix panic when defining minimum gas config as `100stake;100uatom`. Use a `,` delimiter instead of `;`. Fixes the server config getter to use the correct delimiter (backport cosmos/cosmos-sdk#18537)
* (x/fbridge) [\#1361](https://github.com/Finschia/finschia-sdk/pull/1361) Fixes fbridge auth checking bug
* (x/fswap) [\#1365](https://github.com/Finschia/finschia-sdk/pull/1365) fix update swap keys for possibly overlapped keys(`(hello,world) should be different to (hel,loworld)`)
* (x/fswap, x/fbridge) [\#1378](https://github.com/Finschia/finschia-sdk/pull/1378) Fix bug where amino is not supported in fswap and fbridge
* (x/fswap) [\#1379](https://github.com/Finschia/finschia-sdk/pull/1379) add missing router registration
* (x/fswap) [\#1385](https://github.com/Finschia/finschia-sdk/pull/1385) add accidentally deleted event emissions(EventSetSwap, EventAddDenomMetadata)
* (x/fswap) [\#1392](https://github.com/Finschia/finschia-sdk/pull/1392) fix dummy denom coin data for test in fswap
* (style) [\#1414](https://github.com/Finschia/finschia-sdk/pull/1414) improve code quality with new linters
>>>>>>> d60e44aa6 (style: Improve code quality with new linters (#1414))
### Removed

Expand Down
4 changes: 4 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,11 @@ lint: golangci-lint
find . -name '*.go' -type f -not -path "*.git*" | xargs gofmt -d -s

golangci-lint:
<<<<<<< HEAD
@go install github.com/golangci/golangci-lint/cmd/[email protected]
=======
@go install github.com/golangci/golangci-lint/cmd/[email protected]
>>>>>>> d60e44aa6 (style: Improve code quality with new linters (#1414))

lint-fix: golangci-lint
golangci-lint run --fix --out-format=tab --issues-exit-code=0
Expand Down
2 changes: 0 additions & 2 deletions baseapp/abci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,6 @@ func TestGetBlockRentionHeight(t *testing.T) {
}

for name, tc := range testCases {
tc := tc

tc.bapp.SetParamStore(&paramStore{db: dbm.NewMemDB()})
tc.bapp.InitChain(abci.RequestInitChain{
ConsensusParams: &abci.ConsensusParams{
Expand Down
3 changes: 0 additions & 3 deletions baseapp/deliver_tx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ func TestLoadSnapshotChunk(t *testing.T) {
"Zero chunk": {2, 1, 0, false},
}
for name, tc := range testcases {
tc := tc
t.Run(name, func(t *testing.T) {
resp := app.LoadSnapshotChunk(abci.RequestLoadSnapshotChunk{
Height: tc.height,
Expand Down Expand Up @@ -96,7 +95,6 @@ func TestOfferSnapshot_Errors(t *testing.T) {
}, abci.ResponseOfferSnapshot_REJECT},
}
for name, tc := range testcases {
tc := tc
t.Run(name, func(t *testing.T) {
resp := app.OfferSnapshot(abci.RequestOfferSnapshot{Snapshot: tc.snapshot})
assert.Equal(t, tc.result, resp.Result)
Expand Down Expand Up @@ -1839,7 +1837,6 @@ func TestSetLoader(t *testing.T) {
v := []byte("value")

for name, tc := range cases {
tc := tc
t.Run(name, func(t *testing.T) {
// prepare a db with some data
db := dbm.NewMemDB()
Expand Down
2 changes: 0 additions & 2 deletions client/cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,6 @@ func TestSetCmdClientContextHandler(t *testing.T) {
}

for _, tc := range testCases {
tc := tc

t.Run(tc.name, func(t *testing.T) {
ctx := context.WithValue(context.Background(), client.ClientContextKey, &client.Context{})

Expand Down
6 changes: 3 additions & 3 deletions client/config/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func runConfigCmd(cmd *cobra.Command, args []string) error {

conf, err := getClientConfig(configPath, clientCtx.Viper)
if err != nil {
return fmt.Errorf("couldn't get client config: %v", err)
return fmt.Errorf("couldn't get client config: %w", err)
}

switch len(args) {
Expand Down Expand Up @@ -60,7 +60,7 @@ func runConfigCmd(cmd *cobra.Command, args []string) error {
cmd.Println(conf.BroadcastMode)
default:
err := errUnknownConfigKey(key)
return fmt.Errorf("couldn't get the value for the key: %v, error: %v", key, err)
return fmt.Errorf("couldn't get the value for the key: %v, error: %w", key, err)
}

case 2:
Expand All @@ -84,7 +84,7 @@ func runConfigCmd(cmd *cobra.Command, args []string) error {

confFile := filepath.Join(configPath, "client.toml")
if err := writeConfigToFile(confFile, conf); err != nil {
return fmt.Errorf("could not write client config to the file: %v", err)
return fmt.Errorf("could not write client config to the file: %w", err)
}

default:
Expand Down
10 changes: 5 additions & 5 deletions client/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,17 +59,17 @@ func ReadFromClientConfig(ctx client.Context) (client.Context, error) {
// if config.toml file does not exist we create it and write default ClientConfig values into it.
if _, err := os.Stat(configFilePath); os.IsNotExist(err) {
if err := ensureConfigPath(configPath); err != nil {
return ctx, fmt.Errorf("couldn't make client config: %v", err)
return ctx, fmt.Errorf("couldn't make client config: %w", err)
}

if err := writeConfigToFile(configFilePath, conf); err != nil {
return ctx, fmt.Errorf("could not write client config to the file: %v", err)
return ctx, fmt.Errorf("could not write client config to the file: %w", err)
}
}

conf, err := getClientConfig(configPath, ctx.Viper)
if err != nil {
return ctx, fmt.Errorf("couldn't get client config: %v", err)
return ctx, fmt.Errorf("couldn't get client config: %w", err)
}
// we need to update KeyringDir field on Client Context first cause it is used in NewKeyringFromBackend
ctx = ctx.WithOutputFormat(conf.Output).
Expand All @@ -78,15 +78,15 @@ func ReadFromClientConfig(ctx client.Context) (client.Context, error) {

keyring, err := client.NewKeyringFromBackend(ctx, conf.KeyringBackend)
if err != nil {
return ctx, fmt.Errorf("couldn't get key ring: %v", err)
return ctx, fmt.Errorf("couldn't get key ring: %w", err)
}

ctx = ctx.WithKeyring(keyring)

// https://github.com/cosmos/cosmos-sdk/issues/8986
client, err := client.NewClientFromNode(conf.Node)
if err != nil {
return ctx, fmt.Errorf("couldn't get client from nodeURI: %v", err)
return ctx, fmt.Errorf("couldn't get client from nodeURI: %w", err)
}

ctx = ctx.WithNodeURI(conf.Node).
Expand Down
1 change: 0 additions & 1 deletion client/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ func TestConfigCmdEnvFlag(t *testing.T) {
}

for _, tc := range tt {
tc := tc
t.Run(tc.name, func(t *testing.T) {
clientCtx, cleanup := initClientContext(t, tc.envVar)
defer func() {
Expand Down
2 changes: 1 addition & 1 deletion client/debug/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ $ %s debug addr link19wgf6ymq2ur6r59st95e04e49m69z4al4fc982
addr, err3 = sdk.ValAddressFromBech32(addrString)

if err3 != nil {
return fmt.Errorf("expected hex or bech32. Got errors: hex: %v, bech32 acc: %v, bech32 val: %v", err, err2, err3)
return fmt.Errorf("expected hex or bech32. Got errors: hex: %w, bech32 acc: %w, bech32 val: %w", err, err2, err3)
}
}
}
Expand Down
2 changes: 0 additions & 2 deletions client/flags/flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ func TestParseGasSetting(t *testing.T) {
}

for _, tc := range testCases {
tc := tc

t.Run(tc.name, func(t *testing.T) {
gs, err := flags.ParseGasSetting(tc.input)

Expand Down
4 changes: 0 additions & 4 deletions client/grpc/tmservice/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,6 @@ func (s IntegrationTestSuite) TestLatestValidatorSet_GRPC() {
{"with pagination", &tmservice.GetLatestValidatorSetRequest{Pagination: &qtypes.PageRequest{Offset: 0, Limit: uint64(len(vals))}}, false, ""},
}
for _, tc := range testCases {
tc := tc
s.Run(tc.name, func() {
grpcRes, err := s.queryClient.GetLatestValidatorSet(context.Background(), tc.req)
if tc.expErr {
Expand Down Expand Up @@ -177,7 +176,6 @@ func (s IntegrationTestSuite) TestLatestValidatorSet_GRPCGateway() {
{"with pagination", fmt.Sprintf("%s/cosmos/base/tendermint/v1beta1/validatorsets/latest?pagination.offset=0&pagination.limit=2", vals[0].APIAddress), false, ""},
}
for _, tc := range testCases {
tc := tc
s.Run(tc.name, func() {
res, err := rest.GetRequest(tc.url)
s.Require().NoError(err)
Expand Down Expand Up @@ -210,7 +208,6 @@ func (s IntegrationTestSuite) TestValidatorSetByHeight_GRPC() {
{"with pagination", &tmservice.GetValidatorSetByHeightRequest{Height: 1, Pagination: &qtypes.PageRequest{Offset: 0, Limit: 1}}, false, ""},
}
for _, tc := range testCases {
tc := tc
s.Run(tc.name, func() {
grpcRes, err := s.queryClient.GetValidatorSetByHeight(context.Background(), tc.req)
if tc.expErr {
Expand Down Expand Up @@ -239,7 +236,6 @@ func (s IntegrationTestSuite) TestValidatorSetByHeight_GRPCGateway() {
{"with pagination", fmt.Sprintf("%s/cosmos/base/tendermint/v1beta1/validatorsets/%d?pagination.offset=0&pagination.limit=2", vals[0].APIAddress, 1), false, ""},
}
for _, tc := range testCases {
tc := tc
s.Run(tc.name, func() {
res, err := rest.GetRequest(tc.url)
s.Require().NoError(err)
Expand Down
1 change: 0 additions & 1 deletion client/keys/add_ledger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,6 @@ func Test_runAddCmdLedgerDryRun(t *testing.T) {
},
}
for _, tt := range testData {
tt := tt
t.Run(tt.name, func(t *testing.T) {
cmd := AddKeyCommand()
cmd.Flags().AddFlagSet(Commands("home").PersistentFlags())
Expand Down
1 change: 0 additions & 1 deletion client/keys/add_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,6 @@ func Test_runAddCmdDryRun(t *testing.T) {
},
}
for _, tt := range testData {
tt := tt
t.Run(tt.name, func(t *testing.T) {
cmd := AddKeyCommand()
cmd.Flags().AddFlagSet(Commands("home").PersistentFlags())
Expand Down
2 changes: 0 additions & 2 deletions client/keys/codec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ func TestMarshalJSON(t *testing.T) {
{"empty object", args{data.Keys[3]}, data.JSON[3], false},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
got, err := keys.MarshalJSON(tt.args.o)
require.Equal(t, tt.wantErr, err != nil)
Expand Down Expand Up @@ -86,7 +85,6 @@ func TestUnmarshalJSON(t *testing.T) {
{"empty object", args{data.JSON[3], &data.Answers[3]}, false},
}
for idx, tt := range tests {
idx, tt := idx, tt
t.Run(tt.name, func(t *testing.T) {
err := keys.UnmarshalJSON(tt.args.bz, tt.args.ptr)
require.Equal(t, tt.wantErr, err != nil)
Expand Down
1 change: 0 additions & 1 deletion client/keys/list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ func Test_runListCmd(t *testing.T) {
{"keybase: w/key", kbHome2, false},
}
for _, tt := range testData {
tt := tt
t.Run(tt.name, func(t *testing.T) {
cmd.SetArgs([]string{
fmt.Sprintf("--%s=%s", flags.FlagHome, tt.kbDir),
Expand Down
1 change: 0 additions & 1 deletion client/keys/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ func TestParseKey(t *testing.T) {
{"hex", []string{hexstr}, false},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
require.Equal(t, tt.wantErr, doParseKey(ParseKeyStringCommand(), config, tt.args) != nil)
})
Expand Down
4 changes: 2 additions & 2 deletions client/keys/show.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func runShowCmd(cmd *cobra.Command, args []string) (err error) {
if len(args) == 1 {
info, err = fetchKey(clientCtx.Keyring, args[0])
if err != nil {
return fmt.Errorf("%s is not a valid name or address: %v", args[0], err)
return fmt.Errorf("%s is not a valid name or address: %w", args[0], err)
}
if info.GetType() == keyring.TypeMulti {
info, err = keyring.NewMultiInfo(info.GetName(), info.GetPubKey())
Expand All @@ -75,7 +75,7 @@ func runShowCmd(cmd *cobra.Command, args []string) (err error) {
for i, keyref := range args {
info, err := fetchKey(clientCtx.Keyring, keyref)
if err != nil {
return fmt.Errorf("%s is not a valid name or address: %v", keyref, err)
return fmt.Errorf("%s is not a valid name or address: %w", keyref, err)
}

pks[i] = info.GetPubKey()
Expand Down
2 changes: 0 additions & 2 deletions client/keys/show_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,6 @@ func Test_validateMultisigThreshold(t *testing.T) {
{"1-2", args{2, 1}, true},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
if err := validateMultisigThreshold(tt.args.k, tt.args.nKeys); (err != nil) != tt.wantErr {
t.Errorf("validateMultisigThreshold() error = %v, wantErr %v", err, tt.wantErr)
Expand All @@ -202,7 +201,6 @@ func Test_getBechKeyOut(t *testing.T) {
{"cons", args{sdk.PrefixConsensus}, keyring.MkConsKeyOutput, false},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
got, err := getBechKeyOut(tt.args.bechPrefix)
if tt.wantErr {
Expand Down
Loading

0 comments on commit 8aa19f3

Please sign in to comment.