Skip to content

Commit 131cf87

Browse files
committed
Cosmetic changes to linters
1 parent 61dadaa commit 131cf87

7 files changed

Lines changed: 46 additions & 32 deletions

File tree

.golangci.yml

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ linters:
2626
- gosec
2727
- govet
2828
- ineffassign
29+
- iotamixing
2930
- loggercheck
3031
# Check for simple misspellings of words.
3132
- mnd
@@ -211,9 +212,14 @@ linters:
211212
channelcheck:
212213
type: "module"
213214
description: Static analysis for go channel issues
214-
settings:
215+
settings:
215216
CheckBlockingSends: true
216217
CheckUnbufferedChannels: false
218+
CheckEmptyDefault: true
219+
# msgC sends are intentionally blocking (watcher backpressure, not dropping).
220+
IgnoreChannelsByName:
221+
- msgC
222+
- msgChan
217223
exclusions:
218224
generated: lax
219225
presets:

linters/Makefile

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ install-golangci-lint:
1616
github.com/golangci/golangci-lint/v2/cmd/golangci-lint@$(GOLANGCI_LINT_VERSION)
1717

1818
# Custom golangci-lint with our module plugins baked in. Driven by
19-
# .custom-gcl.yml at the repo root; output goes to ./bin per `destination:`.
19+
# .custom-gcl.yml in this directory (golangci-lint custom reads it from the
20+
# cwd); output goes to ./bin per `destination:`.
2021
build-golangci-lint: install-golangci-lint
2122
$(BIN_DIR)/golangci-lint custom
2223

linters/README.md

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ Currently supported linters:
1010

1111
| Name | Purpose | Features |
1212
| -------------- | -------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------- |
13-
| `channelcheck` | Flag channel usage patterns that can deadlock or block. | • Blocking channel sends outside a `select`<br>• Unbuffered channel creation (`make(chan T)`)<br>• Ignore specific channels |
13+
| `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 |
1414

1515

1616
## Build & test
@@ -58,9 +58,20 @@ linters:
5858
blocking: true
5959
unbuffered: false
6060
bufferMax: 0
61+
emptyDefault: false
6162
ignoreChannelsByName: [errC]
6263
```
6364
65+
`channelcheck` settings:
66+
67+
| Setting | Type | Default | Description |
68+
| ---------------------- | ---------- | ------- | ---------------------------------------------------------------------------------------------------------------------------------------- |
69+
| `blocking` | `bool` | `true` | Flag blocking sends that have no escape (timer, `default:`, or `ctx.Done()`), including sends outside any `select`. |
70+
| `unbuffered` | `bool` | `false` | Flag unbuffered channel creation (`make(chan T)` / `make(chan T, 0)`). |
71+
| `bufferMax` | `uint` | `0` | Flag channel buffers larger than this. `0` disables the check. |
72+
| `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. |
73+
| `ignoreChannelsByName` | `[]string` | `[]` | Channel/field names whose direct sends are exempt from the blocking-send, empty-default, and `ctx.Done()` checks. |
74+
6475
## Development
6576

6677
### Adding a new linter
@@ -94,6 +105,8 @@ linters:
94105
path: ./rules/<linter>
95106
```
96107
6. `make test && make build && make build-golangci-lint` to verify.
108+
7. Add linter to `.golangci.yml`.
109+
8. Fix linter errors in the monorepo with legitimate changes or a `nolint` comment.
97110

98111
### Layout
99112

linters/rules/channelcheck/channelcheck.go

Lines changed: 16 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,6 @@ package channelcheck
44
Strategy: Identify proper SendStmt's early. If we don't see it within a Select, then it's not being used correctly.
55
Or, we simply MISS it, making it a false positive which is fine. We'd rather fail open and get the user to
66
use nolints than miss a potential bug altogether.
7-
8-
Steps for blocking channel send:
9-
- Find a SelectStmt
10-
- Check if Comm Clause:
11-
- See if it has a 'ast.SendStmt'.
12-
- If it does, then check 'default' or 'ticker' types there too.
13-
- If we find SelectStmt otherwise, it must be non-blocking and we want to flag it.
147
*/
158
import (
169
"bytes"
@@ -26,15 +19,18 @@ import (
2619
"golang.org/x/tools/go/analysis"
2720
)
2821

29-
type ChannelCheckPlugin struct {
30-
settings Settings
31-
}
22+
// ChannelCheckPlugin is the golangci-lint module-plugin entry point. Its
23+
// configuration lives in the package-level settings var (populated by New),
24+
// which is the single source of truth — shared with the standalone
25+
// cmd/wormhole-lint multichecker, which populates the same var via flags.
26+
type ChannelCheckPlugin struct{}
3227

3328
// Settings holds the configuration for the channelcheck linter.
3429
type Settings struct {
3530
CheckUnbufferedChannels bool // Enable/disable checking for unbuffered channel creation.
3631
CheckBufferAmount uint64 // The amount that can be in a buffer. 0 means don't do this check.
3732
CheckBlockingSends bool // Enable/disable checking for blocking sends without default/timeout.
33+
CheckEmptyDefault bool // Enable/disable the (advisory) empty-default-drops-the-send check.
3834
IgnoreChannelsByName []string // Channel/field names whose direct sends are exempt from the blocking-send check.
3935

4036
// ignoreChannelNames is the lookup form of IgnoreChannelsByName, built
@@ -81,13 +77,12 @@ func New(settingsNew any) (register.LinterPlugin, error) {
8177
if err != nil {
8278
return nil, err
8379
}
84-
settings.CheckBlockingSends = s.CheckBlockingSends
85-
settings.CheckBufferAmount = s.CheckBufferAmount
86-
settings.CheckUnbufferedChannels = s.CheckUnbufferedChannels
87-
settings.IgnoreChannelsByName = s.IgnoreChannelsByName
80+
// Assign the whole struct so a future Settings field can't be silently
81+
// dropped, then derive the lookup map from the decoded slice.
82+
settings = s
8883
settings.ignoreChannelNames = buildIgnoreSet(s.IgnoreChannelsByName)
8984

90-
return &ChannelCheckPlugin{settings: s}, nil
85+
return &ChannelCheckPlugin{}, nil
9186
}
9287

9388
func buildIgnoreSet(names []string) map[string]bool {
@@ -102,21 +97,17 @@ func buildIgnoreSet(names []string) map[string]bool {
10297
}
10398

10499
func (f *ChannelCheckPlugin) BuildAnalyzers() ([]*analysis.Analyzer, error) {
105-
return []*analysis.Analyzer{
106-
{
107-
Name: "channelcheck",
108-
Doc: "reports channel blocking issues",
109-
Run: run,
110-
Flags: flagSet,
111-
},
112-
}, nil
100+
// Reuse the package-level Analyzer (the same one the standalone
101+
// cmd/wormhole-lint multichecker runs) instead of defining a second one.
102+
return []*analysis.Analyzer{Analyzer}, nil
113103
}
114104

115105
// Initialize the flags from the golangci-lint
116106
func init() {
117107

118108
flagSet.BoolVar(&settings.CheckUnbufferedChannels, "unbuffered", false, "Check for unbuffered channel creation")
119109
flagSet.BoolVar(&settings.CheckBlockingSends, "blocking", true, "Check for blocking sends without default/timeout")
110+
flagSet.BoolVar(&settings.CheckEmptyDefault, "emptyDefault", false, "Advise when an empty default case silently drops a send")
120111
flagSet.Uint64Var(&settings.CheckBufferAmount, "bufferMax", 0, "Check for maximum length of channel buffer being exceeded")
121112
Analyzer.Flags = flagSet
122113
register.Plugin("channelcheck", New)
@@ -189,10 +180,10 @@ func run(pass *analysis.Pass) (interface{}, error) {
189180
// in the select (a receive-only select with an empty default has no
190181
// backpressure concern). Anchor at each tracked send so users can
191182
// suppress with a //nolint next to the blocking send.
192-
if selectAnalysis.EmptyDefaultPos != token.NoPos {
183+
if settings.CheckEmptyDefault && selectAnalysis.EmptyDefaultPos != token.NoPos {
193184
for _, send := range trackedSends {
194185
pass.Reportf(send.Pos(),
195-
"empty default case in channel select. Please add logging to it on failure")
186+
"empty default in channel select silently drops the send; log it or document why dropping is intended")
196187
}
197188
}
198189

linters/rules/channelcheck/channelcheck_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ func runFixture(t *testing.T, pkg string, override Settings) {
2020

2121
settings.CheckUnbufferedChannels = override.CheckUnbufferedChannels
2222
settings.CheckBlockingSends = override.CheckBlockingSends
23+
settings.CheckEmptyDefault = override.CheckEmptyDefault
2324
settings.CheckBufferAmount = override.CheckBufferAmount
2425
settings.IgnoreChannelsByName = override.IgnoreChannelsByName
2526
settings.ignoreChannelNames = buildIgnoreSet(override.IgnoreChannelsByName)
@@ -36,7 +37,7 @@ func TestBlockingSend(t *testing.T) {
3637
}
3738

3839
func TestEmptyDefault(t *testing.T) {
39-
runFixture(t, "empty_default", defaultSettings())
40+
runFixture(t, "empty_default", Settings{CheckBlockingSends: true, CheckEmptyDefault: true})
4041
}
4142

4243
func TestCtxDoneOnly(t *testing.T) {

linters/rules/channelcheck/testdata/src/empty_default/empty_default.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ package fixture
44
func emptyDefault() {
55
c := make(chan int, 1)
66
select {
7-
case c <- 1: // want `empty default case`
7+
case c <- 1: // want `empty default in channel select silently drops the send`
88
default:
99
}
1010
}

scripts/lint.sh

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,12 @@ format(){
6868
# Content hash of the linters/ source, used to decide whether a rebuild is needed.
6969
# Hashes every file under linters/ (regardless of git tracking) except the
7070
# bin/ and build/ build outputs, so any source edit triggers exactly one rebuild.
71+
# Runs find from inside LINTERS_DIR so paths are relative: sha256sum embeds the
72+
# path, so an absolute one would make the hash checkout-path dependent.
7173
linters_source_hash() {
72-
find "$LINTERS_DIR" -type f \
74+
( cd "$LINTERS_DIR" && find . -type f \
7375
-not -path '*/bin/*' -not -path '*/build/*' \
74-
-print0 | sort -z | xargs -0 sha256sum | sha256sum | cut -d' ' -f1
76+
-print0 | sort -z | xargs -0 sha256sum | sha256sum | cut -d' ' -f1 )
7577
}
7678

7779
ensure_wormhole_golangci_lint() {

0 commit comments

Comments
 (0)