/rust-code-review

name: rust-code-review
description: Reviews Rust code for ownership, borrowing, lifetime, error handling, trait design, unsafe usage, and common mistakes. Use when reviewing .rs files, checking borrow checker issues, error handling patterns, or trait implementations. Covers Rust 2024 edition patterns and modern idioms.

Rust Code Review

Review Workflow

Follow this sequence to avoid false positives and catch edition-specific issues:

1. **Check `Cargo.toml`** — Note the Rust edition (2018, 2021, 2024) and MSRV if set. Edition 2024 introduces breaking changes to unsafe semantics, RPIT lifetime capture, temporary scoping, and `!` type fallback. This determines which patterns apply. Check workspace structure if present. 2. **Check dependencies** — Note key crates (thiserror vs anyhow, tokio features, serde features). These inform which patterns are expected. 3. **Scan changed files** — Read full functions, not just diffs. Many Rust bugs hide in ownership flow across a function. 4. **Check each category** — Work through the checklist below, loading references as needed. 5. **Verify before reporting** — Complete **Gates** (below), including the verification-protocol gate, before submitting findings.

Gates

These steps are **sequenced**: do not skip ahead with “mental verification.” Each step has an objective **Pass** you can satisfy from files on disk and your own read path.

1. **Crate context** — Before relying on edition-specific checklist rows (Edition 2024, MSRV-sensitive APIs) or dependency assumptions. **Pass:** You opened the relevant `Cargo.toml` (package or workspace manifest) and can state `edition` and `rust-version` (if set) in one line. 2. **Expanded read** — Before reporting a **Major** or **Critical** finding. **Pass:** You read the full function, `unsafe` block, or `impl` / trait item that contains the cited line (not only a diff hunk). 3. **Severity match** — Before each finding line in the report. **Pass:** The **Severity** label matches **Severity Calibration** for that issue class, or you use **Informational** and give a one-line rationale. 4. **Verification protocol** — Before finalizing the report. **Pass:** the [review-verification-protocol](../review-verification-protocol/SKILL.md) skill is loaded and every step in it that applies to this review is completed (do not substitute a vague “I checked”).

Output Format

Report findings as:

[FILE:LINE] ISSUE_TITLE
Severity: Critical | Major | Minor | Informational
Description of the issue and why it matters.

Quick Reference

| Issue Type | Reference | |------------|-----------| | Ownership transfers, borrowing, lifetimes, clone traps, iterators | [references/ownership-borrowing.md](references/ownership-borrowing.md) | | Lifetime variance, covariance/invariance, memory regions | [references/lifetime-variance.md](references/lifetime-variance.md) | | Result/Option handling, thiserror, anyhow, opaque vs enumerated errors, deferred-cleanup with `?` | [references/error-handling.md](references/error-handling.md) | | Async pitfalls, Send/Sync bounds, poll contract, Pin mechanics, cancellation soundness | [references/async-concurrency.md](references/async-concurrency.md) | | Send/Sync semantics, atomics, memory ordering, lock patterns | [references/concurrency-primitives.md](references/concurrency-primitives.md) | | Memory ordering decision tree, fences, ABA, out-of-thin-air | [references/memory-ordering.md](references/memory-ordering.md) | | Hand-rolled spinlocks, channels, Arc, seqlock, CAS retry patterns | [references/lock-free-patterns.md](references/lock-free-patterns.md) | | Shared-memory vs worker-pool vs actor design, async vs threads, race-condition vs data race | [references/concurrency-models.md](references/concurrency-models.md) | | Type layout, alignment, repr, PhantomData, generics vs dyn Trait, wide pointers, auto-trait leakage | [references/types-layout.md](references/types-layout.md) | | Object safety, ergonomic trait impls, Deref discipline, fallible destructors, hidden contracts, is_normal | [references/interface-design.md](references/interface-design.md) | | Index pointers, drop guards, extension traits, crate preludes | [references/patterns-in-the-wild.md](references/patterns-in-the-wild.md) | | Unsafe code, API design, derive patterns, clippy patterns | [references/common-mistakes.md](references/common-mistakes.md) | | Validity vs safety, drop check, may_dangle, provenance, panic safety in unsafe, MaybeUninit, Miri | [references/unsafe-deep.md](references/unsafe-deep.md) |

> For development guidance on performance, pointer types, type state, clippy config, iterators, generics, and documentation, load the [rust-best-practices](../rust-best-practices/SKILL.md) skill.

Review Checklist

Ownership and Borrowing

• [ ] No unnecessary `.clone()` to silence the borrow checker (hiding design issues)
• [ ] No `.clone()` inside loops — prefer `.cloned()` or `.copied()` on iterators
• [ ] No cloning to avoid lifetime annotations (take ownership explicitly or restructure)
• [ ] References have appropriate lifetimes (not overly broad `'static` when shorter lifetime works)
• [ ] **Edition 2024**: RPIT (`-> impl Trait`) captures all in-scope lifetimes by default; use `+ use<'a>` for precise capture control
• [ ] `&str` preferred over `String`, `&[T]` over `Vec<T>` in function parameters
• [ ] `impl AsRef<T>` or `Into<T>` used for flexible API parameters
• [ ] No dangling references or use-after-move
• [ ] Interior mutability (`Cell`, `RefCell`, `Mutex`) used only when shared mutation is genuinely needed
• [ ] Small types (≤24 bytes) derive `Copy` and are passed by value
• [ ] `Cow<'_, T>` used when ownership is ambiguous
• [ ] Iterator chains preferred over index-based loops for collection transforms
• [ ] No premature `.collect()` — pass iterators directly when the consumer accepts them
• [ ] `.sum()` preferred over `.fold()` for summation (compiler optimizes better)
• [ ] `_or_els