Rust Code Review Checklist: Ownership, Borrowing, and Safety

.unwrap() panics crash production when Options or Results contain errors. Excessive .clone() kills performance by copying data unnecessarily. Fighting the borrow checker with unsafe code bypasses Rust's safety guarantees. Ignoring compiler warnings about unused Results hides errors. These issues compile but cause panics, performance problems, or unsafe behavior.

This checklist helps catch what matters: panic-prone code, unnecessary clones, improper error handling, and patterns that fight Rust's ownership system instead of working with it.

Quick Reference Checklist

Use this checklist as a quick reference during code review.

Ownership & Borrowing

• Ownership clear for each value (who owns what)
• Borrows end before owner moves or mutates
• No multiple mutable borrows or mutable + immutable borrows
• References don't outlive referents (lifetime validity)
• Ownership transfer via move intentional and documented
• & for immutable borrow, &mut for mutable borrow
• Slices &[T] preferred over Vec<T> for parameters
• Lifetime annotations only when compiler can't infer

Error Handling

• Result and ? operator instead of unwrap()
• expect() with message instead of unwrap() when panic OK
• match for complex error handling
• Custom error types for domain errors
• thiserror or similar for error type boilerplate
• Library code returns Result, doesn't panic
• Errors propagated not swallowed
• panic! only for truly unrecoverable situations

Pattern Matching

• match expressions exhaustive (all cases covered)
• Unreachable patterns flagged by compiler addressed
• if let for single pattern matching
• Destructuring used to extract values
• _ wildcard only when remaining cases truly don't matter
• Match arms return consistent types
• Guard clauses if in match arms when needed
• Pattern matching on enums instead of unwrap chains

Traits & Generics

• Trait bounds specify requirements: <T: Display>
• impl Trait for simple return types
• dyn Trait for trait objects when needed
• Traits derived when standard impl OK: #[derive(Debug, Clone)]
• Generic parameters constrained appropriately
• No unnecessary trait bounds (compiler suggests removal)
• Associated types used for type families
• Coherence rules followed (orphan rule)

Concurrency

• Send and Sync traits understood for thread safety
• Arc<T> for shared ownership across threads
• Mutex<T> or RwLock<T> for interior mutability
• Channels std::sync::mpsc for thread communication
• Async/await for I/O-bound concurrency
• No data races (compiler prevents these)
• Deadlocks avoided (lock ordering, timeouts)
• Rayon for data parallelism when appropriate

Performance & Efficiency

• Unnecessary clones identified and removed
• &str parameters instead of String when possible
• Iterator chains instead of intermediate collections
• collect() only when materialization needed
• Cow<str> for sometimes-owned strings
• Zero-cost abstractions leveraged
• Profile before optimizing (no premature optimization)
• Allocations minimized in hot paths

Ownership System and Borrowing Rules

Ownership in Rust means each value has a single owner responsible for cleanup. When owner goes out of scope, the value is dropped. This prevents memory leaks and use-after-free without garbage collection. When reviewing code, we verify ownership is clear—looking at a value, we should know what owns it and when it'll be dropped.

Borrowing allows temporary access without transferring ownership. Immutable borrows &T allow reading without modification. Mutable borrows &mut T allow modification. Rust enforces that references don't outlive their referents and prevents aliasing mutable references. When we see borrow errors in compiler output, these violations indicate the code needs restructuring.

Multiple immutable borrows can coexist because reading is safe. Multiple mutable borrows or mixing immutable with mutable borrows create data races, which Rust prevents at compile time. When code needs mutation, we verify only one mutable reference exists at that point.

Lifetime annotations 'a make reference validity explicit when the compiler can't infer it. Most functions don't need lifetime annotations because the compiler infers them. When we see lifetime annotations, we verify they're necessary and correct. Over-specifying lifetimes makes code harder to maintain.

Ownership transfer happens via moves. After let b = a;, a is no longer usable because ownership moved to b. This eliminates use-after-move bugs common in C++. When reviewing code, we verify moves are intentional and the previous owner isn't used afterward.

Slices &[T] as function parameters are more flexible than Vec<T>. Slices work with arrays, vectors, and any contiguous sequence. The pattern fn process(items: &[Item]) accepts more input types than fn process(items: Vec<Item>) which requires ownership transfer.

Error Handling Without Panics

Result<T, E> represents operations that might fail. Functions that can fail should return Result, not panic. The ? operator propagates errors concisely: let value = operation()?; returns early if operation fails, otherwise extracts the success value. This beats explicit match on every Result.

.unwrap() panics when Result is Err or Option is None. This is appropriate in examples or tests but dangerous in production. When we see unwrap in production code, we verify panic is acceptable or suggest proper error handling. The pattern value.unwrap() should raise questions.

.expect("message") documents why panic is acceptable. Instead of unwrap, expect provides context when panic occurs. We use this when panic truly is the right behavior, making the decision explicit. When we see expect, the message should explain why panic is appropriate.

match handles complex error cases. When different error variants need different handling, matching on Result allows variant-specific logic. Simple error propagation uses ?, complex handling uses match.

Custom error types improve error handling for domain-specific failures. Libraries like thiserror reduce boilerplate for error types. When code has many error types, custom errors with context beat generic error strings.

Library code should almost never panic. Libraries return Results letting callers decide how to handle errors. When we see panics in library code (except for truly unrecoverable situations like contract violations), that should return Result instead.

Pattern Matching Exhaustiveness

Match expressions must cover all cases. Rust requires exhaustive matching on enums, ensuring we handle all variants. This catches bugs when new variants are added—the compiler forces updating all matches. When we see match expressions, we verify all cases are meaningful and not just satisfying exhaustiveness with wildcards.

if let simplifies matching a single pattern. Instead of match with one case and _, if let Some(value) = option extracts the value when present. This reads clearer for single-case matches. When we see single-arm matches, if let usually improves clarity.

Destructuring extracts values from structs and enums directly. The pattern match result { Ok(value) => process(value), Err(e) => handle(e) } extracts values inline. This beats matching then accessing fields in the arm body.

The wildcard _ matches anything but should be used carefully. When a match has a _ arm, we verify the remaining cases truly don't need handling. Wildcards can hide bugs when new cases are added. If specific handling exists for some cases, we consider whether _ is too broad.

Guard clauses if in match arms add conditions. The pattern Some(value) if value > 10 => ... matches only when the guard succeeds. This beats nested if expressions inside match arms. When match arms have if statements at the top, guard clauses often simplify the logic.

Traits and Generic Programming

Trait bounds specify requirements for generic parameters. The function fn print<T: Display>(value: T) requires T implements Display. This enables calling format-related methods on value. When generic functions don't constrain type parameters, they often can't do anything useful with the generic values.

impl Trait as a return type hides concrete types. The signature fn make_iter() -> impl Iterator<Item=i32> returns some iterator without specifying which. This provides flexibility and reduces verbosity. When return types are complex iterators or futures, impl Trait simplifies signatures.

Trait objects dyn Trait enable runtime polymorphism. The type Box<dyn Display> can hold any type implementing Display. This costs runtime dispatch but enables heterogeneous collections. When we need collections of different types sharing a trait, trait objects provide the solution.

Derived traits using #[derive(...)] generate standard implementations. For types that don't need custom Debug, Clone, or PartialEq, deriving generates correct implementations. When we see manual implementations of standard traits, we verify derivation wouldn't work or that custom logic is necessary.

Associated types in traits define type families. When a trait has an associated type, implementers specify what that type is. This beats generic parameters when there's only one sensible type for each implementation. When traits have generic parameters that are always the same for a given implementation, associated types clarify the relationship.

Fearless Concurrency

Send and Sync marker traits indicate thread safety. Send means ownership can transfer between threads. Sync means references can be shared between threads. The compiler implements these automatically for safe types. When working with concurrency primitives, understanding Send and Sync prevents race conditions.

Arc<T> provides atomic reference counting for shared ownership across threads. When multiple threads need to access the same data, Arc enables sharing. The pattern let shared = Arc::new(data); let clone = shared.clone(); creates multiple handles to the same data. When we see manual reference counting, Arc handles it safely.

Mutex<T> and RwLock<T> provide interior mutability with locking. Mutex allows one writer or one reader at a time. RwLock allows multiple readers or one writer. The pattern let guard = mutex.lock().unwrap(); acquires the lock and provides access to the data. When shared data needs mutation, these provide thread-safe access.

Channels std::sync::mpsc enable message passing between threads. The pattern creates a sender and receiver, allowing threads to communicate without shared state. This often provides cleaner designs than shared memory with locks.

Async/await handles I/O-bound concurrency efficiently. Instead of blocking threads on I/O, async operations yield control until data is ready. This enables handling thousands of concurrent I/O operations efficiently. When code does lots of I/O, async often beats threads.

Performance Without Sacrificing Safety

Unnecessary .clone() indicates fighting the borrow checker instead of working with it. When we see clone, we verify ownership transfer is necessary or if borrowing would work. Sometimes clone is correct, but often it indicates the code structure could improve.

&str parameters are more flexible than String. Functions that just read strings should take &str, accepting both &str and &String (which coerces to &str). When functions take String parameters, we consider whether &str would work.

Iterator chains process data lazily without intermediate collections. The pattern items.iter().filter(...).map(...).collect() processes elements on demand. Creating intermediate vectors between operations wastes memory. When we see loops building collections to transform again, iterator chains improve efficiency.

collect() materializes iterators into collections. This should happen only when we need the full collection. Code that collects just to iterate again wastes work. When we see collect followed by iteration, skipping collect might improve performance.

Cow<str> (Clone on Write) handles strings that are sometimes owned, sometimes borrowed. This reduces cloning when strings don't change. When functions sometimes modify strings and sometimes return them unchanged, Cow avoids unnecessary copies.

Bringing It Together

Effective Rust code review recognizes patterns that work with Rust's ownership system versus those that fight it. Excessive cloning, unsafe blocks, and panics often indicate the code hasn't embraced Rust's model. Well-written Rust leverages the type system, error handling, and zero-cost abstractions.

Not every clone or unwrap requires fixing. Tests might reasonably unwrap. Cloning small values might not matter. The key is identifying patterns that affect production reliability or performance rather than optimizing code that doesn't need it.

Rust's learning curve stems from its safety guarantees, but those guarantees prevent entire classes of bugs. Code review helps teams learn Rust's patterns and understand when the compiler's suggestions lead to better designs. The borrow checker's restrictions often push toward clearer ownership and better architecture.