Skip to content

S38: pub on everything

S38Severity: Low

Every field and function is public, so the API has no boundary.

Clippy: no lint finds this. Look for it in review.

An account with an overdraft rule. Every field and every method is pub:

pub struct Account {
pub balance_cents: i64,
pub overdraft_limit_cents: i64,
pub history: Vec<i64>,
}
impl Account {
pub fn withdraw(&mut self, cents: i64) -> bool {
if self.balance_cents - cents < -self.overdraft_limit_cents {
return false;
}
self.apply(-cents);
true
}
pub fn apply(&mut self, delta: i64) {
self.balance_cents += delta;
self.history.push(delta);
}
}
  • It fixes privacy errors. When code in another module cannot reach a field (error E0616) or a function (E0603), adding pub is the quickest fix. The agent adds it and moves on.
  • It is the opposite habit from Java, applied by mistake. In Java, a field without a modifier is visible in its package. In Rust, an item without pub is private to its module. An agent that hits “private” errors often makes everything pub to be safe.
  • Generated examples are small. In a one-file example, pub costs nothing. In a real crate, it is part of the API.
  • The rule is not enforced. withdraw checks the overdraft limit. But balance_cents is pub, so any code can set it to any value. apply is pub, so any code can change the balance without a check. history is pub, so the history can disagree with the balance. The test worse_lets_any_code_break_the_rules sets the balance directly and calls apply, and after that the history no longer matches the balance.
  • A negative cents is a deposit. withdraw(-500) adds money, because the parameter is i64. Nothing in the type stops it.
  • The API has no boundary. In a library, every pub item is a promise. Changing or removing it later is a breaking change for users of the crate.
  • Reviewers cannot see what is internal. When everything is pub, a reader cannot tell the entry points from the helpers.
  1. WorseAll fields pub, a pub helper that skips the check, and i64 amounts that can be negative.

    pub struct Account {
    pub balance_cents: i64,
    pub overdraft_limit_cents: i64,
    pub history: Vec<i64>,
    }
    impl Account {
    pub fn withdraw(&mut self, cents: i64) -> bool {
    if self.balance_cents - cents < -self.overdraft_limit_cents {
    return false;
    }
    self.apply(-cents);
    true
    }
    pub fn apply(&mut self, delta: i64) {
    self.balance_cents += delta;
    self.history.push(delta);
    }
    }
  2. BetterPrivate fields, read-only accessors, a private apply, and u32 amounts. withdraw returns a Result.

    #[derive(Debug, Default)]
    pub struct Account {
    balance_cents: i64,
    overdraft_limit_cents: i64,
    history: Vec<i64>,
    }
    #[derive(Debug, PartialEq, Eq)]
    pub struct LimitExceeded;
    impl Account {
    pub fn new(overdraft_limit_cents: i64) -> Self {
    Self {
    overdraft_limit_cents,
    ..Self::default()
    }
    }
    pub fn balance_cents(&self) -> i64 {
    self.balance_cents
    }
    pub fn history(&self) -> &[i64] {
    &self.history // callers can read, not change
    }
    pub fn deposit(&mut self, cents: u32) {
    self.apply(i64::from(cents));
    }
    pub fn withdraw(&mut self, cents: u32) -> Result<(), LimitExceeded> {
    let cents = i64::from(cents);
    if self.balance_cents - cents < -self.overdraft_limit_cents {
    return Err(LimitExceeded);
    }
    self.apply(-cents);
    Ok(())
    }
    fn apply(&mut self, delta: i64) {
    // private: the only way to change the balance, and it always records history
    self.balance_cents += delta;
    self.history.push(delta);
    }
    }

The “better” Account has one way to change the balance, apply, and it is private. Callers can only deposit and withdraw, and both go through apply, so the history always matches. history() returns &[i64]: callers can read it, but not change it. The u32 parameters make negative amounts impossible to pass.

  • Plain data types with no rules: a DTO, a configuration struct, a row from a database. Public fields are correct there, and getters are noise. See S12: Getters and setters for every field.
  • Binary crates (applications), where nothing outside the crate uses the API. pub still documents the entry points of each module, but pub(crate) and pub mean the same in practice.
  • Test helpers, in a #[cfg(test)] module.
  • No Clippy lint reports this pattern.
  • For each struct with pub fields, ask: is there a rule that connects the fields, or that limits a value? If yes, those fields must be private.
  • Look for pub fn helpers that skip a check that another pub fn makes.
  • In a library, cargo doc shows the public API. A page full of helpers and internal types is the smell.
  • The unreachable_pub rustc lint (allowed by default) reports pub items that cannot be reached from outside the crate, which pub(crate) should mark.
Make the fields of Account private and the apply helper private. Keep pub
only for the operations that callers need: new, deposit, withdraw, and
read-only accessors. Accessors return a value or a borrow (&[i64]), never
&mut. Use unsigned types for amounts that cannot be negative, and return
a Result from withdraw. Use pub(crate) for items that only this crate uses.
Run the tests.

Related: S12: Getters and setters for every field, Structs and impl, and Modules, crates, and Cargo.