Custom ci lints #4841
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,4 +33,4 @@ sui/examples/wrapped_coin | |
| /coverage-check | ||
| # Only used for internal testing | ||
| *.testnet.yaml | ||
| coverage.txt | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| version: v2.12.2 | ||
| name: wormhole-golangci-lint | ||
| destination: ./bin | ||
| plugins: | ||
| - module: github.com/wormhole-foundation/wormhole/linters/rules/channelcheck | ||
| path: ./rules/channelcheck |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| # Built binaries. | ||
| /bin/ | ||
|
|
||
| # Stray root-level builds, e.g. from a bare `go build ./...` (the Makefile | ||
| # targets put these under bin/). | ||
| /wormhole-lint | ||
| /wormhole-golangci-lint |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| # Pin by commit hash rather than tag, so a re-pointed/malleable tag can't change | ||
| # what we build. Keep the trailing comment's tag in sync with the hash and with | ||
| # the `version:` field in .custom-gcl.yml. | ||
| GOLANGCI_LINT_HASH ?= c0d3ddc9cf3faa61a4e378e879ece580256d76e5 # v2.12.2 | ||
|
|
||
| BIN_DIR := bin | ||
|
|
||
| .PHONY: build build-golangci-lint install-golangci-lint test clean | ||
|
|
||
| # Standalone multichecker over every rules/<linter>.Analyzer. | ||
| build: | ||
| go build -o $(BIN_DIR)/wormhole-lint ./cmd/wormhole-lint | ||
|
|
||
| # Pinned upstream golangci-lint, dropped into $(BIN_DIR). Used as the driver | ||
| # for `golangci-lint custom`; keep its version aligned with the `version:` | ||
| # field in .custom-gcl.yml. | ||
| install-golangci-lint: | ||
| GOOS= GOARCH= GOBIN=$(abspath $(BIN_DIR)) go install \ | ||
| github.com/golangci/golangci-lint/v2/cmd/golangci-lint@$(GOLANGCI_LINT_HASH) | ||
|
|
||
| # Custom golangci-lint with our module plugins baked in. Driven by | ||
| # .custom-gcl.yml in this directory (golangci-lint custom reads it from the | ||
| # cwd); output goes to ./bin per `destination:`. | ||
| build-golangci-lint: install-golangci-lint | ||
| $(BIN_DIR)/golangci-lint custom | ||
|
|
||
| test: | ||
| go test -v ./... | ||
| @for mod in $$(find rules -name go.mod); do \ | ||
| dir=$$(dirname $$mod); \ | ||
| echo "==> testing $$dir"; \ | ||
| (cd $$dir && go test -v ./...) || exit $$?; \ | ||
| done | ||
|
|
||
| clean: | ||
| rm -rf $(BIN_DIR) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,125 @@ | ||
| # Golang CI Lints | ||
|
|
||
| Custom Go linters used on Wormhole CI. Each linter is a [golangci-lint | ||
| module plugin](https://golangci-lint.run/plugins/module-plugins/) and lives | ||
| as its own Go module under `rules/<linter>/`. | ||
|
|
||
| Prefer to use the `release` builds. | ||
|
|
||
| Currently supported linters: | ||
|
|
||
| | Name | Purpose | Features | | ||
| | -------------- | -------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------- | | ||
| | `channelcheck` | Flag channel usage patterns that can deadlock or block. | • Blocking channel sends outside a `select`<br>• Unbuffered channel creation (`make(chan T)`)<br>• Empty `default:` that silently drops a send (advisory, off by default)<br>• Ignore specific channels | | ||
|
|
||
|
|
||
| ## Build & test | ||
| ``` | ||
| make build # bin/wormhole-lint | ||
| make build-golangci-lint # bin/wormhole-golangci-lint | ||
| make test # root + each rules/<linter> module | ||
| ``` | ||
|
|
||
| `make build-golangci-lint` first installs the pinned upstream | ||
| `golangci-lint` into `bin/` (per `GOLANGCI_LINT_HASH`), then runs | ||
| `golangci-lint custom`. | ||
|
|
||
| ## Use | ||
|
|
||
| Standalone: | ||
|
|
||
| ``` | ||
| bin/wormhole-lint ./... | ||
| ``` | ||
|
|
||
| Via the custom golangci-lint: | ||
|
|
||
| ``` | ||
| bin/wormhole-golangci-lint run --timeout=10m ./... | ||
| ``` | ||
|
|
||
| ## Enable a plugin in `.golangci.yml` | ||
|
|
||
| Module plugins are addressed under `linters.settings.custom.<name>` and | ||
| enabled in `linters.enable` by the plugin's registered name. Example for | ||
| `channelcheck`: | ||
|
|
||
| ```yaml | ||
| version: "2" | ||
| linters: | ||
| enable: | ||
| - channelcheck | ||
| settings: | ||
| custom: | ||
| channelcheck: | ||
| type: module | ||
| description: reports channel blocking issues | ||
| settings: | ||
| blocking: true | ||
| unbuffered: false | ||
| bufferMax: 0 | ||
| emptyDefault: false | ||
| ignoreChannelsByName: [errC] | ||
| ``` | ||
|
|
||
| `channelcheck` settings: | ||
|
|
||
| | Setting | Type | Default | Description | | ||
| | ---------------------- | ---------- | ------- | ---------------------------------------------------------------------------------------------------------------------------------------- | | ||
| | `blocking` | `bool` | `true` | Flag blocking sends that have no escape (timer, `default:`, or `ctx.Done()`), including sends outside any `select`. | | ||
| | `unbuffered` | `bool` | `false` | Flag unbuffered channel creation (`make(chan T)` / `make(chan T, 0)`). | | ||
| | `bufferMax` | `uint` | `0` | Flag channel buffers larger than this. `0` disables the check. | | ||
| | `emptyDefault` | `bool` | `false` | Advisory: flag an empty `default:` in a `select` with a send, since it silently drops the send. Off by default — it fires on the idiomatic non-blocking/coalescing-send pattern, so enable it only where dropping should always be logged or documented. | | ||
| | `ignoreChannelsByName` | `[]string` | `[]` | Channel/field names whose direct sends are exempt from the blocking-send, empty-default, and `ctx.Done()` checks. | | ||
|
|
||
| ## Development | ||
|
|
||
| ### Adding a new linter | ||
|
|
||
| 1. Scaffold the module: | ||
| ``` | ||
| mkdir -p rules/<linter> && cd rules/<linter> | ||
| go mod init github.com/wormhole-foundation/wormhole/linters/rules/<linter> | ||
| ``` | ||
| 2. Implement `<linter>.go` following the channelcheck reference | ||
| (`rules/channelcheck/channelcheck.go`): | ||
| - Export an `Analyzer` of type `*analysis.Analyzer`. | ||
| - Define a `Settings` struct and a `New(any) (register.LinterPlugin, error)` | ||
| constructor that decodes settings via | ||
| `register.DecodeSettings[Settings]`. | ||
| - Implement `BuildAnalyzers()` and `GetLoadMode()` on your plugin type. | ||
| - In `init()`, call `register.Plugin("<name>", New)` so | ||
| `golangci-lint custom` picks it up. | ||
| 3. Add tests + fixtures under `rules/<linter>/testdata/` following | ||
| `rules/channelcheck/channelcheck_test.go`. | ||
| 4. Wire it into the root module so the aggregator can import it: | ||
| - In root `go.mod`, add | ||
| `require github.com/wormhole-foundation/wormhole/linters/rules/<linter> v0.0.0` and | ||
| `replace github.com/wormhole-foundation/wormhole/linters/rules/<linter> => ./rules/<linter>`. | ||
| - In `cmd/wormhole-lint/main.go`, add the import and append | ||
| `<linter>.Analyzer` to the `multichecker.Main` call. | ||
| 5. Wire it into `.custom-gcl.yml` so the custom golangci-lint picks it up: | ||
| ```yaml | ||
| plugins: | ||
| - module: github.com/wormhole-foundation/wormhole/linters/rules/<linter> | ||
| path: ./rules/<linter> | ||
| ``` | ||
| 6. `make test && make build && make build-golangci-lint` to verify. | ||
| 7. Add linter to `.golangci.yml`. | ||
| 8. Fix linter errors in the monorepo with legitimate changes or a `nolint` comment. | ||
|
|
||
| ### Layout | ||
|
|
||
| ``` | ||
| .custom-gcl.yml # plugin manifest for `golangci-lint custom` | ||
| Makefile | ||
| go.mod # root module: cmd/* + replaces for each rules/<linter> | ||
| cmd/ | ||
| wormhole-lint/ # multichecker aggregator binary | ||
| rules/ | ||
| channelcheck/ # standalone Go module per linter | ||
| channelcheck.go | ||
| channelcheck_test.go | ||
| go.mod | ||
| testdata/ | ||
| ``` | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| // Command wormhole-lint runs every custom linter under rules/ as a single | ||
| // standalone binary. Each rules/<linter>/ exports an Analyzer; add new | ||
| // linters by appending one import + one Analyzer below. | ||
| package main | ||
|
|
||
| import ( | ||
| "golang.org/x/tools/go/analysis/multichecker" | ||
|
|
||
| "github.com/wormhole-foundation/wormhole/linters/rules/channelcheck" | ||
| ) | ||
|
|
||
| func main() { | ||
| multichecker.Main( | ||
| channelcheck.Analyzer, | ||
| ) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| module github.com/wormhole-foundation/wormhole/linters | ||
|
|
||
| go 1.25.10 | ||
|
|
||
| require ( | ||
| github.com/wormhole-foundation/wormhole/linters/rules/channelcheck v0.0.0 | ||
| golang.org/x/tools v0.30.0 | ||
| ) | ||
|
|
||
| require ( | ||
| github.com/golangci/plugin-module-register v0.1.1 // indirect | ||
| golang.org/x/mod v0.23.0 // indirect | ||
| golang.org/x/sync v0.11.0 // indirect | ||
| ) | ||
|
|
||
| replace github.com/wormhole-foundation/wormhole/linters/rules/channelcheck => ./rules/channelcheck |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| github.com/golangci/plugin-module-register v0.1.1 h1:TCmesur25LnyJkpsVrupv1Cdzo+2f7zX0H6Jkw1Ol6c= | ||
| github.com/golangci/plugin-module-register v0.1.1/go.mod h1:TTpqoB6KkwOJMV8u7+NyXMrkwwESJLOkfl9TxR1DGFc= | ||
| github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= | ||
| github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= | ||
| golang.org/x/mod v0.23.0 h1:Zb7khfcRGKk+kqfxFaP5tZqCnDZMjC5VtUBs87Hr6QM= | ||
| golang.org/x/mod v0.23.0/go.mod h1:6SkKJ3Xj0I0BrPOZoBy3bdMptDDU9oJrpohJ3eWZ1fY= | ||
| golang.org/x/sync v0.11.0 h1:GGz8+XQP4FvTTrjZPzNKTMFtSXH80RAzG+5ghFPgK9w= | ||
| golang.org/x/sync v0.11.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= | ||
| golang.org/x/tools v0.30.0 h1:BgcpHewrV5AUp2G9MebG4XPFI1E2W41zU1SaqVA9vJY= | ||
| golang.org/x/tools v0.30.0/go.mod h1:c347cR/OJfw5TI+GfX7RUPNMdDRRbjvYTS0jPyvsVtY= |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| ## Channel Check | ||
|
|
||
| Channels are a great feature of Golang but have several footguns that can lead to deadlocks. In particular, if the receiving channel stops processing the messages, a *non-blocking* channel send would fail to continue. In certain mission-critical sections of code, this could lead to a complete deadlock. | ||
|
|
||
| This linter currently has three features: | ||
| - Identify blocking sends | ||
| - Identify non-buffered channel creation | ||
| - Identify buffered channel size exceeds maximum size checks | ||
|
|
||
| Many of these will lead to false positives or situations where we *want* a blocking channel send. In these cases, `nolint:channelcheck` is easy to add (assuming this is integrated directly with golangci-lint). Regardless, having this issue pointed out automatically is a good way to fix bugs. | ||
|
|
||
| ## Configuration | ||
|
|
||
| Each option can be set two ways: as a golangci-lint module setting (under | ||
| `settings.custom.channelcheck.settings:`, using the **Setting** name) or as a | ||
| standalone analyzer flag (when running the `wormhole-lint` binary, using the | ||
| **Flag** name). | ||
|
|
||
| | Setting | Flag | Type | Default | Description | | ||
|
Contributor
There was a problem hiding this comment. Should we include
Contributor
There was a problem hiding this comment. @mdulin2 feel free to mark my 2 comments as resolved and ignore, they are non blockers. |
||
| | ------------------------- | ------------ | ---------- | ------- | ----------------------------------------------------------------------------------------------- | | ||
| | `CheckBlockingSends` | `blocking` | bool | `true` | Flag blocking sends that lack a `default`/timeout/ticker escape in their enclosing `select`. | | ||
| | `CheckUnbufferedChannels` | `unbuffered` | bool | `false` | Flag creation of unbuffered channels (`make(chan T)`). | | ||
| | `CheckBufferAmount` | `bufferMax` | uint64 | `0` | Flag buffered channels whose size exceeds this max. `0` disables the check. | | ||
| | `IgnoreChannelsByName` | *(none)* | []string | `[]` | Channel/field names whose direct sends are exempt from the blocking-send check (e.g. `errC`). Settings-only; no standalone flag. | | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be better to use actual setting names (
CheckBlockingSends) instead of flags (blocking)?