ci: Use golangci-lint (#1150)

.github/workflows/go.yml

@@ -14,14 +14,12 @@ jobs:
 
   build:
     runs-on: ${{ matrix.os }}
+    name: Build and test
+
     strategy:
       matrix:
         os: ["ubuntu-latest", "windows-latest"]
         go: ["1.20.x", "1.21.x"]
-        include:
-        - go: 1.21.x
-          os: "ubuntu-latest"
-          latest: true
 
     steps:
     - name: Checkout code
@@ -31,21 +29,51 @@ jobs:
       uses: actions/setup-go@v5
       with:
         go-version: ${{ matrix.go }}
-        cache: true
 
     - name: Download Dependencies
       run: go mod download
 
-    - name: Lint
-      if: matrix.latest
-      run: make lint
-
     - name: Test
       run: make cover
 
-    - name: Test documentation
-      run: make cover COVER_MODULES=./docs
-      if: matrix.latest
-
     - name: Upload coverage to codecov.io
       uses: codecov/codecov-action@v3
+
+  doctest:
+    runs-on: ubuntu-latest
+    name: Test documentation
+
+    steps:
+    - name: Checkout code
+      uses: actions/checkout@v4
+
+    - name: Setup Go
+      uses: actions/setup-go@v5
+      with:
+        go-version: 1.21.x
+
+    - name: Test
+      run: make cover COVER_MODULES=./docs
+
+  lint:
+    name: Lint
+    runs-on: ubuntu-latest
+
+    steps:
+    - name: Checkout code
+      uses: actions/checkout@v4
+
+    - name: Setup Go
+      uses: actions/setup-go@v5
+      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

.golangci.yml

@@ -0,0 +1,70 @@
+output:
+  # Make output more digestible with quickfix in vim/emacs/etc.
+  sort-results: true
+  print-issued-lines: false
+
+linters:
+  # We'll track the golangci-lint default linters manually
+  # instead of letting them change without our control.
+  disable-all: true
+  enable:
+    # golangci-lint defaults:
+    - gosimple
+    - govet
+    - ineffassign
+    - staticcheck
+    - unused
+
+    # Our own extras:
+    - gofumpt
+    - nolintlint # lints nolint directives
+    - revive
+    - errorlint
+
+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
+
+  exclude-rules:
+    # Don't warn on unused parameters.
+    # Parameter names are useful; replacing them with '_' is undesirable.
+    - linters: [revive]
+      text: 'unused-parameter: parameter \S+ seems to be unused, consider removing or renaming it as _'
+
+    # staticcheck already has smarter checks for empty blocks.
+    # revive's empty-block linter has false positives.
+    # For example, as of writing this, the following is not allowed.
+    #   for foo() { }
+    - linters: [revive]
+      text: 'empty-block: this block is empty, you can remove it'
+
+    # It's okay if internal packages and examples in docs/
+    # don't have package comments.
+    - linters: [revive]
+      path: '.+/internal/.+|^internal/.+|^docs/.+'
+      text: 'should have a package comment'
+
+    # It's okay for tests to use dot imports.
+    - linters: [revive]
+      path: '_test\.go$'
+      text: 'should not use dot imports'
+
+    # Ignore logger.Sync() errcheck failures in example_test.go
+    # since those are intended to be uncomplicated examples.
+    - linters: [errcheck]
+      path: example_test.go
+      text: 'Error return value of `logger.Sync` is not checked'

Makefile

@@ -1,27 +1,27 @@
-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)
 
-GOLINT = $(GOBIN)/golint
-STATICCHECK = $(GOBIN)/staticcheck
 FXLINT = $(GOBIN)/fxlint
 MDOX = $(GOBIN)/mdox
 
-GO_FILES = $(shell \
-	find . '(' -path '*/.*' -o -path './vendor' -o -path '*/testdata/*' ')' -prune \
-	-o -name '*.go' -print | cut -b3-)
-
 MODULES = . ./tools ./docs ./internal/e2e
 
 # 'make cover' should not run on docs by default.
 # We run that separately explicitly on a specific platform.
 COVER_MODULES ?= $(filter-out ./docs,$(MODULES))
 
+.PHONY: all
+all: build lint test
+
 .PHONY: build
 build:
 	go build ./...
 
-.PHONY: install
-install:
-	go mod download
+.PHONY: lint
+lint: golangci-lint tidy-lint license-lint fx-lint docs-lint
 
 .PHONY: test
 test:
@@ -35,52 +35,45 @@ cover:
 		go test -race -coverprofile=cover.out -coverpkg=./... ./... && \
 		go tool cover -html=cover.out -o cover.html) &&) true
 
-$(GOLINT): tools/go.mod
-	cd tools && go install golang.org/x/lint/golint
+.PHONY: tidy
+tidy:
+	@$(foreach dir,$(MODULES),(cd $(dir) && go mod tidy) &&) true
+
+.PHONY: docs
+docs:
+	cd docs && yarn build
 
-$(STATICCHECK): tools/go.mod
-	cd tools && go install honnef.co/go/tools/cmd/staticcheck
+.PHONY: golangci-lint
+golangci-lint:
+	@$(foreach mod,$(MODULES), \
+		(cd $(mod) && \
+		echo "[lint] golangci-lint: $(mod)" && \
+		golangci-lint run --path-prefix $(mod)) &&) true
+
+.PHONY: tidy-lint
+tidy-lint:
+	@$(foreach mod,$(MODULES), \
+		(cd $(mod) && \
+		echo "[lint] tidy: $(mod)" && \
+		go mod tidy && \
+		git diff --exit-code -- go.mod go.sum) &&) true
+
+.PHONY: license-lint
+license-lint:
+	./checklicense.sh
+
+.PHONY: fx-lint
+fx-lint: $(FXLINT)
+	@$(FXLINT) ./...
+
+.PHONY: docs-lint
+docs-lint: $(MDOX)
+	@echo "Checking documentation"
+	@make -C docs check
 
 $(MDOX): tools/go.mod
 	cd tools && go install github.com/bwplotka/mdox
 
 $(FXLINT): tools/cmd/fxlint/main.go
 	cd tools && go install go.uber.org/fx/tools/cmd/fxlint
 
-.PHONY: lint
-lint: $(GOLINT) $(STATICCHECK) $(FXLINT) docs-check
-	@rm -rf lint.log
-	@echo "Checking formatting..."
-	@gofmt -d -s $(GO_FILES) 2>&1 | tee lint.log
-	@echo "Checking vet..."
-	@$(foreach dir,$(MODULES),(cd $(dir) && go vet ./... 2>&1) &&) true | tee -a lint.log
-	@echo "Checking lint..."
-	@$(foreach dir,$(MODULES),(cd $(dir) && $(GOLINT) ./... 2>&1) &&) true | tee -a lint.log
-	@echo "Checking staticcheck..."
-	@$(foreach dir,$(MODULES),(cd $(dir) && $(STATICCHECK) ./... 2>&1) &&) true | tee -a lint.log
-	@echo "Checking fxlint..."
-	@$(FXLINT) ./... | tee -a lint.log
-	@echo "Checking for unresolved FIXMEs..."
-	@git grep -i fixme | grep -v -e vendor -e Makefile -e .md | tee -a lint.log
-	@echo "Checking for license headers..."
-	@./checklicense.sh | tee -a lint.log
-	@[ ! -s lint.log ]
-	@echo "Checking 'go mod tidy'..."
-	@make tidy
-	@if ! git diff --quiet; then \
-		echo "'go mod tidy' resulted in changes or working tree is dirty:"; \
-		git --no-pager diff; \
-	fi
-
-.PHONY: docs
-docs:
-	cd docs && yarn build
-
-.PHONY: docs-check
-docs-check: $(MDOX)
-	@echo "Checking documentation"
-	@make -C docs check | tee -a lint.log
-
-.PHONY: tidy
-tidy:
-	@$(foreach dir,$(MODULES),(cd $(dir) && go mod tidy) &&) true

annotated.go

@@ -117,8 +117,8 @@ type Annotation interface {
 }
 
 var (
-	_typeOfError reflect.Type = reflect.TypeOf((*error)(nil)).Elem()
-	_nilError                 = reflect.Zero(_typeOfError)
+	_typeOfError = reflect.TypeOf((*error)(nil)).Elem()
+	_nilError    = reflect.Zero(_typeOfError)
 )
 
 // annotationError is a wrapper for an error that was encountered while
@@ -178,7 +178,6 @@ func verifyValueQuote(value string) (string, error) {
 		return "", errTagValueSyntaxEndingQuote
 	}
 	return value[i+1:], nil
-
 }
 
 // Check whether the tag follows valid struct.
@@ -211,7 +210,6 @@ func verifyAnnotateTag(tag string) error {
 		tag = value
 	}
 	return nil
-
 }
 
 // Given func(T1, T2, T3, ..., TN), this generates a type roughly
@@ -644,8 +642,8 @@ func (la *lifecycleHookAnnotation) build(ann *annotated) (interface{}, error) {
 }
 
 var (
-	_typeOfLifecycle reflect.Type = reflect.TypeOf((*Lifecycle)(nil)).Elem()
-	_typeOfContext   reflect.Type = reflect.TypeOf((*context.Context)(nil)).Elem()
+	_typeOfLifecycle = reflect.TypeOf((*Lifecycle)(nil)).Elem()
+	_typeOfContext   = reflect.TypeOf((*context.Context)(nil)).Elem()
 )
 
 // buildHookInstaller returns a function that appends a hook to Lifecycle when called,

annotated_test.go

@@ -1701,8 +1701,8 @@ func TestAnnotateApplySuccess(t *testing.T) {
 			require.NoError(t, app.Err())
 		})
 	}
-
 }
+
 func assertApp(
 	t *testing.T,
 	app interface {

app.go

@@ -547,8 +547,11 @@ func (err errorWithGraph) Error() string {
 
 // VisualizeError returns the visualization of the error if available.
 func VisualizeError(err error) (string, error) {
-	if e, ok := err.(errWithGraph); ok && e.Graph() != "" {
-		return string(e.Graph()), nil
+	var erg errWithGraph
+	if errors.As(err, &erg) {
+		if g := erg.Graph(); g != "" {
+			return string(g), nil
+		}
 	}
 	return "", errors.New("unable to visualize error")
 }

app_unixes.go

@@ -25,5 +25,7 @@ package fx
 
 import "golang.org/x/sys/unix"
 
-const _sigINT = unix.SIGINT
-const _sigTERM = unix.SIGTERM
+const (
+	_sigINT  = unix.SIGINT
+	_sigTERM = unix.SIGTERM
+)

app_wasm.go

@@ -25,5 +25,7 @@ package fx
 
 import "syscall"
 
-const _sigINT = syscall.SIGINT
-const _sigTERM = syscall.SIGTERM
+const (
+	_sigINT  = syscall.SIGINT
+	_sigTERM = syscall.SIGTERM
+)

app_windows.go

@@ -25,5 +25,7 @@ package fx
 
 import "golang.org/x/sys/windows"
 
-const _sigINT = windows.SIGINT
-const _sigTERM = windows.SIGTERM
+const (
+	_sigINT  = windows.SIGINT
+	_sigTERM = windows.SIGTERM
+)

decorate.go

@@ -221,7 +221,7 @@ func runDecorator(c container, d decorator, opts ...dig.DecorateOption) (err err
 	decorator := d.Target
 	defer func() {
 		if err != nil {
-			err = fmt.Errorf("fx.Decorate(%v) from:\n%+vFailed: %v", decorator, d.Stack, err)
+			err = fmt.Errorf("fx.Decorate(%v) from:\n%+vFailed: %w", decorator, d.Stack, err)
 		}
 	}()
 

decorate_test.go

@@ -315,8 +315,7 @@ func TestDecorateSuccess(t *testing.T) {
 	})
 
 	t.Run("decoration must execute when required by a member of group", func(t *testing.T) {
-		type Drinks interface {
-		}
+		type Drinks interface{}
 		type Coffee struct {
 			Type  string
 			Name  string