build(lint): enabled more linters (#3)

pamburus · Nov 11, 2024 · 0883e51 · 0883e51
1 parent 907a737
commit 0883e51
Show file tree

Hide file tree

Showing 20 changed files with 326 additions and 69 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -20,17 +20,14 @@ jobs:
       - uses: actions/checkout@v4
       - uses: actions/setup-go@v5
         with:
-          go-version: '1.21'
+          go-version: '1.23'
           cache-dependency-path: |
             go.sum
             test/go.sum
       - name: Lint .
-        uses: golangci/golangci-lint-action@v3
+        uses: golangci/golangci-lint-action@v6
         with:
-          version: v1.56
-          # Disable caching as a workaround for https://github.com/golangci/golangci-lint-action/issues/135.
-          # The line can be removed once the golangci-lint issue is resolved.
-          skip-pkg-cache: true
+          version: v1.61
       - name: Lint other modules
         run: go list -m -f '{{.Dir}}/...' | golangci-lint run
 
@@ -39,7 +36,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        go: ['1.21']
+        go: ['1.23']
     steps:
       - uses: actions/checkout@v4
       - uses: actions/setup-go@v5

diff --git a/.golangci.yml b/.golangci.yml
@@ -13,67 +13,148 @@ linters-settings:
     min-complexity: 32
   goimports:
     local-prefixes: github.com/pamburus
-  govet:
-    check-shadowing: false
   lll:
     line-length: 140
-  maligned:
-    suggest-new: true
   nolintlint:
-    allow-unused: false # report any unused nolint directives
+    allow-unused: true # report any unused nolint directives
     require-explanation: true # require an explanation for nolint directives
-    require-specific: false # don't require nolint directives to be specific about which linter is being skipped
+    require-specific: true # don't require nolint directives to be specific about which linter is being skipped
+  tagliatelle:
+    # Check the struck tag name case.
+    case:
+      # Use the struct field name to check the name of the struct tag.
+      # Default: false
+      use-field-name: true
+      rules:
+        # Any struct tag type can be used.
+        # Support string case: `camel`, `pascal`, `kebab`, `snake`, `upperSnake`, `goCamel`, `goPascal`, `goKebab`, `goSnake`, `upper`, `lower`, `header`.
+        json: kebab
+        yaml: kebab
 
 linters:
-  # Please, do not use `enable-all`: it's deprecated and will be removed soon.
-  # Inverted configuration with `enable-all` and `disable` is not scalable during updates of golangci-lint.
+  # please, do not use `enable-all`: it's deprecated and will be removed soon.
+  # inverted configuration with `enable-all` and `disable` is not scalable during updates of golangci-lint
   disable-all: true
   enable:
+    - asasalint
     - asciicheck
+    - bidichk
+    - bodyclose
+    - canonicalheader
+    - containedctx
+    - copyloopvar
+    - decorder
+    - dogsled
     - dupl
+    - dupword
+    - durationcheck
     - errcheck
-    - exportloopref
+    - errchkjson
+    - errname
+    - errorlint
+    - fatcontext
+    - forbidigo
+    - forcetypeassert
+    - ginkgolinter
+    - gocheckcompilerdirectives
+    - gochecknoinits
+    - gochecksumtype
+    - gocognit
+    - goconst
     - gocritic
     - gocyclo
     - godot
+    - godox
     - gofmt
+    - gofumpt
+    - goheader
     - goimports
+    - gomoddirectives
+    - gomodguard
+    - goprintffuncname
     - gosec
     - gosimple
+    - gosmopolitan
     - govet
+    - grouper
+    - importas
     - ineffassign
+    - interfacebloat
+    - intrange
+    - lll
+    - loggercheck
+    - maintidx
+    - makezero
+    - mirror
     - misspell
+    - musttag
     - nakedret
+    - nestif
+    - nilerr
+    - nilnil
     - nlreturn
+    - noctx
+    - nolintlint
+    - nonamedreturns
+    - nosprintfhostport
+    - perfsprint
     - prealloc
+    - predeclared
+    - promlinter
+    - protogetter
+    - reassign
     - revive
+    - rowserrcheck
+    - sloglint
+    - spancheck
+    - sqlclosecheck
     - staticcheck
     - stylecheck
-    - typecheck
+    - tagalign
+    - tagliatelle
+    - tenv
+    - testableexamples
+    - testifylint
+    - testpackage
+    - thelper
+    - tparallel
     - unconvert
     - unparam
     - unused
-    - vet
-    - vetshadow
+    - usestdlibvars
+    - wastedassign
     - whitespace
+    - wrapcheck
+    - wsl
+    - zerologlint
 
 issues:
-  # Disable this option because it causes golint to hide almost all issues.
+  # Disable this option because it causes golint to hide almost all issues
   exclude-use-default: false
-  # Excluding configuration per-path, per-linter, per-text and per-source.
+  # Excluding configuration per-path, per-linter, per-text and per-source
   exclude-rules:
-    # The dot-imports linter is disabled for the test files because it is convenient to dot-import third-party testing frameworks.
-    - linters: [revive]
-      text: '^dot-imports: '
-      paths:
-        - '*_test.go'
-    # The staticcheck linter reports that `nil` context is passed, but it is intentionally done for the sake of the tests.
-    - linters: [staticcheck]
-      text: '^SA1012:'
-      paths:
-        - '*_test.go'
-    # The duplicate lines in the benchmark_test.go file are expected because any attempt to deduplicate them would affect the performance of the benchmark.
-    - linters: [dupl]
-      text: 'lines are duplicate'
-      paths:
-        - 'benchmark_test.go'
+    - linters: [gocritic]
+      text: '^singleCaseSwitch:'
+    # - linters: [gosec]
+    #   text: '^G115:' # false positives
+    - linters: [gosec] # Use of unsafe calls should be audited
+      text: '^G103:'   # `unsafe` is used for optimizations and is covered by unit tests
+    - path: '_test\.go$'
+      text: '^dot-imports:'
+      linters:
+        - revive
+    - path: '_test\.go$'
+      linters:
+        - dupl
+        - err113
+        - gocognit
+        - goconst
+        - lll
+        - maintidx
+        - testpackage
+        - varnamelen
+        - wsl
+    - linters: [wsl]
+      text: '^append only allowed to cuddle with appended value$'
+    - linters: [wrapcheck]
+      text: '^error returned from interface method should be wrapped:'
diff --git a/Makefile b/Makefile
@@ -0,0 +1,106 @@
+# Common makefile helpers
+include build/make/common.mk
+
+# Common configuration
+SHELL := $(SHELL) -o pipefail
+
+# Set default goal
+.DEFAULT_GOAL := all
+
+# Some constants
+import-path := github.com/pamburus/slogx
+
+# Populate complete module list, including build tools
+ifndef all-modules
+all-modules := $(shell go list -m -f '{{.Dir}}')
+all-modules := $(subst \,/,$(all-modules))
+all-modules := $(all-modules:$(abspath .)=.)
+all-modules := $(all-modules:$(abspath .)/%=%)
+endif
+
+# Auxiliary modules, not to be tested
+aux-modules +=
+
+# Populate module list to test
+ifndef modules
+modules := $(filter-out $(aux-modules),$(all-modules))
+endif
+
+# Tools
+go-test := go test
+go-tool-cover := go tool cover
+coverage-filter := go run github.com/pamburus/go-mod/build/tools/cmd/coverage-filter@latest
+test-filter := go run github.com/pamburus/go-mod/build/tools/cmd/test-filter@latest
+ifeq ($(verbose),yes)
+	coverage-filter += -v
+	go-test += -v
+endif
+
+## Run all tests
+.PHONY: all
+all: ci
+
+# ---
+
+## Run continuous integration tests
+.PHONY: ci
+ci: lint test
+
+# Run continuous integration tests for a module
+.PHONY: ci@/%
+ci@/%: lint@/% test@/%
+
+# ---
+
+## Run linters
+.PHONY: lint
+lint: $(modules:%=lint@/%)
+
+# Run linters for a module
+.PHONY: lint@/%
+lint@/%:
+	golangci-lint run $*/...
+
+# ---
+
+## Run tests
+.PHONY: test
+test: $(modules:%=test@/%)
+
+# Run tests for a module
+.PHONY: test@/%
+test@/%:
+	$(go-test) -fullpath -coverprofile=$*/.cover.out ./$*/... | $(test-filter)
+test@/.:
+	$(go-test) -fullpath -coverprofile=.cover.out ./... | $(test-filter)
+
+# ---
+
+## Show coverage
+.PHONY: coverage
+coverage: $(modules:%=coverage@/%)
+
+# Show coverage for a module
+.PHONY: coverage@/%
+coverage@/%: test@/%
+	$(go-tool-cover) -func=$*/.cover.out | $(coverage-filter) $(import-path)
+
+# ---
+
+## Tidy up
+.PHONY: tidy
+tidy: $(all-modules:%=tidy@/%)
+#	go work sync
+
+# Tidy up a module
+.PHONY: tidy@/%
+tidy@/%:
+	cd $* && go mod tidy
+
+# ---
+
+## Clean up
+.PHONY: clean
+clean:
+	rm -f $(modules:%=%/.cover.out)
+	find . -type f -name go.work.sum -delete
diff --git a/attr.go b/attr.go
@@ -36,11 +36,12 @@ func (r AttrPack) Len() int {
 
 // Enumerate calls f on each Attr in the AttrPack.
 func (r AttrPack) Enumerate(f func(slog.Attr) bool) {
-	for i := 0; i < r.nFront; i++ {
+	for i := range r.nFront {
 		if !f(r.front[i]) {
 			return
 		}
 	}
+
 	for _, a := range r.back {
 		if !f(a) {
 			return
@@ -56,17 +57,21 @@ func (r *AttrPack) Add(attrs ...slog.Attr) {
 		if isEmptyGroup(a.Value) {
 			continue
 		}
+
 		r.front[r.nFront] = a
 		r.nFront++
 	}
+
 	if cap(r.back) > len(r.back) {
 		end := r.back[:len(r.back)+1][len(r.back)]
 		if !end.Equal(slog.Attr{}) {
 			panic("multiple copies of a slogx.AttrPack modified the shared state simultaneously, use Clone to avoid this")
 		}
 	}
+
 	ne := countEmptyGroups(attrs[i:])
 	r.back = slices.Grow(r.back, len(attrs[i:])-ne)
+
 	for _, a := range attrs[i:] {
 		if !isEmptyGroup(a.Value) {
 			r.back = append(r.back, a)
@@ -90,6 +95,7 @@ func (r *AttrPack) Collect() []slog.Attr {
 
 func countEmptyGroups(as []slog.Attr) int {
 	n := 0
+
 	for _, a := range as {
 		if isEmptyGroup(a.Value) {
 			n++