feat(rules): batch 5 — 6 ast-grep YAMLs

docs/go-architecture-patterns.md

@@ -92,7 +92,7 @@ func NewUserService(
 
 **Owner**: go-architecture-assistant
 **Applies when**: a Go `New*` constructor returns the concrete struct type (`*userService`) instead of the interface it implements (`UserService`).
-**Enforcement**: judgment (ast-grep follow-up: `function_declaration` with name `^New` and result containing the concrete struct, paired with the corresponding interface declaration in the same package)
+**Enforcement**: `rules/go/constructor-returns-interface.yml` flags every `func NewXxx(...) *ConcreteStruct` declaration. The agent decides per-finding whether the returned concrete type has a corresponding interface in the same package (real violation) or is a data-holder / config / DTO struct without an interface (exempt) — the same-package interface lookup needs cross-file reasoning ast-grep can't do in a single rule.
 **Why**: Returning the concrete struct leaks implementation: callers can reach for non-interface methods, type-assert downstream, depend on private struct fields via reflection. Returning the interface forces every consumer through the contract surface — refactors stay contained, mocks stay drop-in, dependency direction stays one-way.
 
 #### Bad

docs/go-cli-guide.md

@@ -173,7 +173,7 @@ func Run(ctx context.Context, args []string) error {
 
 **Owner**: go-quality-assistant
 **Applies when**: a *new* Go CLI binary (created after Go 1.21 release; no prior glog usage in the same module) imports `github.com/golang/glog`. Existing glog-using projects are exempt — they should not introduce slog and glog side by side.
-**Enforcement**: judgment (semantic — distinguishing "new project" from "existing project mid-migration" requires checking git history / module age; ast-grep partial: `import "github.com/golang/glog"` in any main module without prior glog usage)
+**Enforcement**: `rules/go/slog-not-glog-in-new-projects.yml` flags every `import "github.com/golang/glog"`. The agent decides whether each finding is a real violation (new project introducing glog) or an exempt case (existing project mid-migration with prior glog usage) — that distinction needs git-history / module-age reasoning ast-grep can't do.
 **Why**: glog has two structural problems slog doesn't: (1) it registers 8+ flags via stdlib `flag.init()` which pollutes every binary's `--help` output (see `go-cli/cobra-not-stdlib-flag`); (2) it predates structured logging — every log line is a free-form string, so log aggregators can't reliably parse `user_id=<X>` / `request_id=<Y>` fields. `log/slog` (stdlib Go 1.21+) emits structured key-value logs, integrates with `context.Context` for request-scoped fields, and has no `flag` pollution. For *existing* glog projects, the migration cost is real and not always worth it — but new projects should not pay the glog tax.
 
 #### Bad

docs/go-glog-guide.md

@@ -8,7 +8,7 @@ Google's [glog](https://github.com/golang/glog) provides more granular logging l
 
 **Owner**: go-quality-assistant
 **Applies when**: a Go file calls `glog.Info` / `glog.Infof` for debug, trace, internal-state, or developer-troubleshooting log lines that should be filtered out in production — instead of `glog.V(N).Info...` / `glog.V(N).Infof...` with an appropriate verbosity level (1 for sysop debug, 2 for dev default, 3+ for deep debug). Scope: applies across ALL V levels — V0 is reserved for production-default operator info; everything debug-shaped goes through `V(N)`.
-**Enforcement**: judgment (ast-grep follow-up: `call_expression` matching `glog.Info(...)` / `glog.Infof(...)` outside startup/shutdown contexts; the agent rules in "is this production-default-worthy?" based on the log content)
+**Enforcement**: `rules/go/glog-info-needs-v.yml` flags every `glog.Info(...)` / `glog.Infof(...)` call. The agent decides per-finding whether the call is V0-worthy production-operator info (startup, shutdown, health change, recovered panic) or debug-shaped content that should be gated behind `V(N)` — that distinction needs reading the log message intent, which ast-grep can't do.
 **Why**: V0 (bare `glog.Info`) is the *always-on* production logging level. Every line at V0 ships to log aggregators, gets indexed, and burns CPU on every request whether anyone reads it or not. Using V0 for debug-style logs ("Cache miss for key %s", "Internal state: %+v") produces gigabytes of noise in production, drowns operators in irrelevant detail, and inflates log-storage cost. The `glog.V(N)` mechanism exists exactly to gate verbosity at runtime: V0 stays operator-readable (startup, shutdown, health changes), V1+ stays off by default and turns on for troubleshooting. Inverting the levels means production logs are useless and dev debugging requires a deploy.
 
 #### Bad

docs/go-json-error-handler-guide.md

@@ -83,17 +83,18 @@ All JSON errors follow this structure:
 ### RULE go-json-error-handler/use-error-code-constants (MUST)
 
 **Owner**: go-http-handler-assistant
-**Applies when**: a Go HTTP handler passes a raw string literal as the error-code argument to `libhttp.WrapWithCode` / `libhttp.NewJSONError` instead of the `libhttp.ErrorCodeXxx` constants.
-**Enforcement**: judgment (ast-grep follow-up: pattern over `libhttp.WrapWithCode($$, $$, $CODE)` / `WrapWithDetails($$, $$, $CODE, $$)` with `$CODE` constrained to be a `interpreted_string_literal` — see PR #11 recipe for the metavariable-constraint shape)
+**Applies when**: a Go HTTP handler passes a raw string literal as the error-code argument to `libhttp.WrapWithCode` / `libhttp.WrapWithDetails` instead of the `libhttp.ErrorCodeXxx` constants.
+**Enforcement**: `rules/go/use-error-code-constants.yml` flags `libhttp.WrapWithCode(err, $CODE, statusCode)` / `libhttp.WrapWithDetails(err, $CODE, statusCode, details)` calls whose `$CODE` argument (position #2 per the libhttp signature) is an `interpreted_string_literal` (raw string) instead of an `ErrorCodeXxx` constant selector_expression. Uses the metavariable-constraint shape (PR #11 recipe).
 **Why**: Error codes are the dispatch surface clients pattern-match on. A typo in `"VAIDATION_ERROR"` ships silently — the client's `if code == "VALIDATION_ERROR"` branch never fires, the error falls through to the generic handler, and the bug surfaces as "validation errors don't show the inline form-field highlight." Constants make typos fail at compile time, give grep a single source of truth for which codes exist, and let the constant's godoc anchor the HTTP-status / semantic contract per code.
 
 #### Bad
 
 ```go
+// libhttp signature: WrapWithCode(err error, code string, statusCode int)
 return libhttp.WrapWithCode(
 	errors.Errorf(ctx, "invalid input"),
-	http.StatusBadRequest,
 	"VAIDATION_ERROR", // typo — client dispatch silently misses this
+	http.StatusBadRequest,
 )
 ```
 
@@ -102,8 +103,8 @@ return libhttp.WrapWithCode(
 ```go
 return libhttp.WrapWithCode(
 	errors.Errorf(ctx, "invalid input"),
-	http.StatusBadRequest,
 	libhttp.ErrorCodeValidation, // typo fails at compile time
+	http.StatusBadRequest,
 )
 ```
 

docs/go-k8s-binary-conventions.md

@@ -20,7 +20,7 @@ Every Go binary that runs in a k8s pod (StatefulSet, Deployment, CronJob, epheme
 
 **Owner**: go-security-specialist
 **Applies when**: an `application` struct field in a Go k8s-deployed binary holds secret material (PEM key, OAuth token, password, API key, JWT signing secret) without the `display:"length"` tag — meaning glog / structured-logging dumps of the application config will print the secret value.
-**Enforcement**: judgment (ast-grep follow-up: `struct_field_declaration` with name matching `PEM*` / `*Token*` / `*Secret*` / `*Password*` / `*Key` outside `display:"length"` tag)
+**Enforcement**: `rules/go/secret-fields-need-display-length.yml` flags `field_declaration` whose name contains `PEM` / `Token` / `Secret` / `Password` / `Key` / `Credential` and whose tag either lacks `display:"length"` or is absent entirely. The agent dismisses incidental name matches (e.g. `MasterKey` for a non-credential master record).
 **Why**: `argument.Parse()` prints the application config at startup. Without `display:"length"`, the secret value lands in stdout / glog / log aggregators — searchable, indexed, and impossible to redact retroactively once the log batch has shipped. `display:"length"` substitutes `length=42` for the value, preserving the "is it set?" signal without the leak. The tag costs zero runtime; the leak costs a credential rotation.
 
 ### RULE go-k8s-binary/argument-struct-not-os-getenv (MUST)

docs/go-patterns.md

@@ -59,7 +59,7 @@ func (s *myService) Process(ctx context.Context, ...) error { ... }
 
 **Owner**: go-quality-assistant
 **Applies when**: a Go file declares a custom pointer helper function (`func stringPtr(s string) *string { return &s }`, `func intPtr(...)`, etc.) instead of importing `github.com/bborbe/collection` and calling `collection.Ptr(...)`.
-**Enforcement**: judgment (ast-grep follow-up: `function_declaration` returning a pointer-to-builtin with a body containing only `return &<param>`)
+**Enforcement**: `rules/go/no-custom-ptr-helpers.yml` matches the canonical pointer-helper signature `func N(P T) *T { return &P }` (same param identifier on both sides). The agent decides per-finding whether the function is a real pointer-helper (one-arg, returns `&that_arg`) or a one-liner constructor that incidentally returns a pointer to a struct literal (`&Type{...}`) — the latter is exempt.
 **Why**: Every codebase that needs pointer values for optional struct fields ends up writing 3-5 of these helpers. They proliferate across packages, each one slightly differently named (`strPtr` vs `stringPtr` vs `pStr`), and tests do their own (`stringPtr("test")` in test A, `&[]string{"test"}[0]` in test B). `github.com/bborbe/collection.Ptr` is a generic single helper that handles every type. Adopting it everywhere collapses the proliferation to one import line per package and lets refactors find every usage with `grep collection.Ptr`.
 
 #### Bad

docs/go-testing-guide.md

@@ -51,7 +51,7 @@ var _ = DescribeTable("foo",
 
 **Owner**: go-test-quality-assistant
 **Applies when**: a Go file in a package that has a Ginkgo `TestSuite` entry-point uses `*testing.T` directly inside test functions other than the suite entry-point itself.
-**Enforcement**: judgment (ast-grep follow-up)
+**Enforcement**: `rules/go/no-testing-t-direct.yml` flags every `func TestXxx(t *testing.T)` declaration. The agent decides per-finding whether the function is the legitimate Ginkgo suite entry-point (`RegisterFailHandler(Fail); RunSpecs(t, "Suite")`) or a stdlib-style test that needs to be ported to Ginkgo — that distinction needs reading the function body's intent, which ast-grep can't do reliably.
 **Why**: Direct `testing.T` use bypasses the Ginkgo lifecycle (`BeforeEach`/`AfterEach`/`JustBeforeEach`), produces flaky setup ordering, and breaks `--focus` filtering. Use `Describe`/`Context`/`It`/`DescribeTable`/`Entry` so the suite runs as one coherent test plan.
 
 #### Bad

rules/go/constructor-returns-interface.yml

@@ -0,0 +1,36 @@
+id: go-architecture/constructor-returns-interface
+language: go
+severity: error
+message: |
+  `New*` constructor returns a pointer type (`*concreteStruct`)
+  instead of the interface the struct implements. Returning the
+  concrete pointer leaks implementation: callers can reach for
+  non-interface methods, type-assert downstream, depend on private
+  struct fields via reflection.
+  Return the interface (`UserService`) — consumers see only the
+  contract surface.
+  EXEMPT: data-holder structs without a corresponding interface
+  (e.g. config / DTO records). The agent decides per-finding by
+  checking whether the same package declares an interface that the
+  returned concrete type implements.
+  See docs/go-architecture-patterns.md
+  (RULE go-architecture/constructor-returns-interface).
+rule:
+  # `func NewXxx(...) *ConcreteStruct { ... }` — function_declaration
+  # with name starting with `New` and result a `pointer_type`. The
+  # interface-vs-data-holder check stays on the agent (needs same-
+  # package interface lookup).
+  kind: function_declaration
+  all:
+    - has:
+        field: name
+        kind: identifier
+        regex: '^New[A-Z]'
+    - has:
+        field: result
+        kind: pointer_type
+ignores:
+  - "**/*_test.go"
+  - "vendor/**"
+  - "**/vendor/**"
+  - "**/mocks/**"

rules/go/glog-info-needs-v.yml

@@ -0,0 +1,49 @@
+id: go-glog/use-v-for-debug-not-info
+language: go
+severity: error
+message: |
+  Bare `glog.Info` / `glog.Infof` runs at V0 (always-on production
+  logging). V0 is reserved for production-default operator info
+  (startup, shutdown, health changes, recovered panics).
+  Use `glog.V(N).Info...` / `glog.V(N).Infof...` for debug, trace,
+  internal-state, or developer-troubleshooting lines (V1 sysop debug,
+  V2 dev default, V3+ deep debug).
+  The agent decides which calls are V0-worthy operator info vs
+  debug-shaped — this YAML flags every glog.Info(f) for review.
+  See docs/go-glog-guide.md (RULE go-glog/use-v-for-debug-not-info).
+rule:
+  any:
+    # `glog.Info(...)` — call_expression with selector_expression operand
+    # operand=glog, field=Info.
+    - kind: call_expression
+      has:
+        field: function
+        kind: selector_expression
+        all:
+          - has:
+              field: operand
+              kind: identifier
+              regex: '^glog$'
+          - has:
+              field: field
+              kind: field_identifier
+              regex: '^Info$'
+    - kind: call_expression
+      has:
+        field: function
+        kind: selector_expression
+        all:
+          - has:
+              field: operand
+              kind: identifier
+              regex: '^glog$'
+          - has:
+              field: field
+              kind: field_identifier
+              regex: '^Infof$'
+ignores:
+  # Test files exempt — test setup/teardown logs at V0 are fine.
+  - "**/*_test.go"
+  - "vendor/**"
+  - "**/vendor/**"
+  - "**/mocks/**"

rules/go/no-custom-ptr-helpers.yml

@@ -0,0 +1,28 @@
+id: go-patterns/bborbe-collection-ptr-not-helpers
+language: go
+severity: error
+message: |
+  Custom pointer helper (e.g. `func stringPtr(s string) *string { return &s }`)
+  duplicates `github.com/bborbe/collection.Ptr`. Every codebase needing
+  pointer values for optional struct fields ends up writing 3-5 of
+  these helpers under different names. Import `github.com/bborbe/collection`
+  and call `collection.Ptr(...)` — generic, single helper for every type.
+  EXEMPT: regular constructors that happen to be one-liners
+  (`func NewX(p T) *X { return &X{Field: p} }`). The agent decides
+  per-finding whether the function is a pointer-helper (return value
+  is `&param` where param is the single argument) vs a constructor
+  (return value is `&Type{...}` literal). This YAML matches the
+  canonical pointer-helper shape via `pattern:`.
+  See docs/go-patterns.md (RULE go-patterns/bborbe-collection-ptr-not-helpers).
+rule:
+  # ast-grep 0.43 Go grammar: function_declaration's name / params /
+  # result / body are positional children, not fields, so `kind +
+  # field: result` doesn't match (verified empirically). Use a
+  # `pattern:` shape instead — it walks the tree positionally and
+  # matches the exact canonical pointer-helper signature.
+  pattern: 'func $N($P $T) *$T { return &$P }'
+ignores:
+  - "**/*_test.go"
+  - "vendor/**"
+  - "**/vendor/**"
+  - "**/mocks/**"

rules/go/no-testing-t-direct.yml

@@ -0,0 +1,51 @@
+id: go-testing/no-testing-t-direct
+language: go
+severity: error
+message: |
+  Direct `*testing.T` use bypasses the Ginkgo lifecycle
+  (BeforeEach/AfterEach/JustBeforeEach), produces flaky setup
+  ordering, and breaks --focus filtering. Use Describe/Context/It/
+  DescribeTable/Entry instead.
+  EXEMPT: the Ginkgo suite entry-point itself
+  (`func TestXxx(t *testing.T) { RegisterFailHandler(Fail); RunSpecs(t, "Suite") }`).
+  The agent decides per-finding whether the function is the legit
+  suite entry-point or a stdlib-style test that should be ported
+  to Ginkgo — this YAML flags every `func TestXxx(t *testing.T)`
+  in a _test.go file.
+  See docs/go-testing-guide.md (RULE go-testing/no-testing-t-direct).
+rule:
+  # `function_declaration` whose name starts with `Test` (Go test
+  # convention) and which has a parameter typed `*testing.T`.
+  kind: function_declaration
+  all:
+    - has:
+        field: name
+        kind: identifier
+        regex: '^Test'
+    - has:
+        field: parameters
+        kind: parameter_list
+        has:
+          stopBy: end
+          kind: pointer_type
+          has:
+            stopBy: end
+            kind: qualified_type
+            all:
+              - has:
+                  field: package
+                  kind: package_identifier
+                  regex: '^testing$'
+              - has:
+                  field: name
+                  kind: type_identifier
+                  regex: '^T$'
+ignores:
+  # Convention: `func TestXxx(t *testing.T)` is only meaningful inside
+  # `_test.go` files (go test only picks those up). ast-grep YAML at
+  # 0.43 has no `files:` include filter, so we exclude the common
+  # non-test code paths and accept that the rare non-test file with
+  # this shape will be flagged for the agent to dismiss.
+  - "vendor/**"
+  - "**/vendor/**"
+  - "**/mocks/**"

rules/go/secret-fields-need-display-length.yml

@@ -0,0 +1,41 @@
+id: go-k8s-binary/secret-fields-need-display-length
+language: go
+severity: error
+message: |
+  Secret-shaped struct field (PEM/Token/Secret/Password/Key) lacks
+  `display:"length"` tag. argument.Parse() prints the application
+  config at startup; without `display:"length"` the secret value
+  lands in stdout / glog / log aggregators — searchable, indexed,
+  and impossible to redact retroactively.
+  Add `display:"length"` to the field's tag. It substitutes
+  `length=42` for the value, preserving the "is it set?" signal
+  without the leak. Zero runtime cost.
+  EXEMPT: non-secret fields incidentally matching the regex (e.g.
+  `MasterKey` for a non-credential master record). The agent
+  decides per-finding whether the field actually holds secret
+  material.
+  See docs/go-k8s-binary-conventions.md
+  (RULE go-k8s-binary/secret-fields-need-display-length).
+rule:
+  # field_declaration with name in the secret-pattern set AND
+  # either (a) a raw-string tag that lacks `display:"length"`, or
+  # (b) no tag at all (unbound but secret-shaped — still leaks
+  # if anyone glogs the struct).
+  kind: field_declaration
+  all:
+    - has:
+        kind: field_identifier
+        regex: '(PEM|Token|Secret|Password|Key|Credential)'
+    - any:
+        - has:
+            kind: raw_string_literal
+            not:
+              regex: 'display:"length"'
+        - not:
+            has:
+              kind: raw_string_literal
+ignores:
+  - "**/*_test.go"
+  - "vendor/**"
+  - "**/vendor/**"
+  - "**/mocks/**"

rules/go/slog-not-glog-in-new-projects.yml

@@ -0,0 +1,26 @@
+id: go-cli/slog-not-glog-in-new-projects
+language: go
+severity: error
+message: |
+  New Go projects should use 'log/slog' (Go 1.21+ stdlib) instead of
+  'github.com/golang/glog'. glog registers 8+ flags via init() that
+  pollute every binary's --help output and emits unstructured strings
+  that log aggregators can't parse reliably.
+  EXEMPT: existing projects mid-migration. The agent decides 'is this
+  a new project?' based on whether the module already uses glog
+  elsewhere — this YAML flags every glog import for review.
+  See docs/go-cli-guide.md (RULE go-cli/slog-not-glog-in-new-projects).
+rule:
+  # ast-grep 0.43.0 shape: match `import_spec` nodes whose path text
+  # equals "github.com/golang/glog". Same shape as the cobra-not-stdlib-flag
+  # YAML's import-detection branch (PR #29).
+  kind: import_spec
+  has:
+    field: path
+    regex: '^"github\.com/golang/glog"$'
+ignores:
+  # Test files exempt — legacy test helpers may keep glog.
+  - "**/*_test.go"
+  - "vendor/**"
+  - "**/vendor/**"
+  - "**/mocks/**"