# Rust Codebase Review Guidelines
This document outlines a structured approach to analysing a Rust codebase. It covers core areas of concern, including warnings, structure, safety, and opportunities for improvement. These guidelines are intended for maintainers, reviewers, and auditors performing deep or recurring reviews.
---
## 1. Compiler Warnings
**Definition:** Anything `rustc` or Clippy flags during build or linting. Treat warnings as errors unless explicitly justified.
**Why it matters:** Warnings correlate with defects, undefined behaviour, and maintainability issues.
**Checklist:**
- [ ] Build with strict linting:
```bash
RUSTFLAGS="-Dwarnings" cargo build --all-targets
cargo clippy --all-targets --all-features -- -D warnings
```
- [ ] Ensure `#[allow(...)]` has local, justified comments.
- [ ] CI rejects any unchecked warnings.
---
## 2. TODO Statements / Comments
**Definition:** Markers like `TODO`, `FIXME`, `HACK`, `XXX`, `UNDONE` in source or docs.
**Why it matters:** They indicate latent scope and technical debt.
**Checklist:**
- [ ] Scan for TODO markers:
```bash
rg -n --no-heading -e 'TODO|FIXME|HACK|XXX|UNDONE'
```
- [ ] Each TODO includes: **Owner**, **Severity (S0–S3)**, **Deadline**, **Issue link**.
- [ ] No anonymous or >90-day-old unresolved TODOs.
---
## 3. Capabilities
**Definition:** What the code can actually do, as defined by APIs, feature flags, and platform guarantees.
**Why it matters:** Prevents overclaiming and enables robust documentation and testing.
**Checklist:**
- [ ] Enumerate public API via:
```bash
cargo doc --no-deps --document-private-items
```
- [ ] Ensure every capability has:
- [ ] Documented contract
- [ ] Positive and negative tests
- [ ] Working doctests
- [ ] Default feature set is safe and minimal.
---
## 4. Code Structure
**Definition:** Suitability of architecture: crate boundaries, module layout, trait boundaries, ownership, and error patterns.
**Why it matters:** Determines maintainability, soundness, and testability.
**Checklist:**
- [ ] `unsafe` and FFI code is isolated in separate crates/modules.
- [ ] Ownership is respected; minimal `Arc<Mutex<...>>`, limited `clone()`s.
- [ ] Clear layering: no circular dependencies, no upward imports.
- [ ] Typed errors in libraries (`thiserror`), `anyhow` only at edges.
- [ ] Sound and documented trait bounds (`Send`, `Sync`, `Clone`, etc).
**Commands:**
```bash
cargo tree -e features
rg -n '\.clone\(\)' src/
rg -n 'unsafe' src/
```
---
## 5. Potential Bugs
**Definition:** Unsafe code, panics, data races, incorrect logic, UB, or unsound trait impls.
**Why it matters:** Safety and correctness.
**Checklist:**
- [ ] Use Clippy correctness lints:
```bash
cargo clippy --all-targets -- -W clippy::correctness
```
- [ ] Interpret tests with Miri:
```bash
cargo miri test
```
- [ ] No `unwrap()`/`expect()` in library code (unless proven unreachable).
- [ ] Arithmetic uses checked operations in critical paths.
- [ ] Fuzzing, property tests, or sanitizers run if applicable.
---
## 6. Potential Improvements
**Definition:** Non-bug changes that improve reliability, performance, or usability.
**Why it matters:** Keeps system evolving safely.
**Areas:**
- [ ] Replace `Vec` with `SmallVec` or `ArrayVec` where appropriate.
- [ ] Use `From`, `TryFrom`, `AsRef`, builders for better API ergonomics.
- [ ] `cargo audit`, `cargo outdated`, `cargo udeps` are clean.
- [ ] Add property-based tests (`proptest`) and fuzzers (`cargo-fuzz`).
**Observability:**
- [ ] Add structured logging via `tracing`.
- [ ] Use `anyhow::Context` for user-facing errors.
---
## CI Enforcement
**Recommended CI steps:**
```yaml
- name: Clippy
run: cargo clippy --all-targets --all-features -- -D warnings
- name: Test
run: cargo test --all-targets
- name: Docs
env:
RUSTDOCFLAGS: "-D warnings"
run: cargo doc --no-deps
- name: Audit
run: cargo audit
- name: Udeps
run: cargo +nightly udeps
- name: Miri (optional)
run: cargo miri test
```
---
## Summary Rubric (0–5)
| Category | Score |
|---------------------|-------|
| Warnings / Lints | |
| Structure & Layout | |
| API & Documentation | |
| Testing Depth | |
| Safety & Correctness| |
| CI & Dependencies | |
Use this rubric to track improvement across reviews.
---
**Maintainer Note:** This document should be revised every 3–6 months to reflect evolving coding standards, team norms, and ecosystem tools.
