rust-review

Compare original and translation side by side

🇺🇸

Original

English

🇨🇳

Translation

Chinese

Rust review

Rust 审查

Review Rust code against the same rules the

rust-best-practices

skill uses for writing it, and report what to change. The output is a review, not a rewrite: the reader decides what to fix. Only rewrite code when the user asks for that.

依据

rust-best-practices

skill 编写代码时采用的同一套规则审查 Rust 代码，并报告需要更改的内容。输出是审查报告，而不是重写：由读者决定要修复什么。只有在用户要求时才重写代码。

1. Decide the scope

1. 确定范围

What to read. Review exactly what the user pointed at: a file, a module, a crate, or a diff. For a diff, review the changed lines and anything they break, not the whole file.
Library or application. Check for a
```
lib.rs
```
,
```
pub
```
items used by other crates, or a published
```
Cargo.toml
```
. Library code is held to the API-design rules strictly. Application code gets the simplicity rules instead: concrete types,
```
anyhow
```
-style errors, and no speculative generality. Say which you assumed at the top of the report.
The edition and MSRV, from
```
Cargo.toml
```
, so you don't recommend features the project can't use.

要审查的内容。 严格审查用户指定的范围：文件、模块、crate 或 diff。对于 diff，只审查变更的行及其所破坏的代码，而不是整个文件。
库还是应用。 检查是否存在
```
lib.rs
```
、供其他 crate 使用的
```
pub
```
项，或已发布的
```
Cargo.toml
```
。库代码要严格遵守 API 设计规则。应用代码则适用简化规则：使用具体类型、
```
anyhow
```
风格错误，并且不做推测性的通用抽象。在报告开头说明你假设的是哪一种。
edition 和 MSRV，从
```
Cargo.toml
```
中获取，以免推荐项目无法使用的特性。

2. Run the tools first, if you can

2. 条件允许时优先运行工具

If a Rust toolchain is available, run these from the crate root and fold the results into the report. They catch the mechanical issues, which leaves you free for design.

bash

cargo fmt --check
cargo clippy --all-targets --all-features -- -W clippy::pedantic
cargo test

Don't paste raw clippy output. Group repeated warnings into one finding, and skip pedantic lints that don't matter for this code. If the tools aren't available, say so in the report and review by reading.

如果可以使用 Rust 工具链，请从 crate 根目录运行以下命令，并将结果纳入报告。这些工具可以发现机械化问题，让你专注于设计审查。

bash

cargo fmt --check
cargo clippy --all-targets --all-features -- -W clippy::pedantic
cargo test

不要直接粘贴原始 clippy 输出。将重复警告归为一项发现，并跳过与当前代码无关的 pedantic lint。如果工具不可用，请在报告中说明，并通过阅读代码进行审查。

3. What to look for, in priority order

3. 审查重点（按优先级排序）

Work through these in order. The first two categories are where real bugs live.

Correctness and soundness

Undefined behavior: an
```
unsafe
```
block without a
```
// SAFETY:
```
comment,
```
transmute
```
to cast enums or extend lifetimes,
```
unsafe impl Send
```
/
```
Sync
```
used to silence the compiler, or a safe function that can cause UB through safe calls (unsound).
Panics on reachable paths:
```
unwrap()
```
,
```
expect()
```
, indexing,
```
as
```
casts that wrap, or integer overflow, wherever input from outside can reach them.
Trait contracts: a hand-written
```
Ord
```
with a derived
```
PartialEq
```
, or
```
Hash
```
that disagrees with
```
Eq
```
. These silently corrupt
```
BTreeMap
```
,
```
HashMap
```
,
```
sort
```
, and
```
dedup
```
.
Lock hazards: a
```
MutexGuard
```
held across
```
.await
```
or returned to a caller, user closures called under a lock, or a poisoned lock recovered with
```
into_inner
```
.
Destructors that panic, block, or can fail.

Error handling

Errors that callers can't use:
```
()
```
,
```
String
```
, or
```
Box<dyn Error>
```
as a library's public error type, an error enum without
```
Display
```
and
```
std::error::Error
```
, or
```
anyhow::Error
```
in a library's public API.
Errors that lose information:
```
map_err(|_| ...)
```
, no
```
source()
```
, or messages without the path, line, or value that failed.
Panics used for expected failures, or
```
catch_unwind
```
used as control flow.

API design (strict for libraries)

Parameters:
```
&String
```
,
```
&Vec<T>
```
, or
```
&Box<T>
```
instead of
```
&str
```
,
```
&[T]
```
, or
```
&T
```
; a
```
bool
```
parameter in a public signature; ownership taken and then cloned, or a borrow cloned internally to get ownership.
Visibility: public fields that should be private,
```
Rc
```
,
```
RefCell
```
, or
```
Arc
```
in public signatures, or third-party types leaking into the API.
Missing standard traits:
```
Debug
```
on public types,
```
Display
```
on errors,
```
Default
```
next to a zero-argument
```
new()
```
, and
```
FromStr
```
instead of an ad-hoc
```
from_str
```
method.
Naming:
```
get_
```
prefixes on getters,
```
as_
```
/
```
to_
```
/
```
into_
```
that don't match their cost and ownership, and
```
Manager
```
/
```
Service
```
/
```
Util
```
names.
Evolution: public enums or structs that should be
```
#[non_exhaustive]
```
, or traits that should be sealed.

Idiom and clarity

Clones that exist only to satisfy the borrow checker.
Check-then-unwrap (
```
if x.is_some() { x.unwrap() }
```
), and
```
match
```
where a combinator is clearer, or the reverse.
String building with
```
+
```
, hand loops that are clearer as iterators, or iterator chains long enough to need a loop.
```
HashMap
```
iteration order leaking into output or tests.
```
#![deny(warnings)]
```
in source,
```
#[allow]
```
without a reason, and glob imports of external crates.

Documentation and tests

Public items without docs, and docs that restate the name.
Missing
```
# Errors
```
,
```
# Panics
```
, or
```
# Safety
```
sections where they apply.
Doc examples that use
```
unwrap()
```
instead of
```
?
```
.
Logic with no tests, or tests that only restate constants.

The

rust-best-practices

skill and its

references/

folder hold the full rules and their reasoning. If it is installed, read the relevant reference when a finding needs a rule you're unsure of. Otherwise, rely on the summary above.

按以下顺序检查。真正的 bug 最常出现在前两类中。

正确性与 soundness

未定义行为：没有
```
// SAFETY:
```
注释的
```
unsafe
```
块、用
```
transmute
```
转换枚举或延长生命周期、为消除编译器报错而使用
```
unsafe impl Send
```
/
```
Sync
```
，或者安全函数可通过安全调用导致 UB（unsound）。
可达路径上的 panic：
```
unwrap()
```
、
```
expect()
```
、索引、可能发生截断或回绕的
```
as
```
转换或整数溢出，只要外部输入可能到达这些路径。
Trait 契约：手写的
```
Ord
```
与派生的
```
PartialEq
```
搭配，或
```
Hash
```
与
```
Eq
```
不一致。这些问题会悄悄破坏
```
BTreeMap
```
、
```
HashMap
```
、
```
sort
```
和
```
dedup
```
。
锁风险：跨
```
.await
```
持有
```
MutexGuard
```
或将其返回给调用方、在持锁状态下调用用户闭包，或对 poisoned 锁调用
```
into_inner
```
进行恢复。
会 panic、阻塞或失败的析构函数。

错误处理

调用方无法使用的错误：库对外公开的错误类型为
```
()
```
、
```
String
```
或
```
Box<dyn Error>
```
，错误枚举未实现
```
Display
```
和
```
std::error::Error
```
，或在库的公开 API 中使用
```
anyhow::Error
```
。
丢失信息的错误：使用
```
map_err(|_| ...)
```
丢弃错误、缺少
```
source()
```
，或者错误消息中没有失败路径、行号或具体值。
对预期失败使用 panic，或将
```
catch_unwind
```
用作控制流。

API 设计（库代码需严格遵守）

参数：在公开签名中使用
```
&String
```
、
```
&Vec<T>
```
或
```
&Box<T>
```
，而不是
```
&str
```
、
```
&[T]
```
或
```
&T
```
；在公开签名中使用
```
bool
```
参数；取得所有权后又进行 clone，或在内部 clone 借用值以取得所有权。
可见性：本应私有的公开字段、在公开签名中暴露
```
Rc
```
、
```
RefCell
```
或
```
Arc
```
，或让第三方类型泄漏到 API 中。
缺少标准 trait：公开类型缺少
```
Debug
```
，错误缺少
```
Display
```
，零参数
```
new()
```
旁缺少
```
Default
```
，以及使用临时自定义的
```
from_str
```
方法而非
```
FromStr
```
。
命名：getter 带
```
get_
```
前缀，
```
as_
```
/
```
to_
```
/
```
into_
```
与实际成本或所有权语义不符，以及使用
```
Manager
```
/
```
Service
```
/
```
Util
```
命名。
演进：公开枚举或结构体应标记
```
#[non_exhaustive]
```
，或 trait 应被 sealed。

惯用性与清晰度

仅为了满足 borrow checker 而进行的 clone。
check-then-unwrap 模式（
```
if x.is_some() { x.unwrap() }
```
），以及在组合器更清晰时使用
```
match
```
，或反之。
用
```
+
```
拼接字符串、手写循环（改用迭代器会更清晰），或者迭代器链长到需要改用循环。
```
HashMap
```
的迭代顺序泄漏到输出或测试中。
源码中的
```
#![deny(warnings)]
```
、没有说明原因的
```
#[allow]
```
，以及对外部 crate 的 glob 导入。

文档与测试

公开项没有文档，或者文档只是复述名称。
缺少适用的
```
# Errors
```
、
```
# Panics
```
或
```
# Safety
```
章节。
文档示例使用
```
unwrap()
```
而不是
```
?
```
。
逻辑没有测试，或测试只是在复述常量。

rust-best-practices

skill 及其

references/

文件夹包含完整规则及其理由。如果该 skill 已安装，当某项发现需要用到你不确定的规则时，请阅读相关参考资料。否则，以上述摘要为准。

4. Write the report

4. 编写报告

Use this structure:

markdown

undefined

使用以下结构：

markdown

undefined

Rust review: <what was reviewed>

Must fix

1. <Short title> (

src/file.rs:42

)

1. <Short title> (

src/file.rs:42

)

Rule: <rule name and source, e.g. "Errors implement std::error::Error (C-GOOD-ERR, Rust API Guidelines)"> Problem: <what is wrong and the concrete consequence, one or two sentences> Fix:

rust

<the smallest code change that fixes it>

Rule: <rule name and source, e.g. "Errors implement std::error::Error (C-GOOD-ERR, Rust API Guidelines)"> Problem: <what is wrong and the concrete consequence, one or two sentences> Fix:

rust

<the smallest code change that fixes it>

Should fix

2. ...

Consider

3. ...

What's already good

<2 to 4 specific things the code does well>


How to sort findings:

- **Must fix** means a bug, a soundness hole, a reachable panic, or an API mistake that
  becomes a breaking change once published.
- **Should fix** means it works but violates a rule with a real cost, such as worse error
  messages, harder testing, or a confusing API.
- **Consider** means style, naming, and polish.

Rules for each finding:

- **Cite a location** as `file:line` for every finding (write `whole file` for a finding
  that has no single location, such as missing tests), and cite the rule with its source:
  an API Guidelines code such as `C-GETTER`, an Effective Rust item number, a Microsoft
  guideline code such as `M-PANIC-ON-BUG`, a clippy lint name, or "Google Rust Style
  Guide". A reader should be able to look the rule up.
- **Show the fix as code**, kept to the lines that change. Don't rewrite whole functions.
- **Group repeats.** One finding for "eight `unwrap()` calls on parse results", listing
  the lines, rather than eight findings.
- **Respect intent.** If something looks deliberate, such as a documented quirk or a
  comment explaining it, mention it under Consider with the trade-off. Don't mark it
  Must fix.
- **Keep it proportionate.** For a short file, five to ten findings is plenty. Don't pad
  the list, and don't hide a real bug among style nits.
- **"What's already good" is required.** It tells the author what to keep and makes the
  rest credible.

<2 to 4 specific things the code does well>


如何对发现进行排序：

- **Must fix** 表示 bug、soundness 漏洞、可达的 panic，或一旦发布就会
  成为 breaking change 的 API 错误。
- **Should fix** 表示代码可以正常工作，但违反了会产生实际成本的规则，例如
  错误消息更差、测试更困难或 API 令人困惑。
- **Consider** 表示风格、命名和打磨。

每项发现必须遵循以下规则：

- **为每项发现标注位置**，格式为 `file:line`；对于没有单一位置的发现，
  例如缺少测试，则写 `whole file`。同时引用规则及其来源：
  API Guidelines 中的代码，例如 `C-GETTER`；Effective Rust 的条目编号；
  Microsoft 指南代码，例如 `M-PANIC-ON-BUG`；clippy lint 名称；或
  “Google Rust Style Guide”。读者应当能够查到该规则。
- **以代码形式展示修复方案**，只保留发生变化的行。不要重写整个函数。
- **归并重复项。** 将“8 个对解析结果调用 `unwrap()` 的位置”归为一项发现并
  列出各行，而不是拆成 8 项发现。
- **尊重意图。** 如果某项做法显然是有意为之，例如有文档说明的特殊行为
  或解释原因的注释，应在 Consider 中说明权衡。不要将其标为 Must fix。
- **保持适度。** 对于短文件，五到十项发现就够了。不要凑数，也不要让真正的
  bug 淹没在风格细节中。
- **“What's already good”部分为必填项。** 它会告诉作者哪些部分应保留，
  也能让其余评价更可信。

5. Offer the lints

5. 提供 lint 配置

If the project lacks a

[lints]

table, end the report by suggesting the BeautifulRust lints config (

lints/lints.toml

and

lints/clippy.toml

in the BeautifulRust repo, https://github.com/morganlinton/BeautifulRust). It makes clippy catch most of the mechanical findings above automatically. Name two or three findings from this review that it would have caught.

如果项目缺少

[lints]

表，请在报告末尾建议使用 BeautifulRust lint 配置（BeautifulRust 仓库中的

lints/lints.toml

和

lints/clippy.toml

， https://github.com/morganlinton/BeautifulRust）。它能让 clippy 自动发现上述大多数机械化问题。指出本次审查中它本可捕获的两到三项发现。

rust-review

Original

Translation

Rust review

Rust 审查

1. Decide the scope

1. 确定范围

2. Run the tools first, if you can

2. 条件允许时优先运行工具

3. What to look for, in priority order

3. 审查重点（按优先级排序）

4. Write the report

4. 编写报告

Rust review: <what was reviewed>

Rust review: <what was reviewed>

Must fix

Must fix

1. <Short title> (
`src/file.rs:42`
)

1. <Short title> (
`src/file.rs:42`
)

Should fix

Should fix

2. ...

2. ...

Consider

Consider

3. ...

3. ...

What's already good

What's already good

5. Offer the lints

5. 提供 lint 配置

rust-review

Original

Translation

Rust review

Rust 审查

1. Decide the scope

1. 确定范围

2. Run the tools first, if you can

2. 条件允许时优先运行工具

3. What to look for, in priority order

3. 审查重点（按优先级排序）

4. Write the report

4. 编写报告

Rust review: <what was reviewed>

Rust review: <what was reviewed>

Must fix

Must fix

1. <Short title> (src/file.rs:42)

1. <Short title> (src/file.rs:42)

Should fix

Should fix

2. ...

2. ...

Consider

Consider

3. ...

3. ...

What's already good

What's already good

5. Offer the lints

5. 提供 lint 配置

1. <Short title> (
`src/file.rs:42`
)

1. <Short title> (
`src/file.rs:42`
)