FindBugsの使用例: CheckstyleやPMDを超える機能
静的分析ツールはオープンソースにも商用ソフトにも数多くありますが、FindBugsもその1つです。Javaの開発には、他にCheckstyleとPMDの2つがよく利用されます。Checkstyleは、命名規則やスペースの用法、Javadocsの有無といったコーディング標準に重点を置いた、従来型のツールです。PMDは、ベストプラクティス、最適化されていないコード、潜在的なエラーに重点を置くツールです。性格上、どの静的分析ツールにも似たような機能がありますが、これはCheckstyleとPMDについては特に顕著です。この2つほどではありませんが、PMDとFindBugsにも共通する部分があります。
FindBugsが得意とする分野を理解するために、具体的な例を見ていきましょう。自治体がペットの犬の登録に使うデータベースを実装すると仮定します。次のクラスは、メインドメインのオブジェクト、つまりペットの犬を表します。
public class Dog {
private String name;
private String breed;
// 以下、getterとsetter
//...
}
次に、プロジェクトの途中で、複数のDogインスタンスを1つのセットに配置することにした、などの理由から、equals()メソッドの実装が必要になったとします。ただし、equals()メソッドを正しく実装するのは厄介な仕事です。経験の浅いプログラマであれば、なおさらでしょう。以下に、上出来とはいえないDogクラスの実装例を示します。
public class Dog {
private String name;
private String breed;
@Override
public boolean equals(Object obj) {
Dog otherDog = (Dog) obj;
if ((otherDog.name.equals(this.name))
&& (otherDog.breed.equals(this.breed))) {
return true;
}
return false;
}
}
この実装には複数のコーディングエラーがあり、アプリケーションがクラッシュしたり、予期しない動きを見せる可能性があります。特に、指定されたオブジェクトがDogクラスのインスタンスではない場合、このメソッドはClassCastExceptionを生成します。また、オブジェクトがnullかどうかをチェックしていません。さらに、equals()メソッドをオーバーライドしたのに、それに対応するhashCode()のオーバーライド実装がありません。Java言語では、同一のオブジェクトは同一のハッシュコード値を返すことが求められます。これが守られないと、高い確率でHashMapsとHashTablesは正常に動作しません。したがって、equals()メソッドをオーバーライドするときは、常にhashCode()メソッドもオーバーライドする必要があります。このルールはベテランのJava開発者には常識ですが、経験の浅い開発者は見逃してしまいがちです。
この実装には、Javadocコメントがないなど、雑なコーディングの習慣も見られます。
この記事で前に触れた静的分析ツールは、どれもこのコードについて複数の問題点を検出します。たとえば、Checkstyleは次のエラーを指摘します。
- Javadocコメントがない。
- パラメータobjはfinalであるべき（メソッド内でobjの値を変更しないため）。
equals()の定義に対応するhashCode()の定義がない。
最初のエラーはコーディング標準の問題、2番目のエラーはコーディングのベストプラクティスの問題、そして最後のエラーは潜在的なバグであり、ベストプラクティスとしても知られるものです。このエラーレポートには、Checkstyleが全般的にコーディング標準と良質のプログラミング習慣の徹底に重点を置いていることがよく現れています。
PMDは、ベストプラクティスをより重視します。このコードをPMDでチェックすると、次の問題が指摘されます。
- メソッドの出口は1箇所のみにし、メソッドの最後のステートメントとして記述するべき。
equals()とhashCode()の両方をオーバーライドすること。- パラメータobjに値が代入されない。これはfinalとして宣言できる。
3つのエラーのうち2つはCheckstyleでも検出されたものです。PMDだけが指摘した問題点は、一般的なJavaコーディングのベストプラクティスである「returnステートメントの数をなるべく減らしてコードをシンプルにする」に由来するものです。ベストプラクティスを重視するPMDの姿勢がよくわかります。PMDは、Javadocコメントの欠落を意に介しませんし、メソッドや変数の命名規則、スペースとインデントの用法、その他の純然たるコーディング標準の問題には注意を向けません。
さて、FindBugsはどうでしょうか。他のツールと同様に、FindBugsでもこのコードに3つの問題点が指摘されますが、その性質は他の2つのツールで検出されるエラーの性質と異なります。
equals()メソッドは、引数が常に特定のデータ型であることを前提としてはならない。- このクラスは、
equals()を定義し、Object.hashCode()を使用している。 equals()メソッドがnull引数をチェックしない。
最初のエラーは、このequals()メソッドのデザインにある大きな欠陥の1つです。引数がDogクラスのインスタンスでない場合、メソッドは正常に動作しません。この問題に関する詳細な情報も以下のようにFindBugsから提供され、問題と解決方法を知るのに役立ちます。
equals(Object o)メソッドはoが常に特定のデータ型であることを前提としてはなりません。oがthisと異なる型の場合はfalseを返す必要があります。 2番目の問題点は、他の2つのツールが指摘したequals()/hashCode()の問題と同じです。ただし、FindBugsが提示する説明は、この問題の原因と解決方法にも言及します。
このクラスはequals(Object)をオーバーライドしますが、hashCode()をオーバーライドしません。hashCode()の実装はjava.lang.Objectから継承されますが、この実装はVMからオブジェクトに割り当てられる任意の値をIDハッシュコードとして返します。したがって、このクラスは、同一のオブジェクトは同一のハッシュコードを持たなくてはならないという原則に反する可能性が非常に高くなります。このクラスのインスタンスがHashMap/HashTableに挿入されることはないと考えている場合は、以下のhashCode実装の使用を推奨します。
public int hashCode() {
assert false : "hashCode not designed";
return 42; // 定数は任意の値でかまわない
}
3番目の問題点は、FindBugsだけが検出する別の潜在的なバグです。このメソッドでは、パラメータのnull値がチェックされません。FindBugsでは、エラーメッセージに示される情報よりもさらに正確にこの問題が分析され、問題の詳細と簡単な解決方法を示す説明が提示されます。
equals(Object)の実装は、引数として渡されるnullをチェックしないため、java.lang.Object.equals()で定義された規約に違反します。すべてのequals()メソッドはnullが渡された場合にfalseを返すべきです。
この簡単な例からも、FindBugsと他の静的分析ツールに次の2つの大きな相違があることがわかります。
- FindBugsは、アプリケーションのバグを招くコーディングの問題にほぼ全面的に重点を置く。
- FindBugsは、検出した問題について明確かつ詳細な説明を提示する。これは問題の理解と修正に役立つ。
別の例をもう1つ見てみましょう。
public void foo() {
List<String> values = null;
List<Integer> results = new ArrayList<Integer>();
for (String str : values) {
int value = Integer.parseInt(str);
results.add(new Integer(value));
}
}
この例について、Checkstyleは、Javadocコメントの欠落以外に大きな問題点を検出しません。Checkstyleを念入りに設定した場合を除き、'<' '>'演算子の前後にスペースがないことが指摘されます。ここでもCheckstyleはコーディングのスタイルとレイアウトにこだわっています。
デフォルト設定のPMDは、6つの問題点を検出します。ほとんどは、パフォーマンスの最適化に関するものです。以下に一部を示します。
- 変数
values、results、valueはfinalとして宣言できる（理屈の上では正しい指摘ですが、実際にはまず見られないコーディングスタイルです）。 - ループ内で変数をインスタンス化することは避ける（これは妥当な指摘です）。
- 変数resultsに'DU'アノマリーがある（ややあいまいな指摘です）。
一方、FindBugsは2つの問題点しか指摘しません。
values変数にnullポインタ逆参照がある。これは実行時にNullPointerExceptionを招く（図4を参照）。new Integer(value)の使い方が非効率。代わりにInteger.valueOf(value)を使用すると、不要なオブジェクトの作成（PMDで検出される2番目の問題と同様の潜在的なパフォーマンスの問題）が避けられる。
ここでもやはり、FindBugsが潜在的なバグに重点を置いていることがわかります。
静的分析ツールに誤報は付き物です。しかし、FindBugsで検出される問題点は高い確率で（個人的な経験から言って50%以上）本物のバグを明らかにします。
