feat: add gofumpt (#2049)

Our codebase is already sufficiently formatted, so we can enable **`gofumpt` by default**. This PR also merges `lint-typo`, `lint-fix-typo`, `format`, `lint-format`, and `lint` Makefile targets into a single `lint` target to ease the software development process. `codespell` CI job is removed in favor of `lint` job. Closes #604 --------- Co-authored-by: Greg Szabo <[email protected]>
cometbft · Jan 25, 2024 · e9637ad · e9637ad
1 parent e8b4efa
commit e9637ad
Show file tree

Hide file tree

Showing 12 changed files with 1,538 additions and 1,545 deletions.
diff --git a/.github/workflows/codespell.yml b/.github/workflows/codespell.yml
diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
@@ -1,11 +1,8 @@
-name: Golang Linter
-# Lint runs golangci-lint over the entire CometBFT repository.
+name: Lint, format and check the code for typos
+# Lint runs `make lint`
 #
 # This workflow is run on every pull request and push to main.
 #
-# The `golangci` job will pass without running if no *.{go, mod, sum}
-# files have been modified.
-#
 # To run this locally, simply run `make lint` from the root of the repo.
 
 on:
@@ -14,25 +11,35 @@ on:
   push:
     branches:
       - main
+
 jobs:
-  golangci:
-    name: golangci-lint
+  lint:
+    name: lint
     runs-on: ubuntu-latest
-    timeout-minutes: 8
+    timeout-minutes: 10
     steps:
       - uses: actions/checkout@v4
+
       - uses: actions/setup-go@v5
         with:
           go-version: '1.21'
-      - uses: technote-space/get-diff-action@v6
-        with:
-          PATTERNS: |
-            **/**.go
-            go.mod
-            go.sum
-      - uses: golangci/golangci-lint-action@v3
+
+      - uses: actions/setup-python@v5
         with:
-          version: latest
-          args: --timeout 10m
-          github-token: ${{ secrets.github_token }}
-        if: env.GIT_DIFF
+          python-version: '3.x'
+
+      - name: "Check the source code"
+        run: |
+          set -euo pipefail
+
+          make lint
+
+          if ! git diff --stat --exit-code ; then
+            echo ">> ERROR:"
+            echo ">>"
+            echo ">> The source code is not linted, not formatted or contains typos."
+            echo ">> Re-run 'make lint' and update this PR."
+            echo ">>"
+            git diff
+            exit 1
+          fi
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -485,6 +485,13 @@ means that you shouldn't update someone else's branch for them; even if it seems
 like you're doing them a favor, you may be interfering with their git flow in
 some way!)
 
+### Formatting
+
+Make sure to format your code with [gofumpt](https://github.com/mvdan/gofumpt).
+To install a Git pre-commit hook, run `make gofumpt-pre-commit`. The hook
+automatically applies `gofumpt` to the staged Go files, so you don't need to do
+anything.
+
 #### Merging Pull Requests
 
 It is also our convention that authors merge their own pull requests, when

diff --git a/Makefile b/Makefile
@@ -112,6 +112,7 @@ testdata-metrics:
 #? mockery: Generate test mocks
 mockery:
 	go generate -run="./scripts/mockery_generate.sh" ./...
+	@go run mvdan.cc/gofumpt@latest -l -w .
 .PHONY: mockery
 
 ###############################################################################
@@ -161,6 +162,7 @@ proto-check-breaking: check-proto-deps
 	@go run github.com/bufbuild/buf/cmd/buf@latest breaking --against ".git"
 .PHONY: proto-check-breaking
 
+#? proto-check-breaking-ci: Check for breaking changes in Protobuf files against v0.34.x. This is only useful if your changes have not yet been committed
 proto-check-breaking-ci:
 	@go run github.com/bufbuild/buf/cmd/buf@latest breaking --against $(HTTPS_GIT)#branch=v0.34.x
 .PHONY: proto-check-breaking-ci
@@ -190,7 +192,7 @@ dist:
 	@BUILD_TAGS=$(BUILD_TAGS) sh -c "'$(CURDIR)/scripts/dist.sh'"
 .PHONY: dist
 
-#? go-mod-cache: Download go modules to local cache 
+#? go-mod-cache: Download go modules to local cache
 go-mod-cache: go.sum
 	@echo "--> Download go modules to local cache"
 	@go mod download
@@ -240,41 +242,28 @@ clean_certs:
 ###                  Formatting, linting, and vetting                       ###
 ###############################################################################
 
-# https://github.com/cometbft/cometbft/pull/1925#issuecomment-1875127862
-# Revisit using format after CometBFT v1 release and/or after 2024-06-01.
-#format:
-#	find . -name '*.go' -type f -not -path "*.git*" -not -name '*.pb.go' -not -name '*pb_test.go' | xargs gofmt -w -s
-#	find . -name '*.go' -type f -not -path "*.git*"  -not -name '*.pb.go' -not -name '*pb_test.go' | xargs goimports -w -local github.com/cometbft/cometbft
-#.PHONY: format
-
-#? lint: Run latest golangci-lint linter
+#? lint: Lint, format and fix typos
 lint:
-	@echo "--> Running linter"
-	@go run github.com/golangci/golangci-lint/cmd/golangci-lint@latest run
+	@echo "--> Linting"
+	@go run github.com/golangci/golangci-lint/cmd/golangci-lint@latest run --fix
+	@echo "--> Formatting"
+	@go run mvdan.cc/gofumpt@latest -w .
+	@echo "--> Fixing typos"
+	@which codespell || pip3 install codespell
+	@codespell -w
 .PHONY: lint
 
-# https://github.com/cometbft/cometbft/pull/1925#issuecomment-1875127862
-# Revisit using lint-format after CometBFT v1 release and/or after 2024-06-01.
-#lint-format:
-#	@go run github.com/golangci/golangci-lint/cmd/golangci-lint@latest run --fix
-#	@go run mvdan.cc/gofumpt -l -w ./..
-#.PHONY: lint-format
-
 #? vulncheck: Run latest govulncheck
 vulncheck:
 	@go run golang.org/x/vuln/cmd/govulncheck@latest ./...
 .PHONY: vulncheck
 
-#? lint-typo: Run codespell to check typos
-lint-typo:
-	which codespell || pip3 install codespell
-	@codespell
-.PHONY: lint-typo
-
-#? lint-typo: Run codespell to auto fix typos
-lint-fix-typo:
-	@codespell -w
-.PHONY: lint-fix-typo
+#? gofumpt-pre-commit: Create gofumpt pre-commit hook
+gofumpt-pre-commit:
+	@echo "--> Creating gofumpt pre-commit hook"
+	@mkdir -p .git/hooks
+	@cp scripts/gofumpt.sh .git/hooks/pre-commit
+.PHONY: gofumpt-pre-commit
 
 DESTINATION = ./index.html.md
 

diff --git a/rpc/jsonrpc/client/integration_test.go b/rpc/jsonrpc/client/integration_test.go
@@ -25,7 +25,7 @@ func TestWSClientReconnectWithJitter(t *testing.T) {
 	// Max wait time is ceil(1+0.999) + ceil(2+0.999) + ceil(4+0.999) + ceil(...) = 2 + 3 + 5 = 10s + ...
 	maxSleepTime := time.Second * time.Duration(((1<<uint(maxReconnectAttempts))-1)+maxReconnectAttempts)
 
-	var errNotConnected = errors.New("not connected")
+	errNotConnected := errors.New("not connected")
 	clientMap := make(map[int]*WSClient)
 	buf := new(bytes.Buffer)
 	logger := log.NewTMLogger(buf)

diff --git a/scripts/gofumpt.sh b/scripts/gofumpt.sh
@@ -0,0 +1,23 @@
+#!/bin/bash
+# Format staged Go files with `gofumpt`
+
+# Get the list of staged Go files.
+files=$(git diff --cached --name-only --diff-filter=ACMR | grep '\.go$')
+
+# Check if gofumpt is installed.
+if ! command -v gofumpt &> /dev/null; then
+	echo "Error: gofumpt is not installed. Please install it by running 'go install mvdan.cc/gofumpt@latest'" >&2
+	exit 1
+fi
+
+# Check if there are Go files to format.
+if [ -n "$files" ]; then
+	echo "Running gofumpt on staged Go files:"
+	for file in $files; do
+		gofumpt -l -w "$file"
+		git add "$file"
+	done
+fi
+
+# Continue with the regular commit process.
+exit 0