Professional Rust Review

Professional Rust Review

Deep code surgery using a field guide extracted from ripgrep (46K LOC), clap (41K LOC), parsel (6K LOC), and ring_api (2.3K LOC). Not design patterns. The actual craft — line-by-line decisions that separate code that rots from code that lives.

When to Use

• Reviewing Rust code for quality (any crate, any size)
• Writing new Rust types, traits, or modules and want professional patterns
• Auditing a crate for scaling problems before they compound
• Reviewing or designing crate/module organization and structure
• User says "review", "judge", "audit", "improve" about Rust code

References

How It Works

Two review modes. Pick based on what the user asks for, or run both.

• Code surgery — line-level craft quality. Uses references/professional-rust.md.
• Structural review — module layout, privacy design, dependency flow. Uses references/codebase-organization.md.

Delegate an opus-level worker with the relevant guide as rubric. Do NOT summarize the guide in the prompt — the worker reads it directly.

Step 1: Identify the Target

Determine which Rust files or crates to review. Use fd -e rs and wc -l to scope the work. Exclude auto-generated files (bindings, protobuf output).

Step 2: Delegate the crew

Delegate using teams with anthropic/claude-opus-4-6.

Step 2a.: Delegate the Surgeon

The worker must:

1. Read references/professional-rust.md completely first 2. Read the target code broadly before diving deep 3. Find specific locations where applying a principle materially changes the code 4. Show BEFORE and AFTER code for each finding 5. Explain why the change matters HERE, not in general

Critical instruction quality rules:

• "Find ALL specific locations" — not "check all 47 principles"
• "Show the exact current code and the exact rewritten code" — not "this pattern is missing"
• "Explain what breaks as the codebase grows without this change" — not "best practice says X"
• "Do NOT produce a checklist. Do NOT say '❌ Missing'."

Step 2b.: Delegate the Zen Master

The worker must:

1. Read references/codebase-organization.md completely first 2. Read the target code broadly before diving deep (not just filenames) 3. Find specific locations where applying a principle materially improves the architecture 4. Show BEFORE and AFTER for each finding 5. Explain why the change matters HERE, not in general

Critical instruction quality rules:

• "Find ALL specific locations" — not "check all principles"
• "Show the current state VS acceptable end state
• "Explain what breaks as the codebase grows without this change" — not "best practice says X"
• "Do NOT produce a checklist. Do NOT say '❌ Missing'."

Step 3: Formulate the Dispatch

Dispatch both workers in parallel using a single teams call:

teams(action: 'delegate', tasks: [
  {
    text: '<surgeon request — see template A below>',
    assignee: 'rust-surgeon',
    model: 'anthropic/claude-opus-4-6'
  },
  {
    text: '<zen-master request — see template B below>',
    assignee: 'rust-zen-master',
    model: 'anthropic/claude-opus-4-6'
  }
])

Template A: Surgeon

Fill in {placeholders}:

# Deep Rust Code Surgery

Read the professional Rust field guide at {skill_dir}/references/professional-rust.md completely first. It contains 47 principles with reasoning and failure modes.

## Target
{description of crates/files to review, with paths}
{any files to EXCLUDE like auto-generated bindings}

## Rules
- Do NOT produce a checklist. Do NOT say '❌ Missing'.
- Do NOT list principles that don't apply.
- Every finding must include BEFORE code and AFTER code from the actual codebase.
- The AFTER code must compile (or be obviously close). No pseudocode.
- Focus on changes that affect SCALABILITY — what makes adding the next feature harder or easier.

## What to Find
Find ALL locations where applying a principle would materially change the code. For each:

1. Show the EXACT current code (file path, line range, the actual code)
2. Explain what principle applies and WHY it matters HERE (not in general)
3. Show the REWRITTEN code — the actual diff
4. Explain what breaks or degrades without the change as the codebase grows

## What to Look For
Don't grep for keywords. READ the code and find:

- A function that takes `String` where `&str` or `impl Into<>` would prevent unnecessary allocation
- An enum matched on externally when methods on the enum would centralize logic
- A struct with `pub` fields mutated from 3 modules when encapsulation would help
- A `HashMap<u64, X>` where a newtype key would prevent mixing up ID types
- A `Result<(), String>` that forces callers to parse strings to understand failures
- A function returning `Option<T>` without `#[must_use]` where callers forget to check
- A hot loop calling a cross-crate function without `#[inline]`
- A type constructed once and never mutated with `String` fields that should be `Box<str>`
- An `.unwrap()` that will panic in production under specific conditions
- A 200+ line function that should be 5 methods on a struct

Read broadly first. Then dive deep.

## Output
Write to {output_path}. Structure each finding as:

# Surgery N: [descriptive title]
**File:** path:line_range
**Principle:** §N — [name]
**Why here:** [specific reasoning for THIS code]

### Before

[actual current code]

### After

[rewritten code]

**What this prevents:** [specific scaling problem avoided]

Order by impact (most impactful first). Do NOT create tickets.

Template B: Zen Master

Fill in {placeholders}:

# Rust Codebase Structural Review

Read the codebase organization guide at {skill_dir}/references/codebase-organization.md completely first. It contains principles for module layout, privacy, dependency direction, and encapsulation with concrete patterns.

## Target
{description of crates/workspace to review, with paths}
{workspace layout if multi-crate}

## Rules
- Do NOT produce a checklist. Do NOT say '❌ Missing'.
- Do NOT list principles that don't apply.
- Every finding must reference specific files, modules, and dependency paths.
- Proposed restructuring must be concrete — show the directory tree or module moves.
- Focus on what makes the NEXT feature harder to add or the next developer slower to onboard.

## What to Find
Find ALL structural problems in this codebase. For each:

1. Show the CURRENT structure (module tree, file paths, pub/use chains)
2. Explain what organizational principle applies and WHY it matters HERE
3. Show the PROPOSED structure — the actual module moves, visibility changes, or re-exports
4. Explain what breaks or degrades without the change as the codebase grows

## What to Look For
Map the codebase first. THEN find:

- Modules organized by technical role (models/, utils/, helpers/) instead of by feature
- A module re-exporting everything with `pub use` — no facade, just a pass-through
- Cross-module dependency loops (A uses B, B uses A)
- `pub` fields on structs where `pub(crate)` or private + methods would enforce invariants
- A `utils.rs` or `helpers.rs` grab-bag that should be `std_ext/` mirroring std
- Tooling (benchmarks, fuzz targets, CLI tools) living inside the main crate instead of workspace members
- A deeply nested module path that should be facaded at a higher level
- Private items that are `pub` for no reason — check who actually imports them
- An `unsafe` block without a safety comment or justification
- A struct with a reset/clear/serialize method that doesn't destructure (risks missing new fields)

Read the full module tree first. Then dive deep.

## Output
Write to {output_path}. Structure each finding as:

# Finding N: [descriptive title]
**Location:** module path / file paths involved
**Principle:** [name from the guide]
**Why here:** [specific reasoning for THIS codebase]

### Current Structure
[actual module tree, dependency arrows, or visibility chain]

### Proposed Structure
[concrete restructuring — directory moves, visibility changes, new modules]

**What this prevents:** [specific scaling or maintainability problem avoided]

Order by impact (most impactful first). Do NOT create tickets.

Step 4: Present Results

When both workers return, synthesize findings from both into a single report. Summarize the top 5 findings from each worker in a table, then point to the full reports.

If the user wants to act on findings, create tickets for the top-priority items using tk.

---

What the Guides Cover (for reference, do not load into worker prompt)

The 47 principles span 10 areas:

Common Mistakes

Checklist mode: The worker produces a table of "present/absent" for each principle. Useless. The instructions explicitly forbid this — if the worker falls into checklist mode, the dispatch prompt needs sharpening.

Theoretical findings: "You could use Cow here." Without showing the current code, the rewrite, and why it matters for THIS codebase, it's noise.

Too many findings, too shallow: 15 deep findings beat 47 shallow ones. The worker should read broadly, then pick the 15 highest-impact sites and go deep on each.

Ignoring auto-generated code: Always exclude bindings, protobuf output, and other generated files from review scope.

Structuring a Rust Codebase

How to organize modules, manage privacy, and keep a complex Rust codebase navigable. Extracted from Datalust's Flare storage engine and Brandon's "Lasagna Code" talk on design and architecture in Rust.

---

Module = Privacy Boundary = Feature

Each module owns one feature. The module boundary is where privacy lives — not the struct, not the impl block. Everything inside a module can see everything else inside it. Everything outside sees only what you export.

Organize by reason to change, not by technical category. Don't make a models/ and a utils/. Make an ingest_buf/ and a page_index/.

Modules are Rust's smallest unit of privacy — equivalent to files in C, classes in Java, capitalization in Go. Implementations never affect privacy rules. A method on a struct still obeys the module boundary, not the impl block.

Start by writing mod inline whenever you need a stronger boundary. Extract to a file later by replacing the block with a semicolon.

---

The Facade Pattern

Expose a flat, simple API from a complex interior. Mark items pub inside a private mod. The parent decides what gets re-exported. Internals stay complex; the surface stays small.

src/
├── store/
│   ├── mod.rs              ← public API lives here (re-exports)
│   ├── ingest_buf/         ← private, facaded from root ingest_buf
│   │   ├── mod.rs
│   │   └── reader.rs
│   ├── stable_span/
│   │   ├── mod.rs
│   │   └── reader.rs
│   ├── nucleus/
│   │   ├── mod.rs
│   │   ├── read.rs
│   │   └── write.rs
│   └── mvcc/
│       ├── mod.rs
│       └── model.rs

Concrete example from Flare: root-level ingest_buf and stable_span modules contain the raw storage formats. store re-exports them as store::ingest_buf and store::stable_span. This seam absorbs internal API changes — the root module's implementation can shift without breakage leaking into store.

Treat root modules as if they were independent crates. This disciplines their public API and content.

---

Depth Hierarchy

Each level adds a privacy boundary. Code at depth N cannot see private items at depth N+1.

---

Privacy Ladder

From loosest to tightest — choose deliberately:

Default to private. Widen only when a consumer demands it. The smaller the holes in your API, the easier each new one is to reason about.

pub(crate) is fairly unique to Rust — few languages offer compilation-unit privacy. Use it as your working default for items that other modules in the crate need.

Why not `pub` everything? Discipline is hard. Making a conscious decision to publicize internals matters. The only thing stopping you from exposing everything is speed bumps you place for yourself.

---

Dependency Direction

store  →  ingest_buf
store  →  stable_span
store  →  mvcc
store  →  std_ext

ingest_buf  →  std_ext
stable_span →  std_ext

Arrows point one way: from orchestration toward infrastructure, from volatile toward stable. Implementation changes faster than definition. API pressure flows from the real world inward — the infrastructure that interacts with the outside world drives what the inner modules expose.

If you draw a loop, your layers are wrong. When module A requires changes in module B and module B requires changes in module A, the code is spaghetti regardless of how many files you split it across.

---

Cross-Cutting Utilities: Mirror std

Every codebase accumulates helpers. Without structure, they scatter or become a grab-bag. Mirror std's module layout in a std_ext module:

src/
├── std_ext/
│   ├── mod.rs
│   ├── fs.rs       ← extends std::fs
│   ├── io.rs       ← extends std::io (ResetRead, SizeRead, etc.)
│   ├── iter.rs     ← extends std::iter
│   ├── option.rs   ← extends Option
│   ├── result.rs   ← extends Result
│   ├── slice.rs
│   ├── sync.rs
│   └── vec.rs

Everything pub(crate). The mirror structure keeps utilities discoverable. When you need an extension trait for Option, you know to look in std_ext::option — not hunt through utils.rs or helpers.rs.

Extension traits are the typical form:

pub(crate) trait OptionMutExt<T> {
    fn map_mut<F>(&mut self, f: F) -> &mut Self
    where F: FnOnce(&mut T);
}

Some extensions are more fundamental — traits that extend std::io::Read for data sources offering additional guarantees (resettable readers, size-aware readers, readers that can detach from their source). These compose: a MemRead trait unifies contiguous and non-contiguous payloads behind a single interface, with an optimized fast path when bytes are contiguous.

---

Auditable unsafe via Macro

#![deny(unsafe_code)] at the crate root. Channel all unsafe through a custom macro that requires a written safety justification:

macro_rules! unsafe_block {
    ($reason:tt => $body:expr) => {{
        #[allow(unsafe_code)]
        let r = unsafe { $body };
        r
    }};
}

Forgetting the macro and writing raw unsafe { ... } produces a compile error. Every unsafe block carries a reason string explaining what invariants guarantee safety:

let mut read_into = unsafe_block!(
    "Our writers don't read from the uninitialized buffer" => {
        buf.bytes_mut()
    }
);

Extend the pattern with similar macros for unsafe functions and unsafe impl blocks.

---

Destructuring + deny(unused_variables) for Field Coverage

When a method must touch every field (reset, serialize, clone-with-changes), destructure the struct and deny unused variables at function scope:

fn reset(&mut self) -> Result<(), io::Error> {
    #![deny(unused_variables)]
    let Decompress {
        ref mut src,
        dec: ref _dec,           // explicitly unused
        ref mut compressed,
        ref mut decompressed,
        ref mut read_head,
    } = *self;

    src.reset()?;
    compressed.clear();
    decompressed.clear();
    *read_head = 0;
    Ok(())
}

Add a field to the struct later? The compiler forces you to handle it here. Bind irrelevant fields with _ prefix to signal deliberate omission.

---

Tooling Lives Outside the Main Crate

workspace/
├── flare/           ← the engine (single crate)
├── bench/           ← benchmarks
├── fuzz/            ← fuzz targets
└── tools/           ← diagnostics, inspectors

Benchmarks, fuzz targets, and diagnostic tools are separate crates in the same workspace. They depend on the main crate; the main crate does not know they exist.

---

Encapsulate Incrementally

Progress through these layers only as complexity demands — not before:

1. Raw data in a collection — no encapsulation, business logic bleeds everywhere. Even here, &mut signals tell you something about access patterns.

2. Functions in a module — operations are separated, but ownership of the underlying data still spreads. The module provides the privacy boundary.

3. Structs — tie data and behavior together. Keep fields pub(crate) for internal types you trust; make them private when invariants matter. A reasonable choice for internal types: exposing fields to trusted code eliminates boilerplate you don't need to write.

4. Traits — define only when you have motivation: reuse across implementations, testing mocks, or ecosystem interop. Not before. Use traits as early as necessary, but no earlier.

---

Trait Constraints: Where to Place Them

Push bounds like K: Hash + Eq onto the struct, not individual methods:

• Better error messages — users discover incompatibility at construction, not

months later when they call a method.

• Clearer documentation — the struct definition states what matters about its

type parameters.

Use method-level constraints only when a specific operation needs an additional bound (like Clone) that the struct does not universally require. In trait implementations, you cannot add method-level constraints — the entire impl block needs the where clause.

---

Separation of Orthogonal Concerns

When two things change for different reasons, put them in separate modules. Cache mechanics and cache metrics are orthogonal — one changes when you change storage strategy, the other when you change what you measure.

Each module owns its data and enforces its own invariants. The consumer module (metrics) depends on the producer module (cache) through its public API. The producer does not know the consumer exists.

Rust's zero-cost abstractions mean this separation costs nothing at runtime. The compiler inlines the boundaries away.

---

Traits as Type-Level Privacy

Modules hide items (code-level privacy). Traits hide concrete types (type-level privacy). Once CacheMetrics depends on a Cache trait instead of a concrete HashMap wrapper, it no longer knows or cares what backs the cache.

This gives single-direction implementation: the consumer knows the cache's shape but not its specific implementation. The "uses" line is isolated from the "implements" line.

---

Type System as Test Replacement

Encode structural invariants in types when possible:

struct CascadeCache {
    first: Box<dyn Cache>,        // at least one — guaranteed by the type
    rest: Vec<Box<dyn Cache>>,
}

A Vec<Box<dyn Cache>> alone requires tests to verify non-emptiness. The split representation makes the compiler enforce it. No test needed.

---

Summary

Professional Rust: A Field Guide

How to write Rust that scales for further development. Extracted from studying ripgrep (46K LOC), clap (41K LOC), parsel (6K LOC), and ring_api (2.3K LOC) — four codebases written by people who know what they're doing.

Not design patterns. Not architecture astronautics. The actual craft — the decisions that separate code that rots from code that lives.

---

Part I — Types

The foundation of professional Rust is type design. Every decision in this section prevents a class of bugs or eliminates a category of boilerplate.

1. Newtypes prevent misuse at zero cost

An amateur represents a match as (usize, usize) or start: usize, end: usize. A professional wraps it:

// ripgrep — crates/matcher/src/lib.rs
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
pub struct Match {
    start: usize,
    end: usize,
}

impl Match {
    pub fn new(start: usize, end: usize) -> Match {
        assert!(start <= end);    // checked once, trusted forever
        Match { start, end }
    }

    pub fn len(&self) -> usize { self.end - self.start }
    pub fn is_empty(&self) -> bool { self.start == self.end }

    pub fn offset(&self, amount: usize) -> Match {
        Match { start: self.start + amount, end: self.end + amount }
    }
}

// Integrates with native slice syntax: &haystack[m]
impl std::ops::Index<Match> for [u8] {
    type Output = [u8];
    fn index(&self, index: Match) -> &[u8] {
        &self[index.start..index.end]
    }
}

Match is Copy — zero overhead. The invariant start <= end is checked at construction. After that, every use site trusts it. The Index impl means you write &haystack[m] instead of &haystack[m.start..m.end].

Without the newtype, someone eventually passes (end, start) in the wrong order. The bug shows up as a panic in haystack[end..start] three functions away.

ring_api does the same for domain concepts:

// ring_api — src/job.rs
pub struct JobId(String);

You can't pass a PDB identifier where a job ID is expected. They're both strings. The newtype makes the compiler enforce the distinction.

When to newtype: When the raw type (usize, String, u8) could mean more than one thing in your domain. The cost is one struct declaration. The benefit: wrong usage doesn't compile.

2. Enums carry state — they replace boolean flags and if-cascades

Amateur:

fn search(pattern: &str, case_sensitive: bool, multiline: bool, word_boundary: bool) {
    if case_sensitive && multiline && word_boundary { ... }
    else if case_sensitive && multiline { ... }
    else if case_sensitive && word_boundary { ... }
    // ... 8 branches, growing with every new option
}

Professional — ripgrep:

// crates/core/search.rs
pub(crate) enum PatternMatcher {
    RustRegex(grep::regex::RegexMatcher),
    #[cfg(feature = "pcre2")]
    PCRE2(grep::pcre2::RegexMatcher),
}

Each variant holds a fully configured, compiled matcher. Case sensitivity, multiline, word boundaries — all baked in at construction time. The dispatch is one match:

match self.matcher {
    RustRegex(ref m) => search_path(m, searcher, printer, path),
    PCRE2(ref m)     => search_path(m, searcher, printer, path),
}

No booleans. No cascading ifs. Each variant carries its own state.

ripgrep's LineTerminator shows the same pattern at a smaller scale:

// crates/matcher/src/lib.rs
pub struct LineTerminator(LineTerminatorImp);   // public opaque wrapper

enum LineTerminatorImp {                        // private inner enum
    Byte(u8),
    CRLF,
}

impl LineTerminator {
    pub fn byte(byte: u8) -> LineTerminator {
        LineTerminator(LineTerminatorImp::Byte(byte))
    }

    pub fn crlf() -> LineTerminator {
        LineTerminator(LineTerminatorImp::CRLF)
    }

    pub fn as_byte(&self) -> u8 {
        match self.0 {
            LineTerminatorImp::Byte(byte) => byte,
            LineTerminatorImp::CRLF => b'\n',
        }
    }

    pub fn as_bytes(&self) -> &[u8] {
        match self.0 {
            LineTerminatorImp::Byte(ref byte) => std::slice::from_ref(byte),
            LineTerminatorImp::CRLF => &[b'\r', b'\n'],
        }
    }
}

Nobody scatters if line_ending == "\r\n" through the codebase. They call terminator.as_byte(). The logic lives on the type.

The scaling difference: With booleans, adding a third case means finding every if-else chain in the codebase. With an enum, you add a variant, and the compiler shows you every match that needs updating. The boolean version silently does the wrong thing for the new case. The enum version refuses to compile.

3. Hide the inner enum, expose methods

Notice that LineTerminator wraps LineTerminatorImp — the inner enum is private. Users interact through methods. This matters because:

• Adding a variant (say, Unicode for \u{2028}) is not a breaking change
• The internal representation can change freely
• Users can't write match on your internals and break when you evolve

clap's Str type takes this further — the inner representation changes based on a feature flag:

// clap_builder/src/builder/str.rs
pub struct Str { name: Inner }

// Without `string` feature (default):
pub(crate) struct Inner(pub(crate) &'static str);

// With `string` feature:
pub(crate) enum Inner {
    Static(&'static str),
    Owned(Box<str>),
}

Same public API. Completely different internal representation depending on compile-time configuration. Users see Str and its methods. They never know which Inner they're using.

The amateur mistake: pub enum LineTerminator { LF, CRLF }. Works until you need a third variant. Then every downstream match breaks. You reach for #[non_exhaustive], but now every consumer needs a _ => branch that panics when the new variant actually arrives.

4. Right-size your types

ripgrep needs a set of bytes (which bytes appear in a pattern, which are line terminators). An amateur uses HashSet<u8>. A professional:

// crates/matcher/src/lib.rs
pub struct ByteSet(BitSet);

#[derive(Clone, Copy)]
struct BitSet([u64; 4]);    // 256 bits = one bit per possible byte value

impl ByteSet {
    pub fn empty() -> ByteSet { ByteSet(BitSet([0; 4])) }
    pub fn full() -> ByteSet { ByteSet(BitSet([u64::MAX; 4])) }

    pub fn add(&mut self, byte: u8) {
        let bucket = byte / 64;
        let bit = byte % 64;
        (self.0).0[usize::from(bucket)] |= 1 << bit;
    }

    pub fn contains(&self, byte: u8) -> bool {
        let bucket = byte / 64;
        let bit = byte % 64;
        (self.0).0[usize::from(bucket)] & (1 << bit) != 0
    }
}

32 bytes on the stack. Copy. O(1) contains. No heap allocation. A HashSet<u8> is heap-allocated, carries 40+ bytes of metadata, hashes on every lookup, and for a set that's at most 256 elements, is slower in every dimension.

The principle: When the domain constrains the range (256 possible bytes, not billions), the type should reflect that constraint. Don't reach for the general-purpose container when a fixed-size one fits exactly.

5. Box<str> when you're done building

clap stores argument names as Box<str>, not String:

enum Inner {
    Static(&'static str),
    Owned(Box<str>),          // not String
}

// Construction:
pub(crate) fn from_string(name: String) -> Self {
    Self::Owned(name.into_boxed_str())
}

String is (pointer, length, capacity) — 24 bytes. Box<str> is (pointer, length) — 16 bytes. The difference is capacity — which matters when you're building the string (appending, pushing), but is wasted space once the string is done.

For a CLI with 50 arguments, each with a name and help string, that's 100 strings × 8 bytes = 800 bytes saved. Small. But it signals: "this string is finished, it won't be modified." The type communicates intent.

6. Cow means "probably won't allocate"

ripgrep's glob matcher:

// crates/globset/src/lib.rs
pub struct Candidate<'a> {
    path: Cow<'a, [u8]>,
    basename: Cow<'a, [u8]>,
    ext: Cow<'a, [u8]>,
}

On Unix, the path comes in as &[u8] and stays borrowed — zero allocation. On Windows, it needs conversion from UTF-16, so it allocates. Cow handles both without the Unix path paying for Windows's needs.

// crates/globset/src/pathutil.rs
pub(crate) fn normalize_path(path: Cow<'_, [u8]>) -> Cow<'_, [u8]> {
    // If already normalized, returns the borrow unchanged
    // If needs work, allocates and returns owned
}

When to use `Cow`: Data that's usually borrowed but sometimes needs transformation. Path normalization, string escaping, case conversion.

When NOT to: When you always own the data (just use Vec/String) or always borrow it (just use &[u8]/&str). Cow adds a branch on every dereference. Only worth it when the common path avoids allocation.

7. Work in bytes internally, strings at the boundary

ripgrep's entire Matcher trait operates on &[u8]:

fn find_at(&self, haystack: &[u8], at: usize) -> Result<Option<Match>, Self::Error>;

Not &str. Files on disk aren't always UTF-8. Filenames on Unix aren't always UTF-8. If your search tool takes &str, it silently skips every file with a non-UTF-8 byte — the user's file doesn't appear in results and they don't know why.

The printer converts to strings at the output boundary, using lossy conversion when needed. Internal processing is all bytes.

The rule: Your internal representation should be the least restrictive type that still lets you do the work. &[u8] can hold anything. &str can only hold valid UTF-8. Accept bytes inside, convert to strings only when you need to show something to a human.

---

Part II — Function Signatures

How you declare function parameters determines how pleasant your API is to use, how much code the compiler generates, and whether callers need to fight with ownership.

8. impl Into<T> in constructors, &T in methods

clap:

// Constructor: accepts anything convertible (stores it)
pub fn new(id: impl Into<Id>) -> Self { ... }

// Setter: accepts anything convertible (stores it)
pub fn arg(mut self, a: impl Into<Arg>) -> Self { ... }

// Query: borrows (just looking)
pub fn get_one<T>(&self, id: &str) -> Option<&T> { ... }
pub fn contains_id(&self, id: &str) -> bool { ... }

Constructors and setters take impl Into<T> — they need to store the value, and Into lets callers pass &str, String, Id, whatever converts. Query methods take &str — they just need to compare, not keep.

Why not `impl Into<T>` everywhere? Generics. Every distinct input type generates a new monomorphized copy of the function. For a constructor called once, fine. For a hot lookup called thousands of times, you want one concrete function.

ripgrep:

// Constructor: flexible input
fn new(rawargs: impl IntoIterator<Item = impl Into<OsString>>) -> Parser { ... }

Accepts &[&str], Vec<String>, std::env::args_os() — anything iterable with items convertible to OsString.

9. AsRef<T> for reading, Into<T> for storing

// ring_api — reads the file (doesn't store path)
pub fn with_pdb_file<P: AsRef<Path>>(file: P) -> Result<Self> { ... }

// ring_api — stores the string
pub fn file_name<T: Into<String>>(mut self, file_name: T) -> Self { ... }

// ripgrep — just needs to look at the path
pub fn new<P: AsRef<Path> + ?Sized>(path: &'a P) -> Candidate<'a> { ... }

The rule:

• `AsRef<T>` — "I need to look at it" (reading, comparing, hashing)
• `Into<T>` — "I'm going to keep it" (storing in a field)
• `&T` — "I know the exact type" (internal methods, hot paths, no generics)

10. ?Sized widens your API for free

pub fn new<P: AsRef<Path> + ?Sized>(path: &'a P) -> Candidate<'a>

Without ?Sized, you can't pass &Path directly — only references to sized types like &PathBuf. With ?Sized, both &Path and &PathBuf work. One word in the bound, broader API.

11. The monomorphization firewall

clap:

pub fn new(name: impl Into<Str>) -> Self {
    /// Non-generic to save code size.
    fn new_inner(name: Str) -> Command {
        Command { name, ..Default::default() }
    }
    new_inner(name.into())
}

The outer function is generic — accepts &str, String, Str. But it's one line: convert and delegate. The inner function is monomorphic — compiled once, regardless of how many types you pass to new().

Without this, Command has 30+ fields, and rustc generates a complete copy of the constructor for every input type. The inner function prevents that.

The rule: When a generic function has a large body, factor it into a non-generic inner function. The generic wrapper does ONLY the type conversion. Free at the source level, saves real binary size.

12. Return &mut Self from builders when reuse matters

ripgrep's builders return &mut Self:

pub fn line_terminator(&mut self, lt: LineTerminator) -> &mut SearcherBuilder {
    self.config.line_term = lt;
    self
}

This means you can reuse a builder:

let mut builder = SearcherBuilder::new();
builder.line_terminator(LineTerminator::crlf());

let searcher_a = builder.build();          // first config
builder.encoding(Some(Encoding::utf8()));  // tweak
let searcher_b = builder.build();          // second config, builder still alive

clap's builders consume self:

pub fn arg(mut self, a: impl Into<Arg>) -> Self { ... }

This makes the chain the only way to use it — you can't reuse a half-built Command. Good for clap's use case (you build one CLI definition and parse).

The decision: &mut Self when callers might build multiple variants from one builder. self when the builder is one-shot and you want to prevent partial use. Pick based on your expected usage, not a dogma.

13. #[must_use] on every method that returns Self

clap has 125+ #[must_use] annotations:

#[must_use]
pub fn arg(mut self, a: impl Into<Arg>) -> Self {
    self.arg_internal(a.into());
    self
}

Without it, this compiles silently and does nothing:

let cmd = Command::new("app");
cmd.arg(Arg::new("input"));   // DROPPED. cmd is unchanged.

With #[must_use], the compiler warns. Zero cost. Catches a bug that's invisible at the call site and manifests as "my flag doesn't work" far away.

---

Part III — Traits

Traits are Rust's core abstraction mechanism. Professional trait design is about knowing what to require, what to provide, and when not to use a trait at all.

14. Minimum required surface, maximum provided convenience

ripgrep's Matcher trait has 2 required methods:

pub trait Matcher {
    type Captures: Captures;
    type Error: std::fmt::Display;

    fn find_at(&self, haystack: &[u8], at: usize) -> Result<Option<Match>, Self::Error>;
    fn new_captures(&self) -> Result<Self::Captures, Self::Error>;

    // ~15 provided methods built on the two above:
    fn find(&self, haystack: &[u8]) -> Result<Option<Match>, Self::Error> { ... }
    fn find_iter<F>(&self, haystack: &[u8], matched: F) -> Result<(), Self::Error> { ... }
    fn captures_at(&self, ...) -> Result<bool, Self::Error> { ... }
    fn replace_with_captures<F>(&self, ...) -> Result<(), Self::Error> { ... }
    // ...
}

Implementing a new regex backend requires 2 methods. Everything else comes for free. This is why ripgrep supports both Rust regex and PCRE2 — the barrier to adding a backend is trivially low.

ripgrep's Sink trait: 1 required method, 5 with defaults:

pub trait Sink {
    type Error: SinkError;

    fn matched(&mut self, searcher: &Searcher, mat: &SinkMatch<'_>) -> Result<bool, Self::Error>;

    // All optional with sensible defaults:
    fn context(&mut self, ...) -> Result<bool, Self::Error> { Ok(true) }
    fn context_break(&mut self, ...) -> Result<bool, Self::Error> { Ok(true) }
    fn binary_data(&mut self, ...) -> Result<bool, Self::Error> { Ok(true) }
    fn begin(&mut self, ...) -> Result<bool, Self::Error> { Ok(true) }
    fn finish(&mut self, ...) -> Result<(), Self::Error> { Ok(()) }
}

A minimal sink is 4 lines. A full-featured sink overrides what it needs. Progressive disclosure — start simple, add complexity only where you need it.

The failure mode: A trait with 12 required methods. Nobody implements it except you. The trait becomes ceremony — an abstraction that abstracts nothing.

15. Choose internal or external iteration based on your weakest implementor

ripgrep's Matcher uses callback-based (push) iteration:

fn find_iter<F>(&self, haystack: &[u8], matched: F) -> Result<(), Self::Error>
where F: FnMut(Match) -> bool;

Not an iterator. Why? PCRE2 is a C library that uses internal iteration. If Matcher required returning an Iterator, PCRE2 couldn't implement it without deep contortions (channels, unsafe hacks). Rust's regex crate can do either style.

The rule: Design trait methods for the most constrained implementor you need to support. Convenience for consumers comes from adapters and provided methods, not from constraining the interface.

16. Generics on hot paths, trait objects on cold paths

ripgrep makes this choice deliberately:

// Hot: generic — compiler inlines find_at into the search loop
fn search<M: Matcher>(matcher: &M, haystack: &[u8]) { ... }

// Cold: trait object — one copy of the parsing code, N flag implementations
static FLAGS: &[&dyn Flag] = &[
    &CaseSensitive,
    &Color,
    &Encoding,
    // ...
];

The decision: Generics when the code runs per-item in a hot loop (you need inlining). Trait objects when the code runs once or per-thread (you want one copy of the function, not N monomorphized copies).

17. Closure wrappers as ergonomic adapters

ripgrep's Sink trait is powerful but verbose. For simple cases:

// The wrapper: a tuple struct holding a closure
pub struct UTF8<F>(pub F)
where
    F: FnMut(u64, &str) -> Result<bool, io::Error>;

// It implements Sink — the search engine doesn't know the difference
impl<F> Sink for UTF8<F>
where
    F: FnMut(u64, &str) -> Result<bool, io::Error>,
{
    type Error = io::Error;

    fn matched(&mut self, _searcher: &Searcher, mat: &SinkMatch<'_>) -> Result<bool, io::Error> {
        let matched = std::str::from_utf8(mat.bytes()).map_err(io::Error::error_message)?;
        let line_number = mat.line_number().ok_or_else(|| ...)?;
        (self.0)(line_number, matched)
    }
}

Usage:

searcher.search_path(&matcher, path, sinks::UTF8(|lnum, line| {
    println!("{lnum}: {line}");
    Ok(true)
}))?;

The closure is zero-cost — no boxing, no dynamic dispatch. The compiler inlines it. The user writes 3 lines instead of implementing a 6-method trait.

Three variants exist: UTF8 (strict), Lossy (replaces invalid UTF-8), Bytes (raw &[u8]). Each wraps a closure and implements Sink.

The pattern: Powerful trait for experts. Closure adapter for common cases. Same search engine, same code path, same performance. Two entry points.

18. Associated types for compile-time pairing

ring_api:

pub trait Request {
    type Body: Serialize;
    type Response: for<'de> Deserialize<'de>;

    fn endpoint(&self) -> Cow<str>;
    fn body(&self) -> RequestBody<&Self::Body> { Default::default() }
}

client.send(SubmitId { ... }) returns Result<SubmitResponse>. Not Result<serde_json::Value>. The response type is determined by the request type at compile time.

// You literally cannot mix up request and response types:
let response: SubmitResponse = client.send(submit_request)?;    // ✓
let response: StatusResponse = client.send(submit_request)?;    // compile error

When to use associated types: When two types are always paired (request/response, encoder/decoder, parser/ast-node). The pairing becomes a compile-time guarantee, not a runtime convention.

clap's trait hierarchy does the same:

pub trait Parser: FromArgMatches + CommandFactory + Sized {
    fn parse() -> Self {
        let matches = <Self as CommandFactory>::command().get_matches();
        <Self as FromArgMatches>::from_arg_matches_mut(&mut matches)
            .map_err(format_error::<Self>)
            .unwrap_or_else(|e| e.exit())
    }
    // try_parse, parse_from, etc. — all provided, built from the two supertraits
}

Parser composes CommandFactory (knows how to describe itself) with FromArgMatches (knows how to populate itself). The parse() method chains them — provided, not required.

19. The derive-generates-builder-calls pattern

clap's derive macros generate code that calls the builder API:

// What #[derive(Args)] generates:
impl clap::Args for Cli {
    fn augment_args(cmd: Command) -> Command {
        cmd.arg(
            Arg::new("verbose")
                .short('v')
                .action(ArgAction::SetTrue)
        )
    }
}

There is no "derive runtime." The derive macro is a code generator targeting the same builder API a human would write. This means:

• Builder and derive can be mixed in the same app
• One runtime, two entry points
• Bug fixes to the builder fix derive behavior automatically
• Users who outgrow derive drop down to builder without rewriting

The failure mode: Two separate systems — derive path with its own parsing logic, builder path with different logic. Two runtimes, two bug surfaces, permanent feature-parity struggle.

---

Part IV — Errors

20. Custom errors in libraries, anyhow in binaries

ripgrep:

• Library crates: Hand-written Error + ErrorKind enum. Typed, matchable.
• Binary crate: anyhow::Result for ergonomic error chaining.

Library consumers can match on ErrorKind::Regex vs ErrorKind::InvalidLineTerminator. The binary can eprintln!("{:#}", err) and get a nice chain.

The pattern — Error wrapping ErrorKind:

// Every library crate follows this structure
pub struct Error {
    kind: ErrorKind,
}

#[non_exhaustive]    // can add variants without breaking semver
pub enum ErrorKind {
    Regex(String),
    NotAllowed(String),
    InvalidLineTerminator(u8),
}

impl std::error::Error for Error {}
impl std::fmt::Display for Error { /* match on kind */ }

The Error struct hides internals. ErrorKind is #[non_exhaustive] — future variants don't break downstream.

21. Separate user errors from programmer errors

clap:

• User errors (bad CLI input) → Result<_, Error> with nice messages,

suggestions, usage

• Programmer errors (bad API usage) → debug_assert! panics

A user typing --verbosee gets: "unexpected argument '--verbosee', did you mean '--verbose'?"

A programmer writing duplicate arg names gets a panic in debug builds:

// clap_builder/src/builder/debug_asserts.rs — 818 lines
// Runs only in #[cfg(debug_assertions)]
pub(crate) fn assert_app(cmd: &Command) {
    // Checks for duplicate argument names
    // Checks for conflicting short/long flags
    // Checks positional ordering rules
    // ...
}

Zero cost in release. Comprehensive invariant checking in debug. Panics point to the programmer's code via #[track_caller].

22. Structured error context, not format strings

clap's errors carry data, not strings:

pub enum ContextKind {
    InvalidArg,
    InvalidValue,
    SuggestedArg,       // "did you mean --verbose?"
    SuggestedValue,
    Usage,
}

pub enum ContextValue {
    String(String),
    Strings(Vec<String>),
    Number(isize),
    StyledStr(StyledStr),
}

A formatter reads this structure and renders it. Different formatters produce different output. Tests can assert err.get(ContextKind::SuggestedArg) without parsing strings.

The failure mode: format!("expected one of: {}", values.join(", ")). You can't programmatically extract the valid values. You can't test the suggestion without parsing the human-readable string.

23. Mechanical error context, not manual

parsel's derive macros wrap every field parse automatically:

// Generated code — you never write this by hand
#field_name: input.parse().map_err(|cause| {
    ::parsel::util::chain_error(cause, "MyEnum", "MyVariant", "field_name")
})?

You never forget to add context because you never add it manually. The derive does it for every field, every variant, every time.

When multiple alternatives fail (parsing an enum), report the one that consumed the most input:

Err(max_by_key(left_error, right_error, |e| e.span().end()))

"Expected ; after expression" (from the variant that got farthest) is more useful than "expected keyword if" (from the first variant tried).

24. Return bool from callbacks for flow control

ripgrep's Sink::matched returns Result<bool, Error>:

• Ok(true) — keep searching
• Ok(false) — stop (enough results)
• Err(e) — stop (something broke)

The walker uses the same: WalkState::Continue / WalkState::Quit.

This is backpressure. The consumer controls the producer. No channels, no shared state. The search loop checks the return value and stops.

The amateur version: Callback returns (). Stopping requires AtomicBool checked periodically. Or the callback panics. Or you collect everything into a Vec and truncate.

25. Broken pipe as graceful exit

Every Unix CLI that pipes output must handle this:

// ripgrep — crates/core/main.rs
fn main() -> ExitCode {
    match run(flags::parse()) {
        Ok(code) => code,
        Err(err) => {
            for cause in err.chain() {
                if let Some(ioerr) = cause.downcast_ref::<std::io::Error>() {
                    if ioerr.kind() == std::io::ErrorKind::BrokenPipe {
                        return ExitCode::from(0);    // not an error
                    }
                }
            }
            eprintln_locked!("{:#}", err);
            ExitCode::from(2)
        }
    }
}

rg pattern | head should exit cleanly, not print an error about broken pipe. This is consistently handled at every output path in the codebase.

---

Part V — Modules & Visibility

26. pub(crate) as your default

ripgrep has 457 uses of `pub(crate)`. Every item's visibility is a deliberate choice.

The Arg struct in clap:

pub struct Arg {
    pub(crate) id: Id,
    pub(crate) help: Option<StyledStr>,
    pub(crate) action: Option<ArgAction>,
    pub(crate) value_parser: Option<super::ValueParser>,
    // ...
}

Users interact through methods. Internal code (parser, validator, help renderer) accesses fields directly. Encapsulated externally, convenient internally.

The rule: Start private. Promote to pub(crate) when another module in the same crate needs it. Promote to pub only when it's part of the intentional API. Every pub is a semver commitment.

27. #![deny(missing_docs)] enforces the discipline

Every ripgrep library crate:

#![deny(missing_docs)]

Every pub item must have a doc comment. If you can't explain why it's public, it probably shouldn't be.

The binary crate does NOT have this lint — internal code doesn't need the same documentation level. The distinction between library and application is enforced.

28. Facade crates hide workspace structure

ripgrep's grep crate — 90 lines total:

pub extern crate grep_cli as cli;
pub extern crate grep_matcher as matcher;
pub extern crate grep_printer as printer;
pub extern crate grep_regex as regex;
pub extern crate grep_searcher as searcher;

Users add one dependency: grep. They never need to know about the 5 sub-crates. Internal crate boundaries can change without affecting downstream.

parsel does the same:

// parsel/src/lib.rs
pub use parsel_derive::{Parse, ToTokens, FromStr, Display};

Users never write use parsel_derive::Parse. The proc-macro crate is an implementation detail.

29. Types carry behavior, functions wire types together

The difference between a 3000-line file of if-cascades and a professional codebase is WHERE the behavior lives.

Amateur: behavior lives in functions. Functions grow because they handle every combination of every option.

// Everything in one function
fn search_and_print(path: &Path, pattern: &str, opts: &Options) {
    let content = read_file(path);               // 50 lines
    let matches = find_matches(content, pattern); // 100 lines of if/else
    format_output(matches, opts);                 // 200 lines of if/else
}

Professional: behavior lives on types. Functions wire types together.

// ripgrep — each type carries its own behavior
fn search_path(matcher: &impl Matcher, searcher: &mut Searcher,
               printer: &mut impl Sink, path: &Path) {
    searcher.search_path(matcher, path, printer)
}

10 lines. Matcher handles matching. Searcher handles buffering and line detection. Sink handles output. The wiring function is trivial.

When a new feature arrives, you add a method to a type or a variant to an enum. You don't add another branch to a 500-line function.

---

Part VI — Macros

30. Macros are for trait impl boilerplate, not for logic

Across all four codebases, macros serve exactly three purposes:

1. Eliminating repetitive trait impls

parsel's impl_literal! generates 8 literal types, each with ~150 lines of trait impls (From, Into, PartialEq, Eq, Ord, Hash, Display, FromStr, ToTokens, Parse). 1200 lines from 120 lines of macro.

2. Test scaffolding

ripgrep's rgtest!:

rgtest!(r16, |dir: Dir, mut cmd: TestCommand| {
    dir.create(".gitignore", "ghi/");
    dir.create("ghi/toplevel.txt", "xyz");
    cmd.arg("xyz").assert_err();
});

Sets up temp directory, constructs test command, generates the #[test] function boilerplate.

3. Small DSLs for domain-specific notation

clap's arg!:

let arg = arg!(-o --output <FILE> "Output file");
// Expands to builder calls

What macros are NOT used for: None of these codebases use macros for business logic, control flow, or conditional branching. No macro does what a function or generic type could do.

The rule: If your macro does anything beyond stamping out trait impls or test scaffolding, it should probably be a function. Macros are hard to debug, invisible to rust-analyzer, and impossible to step through. Use them for structure, not logic.

31. Derive macros should bound field types, not type parameters

parsel:

// Naive: bound type parameter (breaks with associated types)
where T: Parse

// Professional: bound the actual field type
where FieldType1: Parse, FieldType2: Parse

The add_bounds function in parsel_derive collects all field types and bounds those specifically. This handles associated types, complex generics, and type parameter indirection that the naive approach breaks on.

The #[parsel(recursive)] attribute excludes a field from the where clause, breaking infinite constraint cycles for recursive types:

#[derive(Parse)]
enum Expr {
    Literal(LitInt),
    Paren(#[parsel(recursive)] Box<Paren<Expr>>),
}

Without the attribute, the where clause would require Box<Paren<Expr>>: Parse, which requires Expr: Parse, which requires Box<Paren<Expr>>: Parse — cycle.

32. Emit dummy impls on derive error

clap:

// clap_derive/src/dummies.rs
pub(crate) fn parser(name: &Ident) -> TokenStream {
    quote!(
        #[automatically_derived]
        impl clap::Parser for #name {}
    )
}

When a derive macro encounters a parse error in attributes, it still emits dummy trait impls (with empty bodies). Without this, the user sees their one attribute error PLUS 50 cascading "trait not implemented" errors from everywhere the type is used. With dummies, they see one clear error.

---

Part VII — Configuration & Construction

33. Private config, public builder, frozen product

Every configurable type in ripgrep:

// 1. Private — nobody outside sees this
struct Config {
    line_term: LineTerminator,
    binary: BinaryDetection,
    encoding: Option<Encoding>,
    // ...12 more fields
}

impl Default for Config {
    fn default() -> Config { /* sensible defaults */ }
}

// 2. Public — this is the configuration phase
pub struct SearcherBuilder {
    config: Config,
}

impl SearcherBuilder {
    pub fn new() -> SearcherBuilder {
        SearcherBuilder { config: Config::default() }
    }

    pub fn line_terminator(&mut self, lt: LineTerminator) -> &mut SearcherBuilder {
        self.config.line_term = lt;
        self
    }

    // build() freezes the config
    pub fn build(&self) -> Searcher {
        Searcher { config: self.config.clone(), /* ... */ }
    }
}

// 3. The product — immutable, ready to work
pub struct Searcher {
    config: Config,
    // runtime state...
}

This pattern appears in SearcherBuilder, StandardBuilder, SummaryBuilder, JSONBuilder, PathPrinterBuilder, LineBufferBuilder, WalkBuilder, GlobBuilder, RegexMatcherBuilder — all identical structure.

Why three types:

• Config is an implementation detail (private)
• Builder is the mutable configuration phase (public, ergonomic)
• Product is the operational phase (public, immutable)

Users can't change config mid-operation. The builder can be reused to produce variants. The config can change internally without breaking the API.

34. Two-phase processing: validate cheap, then construct expensive

ripgrep:

Phase 1: CLI args → LowArgs  (microseconds — string matching)
Phase 2: LowArgs → HiArgs    (milliseconds — compile regexes, stat filesystem)

If the user types rg --help, phase 1 detects SpecialMode::Help and short-circuits. Phase 2 never runs — no regex compilation, no filesystem access.

The rule: Separate "can I understand this input?" from "let me do the expensive work." If you discover a bad field after you've already compiled three regexes, you've wasted work.

35. Classify inputs by cost, dispatch cheapest-first

ripgrep's globset classifies glob patterns at construction time:

enum MatchStrategy {
    Literal(String),          // "target" → HashMap lookup, O(1)
    Extension(String),        // "*.o" → HashMap lookup, O(1)
    Prefix(String),           // "src/**" → string prefix check
    Suffix { suffix: String },// "**.rs" → string suffix check
    Regex,                    // complex pattern → full regex
}

During matching, strategies run cheapest-first. A typical .gitignore with 20 patterns resolves 18 via hash lookups. Only the complex patterns hit regex.

The principle: When you have a hot path processing heterogeneous inputs, don't apply the general-case algorithm to everything. Classify at construction time. Dispatch by cost.

---

Part VIII — Testing

36. Tests that return Result, not panic

parsel:

#[test]
fn test_parse() -> anyhow::Result<()> {
    let actual: Assignment = parsel::parse_str("foo = 42")?;
    ensure!(actual.name == "foo", "expected 'foo', got '{}'", actual.name);
    Ok(())
}

The ? operator works. Parse errors, IO errors, assertion failures all flow the same way. Output is a clean error message, not a panic backtrace.

37. Roundtrip testing catches what unit tests miss

parsel verifies parse(print(x)) == x:

let ts = ast_original.to_token_stream();
let ast_parsed: Many<Item> = parsel::parse2(ts)?;
assert!(ast_parsed == ast_original);

One property test replaces dozens of example tests and catches edge cases no human would think of. If your system has a natural roundtrip (serialize/ deserialize, encode/decode, parse/print), test the roundtrip.

38. Test behavior, not structure

parsel tests derive macros by testing what the generated code DOES:

#[derive(Parse)]
struct Assignment { name: Ident, eq: Token![=], value: LitInt }

let actual: Assignment = parsel::parse_str("foo = 42")?;
assert_eq!(actual.name, "foo");

Not what tokens it generates. If you test the generated token stream, every whitespace change breaks every test — even though behavior is identical.

ripgrep's integration tests create real directory structures and run the real binary:

rgtest!(r16, |dir: Dir, mut cmd: TestCommand| {
    dir.create(".gitignore", "ghi/");
    dir.create("ghi/toplevel.txt", "xyz");
    cmd.arg("xyz").assert_err();
});

330 such tests, each linked to a GitHub issue. The test IS the regression.

39. Colocate tests with the code they test

ripgrep's flag definitions — each flag is immediately followed by its tests:

#[derive(Debug)]
struct CaseSensitive;

impl Flag for CaseSensitive {
    fn is_switch(&self) -> bool { true }
    fn name_long(&self) -> &'static str { "case-sensitive" }
    fn update(&self, v: FlagValue, args: &mut LowArgs) -> anyhow::Result<()> { ... }
}

#[cfg(test)]
#[test]
fn test_case_sensitive() {
    let args = parse_low_raw(None::<&str>).unwrap();
    assert_eq!(CaseMode::Sensitive, args.case);
}

112 tests in one file, each next to its flag definition. When you add a flag, you add its test right there. The pattern makes it nearly impossible to add a flag without testing it.

---

Part IX — Performance

40. #[inline] on small methods that cross crate boundaries

ripgrep: 199 #[inline] annotations, concentrated on:

• Match::new(), Match::start(), Match::end() — millions of calls
• LineTerminator::as_byte() — once per line
• ByteSet::contains() — once per byte
• Sink default implementations — so they compile away

Without #[inline], the compiler can't inline across crate boundaries (unless using LTO). With it, the function body is available in the caller's codegen unit.

When NOT to: Large functions, same-crate functions (compiler handles those), functions called in one place. Over-inlining bloats code size and hurts instruction cache.

41. FlatMap for small N

clap:

pub(crate) struct FlatMap<K, V> {
    keys: Vec<K>,
    values: Vec<V>,
}

For N < ~50 (typical CLI: 10-30 arguments), linear scan beats hashing. Hash maps pay: computation, bucket management, random access. Linear scan pays: one sequential read through cache-friendly memory.

42. No async where threads suffice

ripgrep uses std::thread::scope with crossbeam work-stealing deques. No tokio. No async-std.

File search is CPU-bound with blocking filesystem reads. Async is for IO multiplexing — thousands of network connections waiting for responses. Adding an async runtime here adds colored functions, lifetime constraints, and runtime initialization for zero benefit.

Thread count caps at 12:

std::thread::available_parallelism().map_or(1, |n| n.get()).min(12)

Beyond 12, filesystem IO contention dominates.

---

Part X — Project Discipline

43. The compiler as quality gate

ring_api:

#![deny(unsafe_code)]
#![deny(clippy::result_unwrap_used)]
#![deny(clippy::option_unwrap_used)]
#![deny(clippy::missing_docs_in_private_items)]
#![deny(clippy::shadow_unrelated)]
#![deny(clippy::cast_precision_loss)]
#![deny(clippy::print_stdout)]
#![deny(variant_size_differences)]

parsel:

#![forbid(unsafe_code)]       // even stronger — can't #[allow] it

These aren't suggestions. They're compile errors. The lint config IS the quality policy.

Recommendations:

• deny(unsafe_code) on every crate that doesn't need unsafe (most crates)
• deny(clippy::unwrap_used) on library code (tests get #[allow])
• deny(missing_docs) on public APIs
• forbid over deny for hard rules (unsafe in a pure-logic crate)

44. #[non_exhaustive] on all public enums

ripgrep and clap both use #[non_exhaustive] on every public enum that might grow:

#[non_exhaustive]
pub enum ErrorKind { ... }

#[non_exhaustive]
pub enum ArgAction { ... }

Adding a variant is not a semver break. Downstream match blocks need a _ => arm — but that's better than forcing a major version bump for every new variant.

45. Document what you chose NOT to do

parsel:

/// `Separated` can't be `Default` because that would mean an empty sequence.
/// It can't be `DerefMut` because users could `.pop()` the last element.
/// `FromIterator` is missing because the input iterator could be empty.

ripgrep:

// I guess memory maps on macOS aren't great. Should re-evaluate.

A new contributor sees Separated without DerefMut. "Must be an oversight." They add it. Users call .pop(). The non-empty invariant dies. Without the comment, the "fix" looks helpful. With the comment, it's obviously wrong.

The rule: When you deliberately omit an impl, trait, or feature, explain why. Future readers will encounter the absence and wonder.

46. Own your critical path

BurntSushi wrote: regex, memchr, aho-corasick, globset, ignore, termcolor, walkdir, bstr, same-file. Every dependency between the user's command and the search output.

ring_api uses reqwest, serde, serde_json — because HTTP and JSON are not its core competency.

The rule: Own what's between your user's input and output. Depend on others for everything outside that path.

47. FromStr + Display before serde

ring_api:

impl FromStr for NodeId { /* parse "A:42:_:ALA" */ }
impl Display for NodeId { /* format "A:42:_:ALA" */ }

// serde delegates:
impl Serialize for NodeId {
    fn serialize<S>(&self, s: S) -> Result<S::Ok, S::Error> {
        s.collect_str(self)    // → Display
    }
}

FromStr and Display are testable without serde. They work in CLI args, log messages, error formatting — contexts where serde doesn't exist.

If you implement parsing inside a serde visitor, your type's string representation is trapped in the serialization framework. You can't parse the string without constructing a deserializer.

---

Appendix A — The Abstraction That Eliminates Code

parsel's core idea: a Rust struct is a sequence (parse fields left-to-right). A Rust enum is an alternation (try variants top-to-bottom).

#[derive(Parse, ToTokens)]
struct Assignment {
    name: Ident,
    eq: Token![=],
    value: LitInt,
}

No grammar file. No parser combinators. The type IS the grammar. The derive macro generates the parser from the struct definition.

Composable types handle grammar concepts:

The derive macros are ~500 LOC total. Trivial — because the types do the work.

The principle: The right abstraction doesn't add a layer — it removes one. If you're maintaining parallel structures (grammar + AST, schema + types), ask: can one generate the other?

---

Appendix B — Lint Configuration Reference

Recommended starting point, drawn from these codebases:

// Library crate root
#![forbid(unsafe_code)]
#![deny(missing_docs)]
#![deny(missing_debug_implementations)]
#![deny(clippy::unwrap_used)]
#![deny(clippy::print_stdout)]
#![deny(clippy::print_stderr)]
#![deny(clippy::dbg_macro)]
#![warn(clippy::todo)]
#![warn(clippy::unimplemented)]
#![warn(clippy::cast_possible_truncation)]
#![warn(clippy::cast_precision_loss)]
#![warn(clippy::implicit_clone)]
#![warn(clippy::inefficient_to_string)]
#![warn(clippy::enum_glob_use)]

For workspace-wide lints (clap pattern):

# Cargo.toml (workspace root)
[workspace.lints.clippy]
unwrap_used = "deny"
print_stdout = "deny"
dbg_macro = "deny"
todo = "warn"
cast_possible_truncation = "warn"
# ...

# Sub-crate Cargo.toml
[lints]
workspace = true

---

Source codebases: ripgrep v15.1.0 (~46K LOC), clap v4.5.60 (~41K LOC), parsel v0.16.0 (~6K LOC), ring_api v0.1.0 (~2.3K LOC). All code examples reference actual files in these repositories.