Add fix guidance to rule READMEs (#23)

Author: benvinegar

src/rules/async-noise/README.md

@@ -41,6 +41,27 @@ async function getJson(url: string) {
 }
 ```
 
+## How to fix / do this better
+
+Prefer one of these instead:
+
+- remove `async` entirely when the function is just forwarding a promise
+- remove redundant `await` when you are immediately returning the awaited value
+- keep the wrapper only if it adds real behavior such as validation, normalization, retries, metrics, or error context
+
+```ts
+function getUser(id: string) {
+  return fetchUser(id);
+}
+
+async function loadUser(id: string) {
+  const user = await fetchUser(id);
+  return normalizeUser(user);
+}
+```
+
+The goal is not "never use async". It is to avoid wrapper ceremony that makes the call graph larger without making behavior clearer.
+
 ## Scoring
 
 Redundant `return await` sites add `1.5` each.

src/rules/barrel-density/README.md

@@ -36,6 +36,23 @@ export function createStore() {
 export { type Store } from "./types";
 ```
 
+## How to fix / do this better
+
+Prefer barrels only when they improve discoverability without hiding module boundaries.
+
+Better options:
+
+- keep a barrel small and intentional
+- export a stable public surface from one place, but avoid creating layers of barrel-to-barrel indirection
+- import directly from the implementation module when a barrel adds little value
+
+```ts
+export { createStore } from "./store";
+export { type Store } from "./types";
+```
+
+If a file is just a wide list of re-exports, ask whether it is actually helping API design or only adding another place to chase symbols through.
+
 ## Scoring
 
 The score starts at `1` and adds `0.5` per re-export statement, capped at `3`.

src/rules/directory-fanout-hotspot/README.md

@@ -47,6 +47,30 @@ src/icons/
 
 Asset-like buckets and test-matrix directories are intentionally suppressed because wide directory shapes are expected there.
 
+## How to fix / do this better
+
+A wide directory is usually a sign that one of these is missing:
+
+- a stronger domain split
+- a deeper subdirectory boundary
+- a more cohesive module with fewer one-file-per-concept fragments
+
+Better patterns:
+
+- group related files into subdomains once a folder becomes a grab bag
+- merge ultra-thin files when the split adds naming overhead but not conceptual clarity
+- separate generated output from hand-written source when possible
+
+```text
+src/
+└── billing/
+    ├── invoices/
+    ├── subscriptions/
+    └── shared/
+```
+
+The goal is not tiny directories everywhere. It is to avoid a single hotspot folder becoming the dumping ground for too many loosely related files.
+
 ## Scoring
 
 The rule starts at `2` and adds a bounded amount based on how far the directory is above the computed threshold.

src/rules/duplicate-function-signatures/README.md

@@ -50,6 +50,26 @@ export function getUser(id: string) {
 
 Pass-through wrappers are excluded, and a duplicate that only appears in 2 files is below the reporting threshold.
 
+## How to fix / do this better
+
+When the same helper shape appears across multiple files, prefer one of these:
+
+- extract the shared logic into a single reusable helper
+- create a small configurable normalizer instead of copy-pasting near-identical functions
+- keep duplication only when the domain concepts are truly diverging and deserve separate behavior
+
+```ts
+function normalizePersonLike(input: { name?: string; email?: string; active?: boolean }) {
+  return {
+    name: input.name?.trim() ?? "",
+    email: input.email?.toLowerCase() ?? "",
+    active: Boolean(input.active),
+  };
+}
+```
+
+The point is not to eliminate all repetition. It is to avoid silent copy-paste drift when several files are maintaining the same logic independently.
+
 ## Scoring
 
 Each duplicate cluster adds `1.25 + 0.5 * (fileCount - 3)` for the current file, capped at `6`.

src/rules/duplicate-mock-setup/README.md

@@ -39,6 +39,24 @@ vi.clearAllMocks();
 
 Generic mock declarations and cleanup-only statements do not contribute to this rule.
 
+## How to fix / do this better
+
+When the same mock setup keeps reappearing, prefer shared test helpers over repeating the setup inline.
+
+Better options:
+
+- move repeated mock wiring into a factory or fixture helper
+- centralize common setup in `beforeEach` when it is truly shared
+- expose small scenario builders so tests vary only the interesting values
+
+```ts
+function mockUserFetch(overrides: Partial<User> = {}) {
+  vi.mocked(api.fetchUser).mockResolvedValue({ id: 1, name: "Ada", ...overrides });
+}
+```
+
+That keeps test intent focused on what changes per case instead of duplicating the same mock plumbing in every file.
+
 ## Scoring
 
 Each duplicate setup cluster adds `1 + 0.5 * (fileCount - 2)` for the current file, capped at `5`.

src/rules/empty-catch/README.md

@@ -44,6 +44,27 @@ export function loadTheme() {
 }
 ```
 
+## How to fix / do this better
+
+An empty catch should usually become one of these instead:
+
+- rethrow the error
+- return a deliberate typed fallback with a comment explaining the boundary behavior
+- log meaningful context and then rethrow
+- validate earlier so the exceptional path is narrower and more intentional
+
+```ts
+export function parseConfig(raw: string) {
+  try {
+    return JSON.parse(raw);
+  } catch (error) {
+    throw new Error("Invalid config JSON", { cause: error });
+  }
+}
+```
+
+If swallowing the error is truly intentional, document why the fallback is safe and keep the scope local.
+
 ## Scoring
 
 Each flagged catch uses the shared try/catch scoring helper, then the file total is capped at `8`.

src/rules/error-obscuring/README.md

@@ -50,6 +50,28 @@ export function readConfig(raw: string) {
 }
 ```
 
+## How to fix / do this better
+
+Prefer preserving failure meaning instead of replacing it with a cheap fallback.
+
+Better patterns:
+
+- rethrow the original error
+- wrap with context while preserving `cause`
+- return a deliberate result type that makes the failure explicit instead of pretending the operation succeeded
+
+```ts
+export function loadProfile(id: string) {
+  try {
+    return fetchProfile(id);
+  } catch (error) {
+    throw new Error(`Failed to load profile ${id}`, { cause: error });
+  }
+}
+```
+
+If you truly need a fallback value, keep it narrow, document why it is safe, and avoid erasing the original failure in code paths that still need diagnosis.
+
 ## Scoring
 
 Each flagged catch uses the shared try/catch scoring helper, then the file total is capped at `8`.

src/rules/error-swallowing/README.md

@@ -39,6 +39,28 @@ export async function syncUser(id: string) {
 }
 ```
 
+## How to fix / do this better
+
+Logging is not a substitute for control flow.
+If the caller still needs to know the operation failed, prefer one of these:
+
+- log and rethrow
+- return an explicit result type such as `{ ok: false, error }`
+- handle the failure completely at this layer only when you can prove continuing is safe
+
+```ts
+export async function syncUser(id: string) {
+  try {
+    await pushUser(id);
+  } catch (error) {
+    logger.error({ error, id }, "failed to sync user");
+    throw error;
+  }
+}
+```
+
+The key is to make failure visible in the API contract instead of only visible in logs.
+
 ## Scoring
 
 Each flagged catch uses the shared try/catch scoring helper, then the file total is capped at `8`.

src/rules/generic-record-casts/README.md

@@ -40,6 +40,32 @@ const token = value as { token: string };
 const metadata = input as Map<string, string>;
 ```
 
+## How to fix / do this better
+
+Treat unknown input as unknown for longer, then validate or narrow it at the boundary.
+
+Better options:
+
+- parse into a real domain type with a schema or decoder
+- keep the value as `unknown` until you prove the fields you need
+- use a very local cast only when you immediately narrow and contain it
+
+```ts
+const input: unknown = JSON.parse(raw);
+const parsed = UserConfigSchema.parse(input);
+```
+
+Or, without a schema library:
+
+```ts
+const input: unknown = JSON.parse(raw);
+if (!isUserConfig(input)) {
+  throw new Error("Invalid user config");
+}
+```
+
+The goal is to avoid turning uncertain external data into a roaming generic object bag that downstream code has to keep guessing about.
+
 ## Scoring
 
 Each generic record cast adds `2` points.

src/rules/generic-status-envelopes/README.md

@@ -35,6 +35,25 @@ return { success: true, user };
 return { error: "missing" };
 ```
 
+## How to fix / do this better
+
+Prefer API shapes that express the actual domain outcome instead of wrapping everything in a shallow boolean envelope.
+
+Better options:
+
+- return the domain object directly on success
+- use typed result variants when callers really need success/failure branching
+- model specific failure cases instead of pushing everything into generic `message` / `error` strings
+
+```ts
+type CreateRepoResult =
+  | { kind: "created"; repository: Repository }
+  | { kind: "forbidden" }
+  | { kind: "conflict"; reason: string };
+```
+
+A small `{ ok, data }` wrapper is sometimes fine, but if it becomes the default shape for every operation it usually means the API is describing transport status rather than domain meaning.
+
 ## Scoring
 
 Each generic status envelope adds `2` points.