Flutter/Dart コードレビューのベストプラクティス

Dart コードレビューのベストプラクティス は何をしますか？

Flutter/Dartアプリケーションをレビューするための、包括的でライブラリに依存しないチェックリストです。これらの原則は、使用する状態管理ソリューション、ルーティングライブラリ、DIフレームワークに関係なく適用されます。

---

1. プロジェクト全体の健全性

• 一貫したフォルダ構造（機能優先またはレイヤー優先）に従っている
• 関心の適切な分離: UI、ビジネスロジック、データレイヤー
• ウィジェットにビジネスロジックが含まれていない。ウィジェットは純粋に表示のみ
• pubspec.yaml はクリーン — 未使用の依存関係がなく、バージョンが適切に固定されている
• analysis_options.yaml に厳格なlintセットと厳格なアナライザー設定が有効になっている
• プロダクションコード内に print() 文がない — dart:developer の log() またはロギングパッケージを使用する
• 生成されたファイル（.g.dart、.freezed.dart、.gr.dart）が最新であるか、.gitignore に含まれている
• プラットフォーム固有のコードが抽象化の背後に隔離されている

---

2. Dart言語の落とし穴

• 暗黙のdynamic: 型アノテーションが不足して dynamic になる — strict-casts、strict-inference、strict-raw-types を有効にする
• Null安全の誤用: 適切なNullチェックやDart 3のパターンマッチング（if (value case var v?)）の代わりに過剰な !（バング演算子）を使用している
• 型プロモーションの失敗: ローカル変数プロモーションが機能する場所で this.field を使用している
• 例外のキャッチが広すぎる: on 句なしの catch (e) — 常に例外タイプを指定する
• Error のキャッチ: Error サブタイプはバグを示すため、キャッチすべきではない
• 未使用の async: async とマークされているが、await を一切使用していない関数 — 不必要なオーバーヘッド
• late の過剰使用: nullableまたはコンストラクタ初期化がより安全な場所で late を使用している。エラーをランタイムに先送りする
• ループ内での文字列連結: 反復的な文字列構築には + の代わりに StringBuffer を使用する
• const コンテキストにおけるミュータブルな状態: const コンストラクタクラスのフィールドはミュータブルであってはならない
• Future の戻り値を無視: await を使用するか、意図を示すために明示的に unawaited() を呼び出す
• final が機能する場所で var を使用: ローカル変数には final を、コンパイル時定数には const を優先する
• 相対インポート: 一貫性のために package: インポートを使用する
• ミュータブルなコレクションの公開: パブリックAPIは生の List/Map ではなく、変更不可能なビューを返すべきである
• Dart 3のパターンマッチングの欠如: 冗長な is チェックや手動キャストよりも、switch式と if-case を優先する
• 複数の戻り値のための使い捨てクラス: 単一使用のDTOの代わりに、Dart 3のレコード (String, int) を使用する
• プロダクションコード内の print(): dart:developer の log() またはプロジェクトのロギングパッケージを使用する。print() にはログレベルがなく、フィルタリングできない

---

3. ウィジェットのベストプラクティス

ウィジェットの分割:

• build() メソッドが約80〜100行を超える単一のウィジェットがない
• カプセル化と、変更方法（リビルド境界）の両方に基づいてウィジェットが分割されている
• ウィジェットを返すプライベートな _build*() ヘルパーメソッドは、個別のウィジェットクラスに抽出されている（要素の再利用、constプロパゲーション、フレームワークの最適化を有効にする）
• ミュータブルなローカル状態が不要な場合は、StatefulよりもStatelessウィジェットを優先する
• 抽出されたウィジェットは、再利用可能な場合は別のファイルに配置する

constの使用:

• 可能な限り const コンストラクタを使用する — 不要なリビルドを防ぐ
• 変更されないコレクションには const リテラルを使用する（const []、const {}）
• 全てのフィールドがfinalの場合、コンストラクタは const として宣言する

Keyの使用:

• リストやグリッドで状態を並べ替え全体にわたって保持するために ValueKey を使用する
• GlobalKey は控えめに使用する — ツリー全体で状態にアクセスする必要がある場合にのみ使用する
• build() 内での UniqueKey の使用を避ける — 毎フレームのリビルドを強制する
• 同一性が単一の値ではなくデータオブジェクトに基づく場合は ObjectKey を使用する

テーマとデザインシステム:

• 色は Theme.of(context).colorScheme から取得する — Colors.red や16進数値のハードコードがないこと
• テキストスタイルは Theme.of(context).textTheme から取得する — 生のフォントサイズを持つインライン TextStyle がないこと
• ダークモードの互換性が確認されている — 明るい背景を想定していないこと
• スペーシングとサイズは、マジックナンバーではなく、一貫したデザイントークンまたは定数を使用する

Buildメソッドの複雑さ:

• build() 内でネットワークコール、ファイルI/O、または重い計算を行わない
• build() 内で Future.then() または async の処理を行わない
• build() 内でサブスクリプションの作成（.listen()）を行わない
• setState() は可能な限り小さなサブツリーにローカライズする

---

4. 状態管理（ライブラリ非依存）

これらの原則は、すべてのFlutter状態管理ソリューション（BLoC、Riverpod、Provider、GetX、MobX、Signals、ValueNotifierなど）に適用されます。

アーキテクチャ:

• ビジネスロジックはウィジェットレイヤーの外部、つまり状態管理コンポーネント（BLoC、Notifier、Controller、Store、ViewModelなど）に存在する
• 状態マネージャーは依存関係を内部で構築するのではなく、注入を介して受け取る
• サービスまたはリポジトリレイヤーがデータソースを抽象化する — ウィジェットと状態マネージャーはAPIやデータベースを直接呼び出してはならない
• 状態マネージャーは単一責任を持つ — 無関係な関心事を処理する「神」マネージャーがないこと
• コンポーネント間の依存関係は、ソリューションの規則に従う:
  • Riverpodの場合: ref.watch を介したプロバイダー間の依存関係は想定内 — 循環または過度に絡み合ったチェーンのみをフラグする
  • BLoCの場合: Blocは他のBlocに直接依存すべきではない — 共有リポジトリまたはプレゼンテーションレイヤーの調整を優先する
  • その他のソリューションの場合: コンポーネント間通信については、文書化された規則に従う

不変性と値の等価性（不変状態ソリューション: BLoC、Riverpod、Reduxの場合）:

• 状態オブジェクトは不変である — copyWith() またはコンストラクタを介して新しいインスタンスが作成され、インプレースで変更されることはない
• 状態クラスは == と hashCode を適切に実装している（比較に全てのフィールドが含まれている）
• プロジェクト全体でメカニズムが一貫している — 手動オーバーライド、Equatable、freezed、Dartレコード、またはその他
• 状態オブジェクト内のコレクションは、生のミュータブルな List/Map として公開されていない

リアクティビティの規律（リアクティブミューテーションソリューション: MobX、GetX、Signalsの場合）:

• 状態は、ソリューションのリアクティブAPI（MobXの @action、Signalsの .value、GetXの .obs）を介してのみ変更される — フィールドの直接変更は変更追跡をバイパスする
• 派生値は、冗長に保存されるのではなく、ソリューションの計算メカニズムを使用する
• リアクションとディスポーザーは適切にクリーンアップされている（MobXの ReactionDisposer、Signalsのエフェクトクリーンアップ）

状態の形状設計:

• 相互に排他的な状態には、ブールフラグ（isLoading、isError、hasData）ではなく、sealed型、共用体型バリアント、またはソリューションの組み込み非同期状態型（例: Riverpodの AsyncValue）を使用する
• すべての非同期操作は、読み込み、成功、エラーを個別の状態としてモデル化する
• UIですべての状態バリアントが網羅的に処理されている — 黙って無視されるケースがない
• エラー状態は表示用のエラー情報を保持する。読み込み状態は古いデータを保持しない
• Null可能データはローディングインジケーターとして使用されない — 状態は明示的である

// BAD — ブール値フラグのスープは不可能な状態を許容する
class UserState {
  bool isLoading = false;
  bool hasError = false; // isLoading && hasError が表現可能！
  User? user;
}

// GOOD（不変アプローチ） — sealed型により不可能な状態を表現不可能にする
sealed class UserState {}
class UserInitial extends UserState {}
class UserLoading extends UserState {}
class UserLoaded extends UserState {
  final User user;
  const UserLoaded(this.user);
}
class UserError extends UserState {
  final String message;
  const UserError(this.message);
}

// GOOD（リアクティブアプローチ） — 監視可能なenum + データ、リアクティビティAPIを介したミューテーション
// enum UserStatus { initial, loading, loaded, error }
// ソリューションのobservable/signalを使用して、ステータスとデータを個別にラップする

リビルドの最適化:

• 状態コンシューマーウィジェット（Builder、Consumer、Observer、Obx、Watchなど）は、可能な限り狭いスコープにする
• 特定のフィールドが変更された場合にのみリビルドするためにセレクターを使用する — すべての状態出力時ではない
• const ウィジェットを使用して、ツリー全体へのリビルドの伝播を停止する
• 計算/派生状態はリアクティブに計算され、冗長に保存されない

サブスクリプションと破棄:

• 全ての手動サブスクリプション（.listen()）は dispose() / close() でキャンセルされている
• ストリームコントローラーは不要になったら閉じられている
• タイマーは破棄ライフサイクルでキャンセルされている
• フレームワーク管理のライフサイクルは、手動サブスクリプションよりも優先される（命令的な .listen() よりも宣言的なビルダーを使用）
• 非同期コールバック内で setState を行う前に mounted チェックを行う
• BuildContext は await 後に context.mounted（Flutter 3.7+）をチェックせずに使用しない — 古いコンテキストはクラッシュを引き起こす
• 非同期ギャップの後、ウィジェットがまだマウントされていることを確認せずに、ナビゲーション、ダイアログ、またはスキャフォールドメッセージを実行しない
• BuildContext はシングルトン、状態マネージャー、または静的なフィールドに格納しない

ローカル状態とグローバル状態:

• 一時的なUI状態（チェックボックス、スライダー、アニメーション）にはローカル状態（setState、ValueNotifier）を使用する
• 共有状態は必要な分だけ上位に持ち上げる — 過度にグローバル化しない
• 機能スコープの状態は、機能がアクティブでなくなったら適切に破棄される

---

5. パフォーマンス

不要なリビルド:

• setState() がルートウィジェットレベルで呼び出されていない — 状態変更をローカライズする
• const ウィジェットを使用してリビルドの伝播を停止する
• 独立して再描画する複雑なサブツリーの周りで RepaintBoundary を使用する
• アニメーションから独立したサブツリーには、AnimatedBuilder の child パラメーターを使用する

build() 内での高コストな操作:

• build() 内で大量のコレクションのソート、フィルタリング、またはマッピングを行わない — 状態管理レイヤーで計算する
• build() 内で正規表現をコンパイルしない
• MediaQuery.of(context) の使用は具体的にする（例: MediaQuery.sizeOf(context)）

画像最適化:

• ネットワーク画像はキャッシュを使用する（プロジェクトに適した任意のキャッシュソリューション）
• ターゲットデバイスに適切な画像解像度（サムネイルに4K画像を読み込まない）
• cacheWidth/cacheHeight を指定した Image.asset を使用して、表示サイズでデコードする
• ネットワーク画像にはプレースホルダーとエラーウィジェットが提供されている

レイジーローディング:

• 大きなまたは動的なリストには、ListView(children: [...]) の代わりに ListView.builder / GridView.builder を使用する（具体的なコンストラクタは小さな静的なリストには問題ない）
• 大規模なデータセットにはページネーションが実装されている
• Webビルドでは、重いライブラリに遅延読み込み（deferred as）を使用している

その他:

• アニメーションでの Opacity ウィジェットの使用を避ける — AnimatedOpacity または FadeTransition を使用する
• アニメーションでのクリッピングを避ける — 画像を事前にクリップする
• operator == をウィジェットでオーバーライドしない — 代わりに const コンストラクタを使用する
• 内在的な寸法ウィジェット（IntrinsicHeight、IntrinsicWidth）は控えめに使用する（余分なレイアウトパス）

---

6. テスト

テストの種類と期待:

• 単体テスト: すべてのビジネスロジック（状態マネージャー、リポジトリ、ユーティリティ関数）をカバーする
• ウィジェットテスト: 個々のウィジェットの動作、インタラクション、視覚的な出力をカバーする
• 統合テスト: 重要なユーザーフローをエンドツーエンドでカバーする
• ゴールデンテスト: デザイン上重要なUIコンポーネントのピクセルパーフェクトな比較

カバレッジ目標:

• ビジネスロジックで80%以上のラインカバレッジを目指す
• すべての状態遷移に対応するテストがあること（読み込み→成功、読み込み→エラー、リトライなど）
• エッジケースのテスト: 空の状態、エラー状態、読み込み状態、境界値

テストの分離:

• 外部依存関係（APIクライアント、データベース、サービス）はモックまたはフェイクにする
• 各テストファイルは1つのクラス/ユニットのみをテストする
• テストは実装の詳細ではなく、動作を検証する
• スタブは各テストに必要な動作のみを定義する（最小限のスタブ化）
• テストケース間で可変状態を共有しない

ウィジェットテストの品質:

• pumpWidget と pump を非同期操作に正しく使用する
• find.byType、find.text、find.byKey を適切に使用する
• タイミングに依存する不安定なテストがない — pumpAndSettle または明示的な pump(Duration) を使用する
• テストはCIで実行され、失敗はマージをブロックする

---

7. アクセシビリティ

セマンティックウィジェット:

• 自動ラベルが不十分な場合に、スクリーンリーダーラベルを提供するために Semantics ウィジェットを使用する
• 純粋に装飾的な要素には ExcludeSemantics を使用する
• 関連するウィジェットを単一のアクセス可能な要素に結合するために MergeSemantics を使用する
• 画像には semanticLabel プロパティが設定されている

スクリーンリーダーサポート:

• すべてのインタラクティブ要素はフォーカス可能で、意味のある説明がある
• フォーカス順序は論理的である（視覚的な読み取り順序に従う）

視覚的なアクセシビリティ:

• テキストと背景のコントラスト比が4.5:1以上である
• タップ可能なターゲットは少なくとも48x48ピクセルである
• 色が状態の唯一の指標ではない（アイコン/テキストを併用する）
• テキストはシステムのフォントサイズ設定に応じてスケーリングされる

インタラクションのアクセシビリティ:

• 何もしない onPressed コールバックがない — すべてのボタンは何かを行うか、無効になっている
• エラーフィールドは修正を提案する
• ユーザーがデータを入力している間、コンテキストが予期せず変更されない

---

8. プラットフォーム固有の考慮事項

iOS/Androidの違い:

• 適切な場所でプラットフォームアダプティブウィジェットを使用する
• 戻るナビゲーションが正しく処理されている（Androidの戻るボタン、iOSのスワイプで戻る）
• ステータスバーとセーフエリアは SafeArea ウィジェットを介して処理されている
• プラットフォーム固有の権限が AndroidManifest.xml と Info.plist で宣言されている

レスポンシブデザイン:

• レスポンシブレイアウトには LayoutBuilder または MediaQuery を使用する
• ブレークポイントが一貫して定義されている（電話、タブレット、デスクトップ）
• テキストが小さい画面でオーバーフローしていない — Flexible、Expanded、FittedBox を使用する
• 横向きの向きがテストされているか、明示的にロックされている
• Web固有: マウス/キーボードのインタラクションがサポートされ、ホバー状態が存在する

---

9. セキュリティ

安全なストレージ:

• 機密データは flutter_secure_storage またはキーチェーン/キーストアに保存される — SharedPreferences やプレーンテキストファイルには保存しない
• 認証トークンはメモリ内で安全に管理され、ログ出力されない

ネットワークセキュリティ:

• HTTPS がすべてのAPI通信に使用されている
• 証明書のピン留めが適切な場合に実装されている
• 転送中の機密データに対して追加の暗号化層が実装されている

入力の検証:

• すべてのユーザー入力は検証されている
• 出力はエンコードされて、クロスサイトスクリプティング（XSS）やインジェクションを防ぐ

---

10. アーキテクチャとパフォーマンス

クリーンアーキテクチャ:

• 依存関係の方向: 外部レイヤーは内部レイヤーに依存するが、その逆はない
• 抽象化への依存: 具象実装ではなく、インターフェース/抽象クラスに依存する
• クリーンなレイヤー境界: プレゼンテーション、ドメイン、データレイヤーは明確に分離されている
• データフローは一貫している: UI→Bloc/Notifier→Repository→DataSource

コードの匂い:

• 長いメソッド: 100行を超えるメソッドはリファクタリングする
• 多数のパラメータ: メソッドが5つ以上のパラメータを持つ場合はリファクタリングを検討する
• 重複コード: DRY原則に違反している

---

チェックリストの使用方法

1. レビューするコードに該当するセクションにチェックを入れる
2. プロジェクトで使用されていない状態管理ライブラリの特定のチェックはスキップする
3. このリストは出発点として使用し、プロジェクト固有のガイドラインで補完する
4. 新しいパターンが見つかった場合は、このリストを拡張する

このチェックリストは、Flutter/Dartコードレビューのためのライブラリに依存しないベースラインを提供することを目的としています。レビューを実施する際は、常にプロジェクトの特定のガイドラインと、レビュー対象のコードのコンテキストを考慮してください。