From f9bd564ecc74b19ef7ca77484548e52efd058f3e Mon Sep 17 00:00:00 2001 From: Benjamin Borbe Date: Wed, 3 Jun 2026 00:00:12 +0200 Subject: [PATCH] feat(rules): ast-grep YAML for go-concurrency/channel-closed-by-sender-only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mechanical YAML count: 27 -> 28. Hybrid pattern: flag every close($X) call. Agent decides per-finding whether enclosing function is producer (sends into channel — close correct, typically 'defer close(ch)') or consumer (only receives — close is the bug). Multi-producer routed through dedicated closer goroutine + WaitGroup is also producer-side once the agent reads the surrounding context. Pattern uses bare 'close($X)' — same simple-pattern approach as PR #33's no-custom-ptr-helpers. The producer-vs-consumer judgment needs reading function body's send/receive direction, exactly the cross-context reasoning ast-grep can't reliably do. Verified via fixture: producer's 'defer close(out)' (correct) AND consumer's 'close(in)' (wrong) both fire; non-close call clean. Agent dismisses the producer side and surfaces the consumer side as the real violation. Enforcement field in go-concurrency-patterns.md updated from 'judgment (ast-grep follow-up: ...)' to YAML path + agent note. --- docs/go-concurrency-patterns.md | 2 +- rules/go/channel-closed-by-sender-only.yml | 27 ++++++++++++++++++++++ rules/index.json | 2 +- 3 files changed, 29 insertions(+), 2 deletions(-) create mode 100644 rules/go/channel-closed-by-sender-only.yml diff --git a/docs/go-concurrency-patterns.md b/docs/go-concurrency-patterns.md index 85490d8..1863ab8 100644 --- a/docs/go-concurrency-patterns.md +++ b/docs/go-concurrency-patterns.md @@ -110,7 +110,7 @@ func process(ctx context.Context) error { **Owner**: go-architecture-assistant **Applies when**: a Go file calls `close(ch)` on a channel that was passed in as a function parameter from elsewhere — i.e. closed by a consumer/receiver rather than by the goroutine that produces values into it. -**Enforcement**: judgment (ast-grep follow-up: `close(X)` where `X` is a parameter type `chan T` or `chan<- T`; the agent rules in whether the function is the producer or consumer based on whether it sends into `X`) +**Enforcement**: `rules/go/channel-closed-by-sender-only.yml` flags every `close($X)` call. The agent decides per-finding whether the enclosing function is the producer (sends into the channel — close correct, often `defer close(ch)`) or a consumer (only receives — `close(ch)` is the bug), and clears multi-producer cases routed through a dedicated closer goroutine + sync.WaitGroup. The producer-vs-consumer judgment needs reading the function body's send/receive direction — exactly the cross-context reasoning ast-grep can't do reliably. **Why**: Closing a channel from the receiver side is a textbook race — the sender may still be writing when the close happens, producing `send on closed channel` panic. The Go convention is: **the producer owns the channel and is the only one allowed to close it.** Receivers learn of "no more values" via `for v := range ch` or the `comma-ok` idiom (`v, ok := <-ch`), never by closing themselves. Multi-producer cases use `sync.WaitGroup` + a single dedicated closer goroutine, not concurrent closes (which also panic). #### Bad diff --git a/rules/go/channel-closed-by-sender-only.yml b/rules/go/channel-closed-by-sender-only.yml new file mode 100644 index 0000000..45b0fcd --- /dev/null +++ b/rules/go/channel-closed-by-sender-only.yml @@ -0,0 +1,27 @@ +id: go-concurrency/channel-closed-by-sender-only +language: go +severity: error +message: | + `close(ch)` call needs a producer-only owner. Closing a channel + from the receiver side races against any still-pending send and + panics `send on closed channel`. The Go convention is: producer + owns the channel and is the only one allowed to close it; + receivers use `for v := range ch` or the comma-ok idiom. + The agent decides per-finding whether the enclosing function is + the producer (sends into the channel — close is correct, often + `defer close(ch)`) or a consumer (only receives — `close(ch)` is + the bug). The agent also clears multi-producer cases that route + through a single dedicated closer goroutine + sync.WaitGroup. + See docs/go-concurrency-patterns.md + (RULE go-concurrency/channel-closed-by-sender-only). +rule: + # Match every `close($X)` call. The producer-vs-consumer + # adjudication needs reading the enclosing function body's + # send/receive direction on $X — exactly the cross-context + # reasoning ast-grep can't do reliably. + pattern: 'close($X)' +ignores: + - "**/*_test.go" + - "vendor/**" + - "**/vendor/**" + - "**/mocks/**" diff --git a/rules/index.json b/rules/index.json index 2955cf6..bd6b8cd 100644 --- a/rules/index.json +++ b/rules/index.json @@ -228,7 +228,7 @@ "anchor": "go-concurrency/channel-closed-by-sender-only", "applies_when": "a Go file calls `close(ch)` on a channel that was passed in as a function parameter from elsewhere — i.e. closed by a consumer/receiver rather than by the goroutine that produces values into it.", "doc_path": "docs/go-concurrency-patterns.md", - "enforcement": "judgment (ast-grep follow-up: `close(X)` where `X` is a parameter type `chan T` or `chan<- T`; the agent rules in whether the function is the producer or consumer based on whether it sends into `X`)", + "enforcement": "`rules/go/channel-closed-by-sender-only.yml` flags every `close($X)` call. The agent decides per-finding whether the enclosing function is the producer (sends into the channel — close correct, often `defer close(ch)`) or a consumer (only receives — `close(ch)` is the bug), and clears multi-producer cases routed through a dedicated closer goroutine + sync.WaitGroup. The producer-vs-consumer judgment needs reading the function body's send/receive direction — exactly the cross-context reasoning ast-grep can't do reliably.", "id": "go-concurrency/channel-closed-by-sender-only", "level": "MUST", "owner": "go-architecture-assistant"