rabot

A linter and formatter for Rust that enforces the principles of The Unwrap: sort everything, name what you built, wrap your primitives, treat errors as data, and write down every exception.

rustfmt decides where the whitespace goes. clippy catches bugs. rabot enforces the opinions in between: the ones that decide whether a codebase reads like architecture or like sediment.

curl -fsSL https://raw.githubusercontent.com/almaju/rabot/main/install.sh | sh
rabot            # lint the current directory
rabot fmt        # sort what can be sorted, then rustfmt the files it touched

How to read the rules

Every rule page has the same shape, so you can skim to the part you need:

• Principle: the one sentence from the article the rule enforces.
• What it checks: exactly when it fires.
• Don’t / Do: the code it rejects, and what to write instead.
• Silence it: the allow comment or the config key, when you have a reason. The reason is not optional.

Each page links to the article that makes the full argument. The rule is the enforcement; the article is the why.

Two commands

rabot check reports every principle violation and writes nothing. rabot fmt rewrites what a machine can fix safely: order. Fields, variants, impl items, derives, struct literals and patterns. Nothing else changes, and rustfmt runs afterwards so the result is what cargo fmt would produce.

rabot is checked by rabot. Its own source passes rabot fmt --check and rabot check --strict in CI, and the handful of places where it breaks its own rules carry a written reason.

Install

One line

Linux and macOS, x86_64 and arm64:

curl -fsSL https://raw.githubusercontent.com/almaju/rabot/main/install.sh | sh

The script downloads the archive for your platform from the latest release, verifies its checksum, and installs rabot and cargo-rabot into ~/.cargo/bin (or ~/.local/bin when there is no cargo). Two variables tune it:

With cargo

cargo install --git https://github.com/almaju/rabot --locked   # builds from source
cargo binstall --git https://github.com/almaju/rabot rabot      # downloads a release

Windows

Every release carries a x86_64-pc-windows-msvc.zip. Unpack it and put the two executables on your PATH.

What you get

Two binaries, rabot and cargo-rabot, so both rabot check and cargo rabot check work. There are no runtime dependencies. rustfmt is used when present, to re-indent after rabot fmt.

Use

rabot                      # = rabot check: lint the current directory, write nothing
rabot check --strict       # warnings fail the build too
rabot check --format json  # for editors and scripts

rabot fmt                  # sort fields, variants, impl items, derives,
                           # struct literals and patterns; then rustfmt
rabot fmt --check          # exit 1 if any file would change
rabot fmt --diff           # show what fmt would change, as a unified diff
rabot fmt --no-rustfmt     # reorder only, leave indentation alone

rabot check --changed      # only files with uncommitted changes
rabot fmt --changed=main   # only files touched since main

rabot hook                 # install a pre-commit hook
rabot rules                # every rule, its default level, the article behind it
rabot explain <rule>       # this documentation, in the terminal
rabot init                 # write a rabot.toml with every rule listed

A diagnostic

warning[primitive-soup]: `send_invoice` takes 3 `String` parameters (`user_id`, `email`, `invoice_id`): the compiler cannot tell them apart, so a swapped call site type-checks
  --> src/billing.rs:12:8
  = help: give each its own newtype (`struct UserId(String);`) and parse at the boundary
  = see: https://almaju.github.io/blog/docs/fundamentals/modeling/primitives

The first line names the rule and states the problem in terms of your code. The help says what to write instead. The link is the argument.

Exit codes

Migrate on contact

Apply alphabetical ordering to all new code going forward. To any file you’re already modifying. Don’t create churn for its own sake.

An existing codebase does not need a big-bang reformat. --changed scopes check and fmt to the files git sees as added, modified or untracked: uncommitted work by default, or everything since a ref with --changed=<ref>. The rest of the codebase is left alone until someone touches it.

rabot hook installs exactly that as a git pre-commit hook: the commit is refused (with the diff) when a staged file would be reordered, and the domain rules run on the staged files.

In CI

- uses: almaju/rabot@main

installs the latest release, fails on anything rabot fmt would reorder (printing the diff), then runs rabot check --strict. Inputs: version, args, fmt-check, working-directory.

Exceptions

You can break the rule. You must document the exception.

Every rule can be silenced for one item with a comment that names the rule and says why:

#![allow(unused)]
fn main() {
// rabot: allow(sorted-fields) drop order matters: the guard must release first
struct Connection {
    guard: MutexGuard<'static, ()>,
    channel: Channel,
}
}

The comment covers the item that follows it: the whole struct, the whole function body, the whole impl. As a trailing comment it covers its own line:

#![allow(unused)]
fn main() {
let port = env::var("PORT").unwrap(); // rabot: allow(panic-in-production) validated by the deploy script
}

Several rules at once, and the whole file:

#![allow(unused)]
fn main() {
// rabot: allow(free-function, primitive-soup) FFI surface mirrors the C header
// rabot: allow-file(mock-usage) legacy suite, being replaced under TEST-88
}

The reason is not optional

An allow comment without a reason is itself reported, at error level, as undocumented-exception. A rule name rabot does not know is unknown-rule. The point of the comment is the sentence after the parenthesis: the next reader, or you in six months, gets the reason instead of a mystery.

Turning a rule off everywhere

When a rule does not apply to a project at all, the config is the place, and the config file is the documentation:

[rules]
free-function = "allow"   # a crate of pure math functions

Configuration

rabot init writes a rabot.toml with every rule listed at its default level. Everything has a default; the file may be empty or absent.

[rules]
# "allow" | "warn" | "error". Rules not listed keep their default.
free-function = "allow"
untyped-error = "error"

[thresholds]
oversized-impl = 20          # methods across inherent impls in one file
primitive-soup = 2           # parameters of the same primitive type
section-comments = 3         # leading comments in one function body
too-many-parameters = 7      # parameters, excluding self
vague-todo-min-words = 6     # words a TODO needs, unless it links a ticket

[naming]
vague-suffixes = ["Controller", "Coordinator", "Handler", "Helper", "Manager",
                  "Processor", "Repository", "Service", "UseCase", "Util", "Utils"]
orphan-modules = ["common", "helper", "helpers", "misc", "util", "utils"]
domain-fields = ["_id", "amount", "email", "latitude", "longitude", "password",
                 "phone", "price", "token", "url", "..."]
enum-fields = ["category", "kind", "level", "mode", "phase", "role", "stage",
               "state", "status"]
escape-hatch-variants = ["Custom", "Generic", "Internal", "Misc", "Other",
                         "Unexpected", "Unknown"]
boundary-suffixes = ["Body", "Dto", "Params", "Payload", "Query", "Record",
                     "Request", "Response", "Row"]

[sorting]
# Pin derives to a position; the rest stay alphabetical in between.
derive-order = ["Debug", "Clone", "Copy", "...", "Serialize", "Deserialize"]

[tests]
# Rules that stay silent in test code.
relax = ["panic-in-production", "primitive-soup", "free-function", "..."]

[global-state]
allowed-names = ["LOG"]      # substring match, case-insensitive

[files]
exclude = ["target"]         # gitignore-style globs

Unknown keys are an error, so a typo cannot silently disable anything.

Levels

Defaults: every rule is warn, except undocumented-exception, unknown-rule and syntax-error, which are error.

Test code

An unwrap in a test is the assertion. A MockClock under #[cfg(test)] is exactly the injectable the testing article asks for. Test code answers to a different standard, and rabot knows where it is.

What counts as test code

• any item or impl item under #[cfg(test)], or a cfg that mentions test (#[cfg(any(test, feature = "test-utils"))])
• #[test], #[tokio::test], #[rstest] and #[bench] functions
• whole files under tests/, benches/ or examples/

What is relaxed there

The domain rules: panic-in-production, swallowed-error, dropped-error-context, untyped-error, boolean-validation, ambient-time, ambient-randomness, ambient-config, primitive-soup, primitive-field, stringly-typed-field, bypassable-constructor, free-function, vague-type-name, orphan-module, oversized-impl, too-many-parameters, sectioned-function.

What is not

Sorting, the comment rules, mock-usage, ignored-test and sleep-in-tests. A test file is still code, and the last three are about tests.

Tuning it

[tests]
relax = []                            # hold tests to the full standard
relax = ["panic-in-production"]       # relax only this one

Rules

Every rule is one principle from one article. fmt fixes the sorting family; check reports all of them. Levels are the defaults; every one can be changed in rabot.toml.

rabot rules prints the same table in the terminal; rabot explain <rule> prints a rule’s page.

Sorting

Sort your code alphabetically unless you have a documented reason not to.

Article: Sorting

Every developer has a system for where a field goes. The system lives in their head, conflicts with everyone else’s, and is invisible to the next person. Six developers later the struct is sediment: layers, each one a person who did not want to argue. Alphabetical order needs no documentation, no politics and no archaeology. You do not scan; you binary-search.

These seven rules are the formatter half of rabot. rabot fmt rewrites all of them in place; rabot check reports them.

What is sorted, and how

Order is case-insensitive and natural, so field2 comes before field10. Comments before a member move with it. Whitespace stays where it is, so a single-line list stays single-line and blank lines keep their place.

What is left alone

Lists whose order is semantic are never touched: #[repr] types, enums with explicit discriminants, enums deriving PartialOrd or Ord, and struct literals whose initializers may have side effects (reported, not rewritten). Function parameters are never sorted; the article calls calling convention a real exception.

sorted-fields

Level: warn · Fixed by rabot fmt · Article: Sorting

Sort alphabetically. Every time. Object properties, table columns, class methods, enum values. Everything that forms a list.

What it checks

Named fields of a struct, and of struct-like enum variants, are in alphabetical order (case-insensitive, field2 before field10). Tuple fields are positional and never sorted. #[repr(..)] structs are skipped: their layout is the point.

Don’t

#![allow(unused)]
fn main() {
struct User {
    id: UserId, // primary key first, obviously
    email: Email,
    name: UserName,
    created_at: DateTime, // metadata at the end
    updated_at: DateTime,
    last_login_at: Option<DateTime>,
    phone_number: Option<PhoneNumber>, // where does this go?
}
}

The logic lives in one developer’s head. The next developer tacks phone_number at the bottom because that is the safe move. Six developers later, the struct is sediment.

Do

#![allow(unused)]
fn main() {
struct User {
    created_at: DateTime, // metadata at the end
    email: Email,
    id: UserId, // primary key first, obviously
    last_login_at: Option<DateTime>,
    name: UserName,
    phone_number: Option<PhoneNumber>, // where does this go?
    updated_at: DateTime,
}
}

Nobody asks where phone_number goes. P comes after N, before U.

If two fields belong together, say so with a type, not with proximity:

#![allow(unused)]
fn main() {
struct UserName { first: String, last: String }
struct UserContact { email: String, phone: String }
struct User {
    contact: UserContact,
    name: UserName,
}
}

Silence it

#![allow(unused)]
fn main() {
// rabot: allow(sorted-fields) drop order matters: the guard must release before the pool
struct Connection {
    guard: MutexGuard<'static, ()>,
    pool: Pool,
}
}

Field order also decides Debug output and serde’s field order. Those are rarely a reason; when they are, write them down.

sorted-variants

Level: warn · Fixed by rabot fmt · Article: Sorting

One rule. Zero documentation required. Culturally neutral.

What it checks

Enum variants are in alphabetical order. Enums whose variant order is semantic are skipped without a diagnostic: #[repr(..)] enums, enums with explicit discriminants (A = 1), and enums deriving PartialOrd or Ord, where declaration order is the comparison order.

Don’t

#![allow(unused)]
fn main() {
enum UserRole {
    Member,
    Admin,
    Guest { invited_by: UserId, since: DateTime },
}
}

Do

#![allow(unused)]
fn main() {
enum UserRole {
    Admin,
    Guest { invited_by: UserId, since: DateTime },
    Member,
}
}

Fields of a struct-like variant are sorted too (that is sorted-fields).

Silence it

An enum whose order carries meaning usually says so already: derive PartialOrd and rabot steps aside. When the meaning is not a derive, write it down:

#![allow(unused)]
fn main() {
// rabot: allow(sorted-variants) matches the on-wire protocol numbering
enum Opcode { Connect, Publish, Subscribe, Disconnect }
}

sorted-impl-items

Level: warn · Fixed by rabot fmt · Article: Sorting

Method ordering: constructor, then pub (alpha), then private (alpha). Fine. It’s documented at the top of the class. Within each section, still alphabetical.

What it checks

Items inside an inherent impl follow the article’s documented exception: associated consts, associated types, constructors (associated functions returning Self), pub methods, then private methods, each group alphabetical. Inside impl Trait for T, consts, types, then fns.

An impl containing a macro invocation is left alone.

Don’t

#![allow(unused)]
fn main() {
impl Users {
    fn exists(&self, id: &UserId) -> bool {
        self.db.contains(id)
    }

    pub fn delete(&self, id: &UserId) {
        self.db.remove(id);
    }

    pub fn create(&self, user: User) {
        self.db.insert(user);
    }

    pub fn new(db: Database) -> Self {
        Self { db }
    }
}
}

Do

#![allow(unused)]
fn main() {
impl Users {
    pub fn new(db: Database) -> Self {
        Self { db }
    }

    pub fn create(&self, user: User) {
        self.db.insert(user);
    }

    pub fn delete(&self, id: &UserId) {
        self.db.remove(id);
    }

    fn exists(&self, id: &UserId) -> bool {
        self.db.contains(id)
    }
}
}

The constructor is where a reader starts. Public API next, in an order that needs no explaining. Implementation details last.

Silence it

#![allow(unused)]
fn main() {
// rabot: allow(sorted-impl-items) distance_to and is_near are inseparable: is_near wraps distance_to
impl GpsCoordinates {
    fn distance_to(&self, other: &Self) -> Distance { .. }
    fn is_near(&self, other: &Self, radius: Distance) -> bool { .. }
}
}

The article’s own example: one sentence says why they are together. If it takes more than one sentence, there is a separate type trying to escape.

sorted-trait-items

Level: warn · Fixed by rabot fmt · Article: Sorting

What it checks

Items in a trait definition: associated consts, then associated types, then functions, each group alphabetical. Trait impls follow the same order (see sorted-impl-items), so a definition and its impls line up.

Don’t

#![allow(unused)]
fn main() {
trait Persist {
    fn save(&self, store: &Store) -> Result<(), SaveError>;
    type Error;
    fn load(id: &Id, store: &Store) -> Result<Self, Self::Error>;
    const TABLE: &'static str;
}
}

Do

#![allow(unused)]
fn main() {
trait Persist {
    const TABLE: &'static str;
    type Error;
    fn load(id: &Id, store: &Store) -> Result<Self, Self::Error>;
    fn save(&self, store: &Store) -> Result<(), SaveError>;
}
}

Silence it

#![allow(unused)]
fn main() {
// rabot: allow(sorted-trait-items) documented as a state machine: methods appear in call order
trait Handshake { .. }
}

sorted-struct-literal

Level: warn · Fixed by rabot fmt (when safe) · Article: Sorting

What it checks

The fields of a struct literal, User { .. }, are in alphabetical order, the same order as the definition. ..base stays last.

rabot only rewrites a literal when every initializer is plainly side-effect free: literals, paths, field accesses, references, Some(..), clone(), Default::default() and the like. Initializers are evaluated in source order, so a literal with calls in it is reported and left for you to reorder by hand.

Don’t

#![allow(unused)]
fn main() {
impl User {
    fn from_signup(input: Signup, now: DateTime) -> Self {
        User {
            role: Role::Member,
            name: input.name,
            id: UserId::new(),
            email: input.email,
            created_at: now,
        }
    }
}
}

Do

#![allow(unused)]
fn main() {
impl User {
    fn from_signup(input: Signup, now: DateTime) -> Self {
        User {
            created_at: now,
            email: input.email,
            id: UserId::new(),
            name: input.name,
            role: Role::Member,
        }
    }
}
}

Same order as the struct, every time it is built. A reviewer comparing the two never has to hunt.

Silence it

#![allow(unused)]
fn main() {
// rabot: allow(sorted-struct-literal) initializers must run in this order: the token is minted before the session
Session { token: mint(&mut rng), id: next_id(&mut rng) }
}

sorted-struct-pattern

Level: warn · Fixed by rabot fmt · Article: Sorting

What it checks

Fields in a struct pattern, in let or in a match arm, are alphabetical. Patterns have no evaluation order, so this is always safe to rewrite. .. stays last.

Don’t

#![allow(unused)]
fn main() {
impl User {
    fn greeting(&self) -> String {
        let User { name, email, .. } = self;
        format!("{name} <{email}>")
    }
}

impl Event {
    fn describe(&self) -> String {
        match self {
            Event::Moved { to, from, at } => format!("{from} -> {to} at {at}"),
        }
    }
}
}

Do

#![allow(unused)]
fn main() {
impl User {
    fn greeting(&self) -> String {
        let User { email, name, .. } = self;
        format!("{name} <{email}>")
    }
}

impl Event {
    fn describe(&self) -> String {
        match self {
            Event::Moved { at, from, to } => format!("{from} -> {to} at {at}"),
        }
    }
}
}

Silence it

#![allow(unused)]
fn main() {
// rabot: allow(sorted-struct-pattern) mirrors the wire order documented in RFC-12
}

sorted-derives

Level: warn · Fixed by rabot fmt · Article: Sorting

What it checks

The list inside #[derive(..)] is alphabetical, with one documented exception: a derive follows the trait it extends. Eq comes right after PartialEq, Ord right after PartialOrd, Copy right after Clone. Reading Eq, PartialEq backwards is what alphabetical order alone would give you, and nobody writes it that way.

Don’t

#![allow(unused)]
fn main() {
#[derive(Serialize, Debug, Eq, Clone, PartialEq, Ord, PartialOrd, Copy, Hash)]
struct UserId(Uuid);
}

Do

#![allow(unused)]
fn main() {
#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, PartialOrd, Ord, Serialize)]
struct UserId(Uuid);
}

Options

Pin derives to a position, in the manner of cargo-sort-derives: names before "..." come first in that order, names after it come last, and the rest sit in between under the rule above. Matching is on the last path segment, so serde::Serialize and Serialize are the same pin.

[sorting]
derive-order = ["Debug", "Clone", "Copy", "...", "Serialize", "Deserialize"]

With that setting the example becomes #[derive(Debug, Clone, Copy, Hash, PartialEq, Eq, PartialOrd, Ord, Serialize)].

Silence it

#![allow(unused)]
fn main() {
// rabot: allow(sorted-derives) the proc macro must see Builder before Default
#[derive(Builder, Default)]
}

Primitives

Wrap every primitive that carries domain meaning in a dedicated type. Let the compiler enforce what your variable names only suggest.

Article: Primitives

In 1999 NASA lost a $327 million orbiter to a float: one team measured in pound-force seconds, another in newton seconds, and the type system had nothing to say. Your bug will be smaller and the mechanism identical: a raw value that meant one thing was used where a raw value meaning another was expected, and nothing caught it.

Four rules push the domain into the type system: two parameters of the same primitive type, a field whose name promises more than its type, a String that is really an enum, and a validating constructor with a back door.

primitive-soup

Level: warn · Article: Primitives

Three bugs. All type-safe. The compiler is happy. Your users are not.

What it checks

A function takes two or more parameters of the same primitive type (String, &str, integers, floats, bool, Option<..> of those). Two String parameters can be swapped at any call site and the program still compiles. Methods inside impl Trait for T are skipped: that signature is the trait’s.

Don’t

#![allow(unused)]
fn main() {
impl Mailer {
    fn send_invoice(&self, user_id: String, email: String, invoice_id: String) {
        self.send_email(&user_id, &invoice_id); // swapped
        self.log_access(&invoice_id, &email); // wrong order
    }
}
}

Do

#![allow(unused)]
fn main() {
struct UserId(String);
struct Email(String);
struct InvoiceId(String);

impl Mailer {
    fn send_invoice(&self, user_id: UserId, email: Email, invoice_id: InvoiceId) {
        self.send_email(&email, &invoice_id);
        self.log_access(&user_id, &invoice_id);
    }
}
}

You write the type once. The build catches the swap instead of the review. The newtype compiles to the same representation as the primitive; the cost is zero.

Options

[thresholds]
primitive-soup = 2   # parameters of the same primitive type before it fires

Silence it

#![allow(unused)]
fn main() {
// rabot: allow(primitive-soup) stateless math with no subject: min, max are both just numbers
fn clamp(value: f64, min: f64, max: f64) -> f64 { .. }
}

The article’s own exception: genuinely stateless math, where no parameter means anything on its own.

primitive-field

Level: warn · Article: Primitives

Data comes in as primitives. It leaves as domain types. It stays as domain types until it leaves the system. No exceptions.

What it checks

A named struct field whose name says “domain concept” and whose type says “anything”: email: String, user_id: u64, latitude: f64, price: Option<f64>. The names come from a configurable list of words and _-suffixes; bool and char never count.

Wire shapes are skipped. A struct named *Request, *Response, *Row, *Dto and so on is where primitives arrive, and parsing happens right after.

Don’t

#![allow(unused)]
fn main() {
struct User {
    email: String,
    id: String,
    latitude: f64,
}
}

email accepts "not an email". id accepts an order id. latitude accepts 400.

Do

#![allow(unused)]
fn main() {
struct User {
    email: Email,
    id: UserId,
    latitude: Latitude,
}

struct Email(String);

impl Email {
    fn parse(s: &str) -> Result<Self, ValidationError> {
        if !s.contains('@') {
            return Err(ValidationError::MissingAt);
        }
        Ok(Self(s.to_lowercase()))
    }
}
}

Validate once, at construction, at the boundary. Everything past that line is typed and nobody checks again.

Options

[naming]
domain-fields = ["_id", "amount", "email", "latitude", "longitude", "password",
                 "phone", "price", "token", "url"]
boundary-suffixes = ["Body", "Dto", "Params", "Payload", "Query", "Record",
                     "Request", "Response", "Row"]

Silence it

#![allow(unused)]
fn main() {
// rabot: allow(primitive-field) mirrors the vendor's CSV columns; parsed into Reading right after
struct RawReading { latitude: f64, longitude: f64 }
}

stringly-typed-field

Level: warn · Article: Primitives

The comment // status: 'pending' | 'approved' | 'rejected' is a Status enum that hasn’t been written yet. Write it. Delete the comment.

What it checks

A named field called status, state, kind, role, mode, level, phase, stage, category or type whose type is String, &str or Option<String>. A value with a handful of valid spellings is an enum, and the compiler checks every match on an enum.

Wire shapes (*Request, *Row, …) are skipped, like primitive-field.

Don’t

#![allow(unused)]
fn main() {
struct Order {
    status: String, // "pending" | "approved" | "rejected"
}

impl Order {
    fn ship_if_approved(self, warehouse: &Warehouse) {
        if self.status == "aproved" {
            warehouse.ship(self); // never ships
        }
    }
}
}

Do

#![allow(unused)]
fn main() {
enum OrderStatus {
    Approved,
    Pending,
    Rejected,
}

struct Order {
    status: OrderStatus,
}

impl Order {
    fn ship_if_approved(self, warehouse: &Warehouse) {
        match self.status {
            OrderStatus::Approved => warehouse.ship(self),
            OrderStatus::Pending | OrderStatus::Rejected => {}
        }
    }
}
}

Parse the string once, where it enters. A typo is now a compile error, and adding a variant makes every match that forgot it fail to build.

Options

[naming]
enum-fields = ["category", "kind", "level", "mode", "phase", "role", "stage", "state", "status"]

Silence it

#![allow(unused)]
fn main() {
// rabot: allow(stringly-typed-field) free-form user label, not a closed set
struct Tag { kind: String }
}

bypassable-constructor

Level: warn · Article: Primitives

The invariant lives in the type. It cannot be violated without going through the constructor. The constructor rejects violations.

What it checks

A single-field tuple struct with a pub field, in the same file as an associated function that returns Result<Self, _> or Option<Self>. The constructor can say no; the pub field lets anyone build the value without asking.

Don’t

#![allow(unused)]
fn main() {
pub struct Percentage(pub f64);

impl Percentage {
    pub fn new(n: f64) -> Result<Self, ValidationError> {
        if !(0.0..=100.0).contains(&n) {
            return Err(ValidationError::OutOfRange(n));
        }
        Ok(Self(n))
    }
}

impl Cart {
    fn apply_discount(&mut self) {
        self.discount = Percentage(250.0); // never went through the door
    }
}
}

Do

#![allow(unused)]
fn main() {
pub struct Percentage(f64);

impl Percentage {
    pub fn new(n: f64) -> Result<Self, ValidationError> {
        if !(0.0..=100.0).contains(&n) {
            return Err(ValidationError::OutOfRange(n));
        }
        Ok(Self(n))
    }

    pub fn value(&self) -> f64 {
        self.0
    }
}
}

One way in. Every Percentage in the program has been checked, by construction, and no function that receives one has to check again.

Silence it

#![allow(unused)]
fn main() {
// rabot: allow(bypassable-constructor) any f64 is a valid Meters; `parse` only exists for the text form
pub struct Meters(pub f64);
}

boolean-validation

Level: warn · Article: Errors

Your type signatures are lying to you.

What it checks

A function named validate_*, verify_* or is_valid* that returns bool. Plain predicates (is_empty, check_flag) are not validation and are left alone. Validation has a reason to say no, and false cannot carry it.

Don’t

#![allow(unused)]
fn main() {
fn validate_email(s: &str) -> bool {
    s.contains('@') && !s.starts_with('@')
}

fn register(input: &str) -> Result<(), ApiError> {
    if !validate_email(input) {
        return Err(ApiError::Invalid("email")); // which rule? the user has to guess
    }
    Ok(())
}
}

Do

#![allow(unused)]
fn main() {
enum EmailError {
    MissingAt,
    MissingLocalPart,
}

struct Email(String);

impl Email {
    fn parse(s: &str) -> Result<Self, EmailError> {
        if !s.contains('@') {
            return Err(EmailError::MissingAt);
        }
        if s.starts_with('@') {
            return Err(EmailError::MissingLocalPart);
        }
        Ok(Email(s.to_lowercase()))
    }
}
}

The reason travels with the failure, and the success is a type: nothing past this line checks the email again.

Silence it

#![allow(unused)]
fn main() {
// rabot: allow(boolean-validation) a pure predicate used in a filter; there is no caller to inform
fn is_valid_utf8(bytes: &[u8]) -> bool { .. }
}

Structs and method ownership

Obsess over your data structures, not your algorithms. Put behavior on the type it belongs to. Repository, Handler, and Service are names for decisions you haven’t made yet.

Articles: Structs, Method Ownership

You use HashMap every day without caring about its bucket layout. Your job is to build the same thing for your domain: a GpsCoordinates with a distance_to, not twelve copies of the Haversine formula with four f64 parameters each. Every function in utils.rs is a method on a type that does not exist yet, and every UserService is a place where the same rule gets implemented twice.

Five rules: free functions that belong on a type, names that dodge a decision, drawer modules, impls that are three types in a trenchcoat, and parameter lists that are a struct waiting to be named.

free-function

Level: warn · Article: Method Ownership

A free function almost always has a home. Either in its return type or its primary parameter.

What it checks

A free function (not in an impl) whose first parameter is one of your own types, or which returns one of your own types. main, extern functions and generic parameters are excluded.

Don’t

#![allow(unused)]
fn main() {
struct User {
    banned: bool,
    name: UserName,
}

struct Url(String);

fn ban(user: &mut User) {
    user.banned = true;
}

fn parse_url(s: &str) -> Result<Url, ParseError> {
    Ok(Url(s.to_string()))
}

fn format_user(user: &User) -> String {
    format!("{}", user.name)
}
}

Six months later somebody who could not find ban adds a second one on UserService. Now there are two, and one is wrong.

Do

#![allow(unused)]
fn main() {
struct User {
    banned: bool,
    name: UserName,
}

impl User {
    fn ban(&mut self) {
        self.banned = true;
    }

    fn display_name(&self) -> String {
        format!("{}", self.name)
    }
}

struct Url(String);

impl Url {
    fn parse(s: &str) -> Result<Self, ParseError> {
        Ok(Self(s.to_string()))
    }
}
}

user.ban(). Url::parse(s). One place to look, one place to add logic, and the compiler knows the method exists so nobody writes it twice.

Silence it

#![allow(unused)]
fn main() {
// rabot: allow(free-function) spans two types and belongs to neither: the transaction orchestrates both
fn commit(store: &Store, orders: &[Order]) -> Result<(), CommitError> { .. }
}

The article’s exceptions: stateless math (clamp), top-level orchestration (App), and operations that genuinely belong to a third thing.

vague-type-name

Level: warn · Article: Method Ownership

When you need a word like “Manager” or “Service” to explain what your code does, you’re admitting you don’t know what your code does.

What it checks

A struct, enum, trait or type alias whose name ends in Service, Manager, Handler, Controller, Repository, Coordinator, Processor, Helper, UseCase, Util or Utils. The suffix must be a whole word: Chandler is fine.

Don’t

#![allow(unused)]
fn main() {
struct UserService {
    db: Database,
}

struct UserRepository {
    db: Database,
}

struct UserManager {
    db: Database, // added six months ago; nobody knows why
}
}

You need to ban a user. Which one owns it? You pick one, ship it, and eight months later there are two ban_users.

Do

#![allow(unused)]
fn main() {
struct User {
    banned: bool,
    id: UserId,
}

struct Store {
    db: Database,
}

impl User {
    fn ban(self) -> Self {
        Self { banned: true, ..self }
    }

    async fn save(&self, store: &Store) -> Result<(), SaveError> {
        store.db.upsert(self).await
    }
}
}

Tell a colleague what you shipped: “the API and the todos”. Those are the structs. TodoController is a name from a tutorial.

Options

[naming]
vague-suffixes = ["Controller", "Coordinator", "Handler", "Helper", "Manager",
                  "Processor", "Repository", "Service", "UseCase", "Util", "Utils"]

Silence it

#![allow(unused)]
fn main() {
// rabot: allow(vague-type-name) implements the DDD Repository contract: the domain never sees SQL
struct OrderRepository { .. }
}

If you are genuinely implementing the pattern, own it. Write down why.

orphan-module

Level: warn · Article: Structs

Every function in your utils file is a method on a type that doesn’t exist yet. The type is there. You just haven’t named it.

What it checks

A module named utils, util, helpers, helper, common or misc, whether declared inline or as mod utils;.

Don’t

#![allow(unused)]
fn main() {
mod utils {
    pub fn distance_between(from: &GpsCoordinates, to: &GpsCoordinates) -> Distance {
        Distance::haversine(from.latitude, from.longitude, to.latitude, to.longitude)
    }

    pub fn format_coordinates(coordinates: &GpsCoordinates) -> String {
        format!("{}, {}", coordinates.latitude, coordinates.longitude)
    }

    pub fn parse_coordinates(input: &str) -> Result<GpsCoordinates, InvalidCoordinates> {
        let (latitude, longitude) = input.split_once(',').ok_or(InvalidCoordinates::MissingComma)?;
        Ok(GpsCoordinates {
            latitude: latitude.parse()?,
            longitude: longitude.parse()?,
        })
    }
}
}

It starts with one function that has no obvious home. Then fifteen. Then it is 800 lines and nobody can say what it is about, because it is not about anything. It is a drawer.

Do

#![allow(unused)]
fn main() {
struct GpsCoordinates {
    latitude: Latitude,
    longitude: Longitude,
}

impl GpsCoordinates {
    fn parse(input: &str) -> Result<Self, InvalidCoordinates> {
        let (latitude, longitude) = input.split_once(',').ok_or(InvalidCoordinates::MissingComma)?;
        Ok(Self {
            latitude: latitude.parse()?,
            longitude: longitude.parse()?,
        })
    }

    fn display(&self) -> String {
        format!("{}, {}", self.latitude, self.longitude)
    }

    fn distance_to(&self, other: &Self) -> Distance {
        Distance::haversine(self, other)
    }
}
}

Five functions that share three parameters are a struct. Name it and the orphans find their home.

Options

[naming]
orphan-modules = ["common", "helper", "helpers", "misc", "util", "utils"]

Silence it

#![allow(unused)]
fn main() {
// rabot: allow(orphan-module) test support only: builders and fixtures for the integration suite
mod helpers;
}

oversized-impl

Level: warn · Article: Structs

A struct with 25 methods is usually three structs that haven’t been separated yet.

What it checks

The inherent impls of one type, in one file, hold more than 20 methods.

Don’t

#![allow(unused)]
fn main() {
struct User {
    events: Vec<UserEvent>,
}

impl User {
    fn activate(&mut self) {
        self.events.push(UserEvent::Activate);
    }

    fn archive(&mut self) {
        self.events.push(UserEvent::Archive);
    }

    fn ban(&mut self) {
        self.events.push(UserEvent::Ban);
    }

    fn charge(&mut self, amount: Money) {
        self.events.push(UserEvent::Charge(amount));
    }

    fn deactivate(&mut self) {
        self.events.push(UserEvent::Deactivate);
    }

    fn delete(&mut self) {
        self.events.push(UserEvent::Delete);
    }

    fn export(&mut self) {
        self.events.push(UserEvent::Export);
    }

    fn invite(&mut self) {
        self.events.push(UserEvent::Invite);
    }

    fn invoice(&mut self, amount: Money) {
        self.events.push(UserEvent::Invoice(amount));
    }

    fn lock(&mut self) {
        self.events.push(UserEvent::Lock);
    }

    fn notify(&mut self, message: Message) {
        self.events.push(UserEvent::Notified(message));
    }

    fn promote(&mut self) {
        self.events.push(UserEvent::Promote);
    }

    fn refund(&mut self, amount: Money) {
        self.events.push(UserEvent::Refund(amount));
    }

    fn rename(&mut self, name: UserName) {
        self.events.push(UserEvent::Renamed(name));
    }

    fn restore(&mut self) {
        self.events.push(UserEvent::Restore);
    }

    fn subscribe(&mut self) {
        self.events.push(UserEvent::Subscribe);
    }

    fn suspend(&mut self) {
        self.events.push(UserEvent::Suspend);
    }

    fn unban(&mut self) {
        self.events.push(UserEvent::Unban);
    }

    fn unlock(&mut self) {
        self.events.push(UserEvent::Unlock);
    }

    fn unsubscribe(&mut self) {
        self.events.push(UserEvent::Unsubscribe);
    }

    fn upgrade(&mut self) {
        self.events.push(UserEvent::Upgrade);
    }
}
}

Do

#![allow(unused)]
fn main() {
struct User {
    billing: BillingProfile,
    events: Vec<UserEvent>,
    notifications: NotificationSettings,
}

impl User {
    fn ban(&mut self) {
        self.events.push(UserEvent::Banned);
    }

    fn promote(&mut self) {
        self.events.push(UserEvent::Promoted);
    }
}

struct BillingProfile {
    ledger: Vec<Charge>,
}

impl BillingProfile {
    fn charge(&mut self, amount: Money) {
        self.ledger.push(Charge::Debit(amount));
    }

    fn invoice(&self) -> Invoice {
        Invoice::from_ledger(&self.ledger)
    }

    fn refund(&mut self, amount: Money) {
        self.ledger.push(Charge::Credit(amount));
    }
}

struct NotificationSettings {
    channels: Vec<Channel>,
}

impl NotificationSettings {
    fn notify(&self, message: Message) {
        for channel in &self.channels {
            channel.send(&message);
        }
    }

    fn subscribe(&mut self, channel: Channel) {
        self.channels.push(channel);
    }

    fn unsubscribe(&mut self, channel: &Channel) {
        self.channels.retain(|existing| existing != channel);
    }
}
}

Ask which methods operate on a subset of the fields. That subset is its own struct, with five methods that actually belong to it.

Options

[thresholds]
oversized-impl = 20

Silence it

#![allow(unused)]
fn main() {
// rabot: allow(oversized-impl) builder: one method per option is the whole point
impl CommandBuilder { .. }
}

too-many-parameters

Level: warn · Article: Structs

The signal that you need a struct: you’re passing the same three parameters to five different functions. Those parameters are trying to tell you something.

What it checks

A function or method takes more than 7 parameters (self not counted). Methods inside impl Trait for T are skipped.

Don’t

#![allow(unused)]
fn main() {
impl Renderer {
    fn render(
        &self,
        title: &str,
        width: Width,
        height: Height,
        dpi: Dpi,
        margin: Margin,
        font: &Font,
        color: Color,
        background: Color,
    ) -> Image {
        self.blank(width, height, dpi)
            .fill(background)
            .text(title, font, color, margin)
    }
}
}

Do

#![allow(unused)]
fn main() {
struct Canvas {
    dpi: Dpi,
    height: Height,
    margin: Margin,
    width: Width,
}

struct Style {
    background: Color,
    color: Color,
    font: Font,
}

impl Renderer {
    fn render(&self, title: &str, canvas: &Canvas, style: &Style) -> Image {
        self.blank(canvas)
            .fill(style.background)
            .text(title, &style.font, style.color, canvas.margin)
    }
}
}

Parameters that travel together are a struct waiting to be named. Once named, they get a home for the logic that was scattered across every caller.

Options

[thresholds]
too-many-parameters = 7

Silence it

#![allow(unused)]
fn main() {
// rabot: allow(too-many-parameters) mirrors the C ABI of libfoo_render exactly
}

Errors

Handle errors like any other value in your program. Every operation that can fail returns a discriminated union. No throws. No panics. Just honest return values that force you to confront reality.

Article: Errors

It is 3am and something between the request and the response is swallowing an error. Six functions. Three nested catch blocks. Forty minutes of reading. The problem started when somebody wrote a signature that promised a value and could not keep the promise.

Three rules: panics in code that could have returned a Result, error types that erase what went wrong, and the silent catch.

panic-in-production

Level: warn · Article: Errors

Every .unwrap() in production is a bet that this particular call site will never fail. You are wrong about that bet more often than you think, and you find out at the worst possible time.

What it checks

.unwrap(), .expect(), .unwrap_err(), .expect_err(), panic!, unreachable!, todo! and unimplemented! outside the two places the article allows: fn main, where a missing config file may legitimately abort, and test code (see Test code).

Don’t

#![allow(unused)]
fn main() {
fn get_user(id: &UserId, db: &Database) -> User {
    db.query("SELECT * FROM users WHERE id = $1", id).unwrap()
}
}

The signature promises a User. It cannot keep that promise, and the caller has no way to know.

Do

fn get_user(id: &UserId, db: &Database) -> Result<User, NotFoundError> {
    db.query("SELECT * FROM users WHERE id = $1", id)
        .ok_or(NotFoundError { user_id: id.clone() })
}

// Startup: the program cannot run without these. Panicking is honest here.
fn main() {
    let config = load_config().expect("config file required for startup");
    serve(config);
}

Startup is the exception: the program cannot run without its config, and main is the one place where panicking is honest.

Silence it

#![allow(unused)]
fn main() {
let first = items.first().unwrap(); // rabot: allow(panic-in-production) `items` was checked non-empty two lines up
}

An invariant that proves programmer error is the article’s other exception. Say which invariant.

untyped-error

Level: warn · Article: Errors

With exceptions, you catch Error and guess. With typed errors, you match the variant and act. The difference is whether your recovery logic is a strategy or a prayer.

What it checks

A function returns Box<dyn Error>, anyhow::Result/anyhow::Error, eyre, or Result<T, String> / Result<T, &str>. fn main and trait impls are skipped: main may bubble anything, and Error::source is std’s signature, not yours.

Don’t

#![allow(unused)]
fn main() {
fn fetch(url: &Url) -> Result<Response, Box<dyn Error>> {
    Ok(http::get(url)?)
}

fn parse(input: &str) -> Result<Config, String> {
    toml::from_str(input).map_err(|error| error.to_string())
}
}

The caller can display the error. It cannot retry on a timeout, refresh on an expired token, and give up on a validation failure, because it cannot tell them apart.

Do

#![allow(unused)]
fn main() {
enum FetchError {
    Auth { refresh_token: RefreshToken },
    Network { retry_after: Duration },
    RateLimited { retry_after: Duration },
    Validation(ValidationError),
}

fn fetch(url: &Url) -> Result<Response, FetchError> {
    http::get(url).map_err(FetchError::from)
}

impl Client {
    fn fetch_with_retry(&self, url: &Url) -> Result<Response, FetchError> {
        match fetch(url) {
            Ok(response) => Ok(response),
            Err(FetchError::Network { retry_after } | FetchError::RateLimited { retry_after }) => {
                self.clock.sleep(retry_after);
                fetch(url)
            }
            Err(FetchError::Auth { refresh_token }) => {
                self.refresh(refresh_token)?;
                fetch(url)
            }
            Err(error @ FetchError::Validation(_)) => Err(error),
        }
    }
}
}

Granular enough to make different decisions. If two failures get identical handling, they are one variant.

Silence it

#![allow(unused)]
fn main() {
// rabot: allow(untyped-error) CLI entry point: every failure ends in the same exit code and message
fn run(args: Args) -> Result<(), Box<dyn Error>> { .. }
}

swallowed-error

Level: warn · Article: Errors

Every silent catch is a future 3am.

What it checks

Three shapes that make a failure disappear:

• an empty Err arm: Err(_) => {}
• an empty if let Err(..) = .. {}
• .ok(); as a statement, which converts the Result to an Option and throws it away

Don’t

#![allow(unused)]
fn main() {
impl Cache {
    fn evict(&self, key: &Key) {
        match self.invalidate(key) {
            Ok(()) => {}
            Err(_) => {} // shouldn't happen
        }
        std::fs::remove_file(self.path_for(key)).ok();
    }
}
}

It happened. The comment lied. Somebody will spend four hours finding which branch swallowed it.

Do

#![allow(unused)]
fn main() {
impl Cache {
    fn evict(&self, key: &Key) -> Result<(), EvictError> {
        if let Err(error) = self.invalidate(key) {
            warn!(%key, %error, "cache entry survives invalidation; serving stale until TTL");
        }
        std::fs::remove_file(self.path_for(key))?;
        Ok(())
    }
}
}

Propagate it, or log it with the context the reader at 3am needs. Either way, the failure leaves a trace.

Silence it

#![allow(unused)]
fn main() {
// rabot: allow(swallowed-error) best-effort cleanup of a temp file; the OS reclaims it anyway
std::fs::remove_file(&tmp).ok();
}

dropped-error-context

Level: warn · Article: Errors

Every nested level loses context. Every silent catch is a future 3am.

What it checks

.map_err(|_| ..), .or_else(|_| ..) or .unwrap_or_else(|_| ..) whose closure ignores the error it receives: a parameter named _ or starting with _. The original failure, the one with the file name and the OS message, is gone before anyone reads it.

Don’t

#![allow(unused)]
fn main() {
impl Config {
    fn load(path: &Path) -> Result<Self, ConfigError> {
        let text = std::fs::read_to_string(path).map_err(|_| ConfigError::Unreadable)?;
        text.parse()
    }
}
}

“Config unreadable.” Which file? Permission denied, or not found, or a directory? The error that knew is gone.

Do

#![allow(unused)]
fn main() {
enum ConfigError {
    Unreadable {
        path: PathBuf,
        #[source]
        source: std::io::Error,
    },
}

impl Config {
    fn load(path: &Path) -> Result<Self, ConfigError> {
        let text = std::fs::read_to_string(path).map_err(|source| ConfigError::Unreadable {
            path: path.to_path_buf(),
            source,
        })?;
        text.parse()
    }
}
}

Or with thiserror, #[from] and ? do it without a closure at all. The caller matches on your variant; the log walks source() down to the OS.

Silence it

#![allow(unused)]
fn main() {
// rabot: allow(dropped-error-context) the only possible failure is Utf8; the position is what matters
let name = String::from_utf8(bytes).map_err(|_| NameError::NotUtf8 { offset })?;
}

escape-hatch-variant

Level: warn · Article: Errors

Granular enough to make different decisions. If two errors require identical handling, they’re the same type. If they require different recovery strategies, split them.

What it checks

An enum whose name ends in Error with a variant named Other, Unknown, Custom, Internal, Generic, Misc or Unexpected, holding a String, a boxed error, or nothing.

Don’t

#![allow(unused)]
fn main() {
enum PaymentError {
    CardDeclined { reason: DeclineReason },
    Other(String),
}
}

Six months later Other carries network timeouts, a provider outage, a currency mismatch and a typo. The retry logic matches on CardDeclined and guesses at everything else.

Do

#![allow(unused)]
fn main() {
enum PaymentError {
    CardDeclined {
        reason: DeclineReason,
    },
    CurrencyMismatch {
        expected: Currency,
        got: Currency,
    },
    Provider {
        retry_after: Option<Duration>,
        #[source]
        source: ProviderError,
    },
}
}

Each variant is a decision the caller can make. Adding a failure mode means adding a variant, and the compiler finds every match that has to decide about it.

Options

[naming]
escape-hatch-variants = ["Custom", "Generic", "Internal", "Misc", "Other", "Unexpected", "Unknown"]

Silence it

#![allow(unused)]
fn main() {
// rabot: allow(escape-hatch-variant) FFI boundary: the C library reports free-form strings we cannot classify
}

Dependencies

Every function and class should declare exactly what it needs. No singletons. No magic injection. If it’s not in the signature, it shouldn’t exist.

Article: Dependencies

A global is a dependency hidden from every signature that uses it. You find out about it when two tests mutate it in parallel, or when you try to move the code and discover what it secretly needed. The compiler cannot help because the dependency is invisible.

One rule, for the one thing a linter can see: mutable global state.

global-state

Level: warn · Article: Dependencies

Invisible dependencies are the cockroaches of software architecture: everywhere, impossible to count, surviving every refactor.

What it checks

A static with interior mutability (Mutex, RwLock, OnceLock, OnceCell, LazyLock, Cell, RefCell, atomics), static mut, or a lazy_static! block. Statics whose name contains LOG are exempt by default: a logger is infrastructure nobody swaps in tests.

Don’t

#![allow(unused)]
fn main() {
static DATABASE: OnceLock<Database> = OnceLock::new();

impl User {
    async fn load(id: &UserId) -> Result<User, LoadError> {
        let db = DATABASE.get().ok_or(LoadError::NotConnected)?; // hidden dependency
        db.query(id).await
    }
}
}

Zero constructor parameters. Looks simple. Until two tests run in parallel against the same global, or you need to point it at another database.

Do

struct Users {
    db: Database,
}

impl Users {
    async fn load(&self, id: &UserId) -> Result<User, LoadError> {
        self.db.query(id).await
    }
}

#[tokio::main]
async fn main() -> Result<(), StartupError> {
    // every dependency constructed in one place, then handed down
    let config = Config::from_env()?;
    let db = Database::connect(&config.db_url).await?;
    let users = Users { db };
    Api::new(users).serve().await
}

Exactly as complex as it actually is, and checked at compile time.

Options

[global-state]
allowed-names = ["LOG"]   # substring match, case-insensitive

Silence it

#![allow(unused)]
fn main() {
// rabot: allow(global-state) compiled-once regex; a pure value, never swapped
static EMAIL: LazyLock<Regex> = LazyLock::new(|| Regex::new(..).unwrap());
}

ambient-config

Level: warn · Article: Dependencies

All dependencies get constructed in one place. One file. That’s it.

What it checks

std::env::var, env::var_os or env::vars outside main and outside functions and types that exist to read configuration (a name containing config, settings or env, such as load_config or Config::from_env).

Don’t

#![allow(unused)]
fn main() {
async fn start_server(app: App) -> Result<(), ServerError> {
    let port: u16 = std::env::var("PORT")?.parse()?;
    app.listen(port).await
}
}

The signature says this needs an App. It also needs PORT, and you find out when it is missing, at runtime, in the environment where it was not set.

Do

struct Config {
    db_url: DatabaseUrl,
    port: Port,
}

impl Config {
    fn from_env() -> Result<Self, ConfigError> {
        Ok(Self {
            db_url: DatabaseUrl::parse(std::env::var("DATABASE_URL")?)?,
            port: Port::parse(std::env::var("PORT")?)?,
        })
    }
}

async fn start_server(app: App, port: Port) -> Result<(), ServerError> {
    app.listen(port).await
}

fn main() {
    let config = Config::from_env().expect("configuration required for startup");
    start_server(App::new(), config.port);
}

Read once, at the top, parsed into types. Everything below takes what it needs as a parameter, and a missing variable fails at startup instead of on the first request that reaches that code path.

Silence it

#![allow(unused)]
fn main() {
// rabot: allow(ambient-config) RUST_LOG is the logger's own contract, read by the logging crate
}

Tests

Your tests are either fast or honest. Real in-memory implementations are both. Mocks are neither.

Article: Tests

Every test mocked find_by_email to return None, because that is what the assertion needed. The unique constraint fired in production. The mock did exactly what it was told; that is the problem. A MemDatabase that enforces the same constraints in a HashMap is fast, honest, and useful outside the test suite.

Two rules: generated mocks, and tests skipped without a reason.

mock-usage

Level: warn · Article: Tests

Mocks test your assumptions. Real implementations test your code. Those are not the same test.

What it checks

use mockall, faux, mry, unimock or mockall_double; the #[automock] attribute; the mock! macro. This rule is about tests, so it is not relaxed in test code.

Don’t

#![allow(unused)]
fn main() {
use mockall::automock;

#[automock]
trait Database {
    fn find_by_email(&self, email: &Email) -> Option<User>;
}

#[test]
fn registers_a_new_user() {
    let mut db = MockDatabase::new();
    db.expect_find_by_email().return_const(None); // "no duplicate, go ahead"
    assert!(Users::new(db).register(Email::parse("ada@example.com")).is_ok());
}
}

The unique constraint on email fires in production. The mock never knew what the database contained, because it was not a database.

Do

#![allow(unused)]
fn main() {
struct MemDatabase {
    users: Mutex<HashMap<UserId, User>>,
}

impl Database for MemDatabase {
    fn find_by_email(&self, email: &Email) -> Option<User> {
        let users = self.users.lock().unwrap_or_else(PoisonError::into_inner);
        users.values().find(|user| user.email == *email).cloned()
    }

    fn insert(&self, user: NewUser) -> Result<User, DbError> {
        let mut users = self.users.lock().unwrap_or_else(PoisonError::into_inner);
        if users.values().any(|existing| existing.email == user.email) {
            return Err(DbError::UniqueViolation("email"));
        }
        let user = User::from(user);
        users.insert(user.id.clone(), user.clone());
        Ok(user)
    }
}

#[test]
fn rejects_a_duplicate_email() {
    let db = MemDatabase::default();
    db.insert(NewUser::named("ada", "ada@example.com")).unwrap();
    let duplicate = db.insert(NewUser::named("ada again", "ada@example.com"));
    assert!(matches!(duplicate, Err(DbError::UniqueViolation("email"))));
}
}

Two hours once per dependency. It enforces the same constraints, runs in milliseconds, and earns its place: local dev, seeding, CI without Docker.

Silence it

#![allow(unused)]
fn main() {
// rabot: allow-file(mock-usage) legacy suite, replaced by MemGateway under TEST-88
}

ignored-test

Level: warn · Article: Tests

The week the senior engineer who understood the mock setup leaves the company and nobody knows why three tests are marked .skip.

What it checks

#[ignore] without a reason. #[ignore = "..."] is the documented exception and never fires.

Don’t

#![allow(unused)]
fn main() {
#[ignore]
#[test]
fn transfers_between_accounts() {
    let bank = Bank::sandbox();
    bank.transfer(Account::Alice, Account::Bob, Money::cents(500));
    assert_eq!(bank.balance(Account::Bob), Money::cents(500));
}
}

Do

#![allow(unused)]
fn main() {
#[ignore = "needs the payments sandbox; run nightly, see PAY-431"]
#[test]
fn transfers_between_accounts() {
    let bank = Bank::sandbox();
    bank.transfer(Account::Alice, Account::Bob, Money::cents(500));
    assert_eq!(bank.balance(Account::Bob), Money::cents(500));
}
}

The reason says whether the test may run again, and who to ask.

Silence it

Add the reason. That is the fix.

ambient-time

Level: warn · Article: Tests

Some things are genuinely hard to test: the clock, random number generators, external HTTP calls. The instinct is to mock them. The right move is to make them injectable.

What it checks

A call to SystemTime::now(), Instant::now(), Utc::now(), Local::now(), OffsetDateTime::now_utc() or the like anywhere but main. The wall clock is a dependency, and this one is hidden from every signature that uses it.

Don’t

#![allow(unused)]
fn main() {
impl Session {
    fn create(user_id: UserId) -> Session {
        let now = Utc::now();
        Session {
            created_at: now,
            expires_at: now + Duration::hours(1),
            user_id,
        }
    }
}
}

The test for “expires one hour after creation” has to compute the current hour, or sleep, or give up.

Do

#![allow(unused)]
fn main() {
trait Clock: Send + Sync {
    fn now(&self) -> DateTime<Utc>;
}

struct Sessions<C: Clock> {
    clock: C,
}

impl<C: Clock> Sessions<C> {
    fn create(&self, user_id: UserId) -> Session {
        let now = self.clock.now();
        Session {
            created_at: now,
            expires_at: now + Duration::hours(1),
            user_id,
        }
    }
}

#[test]
fn expires_one_hour_after_creation() {
    let clock = FixedClock(Utc.with_ymd_and_hms(2024, 1, 15, 12, 0, 0).unwrap());
    let session = Sessions { clock }.create(UserId::new());
    assert_eq!(session.expires_at.hour(), 13);
}
}

The bar for injection: would it be useful outside tests? A Clock is. You freeze time in staging, simulate midnight rollover in a demo, replay a historical scenario.

Silence it

#![allow(unused)]
fn main() {
let started = Instant::now(); // rabot: allow(ambient-time) request timing for the log line; nothing branches on it
}

ambient-randomness

Level: warn · Article: Tests

What it checks

rand::random(), rand::thread_rng(), rand::rng(), StdRng::from_entropy() and friends, anywhere but main. A global generator makes the code correct on average and impossible to replay when it is not.

Don’t

#![allow(unused)]
fn main() {
fn pick_winner(entries: &[Entry]) -> &Entry {
    &entries[rand::thread_rng().gen_range(0..entries.len())]
}
}

The bug report says “the same person won twice”. You cannot reproduce it.

Do

fn pick_winner<'a>(entries: &'a [Entry], rng: &mut impl Rng) -> &'a Entry {
    &entries[rng.gen_range(0..entries.len())]
}

fn main() {
    // one generator, seeded once, handed down to everything that draws
    let mut rng = StdRng::from_entropy();
    Raffle::open(&mut rng).run();
}

#[test]
fn the_draw_is_reproducible() {
    // the same seed, the same winner, every run
    let entries = [Entry::new("ada"), Entry::new("grace")];
    let mut rng = StdRng::seed_from_u64(42);
    assert_eq!(pick_winner(&entries, &mut rng).name(), "grace");
}

Silence it

#![allow(unused)]
fn main() {
// rabot: allow(ambient-randomness) jitter on a retry delay; the exact value never matters
let jitter = rand::random::<u64>() % 50;
}

sleep-in-tests

Level: warn · Article: Tests

Developers who re-run flaky tests twice before investigating.

What it checks

thread::sleep, tokio::time::sleep or task::sleep inside test code. This rule fires only in tests; sleeping in production code is a different question.

Don’t

#![allow(unused)]
fn main() {
#[tokio::test]
async fn delivers_the_event() {
    let (bus, subscriber) = Bus::with_subscriber();
    bus.publish(Event::UserBanned);
    tokio::time::sleep(Duration::from_millis(50)).await; // "enough time"
    assert_eq!(subscriber.received(), vec![Event::UserBanned]);
}
}

Fifty milliseconds is enough on your laptop. On a loaded CI runner it is not, once a week, and someone adds a zero.

Do

#![allow(unused)]
fn main() {
#[tokio::test]
async fn delivers_the_event() {
    let (bus, mut subscriber) = Bus::with_subscriber();
    bus.publish(Event::UserBanned);
    let received = subscriber.next().await; // waits for the event, not for time
    assert_eq!(received, Some(Event::UserBanned));
}
}

When the code under test measures time, inject the clock and advance it: clock.advance(Duration::from_secs(3600)) is instant and exact.

Silence it

#![allow(unused)]
fn main() {
// rabot: allow(sleep-in-tests) exercises the real timeout path against the in-process server
}

Comments

Delete the comment. Fix the code.

Article: Comments

A comment that explains what code does is a confession: the code had something to say and could not say it. Now there are two representations of the same thing, and they will drift. // Get active users still said so six months after the function started returning banned ones.

Three rules for the comments a linter can recognise: code that was commented out instead of deleted, a TODO that says nothing, and a function narrated by section headers.

commented-out-code

Level: warn · Article: Comments

You have git. There is no temporary.

What it checks

A comment block that parses as Rust and carries syntax prose does not use (;, {, (), =, ::, ->). Doc comments are never checked; a comment can legitimately show code.

Don’t

#![allow(unused)]
fn main() {
fn total(items: &[Item]) -> Money {
    // let discount = apply_coupon(&items);
    // items.iter().map(|i| i.price - discount).sum()
    items.iter().map(|i| i.price).sum()
}
}

It creates noise, confuses the reader about what runs, and never gets cleaned up.

Do

#![allow(unused)]
fn main() {
fn total(items: &[Item]) -> Money {
    items.iter().map(|i| i.price).sum()
}
}

If you need it back, git log exists. If you cannot find it there, you did not need it.

Silence it

#![allow(unused)]
fn main() {
// rabot: allow(commented-out-code) the two lines below are the shape of the RFC-12 payload, kept for reference
}

vague-todo

Level: warn · Article: Comments

A TODO without context is noise with a timestamp. It will sit there for three years, mocking every developer who reads it.

What it checks

A TODO, FIXME, XXX or HACK comment with fewer than 6 words after the marker and no reference (a ticket like PERF-112 or #4521, or a URL).

Don’t

#![allow(unused)]
fn main() {
impl Users {
    fn find_by_email(&self, email: &Email) -> Option<&User> {
        // TODO: refactor this
        // FIXME
        self.users.iter().find(|user| user.email == *email)
    }
}
}

Refactor what? Why? When?

Do

#![allow(unused)]
fn main() {
impl Users {
    // TODO: this linear scan works at current scale (~500 users) but will
    // need an index once we hit the enterprise tier. See PERF-112.
    fn find_by_email(&self, email: &Email) -> Option<&User> {
        self.users.iter().find(|user| user.email == *email)
    }
}
}

A known tradeoff and a pointer to the follow-up. That is something the code cannot say.

Options

[thresholds]
vague-todo-min-words = 6

Silence it

Write the sentence. If there is nothing to say, delete the TODO.

sectioned-function

Level: warn · Article: Comments

You have three functions trapped inside one. The comment is an informal table of contents for code that should have been split. The section headers become function names.

What it checks

A function body containing 3 or more leading comment blocks (a comment on its own line, introducing the code below it). Trailing comments beside code, TODO/FIXME/SAFETY notes and ticket links never count.

Don’t

#![allow(unused)]
fn main() {
impl Orders {
    fn process(&self, order: &mut Order) -> Result<(), ProcessError> {
        // step 1: validate
        if order.items.is_empty() {
            return Err(ProcessError::Empty);
        }
        // step 2: transform
        let total = order.items.iter().map(Item::price).sum();
        // step 3: persist
        self.store.save(order, total)
    }
}
}

Do

#![allow(unused)]
fn main() {
impl Orders {
    fn process(&self, order: &mut Order) -> Result<(), ProcessError> {
        order.validate()?;
        let total = order.total();
        self.store.save(order, total)
    }
}

impl Order {
    fn total(&self) -> Money {
        self.items.iter().map(Item::price).sum()
    }

    fn validate(&self) -> Result<(), ProcessError> {
        if self.items.is_empty() {
            return Err(ProcessError::Empty);
        }
        Ok(())
    }
}
}

Each header became a name. The function reads as the summary the comments were trying to be, and each piece can be tested on its own.

Options

[thresholds]
section-comments = 3

Silence it

#![allow(unused)]
fn main() {
// rabot: allow(sectioned-function) the protocol handshake is documented step by step against RFC 6455 §4
fn handshake(..) { .. }
}

Exceptions

The key: the exception is written somewhere. If it’s only in your head, it’s not an exception. It’s just chaos with better intentions.

rabot’s own mechanism has rules too. An allow comment must carry a reason and name a rule that exists. And a file rabot cannot parse is reported rather than skipped, so a syntax error never silently turns a check off.

undocumented-exception

Level: error

You can break the rule. You must document the exception.

What it checks

A // rabot: allow(..) or // rabot: allow-file(..) comment with nothing after the parenthesis.

Don’t

#![allow(unused)]
fn main() {
// rabot: allow(sorted-fields)
struct Connection {
    guard: Guard,
    pool: Pool,
}
}

The comment silences nothing. It is reported as an error, and the rule it tried to allow still fires.

Do

#![allow(unused)]
fn main() {
// rabot: allow(sorted-fields) drop order matters: the guard must release before the pool
struct Connection {
    guard: Guard,
    pool: Pool,
}
}

The sentence is the point. The next reader gets the reason instead of a mystery.

unknown-rule

Level: error

What it checks

A // rabot: allow(..) comment naming a rule rabot does not have, an unknown directive after rabot:, or a missing closing parenthesis.

Don’t

#![allow(unused)]
fn main() {
// rabot: allow(sort-fields) drop order matters: the guard must release before the pool
struct Connection {
    guard: Guard,
    pool: Pool,
}
}

A typo would otherwise be a silent no-op: the comment looks like an exception and does nothing.

Do

#![allow(unused)]
fn main() {
// rabot: allow(sorted-fields) drop order matters: the guard must release before the pool
struct Connection {
    guard: Guard,
    pool: Pool,
}
}

rabot rules lists every name.

syntax-error

Level: error

What it checks

A file that does not parse as Rust. rabot reports it and moves on to the next file, so a broken file never silently disables checking, and never gets rewritten by rabot fmt.

What to do

Fix the file; cargo check says where. If it is generated or vendored code that is not meant to parse on its own, exclude it:

[files]
exclude = ["src/generated"]