はじめに

みなさんこんにちは。CCIの秋本です。
以前Amazon CodeGuru Reviewerを使ってみたことがあったのですが、ブログやら何やらにまったく書いたことがなかったので、こちらについて書いていきたいと思います。

Amazon CodeGuru Reviewerとは？

aws.amazon.com

AWSのベストプラクティスをベースにコードレビューをしてくれるサービスです。
Java とPython が対象言語となっていて、コード上では見つけるのが難しい欠陥や脆弱性を検出してくれて、それらの修正に関する推奨事項などを提供してくれるようです。
利用可能なリポジトリとしては、

AWS CodeCommit
Bitbucket
GitHub
GitHub Enterprise

となっております。

料金については、設定したリポジトリの最初の10万行あたり10.00USDで、以降10万行追加されるたびに30.00USDが加算される料金形態のようです。
コードの行数が多いプロジェクトだと決して安くはないと思うので、利用する際には計画的に使う必要があるかと思います。

設定

私の所属しているチームではAWS CodeCommitを利用してますので、そちらでの設定方法を記載していきます。
対象のリポジトリにはこの間までチームでごりごり開発していたJavaのプロジェクトを使用しました。
どのようなれレビュー結果が返ってくるのかちょっと怖いですね(笑)

①CodeCommitからの設定
まず紐づけたいリポジトリに遷移した後、左のメニューから「設定」を押下し、全般や通知といったタブと並んで表記されている「Amazon CodeGuru Reviewer」を選択。

画面が以下のように遷移したら、「リポジトリを関連付ける」ボタンを押す。

ボタン押下時は以下のように「関連付け中」と表示されるので、しばらく待機。

数分後、以下画面のように表示されれば、AWS CodeGuru Reviewerとの関連付けは完了。

②CodeGuruからの設定
CodeGuruに遷移し、左メニューの「リポジトリ」を選択すると以下の画面に遷移するので、右端にある「リポジトリを関連付けて分析を実行」を押下。

すると以下の画面に遷移するので、紐づけるリポジトリと、プロジェクト名、ブランチ、レビュー名(これは何でもよさそう)を入力して、「リポジトリを関連付けて分析を実行」を押す。

下図に遷移すると、紐づけは完了しリポジトリの分析が開始されるので、分析が完了するのをしばらく待つ。

下図のように表示されれば紐づけから分析まで完了。

使い方

CodeGuru Reviewerでは、

増分コードレビュー
フルリポジトリ分析

の2種類の分析方法があります。
増分コードレビューが、プルリクエストで変更されたコードを解析してレビューしてくれるのに対し、フルリポジトリ分析では設定したリポジトリを丸ごとレビューしてくれます。

今回はどのくらいの指摘がでてくるのかを見てみたかったので、リポジトリ分析を実施することにしました。

やり方ですが、設定方法とほとんど同じで、まずCodeGuruの画面に移動します。
その後左メニューの「コードレビュー」を押下し、以下のような画面に遷移しますので、「フルリポジトリ分析」タブを押下し、右端にある「フルリポジトリ分析を開始する」ボタンを押します。
下図画面に移動するので、リポジトリ、ソースブランチ、レビュー名(これもなんでもよい)を入力して、「フルリポジトリ分析を作成」ボタンを押す。

下図画面に遷移すればフルリポジトリ分析は開始されているので、完了するのを待つ(だいたい5～10分くらい)。

下図のような表示になれば、分析は完了。

レビュー結果を確認してみる

分析が完了したので、レビューの結果を確認してみます。

レビュー結果を確認するには下図のレビュー名をクリックします。

レビュー結果は以下のように表示されます。

さて、レビュー内容を確認してみましょう！

The cyclomatic complexity of this method is 12. By comparison, 98% of the methods in the CodeGuru reference dataset have a lower cyclomatic complexity. This indicates the method has a high number of decisions and it can make the logic difficult to understand and test. We recommend that you simplify this method or break it into multiple methods. For example, consider extracting the code block on lines 128-136 into a separate method. High cyclomatic complexity indicates the method has a high number of decisions and it can make the logic difficult to understand and test. Methods with high cyclomatic complexity should be simplified or broken into multiple simpler methods.

レビュー内容は全部英語ですね、、ちょっとわからないのでGoogle先生に翻訳してもらいます！

このメソッドの循環的複雑度は 12 です。比較すると、CodeGuru 参照データセットのメソッドの 98% は循環的複雑度が低くなっています。これは、メソッドに多数の決定があり、ロジックの理解とテストが困難になる可能性があることを示しています。この方法を単純化するか、複数の方法に分割することをお勧めします。たとえば、行 128 ～ 136 のコードブロックを別のメソッドに抽出することを検討してください。循環的複雑度が高いということは、メソッドに多数の決定があり、ロジックの理解とテストが困難になる可能性があることを示しています。循環的複雑度の高いメソッドは、単純化するか、複数の単純なメソッドに分割する必要があります。

…どうやら一つのメソッドに処理を詰め込み過ぎているらしく、テストもしづらいから分割した方がいい、ということらしいです。
類似の処理が他にもいくつかあるせいか、指摘のいくつかは同じ内容でした。。。
リンク先からCodeCommitの該当ソースに直接移動できるのはありがたいですね。

さて、他の指摘内容も確認してみます。

This method contains 65 lines of code, not including blank lines or lines with only comments, Java punctuation characters, identifiers, or literals. By comparison, 99% of the methods in the CodeGuru reference dataset contain fewer lines of code. Large methods might be difficult to read and have logic that is hard to understand and test. We recommend that you simplify this method or break it into multiple methods. For example, consider extracting the code block on lines 151-159 into a separate method. Large methods might be difficult to read, and have logic that is hard to understand and test. Large methods should be simplified or broken into multiple smaller methods. The size of a method is computed as the number of lines of code, not including blank lines or lines with only comments, Java punctuation characters, identifiers, or literals.

こちらも翻訳します。

このメソッドには 65 行のコードが含まれます。これには、空白行や、コメント、Java 句読点文字、識別子、またはリテラルのみの行は含まれません。比較すると、CodeGuru リファレンスデータセットのメソッドの 99% には、より少ないコード行が含まれています。大規模なメソッドは読みにくく、理解やテストが難しいロジックを持っている可能性があります。この方法を単純化するか、複数の方法に分割することをお勧めします。たとえば、行 151 ～ 159 のコードブロックを別のメソッドに抽出することを検討してください。大規模なメソッドは読みにくく、理解とテストが難しいロジックを持っている可能性があります。大きなメソッドは単純化するか、複数の小さなメソッドに分割する必要があります。メソッドのサイズは、コードの行数として計算されます。これには、空白行や、コメント、Java 句読点文字、識別子、またはリテラルのみの行は含まれません。

1メソッドあたり65行のコードは多い、という指摘みたいですね。
空白行やコメントなどは無視してくれているようなので、純粋にコードが多い、ということみたいです。
似たような指摘多いなこのプロジェクト。。。。~~(ちゃんとレビューしろ)~~

さて、もう1個くらい指摘内容を確認していきます。

Problem: InterruptedException is ignored. This can delay thread shutdown and clear the thread’s interrupt status. Only code that implements a thread’s interruption policy can swallow an interruption request.

Fix: Rethrow the InterruptedException or reinterrupt the current thread using Thread.currentThread().interrupt() so that higher-level interrupt handlers can function correctly. If you are wrapping the InterruptedException inside a RuntimeException, call Thread.currentThread().interrupt() before throwing the RuntimeException.

これも翻訳してみます。

問題: InterruptedException が無視される。これにより、スレッドのシャットダウンが遅延し、スレッドの割り込みステータスがクリアされる可能性があります。スレッドの割り込みポリシーを実装するコードのみが、割り込み要求を飲み込むことができます。

修正: 高レベルの割り込みハンドラーが正しく機能できるように、InterruptedException を再スローするか、Thread.currentThread().interrupt() を使用して現在のスレッドを再中断します。 InterruptedException を RuntimeException 内にラップする場合は、RuntimeException をスローする前に Thread.currentThread().interrupt() を呼び出します。

これはちょっと早めに対応必要かもしれません。。時間見つけて修正するか。。。

まとめ

さて、今回はCodeGuru Reviewerを使ってみた、という話でした。
所感としては、コードレビューでよく指摘されるような内容のものについてはきちんと指摘してくれる、という印象です。
ただ、普段からコードレビューをちゃんとやれている方々であれば使う必要はそれほどないかな、と感じました。
個人的には、