The reviewer's idiom checklist

This is the artefact you keep. Everything else in this course is the justification for one line of it.

It is organised by what a reviewer actually reads, in order — signatures, then allocation, then control flow, then naming — because that is the order in which problems get cheaper to fix. A wrong signature costs every caller; a clumsy loop costs nothing but a rewrite.

Two rules for using it. Keep it to one screen — a checklist that scrolls stops being used, and this one is already at the limit. And respect the contested markers: a checklist that presents opinion as fact trains dogma, and dogma is worse than not having a checklist at all.

1. Signatures first

The signature is the API. It is the part you cannot change later without breaking callers, and it is the part that tells you whether the author thought about ownership at all.

• &str, not &String. A &String accepts strictly fewer things and derefs to &str anyway. Clippy enforces this (ptr_arg, default warn).
• &[T], not &Vec<T>. Same argument. Same lint.
• Ownership should match intent. Taking T by value says “I will consume or store this”. Taking &T says “I only need to look”. Taking &mut T says “I will change your copy”. A function that takes String and only reads it is asking every caller to allocate for nothing — and one that takes &str and immediately calls .to_string() has just moved the allocation somewhere less visible. No lint catches either.
• impl Trait in argument position is fine for one-off convenience and wrong when callers might want to turbofish, because APIT has no name to turbofish with.
• Box<dyn Trait> where a closed enum would do. If you own every implementor, an enum gives you exhaustiveness checking and no allocation. If downstream crates need to add implementors, you need the trait object. Ask which one it is.
• -> impl Trait leaks the concrete type into your semver contract more than people expect: auto traits like Send are inherited from whatever you actually returned, so changing the body can break callers.

2. Then allocation

Cheap to spot, and where most easy performance lives.

• No clone() inside a loop. Ask what it is for. Usually the answer is “to get past a borrow error”, and usually the borrow error had a better answer.
• No to_string() just to compare. a == b works on &str. a.to_string() == b.to_string() allocates twice to learn the same fact.
• with_capacity when the size is known, especially before a collect over filter or flat_map, whose size_hint cannot help.
• Cow<str> when the value is usually borrowed and occasionally owned — the sanitise-if-needed shape. Do not reach for it when it is always one or always the other; it costs a match at every use.
• collect() you immediately iterate again is a whole Vec you did not need.

3. Then control flow

• Iterator chains over index loops, when the chain is shorter. When it is longer, the loop was right; needless_range_loop is a suggestion, not a verdict, and it sometimes suggests code that does not compile.
• if let / let ... else over a match with a dead arm. let Some(x) = opt else { return }; is the single biggest readability win in modern Rust.
• ? over nested match. If you see three levels of match on Result, ask what From impl is missing.
• matches! over a match that returns true in one arm and false in the other.
• Early return over rising indentation. Rust makes this easy and people still do not do it.

4. Then naming, and what each name promises about cost

This is the compressed form of half this course, and it is the entry most worth memorising, because the conventions are about cost, not about style.

prefix	ownership	cost
as_	borrowed → borrowed	free, a view or a cast (as_str, as_slice, as_bytes)
to_	borrowed → owned	allocates (to_string, to_vec, to_owned)
into_	owned → owned	consumes, usually free (into_iter, into_bytes)
from_	constructor	T::from(x)
new	the obvious constructor	takes no configuration
with_*	a configured constructor	with_capacity, with_hasher
iter / iter_mut / into_iter	&T / &mut T / T	the three iteration forms

A method called as_something that allocates is a lie, and readers will believe it. This is why str::to_string and str::as_bytes are named differently: one is an allocation and one is a pointer cast, and the name is the only warning you get.

Also: is_/has_ for predicates; len always pairs with is_empty; getters are field(), not get_field(); a _mut suffix for the exclusive variant.

5. Then the lints, and which level they need

Knowing which level a rule lives at is what lets you argue about it properly.

• Deny by default (correctness) — clippy calls these bugs, and they are: mut_from_ref, not_unsafe_ptr_arg_deref, derive_ord_xor_partial_ord, almost_swapped. An #[allow] here is nearly always wrong.
• Warn by default (style, complexity, perf, suspicious) — ptr_arg, needless_range_loop, redundant_closure, manual_is_multiple_of, missing_safety_doc, macro_metavars_in_unsafe. Usually right, occasionally not; #[allow] with a one-line reason is normal.
• Off by default (pedantic) — must_use_candidate, wildcard_enum_match_arm, match_same_arms, too_many_lines, multiple_unsafe_ops_per_block, undocumented_unsafe_blocks. Turn on deliberately, per project, and expect to allow some.
• Off by default (restriction) — unwrap_used, expect_used, panic, indexing_slicing. These are not “good practice”; they are opt-in policies for code that must not panic. Enabling restriction wholesale is a known mistake.
• Off by default (nursery) — redundant_clone, significant_drop_tightening. Real insight, occasional false positives.

6. The entries no lint catches — memorise these

Everything above can be partly automated. These cannot, which is exactly why they are the ones worth carrying in your head.

• sort_by_cached_key vs sort_by_key. If the key computation allocates or is expensive, sort_by_key recomputes it O(n log n) times and sort_by_cached_key computes it once per element. No lint will ever tell you which you wanted.
• Container choice. Vec versus HashMap versus BTreeMap versus VecDeque versus a sorted Vec with binary search. A linear scan over a 20-element Vec beats a HashMap and always will; a linear scan over 200,000 does not.
• Struct layout and field types. Vec<Option<Box<T>>> versus three parallel vectors versus an index-based arena. String where Box<str> would save a word per element, times a million elements.
• Whether the ownership model matches the problem. The deepest one, and completely invisible to tooling. A design that needs Rc<RefCell<_>> everywhere is usually a design where the ownership tree was drawn wrong.

7. The contested entries

Mark these as contested when you raise them, or you are asserting taste as law.

• Box<dyn Error> versus a custom error enum. Applications generally want the former (or anyhow); libraries generally want the latter, because callers need to match. “Generally” is doing real work in that sentence, and the boundary between application and library is fuzzier than the rule admits.
• impl AsRef<str> versus &str in arguments. More ergonomic for callers, more monomorphisation, worse error messages, harder to read. Both camps are large.
• Rc<RefCell<_>>. One camp treats it as a smell that almost always signals a missing arena or a mis-modelled ownership tree. The other treats it as entirely reasonable for GUI trees, interpreters and observer patterns, where the runtime cost is irrelevant and the alternative is a hand-rolled unsafe graph that will be worse. The actual decision criterion: RefCell panics are a runtime failure mode you are choosing to accept. That is right when the borrow pattern is genuinely dynamic, and wrong when you are using it to avoid thinking about ownership.
• sort_unstable by default. Faster and allocation-free, but reorders equal elements. Fine for primitives; a real behaviour change when the elements carry data your comparator ignores.
• Whether #[must_use] belongs on everything that returns a value. must_use_candidate is pedantic for a reason.

How to use this in an actual review

Read the signatures. Read the allocations. Read the control flow. Say which level each comment lives at — “this is a correctness lint”, “this is my taste”, “this is contested and I lean the other way” — because a reviewer who does not distinguish those trains everyone around them to argue about the wrong things.

And if a comment does not fit on this list, it is probably about the problem rather than the Rust, which usually means it is the more important comment.