[Linting] Enforce 120 max lines, and import order with custom linting

.github/workflows/run-tests.yml

@@ -9,6 +9,11 @@ concurrency:
   group: ${{ github.workflow }}-${{ github.head_ref || github.ref_name }}
   cancel-in-progress: true
 
+permissions:
+  contents: read
+  pull-requests: read
+  checks: write
+
 env:
   GKE_CLUSTER: protocol-us-central1
   GKE_ZONE: us-central1
@@ -44,8 +49,19 @@ jobs:
       - name: Generate mocks
         run: make go_mockgen
 
-      - name: Run golangci-lint
-        run: make go_lint
+      - name: Fix imports
+        run: make go_gci
+
+      - name: Format the repo
+        run: make go_gofumpt
+
+      - name: golangci-lint
+        uses: golangci/golangci-lint-action@v3
+        with:
+          version: latest
+          only-new-issues: true
+          args: --timeout 15m --issues-exit-code=1
+          skip-cache: true
 
       - name: Test
         run: make go_test

.golangci.yml

@@ -1,17 +1,40 @@
-linters-settings:
-  govet:
-    check-shadowing: true
+run:
+  timeout: 15m
+  config: ./.golangci.yml
+  new: true
+  color: always
+  build-tags:
+    - integration
+    - test
+    - tools
 
 # TODO_TECHDEBT: Enable each linter listed, 1 by 1, fixing issues as they appear.
-# Don't forget to delete the `disable-all: true` line as well.
 linters:
-  disable-all: true
   enable:
-#    - govet
-#    - revive
-#    - errcheck
-#    - unused
-    - goimports
+    # - govet
+    # - errcheck
+    # - unused
+    - revive
+    - gofumpt
+    - lll
+    - gci
+
+linters-settings:
+  govet:
+    check-shadowing: true
+  lll:
+    line-length: 120
+    tab-width: 4
+  gci:
+    sections:
+      - standard # std lib
+      - default # external
+      - prefix(github.com/pokt-network/poktroll) # local imports
+    skip-generated: true
+    custom-order: true
+  skip-files:
+    - "*.pb.go"
+    - "*/vendor/*"
 
 issues:
   exclude-use-default: true
@@ -27,30 +50,55 @@ issues:
     # empty import block containing a comment used by ignite CLI.
     - path: ^x/.+/genesis\.go$
       linters:
-        - goimports
+        - gci
     # Exclude cosmos-sdk module module.go files as they are generated with unused
     # parameters and unchecked errors.
     - path: ^x/.+/module\.go$
       linters:
+        - lll
+        - gci
         - revive
         - errcheck
     # Exclude cosmos-sdk module tx.go files as they are generated with unused
     # constants.
     - path: ^x/.+/cli/tx\.go$
       linters:
+        - gci
+        - lll
         - unused
     # Exclude simulation code as it's generated with lots of unused parameters.
     - path: .*/simulation/.*|_simulation\.go$
       linters:
+        - gci
+        - lll
         - revive
     # Exclude cosmos-sdk module codec files as they are scaffolded with a unused
-    # paramerters and a comment used by ignite CLI.
+    # parameters and a comment used by ignite CLI.
     - path: ^x/.+/codec.go$
       linters:
+        - lll
+        - gci
         - revive
     - path: _test\.go$
       linters:
         - errcheck
+    # Exclude line length check for errors.go files as they are easier to read
+    # as as single line.
+    - path: errors\.go$
+      linters:
+        - lll
+    # Exclude app module code as it's generated with lots with long lines and
+    # improperly formatted imports.
+    - path: ^app
+      linters:
+        - lll
+        - gci
+    # Exclude cmd directory code as it's generated with lots with long lines and
+    # improperly formatted imports.
+    - path: ^cmd/poc
+      linters:
+        - lll
+        - gci
+  new: true
   # TODO_IMPROVE: see https://golangci-lint.run/usage/configuration/#issues-configuration
-  #new: true,
   #fix: true,

Makefile

@@ -11,10 +11,11 @@ POCKET_ADDR_PREFIX = pokt
 
 # TODO: Add other dependencies (ignite, docker, k8s, etc) here
 .PHONY: install_ci_deps
-install_ci_deps: ## Installs `mockgen`
-	go install "github.com/golang/mock/[email protected]" && mockgen --version
-	go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest && golangci-lint --version
-	go install golang.org/x/tools/cmd/goimports@latest
+install_ci_deps: ## Installs `mockgen` and `golangci-lint` and other linters
+	go install "github.com/golang/mock/[email protected]" && mockgen --version; \
+	go install github.com/golangci/golangci-lint/cmd/[email protected] && golangci-lint --version; \
+	go install github.com/daixiang0/gci@latest && gci --version; \
+	go install mvdan.cc/gofumpt@latest && gofumpt --version
 
 ########################
 ### Makefile Helpers ###
@@ -71,6 +72,36 @@ check_godoc:
 	fi; \
 	}
 
+.PHONY: check_golanci_lint
+# Internal helper target - check if golangci-lint is installed
+check_golanci_lint:
+	{ \
+	if ( ! ( command -v golangci-lint >/dev/null )); then \
+		echo "Seems like you don't have golanci-lint installed. Make sure you install it via 'go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest' before continuing"; \
+		exit 1; \
+	fi; \
+	}
+
+.PHONY: check_gci
+# Internal helper target - check if gci is installed
+check_gci:
+	{ \
+	if ( ! ( command -v gci >/dev/null )); then \
+		echo "Seems like you don't have gci installed. Make sure you install it via 'go install github.com/daixiang0/gci@latest' before continuing"; \
+		exit 1; \
+	fi; \
+	}
+
+.PHONY: check_gofumpt
+# Internal helper target - check if gofumpt is installed
+check_gofumpt:
+	{ \
+	if ( ! ( command -v gofumpt >/dev/null )); then \
+		echo "Seems like you don't have gofumpt installed. Make sure you install it via 'go install mvdan.cc/gofumpt@latest' before continuing"; \
+		exit 1; \
+	fi; \
+	}
+
 .PHONY: check_npm
 # Internal helper target - check if npm is installed
 check_npm:
@@ -149,11 +180,16 @@ localnet_regenesis: ## Regenerate the localnet genesis file
 ###############
 
 .PHONY: go_lint
-go_lint: ## Run all go linters
-	golangci-lint run --timeout 5m --build-tags test
+go_lint: check_golanci_lint ## Run all go linters
+	golangci-lint run --allow-parallel-runners -c .golangci.yml ./...
+
+.PHONY: go_gci
+go_gci: check_gci ## Run gci (import ordering) on all applicable files, this writes changes in place
+	go run ./tools/scripts/gci
 
-go_imports: check_go_version ## Run goimports on all go files
-	go run ./tools/scripts/goimports
+.PHONY: go_gofumpt
+go_gofumpt: check_gofumpt ## Run gofumpt (stricter gofmt) on all applicable files, this writes changes in place
+	go run ./tools/scripts/gofumpt
 
 #############
 ### Tests ###

app/simulation_test.go

@@ -60,7 +60,9 @@ func fauxMerkleModeOpt(bapp *baseapp.BaseApp) {
 // Running using starport command:
 // `starport chain simulate -v --numBlocks 200 --blockSize 50`
 // Running as go benchmark test:
-// `go test -benchmem -run=^$ -bench ^BenchmarkSimulation ./app -NumBlocks=200 -BlockSize 50 -Commit=true -Verbose=true -Enabled=true`
+//
+//	$ go test -benchmem -run=^$ -bench ^BenchmarkSimulation ./app -NumBlocks=200 \
+//		-BlockSize 50 -Commit=true -Verbose=true -Enabled=true
 func BenchmarkSimulation(b *testing.B) {
 	simcli.FlagSeedValue = time.Now().Unix()
 	simcli.FlagVerboseValue = true
@@ -209,7 +211,8 @@ func TestAppStateDeterminism(t *testing.T) {
 			if j != 0 {
 				require.Equal(
 					t, string(appHashList[0]), string(appHashList[j]),
-					"non-determinism in seed %d: %d/%d, attempt: %d/%d\n", config.Seed, i+1, numSeeds, j+1, numTimesToRunPerSeed,
+					"non-determinism in seed %d: %d/%d, attempt: %d/%d\n",
+					config.Seed, i+1, numSeeds, j+1, numTimesToRunPerSeed,
 				)
 			}
 		}
@@ -344,7 +347,8 @@ func TestAppImportExport(t *testing.T) {
 			bApp.GetKey(stakingtypes.StoreKey), newApp.GetKey(stakingtypes.StoreKey),
 			[][]byte{
 				stakingtypes.UnbondingQueueKey, stakingtypes.RedelegationQueueKey, stakingtypes.ValidatorQueueKey,
-				stakingtypes.HistoricalInfoKey, stakingtypes.UnbondingIDKey, stakingtypes.UnbondingIndexKey, stakingtypes.UnbondingTypeKey, stakingtypes.ValidatorUpdatesKey,
+				stakingtypes.HistoricalInfoKey, stakingtypes.UnbondingIDKey, stakingtypes.UnbondingIndexKey,
+				stakingtypes.UnbondingTypeKey, stakingtypes.ValidatorUpdatesKey,
 			},
 		}, // ordering may change but it doesn't matter
 		{bApp.GetKey(slashingtypes.StoreKey), newApp.GetKey(slashingtypes.StoreKey), [][]byte{}},
@@ -355,7 +359,9 @@ func TestAppImportExport(t *testing.T) {
 		{bApp.GetKey(govtypes.StoreKey), newApp.GetKey(govtypes.StoreKey), [][]byte{}},
 		{bApp.GetKey(evidencetypes.StoreKey), newApp.GetKey(evidencetypes.StoreKey), [][]byte{}},
 		{bApp.GetKey(capabilitytypes.StoreKey), newApp.GetKey(capabilitytypes.StoreKey), [][]byte{}},
-		{bApp.GetKey(authzkeeper.StoreKey), newApp.GetKey(authzkeeper.StoreKey), [][]byte{authzkeeper.GrantKey, authzkeeper.GrantQueuePrefix}},
+		{bApp.GetKey(authzkeeper.StoreKey), newApp.GetKey(authzkeeper.StoreKey), [][]byte{
+			authzkeeper.GrantKey, authzkeeper.GrantQueuePrefix,
+		}},
 	}
 
 	for _, skp := range storeKeysPrefixes {
@@ -366,7 +372,9 @@ func TestAppImportExport(t *testing.T) {
 		require.Equal(t, len(failedKVAs), len(failedKVBs), "unequal sets of key-values to compare")
 
 		fmt.Printf("compared %d different key/value pairs between %s and %s\n", len(failedKVAs), skp.A, skp.B)
-		require.Equal(t, 0, len(failedKVAs), simtestutil.GetSimulationLog(skp.A.Name(), bApp.SimulationManager().StoreDecoders, failedKVAs, failedKVBs))
+		require.Equal(t, 0, len(failedKVAs), simtestutil.GetSimulationLog(
+			skp.A.Name(), bApp.SimulationManager().StoreDecoders, failedKVAs, failedKVBs),
+		)
 	}
 }
 

cmd/pocketd/cmd/config.go

@@ -8,8 +8,9 @@ import (
 
 // InitSDKConfig initializes the SDK's config with the appropriate parameters
 // and prefixes so everything is named appropriately for Pocket Network.
-// TODO_DISCUSS: Exporting publically for testing purposes only.
-// Consider adding a helper per the discussion here: https://github.com/pokt-network/poktroll/pull/59#discussion_r1357816798
+// TODO_DISCUSS: Exporting publicly for testing purposes only.
+// Consider adding a helper per the discussion here:
+// https://github.com/pokt-network/poktroll/pull/59#discussion_r1357816798
 func InitSDKConfig() {
 	// Set prefixes
 	accountPubKeyPrefix := app.AccountAddressPrefix + "pub"

cmd/pocketd/cmd/genaccounts.go

@@ -181,11 +181,20 @@ contain valid denominations. Accounts may optionally be supplied with vesting pa
 		},
 	}
 
-	cmd.Flags().String(flags.FlagKeyringBackend, flags.DefaultKeyringBackend, "Select keyring's backend (os|file|kwallet|pass|test)")
-	cmd.Flags().String(flags.FlagHome, defaultNodeHome, "The application home directory")
-	cmd.Flags().String(flagVestingAmt, "", "amount of coins for vesting accounts")
-	cmd.Flags().Int64(flagVestingStart, 0, "schedule start time (unix epoch) for vesting accounts")
-	cmd.Flags().Int64(flagVestingEnd, 0, "schedule end time (unix epoch) for vesting accounts")
+	cmd.Flags().
+		String(
+			flags.FlagKeyringBackend,
+			flags.DefaultKeyringBackend,
+			"Select keyring's backend (os|file|kwallet|pass|test)",
+		)
+	cmd.Flags().
+		String(flags.FlagHome, defaultNodeHome, "The application home directory")
+	cmd.Flags().
+		String(flagVestingAmt, "", "amount of coins for vesting accounts")
+	cmd.Flags().
+		Int64(flagVestingStart, 0, "schedule start time (unix epoch) for vesting accounts")
+	cmd.Flags().
+		Int64(flagVestingEnd, 0, "schedule end time (unix epoch) for vesting accounts")
 	flags.AddQueryFlagsToCmd(cmd)
 
 	return cmd