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

.golangci.yml

@@ -1,36 +1,59 @@
 run:
-  tests: false
-#   # timeout for analysis, e.g. 30s, 5m, default is 1m
-#   timeout: 5m
+  tests: true
+  timeout: 15m
+  allow-parallel-runners: true
+  build-tags:
+    - ledger
+    - goleveldb
+    - test_ledger_mock
+
+output:
+  sort-results: true
 
 linters:
-  disable-all: false
+  disable-all: true
   enable:
-    - gofmt
-    - goconst
-    - goimports
-    - gosec
+    - errcheck
     - gosimple
     - govet
     - ineffassign
-    - misspell
-    - nakedret
-    - prealloc
     - staticcheck
-    - stylecheck
-    - typecheck
-    - unconvert
     - unused
-    - unparam
-    - misspell
-  disable:
+    - dogsled
+    - gosec
+    - gci
+    - gofumpt
+    - goconst
     - gocritic
-    - errcheck
-    - interfacer
-    - wsl
+    - 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/*
   exclude-rules:
     - text: "Use of weak random number generator"
       linters:
@@ -54,13 +77,105 @@ issues:
       text: "SA1019: token."
       linters:
         - staticcheck
+    - path: "legacy"
+      text: "SA1019:"
+      linters:
+        - staticcheck
+    - text: "SA1019: codec.NewAminoCodec is deprecated" # TODO remove once migration path is set out
+      linters:
+        - staticcheck
+    - text: "SA1019: legacybech32.MustMarshalPubKey" # TODO remove once ready to remove from the sdk
+      linters:
+        - staticcheck
+    - text: "SA1019: legacybech32.MarshalPubKey" # TODO remove once ready to remove from the sdk
+      linters:
+        - staticcheck
+    - text: "SA1019: legacybech32.UnmarshalPubKey" # TODO remove once ready to remove from the sdk
+      linters:
+        - staticcheck
+    - text: "SA1019: params.SendEnabled is deprecated" # TODO remove once ready to remove from the sdk
+      linters:
+        - staticcheck
+    - text: "SA1019: \"github.com/golang/protobuf/proto\" is deprecated" # TODO remove once ready to remove from the sdk
+      linters:
+        - staticcheck
+    - text: "SA1019: vote.Option is deprecated" # TODO remove once ready to remove from the sdk
+      linters:
+        - staticcheck
+    - text: "SA1019: types.QueryNextAccountNumberRequest is deprecated" # TODO remove once ready to remove from the sdk
+      linters:
+        - staticcheck
+    - text: "leading space"
+      linters:
+        - nolintlint
+    - path: _test\.go
+      linters:
+        - goconst
   max-issues-per-linter: 10000
   max-same-issues: 10000
 
 linters-settings:
+  gci:
+    custom-order: true
+    sections:
+      - standard # Standard section: captures all standard packages.
+      - default # Default section: contains all imports that could not be matched to another section type.
+      - prefix(github.com/Finschia/finschia-sdk)
+  revive:
+    rules:
+      - name: redefines-builtin-id
+        disabled: true
+
+  gosec:
+    includes:
+      #  - G101 # Look for hard coded credentials
+      - G102 # Bind to all interfaces
+      - G103 # Audit the use of unsafe block
+      - G104 # Audit errors not checked
+      - G106 # Audit the use of ssh.InsecureIgnoreHostKey
+      - G107 # Url provided to HTTP request as taint input
+      - G108 # Profiling endpoint automatically exposed on /debug/pprof
+      - G109 # Potential Integer overflow made by strconv.Atoi result conversion to int16/32
+      - G110 # Potential DoS vulnerability via decompression bomb
+      - G111 # Potential directory traversal
+      - G112 # Potential slowloris attack
+      - G113 # Usage of Rat.SetString in math/big with an overflow (CVE-2022-23772)
+      - G114 # Use of net/http serve function that has no support for setting timeouts
+      - G201 # SQL query construction using format string
+      - G202 # SQL query construction using string concatenation
+      - G203 # Use of unescaped data in HTML templates
+      - G204 # Audit use of command execution
+      - G301 # Poor file permissions used when creating a directory
+      - G302 # Poor file permissions used with chmod
+      - G303 # Creating tempfile using a predictable path
+      - G304 # File path provided as taint input
+      - G305 # File traversal when extracting zip/tar archive
+      - G306 # Poor file permissions used when writing to a new file
+      - G307 # Deferring a method which returns an error
+      - G401 # Detect the usage of DES, RC4, MD5 or SHA1
+      - G402 # Look for bad TLS connection settings
+      - G403 # Ensure minimum RSA key length of 2048 bits
+      - G404 # Insecure random number source (rand)
+      - G501 # Import blocklist: crypto/md5
+      - G502 # Import blocklist: crypto/des
+      - G503 # Import blocklist: crypto/rc4
+      - G504 # Import blocklist: net/http/cgi
+      - G505 # Import blocklist: crypto/sha1
+      - G601 # Implicit memory aliasing of items from a range statement
+  misspell:
+    locale: US
+  gofumpt:
+    extra-rules: true
   dogsled:
-    max-blank-identifiers: 3
+    max-blank-identifiers: 6
   nolintlint:
     allow-unused: false
     require-explanation: false
-    require-specific: false
+    require-specific: false
+  gosimple:
+    checks: ["all"]
+  gocritic:
+    disabled-checks:
+      - regexpMust
+      - appendAssign
+      - ifElseChain

CHANGELOG.md

@@ -42,7 +42,8 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Improvements
 * (x/fswap) [\#1415](https://github.com/Finschia/finschia-sdk/pull/1415) add more testcases for fswap module
-
+* (style) [\#1426](https://github.com/Finschia/finschia-sdk/pull/1426) Improve code quality with new linters
+
 ### Bug Fixes
 
 ### Removed
@@ -57,4 +58,5 @@ Ref: https://keepachangelog.com/en/1.0.0/
 ### Build, CI
 * (build, ci) [\#1410](https://github.com/Finschia/finschia-sdk/pull/1410) Bump Go from 1.20 to 1.22
 * (build) [\#1413](https://github.com/Finschia/finschia-sdk/pull/1413) Update outdated dependencies for v0.49.x
+
 ### Document Updates

Makefile

@@ -354,7 +354,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.51.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

@@ -112,8 +112,6 @@
 	}
 
 	for name, tc := range testCases {
-		tc := tc
-
 		tc.bapp.SetParamStore(&paramStore{db: dbm.NewMemDB()})
 		tc.bapp.InitChain(abci.RequestInitChain{
 			ConsensusParams: &abci.ConsensusParams{
@@ -140,7 +138,7 @@
 	db := dbm.NewMemDB()
 	name := t.Name()
 	app := NewBaseApp(name, logger, db, nil)
 	app.init()
 
 	app.BeginBlock(ocabci.RequestBeginBlock{Header: tmproto.Header{Height: 1}})
 	app.Commit()
@@ -190,7 +188,7 @@
 			},
 		},
 	})
 	app.init()
 
 	// set block params
 	app.BeginBlock(ocabci.RequestBeginBlock{Header: tmproto.Header{Height: 1}})
@@ -218,7 +216,7 @@
 		panic(err)
 	}
 
 	ps.db.Set(key, bz)
 }
 
 func (ps *paramStore) Has(_ sdk.Context, key []byte) bool {

baseapp/deliver_tx_test.go

@@ -51,7 +51,6 @@
 		"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,
@@ -96,7 +95,6 @@
 		}, 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)
@@ -869,7 +867,7 @@
 	{
 		tx := newTxCounter(0, 0)
 
 		require.PanicsWithValue(t, customPanicMsg, func() { app.Deliver(aminoTxEncoder(), tx) })
 	}
 }
 
@@ -1283,7 +1281,7 @@
 	}
 }
 
 func anteHandlerTxTest(t *testing.T, capKey sdk.StoreKey, storeKey []byte) sdk.AnteHandler {
 	return func(ctx sdk.Context, tx sdk.Tx, simulate bool) (sdk.Context, error) {
 		store := ctx.KVStore(capKey)
 		txTest := tx.(txTest)
@@ -1306,7 +1304,7 @@
 }
 
 // TODO(dudong2): remove this func after reverting CheckTx logic
 func anteHandlerTxTest2(t *testing.T, capKey sdk.StoreKey, storeKey []byte) sdk.AnteHandler {
 	return func(ctx sdk.Context, tx sdk.Tx, simulate bool) (sdk.Context, error) {
 		store := ctx.KVStore(capKey)
 		txTest := tx.(*txTest)
@@ -1337,7 +1335,7 @@
 	}
 }
 
 func handlerMsgCounter(t *testing.T, capKey sdk.StoreKey, deliverKey []byte) sdk.Handler {
 	return func(ctx sdk.Context, msg sdk.Msg) (*sdk.Result, error) {
 		ctx = ctx.WithEventManager(sdk.NewEventManager())
 		store := ctx.KVStore(capKey)
@@ -1839,7 +1837,6 @@
 	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

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

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

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

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

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