Design rules for any-language code, for people and for coding agents. The same rules
drive the linter-driven-development plugin; this file is the version you can read in ten minutes
and keep open while you work, with or without the plugin. Import it from your
CLAUDE.md or AGENTS.md, or read it once before your first PR.
Before a rule fires, ask the question behind it:
- Tell, don't ask. What will the caller do with the value it is requesting — and does that decision belong on the type that owns the value?
- Make illegal states unrepresentable. Can this type hold a value its methods would have to defend against? Delete the possibility, not the symptom.
- Parse, don't validate. Does this check produce a more-typed value (a parse function that returns the value or fails), or just a boolean the next caller must remember? Validation that returns proof is parsing; validation that returns advice is a latent re-check.
- Every indirection must earn its keep. What does this indirection own — a validation, a decision, a second production implementation, a deleted duplication, a real race, a real escaping alias? If the answer is nothing, it is ceremony: delete it.
- Duplication is far cheaper than the wrong abstraction. Is this extraction earning its indirection today, with the callers in hand — or is it a bet on imagined futures?
- Three strikes and you refactor. How many real occurrences exist right now? One is an instance, two is a coincidence, three is a pattern.
- If a test is hard to write, the design is wrong. What is the test's pain telling you? Huge fixtures → the unit is too big; global mutation → a seam is missing; mocks everywhere → the boundaries are wrong. Never silence test pain with test machinery.
- Clear is better than clever. Will a reader get this in 10–15 seconds? Cleverness is a cost paid by every future reader.
- When in Rome, code as the Romans do. Does this diff arrive in the host repo's existing style, or does it import mine? A new test mechanism, dependency, framework, or convention — however good — is an adoption decision that belongs to the repo owner, not a side effect of a feature PR. Strong opinions travel as a discussion or a separate PR, never as a bundled surprise. Reviewers forgive imperfect code in the house style far more readily than perfect code in a foreign one.
Each rule is stated once, with one any-language example and the names of the moves that fix it. The moves are shared vocabulary across every language these rules are rendered for: reviewers cite them by name. Where a rule cites R1's scorecard, the short form is: a type earns its keep by owning validation, behavior or an invariant, and scores zero when its only method unwraps the primitive; the full scorecard lives in the plugin's R1 rule.
Domain concepts must not travel as raw strings, numbers, booleans or lists. When a primitive carries validation rules, behavior, or a domain name, it becomes a type with a validating constructor and named methods. The inverse binds equally: a wrapper that adds no validation, no logic, and no invariant is over-abstraction — score before you wrap.
# ❌ the rule lives at the call site, twice, and 0 means "none"
managementPort(service):
for each port in service.ports:
if port.name == "weka-api" and port.number > 0 and port.number <= 65535:
return port.number
for each port in service.ports:
if port.number > 0 and port.number <= 65535:
return port.number
return 0
# ✅ a Port cannot exist out of range; "first valid" collapses to "first"
parsePort(name, number): # the Port, or a failure naming the port and the range
if number <= 0 or number > 65535: fail "port <name>: <number> out of range 1-65535"
return Port(name, number)
Ports.firstNamed(name): # the Port, or absent
Ports.first(): # the Port, or absent
# ❌ a shape crosses the boundary; the caller pokes the method out of it
bearerToken(headers): # headers is a map from string to string
for each name, value in headers:
if name == "Authorization" and value starts with "Bearer ":
return value without the "Bearer " prefix
return absent
# ✅ the container has a name, and the loop is its method
Headers(raw): # copies the map in; any map is a valid one
Headers.authToken(): # the token, or absent
Spelling: "the value or a failure" is an error result, an exception or a result type; "the value or absent" is an optional type, a declared nullable or a found-flag pair, whichever the repository's language and its own code already use. A
0,""ornullreturned where the signature promises aPortis a sentinel in every language. A wrapper whose only method unwraps the integer scores zero on the scorecard: keep the integer. A map or list of primitives crossing a function boundary is a shape, not a concept: read what the receivers do with it, and name the type. A container inside a container is always a missing named type.
Moves: Replace Primitive with Domain Type · Extract Collection Type · Replace Sentinel with Declared Absence · Name enum strings · Introduce Parameter Object · Name the Container
A type validates its own invariants in its constructor — the only way to obtain a value — and every method thereafter trusts the receiver. Validation ownership never sits upstream: a type that relies on callers to have validated for it is not self-validating, whatever its fields look like.
# ❌ every method re-asks; a literal Retention(-1) is legal
Retention
days # public: any value gets in
Retention.cutoff(now):
if self.days <= 0 or self.days > 365: self.days = 30
return now minus self.days days
# ✅ one constructor, hidden fields, methods trust the receiver
Retention
days # hidden: no literal builds a Retention around the check
parseRetention(days):
if days <= 0 or days > 365: fail "retention <days>: want 1-365"
return Retention(days)
Retention.cutoff(now):
return now minus self.days days
Spelling: hidden fields are the language's visibility form: a private field, a leading underscore, a module that exports the constructor and not the fields. How the constructor fails, an error result or an exception, follows the repository. The optional-collaborator stance is opinionated in every language: a missing destination is a Null Object passed by name, a real implementation that discards, never a
nullthe methods guard, sonewReporter(discardSink())never branches on it.
Moves: Add validating constructor · Hoist method checks into the constructor · Introduce Null Object · Delete re-validation of composed types · Separate Failure from Absence
A top-level function reads like a story: every step is a named call at the same conceptual level, and the whole flow is graspable at a glance. Method calls never mix with string/index manipulation in the same body. A comment that names a block of code is a function name waiting to be extracted.
# ❌ the comment is the function name you did not write
Handler.heartbeat(raw):
# parse and validate
hb = decode(raw) # fails → return the failure
if hb.deviceID is empty: fail noDevice
# already seen? skip
if hb.deviceID in self.seen: return
# ... forty more lines
# ✅ three named steps at one level; each is a leaf you can test with literals
Handler.heartbeat(raw):
hb = device.parseHeartbeat(raw) # fails → return the failure
if self.seen.has(hb.id()): return
return self.fleet.record(hb)
Spelling: the extracted step is a function or a method, whichever the repository uses for a named step at that level. In every language a name reveals its side effect:
align,upsertandsetmutate;parse,validateandisnever do. AparseIP4that writes a field is a storifying bug even when the flow reads well.
Moves: Extract Function named after the comment · Extract Leaf Type · Replace Nesting with Early Returns · Split Phase · Honest Rename
Every extraction raises a second question: where does the helper live? The answer is decided by two axes — juiciness (the scorecard in R1; cite it, never re-derive it) and scope (feature-specific versus domain-generic). Three rungs: internal in place, feature sub-package, shared domain package. Never test privates, and never export a helper into its parent package just so a test can reach it.
# ❌ made public by the feature package only so the test can reach it
public parseCIDRList(raw) # one caller, in this package
# ✅ rung 1: hidden where it is used, tested through the public API that calls it
parseCIDRList(raw) # private to the feature
# ✅ rung 3: networking vocabulary with three callers → its own package, named for the domain
networking.parsePrefixes(raw) # public, with the Prefixes type beside it
Spelling: rung 1 is the language's private form: a lower-case or underscored name, a symbol the module does not export, a file-local declaration. Rung 2 is a sub-package, sub-module or namespace under the feature; rung 3 a shared package under the source root. A test that has to reach a private name, through a friend module, a reflection helper or a test-only export, is a placement signal, never a reason to add the seam. The
"weka-api"port name is feature policy and stays in the feature;Port,PortsandfirstNamedare networking vocabulary and move.
Moves: Demote (rung 1) · Promote to feature sub-package (rung 2) · Promote to domain package (rung 3) · Split policy from vocabulary during promotion · Move Method to the Envied Type
Group code by feature and role, not by technical layer: bad — domain/rotator,
services/rotator; good — rotator/parser.<ext>, rotator/handler.<ext>. All code for a
feature lives in one package, internally separated by role within it. Package names
are flatcase domain vocabulary — never a layer or role name.
❌ by layer ✅ by feature, roles inside
src/models/rotator.<ext> src/rotator/rotator.<ext> # domain type
src/services/rotator.<ext> src/rotator/parser.<ext> # role: parsing
src/repositories/rotator.<ext> src/rotator/handler.<ext> # role: HTTP
src/handlers/rotator.<ext> src/rotator/repository.<ext> # role: persistence
Spelling: the directory is a package, module, crate or namespace, whatever the language calls one unit of code with its own public surface. Its name is the feature noun,
rotator, neverservicesormodels;utils,commonandhelpersname no vocabulary at all, so the first function that lands there has no owner and the next lands beside it. Role names live in the file names inside the slice, and a type with logic gets its own file named after the type.
Moves: Slice out a feature · Rename layer files by role during the move · Split a generic package by owner
A seam that exists only so a test can substitute a double — an interface with one production implementer, a patched attribute, an injection parameter no production caller varies — is deleted; depend on the concrete collaborator. Don't create interfaces until you need them; a test fake is not a need. An interface is justified only by a real second production implementation or a verified import cycle.
# ❌ one production implementer; the abstraction exists for the fake in the test
DeviceStore # an interface / protocol / trait with one method
get(id)
newFleet(store: DeviceStore, config)
FakeDeviceStore # the ONLY other implementer, in the test file
get(id): return self.device
# ✅ depend on the concrete type; the test wires a real store over an embedded database
newFleet(store: sqlite.DeviceStore, config)
testRecord():
fleet = newFleet(sqlite.openDeviceStore(tempDir), config)
Spelling: the seam is an interface, a protocol, a trait, an abstract base class or, in a language that patches, a
patch("service.Store")on a concrete class with no declaration to search for; the one-implementer test transfers unchanged. An earned interface stays small and cohesive. A hand-written stand-in that satisfies a production abstraction only in a test file is a mock, whatever it is called. Faking the true external boundary is fine: the clock, a socket, the environment in an entry-point test.
Moves: Delete the Test Seam · Rewrite the test around real collaborators · Delete the double
Every behavior is tested at the lowest rung of the composition ladder that contains it: rung 0 is pure leaf types (unit tests with literal inputs, 100% coverage, public API only, imported as a consumer would); each rung above adds exactly one real production layer; only the true external boundary is ever faked. Orchestrating types get integration-style tests that cover the seams between their real collaborators — some overlap with leaf coverage is fine; leaf behavior tested only from above is not. On a leaf type, coverage is the floor and the mutation score is the claim: a leaf's tests must fail when its logic is changed, and a mutant that survives them is a missing row or dead logic.
# ❌ one table, a flag, and a branch inside the case
testParsePort():
cases = [("valid", 14000, expectFailure: false), ("zero", 0, expectFailure: true)]
for each case in cases:
result = networking.parsePort("x", case.number)
if case.expectFailure: assert result failed
else: assert result succeeded
# ✅ success and error tables apart, every row a named case, complexity 1 per case
testParsePort_success(): # imported as a consumer would
for each (name, number) in [("plain", 14000), ("edge", 65535)]:
case name:
assert networking.parsePort("x", number) succeeded
testParsePort_error():
for each (name, number) in [("zero", 0), ("above range", 70000)]:
case name:
assert networking.parsePort("x", number) failed
Spelling: the table is the language's parametrized-test form, a list of named cases, a parametrize decorator, a data provider, and every row is registered under its name (a subtest, a parameter id) so a failure names its case. No fixed pause: wait on an event, a channel, a future or a timeout. Orchestrators are tested by wiring their real collaborators over a temp directory, an in-process fake server or an embedded database.
Moves: Move the behavior down a rung · Split Success and Error Tables · Kill the surviving mutant · Replace doubles with real collaborators · Replace sleep with synchronization · Delete private-function tests
Dependencies are passed down from the caller, never reached sideways: no package-level mutable state, no import-time initialization writing state, no singletons fetched from inside business logic, no library code that manufactures its own root cancellation — cancellation flows from caller to callee. Globals are acceptable only at the composition root — the program's entry point, handler setup, application wiring — where they are read once and injected downward.
# ❌ reached sideways from three layers; untestable without the environment
config = loadConfig() # module level: runs at import, read from a leaf
Fleet.record(hb):
if config.readOnly: ...
# ✅ read once at the composition root, pushed down as a value
main():
config = loadConfig()
run(device.newFleet(store, config.fleet))
Spelling: silent everywhere: constants, enums, the module's logger, the error or exception types. Silent only at the composition root, the entry point, handler setup or application wiring: reading the environment, building the framework's app object, filling a registry by hand, starting the event loop. Reported elsewhere: an environment read, a module-level container that functions write into, an import-time or static initializer that writes state, library code that manufactures its own cancellation root. A test that mutates production configuration to run is evidence against the production code, not a fix for the test.
Moves: Extract Clean Island · Push the Global Up One Level · Replace Import-Time Initialization with a Constructor · Pass Cancellation Down
Documentation is a network ranked by the documentation ladder: storified code → doc comments → repo docs → the index, each fact placed at the lowest rung that can carry it, higher rungs summarizing and pointing down, never duplicating. Two invariants hold the network together: reachability (every doc is reachable from the root: CLAUDE.md → index.md → doc — no orphans) and bidirectionality (code points up at its feature doc; docs point down at code via greppable symbols; the index points everywhere).
# ❌ restates the signature; the reader learned nothing
<comment-marker> parsePort parses a port from a name and a number.
parsePort(name, number)
# ✅ says why the type exists and where the long story lives
<comment-marker> parsePort is the only way to obtain a Port, so no downstream code re-checks the
<comment-marker> range: an out-of-range port cannot exist. Selection policy: docs/management-port.md.
parsePort(name, number)
Spelling: the doc comment is the language's documentation form:
///or//line comments, a/** */block, a docstring. A summary line the tooling requires states the contract and is never a restatement finding; the lines after it earn their place by saying why, in one to five lines. Anything longer moves todocs/<feature>.mdwith a line that points there. A comment that restates the code is deleted, and a private symbol carries none by default.
Moves: Push the fact down a rung · Convert WHAT to WHY or delete · Rewire orphan doc · Wire the root · Add missing frontmatter · Update the stale doc with the behavior change
Every concurrent task has an owner and a provable exit path; shared mutable state is owned by one type and guarded where it lives; production code never sleeps to pace or synchronize cancellable work. Concurrency is designed at construction time — who owns the state, who stops the concurrent task — never patched in afterward.
# ❌ no owner, no exit, paced by sleep
spawn:
loop forever:
poll()
sleep(interval)
# ✅ the type that starts it holds what stops it; the wait is cancellable; close joins
startPoller(interval):
poller = Poller(interval)
poller.cancel = a cancellation signal
poller.task = spawn poller.run(poller.cancel)
return poller
Poller.run(cancel):
timer = every(self.interval)
loop:
wait for the first of: cancel is signalled | timer fires
cancelled → return
self.poll(cancel)
Poller.close():
signal self.cancel
join self.task # returns only when run has exited
Spelling: the spawn is a
gostatement, a thread or an async task; the cancellation signal a context, an event or a token; the join a wait group, ajoin()or an awaited handle. A bare sleep in production code becomes a wait on the timer and the cancellation signal; a sleep the runtime cancels for you, asasyncio.sleepdoes, is fine. A lock lives beside the fields it guards, and a guard around fields only one task touches is deleted.
Moves: Inject the Exit Path · Make Concurrent Work Joinable · Extract Synchronized Owner · Replace Sleep with Cancellable Wait · Delete Unearned Guards
A conditional that asks what a value is — a type switch, or a switch/if-chain on a kind/status/mode discriminator — may exist once. The second copy of that discriminator is a missing polymorphic type: the variants want to be implementations of an interface (or entries in a dispatch map), chosen once at the boundary, so downstream code tells the value what to do instead of asking what it is. One well-placed, exhaustive switch is not a defect; a duplicated one always is.
# ❌ the same discriminator in send, validate and retry
send(alert):
switch alert.channel:
"email": ...
"slack": ...
otherwise: fail "unknown channel <channel>"
# ✅ chosen once at the boundary; everything downstream tells
Channel # an interface / protocol / trait
send(alert)
validRecipient(recipient)
retryDelay()
parseChannel(raw): # the ONE place the raw string is inspected
switch raw:
"email": return Email()
"slack": return Slack()
"pagerduty": return PagerDuty()
otherwise: fail "unknown channel <raw>"
Spelling: the kept switch is over a closed set the compiler or the linter can prove exhaustive, an enum, a sealed type, a tagged union, and it is closed by that proof, not by an
otherwisethat returns "unknown kind" from deep inside the logic. Prefer a dispatch table from name to variant first, an interface or trait per variant second; in a language without interfaces, a class per variant with a shared base. A boolean parameter that selects a branch is a Split Flag Argument candidate.
Moves: Replace Duplicated Switch with Interface Dispatch · Replace If-Chain with Strategy Map · Introduce Null Object · Split Flag Argument · Keep the Single Exhaustive Switch
A validated value changes state only through methods that own its invariants — never through leaked internals. Constructors copy the collections they are given; queries return copies (or iterators), not the internal reference; a method is a query or a modifier, not both; and a type with a validating constructor exposes no setter that skips the validation. This rule adapts Fowler's Mutable Data smell family (Refactoring, 2nd ed.: Encapsulate Collection, Separate Query from Modifier, Remove Setting Method, Split Variable) to languages where collections are passed by reference into shared backing storage.
# ❌ the caller's list is now the type's list; a later append bypasses the constructor
newPorts(items): return Ports(items)
Ports.items(): return self.items
# ✅ copy on the way in; hand out a copy or an iterator on the way out
newPorts(items): return Ports(copy of items)
Ports.each(): yield each port in self.items
Spelling: where lists and maps are references into shared storage, a constructor copies what it is given and a query never returns the field itself. Where the language has immutable collections, storing a frozen copy is the same move with the copy made once; freezing the binding is not freezing the value, so a frozen record holding a mutable list is mutable through that list. A mutable default argument, where the language has them, is this rule's most common form.
Moves: Copy on the Way In · Copy on the Way Out / Encapsulate Collection · Separate Query from Modifier · Remove Setting Method · Split Variable
Rules that are not one of the twelve but hold in every language, then the any-language house rules. Same shape as above, with their own numbers so a review can cite them.
A lint-suppression directive is never added on your own: fix the code, and when the
finding is a true false positive, propose the exclusion in the linter's configuration
and get it reviewed. A new suppression in a diff is itself a finding, and no automated
lint-fix pass adds one or edits the configuration.
Spelling: the language's suppression comment (
# noqa,eslint-disable,#[allow(...)],@SuppressWarnings, anolintcomment) and the linter's configuration file, whatever the repository uses; the configuration entry is the reviewed form.
Review: Did the diff add a suppression or edit the linter's configuration?
Handle an error once: wrap it with the context of the boundary it crossed and its cause, or handle it, never both log it and pass it on. Catch narrowly, the failure you can handle, never everything. Failure vocabulary belongs to the package that raises it: one named error per thing that can go wrong, made public only when a caller decides on it.
Spelling: wrapping is the language's cause-chaining form: an error wrapped with a message and its cause, an exception raised from the one caught, a result mapped with context. Catching narrowly is the specific error or exception type, never the root of the hierarchy and never a message string.
Review: Is any error both logged and passed on, caught broadly, or inspected by its message?
The examples above are pseudocode: the shape of each rule, with the language's spelling named in the note beneath it. Two rules exist because no binding knows this repository's language; everything else that would be a house rule here is the repository's own convention, read before the first edit.
Test and lint run through the commands the repository already defines: its Taskfile, Makefile, package scripts or CI. A linter, test framework, formatter or type checker the repository does not use is never installed or run; a repository with no linter is a finding to raise, not a gap to fill in a feature PR. A repository in several languages is worked one language at a time.
Review: Does the diff add a tool, dependency or configuration file the repository did not already use?
Every rule fixes a shape, and the move names are the same in every language; the spelling is not. Absence, failure, visibility, the doc form, the spawn and the join each have one form the repository already uses, and the fix arrives in that form: a result type where the code returns results, an exception where it raises, an optional where it declares one. An idiom carried in from another language, a sentinel where the language has optionals, a checked-error pair where it throws, a class hierarchy where a dispatch table is the norm, is a finding even when the rule it serves is satisfied.
Review: Does the diff spell a fix in another language's idiom where this one has its own form?
Before you ask for review, answer each with a file and line, not a feeling. The plugin's reviewers ask every rule question with a detection command behind it; these are the ones that catch the most. The house-rule questions are review questions only.
- R1 Does the diff validate a primitive inline instead of constructing a type? · Is the same predicate enforced in more than one place? · Does any function return a sentinel to mean "not found / invalid"? · Does a flat container of primitives cross a function boundary?
- R2 Can the type exist in an invalid state? · Does anything return or accept the missing value as a value?
- R3 Does one body mix abstraction levels? · Do block comments narrate sections inside a function body?
- R4 Are internal helpers tested directly? · Does a new shared package have a role name?
- R5 Is any package or module named after a layer or role? · Is one feature's code spread across ≥2 layer directories?
- R6 Is the only other implementer a test double? · Does the diff justify a new interface with "for testing" or "import cycle"?
- R7 Does any test case body contain a conditional? · Does any test reach past the public surface? · Does any test sleep to synchronize?
- R8 Does any package declare mutable state at package level? · Does deep code read a global config?
- R9 Does a doc comment on a public symbol state WHAT instead of WHY? · Did behavior change silently under an existing doc?
- R10 Does every concurrent task started in the diff have a provable exit path? · Does production code sleep?
- R11 Is the same discriminator inspected in more than one place? · Does a default branch (or trailing
else) handle "unknown kind" away from the boundary? · Does a boolean parameter select between behaviors? - R12 Does a method return an internal list or map by reference? · Can a validated type be mutated around its constructor?
- House rules Did the diff add a suppression or edit the linter's configuration? · Is any error both logged and passed on, caught broadly, or inspected by its message? · Does the diff add a tool, dependency or configuration file the repository did not already use? · Does the diff spell a fix in another language's idiom where this one has its own form?
| Test | the repository's test command |
| Lint | the repository's lint command |
| Lint and fix | the repository's lint command with its fix flag |
| Suppression | lint-suppression — never added on your own; a new one in a diff is a review finding |
| Docs | doc comment: short, says why, not what; long-form under docs/ |
| Discovery | the test, lint and lint-fix commands are the ones the repository defines in its README, CLAUDE.md, Taskfile, Makefile, package scripts or CI; none found is a finding, not a guess |
| Suppression forms | # noqa, eslint-disable, #[allow(...)], @SuppressWarnings, a nolint comment: one rule for all of them, see H1 |
| Examples | pseudocode: fail is the language's failure form, absent its declared absence, spawn and join its concurrency primitives, hidden its private visibility |
| Tests | beside the code or under a tests/ tree, whichever the repository does; every table row named; the package imported as a consumer would |
| Workflow | RED → GREEN → REFACTOR per behavior; package-scoped lint every cycle; review per finished slice; commit only a green tree |