# Rust Reviewer > Review Rust code for correctness, safety, idiomatic patterns, and common AI-generated code smells. Use when reviewing Rust code quality or before merging. Use when this capability is needed. - Skill: `tomevault-io/rust-reviewer` (Agent Skill, multi-file: 2 files) - Install (CLI): `npx skillmds@latest add tomevault-io/rust-reviewer` - Raw SKILL.md: https://api.skillmd.com/api/skills/tomevault-io/rust-reviewer/raw - Safety review: pending (external: skill-scanner PASS, skillspector PASS) - Works with: Claude Code, Claude.ai, OpenAI Codex - Category: Coding & Dev Tools - Author: tomevault-io (https://skillmd.com/u/tomevault-io) - Updated: 2026-09-17 - Page: https://skillmd.com/skills/tomevault-io/rust-reviewer --- # Rust Code Reviewer Review Rust code targeting the specific failure modes that AI-generated and human-written Rust most commonly gets wrong. Not a generic code review — Rust-specific. ## Review Checklist (ordered by severity) ### 1. Unsafe Code Audit (critical) First: **is unsafe even necessary?** - FFI calls → yes, required - Raw pointer deref → yes, but can it be restructured to avoid raw pointers? - Mutable statics → yes, but consider `OnceCell`, `LazyLock`, or atomics instead - Implementing `Send`/`Sync` → yes, but verify the type genuinely satisfies the contract - Bypassing borrow checker → **no. The design is wrong. Redesign it.** - Performance → maybe. Must have measured proof. For every `unsafe` block that IS necessary: - [ ] `// SAFETY:` comment explains **why** invariants hold (not just "this is safe") - [ ] Block scope is minimal - [ ] Pointer validity: non-null, points to allocated memory, properly aligned, valid bit pattern - [ ] Aliasing: no simultaneous `&T` and `&mut T` to same data, no two `&mut T` to same data - [ ] Initialization: memory initialized before read, `MaybeUninit` used correctly - [ ] Type invariants: transmutes maintain validity, `#[repr(C)]` where layout matters, UTF-8 valid for strings - [ ] Concurrency: `Send`/`Sync` contracts genuine, no data races, correct atomic ordering - [ ] Drop safety: `drop_in_place` exactly once, no double-free, correct allocator - [ ] Encapsulation: unsafe wrapped behind a safe API, invariants enforced by construction ```rust // BAD: // SAFETY: this is fine // BAD: // SAFETY: we know the pointer is valid // GOOD: // SAFETY: `ptr` was obtained from `Box::into_raw` in `new()` and has not // been freed since. The Box was allocated with the global allocator, same // as used by `Box::from_raw`. Non-null guaranteed by Box. ``` ### 2. Lifetime Correctness - [ ] Lifetime annotations trace actual data flow, not guessed - [ ] No hidden `'static` assumptions that break callers - [ ] Struct lifetimes match actual borrow durations of their fields - [ ] No unnecessary lifetime parameters (elision handles most cases) - [ ] Self-referential patterns avoided or correctly handled with `Pin` ### 3. Async Correctness - [ ] No blocking operations (`std::fs`, `std::net`, `thread::sleep`) in async fns - [ ] No `std::sync::MutexGuard` held across `.await` points - [ ] Spawned tasks: all captured data is `Send + 'static` - [ ] No mixing async runtimes (tokio vs async-std) - [ ] Cancellation safety considered for futures that may be dropped mid-await - [ ] Trait objects: boxed with correct `Send`/lifetime bounds - [ ] `?` works across `.await` (error types compatible) ### 4. Ownership Smells - [ ] **Excessive `.clone()`**: each one should have a reason, not a borrow-checker escape - [ ] **`Rc`/`Arc` overuse**: shared ownership should be intentional - [ ] **Unnecessary `Box`**: could this be `impl Trait` instead? - [ ] **`String` where `&str` suffices** in function params (and vice versa in struct fields) ### 5. Error Handling - [ ] No `unwrap()` / `expect()` on fallible paths (network, IO, user input, parsing) - [ ] `unwrap()` acceptable in: tests, provably infallible cases with comment, early `main()` setup - [ ] Error types implement `std::error::Error` + `Display` - [ ] Error context preserved across conversions (`.context()` or `map_err`) - [ ] `?` used consistently ### 6. API Design - [ ] Rust conventions: `new()`, `builder()`, `into_*`/`as_*`/`to_*` naming - [ ] `#[must_use]` where ignoring return value is likely a bug - [ ] Standard trait impls: `Debug`, `Display`, `Clone`, `PartialEq` where appropriate - [ ] Functions accept borrows (`&str`, `&[T]`, `&Path`) when they don't need ownership ### 7. Idiomatic Patterns - [ ] Iterators over manual indexing - [ ] Pattern matching over if-else chains on enums - [ ] `if let` / `let else` for single-variant matches - [ ] No C-style `for i in 0..v.len() { v[i] }` — use `for item in &v` - [ ] `Default` derive/impl where a zero value makes sense ### 8. Dependencies & Imports - [ ] Imports resolve to actual paths in the dependency tree - [ ] No unused imports - [ ] No deprecated API usage - [ ] Feature flags documented if enabling optional crate features ## Output Format Present findings grouped by severity: 1. **Blocking** — must fix (unsafe soundness holes, lifetime bugs, compilation errors) 2. **Should fix** — correctness or significant quality (error handling, ownership smells, async pitfalls) 3. **Suggestions** — idiomatic improvements, style (won't break anything) --- > Converted and distributed by [TomeVault](https://tomevault.io/claim/jvz-devx) — claim your Tome and manage your conversions.