ソフトウェア工学研究の日々

ソフトウェア工学の学術研究を紹介しています。ソフトウェア開発に関する調査と実験が大好きです。

コードレビューの修正内容は開発者の勉強に役立ちそう

コードレビューはソースコード潜在的な問題を取り除き、読みやすさを向上するために重要な活動だと言われています。私たちの研究室では、将来的にコードレビューをある程度自動化したいという考えを持っていて、コードレビューがどんな修正を行っているのか(そして自動化できそうか)を調査することに取り組みました。

コードレビュー自体はあらゆるソースコードが対象ですが、プログラムの if 文の条件式はけっこうバグの原因になりがち(バグ修正時に if 文の条件式が直されていることが多い)ということが知られていたので、手始めに if 文に対するコードレビューの修正内容を調査し、論文として発表しました。

How are IF-Conditional Statements Fixed Through Peer CodeReview?

この調査でのデータセットは、Gerrit コードレビューのデータを集めることができた Qt、OpenStack プロジェクト(それぞれ C++Python)のパッチです。レビュー数で合計3000件ぐらいあります。コードレビューシステムに投稿された最初のバージョンのパッチと、レビューが終わってソースコードに取り込まれた最終版のパッチの diff (パッチ自体がそれぞれ diff 形式なので、diff の diff になっています)を取って、どんな修正をしたかを調べました。

任意の修正を調べるのは大変そうだったので、「どんな記号が追加されたか・削除されたか」という単位でグループ化し、相関ルールマイニングを使って「初期パッチに〇〇が含まれているなら、△△という修正がよく起きる」という頻出ルールを取り出しています。

結果はプロジェクト特性がけっこう出てしまった気もしますが、Qt プロジェクトでは関数の呼び出し(括弧)の追加や削除が多いという結果になりました。一番典型的だと思う例は関数呼び出しの追加・削除で、たとえば投稿されたパッチにおいて

if (qmlviewerCommand().isEmpty())

とあった条件式が、最終版のパッチでは

if (QtVersion() >= QtSupport::QtVersionNumber(4, 7, 0) &&
    qmlviewerCommand().isEmpty())

とバージョン番号の確認が挿入されていました。

OpenStack プロジェクトのほうでは、否定記号(「!」)がよく削除対象となっていて、たとえば以下のような条件式

if bytes[0] != '−' and bytes[−1] != '−':

は、then-else の内容を入れ替え、配列のインデクスに頼らない(長さ 0 の文字列でも問題ない)以下のような形式に修正されていました。

if bytes.startswith(’−’) or bytes.endswith('−'):

これらの修正は、それぞれプロジェクトのこと、プログラミング言語のことをよく知っていれば書けるけれども、パッチを作っている本人が軽くテストする程度では見逃しそうなところをうまくカバーしているように見えます。

コードレビューには可読性に関する修正もあって、OpenStack プロジェクトでは、以下のような例がありました。

if 'size' in values and values['size']:

この条件文は、辞書型の values に size という値が設定されていることを条件としていましたが、レビューで以下のように書き直されています。

if values.get('size') is not None:

 これは Python の values['size'] が値が存在しないと実行時エラーなのに対し、 values.get('size') は None を返すことを利用した書き換えになっています。

この論文の調査では、結局、レビューの修正は本当に色々あることが分かり、残念ながらすぐに応用が利くような強い結論は出ませんでしたが、コードレビューは間違いなく品質を高めるような修正を行っており、その修正の中には開発者の勉強素材になりそうな知識がたくさん詰まっていそうであるということが分かった点が収穫でした。if 文に限定しない調査も実施していますので、その結果については別途、ご紹介したいと思います。