この記事では、AIを活用したコードレビューの効率化に挑戦し、コードレビューにおける効果的なプロンプトを作成したのでその内容を共有します。
以下の記事の続きとなりますので、こちらを読まれてからの方がより理解が捗るかと思います。
この記事でわかること
- AIをつかったコードレビューがどんな精度なのか
- AIを使ったコードレビューを効果的に行うためにできること
前提
- 使っているツールは、「AWS Bedrock Claude3.5 Sonnet v1」、「Continue(VScode拡張機能)」、「VScode」
- コードレビューしてほしいファイルは指定する
import React, { useState, useEffect } from 'react';
import { useParams, useNavigate, Link } from 'react-router-dom';
import { generateClient } from 'aws-amplify/api';
import { Schema } from '../../amplify/data/resource';
import './PostDetail.css';
const client = generateClient<Schema>();
type PostMessage = Schema['PostMessage']['type'];
const PostDetail: React.FC = () => {
const { postId } = useParams<{ postId: string }>();
const navigate = useNavigate();
const [post, setPost] = useState<PostMessage | null>(null);
const [replies, setReplies] = useState<PostMessage[]>([]);
const [newReply, setNewReply] = useState('');
const [userName, setUserName] = useState('');
const [showReplyForm, setShowReplyForm] = useState(false);
const [isLoading, setIsLoading] = useState(true);
useEffect(() => {
if (postId) {
fetchPost(postId);
fetchReplies(postId);
}
}, [postId]);
const fetchPost = async (id: string) => {
try {
setIsLoading(true);
const result = await client.models.PostMessage.get({ id });
if (result.data) {
setPost(result.data);
}
} catch (error) {
console.error('Error fetching post:', error);
} finally {
setIsLoading(false);
}
};
const fetchReplies = async (parentId: string) => {
try {
setIsLoading(true);
const result = await client.models.PostMessage.list({ filter: { parentId: { eq: parentId } } });
setReplies(result.data);
} catch (error) {
console.error('Error fetching replies:', error);
} finally {
setIsLoading(false);
}
};
const handleReply = async (e: React.FormEvent) => {
e.preventDefault();
if (newReply.trim() && userName.trim() && postId) {
try {
const result = await client.models.PostMessage.create({
message: newReply.trim(),
userName: userName.trim(),
parentId: postId,
good: 0
});
if (result.data) {
setReplies([result.data, ...replies]);
setNewReply('');
setUserName('');
setShowReplyForm(false);
}
} catch (error) {
console.error('Error creating reply:', error);
}
}
};
const handleLike = async () => {
if (post) {
try {
const result = await client.models.PostMessage.update({
id: post.id,
good: (post.good || 0) + 1
});
if (result.data) {
setPost(result.data);
}
} catch (error) {
console.error('Error updating like:', error);
}
}
};
if (isLoading) return <div>Loading...</div>;
if (!post) return <div>Post not found</div>;
return (
<div className="post-detail">
<button onClick={() => navigate('/')}>戻る</button>
<div className="post">
<h2>{post.userName}</h2>
<p>{post.message}</p>
<p>投稿日時: {new Date(post.createdAt).toLocaleString()}</p>
<p>更新日時: {new Date(post.updatedAt).toLocaleString()}</p>
<button onClick={handleLike}>👍 {post.good || 0}</button>
<button onClick={() => setShowReplyForm(!showReplyForm)}>
{showReplyForm ? '返信をキャンセル' : '返信する'}
</button>
</div>
{showReplyForm && (
<form onSubmit={handleReply} className="reply-form">
<input
type="text"
value={userName}
onChange={(e) => setUserName(e.target.value)}
placeholder="ユーザー名"
required
/>
<textarea
value={newReply}
onChange={(e) => setNewReply(e.target.value)}
placeholder="返信を入力"
required
/>
<button type="submit">返信する</button>
</form>
)}
{replies.length > 0 && (
<div className="replies">
<h3>返信</h3>
{replies.map(reply => (
<div key={reply.id} className="reply">
<Link to={`/post/${reply.id}`} className="reply-link">
<h4>{reply.userName}</h4>
<p>{reply.message}</p>
<p>投稿日時: {new Date(reply.createdAt).toLocaleString()}</p>
<p>👍 {reply.good || 0}</p>
</Link>
</div>
))}
</div>
)}
</div>
);
};
export default PostDetail;
やったこと
まずは、ネットにあるAIコードレビューの記事を調べ、その中で使われているプロンプトを分析しました。
結果、記事のプロンプトの指示内容には大きく次の3つのパターンがあることがわかりました。
次に、標準となるコードレビュープロンプトを作成し、そこに①②③の条件の組み合わせを追加して、出力されたレビュー結果の良し悪しを目視で確認しました。
標準コードプロンプト決め
最初は、次のプロンプトで特定の1ファイルのコードレビューを行いました。
@PostDetail.tsx コードレビューして
出されたレビュー結果は、文章の説明のみで一切コードがありませんでした。
PostDetail.tsx のコードレビューを行います。全体的に見て、よく構造化されたコンポーネントですが、いくつかの改善点や注意点があります。
良い点:
TypeScriptを使用して型安全性を確保しています。
useEffect を適切に使用して、コンポーネントのマウント時にデータを取得しています。
非同期処理でのエラーハンドリングが実装されています。
ユーザーインターフェースが直感的で、返信フォームの表示/非表示の切り替えなどの機能が含まれています。
改善点と提案:
エラーハンドリング:
エラーをコンソールに出力するだけでなく、ユーザーにも表示するべきです。
エラー状態を管理するための state を追加することを検討してください。
ローディング状態の管理:
現在、isLoading state が複数の非同期操作で共有されています。これにより、一方の操作が完了しても他方が進行中の場合にローディング表示が消えてしまう可能性があります。
各操作(投稿取得、返信取得)に対して個別のローディング状態を持つことを検討してください。
いいね機能の最適化:
現在のいいね機能は、APIレスポンスを待ってから更新しています。UXを向上させるために、楽観的更新(即座にUIを更新し、エラー時にロールバック)を検討してください。
日付フォーマット:
日付のフォーマットにライブラリ(例:date-fns)を使用することで、一貫性のあるフォーマットを維持しやすくなります。
返信の並び順:
現在、新しい返信が配列の先頭に追加されています。ユースケースによっては、古い順に表示する方が自然かもしれません。
パフォーマンス最適化:
返信リストが長くなる可能性がある場合、仮想化(例:react-window)の使用を検討してください。
アクセシビリティ:
セマンティックHTMLの使用を増やし、適切なARIAラベルを追加することで、アクセシビリティを向上させることができます。
テスト:
ユニットテストやインテグレーションテストの追加を検討してください。
環境変数の使用:
API URLやその他の設定値は、環境変数を使用して管理することを検討してください。
コードの分割:
コンポーネントが大きくなってきた場合、小さなサブコンポーネント(例:ReplyForm, ReplyList)に分割することを検討してください。
全体として、このコードは良く構造化されており、機能的です。上記の改善点を考慮することで、さらに堅牢で保守性の高いコンポーネントになるでしょう。
具体的にどこをどう修正するかが全くわからなかったため、修正コードを出力するように指示を見直すことで、効果的な標準プロンプトが作成できました。
@PostDetail.tsx コードレビューしてください。レビューで問題点を見つけた場合、問題点と問題点を修正するためのコードを出力してください。問題点を修正することで発生するメリット、デメリットについて出力してください。
PostDetail.tsx のコードレビューを行い、いくつかの問題点を見つけました。以下に問題点と修正案、そのメリットとデメリットを示します。
エラーハンドリングの改善
問題点: エラーがコンソールに出力されるだけで、ユーザーに表示されていません。
修正案:
const [error, setError] = useState<string | null>(null);
// エラーハンドリングの例(fetchPost関数内)
try {
// ... 既存のコード ...
} catch (error) {
console.error('Error fetching post:', error);
setError('投稿の取得に失敗しました。');
} finally {
setIsLoading(false);
}
// JSXでエラーを表示
{error && <div className="error-message">{error}</div>}
メリット:
ユーザーに問題を通知し、より良いUXを提供できる
デバッグが容易になる
デメリット:
コードの複雑さが若干増加する
ローディング状態の分離
問題点: 複数の非同期操作で単一のローディング状態を共有しています。
修正案:
const [isPostLoading, setIsPostLoading] = useState(true);
const [areRepliesLoading, setAreRepliesLoading] = useState(true);
// fetchPost関数内
setIsPostLoading(true);
// ...
setIsPostLoading(false);
// fetchReplies関数内
setAreRepliesLoading(true);
// ...
setAreRepliesLoading(false);
// JSXでの表示
if (isPostLoading) return <div>投稿を読み込み中...</div>;
if (areRepliesLoading) return <div>返信を読み込み中...</div>;
メリット:
より正確なローディング状態を表示できる
ユーザーに詳細な進行状況を提供できる
デメリット:
状態管理が若干複雑になる
いいね機能の最適化
問題点: いいね機能がサーバーレスポンスを待ってから更新されています。
修正案:
const handleLike = async () => {
if (post) {
const optimisticPost = { ...post, good: (post.good || 0) + 1 };
setPost(optimisticPost);
try {
const result = await client.models.PostMessage.update({
id: post.id,
good: optimisticPost.good
});
if (!result.data) {
// ロールバック
setPost(post);
}
} catch (error) {
console.error('Error updating like:', error);
// ロールバック
setPost(post);
}
}
};
メリット:
ユーザーの操作に対して即座に反応し、UXが向上する
サーバーのレスポンスを待たずに更新されるため、アプリケーションの反応が速くなる
デメリット:
楽観的更新によりデータの一貫性が一時的に損なわれる可能性がある
エラー時のロールバック処理が必要になり、コードが複雑化する
日付フォーマットの一貫性
問題点: 日付のフォーマットが直接 toLocaleString() を使用しています。
修正案:
import { format } from 'date-fns';
// ...
<p>投稿日時: {format(new Date(post.createdAt), 'yyyy/MM/dd HH:mm:ss')}</p>
<p>更新日時: {format(new Date(post.updatedAt), 'yyyy/MM/dd HH:mm:ss')}</p>
メリット:
日付フォーマットの一貫性が保たれる
国際化対応が容易になる
デメリット:
追加のライブラリ(date-fns)が必要になる
これらの修正を適用することで、コードの品質と保守性が向上し、ユーザー体験も改善されます。ただし、一部の修正ではコードの複雑さが増加するため、チームのスキルレベルやプロジェクトの要件に応じて適切に判断する必要があります。
役割の追加
次は、標準プロンプトに役割をあたえることを試してみました。
まず1人目の役割は「シニアエンジニア」です。プロンプトは以下の通りになります。
@PostDetail.tsx あなたはシニアエンジニアです。コードレビューして。レビューで問題点を見つけた場合、問題点と問題点を修正するためのコードを出力してください。問題点を修正することで発生するメリット、デメリットについて出力してください。
これまで、一般大衆向けの優しい説明が、この一言で専門用語を使ったエンジニア向けの言葉に変わりました。
またパフォーマンスやセキュリティの観点など、よりエンジニアらしいレビューが返ってくるようになりました。
シニアエンジニアとしてPostDetail.tsxのコードレビューを行います。以下に主な問題点と修正案、そのメリットとデメリットを示します。
エラーハンドリングの改善
問題点: エラーがコンソールに出力されるだけで、ユーザーに表示されていません。
修正案:
const [error, setError] = useState<string | null>(null);
// エラーハンドリングの例(fetchPost関数内)
try {
// ... 既存のコード ...
} catch (error) {
console.error('Error fetching post:', error);
setError('投稿の取得に失敗しました。再度お試しください。');
}
// JSXでエラーを表示
{error && <div className="error-message" role="alert">{error}</div>}
メリット:
ユーザーに問題を通知し、より良いUXを提供できる
デバッグが容易になる
デメリット:
状態管理が若干複雑になる
ローディング状態の分離
問題点: 複数の非同期操作で単一のローディング状態を共有しています。
修正案:
const [isPostLoading, setIsPostLoading] = useState(true);
const [areRepliesLoading, setAreRepliesLoading] = useState(true);
// fetchPost関数内
setIsPostLoading(true);
// ...
setIsPostLoading(false);
// fetchReplies関数内
setAreRepliesLoading(true);
// ...
setAreRepliesLoading(false);
// JSXでの表示
if (isPostLoading) return <div aria-live="polite">投稿を読み込み中...</div>;
if (areRepliesLoading) return <div aria-live="polite">返信を読み込み中...</div>;
メリット:
より正確なローディング状態を表示できる
ユーザーに詳細な進行状況を提供できる
デメリット:
状態管理が複雑になる
パフォーマンス最適化
問題点: 返信リストが長くなった場合にパフォーマンスが低下する可能性があります。
修正案:
import { FixedSizeList as List } from 'react-window';
// ...
const Row = ({ index, style }: { index: number, style: React.CSSProperties }) => (
<div style={style} className="reply">
<Link to={`/post/${replies[index].id}`} className="reply-link">
<h4>{replies[index].userName}</h4>
<p>{replies[index].message}</p>
<p>投稿日時: {new Date(replies[index].createdAt).toLocaleString()}</p>
<p>👍 {replies[index].good || 0}</p>
</Link>
</div>
);
// JSXでの表示
{replies.length > 0 && (
<div className="replies">
<h3>返信</h3>
<List
height={400}
itemCount={replies.length}
itemSize={100}
width="100%"
>
{Row}
</List>
</div>
)}
メリット:
大量の返信がある場合でもパフォーマンスが維持される
デメリット:
追加のライブラリ(react-window)が必要になる
実装が複雑になる
いいね機能の最適化
問題点: いいね機能がサーバーレスポンスを待ってから更新されています。
修正案:
const handleLike = async () => {
if (post) {
const optimisticPost = { ...post, good: (post.good || 0) + 1 };
setPost(optimisticPost);
try {
const result = await client.models.PostMessage.update({
id: post.id,
good: optimisticPost.good
});
if (!result.data) {
setPost(post); // ロールバック
setError('いいねの更新に失敗しました。');
}
} catch (error) {
console.error('Error updating like:', error);
setPost(post); // ロールバック
setError('いいねの更新に失敗しました。');
}
}
};
メリット:
ユーザーの操作に対して即座に反応し、UXが向上する
デメリット:
楽観的更新によりデータの一貫性が一時的に損なわれる可能性がある
セキュリティの向上
問題点: ユーザー入力に対するサニタイズが行われていません。
修正案:
import DOMPurify from 'dompurify';
// ...
<p>{DOMPurify.sanitize(post.message)}</p>
メリット:
XSS攻撃のリスクを軽減できる
デメリット:
追加のライブラリ(DOMPurify)が必要になる
これらの修正を適用することで、コードの品質、セキュリティ、パフォーマンス、およびユーザー体験が向上します。ただし、一部の修正では実装の複雑さが増加し、追加のライブラリが必要になるため、プロジェクトの要件やチームのスキルレベルに応じて適切に判断する必要があります。
2人目の役割は「Reactプログラマー」です。プロンプトは以下の通りになります。
@PostDetail.tsx あなたはプロのReactプログラマーです。コードレビューして。レビューで問題点を見つけた場合、問題点と問題点を修正するためのコードを出力してください。問題点を修正することで発生するメリット、デメリットについて出力してください。
Reactのプログラムに寄ったレビューが返ってきました。
プロのReactプログラマーとして、PostDetail.tsxのコードレビューを行います。以下に主な問題点と修正案、そのメリットとデメリットを示します。
副作用の依存配列の問題
問題点: useEffectの依存配列に関数が含まれていません。
修正案:
useEffect(() => {
if (postId) {
fetchPost(postId);
fetchReplies(postId);
}
}, [postId, fetchPost, fetchReplies]);
const fetchPost = useCallback(async (id: string) => {
// ... 既存のコード ...
}, []);
const fetchReplies = useCallback(async (parentId: string) => {
// ... 既存のコード ...
}, []);
メリット:
不要な再レンダリングを防ぐ
副作用の依存関係が明確になる
デメリット:
コードが若干複雑になる
状態更新の最適化
問題点: 複数の状態更新が個別に行われています。
修正案:
const handleReply = async (e: React.FormEvent) => {
e.preventDefault();
if (newReply.trim() && userName.trim() && postId) {
try {
const result = await client.models.PostMessage.create({
message: newReply.trim(),
userName: userName.trim(),
parentId: postId,
good: 0
});
if (result.data) {
setReplies(prevReplies => [result.data, ...prevReplies]);
setNewReply('');
setUserName('');
setShowReplyForm(false);
}
} catch (error) {
console.error('Error creating reply:', error);
}
}
};
メリット:
パフォーマンスが向上する
状態更新のバッチ処理が可能になる
デメリット:
特になし
条件付きレンダリングの最適化
問題点: 条件付きレンダリングが冗長です。
修正案:
return (
<div className="post-detail">
<button onClick={() => navigate('/')}>戻る</button>
{post && (
<div className="post">
<h2>{post.userName}</h2>
<p>{post.message}</p>
<p>投稿日時: {new Date(post.createdAt).toLocaleString()}</p>
<p>更新日時: {new Date(post.updatedAt).toLocaleString()}</p>
<button onClick={handleLike}>👍 {post.good || 0}</button>
<button onClick={() => setShowReplyForm(!showReplyForm)}>
{showReplyForm ? '返信をキャンセル' : '返信する'}
</button>
</div>
)}
{/* ... 残りのJSX ... */}
</div>
);
メリット:
コードの可読性が向上する
不要なレンダリングを防ぐ
デメリット:
特になし
メモ化によるパフォーマンス最適化
問題点: 子コンポーネントの不要な再レンダリングが発生する可能性があります。
修正案:
import React, { memo } from 'react';
const Reply = memo(({ reply }: { reply: PostMessage }) => (
<div className="reply">
<Link to={`/post/${reply.id}`} className="reply-link">
<h4>{reply.userName}</h4>
<p>{reply.message}</p>
<p>投稿日時: {new Date(reply.createdAt).toLocaleString()}</p>
<p>👍 {reply.good || 0}</p>
</Link>
</div>
));
// JSXでの使用
{replies.map(reply => <Reply key={reply.id} reply={reply} />)}
メリット:
不要な再レンダリングを防ぎ、パフォーマンスが向上する
デメリット:
小さなコンポーネントの場合、オーバーヘッドが大きくなる可能性がある
カスタムフックの使用
問題点: ロジックとUIが密接に結合しています。
修正案:
// usePost.ts
import { useState, useEffect, useCallback } from 'react';
import { generateClient } from 'aws-amplify/api';
import { Schema } from '../../amplify/data/resource';
const client = generateClient<Schema>();
export const usePost = (postId: string) => {
const [post, setPost] = useState<PostMessage | null>(null);
const [isLoading, setIsLoading] = useState(true);
const [error, setError] = useState<string | null>(null);
const fetchPost = useCallback(async () => {
try {
setIsLoading(true);
const result = await client.models.PostMessage.get({ id: postId });
if (result.data) {
setPost(result.data);
}
} catch (error) {
console.error('Error fetching post:', error);
setError('投稿の取得に失敗しました。');
} finally {
setIsLoading(false);
}
}, [postId]);
useEffect(() => {
fetchPost();
}, [fetchPost]);
return { post, isLoading, error };
};
// PostDetail.tsx
const PostDetail: React.FC = () => {
const { postId } = useParams<{ postId: string }>();
const { post, isLoading, error } = usePost(postId!);
// ... 残りのコンポーネントロジック ...
};
メリット:
ロジックの再利用が可能になる
コンポーネントがシンプルになり、テストが容易になる
デメリット:
ファイル数が増える
これらの修正を適用することで、コードの品質、再利用性、パフォーマンスが向上します。ただし、一部の修正ではコードの複雑さが増す可能性があるため、チームのスキルレベルやプロジェクトの規模に応じて適切に判断する必要があります。
今回のレビューではReactに寄ったレビューよりも、エンジニアの観点に寄ったレビューが適しているため、以降のパターンの検証の際にも役割を「シニアエンジニア」に設定しています。
確認ポイントの追加
役割を与えたプロンプトに、確認して欲しいポイントを加えて試してみました。
プロンプトは以下の通りになります。
@PostDetail.tsx あなたはシニアエンジニアです。コードレビューして。レビューで問題点を見つけた場合、問題点と問題点を修正するためのコードを出力してください。問題点を修正することで発生するメリット、デメリットについて出力してください。
特に以下の点を指摘してください:
・誤解を招いたり、実態を正確に表していない命名があるか
・適切な粒度で変数が定義されているか
・メソッド・関数の区分が適切な粒度か
・冗長な書き方のコードがないか
・複雑な条件式が作られていないか
・明らかなセキュリティの問題があるか
・可読性が高いか
チェック観点に対してレビューを返してくれるようになりました。
中身を見た限り、取り込むべきと判断できる内容はあまりありませんでした。
シニアエンジニアとして、PostDetail.tsxのコードレビューを行います。指摘された点を中心に問題点と修正案、そのメリットとデメリットを示します。
命名の問題:
fetchPost と fetchReplies は非同期関数なので、fetchPostAsync と fetchRepliesAsync とする方が適切です。
修正案:
const fetchPostAsync = async (id: string) => { ... }
const fetchRepliesAsync = async (parentId: string) => { ... }
メリット: 関数の非同期性が名前から明確になる。 デメリット: 特になし。
変数の粒度:
isLoading が投稿と返信の両方のローディング状態を管理しているため、分離すべきです。
修正案:
const [isPostLoading, setIsPostLoading] = useState(true);
const [areRepliesLoading, setAreRepliesLoading] = useState(true);
メリット: より細かな制御が可能になり、UIの正確性が向上する。 デメリット: 状態管理が若干複雑になる。
メソッド・関数の粒度:
handleReply 関数が長いので、投稿作成のロジックを分離すべきです。
修正案:
const createReply = async (message: string, userName: string, parentId: string) => {
const result = await client.models.PostMessage.create({
message: message.trim(),
userName: userName.trim(),
parentId,
good: 0
});
return result.data;
};
const handleReply = async (e: React.FormEvent) => {
e.preventDefault();
if (newReply.trim() && userName.trim() && postId) {
try {
const newReplyData = await createReply(newReply, userName, postId);
if (newReplyData) {
setReplies(prev => [newReplyData, ...prev]);
setNewReply('');
setUserName('');
setShowReplyForm(false);
}
} catch (error) {
console.error('Error creating reply:', error);
}
}
};
メリット: 関数の責務が明確になり、テストが容易になる。 デメリット: コード量が若干増える。
冗長な書き方:
post.good || 0 の部分が複数回出現しています。
修正案:
const likeCount = post.good ?? 0;
メリット: コードの重複が減り、可読性が向上する。 デメリット: 特になし。
複雑な条件式:
特に複雑な条件式は見当たりません。
セキュリティの問題:
ユーザー入力(post.message, reply.message)がそのまま表示されており、XSS攻撃の可能性があります。
修正案:
import DOMPurify from 'dompurify';
// ...
<p>{DOMPurify.sanitize(post.message)}</p>
// ...
<p>{DOMPurify.sanitize(reply.message)}</p>
メリット: XSS攻撃のリスクを軽減できる。 デメリット: 追加のライブラリが必要になる。
可読性:
全体的に可読性は高いですが、エラーハンドリングが不十分です。
修正案:
const [error, setError] = useState<string | null>(null);
// エラーハンドリングの例
try {
// ...
} catch (error) {
console.error('Error:', error);
setError('エラーが発生しました。もう一度お試しください。');
}
// JSXでエラーを表示
{error && <div className="error-message">{error}</div>}
メリット: ユーザーにエラー情報を提供し、UXが向上する。 デメリット: 状態管理が若干複雑になる。
その他の改善点:
useEffectの依存配列に関数を含めるべきです。
パフォーマンス最適化のため、useCallbackやuseMemoの使用を検討すべきです。
日付フォーマットにはライブラリ(例:date-fns)の使用を検討すべきです。
これらの修正を適用することで、コードの品質、セキュリティ、可読性、および保守性が向上します。ただし、一部の修正では実装の複雑さが増加するため、プロジェクトの要件やチームのスキルレベルに応じて適切に判断する必要があります。
規約ドキュメントの追加
役割を与えたプロンプトに、参照して欲しい規約ドキュメントを加えて試してみました。
参照してもらったドキュメントは2つあります。
1つ目はWeb上から取得してきたReactとTypeScriptのコーディング規約についてまとめた記事のリンクを登録して参照してもらいました。
プロンプトは以下の通りになります。
@PostDetail.tsx @React×TypeScript あなたはシニアエンジニアです。コードレビューして。レビューで問題点を見つけた場合、問題点と問題点を修正するためのコードを出力してください。問題点を修正することで発生するメリット、デメリットについて出力してください。規約に遵守したレビューを行なってください。
参照用に渡した規定の通りのレビューは行ってくれました。
しかし、自分たちが見て欲しい観点がまとめられた規約ではなかったので、意図に沿ったレビューは返してくれなかったです。
シニアエンジニアとして、PostDetail.tsxのコードレビューを行います。規約に従って問題点を指摘し、修正案を提示します。
useEffectの使用
// TODO: useEffectの使用を避ける
useEffect(() => {
if (postId) {
fetchPost(postId);
fetchReplies(postId);
}
}, [postId]);
修正案:
// NOTE: useEffectを使用せずにデータ取得を行う
const { postId } = useParams<{ postId: string }>();
if (postId) {
fetchPost(postId);
fetchReplies(postId);
}
メリット: useEffectの使用を避けることで、副作用の管理が簡単になります。 デメリット: 初期レンダリング時にデータ取得が複数回行われる可能性があります。
変数の命名
// TODO: 複数形に変更
const [replies, setReplies] = useState<PostMessage[]>([]);
修正案:
const [replyList, setReplyList] = useState<PostMessage[]>([]);
メリット: 命名規則に従うことで、コードの一貫性が向上します。
関数名の命名
// TODO: 関数名を動詞から始める
const fetchPost = async (id: string) => { ... }
const fetchReplies = async (parentId: string) => { ... }
修正案:
const getPost = async (id: string) => { ... }
const getReplyList = async (parentId: string) => { ... }
メリット: 関数の目的がより明確になります。
エラーハンドリング
// TODO: エラーハンドリングの改善
try {
// ...
} catch (error) {
console.error('Error fetching post:', error);
}
修正案:
const [error, setError] = useState<string | null>(null);
try {
// ...
} catch (error) {
console.error('Error fetching post:', error);
setError('投稿の取得に失敗しました。再度お試しください。');
}
// JSXでエラーを表示
{error && <div className="error-message">{error}</div>}
メリット: ユーザーにエラー情報を提供し、UXが向上します。
型定義
// TODO: typeで型定義を行う
type PostMessage = Schema['PostMessage']['type'];
このコードは既に規約に従っているため、修正は不要です。
コンポーネントの行数
現在のコンポーネントは500行を超えていませんが、機能が増えた場合は分割を検討する必要があります。
クラス名の使用
// TODO: Tailwindクラスをclsxで管理
<div className="post-detail">
修正案:
import clsx from 'clsx';
// ...
<div className={clsx(
'post-detail',
// その他の条件付きクラス
)}>
メリット: クラス名の管理が容易になり、条件付きスタイリングが簡単になります。
コメントの追加
/**
* 投稿詳細を表示するコンポーネント
* 投稿の詳細情報と返信一覧を表示し、新規返信の作成機能を提供する
*/
const PostDetail: React.FC = () => {
// ...
}
メリット: コンポーネントの目的と機能が明確になり、他の開発者の理解を助けます。
これらの修正を適用することで、コードの品質、可読性、保守性が向上します。ただし、一部の修正(特にuseEffectの削除)は慎重に行う必要があり、アプリケーションの動作に影響を与える可能性があるため、十分なテストが必要です。
2つ目はContinueに元から登録されているドキュメント(TypeScript)を参照してもらいました。
プロンプトは以下の通りになります。
@PostDetail.tsx @Typescript あなたはシニアエンジニアです。コードレビューして。レビューで問題点を見つけた場合、問題点と問題点を修正するためのコードを出力してください。問題点を修正することで発生するメリット、デメリットについて出力してください。規約に遵守したレビューを行なってください。
TypeScriptのドキュメントに沿ったレビューが返ってきました。
TypeScriptを使用している限りは、気にする観点が返されていたので有用かと思われます。
シニアエンジニアとして、PostDetail.tsxのコードレビューを行います。TypeScriptの規約に従って問題点を指摘し、修正案を提示します。
型アサーションの使用
const { postId } = useParams<{ postId: string }>();
この部分は型アサーションを使用していますが、TypeScriptの規約では型アサーションの使用を避けることが推奨されています。
修正案:
const { postId } = useParams() as { postId: string };
メリット: 型の安全性が向上します。 デメリット: コードが若干冗長になります。
any型の暗黙的な使用
} catch (error) {
console.error('Error fetching post:', error);
}
error変数の型が暗黙的にanyになっています。
修正案:
} catch (error: unknown) {
console.error('Error fetching post:', error);
}
メリット: 型の安全性が向上し、エラーハンドリングが明示的になります。 デメリット: エラーの詳細な情報を扱う際に追加の型チェックが必要になります。
オプショナルチェイニングの使用
if (post) {
try {
const result = await client.models.PostMessage.update({
id: post.id,
good: (post.good || 0) + 1
});
// ...
}
}
post.goodの存在チェックにオプショナルチェイニングを使用できます。
修正案:
if (post) {
try {
const result = await client.models.PostMessage.update({
id: post.id,
good: (post.good ?? 0) + 1
});
// ...
}
}
メリット: コードがより簡潔になり、nullishな値の処理が明確になります。 デメリット: 特にありません。
非同期関数の戻り値の型指定
const fetchPost = async (id: string) => {
// ...
};
非同期関数の戻り値の型が指定されていません。
修正案:
const fetchPost = async (id: string): Promise<void> => {
// ...
};
メリット: 関数の戻り値の型が明確になり、型の安全性が向上します。 デメリット: 特にありません。
条件付きレンダリングの型安全性
{replies.length > 0 && (
// ...
)}
この条件付きレンダリングは、repliesが未定義の場合にエラーを引き起こす可能性があります。
修正案:
{replies?.length > 0 && (
// ...
)}
メリット: nullやundefinedの場合のエラーを防ぎ、型の安全性が向上します。 デメリット: 特にありません。
これらの修正を適用することで、コードの型安全性と可読性が向上し、潜在的なバグを防ぐことができます。ただし、一部の修正では追加のチェックが必要になるため、パフォーマンスにわずかな影響を与える可能性があります。
全部のせ
標準プロンプトに役割と確認ポイント、規定ドキュメントを全パターンを含めて試しました。
プロンプトは以下の通りです。
@PostDetail.tsx @Typescriptあなたはシニアエンジニアです。コードレビューして。レビューで問題点を見つけた場合、問題点と問題点を修正するためのコードを出力してください。問題点を修正することで発生するメリット、デメリットについて出力してください。
規約に遵守したレビューを行なってください。
特に以下の点を指摘してください:
・誤解を招いたり、実態を正確に表していない命名があるか
・適切な粒度で変数が定義されているか
・メソッド・関数の区分が適切な粒度か
・冗長な書き方のコードがないか
・複雑な条件式が作られていないか
・明らかなセキュリティの問題があるか
・可読性が高いか
参照ドキュメントとして含めた規約ドキュメントが無視され、プロンプト内に含めた指摘ポイントに対してのレビューのみが返ってきました。
シニアエンジニアとして、PostDetail.tsxのコードレビューを行います。指定された点に特に注意して問題点を指摘し、修正案を提示します。
命名の問題:
const [replies, setReplies] = useState<PostMessage[]>([]);
replies は複数形で適切です。TypeScriptでは配列を表す変数名は複数形が一般的です。
変数の粒度:
const [isLoading, setIsLoading] = useState(true);
isLoading は投稿と返信の両方のローディング状態を管理しています。これを分離すべきです。
修正案:
const [isPostLoading, setIsPostLoading] = useState(true);
const [areRepliesLoading, setAreRepliesLoading] = useState(true);
メリット: より細かな制御が可能になり、UIの正確性が向上します。 デメリット: 状態管理が若干複雑になります。
メソッド・関数の粒度:
handleReply 関数が長く、複数の責務を持っています。
修正案:
const createReply = async (message: string, userName: string, parentId: string): Promise<PostMessage | null> => {
try {
const result = await client.models.PostMessage.create({
message: message.trim(),
userName: userName.trim(),
parentId,
good: 0
});
return result.data;
} catch (error) {
console.error('Error creating reply:', error);
return null;
}
};
const handleReply = async (e: React.FormEvent) => {
e.preventDefault();
if (newReply.trim() && userName.trim() && postId) {
const newReplyData = await createReply(newReply, userName, postId);
if (newReplyData) {
setReplies([newReplyData, ...replies]);
setNewReply('');
setUserName('');
setShowReplyForm(false);
}
}
};
メリット: 関数の責務が明確になり、テストが容易になります。 デメリット: コード量が若干増えます。
冗長な書き方:
{post.good || 0}
この表現が複数回使用されています。
修正案:
const likeCount = post.good ?? 0;
メリット: コードの重複が減り、可読性が向上します。 デメリット: 特にありません。
複雑な条件式:
特に複雑な条件式は見当たりません。
セキュリティの問題:
ユーザー入力(post.message, reply.message)がそのまま表示されており、XSS攻撃の可能性があります。
修正案:
import DOMPurify from 'dompurify';
// ...
<p>{DOMPurify.sanitize(post.message)}</p>
// ...
<p>{DOMPurify.sanitize(reply.message)}</p>
メリット: XSS攻撃のリスクを軽減できます。 デメリット: 追加のライブラリが必要になります。
可読性:
全体的に可読性は高いですが、エラーハンドリングが不十分です。
修正案:
const [error, setError] = useState<string | null>(null);
// エラーハンドリングの例
try {
// ...
} catch (error) {
console.error('Error:', error);
setError('エラーが発生しました。もう一度お試しください。');
}
// JSXでエラーを表示
{error && <div className="error-message">{error}</div>}
メリット: ユーザーにエラー情報を提供し、UXが向上します。 デメリット: 状態管理が若干複雑になります。
これらの修正を適用することで、コードの品質、セキュリティ、可読性、および保守性が向上します。ただし、一部の修正では実装の複雑さが増加するため、プロジェクトの要件やチームのスキルレベルに応じて適切に判断する必要があります。
わかったこと
- 役割指定しないと、一般大衆にわかりやすい表現の指摘になった。
- エンジニアには専門的用語で指摘してくれるシニアエンジニアが役割に相応しかった。
- 言語のプロを指定すると、特に問題なしのコードも、教科書通りの指示を細かくしてくる感じだった。
- 確認ポイントの追加はあまり効果的ではなかった。
- ドキュメントの指定は、登録済ドキュメントを指定すると、なるほどと感心する指摘だったので指定すべき。
- 全部のせは、他の条件と排他関係になって出力されない部分があったので、効果的ではなかった。
まとめ
試した中で、一番良かったプロンプトは標準+役割追加+規定ドキュメント追加でした。
このプロンプトの有効性を確認するため、別のコードで試したところ、過剰に指摘されたり指摘が不十分だったりということはなく、バランスの良いレビュー指摘が出力されました。
AIコードレビューを効率化するプロンプト作成は試行錯誤の連続でしたが、その結果、効果的な方法を見つけることができました。この記事が皆さんのレビュー効率向上の一助となれば幸いです。
①役割を決める
②確認ポイントのリストを与える
③規約ドキュメントを指定する