効果的なコードレビューのためのDiffの読み方

コードレビューのためのDiffの読み方

コードレビューはDiffツールの最も一般的な使用方法の一つです。Diffを素早く読み、潜在的な問題を特定できることは、開発者にとって重要なスキルです。このガイドでは効率的なDiffベースのコードレビューの手法を解説します。

Diff読み取り戦略

1. サマリーから始める — 変更されたファイル数、追加/削除行数
2. ファイルリストを読む — 変更の範囲を理解
3. 重要なファイルを優先 — ロジック変更、セキュリティに敏感なコードに注目
4. コンテキストを読む — 変更行だけでなく、周囲のコードを理解
5. テストカバレッジを確認 — 対応するテスト変更を探す

何を探すか

 function processPayment(amount) {
-  if (amount <= 0) {
+  if (amount < 0) {
     throw new Error("Invalid amount");
   }

この小さな変更はゼロ額のチェックを除去しています — 潜在的なバグです。Diffを注意深く読まなければ、この1文字の変更を見逃す可能性があります。

一般的なレビューパターン

セキュリティの問題：

-const query = db.prepare("SELECT * FROM users WHERE id = ?");
+const query = db.query("SELECT * FROM users WHERE id = " + userId);

パラメータ化クエリが文字列連結に置き換えられた — SQLインジェクションリスク。

パフォーマンスリグレッション：

 async function getUsers() {
-  return cache.get("users") || await db.query("SELECT * FROM users");
+  return await db.query("SELECT * FROM users");
 }

キャッシュが削除された — すべての呼び出しがデータベースにアクセス。

エラー処理の削除：

 try {
   await sendEmail(user.email);
-} catch (error) {
-  logger.error("Email failed", error);
-  throw error;
-}
+} catch {}

サイレントなエラー抑制 — 失敗が見えなくなる。

Diffからのレビューチェックリスト

• 変更はPR説明と一致しているか？
• セキュリティへの影響はあるか？
• エラー処理は適切か？
• 対応するテスト変更はあるか？
• 命名規則はコードベースと一致しているか？
• 設定可能にすべきハードコード値はないか？
• デッドコードが追加されていないか？

Diffナビゲーションのヒント

• 変更のないファイルを折りたたむ — 重要なものに焦点を当てる
• ファイルタイプフィルターを使用 — 設定ファイルの前にロジックファイルをレビュー
• データフローを追跡 — 変更を通じて入力がどのように処理されるかを追跡