Rust Review

A review reports what to change and why; the author decides what to fix. Do not edit the code under review unless the request asks for a fix. This skill holds the method and the report. The rules live in the skills that own them: the table in Where to look names the owner of each area, and the check to run when that skill is not installed.

Scope

1. Read what the request names. For a diff, read the changed lines and the code they can break: callers of a changed signature, invariants of a changed type, and tests of a changed behavior. git diff --stat <base>...HEAD lists the files; git diff <base>...HEAD shows the lines. Do not review unchanged code that the diff cannot affect.
2. Decide library or application. A crate that other crates use (a lib.rs, publish not set to false, or a path dependency of another crate) gets the API rules in full. An application gets the simpler rules: concrete types, one error type with context, no speculative generics. Say which one you assumed in the report.
3. Read the edition and the MSRV (edition, rust-version in Cargo.toml). Do not propose an API or a syntax that is newer than the MSRV.
4. Respect the framework. A framework convention wins over generic advice: system parameters by value in Bevy, a framework prelude glob, an attribute macro that generates pub items. At most, report it under Consider.

Run the tools first

cargo clippy and cargo test run code from the reviewed tree: build scripts, procedural macros, and the tests themselves. Before you run them on code from someone you do not trust, such as a pull request from outside the team, read build.rs, the proc-macro crates, and the changed tests, and get the user's approval. Run them in a sandbox with no secrets and no network. Without that approval, review by reading, and say so in the report.

When the code is trusted, or after approval, run these from the workspace root:

cargo fmt --all -- --check
cargo clippy --locked --workspace --all-targets -- -D warnings
cargo test --locked --workspace

• Do not run cargo fmt without --check: it rewrites the code under review.
• Report each command and its result. Group clippy output by lint, with the lines; do not paste it raw. A lint that the project allows on purpose, with a reason, is not a finding.
• If a tool cannot run (no toolchain, no network for a dependency, no approval), say so in the report and review by reading.

Where to look

Work through the areas in this order. The first four are where real defects live.

For a full API pass on a library, read the review checklist reference of the rust-discipline skill when it is installed; otherwise run clippy::pedantic once and use the API shape row above.

Severity

A Must fix needs evidence: a failing test, a tool line, or the input that triggers it. Without evidence, put it under Should fix and mark it "not verified".

The report

# Review: <what was reviewed>

<One paragraph: what the code does, library or application, the verdict, and the one thing to
fix first. List the commands that ran and their results, or say that none could run.>

## Must fix

### 1. <Short title> (`src/file.rs:42`)
Rule: <owner skill, and the rule code where one exists: C-GETTER, clippy::ptr_arg, E0502>
Problem: <what goes wrong, and for which input or caller>
Fix:
<the smallest change, as a code block of the lines that change>

## Should fix
### 2. ...

## Consider
### 3. ...

## Already good
- <two to four specific things the code does well>

• Give every finding a location (file:line, or "whole crate" for a missing test suite) and a rule that a reader can look up.
• Show the fix as code, kept to the lines that change. Do not rewrite whole functions.
• Group repeats: one finding for "four unwrap() calls on parsed input", with all the lines.
• Keep it in proportion: five to ten findings for one file. Do not bury a Must fix under style notes, and do not pad the list.
• When code looks deliberate (a comment explains it, or a test pins it), name the trade-off under Consider instead of calling it a defect.
• "Already good" is required. It tells the author what to keep, and it shows that the reviewer read the code.