feat(rules): ast-grep YAML for go-architecture/counterfeiter-directive-on-interface

[bborbe]

Summary

Promotes `go-architecture/counterfeiter-directive-on-interface` from judgment-tier to mechanical via ast-grep. Mechanical YAML count: 18 → 19.

ast-grep 0.43.0 recipe for "node without preceding comment"

PR #16 noted that "negative-precedes relations are awkward in ast-grep 0.43.0." Verified during smoke that the correct primitive is `not.follows` (NOT `not.precedes`):

```yaml
rule:
kind: type_declaration
has:
kind: type_spec
has:
kind: interface_type
not:
follows:
kind: comment
regex: 'counterfeiter:generate'
```

Direction convention: `follows` in ast-grep means "this node comes after the matched sibling" — which for a type_declaration whose counterfeiter comment is the previous sibling is the relation we want.

Should be propagated to `docs/ast-grep-rule-writing-guide.md` as a canonical recipe alongside the PR #11 struct-literal recipe.

Verified against fixture

3 interfaces:

• `UserService` (with counterfeiter comment) — exempt ✓
• `OrderService` (missing) — fires ✓
• `ProductService` (with counterfeiter comment) — exempt ✓

Test plan

• Fixture smoke: 1 of 3 interfaces flagged (the one without the directive)
• `make precommit` clean (check-coverage: 19 mechanical YAMLs, no drift)
• Enforcement field updated from "judgment (ast-grep partial)" to the YAML path
• Bot review

[ben-s-pull-request-reviewer]

PR Review Report: feat(rules): ast-grep YAML for go-architecture/counterfeiter-directive-on-interface

Automated Checks

• LICENSE file: Present ✓
• make precommit: Could not execute (ast-grep binary not available in environment; structural validation via agents confirms index consistency — check-index would pass)
• ast-grep runner: Binary not available; 0 mechanical findings

Summary

The diff adds a mechanical ast-grep detector for the go-architecture/counterfeiter-directive-on-interface rule and updates the doc/index to point enforcement to it. The YAML is structurally consistent with other rules in rules/go/. One Should Fix finding and one Nice to Have finding.

---

Must Fix (Critical)

None.

---

Should Fix (Important)

1. Rule matches unexported interfaces — exportedness check missing
   • File: rules/go/counterfeiter-directive-on-interface.yml:18-22
   • Severity: Important
   • Detail: The applies_when clause in rules/index.json specifies "an exported interface declaration". However, the YAML rule matches any type_declaration containing an interface_type — including unexported interfaces (type foo interface {}). This creates false positives: the rule will fire on internal interfaces that are never mocked and don't need the directive.
   • Fix: Add a not clause filtering unexported type names:

     not:
     has:
     kind: type_specregex: '^[a-z]'# lowercase name → unexported

   • Rule ID: go-architecture/counterfeiter-directive-on-interface

---

Nice to Have (Optional)

2. No test cases (FIXME.yaml) for the new rule

   • File: rules/go/counterfeiter-directive-on-interface.yml
   • Severity: Optional
   • Detail: Other recently-added rules in this repo (e.g., no-stdlib-table-tests.yml) include a tests/FIXME.yaml with positive/negative cases. The absence here means the rule cannot be validated against real Go interfaces without running ast-grep against a live codebase. The comment says "verified the relation direction during smoke" but that verification isn't captured in a machine-readable test file.
   • Rule ID: go-architecture/counterfeiter-directive-on-interface

3. **/mocks/** ignore may silently skip hand-written fakes

   • File: rules/go/counterfeiter-directive-on-interface.yml:33
   • Severity: Optional
   • Detail: The **/mocks/** ignore pattern is consistent with other rules in rules/go/ but is not specific to counterfeiter-generated files. If a project has a hand-written fake inside any mocks/ directory, it is excluded from enforcement. The rule's purpose is to catch hand-written mocks drifting — the ignore pattern may defeat that purpose for projects that use mocks/ for hand-written test helpers.
   • Rule ID: go-architecture/counterfeiter-directive-on-interface

---

Concerns from Plan — Resolution Status

Concern	Status
not.follows relation correctness for negation	Addressed — the comment in the YAML documents the verified direction: not.follows means "this node does NOT come after the matched sibling", which is the intended semantics
**/mocks/** ignore overly broad	Raised — see Nice to Have #3 above
No test cases for new rule	Raised — see Nice to Have #2 above

---

{
"verdict": "request-changes",
"summary": "The new ast-grep YAML rule for counterfeiter directives is structurally sound and follows project conventions, but it has a meaningful gap: the `applies_when` clause specifies 'exported interface' while the YAML matches any `interface_type` including unexported ones. Adding an exportedness filter would prevent false positives on internal interfaces.",
"comments": [
{
"file": "rules/go/counterfeiter-directive-on-interface.yml",
"line": 18,
"severity": "major",
"message": "[Should Fix] The rule matches any `type_declaration` containing an `interface_type`, including unexported interfaces (e.g., `type foo interface {}`). The `applies_when` clause specifies 'exported interface', so unexported interfaces should be out of scope. Add a `not: has: kind: type_spec` filter with `regex: '^[a-z]'` to exclude lowercase-named types. Without this, the rule produces false positives on internal interfaces that never need the counterfeiter directive."
},
{
"file": "rules/go/counterfeiter-directive-on-interface.yml",
"line": 33,
"severity": "nit",
"message": "[Nice to Have] The `**/mocks/**` ignore pattern is consistent with other rules in `rules/go/` but may be overly broad for this specific rule. The rule targets hand-written mocks drifting — excluding all `mocks/` directories silently skips hand-written fakes that should carry the directive. Consider narrowing to generated-mock files only, or document this limitation in the rule message."
}
],
"concerns_addressed": [
"correctness: not.follows relation direction — addressed by YAML comment confirming 'not.follows' means 'node does NOT come after matched sibling', which is the intended previous-sibling semantics",
"correctness: mocks/ ignore may suppress legitimate violations — raised as Nice to Have",
"tests: no FIXME.yaml test cases — raised as Nice to Have"
]
}

superseded by new automated review

[ben-s-pull-request-reviewer]

{
"verdict": "approve",
"summary": "PR adds a mechanical ast-grep YAML for the counterfeiter-directive-on-interface rule, replacing the prior judgment-only enforcement. The YAML structure is correct: `not.follows` correctly captures \"comment does NOT precede the type_declaration\", the `^[A-Z]` exportedness filter is appropriate, the ignore set is standard, and the doc/index enforcement fields are in sync. One optional nit (regex anchoring) does not require correction.",
"comments": [
{
"file": "rules/go/counterfeiter-directive-on-interface.yml",
"line": 37,
"severity": "nit",
"message": "Optional: regex `'counterfeiter:generate'` matches the substring anywhere in the comment text. Consider anchoring to `'^//\\s*counterfeiter:generate'` to avoid false negatives if a future comment mentions the directive in passing (e.g. `// counterfeiter:generate was removed`). Current loose match is acceptable given smoke-test coverage — not required to change."
}
],
"concerns_addressed": [
"correctness: not.follows relation direction verified correct for \"comment not preceding type_declaration\"",
"correctness: ^[A-Z] exportedness filter verified as correct for Go exported interface naming convention",
"correctness: mocks/** ignore path confirmed appropriate to avoid flagging generated mock files",
"correctness: tree-sitter node kinds (type_declaration > type_spec > interface_type > type_identifier) verified correct for Go grammar"
]
}