コードレビューチェックリスト

概要

コードレビューを体系的に実施するためのチェックリストを提供します。このスキルはレビュアーがコード品質を確保し、バグを発見し、セキュリティ問題を特定し、コードベース全体の一貫性を保つのに役立ちます。

このスキルを使用するタイミング

• プルリクエストをレビューする時
• コード監査を実施する時
• チーム向けのコードレビュー基準を確立する時
• 新しい開発者にコードレビュー実践を教える時
• レビューで何も見落とさないようにしたい時
• コードレビュードキュメントを作成する時

仕組み

ステップ 1: コンテキストを理解する

コードをレビューする前に、以下を理解するのをお手伝いします:

• このコードはどの問題を解決しますか?
• 要件は何ですか?
• どのファイルが変更されましたか、そしてなぜですか?
• 関連する問題やチケットはありますか?
• テスト戦略は何ですか?

ステップ 2: 機能性をレビューする

コードが正しく動作するかを確認します:

• 述べられた問題を解決しますか?
• エッジケースは処理されていますか?
• エラーハンドリングは適切ですか?
• 論理エラーはありませんか?
• 要件と一致していますか?

ステップ 3: コード品質をレビューする

コード保守性を評価します:

• コードは読みやすく明確ですか?
• 名前は説明的ですか?
• 適切に構造化されていますか?
• 関数/メソッドは焦点が絞られていますか?
• 不要な複雑さはありませんか?

ステップ 4: セキュリティをレビューする

セキュリティ問題をチェックします:

• 入力は検証されていますか?
• センシティブデータは保護されていますか?
• SQLインジェクションのリスクはありませんか?
• 認証/認可は正しいですか?
• 依存関係は安全ですか?

ステップ 5: パフォーマンスをレビューする

パフォーマンス問題を探します:

• 不要なループはありませんか?
• データベースアクセスは最適化されていますか?
• メモリリークはありませんか?
• キャッシングは適切に使用されていますか?
• N+1 クエリ問題はありませんか?

ステップ 6: テストをレビューする

テストカバレッジを検証します:

• 新しいコードのテストはありますか?
• テストはエッジケースをカバーしていますか?
• テストは意味があるものですか?
• すべてのテストが成功していますか?
• テストカバレッジは十分ですか?

例

例 1: 機能性レビューチェックリスト

## 機能性レビュー

### 要件
- [ ] コードは述べられた問題を解決する
- [ ] すべての受け入れ基準を満たしている
- [ ] エッジケースが処理されている
- [ ] エラーケースが処理されている
- [ ] ユーザー入力が検証されている

### ロジック
- [ ] 論理エラーやバグがない
- [ ] 条件が正しい (オフバイワンエラーがない)
- [ ] ループは正しく終了する
- [ ] 再帰は適切な基本ケースを持つ
- [ ] 状態管理が正しい

### エラーハンドリング
- [ ] エラーが適切にキャッチされている
- [ ] エラーメッセージは明確で有用
- [ ] エラーがセンシティブ情報を露出しない
- [ ] 失敗した操作はロールバックされる
- [ ] ロギングが適切

### キャッチするべき問題の例:

**❌ 悪い例 - 検証がない:**
\`\`\`javascript
function createUser(email, password) {
  // 検証がない!
  return db.users.create({ email, password });
}
\`\`\`

**✅ 良い例 - 適切な検証:**
\`\`\`javascript
function createUser(email, password) {
  if (!email || !isValidEmail(email)) {
    throw new Error('Invalid email address');
  }
  if (!password || password.length < 8) {
    throw new Error('Password must be at least 8 characters');
  }
  return db.users.create({ email, password });
}
\`\`\`

例 2: セキュリティレビューチェックリスト

## セキュリティレビュー

### 入力検証
- [ ] すべてのユーザー入力が検証されている
- [ ] SQLインジェクションが防止されている (パラメータ化クエリを使用)
- [ ] XSSが防止されている (出力をエスケープ)
- [ ] CSRF保護が実装されている
- [ ] ファイルアップロードが検証されている (タイプ、サイズ、内容)

### 認証と認可
- [ ] 認証が必要な場所で実装されている
- [ ] 認可チェックが存在する
- [ ] パスワードがハッシュ化されている (平文で保存されていない)
- [ ] セッションが安全に管理されている
- [ ] トークンが適切に期限切れになる

### データ保護
- [ ] センシティブデータが暗号化されている
- [ ] API キーがハードコードされていない
- [ ] 環境変数がシークレット用に使用されている
- [ ] 個人データがプライバシー規制に従っている
- [ ] データベース認証情報が安全

### 依存関係
- [ ] 既知の脆弱な依存関係がない
- [ ] 依存関係が最新
- [ ] 不要な依存関係が削除されている
- [ ] 依存関係バージョンがピン留めされている

### キャッチするべき問題の例:

**❌ 悪い例 - SQLインジェクションリスク:**
\`\`\`javascript
const query = \`SELECT * FROM users WHERE email = '\${email}'\`;
db.query(query);
\`\`\`

**✅ 良い例 - パラメータ化クエリ:**
\`\`\`javascript
const query = 'SELECT * FROM users WHERE email = $1';
db.query(query, [email]);
\`\`\`

**❌ 悪い例 - ハードコードされたシークレット:**
\`\`\`javascript
const API_KEY = 'sk_live_abc123xyz';
\`\`\`

**✅ 良い例 - 環境変数:**
\`\`\`javascript
const API_KEY = process.env.API_KEY;
if (!API_KEY) {
  throw new Error('API_KEY environment variable is required');
}
\`\`\`

例 3: コード品質レビューチェックリスト

## コード品質レビュー

### 可読性
- [ ] コードは理解しやすい
- [ ] 変数名は説明的
- [ ] 関数名は何をするかを説明している
- [ ] 複雑なロジックにはコメントがある
- [ ] マジックナンバーが定数に置き換えられている

### 構造
- [ ] 関数は小さく焦点が絞られている
- [ ] コードは DRY 原則に従っている (Don't Repeat Yourself)
- [ ] 関心事の適切な分離
- [ ] コードスタイルが一貫している
- [ ] デッドコードやコメントアウトされたコードがない

### 保守性
- [ ] コードはモジュール化で再利用可能
- [ ] 依存関係は最小限
- [ ] 変更は後方互換性がある
- [ ] 破壊的変更が文書化されている
- [ ] 技術的負債が記録されている

### キャッチするべき問題の例:

**❌ 悪い例 - 不明確な命名:**
\`\`\`javascript
function calc(a, b, c) {
  return a * b + c;
}
\`\`\`

**✅ 良い例 - 説明的な命名:**
\`\`\`javascript
function calculateTotalPrice(quantity, unitPrice, tax) {
  return quantity * unitPrice + tax;
}
\`\`\`

**❌ 悪い例 - 関数が多すぎることをしている:**
\`\`\`javascript
function processOrder(order) {
  // 注文を検証
  if (!order.items) throw new Error('No items');
  
  // 合計を計算
  let total = 0;
  for (let item of order.items) {
    total += item.price * item.quantity;
  }
  
  // クーポンを適用
  if (order.coupon) {
    total *= 0.9;
  }
  
  // 支払いを処理
  const payment = stripe.charge(total);
  
  // メールを送信
  sendEmail(order.email, 'Order confirmed');
  
  // 在庫を更新
  updateInventory(order.items);
  
  return { orderId: order.id, total };
}
\`\`\`

**✅ 良い例 - 関心事が分離:**
\`\`\`javascript
function processOrder(order) {
  validateOrder(order);
  const total = calculateOrderTotal(order);
  const payment = processPayment(total);
  sendOrderConfirmation(order.email);
  updateInventory(order.items);
  
  return { orderId: order.id, total };
}
\`\`\`

ベストプラクティス

✅ これをする

• 小さな変更をレビューする - 小さい PR はより徹底的にレビューできます
• テストを最初にチェックする - テストが成功し、新しいコードをカバーしていることを確認
• コードを実行する - 可能な限りローカルでテストする
• 質問する - 仮定しないで、明確化を求める
• 建設的にいく - 批判するだけでなく、改善を提案する
• 重要な問題に焦点を当てる - マイナーなスタイルの問題を細かく指摘しない
• 自動化ツールを使用する - リンター、フォーマッター、セキュリティスキャナー
• ドキュメントをレビューする - ドキュメントが更新されているか確認
• パフォーマンスを考慮する - スケールと効率性を考える
• 回帰をチェックする - 既存の機能が引き続き動作することを確認

❌ これをしないで

• 読まずに承認しない - 実際にコードをレビューする
• 曖昧にしない - 具体的なフィードバックと例を提供する
• セキュリティを無視しない - セキュリティ問題は重大
• テストをスキップしない - テストされていないコードは問題を引き起こす
• 失礼にならない - 尊重して専門的に
• ゴム判を押さない - すべてのレビューは価値を加えるべき
• 疲れている時にレビューしない - 重要な問題を見落とします
• コンテキストを忘れない - より大きな状況を理解する

完全なレビューチェックリスト

レビュー前

• PR 説明と関連する問題を読む
• どの問題が解決されるのかを理解する
• CI/CD でテストが成功しているか確認
• ブランチをプルしてローカルで実行

機能性

• コードは述べられた問題を解決する
• エッジケースが処理されている
• エラーハンドリングが適切
• ユーザー入力が検証されている
• 論理エラーがない

セキュリティ

• SQLインジェクション脆弱性がない
• XSS 脆弱性がない
• 認証/認可が正しい
• センシティブデータが保護されている
• ハードコードされたシークレットがない

パフォーマンス

• 不要なデータベースクエリがない
• N+1 クエリ問題がない
• 効率的なアルゴリズムが使用されている
• メモリリークがない
• キャッシングが適切に使用されている

コード品質

• コードは読みやすく明確
• 名前は説明的
• 関数は焦点が絞られ小さい
• コードの重複がない
• プロジェクト規約に従っている

テスト

• 新しいコードにはテストがある
• テストはエッジケースをカバーしている
• テストは意味がある
• すべてのテストが成功している
• テストカバレッジが十分

ドキュメント

• コメントは「何」ではなく「なぜ」を説明している
• API ドキュメントが更新されている
• README が必要に応じて更新されている
• 破壊的変更が文書化されている
• 必要に応じてマイグレーションガイドが提供されている

Git

• コミットメッセージが明確
• マージコンフリクトがない
• ブランチが main に最新
• 不要なファイルがコミットされていない
• .gitignore が適切に設定されている

よくある落とし穴

問題: エッジケースが見落とされている

症状: コードはハッピーパスで動作するが、エッジケースで失敗 解決策: 「もし...だったら?」という質問をする

• 入力が null だったら?
• 配列が空だったら?
• ユーザーが認証されていなかったら?
• ネットワークリクエストが失敗したら?

問題: セキュリティ脆弱性

症状: コードはセキュリティリスクを露出している 解決策: セキュリティチェックリストを使用

• セキュリティスキャナーを実行 (npm audit, Snyk)
• OWASP Top 10 をチェック
• すべての入力を検証
• パラメータ化クエリを使用
• ユーザー入力を決して信頼しない

問題: テストカバレッジが不足

症状: 新しいコードにテストがないか、不十分 解決策: すべての新しいコードにテストを要求

• 関数のユニットテスト
• 機能の統合テスト
• エッジケーステスト
• エラーケーステスト

問題: コードが不明確

症状: レビュアーがコードが何をするか理解できない 解決策: 改善をリクエスト

• より良い変数名
• 説明的なコメント
• より小さな関数
• 明確な構造

レビューコメントテンプレート

変更をリクエスト

**問題:** [問題を説明]

**現在のコード:**
\`\`\`javascript
// 問題のあるコード
\`\`\`

**提案される修正:**
\`\`\`javascript
// 改善されたコード
\`\`\`

**理由:** [これがなぜ良いのか説明]

質問する

**質問:** [あなたの質問]

**コンテキスト:** [なぜ聞いているのか]

**提案:** [提案がある場合]

良いコードを褒める

**素晴らしい!** [好きなことを説明]

これが素晴らしいのは [理由を説明]

関連するスキル

• @requesting-code-review - レビュー用にコードを準備
• @receiving-code-review - レビューフィードバックに対応
• @systematic-debugging - レビューで見つかった問題をデバッグ
• @test-driven-development - コードにテストがあることを確認

追加リソース

• Google Code Review Guidelines
• OWASP Top 10
• Code Review Best Practices
• How to Review Code

---

Pro Tip: すべてのレビューにチェックリストテンプレートを使用して、一貫性と徹底性を確保しましょう。チームの特定のニーズに合わせてカスタマイズしてください!