Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add strict linter rules #38

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions .github/workflows/go.yml
Original file line number Diff line number Diff line change
Expand Up @@ -53,24 +53,33 @@ jobs:
lint:
name: Lint
runs-on: ubuntu-latest
continue-on-error: true
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
- name: Commit linting
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.58.1
only-new-issues: false
- name: GolangCI-Lint strict
uses: golangci/golangci-lint-action@v6
with:
skip-cache: false
version: v1.54.1
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
Expand Down
201 changes: 201 additions & 0 deletions .golangci-strict.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,201 @@
run:
timeout: 10m
linters:
enable:
- asasalint
- asciicheck
- bidichk
- bodyclose
- containedctx
- contextcheck
# - copyloopvar # requires Go 1.22
- cyclop
- decorder
# - depguard # a list of acceptable packages is necessary to use this
- dogsled
- dupl
- dupword
- durationcheck
- errcheck
- errchkjson
- errname
- errorlint
# - execinquery # deprecated since v1.58.0
- exhaustive
# - exhaustruct # anoying
- exportloopref
- forbidigo
- forcetypeassert
- funlen
- gci
- ginkgolinter
- gocheckcompilerdirectives
- gochecknoglobals
- gochecknoinits
- gochecksumtype
- gocognit
- goconst
- gocritic
- gocyclo
- godot
# - godox # "TODO" is allowed
- err113
- gofmt
- gofumpt
- goheader
- goimports
# - gomnd # deprecated since v1.58.0
- gomoddirectives
- gomodguard
- goprintffuncname
- gosec
- gosimple
- gosmopolitan
- govet
- grouper
- importas
- inamedparam
- ineffassign
- interfacebloat
# - intrange # requires Go 1.22
- ireturn
- lll
- loggercheck
- maintidx
- makezero
- mirror
- misspell
- mnd
- 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:
- path: _test\.go
linters:
- funlen
- dupl
- errcheck
- path: mock/*
linters:
- errcheck
- path: version\.go
linters:
- gochecknoglobals
- 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
max-issues-per-linter: 0
output:
formats:
- format: colored-line-number
path: stdout
print-issued-lines: false
show-stats: true
uniq-by-line: false
sort-results: true
sort-order:
- file # filepath, line, and column.
- linter
- severity
severity:
default-severity: error
rules:
- linters:
- revive
severity: warning
- linters:
- lll
severity: info
6 changes: 5 additions & 1 deletion CODING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
aranyia marked this conversation as resolved.
Show resolved Hide resolved
Linting is done by using <https://github.com/golangci/golangci-lint> 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

Expand Down
13 changes: 8 additions & 5 deletions CODINGSTYLE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -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.
aranyia marked this conversation as resolved.
Show resolved Hide resolved
- 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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -206,6 +207,7 @@ func TestSomething(t *testing.T) {
</tbody></table>

If needed, use an underscore to disambiguate tests that are hard to name:

```go
func TestScenario_EdgeCase(t *testing.T) {
...
Expand Down Expand Up @@ -398,7 +400,8 @@ var ErrLimitExceeded = errors.New("limit exeeded)

</td></tr>
</tbody></table>
*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`.

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1429,6 +1430,7 @@ func (s *signer) Sign(msg string) string {
return signWithTime(msg, now)
}
```

</td></tr>
<tr><td>

Expand Down Expand Up @@ -1680,6 +1682,7 @@ func (f Foo) String() string {


```

</td></tr>
</tbody></table>

Expand Down
12 changes: 10 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
@@ -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.58.1
GOGOPROTOBUF ?= protoc-gen-gogofaster
GOGOPROTOBUF_VERSION ?= v1.3.1
BEEKEEPER_INSTALL_DIR ?= $(GOBIN)
Expand Down Expand Up @@ -88,9 +88,17 @@ 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)
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:
Expand Down