rust-review

Rust のレビュー

Rust コードを、rust-best-practices スキルでコードを書くときに使うルールと同じ基準でレビューし、変更すべき点を報告します。出力はレビューであり、書き直しではありません。修正するかどうかは読者が判断します。ユーザーから依頼があった場合に限り、コードを書き直してください。

1. 対象範囲を決める

• 何を読むか。 ユーザーが指定した対象（ファイル、モジュール、クレート、差分）だけをレビューしてください。差分の場合は、変更された行と、それによって問題が生じる箇所をレビューし、ファイル全体は対象にしないでください。
• ライブラリかアプリケーションか。 lib.rs、他のクレートから使われる pub アイテム、公開済みの Cargo.toml があるかを確認してください。ライブラリのコードには API 設計のルールを厳格に適用します。アプリケーションのコードには、代わりにシンプルさのルールを適用します。具体的な型、anyhow スタイルのエラー、推測に基づく過度な汎用化を避けることが含まれます。レポートの冒頭で、どちらを前提にしたかを明記してください。
• エディションと MSRV。 Cargo.toml を確認し、プロジェクトで使えない機能を推奨しないようにしてください。

2. 可能であれば、まずツールを実行する

Rust ツールチェーンを利用できる場合は、クレートのルートディレクトリから以下を実行し、その結果をレポートに反映してください。機械的な問題を検出できるため、設計に集中できます。

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

Clippy の生の出力を貼り付けないでください。繰り返し出る警告は1つの指摘にまとめ、このコードに関係のない pedantic lint は省いてください。ツールを利用できない場合は、その旨をレポートに記載し、コードを読んでレビューしてください。

3. 優先順位に沿って確認する項目

以下の順に確認してください。最初の2つのカテゴリに、実際のバグが潜んでいます。

正確性と健全性

• 未定義動作：// SAFETY: コメントのない unsafe ブロック、列挙型のキャストやライフタイムの延長に使われる transmute、コンパイラーを黙らせるための unsafe impl Send／Sync、または安全な呼び出しを通じて未定義動作を引き起こす可能性のある安全な関数（健全性の欠如）。
• 到達可能な経路でのパニック：外部からの入力が到達できる箇所にある unwrap()、expect()、インデックスアクセス、値がラップする as キャスト、または整数オーバーフロー。
• トレイトの契約：手書きの Ord と導出された PartialEq の組み合わせや、Eq と一致しない Hash。これらは BTreeMap、HashMap、sort、dedup を気づかないうちに破損させます。
• ロックに関する危険：.await をまたいで保持される MutexGuard や、呼び出し元に返される MutexGuard、ロック中に呼び出されるユーザー提供のクロージャー、または into_inner で復旧されるポイズン状態のロック。
• パニックを起こす、ブロックする、または失敗する可能性のあるデストラクター。

エラー処理

• 呼び出し元が活用できないエラー：ライブラリの公開エラー型としての ()、String、Box<dyn Error>、Display と std::error::Error を実装していないエラー列挙型、またはライブラリの公開 API における anyhow::Error。
• 情報が失われるエラー：map_err(|_| ...)、source() がない、または失敗したパス、行、値を含まないメッセージ。
• 想定される失敗に対してパニックを使うこと、または制御フローとして catch_unwind を使うこと。

API 設計（ライブラリでは厳格に確認）

• パラメーター：&str、&[T]、&T ではなく &String、&Vec<T>、&Box<T> を使うこと、公開シグネチャの bool パラメーター、所有権を受け取ってからクローンすること、または所有権を得るために借用を内部でクローンすること。
• 可視性：非公開にすべき公開フィールド、公開シグネチャに含まれる Rc、RefCell、Arc、または API に漏れ出すサードパーティー型。
• 標準トレイトの不足：公開型の Debug、エラーの Display、引数なしの new() と併せた Default、および独自の from_str メソッドではなく FromStr。
• 命名：ゲッターの get_ プレフィックス、コストや所有権と一致しない as_／to_／into_、および Manager／Service／Util という名前。
• 将来の拡張性：#[non_exhaustive] にすべき公開列挙型や構造体、または封印すべきトレイト。

慣用性と明瞭さ

• 借用チェッカーを満たすことだけを目的としたクローン。
• チェックしてから unwrap する処理（if x.is_some() { x.unwrap() }）、コンビネーターを使うほうが明瞭な match、またはその逆。
• + を使った文字列構築、イテレーターのほうが明瞭な手書きループ、またはループにしたほうがよいほど長いイテレーターチェーン。
• HashMap の反復順序が出力やテストに影響すること。
• ソースコードでの #![deny(warnings)]、理由のない #[allow]、および外部クレートの glob インポート。

ドキュメントとテスト

• ドキュメントのない公開アイテム、および名前を言い換えただけのドキュメント。
• 該当する場合に不足している # Errors、# Panics、# Safety セクション。
• ? ではなく unwrap() を使うドキュメント例。
• テストのないロジック、または定数を言い換えただけのテスト。

rust-best-practices スキルとその references/ フォルダーには、ルール全体とその理由が記載されています。インストールされている場合は、判断に迷う指摘に関連するリファレンスを読んでください。そうでない場合は、上記の概要に従ってください。

4. レポートを作成する

次の構成を使用します。

# Rust review: <what was reviewed>

<One paragraph: what the code does, library or application, overall verdict, and the
single most important thing to fix. Say whether clippy and tests were run.>

## Must fix
### 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>
```

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

## Consider
### 3. ...

## What's already good
- <2 to 4 specific things the code does well>

指摘事項の分類方法：

• 必ず修正は、バグ、健全性の欠如、到達可能なパニック、または公開後に破壊的変更となる API の誤りを意味します。
• 修正を推奨は、動作はするものの、エラーメッセージの悪化、テストの難しさ、分かりにくい API など、実際のコストを伴うルール違反を意味します。
• 検討は、スタイル、命名、仕上げに関するものです。

各指摘事項のルール：

• 場所を記載します。 各指摘事項に file:line の形式で場所を記載します。テストの不足など、単一の場所を特定できない指摘事項には whole file と記載し、出典とともにルールを示します。出典には、C-GETTER のような API Guidelines のコード、Effective Rust の項目番号、M-PANIC-ON-BUG のような Microsoft のガイドラインコード、clippy lint 名、または「Google Rust Style Guide」を使用します。読者がルールを調べられるようにしてください。
• 修正方法をコードで示し、変更する行だけにします。 関数全体を書き換えないでください。
• 同じ指摘はまとめます。 たとえば、parse 結果に対する unwrap() 呼び出しが 8 か所ある場合は、8 件の指摘事項に分けず、行番号を列挙した 1 件の指摘事項にします。
• 意図を尊重します。 文書化された仕様や説明コメントなど、意図的と思われるものは、トレードオフとともに「検討」として記載します。「必ず修正」とはしないでください。
• 適切な分量にします。 短いファイルなら、指摘事項は 5〜10 件で十分です。無理に件数を増やさず、実際のバグをスタイル上の小さな指摘に紛れ込ませないでください。
• 「すでに良い点」は必須です。 著者が維持すべき点を伝えることで、残りの指摘事項にも信頼性が生まれます。

5. lint を提案する

プロジェクトに [lints] テーブルがない場合は、レポートの最後に BeautifulRust の lint 設定を提案します（BeautifulRust リポジトリの lints/lints.toml と lints/clippy.toml、https://github.com/morganlinton/BeautifulRust）。この設定を使うと、上記の機械的な指摘事項の大半を clippy が自動的に検出できます。このレビューで挙げた指摘事項のうち、検出できたはずのものを 2〜3 件示してください。