rust-code-review

Rust コードレビュー

レビューワークフロー

誤検知を避け、エディション固有の問題を見つけるため、次の順序に従ってください。

1. Cargo.toml を確認する — Rust エディション（2018、2021、2024）と、設定されている場合は MSRV を確認します。エディション 2024 では、unsafe のセマンティクス、RPIT のライフタイムキャプチャ、一時値のスコープ、! 型のフォールバックに破壊的変更が導入されています。どのパターンが該当するかは、これによって決まります。ワークスペース構成がある場合は、それも確認します。
2. 依存関係を確認する — 主要なクレート（thiserror と anyhow の違い、tokio の機能、serde の機能）を確認します。これらは、想定されるパターンを判断する手がかりになります。
3. 変更されたファイルを確認する — 差分だけでなく、関数全体を読みます。Rust のバグの多くは、関数内の所有権の流れに潜んでいます。
4. 各カテゴリを確認する — 以下のチェックリストを順に確認し、必要に応じてリファレンスを読み込みます。
5. 報告前に検証する — 検証プロトコルのゲートを含む、以下のゲートをすべて完了してから、指摘事項を提出します。

ゲート

これらの手順には順序があります。「頭の中での検証」で先に進んだり、手順を飛ばしたりしないでください。各ステップには、ファイルと自分の読み取り履歴から確認できる、客観的な合格条件があります。

1. クレートのコンテキスト — エディション固有のチェックリスト項目（エディション 2024、MSRV に依存する API）や依存関係に関する前提に基づいて判断する前に実施します。合格条件： 関連する Cargo.toml（パッケージまたはワークスペースのマニフェスト）を開き、edition と、設定されている場合は rust-version を一行で述べられること。
2. 対象範囲を広げて読む — 重大または致命的な指摘を報告する前に実施します。合格条件： 引用した行を含む関数、unsafe ブロック、または impl／トレイト項目の全体を読み終えていること（差分の該当箇所だけでは不十分です）。
3. 重大度の適合 — レポートに指摘事項を記載する前に毎回実施します。合格条件： 重大度の判定基準に従ってその問題の種類に合った重大度ラベルを付けるか、情報提供のみとし、その理由を一行で説明すること。
4. 検証プロトコル — レポートを確定する前に実施します。合格条件： review-verification-protocol スキルを読み込み、そのレビューに該当するすべての手順を完了していること（曖昧な「確認しました」で済ませないでください）。

出力形式

指摘事項は次の形式で報告します。

[FILE:LINE] ISSUE_TITLE
Severity: Critical | Major | Minor | Informational
Description of the issue and why it matters.

クイックリファレンス

問題の種類	参照先
所有権の移動、借用、ライフタイム、clone の落とし穴、イテレーター	references/ownership-borrowing.md
ライフタイムの分散、共変性／不変性、メモリー領域	references/lifetime-variance.md
Result／Option の処理、thiserror、anyhow、不透明なエラーと列挙型エラー、? を使った遅延クリーンアップ	references/error-handling.md
非同期処理の落とし穴、Send／Sync 境界、poll の契約、Pin の仕組み、キャンセルの健全性	references/async-concurrency.md
Send／Sync の意味、アトミック操作、メモリー順序、ロックのパターン	references/concurrency-primitives.md
メモリー順序の判断ツリー、フェンス、ABA、薄い空気からの値	references/memory-ordering.md
手書きのスピンロック、チャネル、Arc、シーケンスロック、CAS の再試行パターン	references/lock-free-patterns.md
共有メモリーとワーカープールとアクターの設計、非同期処理とスレッド、競合状態とデータ競合	references/concurrency-models.md
型レイアウト、アラインメント、repr、PhantomData、ジェネリクスと dyn Trait、ワイドポインター、自動トレイトの漏洩	references/types-layout.md
オブジェクト安全性、使いやすいトレイト実装、Deref の規律、失敗するデストラクター、隠れた契約、is_normal	references/interface-design.md
インデックスポインター、ドロップガード、拡張トレイト、クレートのプレリュード	references/patterns-in-the-wild.md
unsafe コード、API 設計、derive のパターン、clippy のパターン	references/common-mistakes.md
有効性と安全性、ドロップチェック、may_dangle、プロヴェナンス、unsafe におけるパニック安全性、MaybeUninit、Miri	references/unsafe-deep.md

パフォーマンス、ポインター型、型状態、clippy の設定、イテレーター、ジェネリクス、ドキュメントに関する開発ガイダンスが必要な場合は、rust-best-practices スキルを読み込んでください。

レビューチェックリスト

所有権と借用

• 借用チェッカーを黙らせるための不要な .clone() がない（設計上の問題を隠していない）
• ループ内で .clone() を使っていない — イテレーターでは .cloned() または .copied() を優先する
• ライフタイム注釈を避けるためにクローンしていない（明示的に所有権を取得するか、構造を見直す）
• 参照のライフタイムが適切である（より短いライフタイムで十分な場合に、過度に広い 'static を指定していない）
• Edition 2024: RPIT（-> impl Trait）は、スコープ内のすべてのライフタイムをデフォルトでキャプチャする。キャプチャを正確に制御するには + use<'a> を使う
• 関数の引数では String より &str、Vec<T> より &[T] を優先する
• 柔軟な API 引数には impl AsRef<T> または Into<T> を使う
• ダングリング参照やムーブ後の使用がない
• 内部可変性（Cell、RefCell、Mutex）は、共有された状態の変更が本当に必要な場合にのみ使う
• 小さな型（24 バイト以下）は Copy を derive し、値渡しする
• 所有権が曖昧な場合は Cow<'_, T> を使う
• コレクションの変換では、インデックスベースのループよりイテレーターチェーンを優先する
• 早すぎる .collect() を使っていない — 受け取り側が対応している場合は、イテレーターを直接渡す
• 合計の計算では .fold() より .sum() を優先する（コンパイラーによる最適化の効率が高い）
• フォールバックでメモリー確保が発生する場合は _or_else 系のバリアントを使う
• Edition 2024: if let の一時値は if let の終わりにドロップされる — 一時値が else 分岐の間も存続することに依存するコードは、構造を見直す必要がある
• Edition 2024: Box<[T]> は IntoIterator を実装している — まず into_vec() を呼ぶより、直接反復することを優先する

エラー処理

• 回復可能なエラーには Result<T, E> を使用し、panic!／unwrap／expect は使用しません
• エラー型はコンテキストを提供します（#[error("...")] を使う thiserror、または手動の Display）
• 適切な From 実装、または .map_err() と組み合わせて ? 演算子を使用します
• unwrap()／expect() はテスト、例、または安全性が明らかなコンテキストでのみ使用します
• エラーのバリアントは、呼び出し側が対処できる程度に具体的です
• アプリケーションでは anyhow、ライブラリでは thiserror を使用します（別の方法を使う場合は明確な理由を示します）
• フォールバックでメモリ確保が発生する場合は _or_else バリアントを使用します（ok_or_else、unwrap_or_else）
• 失敗時に早期リターンする場合は let-else を使用します（let Ok(x) = expr else { return ... }）
• エラーのログ記録には inspect_err、エラーの変換には map_err を使用します

トレイトと型

• トレイトは最小限で、責務がまとまっています
• 型に適した derive マクロを使用します（Clone、Debug、PartialEq を適切に使います）
• プリミティブ型への過度な依存を防ぐために Newtype を使用します（例：単なる Uuid ではなく struct UserId(Uuid)）
• From／Into の実装は情報を失わず、失敗しないものにし、失敗する変換には TryFrom を使用します
• 外部からの実装を許可しない場合は、シールドトレイトを使用します
• 適切な場合はデフォルト実装を提供します
• スレッド間で共有される型について、Send + Sync の境界を確認します
• ユーザーがトレイトの実装を忘れた場合に明確なエラーメッセージを表示するため、公開トレイトで #[diagnostic::on_unimplemented] を使用します

インターフェース設計

詳細なガイダンス：references/interface-design.md

• dyn を想定したトレイトのメソッドでは、値渡しの Self、ジェネリックパラメーター、関連定数を使用しません（または where Self: Sized で制約します）
• 新しいトレイトには &T、&mut T、Box<T> 向けの包括実装を追加し、参照やスマートポインターを使った引数にも対応します
• イテラブル型は Self だけでなく、&Self と &mut Self に対しても IntoIterator を実装します
• Deref は透過的な転送にのみ使用し、「継承」目的では決して使用しません。固有メソッドの曖昧さは、実際にバグを引き起こす要因です
• 失敗する可能性のあるクリーンアップには、Result を返す明示的な close()／shutdown() を使用します。Drop はベストエフォートのフォールバックとしてのみ使用します
• Drop 内で block_on(...) や新しいランタイムを使用しません（非同期ランタイムでデッドロックします）
• 公開型には、ビルド時に自動トレイトのリグレッションを検出できるよう、コンパイル時の fn is_normal<T: Sized + Send + Sync + Unpin>() {} テストを用意します
• 公開 API で再エクスポートされる外部型にはフラグを立てます。下流でのメジャーバージョン更新が、このクレートの破壊的変更になります
• ゲッターメソッドは fn get_name(&self) ではなく fn name(&self) の規約に従います（get_* は Option を返す場合や、特別な検索処理に限定します）
• as_* は参照から参照への低コストな変換、to_* はメモリ確保を伴う場合があり、into_* は値を消費します。コストが接頭辞と一致することを確認します
• すべての公開型で標準的な derive（Debug、Clone、Default、PartialEq、Eq、Hash）を検討します。Copy は本当に低コストで値として扱える場合にのみ使用します（後から削除すると破壊的変更になります）

実際のコードで見られるパターン

詳細なガイダンス：references/patterns-in-the-wild.md

• インデックスポインターグラフでは世代付きインデックス（slotmap::DefaultKey、petgraph::NodeIndex）を使用します。単なる usize は Vec::swap_remove の後に孤立します
• Drop ガードは let _ = ... ではなく let _guard = ... に束縛します（後者はスコープ終了時ではなく、直ちにドロップされます）
• panic = "abort" の場合に Drop ガードによるクリーンアップに依存しません（デストラクターは実行されません）
• 拡張トレイトは対象の型が外部型の場合にのみ使用します。所有する型には、直接固有 impl を使用します
• 拡張トレイトのメソッドが、同じ型にある広く使われている既存メソッドを隠さないようにします（呼び出し箇所が曖昧になります）
• クレートの prelude モジュールへの追加は semver のマイナー変更として扱います（RFC 1105）。可能であればメジャーリリースまで追加を控えます

並行性の設計（モデル）

詳細なガイダンス：references/concurrency-models.md

• 並行性モデル（共有メモリ、ワーカープール、アクター）を設計で明示し、プリミティブがモデルに適合している
• ミューテックスのクリティカルセクションは短く、測定可能で、I/O、ネットワーク呼び出し、.await の箇所を含まない
• ワーカープールのキューは有界であり、バックプレッシャー戦略が明示されている
• アクターのメールボックスチャネルは想定負荷に合わせたサイズであり、アクター間の循環によるデッドロックをレビューしている
• async を並列処理と混同していない — join! は1つのスレッド上でインターリーブし、並列化するのは（マルチスレッドランタイム上の）tokio::spawn（または同等のもの）のみ
• 高頻度にアクセスされるアトミック変数上の CAS リトライループは、fetch_add／シャーディングへの置き換えを検討している（競合時、CAS は O(N²)）
• 競合状態のデバッグ手段として println!／dbg! を使用していない（Stdout ミューテックスが競合状態を変化させる）

Unsafe コード

• unsafe ブロックには、不変条件を説明する安全性コメントがある
• unsafe は最小限であり、本当に危険な操作だけがブロック内にある
• 安全性の不変条件が文書化され、周囲の安全なコードによって維持されている
• 未定義動作がない（ヌルポインターの参照外し、データ競合、無効なメモリアクセス）
• unsafe トレイト実装では、契約が守られる理由を説明している
• Edition 2024：unsafe fn の本体では、危険な操作を明示的な unsafe {} ブロックで囲んでいる（unsafe_op_in_unsafe_fn は deny）
• Edition 2024：extern "C" {} ブロックを unsafe extern "C" {} として記述している
• Edition 2024：#[no_mangle] と #[export_name] を #[unsafe(no_mangle)] と #[unsafe(export_name)] として記述している

並行性（メモリ順序とロックフリーパターン）

詳細なガイダンス：references/memory-ordering.md、references/lock-free-patterns.md

• スレッド間で共有される型に適切な Send／Sync 境界があり、unsafe impl Send/Sync には不変条件を明記したコメントがある
• 各アトミック操作が、名前の明示された happens-before エッジ（spawn/join、同じアトミック上の Release／Acquire、またはフェンス）と対応している。Release はデータを公開し、Acquire はデータを観測する
• デフォルトで SeqCst を使用していない — 2つ以上の独立したアトミック変数に単一のグローバルな全順序が必要な場合に限り、要件を明記したコメントを添えて使用する
• store(.., Acquire)／load(.., Release)／load(.., AcqRel) を使用していない（指定位置の型の側で拒否されるため）
• 非アトミックデータの公開や観測に Relaxed を使用していない（Release／Acquire を使用する）
• compare_exchange_weak はリトライループ内で使用し、強い compare_exchange は一度限りの更新に限定している。クリティカルセクションを取得する場合、成功時の順序は少なくとも Acquire
• 手動実装のスピンロックは、ビジーウェイト中に std::hint::spin_loop()、指数バックオフ、最終的な thread::yield_now() を含む。Mutex が適さない理由を文書化していない限り、通常のユーザー空間バイナリでは使用しない
• 手動実装の Arc は、クローン時に Relaxed、ドロップ時に Release を使用し、最後のデクリメント時に fence(Acquire) を実行し、オーバーフローガードを含む
• Arc<Mutex<...>> の循環を Weak で解消している。Arc<Mutex<Copy>> は Arc<AtomicT> への置き換えを検討している
• 手動実装のロックフリープリミティブには #[cfg(loom)] のテストモジュールと、Miri で実行可能なテストがある（包括的な cfg_attr(miri, ignore) は使用しない）
• 新規コードでは once_cell／lazy_static より OnceLock／LazyLock を優先している（MSRV ≥ 1.80）
• .await をまたいで MutexGuard を保持していない（tokio::sync::Mutex を使用するか、先にガードを解放する）
• 競合するキャッシュライン上の高頻度にアクセスされるアトミック変数は、偽共有を避けるために CachePadded または #[repr(align(64))] でラップしている
• 共有ミューテーションは UnsafeCell<T> を介して行う（裸の *mut T や、& を &mut にトランスミュートしたものは使用しない）
• ポインタベースの CAS（AtomicPtr<Node>）では、エポック／ハザードポインター／タグ付きポインターによるメモリ回収を使用し、ABA の危険性を検討している

命名とスタイル

• 型には PascalCase、関数／メソッドには snake_case、定数には SCREAMING_SNAKE_CASE を使用します
• モジュールには snake_case を使用します
• 真偽値を返すメソッドには is_、has_、can_ の接頭辞を使用します
• ビルダーパターンのメソッドは、チェーン可能にするため、self を受け取り、self を返します（&mut self ではありません）
• 公開項目にはドキュメントコメント（///）を付けます
• 戻り値を無視するとバグになる可能性が高い関数には #[must_use] を付けます
• インポートは std → 外部クレート → ワークスペース → クレート／super の順に並べます
• lint の抑制には #[allow(...)] より #[expect(clippy::...)] を優先します

パフォーマンス

詳細なガイダンス：rust-best-practices スキルの references/performance.md

• ホットパスで不要な割り当てを行いません（String より &str、Vec<T> より &[T] を優先します）
• collect() の型を指定するか、推論できるようにします
• コレクションの変換には、インデックス付きループよりイテレーターを優先します
• サイズが分かっている場合は Vec::with_capacity() を使用します
• 冗長な .to_string()／.to_owned() の連鎖を避けます
• イテレーターを直接渡せる場合は、中間の .collect() を避けます
• 合計の計算には .fold() より .sum() を優先します
• 柔軟性が必要な場合を除き、動的ディスパッチ（dyn Trait）より静的ディスパッチ（impl Trait）を使用します

Clippy の設定

詳細なガイダンス：rust-best-practices スキルの references/clippy-config.md

• ワークスペースレベルの lint を Cargo.toml（[workspace.lints.clippy] または [lints.clippy]）で設定します
• #[allow(...)] より #[expect(clippy::lint)] を使用します。抑制が不要になった場合に警告が出ます
• lint を抑制する場合は、理由をコメントで記載します
• 主要な lint を適用します：redundant_clone、large_enum_variant、needless_collect、perf グループ
• cargo clippy --all-targets --all-features -- -D warnings が成功します
• ライブラリクレートではドキュメント関連の lint（missing_docs、broken_intra_doc_links）を有効にします

型状態パターン

詳細なガイダンス：rust-best-practices スキルの references/type-state-pattern.md

• 実行時の enum／真偽値ではなく、ゼロコストのコンパイル時状態機械に PhantomData<State> を使用します
• 状態遷移では self を消費して新しい状態型を返し、古い状態の再利用を防ぎます
• 各状態で適用可能なメソッドのみを使用できるようにします（無効な操作はコンパイルエラーになります）
• 安全性の向上につながる箇所でこのパターンを使用します（必須フィールドのあるビルダー、接続状態、ワークフローなど）
• 単純な状態に過剰適用しません（実行時の柔軟性が必要な場合は、単純な enum で問題ありません）

重大度の基準

Critical（マージをブロック）

• 不健全な不変条件や未定義動作を伴う unsafe コード
• 解放後使用やダングリング参照のパターン
• 本番コードでのユーザー入力または外部データに対する unwrap()
• データ競合（同期なしでの並行変更）
• 共有データを制御する atomic のメモリー順序の誤り（データ競合）
• 弱参照を使わない循環 Arc<Mutex<...>> によるメモリーリーク

Major（修正を推奨）

• コンテキストを付けずにエラーを返すこと（単に return err するのと同等）
• ホットパスで所有権設計の問題を覆い隠す .clone()
• スレッド間で使用される型に Send／Sync 境界がないこと
• ライブラリコードで回復可能なエラーに panic! を使うこと
• API 設計上の問題を覆い隠す過度に広い 'static ライフタイム

Minor（修正を検討）

• 公開項目にドキュメントコメントがないこと
• &str または impl AsRef<str> で対応できるのに、引数に String を使うこと
• derive マクロを付けるべき型に付いていないこと
• Cargo.toml の未使用のフィーチャーフラグ
• 効率の悪いイテレーターチェーン（1回で済む処理で複数回割り当てが発生すること）

Informational（注意のみ）

• ドメインモデリングのための新しい型の導入提案
• トレイト設計のリファクタリング案
• 測定による効果が確認されていないパフォーマンス最適化
• #[must_use] または #[non_exhaustive] の追加提案

参照を読み込むタイミング

• 所有権、借用、ライフタイム、クローンの落とし穴を確認する場合 → ownership-borrowing.md
• ライフタイムの分散、共変性／不変性、複数のライフタイムパラメーターを確認する場合 → lifetime-variance.md
• Result／Option の処理、エラー型、opaque と enumerated、クリーンアップの遅延、Error トレイトの実装を確認する場合 → error-handling.md
• 非同期コード、poll の契約、Pin の仕組み、キャンセルの健全性、ランタイム間の連携を確認する場合 → async-concurrency.md
• 並行処理の設計判断（共有メモリーとワーカープールとアクター、非同期とスレッド）、データ競合と競合状態を確認する場合 → concurrency-models.md
• Send／Sync、アトミック、ミューテックス、ロックパターンを確認する場合 → concurrency-primitives.md
• メモリー順序の判断、フェンス、ABA、out-of-thin-air を確認する場合 → memory-ordering.md
• 独自実装のスピンロック、チャネル、Arc、シーケンスロック、CAS の再試行パターンを確認する場合 → lock-free-patterns.md
• 型のレイアウト、アラインメント、repr、PhantomData、ワイドポインター、自動トレイトの漏出、Sized／?Sized を確認する場合 → types-layout.md
• インターフェース設計（オブジェクト安全性、使いやすいブランケット実装、Deref の規律、失敗する可能性のある／ブロッキングするデストラクター、隠れた契約、命名）を確認する場合 → interface-design.md
• インデックスとポインターのグラフ、ドロップガード、拡張トレイト、クレートのプレリュードを確認する場合 → patterns-in-the-wild.md
• unsafe コード、API 設計、derive マクロ、clippy のパターンを確認する場合 → common-mistakes.md
• 有効性と安全性の違い、drop check と may_dangle、来歴、unsafe におけるパニック安全性、MaybeUninit を確認する場合 → unsafe-deep.md
• パフォーマンス、ポインター型、型状態、ジェネリクス、イテレーター、ドキュメントを確認する場合 → rust-best-practices スキル

有効なパターン（報告しないでください）

以下は許容される Rust のパターンです。報告すると開発者の時間を無駄にします。

• テストでの .clone() — テストコードではパフォーマンスより明瞭さを優先します
• テストやサンプルでの unwrap() — 失敗時にパニックする意図がある場合は許容されます
• 単純なバイナリーでの Box<dyn Error> — すべてのアプリケーションにカスタムエラー型が必要なわけではありません
• 構造体の String フィールド — 構造体で所有データを使うのは適切です。&str フィールドにはライフタイムパラメーターが必要です
• 開発中の #[allow(dead_code)] — 反復開発では一般的です
• 新しいコードでの todo!()／unimplemented!() — 開発中の有効なプレースホルダーです
• 明確なメッセージを伴う .expect("reason") — 自己文書化されており、不変条件の確認に使用できます
• テストモジュールでの use super::* — #[cfg(test)] モジュールの標準的なパターンです
• 複雑な型の型エイリアス — type Result<T> = std::result::Result<T, MyError> は慣用的です
• 戻り値の位置での impl Trait — ゼロコスト抽象化であり、標準的なパターンです
• ターボフィッシュ構文 — 型推論の補助が必要な場合の collect::<Vec<_>>() は慣用的です
• 意図的に未使用の変数に付ける _ プレフィックス — コンパイラーの慣例です
• 理由を添えた #[expect(clippy::...)] — リント抑制が不要になると自動的に解消されます
• Arc::clone(&arc) — 明示的な Arc のクローンは慣用的で、推奨されます
• 非同期コードの短いクリティカルセクションでの std::sync::Mutex — Tokio のドキュメントでも推奨されています
• イテレーターに対する for ループ — 早期終了や副作用が必要な場合に使用します
• トレイト定義内の async fn — 1.75 から安定しています。dyn Trait または MSRV が 1.75 より前の場合にのみ async-trait クレートが必要です
• 標準ライブラリーの LazyCell／LazyLock — 1.80 から安定しています。新しいコードでは once_cell と lazy_static の代わりになります
• + use<'a, T> の正確なキャプチャ構文 — RPIT のライフタイムキャプチャを制御するための Edition 2024 の構文です

コンテキストに応じたルール

次の問題は、具体的な条件に該当する場合にのみ指摘してください。

問題	次の場合にのみ指摘
エラーコンテキストの不足	エラーがコンテキストなしでモジュール境界を越える
不要な .clone()	テスト／セットアップコードではなく、ホットパスまたは繰り返し呼び出される箇所にある
ドキュメントコメントの不足	項目が pub であり、#[cfg(test)] モジュール内ではない
unwrap() の使用	テスト／サンプル／安全性が明らかな箇所ではなく、本番コードパスにある
Send + Sync の不足	型が実際にスレッド／タスク境界を越えて共有される
広すぎるライフタイム	より短いライフタイムで対応でき、かつ API が公開されている
#[must_use] の不足	関数が、呼び出し側が無視しがちな値を返す
古い #[allow] 抑制	リント管理を自己修復可能にするため、#[expect] にすべき
Copy derive の不足	型が 24 バイト以下で、すべてのフィールドが Copy であり、頻繁に使用される
Edition 2024：! 型のフォールバック	Result<T, !> に対する match、または () へのフォールバックを前提とした発散式を使用している — ! は () ではなく ! にフォールバックするようになりました
Edition 2024：r#gen 識別子	コードで gen を識別子として使用している — edition 2024 では予約語のため、r#gen にする必要がある

指摘を提出する前に

Gates § 検証プロトコル（手順 4）の要件を満たしてください。問題を報告する前に、review-verification-protocol スキルを読み込み、その指示に従ってください。