style: Improve code quality with new linters (#1414)

* 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`

.github/workflows/lint.yml

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

.golangci.yml

@@ -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:
@@ -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

CHANGELOG.md

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

Makefile

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

baseapp/abci_test.go

@@ -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{

baseapp/deliver_tx_test.go

@@ -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,
@@ -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)
@@ -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()

client/cmd_test.go

@@ -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{})
 

client/config/cmd.go

@@ -30,7 +30,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) {
@@ -59,7 +59,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:
@@ -83,7 +83,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:

client/config/config.go

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

client/config/config_test.go

@@ -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() {

client/debug/main.go

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

client/flags/flags_test.go

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

client/grpc/tmservice/service_test.go

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

client/keys/add_ledger_test.go

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

client/keys/add_test.go

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

client/keys/codec_test.go

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

client/keys/list_test.go

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

client/keys/parse_test.go

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

client/keys/show.go

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

client/keys/show_test.go

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

client/tx/tx_test.go

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

client/utils_test.go

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