概要
下記の記事にあるように、GitHub上でCopilotによるレビューを行う際、copilot-instructions.md
に記述された内容を読み込んでレビューしてくれるようになりました。これで人間によるコードレビューの負荷を少しは下げられそうです。
ただ実際どこまでカスタム命令を読み込んでくれているのか、ちょっと懐疑的です。ので実際にこのカスタム命令に従っていないコードをレビューしてもらい、どこまで指摘してくれるのか試してみましょう!
やること
- GitHub上でのカスタム命令有効化
- 適当な
copilot-instructions.md
を記述 - それに違反するコードを書いたPRを作り、どこまで指摘されるか実験
前提
- 今回は適当なSpring Bootでのアプリケーションで試します
- ある程度共通処理が出来上がっているものから、機能を追加するPRを出します
- 現在パブリックプレビューの機能で、有料のGitHub Copilotライセンスが必要です
基になるプロジェクト
- ありがちなユーザ管理機能を持つプロジェクトです
- ユーザ取得機能まで実装されています
- ユーザ登録機能を作成するPRを出していきます
GitHubでのカスタム命令有効化
リポジトリのSettings > Copilot > Code review
から有効にできます。私の場合はデフォルトで有効になっていました。
カスタム命令ファイルの作成
./github/copilot-instructions.md
を作成していろいろ書いていきます。今回は元のコードからCopilotに生成させたうえで、自分で調整してみました。
# コード規約 - Spring Boot API プロジェクト(レビュー観点)
## 1. 基本設計原則
### 1.1 責任分離
- **1エンドポイント1Controller/1Service** の原則を守る
- Controller → Service → Repository の依存関係を維持
- DTOとEntityを適切に分離
### 1.2 命名規則
- **Controller**: `{Entity}{Action}Controller` (例: `UserCreateController`)
- **Service**: `{Entity}{Action}Service` / `{Entity}{Action}ServiceImpl`
- **DTO**: `{Entity}{Action}RequestDto` / `{Entity}ResponseDto`
- **Exception**: `{Entity}{Type}Exception`
### 1.3 パッケージ構成
jp.ne.papapa.copilot_instructions/
├── controller/ # 1エンドポイント1クラス
├── service/ # 1操作1インターフェース
├── dto/ # リクエスト/レスポンス
├── entity/ # JPAエンティティ
├── repository/ # データアクセス
├── exception/ # 例外・例外ハンドラー
└── common/ # 定数・ユーティリティ
## 2. レビューチェックポイント
### 2.1 Controller層
- `@RestController`, `@RequiredArgsConstructor`, `@Slf4j` が付与されているか
- 1つのエンドポイントのみ実装されているか
- `@Valid` でリクエストバリデーションが行われているか
- Service層のみに依存し、Repository層に直接アクセスしていないか
### 2.2 Service層
- インターフェースと実装クラスが分離されているか
- `@Service`, `@RequiredArgsConstructor`, `@Transactional` が適切に付与されているか
- 1つのビジネス操作のみ実装されているか
- `ExceptionFactory` を使って例外を生成しているか
- DTOとEntityの変換が適切に行われているか
### 2.3 Repository層
- `JpaRepository<Entity, ID>`を継承しているか
- メソッド命名規則に従っているか(`findByXxx`, `existsByXxx`, `countByXxx`)
- 複雑な検索は`@Query`アノテーションを使用しているか
### 2.4 DTO設計
- `@Data`, `@Builder`, `@NoArgsConstructor`, `@AllArgsConstructor` が付与されているか
- リクエストDTOにバリデーションアノテーションが適切に設定されているか
- メッセージキーが`messages.properties`で管理されているか
- Entity変換メソッド(`toEntity`, `fromEntity`)が実装されているか
### 2.5 Entity設計
- `@Entity`, `@Table`, `@Data`, `@Builder` が付与されているか
- カラム長が`Constants`で定数化されているか
- `@CreationTimestamp`, `@UpdateTimestamp`で日時管理されているか
- Enumは`@Enumerated(EnumType.STRING)`で保存されているか
- `nullable`, `unique`制約が適切に設定されているか
### 2.6 例外処理
- カスタム例外クラスが適切に定義されているか
- `ExceptionFactory`を使って例外を生成しているか
- `@RestControllerAdvice`でグローバル例外ハンドラーが実装されているか
## 3 開発・保守のポイント
### 3.1 新機能追加手順
1. Entity → Repository → DTO → Service → Controller の順で実装
2. 例外処理とメッセージ定義を忘れずに追加
3. テストコードも合わせて実装
### 3.2 禁止事項
- ControllerでのEntity直接使用
- Repository層でのビジネスロジック実装
- ハードコーディング(定数・メッセージの直書き)
- 1つのController/Serviceで複数機能実装
- 例外メッセージの直接記述
### 3.3 推奨事項
- `@RequiredArgsConstructor` でコンストラクタ注入
- `@Slf4j` でログ機能使用
- `@Builder` でオブジェクト生成
- メソッドチェーンでの可読性向上
- Optional型でNull安全性確保
## 4. その他開発ルール
- 想定外の例外処理(DBエラーなど)はControllerAdviceで行うので、個別での処理は不要
- 開始ログや終了ログはAspectで一括管理するので、個別での処理は不要
結構細かく用意しました。また今後の拡張性を考えて、4の項番を用意しています。開発中でのルール追加や指摘されることの多いレビュー項目などをここに追加していく運用を想定しました。
実験開始!
違反しまくりコードを書く
新しく「ユーザ登録」機能を作成するにあたって、可能な限り先ほどの方針に反する形で実装してみました。
主に以下を意識して(?)作っています。
- そもそも動かない
- アノテーションの付け忘れ
- リクエストバリデーションの付け忘れ
- ControllerからRepositoryの直参照
- Handlerを使わない個別の例外処理
- Aspectを無視した個別のログ出力
- 定数などのハードコーディング
@RestController
@RequiredArgsConstructor
public class UserCreateController {
private final UserRepository userRepository;
private static final Logger log = LoggerFactory.getLogger(UserCreateController.class);
public ResponseEntity<UserResponseDto> creaiteUser(@RequestBody UserCreateRequestDto userDto) {
// 開始ログの出力
log.info("POST /api/v1/users - Creating new user: {}", userDto);
// ユーザーの作成処理
try{
UserResponseDto createdUser = UserResponseDto.fromEntity(userRepository.save(userDto.toEntity()));
return ResponseEntity.ok(createdUser); // 作成したユーザー情報を返す
} catch (Exception e) {
log.error("Error creating user: {}", e.getMessage());
return ResponseEntity.status(500).body(null); // エラー時は500を返す
}
}
}
初回コードレビュー
なんか中国語なんですけど!?!?!?
多分日本語でのレビューを指定せず、カスタム命令ファイルに漢字が含まれていたからですかね...
下記をカスタム命令に追加して再度動かしてみましょう。ややこしくなるのでいったんPRは作り直しました。
<!-- I want to review in Japanese. -->
# コード規約 - Spring Boot API プロジェクト(レビュー観点)
## 1. 基本設計原則
...
<!-- I want to review in Japanese. -->
気を取り直してもう一度初回レビュー
なんか...指摘自体はどれもまっとうなんですが、別にカスタム命令ファイルに書いてないことばかり...?
いや、書いていることの指摘もあるんですが、もっと別で指摘してほしいことも多い気が...
修正して再レビュー(2回目)
なんで!?英語に戻ってんじゃん!?
これ、カスタム命令ファイルガン無視してないか...?
この辺りからちょっとこの機能に対する疑念が積もってきています。
またまた修正して再レビュー(3回目)
お!日本語に戻りました。しかもこの指摘に関しては、ちゃんとカスタム設定ファイルから読んできた指摘みたいです。
修正して再レビュー(4回目)
あれ!なんか急にめちゃくちゃいい感じのレビューをしてくれるようになった!
この回、レビュー指摘箇所も多ければ、いわゆるカスタム命令に書いたこのプロジェクト特有の部分のレビューもめちゃくちゃしてくれています。
そしてよく見たら...
こんな感じで、明らかにcopilot-instructions.md
を参照してレビューを作成したという補足もついています。なにがトリガーになったのかよくわかりませんが、結構細かい部分まで見てくれていそうです。
その後も修正&再レビューを繰り返し...
このあたりの定数のハードコーディングあたりもしっかり指摘してくれました。
今回意識した以下の観点、すべて指摘してくれています。
- そもそも動かない
- アノテーションの付け忘れ
- リクエストバリデーションの付け忘れ
- ControllerからRepositoryの直参照
- Handlerを使わない個別の例外処理
- Aspectを無視した個別のログ出力
- 定数などのハードコーディング
またそれ以外にも、明らかなスペルミスや可読性を上げるための記述、不要なインポート文などの指摘はしてくれていそうです。
感想
まだプレビュー版だからか、確実にカスタム命令を読み込んでレビューを指摘してくれる、という状態にはなっていないように見えます。
一方、複数回レビューを頼んだ場合、ある程度細かい命令であってもきちんと指摘してくれることもわかりました。今回のコードが指摘箇所多すぎなだけだったかもしれませんが...
静的なルールだけでは検出が難しい、共通のシステムアーキテクチャ部分やプロジェクト固有のルール(例外処理やログ出力)なども検出して指摘してくれています。
当初の狙いであった「レビュワーの負荷軽減」という意味よりは、製造時での「品質の向上」といった意図であれば現状でも結構使えそうです。
「確実に」指摘してくれるということはまだまだできなさそうですが、「AI」と「人間」によるダブルチェックで品質の向上を狙う使い道が、今のところの限界かなと思いました。
最終的には、ルール的な部分のチェックはAIがすべて行い、業務的な仕様との照らし合わせだけ人間が行えるくらいになれば、ある程度レガシーなプロジェクトにも適用できそうかな...?
オマケ:今回のレビュー全文