From d871ecbbb513ef9c94ed5cfc63c70079837fcf4b Mon Sep 17 00:00:00 2001 From: kopi-solarpunk Date: Mon, 29 Apr 2024 13:15:14 +0200 Subject: [PATCH 1/7] ci: add strict linter rules --- .golangci-strict.yml | 176 +++++++++++++++++++++++++++++++++++++++++++ CODING.md | 6 +- CODINGSTYLE.md | 13 ++-- Makefile | 10 ++- 4 files changed, 198 insertions(+), 7 deletions(-) create mode 100644 .golangci-strict.yml diff --git a/.golangci-strict.yml b/.golangci-strict.yml new file mode 100644 index 00000000000..c32761ca367 --- /dev/null +++ b/.golangci-strict.yml @@ -0,0 +1,176 @@ +run: + timeout: 10m +linters: + enable: + - asasalint + - asciicheck + - bidichk + - bodyclose + - containedctx + - contextcheck + # - copyloopvar # requires Go 1.22 + - cyclop + - decorder + # - depguard disable temporary until this issue is resolved: https://github.com/golangci/golangci-lint/issues/3906 + - dogsled + - dupl + - dupword + - durationcheck + - errcheck + - errchkjson + - errname + - errorlint + - execinquery + - exhaustive + # - exhaustruct # anoying + - exportloopref + - forbidigo + - forcetypeassert + - funlen + - gci + - ginkgolinter + - gocheckcompilerdirectives + - gochecknoglobals + - gochecknoinits + - gochecksumtype + - gocognit + - goconst + - gocritic + - gocyclo + - godot + # - godox # "TODO" is allowed + - goerr113 + - gofmt + - gofumpt + - goheader + - goimports + - gomnd + - gomoddirectives + - gomodguard + - goprintffuncname + - gosec + - gosimple + - gosmopolitan + - govet + - grouper + - importas + - inamedparam + - ineffassign + - interfacebloat + # - intrange # requires Go 1.22 + - ireturn + - lll + - loggercheck + - maintidx + - makezero + - mirror + - misspell + - musttag + - nakedret + - nestif + - nilerr + - nilnil + # - nlreturn # annoying + - noctx + - nolintlint + # - nonamedreturns # Named returns can be necessary sometimes + - nosprintfhostport + - paralleltest + - perfsprint + - prealloc + - predeclared + - promlinter + - protogetter + - reassign + - revive + - rowserrcheck + - sloglint + - spancheck + - sqlclosecheck + - staticcheck + - stylecheck + - tagalign + - tagliatelle + - tenv + - testableexamples + - testifylint + - testpackage + - thelper + - tparallel + - typecheck + - unconvert + - unparam + - unused + - usestdlibvars + # - varnamelen # needs tuning + - wastedassign + - whitespace + - wrapcheck + # - wsl # anoying + - zerologlint + +linters-settings: + govet: + enable-all: true + disable: + - fieldalignment ## temporally disabled + - shadow ## temporally disabled + goheader: + values: + regexp: + date: "20[1-2][0-9]" + template: |- + Copyright {{date}} The Swarm Authors. All rights reserved. + Use of this source code is governed by a BSD-style + license that can be found in the LICENSE file. + paralleltest: + # Ignore missing calls to `t.Parallel()` and only report incorrect uses of `t.Parallel()`. + ignore-missing: true +issues: + exclude-rules: + - linters: + - goheader + text: "go-ethereum Authors" ## disable check for other authors + - path: _test\.go + linters: + - goconst ## temporally disable goconst in test + - linters: + - forbidigo + path: cmd/bee/cmd + text: "use of `fmt.Print" ## allow fmt.Print in cmd directory + - linters: + - dogsled + path: pkg/api/(.+)_test\.go # temporally disable dogsled in api test files + - linters: + - dogsled + path: pkg/pushsync/(.+)_test\.go # temporally disable dogsled in pushsync test files + # temporally disable paralleltest in following packages + - linters: + - paralleltest + path: pkg/postage + - linters: + - paralleltest + path: pkg/log + - linters: + - paralleltest + path: pkg/statestore + - linters: + - paralleltest + path: pkg/storer + - linters: + - paralleltest + path: pkg/storage + exclude-files: + - ".*generated.*\\.go$" + - ".*\\.pb\\.go" + - "asdf.*" + exclude-use-default: false +output: + print-issued-lines: false + show-stats: true + uniq-by-line: false + sort-results: true + sort-order: + - file # filepath, line, and column. + - linter + - severity \ No newline at end of file diff --git a/CODING.md b/CODING.md index 98efe16e63f..4019ad1c851 100644 --- a/CODING.md +++ b/CODING.md @@ -6,7 +6,11 @@ Developers should keep their Go tooling up to date with the latest stable versio ## Static code analysis -Linting is done by using https://github.com/golangci/golangci-lint with configuration defined in the root of the project and also `go vet`. Executing `make lint vet` should pass without any warnings or errors. +Linting is done by using with configuration defined in the root of the project. Executing `make lint` should pass without any warnings or errors. + +To run the linter with a more strict set of rules, run `make lint-strict`. + +To run the linter only on the new or changed lines, run `make lint-new`. This uses the strict linter rules. ## Testing diff --git a/CODINGSTYLE.md b/CODINGSTYLE.md index 433d51d49a3..c5b93bcf1f4 100644 --- a/CODINGSTYLE.md +++ b/CODINGSTYLE.md @@ -46,7 +46,7 @@ Naming is difficult, but prefer the consistency of naming throughout the code base. If a naming pattern is already established in the codebase, follow it. If you are unsure, look in the Golang standard library for inspiration. -It's similar to the `gofmt` tool, the formatting isn't to everyone's liking, but it is consistent. +It's similar to the `gofumpt` tool, the formatting isn't to everyone's liking, but it is consistent. Prefer american spellings over British spellings, avoid Latin abbreviations. @@ -81,7 +81,7 @@ Prefer american spellings over British spellings, avoid Latin abbreviations. ## Code Formatting - Keep line length, argument count and function size reasonable. -- Run `make lint-style` command to lint the codebase using a set of style linters. +- Run `make lint` command to lint the codebase using a set of style linters. - Run `make format FOLDER=pkg/mypackage` to format the code using `gci` and `gfumpt` formatters. ## Unused Names @@ -155,6 +155,7 @@ func getID() (id int, err error) { return } ``` + ## Testing Use the Golang [testing package](https://pkg.go.dev/testing) from the standard library for writing tests. @@ -206,6 +207,7 @@ func TestSomething(t *testing.T) { If needed, use an underscore to disambiguate tests that are hard to name: + ```go func TestScenario_EdgeCase(t *testing.T) { ... @@ -398,7 +400,8 @@ var ErrLimitExceeded = errors.New("limit exeeded) -*Note:* in the case of serialization it might make sense to use a zero length initialized slice. For instance the JSON representation of a _nil_ slice is `null`, however a zero length allocated slice would be translated to `[]`. + +*Note:* in the case of serialization it might make sense to use a zero length initialized slice. For instance the JSON representation of a *nil* slice is `null`, however a zero length allocated slice would be translated to `[]`. - To check if a slice is empty, always use `len(s) == 0`. Do not check for `nil`. @@ -1038,7 +1041,6 @@ type Config struct { When it is not possible to use `time.Time` in these interactions, unless an alternative is agreed upon, use `string` and format timestamps as defined in [RFC 3339]. This format is used by default by [`Time.nmarshalText`] and is available for use in `Time.Format` and `time.Parse` via [`time.RFC3339`]. - [`Time.UnmarshalText`]: https://golang.org/pkg/time/#Time.UnmarshalText [`time.RFC3339`]: https://golang.org/pkg/time/#RFC3339 Although this tends to not be a problem in practice, keep in mind that the `"time"` package does not support parsing timestamps with leap seconds ([8728]), nor does it account for leap seconds in calculations ([15190]). If you compare two instants of time, the difference will not include the leap seconds that may have occurred between those two instants. @@ -1262,7 +1264,6 @@ However once the error is sent to another system, it should be clear the message See also [Don't just check errors, handle them gracefully]. - [`"pkg/errors".Cause`]: https://godoc.org/github.com/pkg/errors#Cause [Don't just check errors, handle them gracefully]: https://dave.cheney.net/2016/04/27/dont-just-check-errors-handle-them-gracefully ### Handle Type Assertion Failures @@ -1429,6 +1430,7 @@ func (s *signer) Sign(msg string) string { return signWithTime(msg, now) } ``` + @@ -1680,6 +1682,7 @@ func (f Foo) String() string { ``` + diff --git a/Makefile b/Makefile index 46da4995b14..10a94baa46d 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ GO ?= go GOBIN ?= $$($(GO) env GOPATH)/bin GOLANGCI_LINT ?= $(GOBIN)/golangci-lint -GOLANGCI_LINT_VERSION ?= v1.55.0 +GOLANGCI_LINT_VERSION ?= v1.57.2 GOGOPROTOBUF ?= protoc-gen-gogofaster GOGOPROTOBUF_VERSION ?= v1.3.1 BEEKEEPER_INSTALL_DIR ?= $(GOBIN) @@ -88,6 +88,14 @@ format: lint: linter $(GOLANGCI_LINT) run ./... +.PHONY: lint-strict +lint-strict: linter + $(GOLANGCI_LINT) run -c .golangci-strict.yml ./... + +.PHONY: lint-new +lint-new: linter + $(GOLANGCI_LINT) run -c .golangci-strict.yml -n ./... + .PHONY: linter linter: test -f $(GOLANGCI_LINT) || curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $$($(GO) env GOPATH)/bin $(GOLANGCI_LINT_VERSION) From e44cea2dd263e06f1bd1526c5a261b34454c70f4 Mon Sep 17 00:00:00 2001 From: kopi-solarpunk Date: Mon, 13 May 2024 12:24:14 +0200 Subject: [PATCH 2/7] ci: add linter rules --- .golangci-strict.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.golangci-strict.yml b/.golangci-strict.yml index c32761ca367..83671dafe37 100644 --- a/.golangci-strict.yml +++ b/.golangci-strict.yml @@ -11,7 +11,7 @@ linters: # - copyloopvar # requires Go 1.22 - cyclop - decorder - # - depguard disable temporary until this issue is resolved: https://github.com/golangci/golangci-lint/issues/3906 + # - depguard # a list of acceptable packages is necessary to use this - dogsled - dupl - dupword @@ -128,6 +128,10 @@ linters-settings: ignore-missing: true issues: exclude-rules: + - path: _test\.go + linters: + - funlen + - dupl - linters: - goheader text: "go-ethereum Authors" ## disable check for other authors @@ -165,6 +169,7 @@ issues: - ".*\\.pb\\.go" - "asdf.*" exclude-use-default: false + max-issues-per-linter: 0 output: print-issued-lines: false show-stats: true From a8829dab45dc136c5cb9dea18ce965c87274f043 Mon Sep 17 00:00:00 2001 From: kopi-solarpunk Date: Mon, 13 May 2024 12:24:46 +0200 Subject: [PATCH 3/7] ci: update linter version --- .github/workflows/go.yml | 9 +++++---- .golangci-strict.yml | 2 +- Makefile | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 891b0ab317a..23617ab2ed1 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -55,11 +55,11 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 with: fetch-depth: 0 - name: Setup Go - uses: actions/setup-go@v3 + uses: actions/setup-go@v5 with: cache: false go-version-file: go.mod @@ -67,10 +67,11 @@ jobs: if: github.ref != 'refs/heads/master' uses: wagoid/commitlint-github-action@v5 - name: GolangCI-Lint - uses: golangci/golangci-lint-action@v3 + uses: golangci/golangci-lint-action@v6 with: skip-cache: false - version: v1.54.1 + version: v1.58.1 + only-new-issues: true - name: Whitespace check run: make check-whitespace - name: go mod tidy check diff --git a/.golangci-strict.yml b/.golangci-strict.yml index 83671dafe37..1c00a442e23 100644 --- a/.golangci-strict.yml +++ b/.golangci-strict.yml @@ -93,7 +93,7 @@ linters: - tagliatelle - tenv - testableexamples - - testifylint + - testifylint - testpackage - thelper - tparallel diff --git a/Makefile b/Makefile index 10a94baa46d..e0511d4d0dc 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ GO ?= go GOBIN ?= $$($(GO) env GOPATH)/bin GOLANGCI_LINT ?= $(GOBIN)/golangci-lint -GOLANGCI_LINT_VERSION ?= v1.57.2 +GOLANGCI_LINT_VERSION ?= v1.58.1 GOGOPROTOBUF ?= protoc-gen-gogofaster GOGOPROTOBUF_VERSION ?= v1.3.1 BEEKEEPER_INSTALL_DIR ?= $(GOBIN) From 1e3cad68a7eca28d6a10f429301dde4054ba3728 Mon Sep 17 00:00:00 2001 From: kopi-solarpunk Date: Mon, 13 May 2024 14:29:37 +0200 Subject: [PATCH 4/7] ci: update linters to v1.58.1 --- .golangci-strict.yml | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/.golangci-strict.yml b/.golangci-strict.yml index 1c00a442e23..13e588d4b11 100644 --- a/.golangci-strict.yml +++ b/.golangci-strict.yml @@ -20,7 +20,7 @@ linters: - errchkjson - errname - errorlint - - execinquery + # - execinquery # deprecated since v1.58.0 - exhaustive # - exhaustruct # anoying - exportloopref @@ -39,12 +39,12 @@ linters: - gocyclo - godot # - godox # "TODO" is allowed - - goerr113 + - err113 - gofmt - gofumpt - goheader - goimports - - gomnd + # - gomnd # deprecated since v1.58.0 - gomoddirectives - gomodguard - goprintffuncname @@ -65,6 +65,7 @@ linters: - makezero - mirror - misspell + - mnd - musttag - nakedret - nestif @@ -132,6 +133,9 @@ issues: linters: - funlen - dupl + - path: version\.go + linters: + - gochecknoglobals - linters: - goheader text: "go-ethereum Authors" ## disable check for other authors @@ -171,6 +175,9 @@ issues: exclude-use-default: false max-issues-per-linter: 0 output: + formats: + - format: colored-line-number + path: stdout print-issued-lines: false show-stats: true uniq-by-line: false @@ -178,4 +185,13 @@ output: sort-order: - file # filepath, line, and column. - linter - - severity \ No newline at end of file + - severity +severity: + default-severity: error + rules: + - linters: + - revive + severity: warning + - linters: + - lll + severity: info From ce60739522f20f4cd85cfda76710073507125e41 Mon Sep 17 00:00:00 2001 From: kopi-solarpunk Date: Mon, 13 May 2024 14:56:38 +0200 Subject: [PATCH 5/7] ci: add linter ruler --- .golangci-strict.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.golangci-strict.yml b/.golangci-strict.yml index 13e588d4b11..ac49f5ac150 100644 --- a/.golangci-strict.yml +++ b/.golangci-strict.yml @@ -133,6 +133,10 @@ issues: linters: - funlen - dupl + - errcheck + - path: mock/* + linters: + - errcheck - path: version\.go linters: - gochecknoglobals From e7e190c313ccdb10e1a3c556175c9c856babe9fd Mon Sep 17 00:00:00 2001 From: kopi-solarpunk Date: Fri, 17 May 2024 12:12:35 +0200 Subject: [PATCH 6/7] ci: add strict linter step to Github Actions --- .github/workflows/go.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 23617ab2ed1..ffa5fa6a5d2 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -53,6 +53,7 @@ jobs: lint: name: Lint runs-on: ubuntu-latest + continue-on-error: true steps: - name: Checkout uses: actions/checkout@v4 @@ -67,11 +68,18 @@ jobs: if: github.ref != 'refs/heads/master' uses: wagoid/commitlint-github-action@v5 - name: GolangCI-Lint + uses: golangci/golangci-lint-action@v6 + with: + skip-cache: false + version: v1.58.1 + only-new-issues: false + - name: GolangCI-Lint strict uses: golangci/golangci-lint-action@v6 with: skip-cache: false version: v1.58.1 only-new-issues: true + args: --config=.golangci-strict.yml - name: Whitespace check run: make check-whitespace - name: go mod tidy check From 5177858609463dc0db350d459bdae88f025d411a Mon Sep 17 00:00:00 2001 From: kopi-solarpunk Date: Thu, 6 Jun 2024 11:58:49 +0200 Subject: [PATCH 7/7] fix: make linter installs golangci-lint if the version is not the same as specified in Makefile --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index e0511d4d0dc..9a7924d7ef5 100644 --- a/Makefile +++ b/Makefile @@ -98,7 +98,7 @@ lint-new: linter .PHONY: linter linter: - test -f $(GOLANGCI_LINT) || curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $$($(GO) env GOPATH)/bin $(GOLANGCI_LINT_VERSION) + test v"`$(GOLANGCI_LINT) version --format short 2> /dev/null`" = $(GOLANGCI_LINT_VERSION) || curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $$($(GO) env GOPATH)/bin $(GOLANGCI_LINT_VERSION) .PHONY: check-whitespace check-whitespace: