ci: Use golangci-lint for linting (#121)

Instead of hand-managing and running linters, use golangci-lint.

Along with the golangci-lint defaults,
enable a couple other linters we generally agree with.
See also uber-go/zap#1323 for a similar change.

As a result of this, we can:

- Drop the dependabot for tools
- Run the lint job in parallel with build/test
- Simplify the Makefile

.github/dependabot.yml

@@ -5,11 +5,6 @@ updates:
     schedule:
       interval: "daily"
 
-  - package-ecosystem: "gomod"
-    directory: "/tools"
-    schedule:
-      interval: "daily"
-
   - package-ecosystem: "github-actions"
     directory: "/"
     schedule:

.github/workflows/go.yml

@@ -23,15 +23,37 @@ jobs:
         go-version: 1.21.x
         cache: true
 
-    - name: Lint
-      run: make lint
+    - name: Build
+      run: make build
 
     - name: Test
       run: make cover
 
     - name: Upload coverage to codecov.io
       uses: codecov/codecov-action@v3
 
+  lint:
+    name: Lint
+    runs-on: ubuntu-latest
+
+    steps:
+    - uses: actions/checkout@v4
+      name: Check out repository
+    - uses: actions/setup-go@v4
+      name: Set up Go
+      with:
+        go-version: 1.21.x
+        cache: false  # managed by golangci-lint
+
+    - uses: golangci/golangci-lint-action@v3
+      name: Install golangci-lint
+      with:
+        version: latest
+        args: --version  # make lint will run the linter
+
+    - run: make lint
+      name: Lint
+
   docker:
     runs-on: ubuntu-latest
     steps:

.golangci.yml

@@ -0,0 +1,28 @@
+output:
+  # Make output more digestible with quickfix in vim/emacs/etc.
+  sort-results: true
+  print-issued-lines: false
+
+linters:
+  enable:
+    - gofumpt
+    - nolintlint
+    - revive
+
+linters-settings:
+  govet:
+    # These govet checks are disabled by default, but they're useful.
+    enable:
+      - niliness
+      - reflectvaluecompare
+      - sortslice
+      - unusedwrite
+
+issues:
+  # Print all issues reported by all linters.
+  max-issues-per-linter: 0
+  max-same-issues: 0
+
+  # Don't ignore some of the issues that golangci-lint considers okay.
+  # This includes documenting all exported entities.
+  exclude-use-default: false

Makefile

@@ -1,61 +1,47 @@
-export GOBIN = $(shell pwd)/bin
+# Directory containing the Makefile.
+PROJECT_ROOT = $(dir $(abspath $(lastword $(MAKEFILE_LIST))))
+
+export GOBIN = $(PROJECT_ROOT)/bin
 export PATH := $(GOBIN):$(PATH)
 
 GO_FILES = $(shell find . \
 	   -path '*/.*' -prune -o \
 	   '(' -type f -a -name '*.go' ')' -print)
 
-REVIVE = bin/revive
-STATICCHECK = bin/staticcheck
-TOOLS = $(REVIVE) $(STATICCHECK)
-
 TEST_FLAGS ?= -race
 
 .PHONY: all
-all: lint install test
+all: lint build test
 
 .PHONY: lint
-lint: gofmt revive staticcheck
-
-.PHONY: gofmt
-gofmt:
-	$(eval FMT_LOG := $(shell mktemp -t gofmt.XXXXX))
-	@gofmt -e -s -l $(GO_FILES) > $(FMT_LOG) || true
-	@[ ! -s "$(FMT_LOG)" ] || \
-		(echo "gofmt failed. Please reformat the following files:" | \
-		cat - $(FMT_LOG) && false)
-
-.PHONY: staticcheck
-staticcheck: $(STATICCHECK)
-	$(STATICCHECK) ./...
-
-$(STATICCHECK): tools/go.mod
-	cd tools && go install honnef.co/go/tools/cmd/staticcheck
-
-.PHONY: revive
-revive: $(REVIVE)
-	$(REVIVE) -set_exit_status ./...
+lint: golangci-lint tidy-lint
 
-$(REVIVE): tools/go.mod
-	cd tools && go install github.com/mgechev/revive
-
-.PHONY: install
-install:
+.PHONY: build
+build:
 	go install .
 
 .PHONY: test
 test:
 	go test $(TEST_FLAGS) ./...
 
+.PHONY: build
+run: build
+	sally
+
 .PHONY: cover
 cover:
 	go test $(TEST_FLAGS) -coverprofile=cover.out -covermode=atomic -coverpkg=./... ./...
 	go tool cover -html=cover.out -o cover.html
 
-.PHONY: clean
-clean:
-	rm -rf _tmp
+.PHONY: golangci-lint
+golangci-lint:
+	golangci-lint run
 
-.PHONY: install
-run: install
-	sally
+.PHONY: tidy
+tidy:
+	go mod tidy
+
+.PHONY: tidy-lint
+tidy-lint:
+	go mod tidy
+	git diff --exit-code -- go.mod go.sum

handler_test.go

@@ -168,7 +168,9 @@ func TestPostRejected(t *testing.T) {
 
 			res, err := http.Post(srv.URL+tt.path, "text/plain", strings.NewReader("foo"))
 			require.NoError(t, err)
-			defer res.Body.Close()
+			defer func() {
+				assert.NoError(t, res.Body.Close())
+			}()
 
 			body, err := io.ReadAll(res.Body)
 			require.NoError(t, err)