rust-trait-design-reviewer

Design traits that are easy to implement, hard to misuse and safe to evolve, and review existing trait-heavy code against the Rust API Guidelines. Every decision is a trade-off between three axes: dispatch (generics vs impl Trait vs dyn Trait), extensibility (who may implement the trait: anyone, nobody outside the crate, or only via a blanket impl) and evolution (which later changes are semver-major). Small std-only Rust programs in scripts/ scan source text for the mistakes that are cheap to catch mechanically (dyn-incompatible traits used as dyn, impl Into, missing Debug/Error impls, Deref inheritance, non-Send Box<dyn Error>, &Vec<T> parameters, unsendable public async fn traits), and scaffold_trait prints compiling templates for sealed, extension, typestate, newtype, dyn-compatible and async-Send traits.

#Trigger Conditions

Activate this skill when the user:

• Designs a public trait, a plugin/strategy/backend abstraction or a generic library API in Rust
• Asks "generics or dyn?", "impl Trait or T: Trait?", "associated type or generic parameter?"
• Hits error[E0038]: the trait X is not dyn compatible / cannot be made into an object, E0117 / E0210 (orphan rule), E0119 (conflicting implementations), E0277 (trait bound not satisfied), E0599 (method exists but trait bounds not satisfied / trait not in scope), E0191 (associated type must be specified), E0225
• Uses async fn in a trait and sees future cannot be sent between threads safely or the async_fn_in_trait warning, or needs dyn with async methods
• Reviews a PR adding traits, blanket impls, Deref, From/Into, error types or builder APIs
• Wants to seal a trait, add an extension trait, a newtype, a typestate builder or a mockable trait
• Asks which std traits a public type should implement, or whether a change is semver-breaking

#Workflow

0. BUILD     -> once: for t in dyn_compat trait_shape api_traits api_signatures scaffold_trait; do
                  rustc -O --edition 2021 "$SKILL/scripts/$t.rs" -o "/tmp/$t"; done
1. SCAN      -> for t in dyn_compat trait_shape api_traits api_signatures; do /tmp/$t src/; done
                (whole crate: impls and `dyn` uses in other files count)
2. CLASSIFY  -> For each trait: who calls it (generic code, dyn, both)? who implements it (us, users, tests)?
                Use the dispatch and extensibility tables below.
3. CHECK     -> dyn compatibility, coherence (could a user or upstream crate write a conflicting impl?),
                Send/Sync story, std trait checklist for every public type, semver surface
4. FIX       -> Smallest change that keeps call sites compiling: `where Self: Sized`, From instead of Into,
                derive Debug, thiserror, &[T] instead of &Vec<T>, seal before 1.0
                Templates: /tmp/scaffold_trait sealed --name Backend
5. VERIFY    -> cargo check && cargo clippy -- -W clippy::pedantic; cargo test (incl. compile_fail doctests);
                cargo semver-checks for published crates; each scanner with --strict exits 1 on findings

#Dispatch: Generics vs impl Trait vs dyn Trait

use std::fmt::Display;
use std::path::Path;

pub fn join_all<T: Display>(items: &[T]) -> String {
    items.iter().map(ToString::to_string).collect::<Vec<_>>().join(", ")
}

pub fn bracket(item: impl Display) -> String {
    format!("[{item}]")
}

pub fn join_mixed(items: &[&dyn Display]) -> String {
    items.iter().map(|i| i.to_string()).collect::<Vec<_>>().join(", ")
}

// Generic shell, non-generic body: only the tiny shell is monomorphized (std::fs::read does this).
pub fn line_count<P: AsRef<Path>>(path: P) -> std::io::Result<usize> {
    fn inner(path: &Path) -> std::io::Result<usize> {
        Ok(std::fs::read_to_string(path)?.lines().count())
    }
    inner(path.as_ref())
}

fn main() {
    let mixed: [&dyn Display; 3] = [&1, &"two", &3.5];
    println!("{} | {} | {}", join_all(&[1, 2, 3]), bracket("x"), join_mixed(&mixed));
}

#Dyn Compatibility (Object Safety)

A trait can be used as dyn Trait only if every method can be called through a vtable. Methods that cannot must opt out with where Self: Sized (they are then callable on concrete types only).

Allowed: lifetime parameters on methods, trait-level generic parameters (dyn Codec<u32>), associated types (must be named: dyn Iterator<Item = u8>, else E0191), receivers &self, &mut self, self: Box<Self>, Rc<Self>, Arc<Self>, Pin<&mut Self>. Only auto traits may be added: dyn Read + Send works, dyn Read + Write is E0225 (make trait ReadWrite: Read + Write {} with a blanket impl).

use std::fmt::Debug;

pub trait Storage: Debug + Send + Sync {
    fn get(&self, key: &str) -> Option<Vec<u8>>;
    fn put_bytes(&mut self, key: &str, value: &[u8]);

    fn open(path: &str) -> std::io::Result<Self>
    where
        Self: Sized;

    fn put_all<I: IntoIterator<Item = (String, Vec<u8>)>>(&mut self, items: I)
    where
        Self: Sized,
    {
        for (k, v) in items {
            self.put_bytes(&k, &v);
        }
    }
}

#[derive(Debug, Default)]
pub struct Memory(std::collections::HashMap<String, Vec<u8>>);

impl Storage for Memory {
    fn get(&self, key: &str) -> Option<Vec<u8>> {
        self.0.get(key).cloned()
    }
    fn put_bytes(&mut self, key: &str, value: &[u8]) {
        self.0.insert(key.to_owned(), value.to_vec());
    }
    fn open(_path: &str) -> std::io::Result<Self> {
        Ok(Self::default())
    }
}

fn main() {
    let mut store: Box<dyn Storage> = Box::new(Memory::open("mem").unwrap());
    store.put_bytes("k", b"v");
    assert_eq!(store.get("k").as_deref(), Some(&b"v"[..]));
}

Trait upcasting (Rust 1.86): &dyn Sub coerces to &dyn Super (also Box, Arc) when trait Sub: Super. With trait Plugin: Any, let any: &dyn Any = plugin; then any.downcast_ref::<T>() works without an as_any() helper. Before 1.86 add fn as_any(&self) -> &dyn Any.

#Associated Types vs Generic Parameters

Rule: use an associated type when the type is an output determined by Self; use a generic parameter (with a default, trait Add<Rhs = Self>) when the same type sensibly implements the trait several times. Associated type bounds T: Iterator<Item: Display> are stable since 1.79.

#Who May Implement It: Extensibility Patterns

mod private {
    pub trait Sealed {}
}

/// Implemented for `Json` and `Toml` only; new methods are not breaking changes.
pub trait Format: private::Sealed {
    fn extension(&self) -> &'static str;
}

#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Default)]
pub struct Json;

impl private::Sealed for Json {}
impl Format for Json {
    fn extension(&self) -> &'static str {
        "json"
    }
}

Coherence in one paragraph: impl ForeignTrait for ForeignType is E0117; impl<T> ForeignTrait for T is E0210 (uncovered type parameter); a local type must appear before any uncovered type parameter, so impl<T> From<Local> for Vec<T> and impl Mul<Vec2> for f64 are fine (Rust 1.41+). &T, &mut T, Box<T> and Pin<T> are fundamental: impl Display for Box<Local> is allowed. Two impls that could overlap for some type are E0119, including "upstream crates may add a new impl" cases. Details and fixes: coherence-and-trait-patterns.md.

#async fn in Traits (Rust 1.75+)

// Cargo.toml: trait-variant = "0.1"
#[trait_variant::make(Fetcher: Send)]
pub trait LocalFetcher {
    async fn fetch(&self, url: &str) -> Result<String, std::io::Error>;
}
// `Fetcher` is generated: same method, future is `+ Send`; impl Fetcher for your type and use
// `F: Fetcher + Clone + 'static` in code that tokio::spawn()s.

#Core Rules

• Implement From, never Into (Into comes from a blanket impl and ? uses From); TryFrom for fallible conversions; AsRef<T> for cheap reference views; Borrow<T> only when Eq/Hash/Ord agree (map keys).
• Every public type: Debug (C-DEBUG). Then eagerly: Clone, Copy (small plain data only, removing it later is breaking), PartialEq/Eq, Hash, PartialOrd/Ord (when meaningful), Default, Display (user facing text), Send + Sync (check, do not assume), serde behind a serde cargo feature (C-SERDE).
• Error types: an enum or struct with Display + Error + Send + Sync + 'static, #[non_exhaustive], source() wired (thiserror does all of it). Dynamic errors are Box<dyn Error + Send + Sync + 'static>.
• Accept borrowed, general inputs (&[T], &str, &Path, impl AsRef<Path>, impl IntoIterator); return owned, concrete outputs. Take R: Read/W: Write by value (C-RW-VALUE; &mut R also implements them).
• Bounds go on impls and functions, not on struct definitions (C-STRUCT-BOUNDS), except where the type's Drop or layout needs them.
• Deref only for smart pointers and transparent wrappers (C-DEREF); never to fake inheritance.
• Operators only with their mathematical meaning (C-OVERLOAD); implement the &a + &b forms for non-Copy types.
• #[must_use] on builders and on functions whose only effect is their return value; #[non_exhaustive] on public enums/structs you expect to grow.
• Callbacks: accept the weakest Fn trait you can (FnOnce > FnMut > Fn for caller freedom); store Box<dyn Fn(..) + Send + Sync>; async callbacks: AsyncFn traits (1.85+) or Fn() -> Fut.
• A trait with one implementor and no dyn/test use is indirection: start concrete, extract a trait later.
• Seal or #[doc(hidden)] anything you are not ready to promise; adding a required method to an open trait is a major release.

More: std-traits-and-api-guidelines.md, semver-and-evolution.md.

#Symptom -> Cause -> Fix

#Output Format

When reviewing, answer with:

1. Summary - one paragraph: what the abstraction is for, who implements it, who calls it, and the verdict.
2. Findings - ordered by severity; each with file:line, rule id (TRxxx when the script flags it, or the API guideline id such as C-DEBUG), why it matters for users or semver, and the concrete fix.
3. Design decisions - dispatch choice, extensibility (open/sealed/extension), Send/Sync story, error type, each with the alternative considered and why it lost.
4. Code - complete corrected trait + one impl + one call site (generic and/or dyn) that compiles on the stated MSRV; mark features needing a newer compiler (AFIT 1.75, upcasting 1.86, AsyncFn 1.85).
5. Verification - cargo check, cargo clippy, the test that pins the property (e.g. a dyn usage or assert_send test, a compile_fail doctest), cargo semver-checks for published crates.

When designing from scratch, lead with the trait definition and a table of decisions, then the code.

#Scripts

Small, standard-library-only Rust programs that read source text (no Cargo project, never compile your code). Build each once with rustc -O --edition 2021 scripts/<name>.rs -o /tmp/<name>.

/tmp/dyn_compat src/                                   # files and/or directories (skips target/, .git/)
/tmp/api_traits src/lib.rs --json                      # machine-readable findings with fix
cat src/lib.rs | /tmp/api_signatures -                 # stdin
/tmp/trait_shape src/ --strict --max-methods 6         # exit 1 on any finding (CI gate)
/tmp/api_traits src/ --select TR003,TR004 --ignore TR012
/tmp/dyn_compat --list-rules                           # each scanner lists its rules with why and fix
/tmp/scaffold_trait typestate --name Request           # also: sealed extension newtype dyn-compatible async-send

Scan the whole crate: impls in other files satisfy TR003/TR004 and dyn uses anywhere trigger TR001. #[cfg(test)] modules are skipped by the hygiene rules; silence a line with // noqa or // noqa: TR010. These are heuristic scanners, not rustc: macros, cfg variants and re-exports are invisible. Exit codes: 0 clean (or findings without --strict), 1 findings with --strict, 2 usage error or unreadable file.

#Additional Resources