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

style: Improve code quality with new linters #1414

Merged
merged 11 commits into from
Jun 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ jobs:
- uses: golangci/golangci-lint-action@v6
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.55
version: v1.59
args: --timeout 10m
github-token: ${{ secrets.GITHUB_TOKEN }}
if: env.GIT_DIFF
52 changes: 31 additions & 21 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -1,47 +1,59 @@
run:
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

output:
sort-results: true

linters:
disable-all: true
enable:
- errcheck
- dogsled
- exportloopref
- goconst
- gocritic
- gci
- gofumpt
- gosec
- gosimple
- govet
- ineffassign
- misspell
- staticcheck
- unused
- dogsled
- gosec
- gci
- gofumpt
- goconst
- gocritic
- nakedret
- nolintlint
- staticcheck
- revive
- misspell
- stylecheck
- typecheck
- thelper
- unconvert
- unused
- 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/*
exclude-rules:
- text: "Use of weak random number generator"
linters:
Expand Down Expand Up @@ -156,8 +168,6 @@ linters-settings:
extra-rules: true
dogsled:
max-blank-identifiers: 6
maligned:
suggest-new: true
nolintlint:
allow-unused: false
require-explanation: false
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (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

### Removed

Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ lint: golangci-lint
find . -name '*.go' -type f -not -path "*.git*" | xargs gofmt -d -s

golangci-lint:
@go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.55.2
@go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.59

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 @@ -110,8 +110,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 @@ -49,7 +49,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 @@ -94,7 +93,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 @@ -1819,7 +1817,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 @@ -30,7 +30,7 @@

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)

Check warning on line 33 in client/config/cmd.go

View check run for this annotation

Codecov / codecov/patch

client/config/cmd.go#L33

Added line #L33 was not covered by tests
}

switch len(args) {
Expand Down Expand Up @@ -59,7 +59,7 @@
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)

Check warning on line 62 in client/config/cmd.go

View check run for this annotation

Codecov / codecov/patch

client/config/cmd.go#L62

Added line #L62 was not covered by tests
}

case 2:
Expand All @@ -83,7 +83,7 @@

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)

Check warning on line 86 in client/config/cmd.go

View check run for this annotation

Codecov / codecov/patch

client/config/cmd.go#L86

Added line #L86 was not covered by tests
}

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 @@
// 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)

Check warning on line 62 in client/config/config.go

View check run for this annotation

Codecov / codecov/patch

client/config/config.go#L62

Added line #L62 was not covered by tests
}

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)

Check warning on line 66 in client/config/config.go

View check run for this annotation

Codecov / codecov/patch

client/config/config.go#L66

Added line #L66 was not covered by tests
}
}

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)

Check warning on line 72 in client/config/config.go

View check run for this annotation

Codecov / codecov/patch

client/config/config.go#L72

Added line #L72 was not covered by tests
}
// 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 @@

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)

Check warning on line 81 in client/config/config.go

View check run for this annotation

Codecov / codecov/patch

client/config/config.go#L81

Added line #L81 was not covered by tests
}

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)

Check warning on line 89 in client/config/config.go

View check run for this annotation

Codecov / codecov/patch

client/config/config.go#L89

Added line #L89 was not covered by tests
}

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 @@ -88,7 +88,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 @@
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)

Check warning on line 87 in client/debug/main.go

View check run for this annotation

Codecov / codecov/patch

client/debug/main.go#L87

Added line #L87 was not covered by tests
}
}
}
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 @@ -307,7 +307,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 @@ -47,7 +47,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 @@ -178,7 +178,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 @@ -204,7 +203,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
11 changes: 5 additions & 6 deletions client/tx/tx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,23 +71,22 @@ func TestCalculateGas(t *testing.T) {
}

for _, tc := range testCases {
stc := tc
txCfg := NewTestTxConfig()

txf := tx.Factory{}.
WithChainID("test-chain").
WithTxConfig(txCfg).WithSignMode(txCfg.SignModeHandler().DefaultMode())

t.Run(stc.name, func(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
mockClientCtx := mockContext{
gasUsed: tc.args.mockGasUsed,
wantErr: tc.args.mockWantErr,
}
simRes, gotAdjusted, err := tx.CalculateGas(mockClientCtx, txf.WithGasAdjustment(stc.args.adjustment))
if stc.expPass {
simRes, gotAdjusted, err := tx.CalculateGas(mockClientCtx, txf.WithGasAdjustment(tc.args.adjustment))
if tc.expPass {
require.NoError(t, err)
require.Equal(t, simRes.GasInfo.GasUsed, stc.wantEstimate)
require.Equal(t, gotAdjusted, stc.wantAdjusted)
require.Equal(t, simRes.GasInfo.GasUsed, tc.wantEstimate)
require.Equal(t, gotAdjusted, tc.wantAdjusted)
require.NotNil(t, simRes.Result)
} else {
require.Error(t, err)
Expand Down
1 change: 0 additions & 1 deletion client/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ func TestPaginate(t *testing.T) {
}

for i, tc := range testCases {
i, tc := i, tc
t.Run(tc.name, func(t *testing.T) {
start, end := client.Paginate(tc.numObjs, tc.page, tc.limit, tc.defLimit)
require.Equal(t, tc.expectedStart, start, "invalid result; test case #%d", i)
Expand Down
Loading
Loading