diff --git a/docs/go-architecture-patterns.md b/docs/go-architecture-patterns.md index 40e0ab3..17e87b6 100644 --- a/docs/go-architecture-patterns.md +++ b/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 diff --git a/docs/go-cli-guide.md b/docs/go-cli-guide.md index 4c27ce6..935217f 100644 --- a/docs/go-cli-guide.md +++ b/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=` / `request_id=` 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 diff --git a/docs/go-glog-guide.md b/docs/go-glog-guide.md index 717c7da..d66b2a3 100644 --- a/docs/go-glog-guide.md +++ b/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 diff --git a/docs/go-json-error-handler-guide.md b/docs/go-json-error-handler-guide.md index 0da6901..b46a182 100644 --- a/docs/go-json-error-handler-guide.md +++ b/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, ) ``` diff --git a/docs/go-k8s-binary-conventions.md b/docs/go-k8s-binary-conventions.md index 904dba7..a0d485f 100644 --- a/docs/go-k8s-binary-conventions.md +++ b/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) diff --git a/docs/go-patterns.md b/docs/go-patterns.md index 6f69b2f..50360fd 100644 --- a/docs/go-patterns.md +++ b/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 &`) +**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 diff --git a/docs/go-testing-guide.md b/docs/go-testing-guide.md index d55880b..0e20d59 100644 --- a/docs/go-testing-guide.md +++ b/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 diff --git a/rules/go/constructor-returns-interface.yml b/rules/go/constructor-returns-interface.yml new file mode 100644 index 0000000..183b8ba --- /dev/null +++ b/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/**" diff --git a/rules/go/glog-info-needs-v.yml b/rules/go/glog-info-needs-v.yml new file mode 100644 index 0000000..156fefc --- /dev/null +++ b/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/**" diff --git a/rules/go/no-custom-ptr-helpers.yml b/rules/go/no-custom-ptr-helpers.yml new file mode 100644 index 0000000..7f35722 --- /dev/null +++ b/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 `¶m` 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/**" diff --git a/rules/go/no-testing-t-direct.yml b/rules/go/no-testing-t-direct.yml new file mode 100644 index 0000000..f2b16ce --- /dev/null +++ b/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/**" diff --git a/rules/go/secret-fields-need-display-length.yml b/rules/go/secret-fields-need-display-length.yml new file mode 100644 index 0000000..845ab47 --- /dev/null +++ b/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/**" diff --git a/rules/go/slog-not-glog-in-new-projects.yml b/rules/go/slog-not-glog-in-new-projects.yml new file mode 100644 index 0000000..fb94a15 --- /dev/null +++ b/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/**" diff --git a/rules/go/use-error-code-constants.yml b/rules/go/use-error-code-constants.yml new file mode 100644 index 0000000..8898bf6 --- /dev/null +++ b/rules/go/use-error-code-constants.yml @@ -0,0 +1,37 @@ +id: go-json-error-handler/use-error-code-constants +language: go +severity: error +message: | + HTTP error-code argument to `libhttp.WrapWithCode` / + `libhttp.WrapWithDetails` is a raw string literal instead of a + `libhttp.ErrorCodeXxx` constant. Typos in raw strings ship + silently — clients pattern-matching on the code surface the wrong + dispatch arm. Use the constants (`libhttp.ErrorCodeValidation`, + `ErrorCodeNotFound`, etc.) so typos fail at compile time and + grep finds every code from one source of truth. + See docs/go-json-error-handler-guide.md + (RULE go-json-error-handler/use-error-code-constants). +rule: + # libhttp signatures (verified against + # ~/Documents/workspaces/http/http_error-handler.go): + # WrapWithCode(err error, code string, statusCode int) — code is arg #2 + # WrapWithDetails(err error, code string, statusCode int, details) — code is arg #2 + # `NewJSONError` is NOT a real libhttp function (the doc text mis-named + # `NewJSONErrorHandler`, which is a handler factory and not an error + # constructor — different surface entirely). Drop it from the regex. + # + # `constraints:` only applies at rule-top-level (not inside any/all + # items). Shared $CODE metavariable in position #2 across both arities. + any: + - all: + - pattern: 'libhttp.WrapWithCode($A, $CODE, $C)' + - all: + - pattern: 'libhttp.WrapWithDetails($A, $CODE, $C, $D)' +constraints: + CODE: + kind: interpreted_string_literal +ignores: + - "**/*_test.go" + - "vendor/**" + - "**/vendor/**" + - "**/mocks/**" diff --git a/rules/index.json b/rules/index.json index 2955cf6..8d16cd6 100644 --- a/rules/index.json +++ b/rules/index.json @@ -129,7 +129,7 @@ "anchor": "go-architecture/constructor-returns-interface", "applies_when": "a Go `New*` constructor returns the concrete struct type (`*userService`) instead of the interface it implements (`UserService`).", "doc_path": "docs/go-architecture-patterns.md", - "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.", "id": "go-architecture/constructor-returns-interface", "level": "MUST", "owner": "go-architecture-assistant" @@ -201,7 +201,7 @@ "anchor": "go-cli/slog-not-glog-in-new-projects", "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.", "doc_path": "docs/go-cli-guide.md", - "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.", "id": "go-cli/slog-not-glog-in-new-projects", "level": "MUST", "owner": "go-quality-assistant" @@ -435,7 +435,7 @@ "anchor": "go-glog/use-v-for-debug-not-info", "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)`.", "doc_path": "docs/go-glog-guide.md", - "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.", "id": "go-glog/use-v-for-debug-not-info", "level": "MUST", "owner": "go-quality-assistant" @@ -505,9 +505,9 @@ }, { "anchor": "go-json-error-handler/use-error-code-constants", - "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.", + "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.", "doc_path": "docs/go-json-error-handler-guide.md", - "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)", + "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).", "id": "go-json-error-handler/use-error-code-constants", "level": "MUST", "owner": "go-http-handler-assistant" @@ -525,7 +525,7 @@ "anchor": "go-k8s-binary/secret-fields-need-display-length", "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.", "doc_path": "docs/go-k8s-binary-conventions.md", - "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).", "id": "go-k8s-binary/secret-fields-need-display-length", "level": "MUST", "owner": "go-security-specialist" @@ -660,7 +660,7 @@ "anchor": "go-patterns/bborbe-collection-ptr-not-helpers", "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(...)`.", "doc_path": "docs/go-patterns.md", - "enforcement": "judgment (ast-grep follow-up: `function_declaration` returning a pointer-to-builtin with a body containing only `return &`)", + "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.", "id": "go-patterns/bborbe-collection-ptr-not-helpers", "level": "MUST", "owner": "go-quality-assistant" @@ -849,7 +849,7 @@ "anchor": "go-testing/no-testing-t-direct", "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.", "doc_path": "docs/go-testing-guide.md", - "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.", "id": "go-testing/no-testing-t-direct", "level": "MUST", "owner": "go-test-quality-assistant"