---
name: rust-code-review
description: "Rust Code review checklist and decision framework for Rust PRs, derived from rust-best-practices."
globs:
  - "**/*.rs"
alwaysApply: true
---

# Rust Code Review

Use this for Rust review work where consistency, safety, and maintainability matter.

Follow ths standards in https://github.com/apollographql/rust-best-practices

## Mandatory pre-merge checks

- Ownership and data flow are intentional.
- Error handling is explicit and aligns with crate/binary boundaries.
- Clippy and format quality are clean in touched files.
- Performance changes are measured before acceptance.
- Public APIs are documented; docs match runtime behavior.
- Tests cover intended behavior and error paths.
- Unsafe or raw-pointer usage is justified and constrained.

## Severity matrix

- P0: unsafe memory bug, panic in recoverable production path, silent data corruption.
- P1: correctness bug, missing error propagation, invalid API contract.
- P2: likely performance regression, missing public API docs, flaky tests.
- P3: style/readability issues, avoidable clone/allocation, unnecessary complexity.

## Review skills

### Ownership-first coding

- Prefer borrowing (`&T`, `&mut T`) over cloning.
- Use `Clone` only when ownership is required or snapshots are explicitly needed.
- Treat unnecessary clones (especially in loops) as likely regressions.
- Reject `clone` on `Copy` types.

### Value vs reference

- Pass `Copy`/small POD types by value.
- Pass large heap-backed or non-trivial objects by reference.
- Surface ownership intent in function signatures.
- Use `Cow<'_, T>` when input may be borrowed or owned.

### Fallible control flow

- Use `let PATTERN = EXPR else { ... }` for expected early exits.
- Use `if let ... else` when divergence needs additional logic.
- Prefer `?` for bubbling errors.
- Avoid `unwrap`/`expect` in production except when impossible-by-design cases are documented.

### Allocation and allocation timing

- Prefer `_else` APIs to avoid eager allocation (`ok_or_else`, `map_or_else`, etc.).
- Keep iterator chains lazy; allocate only when required by terminal ops.
- Do not collect and allocate only to throw away data.

### Iterator vs loop

- Use iterator chains for data transformation and composition.
- Use `for` for early exits and side-effect-heavy or control-heavy loops.
- Require readable formatting; avoid long unreadable chains.

### Lints and static checks

- Run and fix warnings from:
  - `cargo clippy --all-targets --all-feature --locked -- -D warnings`
- Do not globally silence useful lints.
- Prefer `#[expect(clippy::...)]` with rationale instead of `#[allow(...)]` unless fully justified.

### Error discipline

- Libraries: prefer typed errors (`thiserror` and `#[from]` conversions).
- Binaries: `anyhow` acceptable, but keep context rich and actionable.
- Test both success and error behavior.

### Tests as behavior docs

- One behavior per test.
- One core assertion per test where possible.
- Names should be descriptive sentence-like statements.
- Prefer unit tests for internals, integration tests for public behavior.
- Use snapshot tests only for complex, stable structured outputs.

### Documentation and comments

- Use `///`/`//!` for API behavior and constraints.
- Use `//` for `why`, safety rationale, platform constraints, and assumptions.
- Remove stale comments; prefer smaller functions over narrative comments.
- Link TODOs to issues instead of leaving bare `TODO:`.

### Pointers and concurrency

- Prefer `&`/`&mut` before any heap pointer.
- Use `Arc` for cross-thread shared ownership; `Rc` for single-threaded.
- Use `Box` for recursive/heap allocation needs.
- Review raw pointer usage as unsafe boundaries with explicit invariants.

## Quick rejection triggers

- Unnecessary clones in hot paths.
- Unjustified `allow(clippy::...)`.
- Silent recovery from `Err` that discards root cause.
- Copying large types by value without a proof of intent.
- Comments that simply restate what code already expresses.
- TODOs without ownership/context.