From dcc7a1ce2c2f1e7a04157c86a370a91e7118ccba Mon Sep 17 00:00:00 2001 From: can1357 Date: Tue, 30 Jun 2026 09:49:59 +0200 Subject: [PATCH] feat(coding-agent/discovery): added Go syntax and API discovery rules - Added eight new Go-specific rules to the discovery package. - Registered the new Go rules in the default rule source index. - Covered the new Go AST matching conditions with test cases in `builtin-defaults.test.ts`. --- packages/coding-agent/CHANGELOG.md | 4 + .../discovery/builtin-rules/go-add-cleanup.md | 32 ++++++++ .../discovery/builtin-rules/go-bench-loop.md | 36 +++++++++ .../builtin-rules/go-exp-promoted.md | 39 ++++++++++ .../src/discovery/builtin-rules/go-ioutil.md | 36 +++++++++ .../builtin-rules/go-join-hostport.md | 29 +++++++ .../discovery/builtin-rules/go-new-expr.md | 44 +++++++++++ .../src/discovery/builtin-rules/go-rand-v2.md | 40 ++++++++++ .../discovery/builtin-rules/go-range-int.md | 45 +++++++++++ .../src/discovery/builtin-rules/index.ts | 16 ++++ .../test/discovery/builtin-defaults.test.ts | 77 +++++++++++++++++++ .../test/tools/web-search-duckduckgo.test.ts | 2 +- 12 files changed, 399 insertions(+), 1 deletion(-) create mode 100644 packages/coding-agent/src/discovery/builtin-rules/go-add-cleanup.md create mode 100644 packages/coding-agent/src/discovery/builtin-rules/go-bench-loop.md create mode 100644 packages/coding-agent/src/discovery/builtin-rules/go-exp-promoted.md create mode 100644 packages/coding-agent/src/discovery/builtin-rules/go-ioutil.md create mode 100644 packages/coding-agent/src/discovery/builtin-rules/go-join-hostport.md create mode 100644 packages/coding-agent/src/discovery/builtin-rules/go-new-expr.md create mode 100644 packages/coding-agent/src/discovery/builtin-rules/go-rand-v2.md create mode 100644 packages/coding-agent/src/discovery/builtin-rules/go-range-int.md diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index c44e8b2da..02fd02778 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Added + +- Added built-in Go coding rules including `go-add-cleanup`, `go-bench-loop`, `go-exp-promoted`, `go-ioutil`, `go-join-hostport`, `go-new-expr`, `go-rand-v2`, and `go-range-int` + ## [16.2.7] - 2026-06-30 ### Breaking Changes diff --git a/packages/coding-agent/src/discovery/builtin-rules/go-add-cleanup.md b/packages/coding-agent/src/discovery/builtin-rules/go-add-cleanup.md new file mode 100644 index 000000000..dc0ccc3c6 --- /dev/null +++ b/packages/coding-agent/src/discovery/builtin-rules/go-add-cleanup.md @@ -0,0 +1,32 @@ +--- +description: "Prefer runtime.AddCleanup over runtime.SetFinalizer for new code (Go 1.24)" +condition: 'runtime\.SetFinalizer' +scope: "tool:edit(*.go), tool:write(*.go)" +--- + +Go 1.24 added `runtime.AddCleanup`, a finalization mechanism that is more flexible and less error-prone than `runtime.SetFinalizer`. The release notes state plainly: **new code should prefer `AddCleanup` over `SetFinalizer`.** + +## Why AddCleanup wins + +- Multiple cleanups may attach to one object; `SetFinalizer` allows only one. +- Cleanups may attach to interior pointers. +- Objects that form a reference cycle still get cleaned up — finalizers leak them. +- A cleanup does not resurrect its object or delay freeing it (and what it points to) by an extra GC cycle. + +## Migration + +```go +// Before +runtime.SetFinalizer(obj, func(o *T) { o.release() }) + +// After — the cleanup func receives a value you supply, NOT the object, +// so it cannot accidentally keep the object alive. +runtime.AddCleanup(obj, func(h handle) { h.release() }, obj.handle) +``` + +The cleanup argument must not reference `obj` itself (that would keep it reachable forever). Capture only the data the cleanup needs — a file descriptor, handle, or pointer that is independent of `obj`. + +## Keep SetFinalizer only when + +- The module targets a Go release older than 1.24. +- You depend on finalizer-specific behavior (e.g. object resurrection) that `AddCleanup` deliberately does not provide. diff --git a/packages/coding-agent/src/discovery/builtin-rules/go-bench-loop.md b/packages/coding-agent/src/discovery/builtin-rules/go-bench-loop.md new file mode 100644 index 000000000..67ae73855 --- /dev/null +++ b/packages/coding-agent/src/discovery/builtin-rules/go-bench-loop.md @@ -0,0 +1,36 @@ +--- +description: "Use for b.Loop() in benchmarks instead of the for i := 0; i < b.N; i++ loop (Go 1.24)" +interruptMode: never +scope: "tool:edit(*_test.go), tool:write(*_test.go)" +astCondition: + - "func $F($B *testing.B) { $$$PRE for $I := 0; $I < $B.N; $I++ { $$$BODY } $$$POST }" +--- + +Go 1.24 added `testing.B.Loop`. Write `for b.Loop() { ... }` instead of looping over `b.N`. + +## Why + +- Setup and teardown outside the loop run exactly once per `-count`, not once per `b.N` re-estimation, so expensive fixtures are no longer timed or repeated. +- The compiler keeps the loop's parameters and results alive, so it can't optimize away the body you are trying to measure — a classic `b.N` benchmarking footgun. + +## Avoid + +```go +func BenchmarkEncode(b *testing.B) { + for i := 0; i < b.N; i++ { + Encode(input) + } +} +``` + +## Use + +```go +func BenchmarkEncode(b *testing.B) { + for b.Loop() { + Encode(input) + } +} +``` + +Requires Go 1.24+. If the module targets an older Go, keep the `b.N` loop. diff --git a/packages/coding-agent/src/discovery/builtin-rules/go-exp-promoted.md b/packages/coding-agent/src/discovery/builtin-rules/go-exp-promoted.md new file mode 100644 index 000000000..76fde42c8 --- /dev/null +++ b/packages/coding-agent/src/discovery/builtin-rules/go-exp-promoted.md @@ -0,0 +1,39 @@ +--- +description: "Use the standard library slices and maps packages instead of golang.org/x/exp/{slices,maps}" +condition: + - '"golang.org/x/exp/slices"' + - '"golang.org/x/exp/maps"' +scope: "tool:edit(*.go), tool:write(*.go)" +--- + +`golang.org/x/exp/slices` and `golang.org/x/exp/maps` were promoted into the standard library as `slices` and `maps` in Go 1.21. Import the stdlib packages in new code instead of the experimental ones. + +## Migration + +```go +// Before +import ( + "golang.org/x/exp/slices" + "golang.org/x/exp/maps" +) + +// After +import ( + "slices" + "maps" +) +``` + +Most call sites are unchanged: `slices.Sort`, `slices.Contains`, `slices.Index`, `slices.Equal`, `maps.Clone`, etc. + +## Watch the signature differences + +The promoted APIs were tweaked, so a blind path swap can break the build: + +- `x/exp/maps.Keys(m)` / `Values(m)` returned a slice; the stdlib `maps.Keys(m)` / `maps.Values(m)` return an **iterator** (`iter.Seq`). Use `slices.Collect(maps.Keys(m))` to recover a slice, or range over the iterator. +- `slices.SortFunc` takes a comparison returning `int` (cmp-style), matching the stdlib signature. + +## Keep x/exp when + +- The module's `go` directive is below 1.21 (stdlib `slices`/`maps` don't exist yet). +- You need an `x/exp` helper that was not promoted (e.g. parts of `x/exp/constraints` still live outside the stdlib). diff --git a/packages/coding-agent/src/discovery/builtin-rules/go-ioutil.md b/packages/coding-agent/src/discovery/builtin-rules/go-ioutil.md new file mode 100644 index 000000000..394dd77e0 --- /dev/null +++ b/packages/coding-agent/src/discovery/builtin-rules/go-ioutil.md @@ -0,0 +1,36 @@ +--- +description: "Use io and os instead of the deprecated io/ioutil package" +condition: '"io/ioutil"' +scope: "tool:edit(*.go), tool:write(*.go)" +--- + +`io/ioutil` has been deprecated since Go 1.16. Every function moved to `io` or `os` with the same behavior. Do not import it in new code. + +## Mapping + +| io/ioutil | Replacement | +| --- | --- | +| `ioutil.ReadAll` | `io.ReadAll` | +| `ioutil.ReadFile` | `os.ReadFile` | +| `ioutil.WriteFile` | `os.WriteFile` | +| `ioutil.ReadDir` | `os.ReadDir` (returns `[]os.DirEntry`, not `[]os.FileInfo`) | +| `ioutil.TempFile` | `os.CreateTemp` | +| `ioutil.TempDir` | `os.MkdirTemp` | +| `ioutil.NopCloser` | `io.NopCloser` | +| `ioutil.Discard` | `io.Discard` | + +## Migration + +```go +// Before +import "io/ioutil" +data, err := ioutil.ReadFile(path) +_ = ioutil.WriteFile(out, data, 0o644) + +// After +import "os" +data, err := os.ReadFile(path) +_ = os.WriteFile(out, data, 0o644) +``` + +`os.ReadDir` returns `[]os.DirEntry` rather than `[]os.FileInfo` — call `entry.Info()` if you need the old `FileInfo`. Everything else is a drop-in rename. diff --git a/packages/coding-agent/src/discovery/builtin-rules/go-join-hostport.md b/packages/coding-agent/src/discovery/builtin-rules/go-join-hostport.md new file mode 100644 index 000000000..807b6bad2 --- /dev/null +++ b/packages/coding-agent/src/discovery/builtin-rules/go-join-hostport.md @@ -0,0 +1,29 @@ +--- +description: "Build network addresses with net.JoinHostPort, not fmt.Sprintf(\"%s:%d\", host, port) — the Sprintf form breaks on IPv6" +condition: 'fmt\.Sprintf\("%s:%d"' +scope: "tool:edit(*.go), tool:write(*.go)" +--- + +Use `net.JoinHostPort(host, port)` to assemble a `host:port` address. `fmt.Sprintf("%s:%d", host, port)` produces invalid addresses for IPv6 hosts, which must be bracketed (`[::1]:80`). Go 1.25's `go vet` `hostport` analyzer flags exactly this pattern. + +## Why + +- An IPv6 literal like `::1` has its own colons, so `fmt.Sprintf("%s:%d", "::1", 80)` yields `::1:80` — unparseable by `net.Dial`. +- `net.JoinHostPort` adds the brackets when the host contains a colon and leaves IPv4/hostnames untouched. + +## Avoid + +```go +addr := fmt.Sprintf("%s:%d", host, port) +conn, err := net.Dial("tcp", addr) +``` + +## Use + +```go +// port is a string here; convert an int with strconv.Itoa. +addr := net.JoinHostPort(host, strconv.Itoa(port)) +conn, err := net.Dial("tcp", addr) +``` + +`net.JoinHostPort` takes the port as a string. For an `int` port, wrap it in `strconv.Itoa`. The function is available in every supported Go version. diff --git a/packages/coding-agent/src/discovery/builtin-rules/go-new-expr.md b/packages/coding-agent/src/discovery/builtin-rules/go-new-expr.md new file mode 100644 index 000000000..25a4dc7c4 --- /dev/null +++ b/packages/coding-agent/src/discovery/builtin-rules/go-new-expr.md @@ -0,0 +1,44 @@ +--- +description: "Use new(expr) for pointer-to-value helpers instead of `func ptr[T any](v T) *T { return &v }` (Go 1.26)" +interruptMode: never +scope: "tool:edit(*.go), tool:write(*.go)" +astCondition: + - "func $F($V $T) *$T { return &$V }" + - "func $F[$$$TP]($V $T) *$T { return &$V }" +--- + +Go 1.26 lets `new` take an expression: `new(expr)` allocates, stores `expr`, and returns its `*T`. That removes the need for hand-written `Ptr`/`boolPtr`/`Int64`-style helpers and the `x := v; p := &x` two-step. + +## Why + +- One builtin replaces a helper per type (`boolPtr`, `strPtr`, `int64Ptr`, …) and the generic `func Ptr[T any](v T) *T`. +- No extra function-call frame and no separate heap escape — the value is constructed directly in the allocation. +- The intent (`new(false)`) reads at the call site instead of hiding behind a helper name. + +## Avoid + +```go +// A helper that just takes a value and returns its address. +func boolPtr(v bool) *bool { return &v } +func strPtr(v string) *string { return &v } +func Ptr[T any](v T) *T { return &v } + +cfg := Config{Enabled: boolPtr(true), Name: strPtr("svc")} +``` + +## Use + +```go +cfg := Config{Enabled: new(true), Name: new("svc")} + +// Was: x := int64(300); p := &x +p := new(int64(300)) +``` + +`new(true)` / `new(false)` give you `*bool`; `new(expr)` works for any expression, including function results (`new(time.Now())`). + +## Notes + +- Requires Go 1.26+. If the module's `go` directive is older, keep the helper or the temp-variable form until the toolchain is bumped. +- This is for helpers that *only* take a value and return its address. A function that does real work before taking an address is not in scope. +- `new(T)` (a bare type) is unchanged and still zero-initializes. diff --git a/packages/coding-agent/src/discovery/builtin-rules/go-rand-v2.md b/packages/coding-agent/src/discovery/builtin-rules/go-rand-v2.md new file mode 100644 index 000000000..6147ff0ec --- /dev/null +++ b/packages/coding-agent/src/discovery/builtin-rules/go-rand-v2.md @@ -0,0 +1,40 @@ +--- +description: Prefer math/rand/v2 over the legacy math/rand package +condition: '"math/rand"' +scope: "tool:edit(*.go), tool:write(*.go)" +--- + +Use `math/rand/v2` instead of the legacy `math/rand` package (stable since Go 1.22). + +## Why + +- No global `Seed`: `math/rand`'s top-level functions read a process-global generator (auto-seeded since Go 1.20), so a fixed seed is global mutable state that's easy to misuse; `v2` drops the global `Seed` entirely. +- Cleaner, better-bounded API: `rand.IntN(n)` / generic `rand.N(n)` replace `rand.Intn(n)`, and `Shuffle`, `Perm`, `Float64` carry over with clearer names. +- Modern generators: `v2` exposes `PCG` and `ChaCha8` sources instead of the old default LCG. + +## Migration + +```go +// Before +import "math/rand" +n := rand.Intn(100) +f := rand.Float64() + +// After +import "math/rand/v2" +n := rand.IntN(100) +f := rand.Float64() +``` + +| math/rand | math/rand/v2 | +| --- | --- | +| `rand.Intn(n)` | `rand.IntN(n)` | +| `rand.Int63n(n)` | `rand.Int64N(n)` | +| `rand.Intn`/`Int31n` on a `*Rand` | `(*Rand).IntN` / `Int32N` | +| `rand.Seed(x)` | drop it — `v2` has no global seed | +| explicit `rand.New(rand.NewSource(seed))` | `rand.New(rand.NewPCG(s1, s2))` or `rand.NewChaCha8(seed)` | + +## Keep math/rand only when + +- You need a reproducible stream from a fixed seed via the classic `NewSource`/`Seed` API that a caller already depends on. +- Reach for `crypto/rand` instead when the values are security-sensitive — neither `math/rand` variant is cryptographically secure. diff --git a/packages/coding-agent/src/discovery/builtin-rules/go-range-int.md b/packages/coding-agent/src/discovery/builtin-rules/go-range-int.md new file mode 100644 index 000000000..7b72c62e7 --- /dev/null +++ b/packages/coding-agent/src/discovery/builtin-rules/go-range-int.md @@ -0,0 +1,45 @@ +--- +description: "Use for i := range n instead of the C-style for i := 0; i < n; i++ loop (Go 1.22)" +interruptMode: never +scope: "tool:edit(*.go), tool:write(*.go)" +astCondition: + - "for $I := 0; $I < $N; $I++ { $$$BODY }" +--- + +Go 1.22 lets `for` range over an integer. A plain counting loop from `0` to `n` with step `1` reads better as `for i := range n` (or `for range n` when the index is unused). + +## Avoid + +```go +for i := 0; i < n; i++ { + use(i) +} + +for i := 0; i < len(s); i++ { + use(s[i]) +} +``` + +## Use + +```go +for i := range n { + use(i) +} + +// Ranging the slice directly is usually clearer than indexing. +for i := range s { + use(s[i]) +} + +// Index unused → drop it entirely. +for range n { + tick() +} +``` + +## When it does not apply + +- Non-zero start, step other than `++`, or a descending loop (`for i := n - 1; i >= 0; i--`) — keep the explicit form. +- The body reassigns the loop variable or depends on `i` surviving past the loop. +- Requires Go 1.22+. If the module's `go` directive is older, keep the classic loop. diff --git a/packages/coding-agent/src/discovery/builtin-rules/index.ts b/packages/coding-agent/src/discovery/builtin-rules/index.ts index 01c6cd6e6..71718f012 100644 --- a/packages/coding-agent/src/discovery/builtin-rules/index.ts +++ b/packages/coding-agent/src/discovery/builtin-rules/index.ts @@ -8,6 +8,14 @@ * Registered by the lowest-priority `builtin-defaults` rule provider so any * user/project/tool rule with the same name overrides the bundled copy. */ +import goAddCleanup from "./go-add-cleanup.md" with { type: "text" }; +import goBenchLoop from "./go-bench-loop.md" with { type: "text" }; +import goExpPromoted from "./go-exp-promoted.md" with { type: "text" }; +import goIoutil from "./go-ioutil.md" with { type: "text" }; +import goJoinHostport from "./go-join-hostport.md" with { type: "text" }; +import goNewExpr from "./go-new-expr.md" with { type: "text" }; +import goRandV2 from "./go-rand-v2.md" with { type: "text" }; +import goRangeInt from "./go-range-int.md" with { type: "text" }; import rsBoxLeak from "./rs-box-leak.md" with { type: "text" }; import rsFuturePrelude from "./rs-future-prelude.md" with { type: "text" }; import rsLazylock from "./rs-lazylock.md" with { type: "text" }; @@ -35,6 +43,14 @@ export interface BuiltinRuleSource { /** All bundled default rules, ordered by name. */ export const BUILTIN_RULE_SOURCES: readonly BuiltinRuleSource[] = [ + { name: "go-add-cleanup", content: goAddCleanup }, + { name: "go-bench-loop", content: goBenchLoop }, + { name: "go-exp-promoted", content: goExpPromoted }, + { name: "go-ioutil", content: goIoutil }, + { name: "go-join-hostport", content: goJoinHostport }, + { name: "go-new-expr", content: goNewExpr }, + { name: "go-rand-v2", content: goRandV2 }, + { name: "go-range-int", content: goRangeInt }, { name: "rs-box-leak", content: rsBoxLeak }, { name: "rs-future-prelude", content: rsFuturePrelude }, { name: "rs-lazylock", content: rsLazylock }, diff --git a/packages/coding-agent/test/discovery/builtin-defaults.test.ts b/packages/coding-agent/test/discovery/builtin-defaults.test.ts index 4a6cc9ad2..80b626b99 100644 --- a/packages/coding-agent/test/discovery/builtin-defaults.test.ts +++ b/packages/coding-agent/test/discovery/builtin-defaults.test.ts @@ -146,6 +146,83 @@ describe("builtin-defaults rule provider", () => { }), ).toEqual([]); }); + it("go-new-expr matches value→pointer helpers (named + generic) but not real functions, only on *.go", async () => { + const rules = await loadBuiltinRules(); + const rule = rules.find(r => r.name === "go-new-expr"); + if (!rule) throw new Error("go-new-expr rule missing"); + const manager = new TtsrManager(); + expect(manager.addRule(rule)).toBe(true); + const ctx: TtsrMatchContext = { source: "tool", toolName: "edit", filePaths: ["pkg/foo.go"] }; + + const hits = [ + "package p\nfunc boolPtr(v bool) *bool { return &v }", + "package p\nfunc Ptr[T any](v T) *T { return &v }", + ]; + for (const snippet of hits) { + manager.resetBuffer(); + expect( + (await manager.checkAstSnapshot(snippet, ctx)).map(m => m.name), + snippet, + ).toEqual(["go-new-expr"]); + } + + const misses = [ + "package p\nfunc add(a int, b int) *int { return &a }", + "package p\nfunc (s *S) Get() *int { return &s.x }", + ]; + for (const snippet of misses) { + manager.resetBuffer(); + expect(await manager.checkAstSnapshot(snippet, ctx), snippet).toEqual([]); + } + + // AST conditions never reach a non-go path. + manager.resetBuffer(); + expect( + await manager.checkAstSnapshot(hits[0], { source: "tool", toolName: "edit", filePaths: ["pkg/foo.ts"] }), + ).toEqual([]); + }); + + it("go-bench-loop fires on a *testing.B b.N loop but not an ordinary .N counter", async () => { + const rules = await loadBuiltinRules(); + const rule = rules.find(r => r.name === "go-bench-loop"); + if (!rule) throw new Error("go-bench-loop rule missing"); + const manager = new TtsrManager(); + expect(manager.addRule(rule)).toBe(true); + const ctx: TtsrMatchContext = { source: "tool", toolName: "edit", filePaths: ["pkg/foo_test.go"] }; + + const bench = + "package p\nfunc BenchmarkX(b *testing.B) {\n\tsetup()\n\tfor i := 0; i < b.N; i++ {\n\t\twork()\n\t}\n}"; + manager.resetBuffer(); + expect((await manager.checkAstSnapshot(bench, ctx)).map(m => m.name)).toEqual(["go-bench-loop"]); + + // A `.N` selector on something that is not the benchmark receiver must not fire. + const helper = + "package p\nfunc TestThing(t *testing.T) {\n\treq := build()\n\tfor i := 0; i < req.N; i++ {\n\t\twork()\n\t}\n}"; + manager.resetBuffer(); + expect(await manager.checkAstSnapshot(helper, ctx)).toEqual([]); + }); + + it("go-range-int fires only on *.go, never on a same-named non-go path", async () => { + const rules = await loadBuiltinRules(); + const rule = rules.find(r => r.name === "go-range-int"); + if (!rule) throw new Error("go-range-int rule missing"); + const manager = new TtsrManager(); + expect(manager.addRule(rule)).toBe(true); + + const loop = "package p\nfunc f(n int) {\n\tfor i := 0; i < n; i++ {\n\t\tuse(i)\n\t}\n}"; + manager.resetBuffer(); + expect( + (await manager.checkAstSnapshot(loop, { source: "tool", toolName: "edit", filePaths: ["pkg/foo.go"] })).map( + m => m.name, + ), + ).toEqual(["go-range-int"]); + // A step-2 loop is not equivalent to range-over-int and must not fire. + const step2 = "package p\nfunc f(n int) {\n\tfor i := 0; i < n; i += 2 {\n\t\tuse(i)\n\t}\n}"; + manager.resetBuffer(); + expect( + await manager.checkAstSnapshot(step2, { source: "tool", toolName: "edit", filePaths: ["pkg/foo.go"] }), + ).toEqual([]); + }); it("is the lowest-priority rule provider so user/project rules override defaults", () => { const { cap, provider } = ruleProvider(); diff --git a/packages/coding-agent/test/tools/web-search-duckduckgo.test.ts b/packages/coding-agent/test/tools/web-search-duckduckgo.test.ts index 4da54bddd..8c3ea24be 100644 --- a/packages/coding-agent/test/tools/web-search-duckduckgo.test.ts +++ b/packages/coding-agent/test/tools/web-search-duckduckgo.test.ts @@ -73,7 +73,7 @@ describe("DuckDuckGo web search provider", () => { const headers = capturedInit?.headers as Record; expect(headers["Content-Type"]).toBe("application/x-www-form-urlencoded"); expect(headers["User-Agent"]).toContain("Mozilla/5.0"); - expect(headers["Referer"]).toBe("https://html.duckduckgo.com/"); + expect(headers.Referer).toBe("https://html.duckduckgo.com/"); expect(headers["Accept-Language"]).toContain("en"); expect(headers["Sec-Fetch-Mode"]).toBe("navigate"); expect(headers["Sec-Ch-Ua"]).toContain("Chromium");