diff --git a/code-languages/csharp-dotnet.md b/code-languages/csharp-dotnet.md index 8e8474a..a935f83 100644 --- a/code-languages/csharp-dotnet.md +++ b/code-languages/csharp-dotnet.md @@ -37,8 +37,11 @@ dotnet build MySolution.sln -warnaserror - **Nullable warnings are real bugs.** Never silence one with `!` (null-forgiving) — narrow the type or fix the contract. `!` is an assertion, and an assertion you cannot prove is a future `NullReferenceException`. -- Never blanket-disable an analyzer. If a rule is genuinely wrong for the codebase, configure - it once in `.editorconfig` with a comment. +- Never blanket-disable an analyzer. When a rule is genuinely wrong for a piece of code, + scope the suppression to it — `#pragma warning disable CA1822` around the smallest region + that needs it (with a matching `restore`), or a targeted `[SuppressMessage]` attribute — + and always with a `Justification` / comment saying why. An `.editorconfig` entry turns the + rule off for every file it covers; use it only when the rule is wrong codebase-wide. --- diff --git a/code-languages/js-typescript.md b/code-languages/js-typescript.md index 79eeffa..922c889 100644 --- a/code-languages/js-typescript.md +++ b/code-languages/js-typescript.md @@ -43,13 +43,14 @@ into a project that has neither. } ``` -**`tsc --noEmit` must pass with 0 errors**, and it belongs in the build gate alongside lint -and tests — not as a thing someone runs occasionally in their editor. +**`tsc --noEmit` must pass with zero errors *and* zero warnings**, and it belongs in the build +gate alongside lint and tests — not as a thing someone runs occasionally in their editor. > [!NOTE] > Some established codebases deliberately relax `noImplicitAny` or `noUncheckedIndexedAccess` -> because their idioms depend on it. Respect the project's `tsconfig.json`; the table above is -> the default for **new** projects. +> because their idioms depend on it. When **contributing upstream**, respect the project's +> `tsconfig.json` as-is. In **our own repos and non-upstreamed forks**, the settings above are +> the target — tightening them is fair game (as its own change, not smuggled into a feature). --- diff --git a/code-languages/rust.md b/code-languages/rust.md index 39dd9a8..a941b5f 100644 --- a/code-languages/rust.md +++ b/code-languages/rust.md @@ -61,8 +61,17 @@ workspace = true > carries a second copy that drifts — which is how a binary once kept reading its own stale > allows while the workspace's were being actively edited. -- **Never `#[allow]` on a whim.** If a lint is genuinely wrong for the codebase, allow it - **workspace-wide** with a comment explaining why, so the decision is visible. +- **Never `#[allow]` on a whim** — but when a lint is genuinely wrong for a piece of code, + **allow it at the narrowest scope that covers it** (the expression, the function, the + module) with a comment saying why: + ```rust + // The FFI callback signature is fixed by the C API; we cannot take fewer arguments. + #[allow(clippy::too_many_arguments)] + extern "C" fn on_event(/* … */) { } + ``` +- A **workspace-wide** allow in `[workspace.lints]` is for the rare case where the lint is + wrong for the *entire* codebase — not a shortcut for one stubborn call site. It silences + the rule in code nobody has written yet. - **Re-test every `#[allow]` periodically.** If removing one produces no warning, delete it. - `too_many_lines` is the mechanical enforcement of the 100-line function limit. A function that trips it wants extracting, not an allow. diff --git a/coding-dev.md b/coding-dev.md index e26572e..9579e65 100644 --- a/coding-dev.md +++ b/coding-dev.md @@ -14,9 +14,17 @@ These are the defaults for **all** code work, in any language. They are preferen reasons attached, not style trivia — most exist because ignoring them caused a real problem. > [!IMPORTANT] -> **A project's own rules win.** If a repository has an `AGENTS.md`, `CLAUDE.md`, `AGENT.md`, -> `.rules`, or `CONTRIBUTING.md`, read it first — it is more specific than this document and -> overrides it on conflict. This file is the default for everything those files do not say. +> **Whose rules win depends on where the code is going.** Always read a repository's +> `AGENTS.md`, `CLAUDE.md`, `AGENT.md`, `.rules`, or `CONTRIBUTING.md` first — but what you do +> on a conflict differs: +> +> | Situation | Precedence | +> | :--- | :--- | +> | **Contributing upstream** — a PR, a patch, or anything landing in someone else's repo | **The project's rules win.** Match their conventions exactly, even where they contradict this document. A PR that fights the house style gets rejected on style instead of reviewed on substance. | +> | **Local / private work** — our own repos, forks we don't upstream, private branches, local patches | **These rules win.** A project's conventions are a data point, not an obligation. Adopt the ones that are load-bearing (their build gate, their architecture contracts, anything that breaks if ignored) and apply our own preferences for the rest. | +> +> When in doubt about which mode you are in, **ask** — it changes the answer on strictness, +> file size, typing, and test policy all at once. Per-language specifics live in [`code-languages/`](./code-languages/): @@ -39,10 +47,15 @@ Strict mode is the starting point, not an aspiration to migrate toward later. - Warnings are errors in CI and in the build gate (`-warnaserror`, `-D warnings`, `--strict`). - Enable the pedantic/extra lint tiers, not just the defaults. -- Type checking must pass with **zero** errors — `tsc --noEmit`, `mypy`, `cargo check` alike. +- Type checking must pass with **zero errors or warnings** — `tsc --noEmit`, `mypy`, + `cargo check` alike. - Never weaken a check to make a build pass. Fix the failing step. If a check is genuinely - wrong for the codebase, disable it **project-wide in one place with a comment explaining - why**, so the decision is visible and reviewable — never with a scattered inline suppression. + wrong for a specific piece of code, **suppress it at the narrowest scope that covers the + problem — the line, the function, or the module — with a comment explaining why.** A + project-wide disable is a bigger hammer than it looks: it silences the rule everywhere, + including in code written later by someone who never saw the reason. Reach for it only when + the rule is genuinely wrong for the *whole* codebase, never as a shortcut for a stubborn + call site. ### Suppressions are a last resort, and they expire @@ -50,9 +63,14 @@ Strict mode is the starting point, not an aspiration to migrate toward later. - When you do suppress one, **say so explicitly in your report to the user** — name the warning, where it is, and why it could not be fixed. A silent suppression is the failure mode this rule exists to prevent. -- **Re-test suppressions periodically.** Removing an `#[allow]` / `// eslint-disable` / - `#pragma warning disable` that no longer suppresses anything should produce no warning — - if it doesn't, delete it. Stale suppressions accumulate and hide real regressions. +- **Every suppression carries a comment saying why.** A bare `#[allow]` / `// eslint-disable` + / `# noqa` with no reason is indistinguishable from someone giving up, and nobody will ever + dare remove it. +- **Name the specific rule**, never a blanket suppression: `# noqa: S603`, not `# noqa`; + `//nolint:errcheck`, not `//nolint`. +- **Re-test suppressions periodically.** Removing one that no longer suppresses anything + should produce no warning — if it doesn't, delete it. Stale suppressions accumulate and + hide real regressions. ---