AWS CodeCommitにpushしたソースをAmazon CodeGuruでレビューしてもらったメモ

1. 概要

以前 AWS CodeCommit の使い方について書いた記事で、リポジトリにAmazon CodeGuru を紐づけて機械学習によってコードレビューをして貰えるという話がありました。そのとき後で使ってみると書いていましたが、その後使ってみた結果の内容メモとなります。¹

2. リポジトリ側の設定

リポジトリを作成するときに、そのリポジトリを Amazon CodeGuru へ紐づけることができます。

画面下の「Amazon CodeGuru Reviewer for Java and Python を有効にする 」のチェックボックスをオンにして有効にしておきます。

CodeGuru側からリポジトリを見るとこうなっています。「associated」となっていれば CodeGuruとリポジトリの紐づけができています。

3. フルリポジトリ分析

CodeGuruによるレビューはその対象範囲によって2種類あります。１つはリポジトリ内の全ソースを対象とした「フルリポジトリ分析」です。もう１つはプルリクエストしたときにコミットしたソースを対象とする「増分コードレビュー」です。まずはフルリポジトリ分析をやってみます。

CodeGuru ＞ コードレビュー の画面から 「フルリポジトリ分析を作成」ボタンを押下します。

ここで対象のリポジトリ、ブランチ、コードレビュー名を指定します。コードレビュー名は特にこだわりがなければ自動的に付けられたもので良いでしょう。
「解析設定ファイル - オプション」というのは、yml ファイルを使って特定のファイルやディレクトリをレビューの対象外にしたりできる機能らしいですが、通常は特に使わなくてよいでしょう。

「フルリポジトリ分析を作成」ボタンを押すと分析が始まります。5分～１０分くらいかかるそうです。

CodeGuruの一覧のページに戻ると分析中はステータスが「Pending」になっています。

分析が終わった後コードレビュー画面を見たところです。

コード行4538行に対して 57件も指摘をくらってしまいました。

4. レビュー結果の確認

ではどんなところが指摘を受けているか、少し見てみます。なお、今回の開発言語はPythonです。

(1) 指摘その１. len(文字列) で空文字判定をすべきではない

最初の指摘はこういう内容でした。

指摘内容 (見やすさのため改行しています)

To check if a container or sequence (string, list, tuple) is empty, use if not val. 
Do not compare its length using if len(val) == 0 or if len(val) > 0

...

Similar issue at line numbers 120 and 123.

Following PEP8 makes your code clear and more readable. 
Often there are several ways to perform a similar action in Python. 
PEP 8 provides recommendations to remove that ambiguity and preserve consistency.

指摘されたソースの該当箇所はこんな感じでした。

指摘を受けたコード

    s += ", " if len(s) > 0 else ""

s という文字列の変数の中身が空文字ではなかったらカンマを編集しています。
指摘通りに修正するなら、こんな感じでしょうか。

修正コード

    s += ", " if s else ""

PEP8 に従うならこの方が clear and more readable だとのこと。そうかなぁ…元の形の方がコードの意図が明確に分かって読みやすいのでは？とも思いますが（個人の感想です）。
もう１つ見てみましょう。

(2) 指摘その２．True/False を == で判定すべきではない

指摘内容

The == and != operators use the compared objects' __eq__ method to test 
if they are equal. To check if an object is a singleton, such as None, 
we recommend that you use the is identity comparison operator.

Learn more 

Similar issue at line number 51.

Confusion between equality ==, != and identity is in conditional expressions 
can lead to unintended behavior.

該当のソースは以下。

指摘されたコード

    if len(ze_div_cd) > 0: # ここも１点目と類似で指摘されてそうだが…
        bz = ze_div_cd in cds
        if bz == False:  # 今回の指摘された箇所はここ
            ...

今回の指摘は、ze_div_cd （とあるコード）が cds （いろいろなコードの一覧が入ったリスト）の中に含まれるかを in で判定した結果 bz について True/False 判定をしている箇所についてです。
一見して、これの何がいかんの？と思いますが、指摘内容を見るとNone（およびTrue/False）などの singleton は is 演算子で判定すべきであり、 == 演算子は値が等しいかを判定するものだから混同しない方がよいと。それと、== 演算子は特殊メソッド __eq__() を使って動作をオーバライドできるので思わぬ意図しない動作をしてしまう可能性があるからバグの元だよ、ということを言いたいようです。
なるほど、そう言われればそうかもしれません。None は is で判定していたけれども、True/False も is で判定すべきだとは思いませんでした(Java脳というか…）。この点は勉強になりました。

5. 増分コードレビュー

フルリポジトリ分析とは別に、プルリクエストを出すごとにその差分についてCodeGuruが自動的にレビューをしてくれるという「増分コードレビュー」もあります。プルリクエストの考え方はCodeCommitでも GitHub で行う場合とまったく同じで、コードに対してなんらかの修正をするときにまずフィーチャーブランチ（修正作業用のブランチ）を切ってそこで作業しておき、commit → push をした後そのフィーチャーブランチを master ブランチへマージするためのプルリクエストを 出せるようになっています。
ここで行われるCodeGuruによる自動レビューの指摘観点自体はフルリポジトリ分析とまったく同じものが使われているようです。
プルリクエストの話は今回の主題である CodeGuru の動きとはあまり関係ないため詳細な説明は割愛しますが、プルリクエストの概念さえわかっていれば特に難しいことはなく、画面を見れば問題なく操作できると思います。もちろんCodeGuruによるレビューだけでなく、通常の「人間が見る」レビューも行えます。

【参考になる記事】

6. まとめ

ひとまず少し使ってみての所感をまとめます。

• 「機械学習によるレビュー」と謳っているわりには、ソースの部分的なパターンへの当てはめをしているだけに見え、これだと普通の静的解析ツールと何が違うのかと感じました。機械学習と言っているくらいだからもっとソース全体を読んで内容を把握してレビューしてくれるのかなぁと思っていましたが、期待しすぎだったようです。
• 指摘された内容に対して、レビュイーがどのように対応したか、どこまで対応したかを管理する機能がまったくないのは残念です。対応したしないの消込管理ができた方が嬉しいですよね。そして今全何件中何件まで対応したとか言うことがわかりたいですが、そういう機能はまったくありません。(もしかして、他のAWS Code系のどれかで対応しているのかも？要確認です）
• 今回上で挙げた１件目の指摘などは、指摘された現状の方がソースが読みやすいからこれでよしとする、という考え方もありそうな気がします。そのような場合に、指摘された内容に対してこれこれこういう理由で対応不要とします、などとコメントをつけて却下するといこともあるのではないかと思います。
• レビューはPEPなどの一般的なルールに基づくもので、プロジェクトごとのローカルルールには対応してくれない。ローカルで決めたコーディング規約に沿っているかどうかのレビューなどは全然できなさそうです。
• 同様に、そのプロジェクトの設計書の内容に沿っているかどうかのレビューはしてくれない。これは残念なところで、機械学習と言われるとそのくらいのレベルを期待してしまいます。近々発表されるというGitHub Copilot X はそのレベルまでやってくれそうなことを言っています。

• 分析をした後にコミットが行われてソースの行数が変わってしまうと指摘対象箇所がわからなくなる。ソースの修正に追従しれくれないようです。
• いろいろ不満ばかり書いてしまったので良い所も書くと、指摘された内容はおおむね妥当な指摘が多く、まじめに使えばコードの品質UPに一定の貢献はしてくれそうです。

ということで、若干辛口な感想になってしまいましたが、AWSも今後は機能強化してくると思いますので、期待したいと思います。

今回は以上です。ここまでお読みいただきありがとうございました。

1. 実施してから記事にするまでに少々間が空いてしまったためキャプチャの日付が1月とかになっていますが、記事内容とはあまり関係ありませんので気にしないでください。 ↩