# GB5 Backend Coding Standards — Enforcement Checklist

Status: **Draft v1 — for review/fine-tuning before any tooling is built.**

This is the concrete, enforceable rule list derived from `CLAUDE.md`, plus the violations already
being found in review. Each rule has an ID (used later to map 1:1 to an analyzer/`.editorconfig`
rule), a target severity for when enforcement ships, and a short rationale. `CLAUDE.md` stays the
narrative reference — this doc is the checklist that tooling gets built against.

Severity legend:
- **Error (day one)** — mechanical, low false-positive risk, ships as build-breaking as soon as
  the fix-up pass for that rule is done.
- **Warning (burn-in)** — real judgment calls exist; stays a warning until a full-repo triage
  confirms the false-positive rate is acceptable, then promotes to error.

---

## 1. Async / Threading

| ID | Rule | Severity | Notes |
|----|------|----------|-------|
| BE-001 | No `.Result`, `.Wait()`, `.GetAwaiter().GetResult()`, or `Thread.Sleep` inside an `async` method | Error (day one) | Covered by `Meziantou.Analyzer` MA0045 — no custom analyzer needed |
| BE-002 | No `async void` — always `async Task` | Error (day one) | Compiler-visible pattern, near-zero false-positive risk |
| BE-003 | `ConfigureAwait(false)` on every `await` in BLL/DAL (non-ASP.NET context) | Warning (burn-in) | High volume; needs a repo-wide sweep before promoting |
| BE-004 | No `Task.Run` wrapping an already-async call | Warning (burn-in) | Less common but has appeared in review |

## 2. i18n / User-Facing Messages

| ID | Rule | Severity | Notes |
|----|------|----------|-------|
| BE-010 | BLL methods must return `SuccessResponse`/`ErrorResponse` resource members, never hardcoded English string literals, for user-facing messages | Warning (burn-in) → Error | Custom analyzer (`GB5001`); narrow trigger (return-shape + `Message`-property-shape) to avoid flagging business-data string fields |

## 3. Entity Save Pipeline

| ID | Rule | Severity | Notes |
|----|------|----------|-------|
| BE-020 | Any BLL method that persists a primary business entity must route through `ExecuteSaveAsync` (`BaseEntityAppService<TDto>`), not a hand-rolled `BeginTransactionAsync`/commit/rollback | Warning (indefinite) | Custom analyzer (`GB5003`); architectural-intent call, not mechanical — needs `[ManualTransactionJustified("reason")]` escape hatch for genuine multi-entity/composite exceptions. ~70 files currently need triage. `CLAUDE.md` now documents the corrected pattern. |

## 4. Data Access (Dapper)

| ID | Rule | Severity | Notes |
|----|------|----------|-------|
| BE-030 | No string-concatenated or interpolated SQL — parameters only | Error (day one) | Security-critical; should already be near-zero occurrence, verify with a repo sweep before flipping to error |
| BE-031 | All SQL lives in `*QB.cs` constants — never inline in BLL/DAL | Warning (burn-in) | Structural convention; needs a sweep to confirm compliance level first |
| BE-032 | Every query touching tenant data includes a `ClientId`/`DatabaseName`/`TenantId` filter | Warning (burn-in) → Error | Security-critical but needs a real analyzer (SQL-aware) to check reliably — flagged as a **Phase 4 candidate**, not building custom tooling for it in the first pass; until then, rely on manual review checklist |
| BE-033 | No `QuerySingleOrDefaultAsync` (doesn't exist on `IQueryExecutor`) — use `QueryAsync<T>().FirstOrDefault()` | Error (day one) | Compile-time catchable if someone tries it; otherwise a naming-convention analyzer check |
| BE-034 | Use `StreamAsync<T>()` for queries potentially returning >5,000 rows; `BulkInsertAsync<T>()` for >100-row batch inserts, never a looped `ExecuteAsync` | Warning (burn-in) | Hard to detect mechanically (needs row-count knowledge); mostly a code-review checklist item, not a lint rule, for now |

## 5. Naming Conventions

| ID | Rule | Severity | Notes |
|----|------|----------|-------|
| BE-040 | Interfaces prefixed `I` + PascalCase | Error (day one) | `.editorconfig` `dotnet_naming_rule`, promote default `IDE1006` severity |
| BE-041 | Classes/types PascalCase | Error (day one) | `.editorconfig` naming rule |
| BE-042 | Query builder classes suffixed `QB`; request DTOs suffixed `Parameters`; cross-layer DTOs suffixed `DTO` | Not enforced via `.editorconfig` (see rationale) | Can't be expressed as an `.editorconfig` naming symbol without also forcing every unrelated class to match the suffix. Custom-analyzer candidate — Phase 4, not committed to this pass |
| BE-043 | 4-space indentation; `using` directives outside namespace; `System.*` usings first; no duplicate `using`s | Error (day one) | Standard `.editorconfig` formatting rules, `EnforceCodeStyleInBuild=true` |

## 6. Caching

| ID | Rule | Severity | Notes |
|----|------|----------|-------|
| BE-050 | `GetCacheKey()` returns `null` for every mutation endpoint (POST/PUT/DELETE) | Warning (burn-in) → Error | Custom analyzer candidate: flag any `GetCacheKey` override in a `Save*`/`Update*`/`Delete*`-named endpoint class that doesn't return a bare `null` |
| BE-051 | Financial/ledger data never cached — always `CacheKeyLevel.NOT_REQUIRED` | Warning (burn-in) | Needs domain knowledge of which endpoints are "financial" — likely a manual checklist item rather than a lint rule initially |

## 7. Error Handling / Observability

| ID | Rule | Severity | Notes |
|----|------|----------|-------|
| BE-060 | Never swallow an exception silently (`catch (Exception) { return ...; }` with no logging) | Warning (burn-in) → Error | Custom analyzer candidate — flag empty or log-free catch blocks in BLL/DAL |
| BE-061 | Structured logging only — never string interpolation in `_logger.Log*` calls | Error (day one) | Covered by built-in `CA2254` (NetAnalyzers) — just needs `AnalysisLevel=latest` |
| BE-062 | `GB5Trace.Step`/`MarkFailed` present in every BLL save method per the documented pattern | Warning (burn-in) | High-effort custom analyzer for a process convention, not a bug risk — deprioritized behind BE-010/BE-020 |

## 8. DI / Memory

| ID | Rule | Severity | Notes |
|----|------|----------|-------|
| BE-070 | No `IDbConnection` stored as an instance field | Warning (burn-in) → Error | Custom analyzer candidate, low volume expected |
| BE-071 | No `new HttpClient()` — always `IHttpClientFactory` | Error (day one) | Covered by NetAnalyzers `CA2000`/community rule sets — verify coverage during Phase 0 |
| BE-072 | No unbounded static `Dictionary`/collection fields | Warning (burn-in) | Hard to detect reliably; manual-review checklist item for now |

## 9. SignalR (only for Hub-containing modules)

| ID | Rule | Severity | Notes |
|----|------|----------|-------|
| BE-080 | Hub classes always `Hub<TClient>`, never untyped `Hub` | Error (day one) | Compile-time-adjacent, low false-positive risk |
| BE-081 | `LoginDTO` reconstructed from `Context.GetHttpContext()`, never accepted as a Hub method parameter | Warning (burn-in) | Custom analyzer candidate — small blast radius (few Hub modules exist today) |
| BE-082 | `OnDisconnectedAsync` always mirrors `OnConnectedAsync`'s group add with a group remove | Warning (burn-in) | Same — low volume, manual review acceptable initially |

---

## Not building custom tooling for (explicitly out of scope this round)

- `Nullable` reference-type strictness (`annotations` → `enable`) — an order of magnitude bigger
  backlog (35k–70k+ warnings) than everything else combined; tracked separately, opt-in per module
  via `GB5NullableStrict`.
- XML doc-comment mismatches (`CS157x`/`CS1570`/`CS1587`) — no safe auto-fix exists; team decision
  pending on whether to invest in a codemod or permanently exclude this category from the gate.
- Tenant-filter-missing-WHERE-clause detection, `*QB`/`*Parameters`/`*DTO` suffix enforcement —
  both need a real custom analyzer with SQL/AST awareness; called out as Phase 4 candidates.

## Next steps

1. Review this list — adjust severities, add/remove rules, confirm the "not building" list is
   acceptable for now.
2. Once confirmed, land the baseline tooling (`.editorconfig`, `Directory.Build.props` analyzer
   packages) for the Error-day-one rules first — these are safe to turn on immediately.
3. Build the three custom analyzers (BE-010/GB5001, BE-020/GB5003, plus adopt MA0045 for BE-001)
   and start the fix-up triage described in the implementation plan.