Rust Code Review

Includes all guidelines from code-review skill, plus Rust-specific rules.

No

unwrap

in Non-test Code

The reviewer MUST:

• Flag every

  unwrap()

  ,

  unwrap_err()

  , or

  expect_err()

  outside tests.

Preferred alternatives:

• Use

  ?

  and propagate errors when the function returns

  Result

  .
• Handle errors explicitly (map to a domain error, log and fallback, or early return).
• Use

  expect()

  only when failure is logically impossible or globally fatal, and the message explains why.

Bad:

let cfg = read_config().unwrap();

Better:

let cfg = read_config()?;

Acceptable only with proof:

let cfg = read_config().expect(
    "config is validated and loaded at startup; reaching here means startup checks passed"
);

The reviewer MUST reject vague

expect

messages (e.g. "should not fail").

No

panic!

in Non-test Code

The reviewer MUST:

• Flag all

  panic!

  ,

  todo!

  , and

  unimplemented!

  outside tests.

The reviewer SHOULD:

• Prefer returning and propagating proper errors.
• Fail early at startup via error returns instead of panics deep in logic.
• Encourage tests to use panics and unwraps only with clear messages (e.g.

  expect("reason")

  ).

unreachable!()

Usage

The reviewer MUST:

• Flag

  unreachable!()

  unless a clear invariant explanation is provided.

It MAY be accepted if:

• The branch is genuinely impossible by construction.
• A comment documents the invariant (why this branch cannot be reached).

Otherwise, prefer returning a domain error instead of

unreachable!()

.

No Silently Ignored Errors (Non-test Code)

The reviewer MUST:

• Flag any ignored

  Result

  or

  Option

  unless the error is handled or logged, there is a clear comment explaining why ignoring is safe, or the error type is

  ()

  and this is intentional.

Bad:

let _ = do_something_fallible();
do_something_fallible().ok();

Acceptable:

if let Err(e) = do_something_fallible() {
    log::warn!("failed to do something: {}", e);
}

// Safe to ignore: telemetry failures do not affect correctness.
let _ = send_telemetry(&metrics);

Tests MAY ignore errors, but explicit assertions are encouraged.

Idiomatic Rust and Built-ins

The reviewer SHOULD:

• Suggest

  ?

  for simple error propagation instead of manual

  match

  .
• Use iterator methods (

  map

  ,

  filter

  ,

  collect

  , etc.) when they simplify logic.
• Use

  Option

  and

  Result

  combinators (

  map

  ,

  and_then

  ,

  ok_or

  , etc.) where they make code clearer.

The reviewer MUST NOT:

• Suggest overly clever refactors that hurt readability.

Performance

The reviewer SHOULD:

• Point out obvious waste such as repeated

  to_string

  or

  clone

  in hot loops, or missing

  with_capacity

  for growing collections.

The reviewer MUST:

• Favor correctness and clarity over small micro-optimizations.
• Avoid speculative performance claims without a clear reason.

Documentation and Comments

The reviewer SHOULD:

• Ensure new or changed public APIs have basic

  ///

  docs covering behavior, arguments, return values, and possible errors.
• Encourage comments where logic is non-obvious, especially around

  unsafe

  code, concurrency and ordering assumptions, or invariants the type system does not enforce.

The reviewer SHOULD NOT:

• Request redundant "code-as-English" comments.