chore: update, tighten and modernize linters

.golangci.yml

@@ -5,6 +5,7 @@ linters:
   default: none
   enable:
     - errcheck
+    - errorlint
     - goconst
     - gocritic
     - gosec
@@ -18,73 +19,79 @@ linters:
     - unconvert
     - unparam
     - unused
+    - mirror
+    - copyloopvar
+    - iface
+    - importas
+    - intrange
+    - nakedret
+    - nilnesserr
+    - nilnil
+    - nonamedreturns
+    - prealloc
+    - predeclared
+    - reassign
+    - recvcheck
+    - testifylint
+    - testpackage
+    - usestdlibvars
+    - usetesting
+    - wastedassign
   settings:
-    gocritic:
-      disabled-checks:
-        - appendAssign
     gosec:
       excludes:
-        - G101
-        - G107
-        - G115
-        - G404
-      confidence: medium
+        - G107 # Url provided to HTTP request as taint input
+        - G115 # Potential integer overflow when converting between integer types
+        - G404 # Insecure random number source (rand)
+      confidence: low
+    nakedret:
+      max-func-lines: 0
     revive:
       enable-all-rules: true
+      # See the rule descriptions here: https://github.com/mgechev/revive/blob/HEAD/RULES_DESCRIPTIONS.md
       rules:
-        - name: redundant-import-alias
-          disabled: true
-        - name: use-any
+        # Rules with configuration
+        - name: receiver-naming
+          disabled: false
+          arguments:
+            - max-length: 4
+        - name: unhandled-error
+          disabled: false
+          arguments: # Disable for these ones only
+            - fmt.Printf
+            - fmt.Print
+            - fmt.Println
+            - myFunction
+        # Disabled rules
+        # TODO: Some of these are kinda code smelly, should reconsider some of them
+        - name: add-constant
           disabled: true
-        - name: if-return
+        - name: argument-limit
           disabled: true
-        - name: max-public-structs
+        - name: banned-characters
           disabled: true
         - name: cognitive-complexity
           disabled: true
-        - name: argument-limit
+        - name: confusing-naming
+          disabled: true
+        - name: confusing-results
           disabled: true
         - name: cyclomatic
           disabled: true
-        - name: file-header
+        - name: deep-exit
           disabled: true
         - name: function-length
           disabled: true
-        - name: function-result-limit
-          disabled: true
-        - name: line-length-limit
-          disabled: true
         - name: flag-parameter
           disabled: true
-        - name: add-constant
-          disabled: true
-        - name: empty-lines
-          disabled: true
-        - name: banned-characters
-          disabled: true
-        - name: deep-exit
-          disabled: true
-        - name: confusing-results
-          disabled: true
-        - name: unused-parameter
-          disabled: true
-        - name: modifies-value-receiver
-          disabled: true
-        - name: early-return
+        - name: function-result-limit
           disabled: true
-        - name: confusing-naming
+        - name: line-length-limit
           disabled: true
-        - name: defer
+        - name: max-public-structs
           disabled: true
         - name: unused-parameter
           disabled: true
-        - name: unhandled-error
-          arguments:
-            - fmt.Printf
-            - fmt.Print
-            - fmt.Println
-            - myFunction
-          disabled: false
   exclusions:
     generated: lax
     presets:
@@ -93,25 +100,24 @@ linters:
       - legacy
       - std-error-handling
     rules:
-      - linters:
-          - revive
-        text: differs only by capitalization to method
-      - linters:
-          - gosec
-        text: Use of weak random number generator
-      - linters:
-          - gosec
-        text: 'G115: integer overflow conversion'
       - linters:
           - staticcheck
-        text: 'SA1019:'
-      - linters:
-          - gosec
-        text: 'G115: integer overflow conversion'
+        text: 'SA1019:' # TODO: This should really be removed!
+      - path: _test\.go
+        linters:
+          - prealloc
+      - path: e2e/*
+        linters:
+          - prealloc
+          - testpackage
+      - path: testing/*
+        linters:
+          - prealloc
     paths:
       - third_party$
       - builtin$
       - examples$
+      - testing/mock/*
 issues:
   max-issues-per-linter: 10000
   max-same-issues: 10000

AGENTS.md

@@ -30,7 +30,7 @@ This file provides guidance for automated agents contributing to this repository
 ## Development Workflow
 
 1. **Formatting and linting**
-  - Run `make lint-all` to lint all modules
+  - Run `make lint` to lint all modules
   - Run `make lint-fix` to automatically fix lint issues via `golangci-lint`.
   - Run `make format` to format Go code with `gofumpt`.
 2. **Testing**

cmd/build_test_matrix/main.go

@@ -90,7 +90,7 @@ func getGithubActionMatrixForTests(e2eRootDirectory, testName string, suite stri
 	fset := token.NewFileSet()
 	err := filepath.Walk(e2eRootDirectory, func(path string, info fs.FileInfo, err error) error {
 		if err != nil {
-			return fmt.Errorf("error walking e2e directory: %s", err)
+			return fmt.Errorf("error walking e2e directory: %w", err)
 		}
 
 		// only look at test files
@@ -100,7 +100,7 @@ func getGithubActionMatrixForTests(e2eRootDirectory, testName string, suite stri
 
 		f, err := parser.ParseFile(fset, path, nil, 0)
 		if err != nil {
-			return fmt.Errorf("failed parsing file: %s", err)
+			return fmt.Errorf("failed parsing file: %w", err)
 		}
 
 		suiteNameForFile, testCases, err := extractSuiteAndTestNames(f)

cmd/build_test_matrix/main_test.go

@@ -7,7 +7,7 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 const (
@@ -20,15 +20,15 @@ func TestGetGithubActionMatrixForTests(t *testing.T) {
 	t.Run("empty dir with no test cases fails", func(t *testing.T) {
 		testingDir := t.TempDir()
 		_, err := getGithubActionMatrixForTests(testingDir, "", "", nil)
-		assert.Error(t, err)
+		require.Error(t, err)
 	})
 
 	t.Run("only test functions are picked up", func(t *testing.T) {
 		testingDir := t.TempDir()
 		createFileWithTestSuiteAndTests(t, "FeeMiddlewareTestSuite", "TestA", "TestB", testingDir, goTestFileNameOne)
 
 		gh, err := getGithubActionMatrixForTests(testingDir, "", "", nil)
-		assert.NoError(t, err)
+		require.NoError(t, err)
 
 		expected := GithubActionTestMatrix{
 			Include: []TestSuitePair{
@@ -51,7 +51,7 @@ func TestGetGithubActionMatrixForTests(t *testing.T) {
 		createFileWithTestSuiteAndTests(t, "TransferTestSuite", "TestC", "TestD", testingDir, goTestFileNameTwo)
 
 		gh, err := getGithubActionMatrixForTests(testingDir, "", "", nil)
-		assert.NoError(t, err)
+		require.NoError(t, err)
 
 		expected := GithubActionTestMatrix{
 			Include: []TestSuitePair{
@@ -83,7 +83,7 @@ func TestGetGithubActionMatrixForTests(t *testing.T) {
 		createFileWithTestSuiteAndTests(t, "TransferTestSuite", "TestC", "TestD", testingDir, goTestFileNameTwo)
 
 		gh, err := getGithubActionMatrixForTests(testingDir, "TestA", "TestFeeMiddlewareTestSuite", nil)
-		assert.NoError(t, err)
+		require.NoError(t, err)
 
 		expected := GithubActionTestMatrix{
 			Include: []TestSuitePair{
@@ -102,16 +102,16 @@ func TestGetGithubActionMatrixForTests(t *testing.T) {
 		createFileWithTestSuiteAndTests(t, "FeeMiddlewareTestSuite", "TestA", "TestB", testingDir, goTestFileNameOne)
 
 		_, err := getGithubActionMatrixForTests(testingDir, "TestThatDoesntExist", "TestFeeMiddlewareTestSuite", nil)
-		assert.Error(t, err)
+		require.Error(t, err)
 	})
 
 	t.Run("non test files are skipped", func(t *testing.T) {
 		testingDir := t.TempDir()
 		createFileWithTestSuiteAndTests(t, "FeeMiddlewareTestSuite", "TestA", "TestB", testingDir, nonTestFile)
 
 		gh, err := getGithubActionMatrixForTests(testingDir, "", "", nil)
-		assert.Error(t, err)
-		assert.Empty(t, gh.Include)
+		require.Error(t, err)
+		require.Empty(t, gh.Include)
 	})
 
 	t.Run("fails when there are multiple suite runs", func(t *testing.T) {
@@ -131,10 +131,10 @@ type FeeMiddlewareTestSuite struct {}
 `
 
 		err := os.WriteFile(path.Join(testingDir, goTestFileNameOne), []byte(fileWithTwoSuites), os.FileMode(0o777))
-		assert.NoError(t, err)
+		require.NoError(t, err)
 
 		_, err = getGithubActionMatrixForTests(testingDir, "", "", nil)
-		assert.Error(t, err)
+		require.Error(t, err)
 	})
 }
 
@@ -159,7 +159,7 @@ func assertGithubActionTestMatricesEqual(t *testing.T, expected, actual GithubAc
 		}
 		return memberI.EntryPoint < memberJ.EntryPoint
 	})
-	assert.Equal(t, expected.Include, actual.Include)
+	require.Equal(t, expected.Include, actual.Include)
 }
 
 func goTestFileContents(suiteName, fnName1, fnName2 string) string {
@@ -187,5 +187,5 @@ func createFileWithTestSuiteAndTests(t *testing.T, suiteName, fn1Name, fn2Name,
 	t.Helper()
 	goFileContents := goTestFileContents(suiteName, fn1Name, fn2Name)
 	err := os.WriteFile(path.Join(dir, filename), []byte(goFileContents), os.FileMode(0o777))
-	assert.NoError(t, err)
+	require.NoError(t, err)
 }

docs/dev/go-style-guide.md

@@ -363,5 +363,5 @@ s.Require().NoError(err)
 A better and more cleaner approach could be,
 
 ```go
-s.NoError(err)
+s.Require().NoError(err)
 ```

e2e/dockerutil/dockerutil.go

@@ -28,7 +28,7 @@
 		),
 	})
 	if err != nil {
-		return nil, fmt.Errorf("failed listing containers: %s", err)
+		return nil, fmt.Errorf("failed listing containers: %w", err)
 	}
 
 	return testContainers, nil
@@ -41,7 +41,7 @@
 		ShowStderr: true,
 	})
 	if err != nil {
-		return nil, fmt.Errorf("failed reading logs in test cleanup: %s", err)
+		return nil, fmt.Errorf("failed reading logs in test cleanup: %w", err)
 	}
 	return io.ReadAll(readCloser)
 }

e2e/tests/interchain_accounts/base_test.go

@@ -102,7 +102,7 @@ func (s *InterchainAccountsTestSuite) testMsgSendTxSuccessfulTransfer(order chan
 
 		channels, err := relayer.GetChannels(ctx, s.GetRelayerExecReporter(), chainA.Config().ChainID)
 		s.Require().NoError(err)
-		s.Require().Equal(len(channels), 2)
+		s.Require().Len(channels, 2)
 		icaChannel := channels[0]
 
 		s.Require().Contains(orderMapping[order], icaChannel.Ordering)
@@ -201,7 +201,7 @@ func (s *InterchainAccountsTestSuite) TestMsgSendTx_FailedTransfer_InsufficientF
 
 		channels, err := relayer.GetChannels(ctx, s.GetRelayerExecReporter(), chainA.Config().ChainID)
 		s.Require().NoError(err)
-		s.Require().Equal(len(channels), 2)
+		s.Require().Len(channels, 2)
 	})
 
 	t.Run("fail to execute bank transfer over ICA", func(t *testing.T) {
@@ -480,7 +480,7 @@ func (s *InterchainAccountsTestSuite) testMsgSendTxSuccessfulGovProposal(order c
 
 		channels, err := relayer.GetChannels(ctx, s.GetRelayerExecReporter(), chainA.Config().ChainID)
 		s.Require().NoError(err)
-		s.Require().Equal(len(channels), 2)
+		s.Require().Len(channels, 2)
 		icaChannel := channels[0]
 
 		s.Require().Contains(orderMapping[order], icaChannel.Ordering)

e2e/tests/interchain_accounts/gov_test.go

@@ -76,11 +76,11 @@ func (s *InterchainAccountsGovTestSuite) TestInterchainAccountsGovIntegration()
 		var err error
 		interchainAccAddr, err = query.InterchainAccount(ctx, chainA, govModuleAddress.String(), ibctesting.FirstConnectionID)
 		s.Require().NoError(err)
-		s.Require().NotZero(len(interchainAccAddr))
+		s.Require().NotEmpty(interchainAccAddr)
 
 		channels, err := relayer.GetChannels(ctx, s.GetRelayerExecReporter(), chainA.Config().ChainID)
 		s.Require().NoError(err)
-		s.Require().Equal(len(channels), 2)
+		s.Require().Len(channels, 2)
 	})
 
 	t.Run("interchain account executes a bank transfer on behalf of the corresponding owner account", func(t *testing.T) {

e2e/tests/interchain_accounts/groups_test.go

@@ -153,7 +153,7 @@ func (s *InterchainAccountsGroupsTestSuite) TestInterchainAccountsGroupsIntegrat
 
 		channels, err := relayer.GetChannels(ctx, s.GetRelayerExecReporter(), chainA.Config().ChainID)
 		s.Require().NoError(err)
-		s.Require().Equal(len(channels), 2) // 1 transfer (created by default), 1 interchain-accounts
+		s.Require().Len(channels, 2) // 1 transfer (created by default), 1 interchain-accounts
 	})
 
 	t.Run("fund interchain account wallet", func(t *testing.T) {

e2e/tests/interchain_accounts/params_test.go

@@ -165,7 +165,7 @@ func (s *InterchainAccountsParamsTestSuite) TestHostEnabledParam() {
 
 			channels, err := relayer.GetChannels(ctx, s.GetRelayerExecReporter(), chainA.Config().ChainID)
 			s.Require().NoError(err)
-			s.Require().Equal(len(channels), 2)
+			s.Require().Len(channels, 2)
 		})
 
 		t.Run("stop relayer", func(t *testing.T) {

e2e/tests/packet_forward_middleware/packet_forward_test.go

@@ -286,11 +286,11 @@ func (s *PFMTestSuite) TestForwardPacket() {
 
 		escrowBalB, err = query.Balance(ctx, chainB, escrowAddrB.String(), denomB)
 		s.Require().NoError(err)
-		s.Require().Equal(escrowBalB.Int64(), testvalues.IBCTransferAmount)
+		s.Require().Equal(testvalues.IBCTransferAmount, escrowBalB.Int64())
 
 		balanceA, err := query.Balance(ctx, chainA, userA.FormattedAddress(), ibcTokenA.IBCDenom())
 		s.Require().NoError(err)
-		s.Require().Equal(balanceA.Int64(), testvalues.IBCTransferAmount)
+		s.Require().Equal(testvalues.IBCTransferAmount, balanceA.Int64())
 
 		// Proof that unwinding happens.
 		txResp = s.Transfer(ctx, chainA, userA, chanAB.PortID, chanAB.ChannelID, testvalues.DefaultTransferAmount(ibcTokenA.IBCDenom()), userA.FormattedAddress(), userB.FormattedAddress(), s.GetTimeoutHeight(ctx, chainA), 0, "")