rust-code-review

Rust Code Review

レビューとは、変更が何に応えるべきかを述べるものです。変更を求めた依頼と、リポジトリですでに守られている基準の両方に応える必要があります。このスキルでは、それぞれを独立したパスで確認し、判定を先頭に置いた1つのレポートにまとめます。ルールが対応するコード上の問題は繰り返しません。基準についてはクラフトスキルと/rust-testingが管理し、レビューではそれらを参照します。

2つの軸を個別に確認する

依頼に対する正しさとコードの作りの両方を1人のレビュアーが確認しても、どちらもうまくいきません。仕様からの逸脱が一見きれいなコードに隠れたり、スタイル上の細かな指摘が未対応のエッジケースを埋もれさせたりします。基準パスと仕様パスをそれぞれ独立して実行し、ハーネスが対応していれば並列のサブエージェントとして、そうでなければ順番に実行してから、結果をまとめます。両方のプロンプト、統合ルール、レポート形式はREVIEW-PASSES.mdにあります。

まず機械に確認させる

どちらのパスでもコードを読む前に、以下の検証手順を実行し、その出力を確認します。clippyがすでに報告している内容は、レビューで指摘する問題ではありません。誰かが実行し忘れたビルド失敗です。レビューで注目するのは、ツールでは見つけられないものです。判断が必要な点、見落とされたエッジケース、広がりすぎたスコープなどです。

スタイルを判断する前に、リポジトリのlint設定を読む

Cargo.tomlの[lints.clippy]、clippy.toml、rustfmt.toml、クレートのルートにある#![deny(...)]を確認します。リポジトリが意図的に設定したレベルに反する指摘は、問題点ではなく提案であり、そのことを明示します。リポジトリの設定より厳しいレベルで再実行し、追加で検出された項目を欠陥として報告してはいけません。コード上の問題の基準はこの設定に追加するものであり、置き換えるものではありません。リポジトリのlint設定がすでに許容している内容を指摘する場合は、その指摘は設定に関するものであり、別途提起します。

コード上の問題の基準

clippyに加えてこのスキルが扱うのは、lintでは判断できないことです。問題点、その差分での現れ方、欠陥である理由、担当するスキルをまとめた一覧はSMELLS.mdにあります。主なものは次のとおりです。

• 呼び出し箇所に説明のないcloneがある — /ownership-not-clone
• 呼び出し元が制御できる経路でunwrapまたはexpectを使っている — /rust-errors
• 外部から呼び出されない項目にpubが付いている — /rust-api-design
• enumにすべきブール値または文字列型のパラメーターがある — /type-driven-design
• async関数内でブロッキング処理をしている — /async-rust
• // SAFETY:コメントのないunsafeブロックがある — /unsafe-rust
• 動作を変更しているのに、その変更を検出できるテストがない — /rust-testing

理由が失われた後も残る#[allow(lint)]は、検出できず、永久に残ります。#[expect(lint, reason = "...")]は、記述の根拠となった状況がなくなると失敗します。これにより、抑制されたlintが古くなったとき、自身でそのことを報告するようになります。レビューでは、理由のない#[allow]は指摘事項、理由がある場合は議論の対象です。

重大度と、何も言わないこと

レベルは3つだけです。blocking（動作が誤っている、安全性に問題がある、バージョンを上げずに公開APIを破壊している）、should-fix（作者が知っておくべき実際の欠陥）、note（作者が対応しないことを選んでもよい、有益な改善提案）です。この基準に満たないものは、記録せずに除外します。細かな指摘が40件と健全性に関わるバグが1件あるレビューでは、健全性に関わるバグが埋もれてしまいます。賞賛で水増しせず、差分の内容を作者に言い直すこともしません。

指摘の形式

1行に1件、重大度の高いものから順に記載します。形式はpath:line — severity: what is wrong. What to do instead.です。

src/cache.rs:41 — blocking: guard held across .await; drop it before the fetch.
src/parse.rs:8 — should-fix: unwrap on caller input; return the ParseError instead.
src/lib.rs:102 — note: this fn takes String where &str would do.

具体的な修正方法を示せない指摘は質問であり、質問として記述します。

このスキルが行わないこと

コードを書き換えることはありません。作者から依頼された場合を除き、行うのは指摘であり、パッチではありません。ビルドできない差分をレビューすることもありません。実行できないチェックは、取り繕わずにレポートに明記します。ポートの互換性に関する取り決めを決めることもありません。それは/port-to-rustの担当です。

検証

報告する前に実行し、結果を報告に引用してください。

cargo fmt --check
cargo clippy --all-targets --all-features   # at the repo configured level; add -- -D warnings only if there is no lint config
cargo test --all-features

3 つのチェックのいずれかを実行できない場合（依存関係を取得するためのネットワークがない場合、または必須コンポーネントに nightly がない場合）は、問題がなかったかのように記載せず、実行できなかったチェックを報告してください。