fix(rules): no-fmt-errorf — rewrite as structural rule to bypass Go type-conversion ambiguity

[bborbe]

Summary

`rules/go/no-fmt-errorf.yml` was parse-broken since the YAML shipped — the original `pattern: fmt.Errorf($$$ARGS)` was parsed by tree-sitter Go grammar as a `type_conversion_expression` because `Type(arg)` is a valid Go type cast at pattern-compile time (no surrounding expression context disambiguates). Real call sites like `return fmt.Errorf(...)` parse as `call_expression` because their surrounding context fixes the parse. Net effect: the rule matched nothing in real code and silently emitted zero findings on every PR review since it shipped.

Surfaced 2026-06-03 during scenario 004's walk: 4 of 5 deliberate violations fired, the fmt.Errorf one did not.

Why `kind: call_expression` alone doesn't fix it

`kind:` filters output (which AST node kinds are reported), not how the `pattern:` line is parsed at compile time. The pattern still mis-parses as type-conversion and produces no matches to filter. Anchoring `kind` is necessary but not sufficient.

Fix

Replace the broken pattern with a fully structural rule:

```yaml
rule:
kind: call_expression
has:
field: function
kind: selector_expression
all:
- has: { field: operand, kind: identifier, regex: '^fmt$' }
- has: { field: field, kind: field_identifier, regex: '^Errorf$' }
```

Verification

Fixture	Expected	Result
Local: 2 `fmt.Errorf(...)` calls	2 hits	✅ 2
bborbe/maintainer#2 `Boom` func	1 hit	✅ 1
`string(b)` type conversion	0 hits	✅ 0

Scenario 004's `findings_count` floor lifted ≥4 → ≥5 in the same commit (the documented baseline assumption is now satisfied).

Test plan

• `make precommit` clean (131 rules, 28 mechanical YAMLs, no drift)
• Fixture smoke: 2 + 1 + 0 (positive + perpetual-test-PR + negative)
• Scenario 004 contract updated to ≥5 floor + rationale removed
• Inline rationale in YAML comment block (Gotcha cross-references [[ast-grep - Structural Code Search]] Gotchas)
• Bot review

[ben-s-pull-request-reviewer]

Reviewed by ben-s-pull-request-reviewer[bot] — no concerns flagged.

[ben-s-pull-request-reviewer]

PR Review: fix/no-fmt-errorf-call-expression → master

Scope

3 files changed — all directly related to the bugfix contract.

---

Automated Checks

JSON syntax validation — all three files pass:

• rules/index.json — valid
• .claude-plugin/plugin.json — valid
• .claude-plugin/marketplace.json — valid

make precommit — skipped (requires approval; make touches build tooling). The individual JSON lint checks above cover the precommit criteria relevant to this diff.

---

Change Analysis

1. rules/go/no-fmt-errorf.yml (core fix)

Aspect	Assessment
Root cause diagnosis	Correct. tree-sitter Go grammar parses bare fmt.Errorf($A) at pattern-compile time as type_conversion_expression (ambiguous with Type(arg) cast), not call_expression. Without surrounding expression context there is no disambiguation, so the pattern matched nothing in real code.
Fix strategy	Correct. Structural rule: kind: call_expression + has.selector_expression with operand: identifier[/^fmt$/] + field: field_identifier[/^Errorf$/]. This forces tree-sitter to interpret the node in call context, not cast context.
Ignores	Unchanged — main.go, *_test.go, vendor/**, mocks/** correctly excluded.
Rule ID alignment	go-errors/no-fmt-errorf exists in rules/index.json with owner: go-error-assistant, level: MUST, enforcement: rules/go/no-fmt-errorf.yml — consistent.
Doc reference	docs/go-error-wrapping-guide.md has ### RULE go-errors/no-fmt-errorf (MUST) at line 242 — consistent.

Finding: ✅ No issues.

2. scenarios/004-findings-exist-path.md

Aspect	Assessment
Narrative update	Line 19: Added "All 5 violations now fire after…" clause — accurately reflects the fix.
Expected count	Line 42: findings_count floor lifted from ≥ 4 to ≥ 5. Correct per the fixture (pkg/scenarios-test-fixture/violations.go in bborbe/maintainer#2 contains exactly 5 deliberate violations).
Removed stale caveat	Previous text noting the fmt.Errorf violation didn't fire has been replaced with the corrected expectation — no longer misleading.

Finding: ✅ No issues.

3. CHANGELOG.md

Aspect	Assessment
Location	New ## Unreleased section at top — correct changelog ordering.
Entry content	Names the specific YAML, explains the root cause (tree-sitter ambiguity), describes the fix (structural rule), references the verification fixture and scenario. Concise but complete.
Version strings	No version bump — correctly leaves v0.15.0 as the last released version.

Finding: ✅ No issues.

---

Must Fix (Critical)

None.

Should Fix (Important)

None.

Nice to Have (Optional)

None.

---

Concerns from PR Plan

Concern	Status
correctness: Rule silently matching zero findings since ship	Addressed — structural rewrite correctly targets call_expression with selector_expression on fmt.Errorf
tests: Scenario 004 findings floor lifted ≥4 → ≥5	Addressed — scenario doc updated; floor now ≥5 consistent with 5 actual violations in fixture

---

Next Steps

No additional work needed. The changes are minimal, targeted, and correct.

---

{
  "verdict": "approve",
  "summary": "Bugfix correctly rewrites the `go-errors/no-fmt-errorf` ast-grep rule as a structural pattern that resolves the tree-sitter Go grammar ambiguity causing zero findings since ship. Scenario 004 doc updated to reflect the fix, and CHANGELOG entry is accurate. No issues identified.",
  "comments": [],
  "concerns_addressed": [
    "correctness: structural rule fix in rules/go/no-fmt-errorf.yml bypasses tree-sitter type_conversion_expression ambiguity",
    "tests: scenario 004 findings_count floor lifted ≥4 → ≥5 in scenarios/004-findings-exist-path.md"
  ]
}