Code review in Rust: what to check and how to give feedback

Introduction

Code review is an integral part of professional development. In the Rust ecosystem, where memory safety and performance are paramount, code review takes on special significance. Incorrect use of unsafe, ignoring ownership rules, or implicit panics can negate all the advantages of the language.

In this article, we will examine what to look for during code review in Rust, how to structure feedback, and which tools can help automate checks. The material will be useful for both beginners and experienced developers looking to streamline the review process.

1. Safety: checking unsafe code and implicit panics

Rust guarantees memory safety, but unsafe is a waiver of those guarantees. Every unsafe block must be carefully checked.

What to check in unsafe code?

• Pointer correctness: no dereferencing of null or dangling pointers.
• Data alignment: correct alignment when casting raw pointers.
• Aliasing: no violations of aliasing rules (e.g., simultaneous mutable and immutable borrowing).
• Invariants: whether data structure invariants are maintained (e.g., a binary heap remains a heap).

// Bad example: unsafe cast without alignment checkfn bad_unsafe(data: &[u8]) -> &[u32] {    unsafe {        std::slice::from_raw_parts(            data.as_ptr() as *const u32,            data.len() / 4        )    }}

// Good example: with alignment checkfn good_unsafe(data: &[u8]) -> Option<&[u32]> {    if data.len() % 4 != 0 || data.as_ptr() as usize % 4 != 0 {        return None;    }    Some(unsafe {        std::slice::from_raw_parts(            data.as_ptr() as *const u32,            data.len() / 4        )    })}

Implicit panics

In Rust, many operations can cause a panic: unwrap(), expect(), panic!(), division by zero, array index out of bounds. During code review, pay attention to:

• Use unwrap() only in tests or when an error is truly impossible.
• Prefer unwrap_or(), unwrap_or_else(), or handling via match.
• Check that all as conversions do not lose data (e.g., u64 as u32).

// Bad: implicit paniclet value = some_option.unwrap();

// Good: explicit handlinglet value = some_option.unwrap_or_else(|| {    log::error!("Missing expected value");    default_value()});

2. Ownership and borrowing: typical mistakes

The ownership system is the main feature of Rust. Compiler errors here are your best friends, but code review helps find logical errors that the compiler does not catch.

Excessive cloning

A common problem is cloning where a reference could be passed. This reduces performance and increases memory usage.

// Bad: cloning the entire stringfn process(data: Vec<String>) {    for item in data {        let cloned = item.clone();        do_something(&cloned);    }}

// Good: passing a referencefn process(data: &[String]) {    for item in data {        do_something(item);    }}

Unnecessary borrows and lifetimes

Check if the lifetime of references is too long. Sometimes you can simplify a function signature by removing unnecessary lifetime annotations.

// Complex: excessive lifetimesfn longest<'a>(x: &'a str, y: &'a str) -> &'a str {    if x.len() > y.len() { x } else { y }}

// If