chore: update, tighten and modernize linters

.golangci.yml

@@ -5,6 +5,7 @@ linters:
   default: none
   enable:
     - errcheck
+    - errorlint
     - goconst
     - gocritic
     - gosec
@@ -18,73 +19,78 @@ 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
+        # 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 +99,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

@@ -50,7 +50,7 @@ b := f
 
 ## Linting
 
-- Run `make lint-fix` to fix any linting errors.
+- Run `make lint` to see linting errors and `make lint-fix` to fix many issues (some linters do not support auto-fix).
 
 ## Various
 
@@ -119,7 +119,7 @@ sdkerrors.Wrapf(
 )
 ```
 
-## Common Mistakes
+## Common mistakes
 
 This is a compilation of some of the common mistakes we see in the repo that should be avoided.
 
@@ -242,7 +242,7 @@ cases := []struct {
 
 **Testing context**
 
-Go 1.24 added a (testing.TB).Context() method. In tests, prefer using (testing.TB).Context() over context.Background() to provide the initial context.Context used by the test. Helper functions, environment or test double setup, and other functions called from the test function body that require a context should have one explicitly passed. [Referance](https://google.github.io/styleguide/go/decisions#contexts)
+Go 1.24 added a (testing.TB).Context() method. In tests, prefer using (testing.TB).Context() over context.Background() to provide the initial context.Context used by the test. Helper functions, environment or test double setup, and other functions called from the test function body that require a context should have one explicitly passed. [Reference](https://google.github.io/styleguide/go/decisions#contexts)
 
 ---
 **Error Logging**
@@ -253,7 +253,7 @@ If you return an error, it’s usually better not to log it yourself but rather
 ---
 **Struct defined outside of the package**
 
-Must have fields specified. [Referance](https://google.github.io/styleguide/go/decisions#field-names)
+Must have fields specified. [Reference](https://google.github.io/styleguide/go/decisions#field-names)
 
 ```go
 // Good:
@@ -270,7 +270,7 @@ r := csv.Reader{',', '#', 4, false, false, false, false}
 ```
 
 ---
-**Naming struct fileds in tabular tests**
+**Naming struct fields in tabular tests**
 
 If tabular test struct has more than two fields, consider explicitly naming them. If the test struct has one name and one error field, then we can allow upto three fields. If test struct has more fields, consider naming them when writing test cases.
 
@@ -342,26 +342,13 @@ testCases := []struct {
 
 ## Known Anti Patterns
 
-It's strongly recommended [not to create a custom context](https://google.github.io/styleguide/go/decisions#custom-contexts). The cosmos sdk has it's own context that is passed around, and we should not try to work against that pattern to avoid confusion.
+It's strongly recommended [not to create a custom context](https://google.github.io/styleguide/go/decisions#custom-contexts). The Cosmos SDK has it's own context that is passed around, and we should not try to work against that pattern to avoid confusion.
 
 ---
-Test outputs should include the actual value that the function returned before printing the value that was expected. A standard format for printing test outputs is YourFunc(%v) = %v, want %v. Where you would write “actual” and “expected”, prefer using the words “got” and “want”, respectively. [Referance](https://google.github.io/styleguide/go/decisions#got-before-want)
+Test outputs should include the actual value that the function returned before printing the value that was expected. A standard format for printing test outputs is YourFunc(%v) = %v, want %v. Where you would write “actual” and “expected”, prefer using the words “got” and “want”, respectively. [Reference](https://google.github.io/styleguide/go/decisions#got-before-want)
 
 But testify has it other way around.
 
 `Require.Equal(Expected, Actual)`
 
 This is a known anti pattern that we allow as the testify package is used heavily in tests.
-
----
-In tests suites we are embedding `testify/require` package for assertions. Since it's embedded and there are no name collision on `require` types methods, when calling, we should "promote" those methods to the suite. But throughout the repo we make explicit calls.
-
-```go
-s.Require().NoError(err)
-```
-
-A better and more cleaner approach could be,
-
-```go
-s.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)
 }