Checkstyleで汎用的に使えそうなルールをピックアップしてみた
Checkstyleを導入しようと思って色々試したところ、色々なルールがあって精査するのに苦労しました。
そこで、汎用的に使えそうなものを今後のためにまとめておきます。
ルールの選定基準
Checkstyleの全てのルールはここにあります。
…が、これを全て精査するのは絶望的です。
そこで、google_checks.xmlとsun_checks.xmlからピックアップしました。
ルールの選定基準は大きく2点です。
- 不具合の未然防止に効果がありそうなもの
- 議論の余地が小さいもの（意見が分かれそうなルールは除外）
ピックアップしたルールのファイル
需要がありそうなので、先に書いておきます。これです。
手軽に試してみたい方はどうぞ。
バグの混入防止に効果がありそうなルール
バグの混入防止、未然防止の観点から、明らかに効果がありそうなルールをピックアップしました。
EqualsHashCode
内容：equals()とhashCode()は同時にオーバーライドされていることの確認
理由：hashCode()が実装されていない場合、HashMapなどのクラスは正しく動作しないため。Effective Javaにも書かれている
References:
FileLength, MethodLength
内容：ファイルが2000行以上になっていないことの確認と、 メソッドが150行以上になっていないことの確認
理由：ファイルやメソッドが長すぎる場合は責務が大きすぎる可能性があり、責務の分割を検討したほうがよいため
References:
OuterTypeFilename, OneTopLevelClass
内容：ファイル名とクラス名の一致確認と、トップレベルには1クラスしか定義されていないことの確認
理由：これらが守られないとクラス名の衝突に繋がるため
References:
MissingSwitchDefault, FallThrough
内容：スイッチ文にdefaultが書かれていることの確認と、各caseでbreakやreturnなどの制御文が書かれていることの確認
理由：switch文でよくあるバグの温床であるため
References:
EmptyBlock, EmptyCatchBlock
内容：ブロックが空でないことの確認と、Catchのブロックが空でないことの確認
理由：書き忘れ、例外の握り潰しなどを防止するため
References:
NoFinalizer
内容：ファイナライザが使われていないことの確認
理由：よく知られたアンチパターンであるため。Effective Javaにも書かれている
References:
NeedBraces
内容：if文やfor文などの制御文で、必ず括弧がついていることの確認
理由：制御文の周辺のインデントが間違っている場合、制御文の適用範囲を勘違することでバグに繋がるため
References:
AvoidStarImport
内容：アスタリスクでインポートしていないことの確認
理由：アスタリスクでインポートすると、偶然クラス名が合致した時など意図していないクラスを使う可能性があるため
References:
SimplifyBooleanExpression, SimplifyBooleanReturn
内容：使っていないor冗長なインポートが定義されていないことの確認
理由：シンプルな実装を考え直すきっかけに繋がるため
References:
HiddenField
内容：フィールド名と、メソッドの引数名が重複していないことの確認
理由：参照先を誤る可能性があるため
なお、コンストラクタやsetterではこのパターンに該当してしまうことも多くあります。
その場合は以下のようにプロパティを指定してコンストラクタやsetterは対象外にするのがよさそうです。
<module name="HiddenField">
<property name="ignoreConstructorParameter" value="true"/>
<property name="ignoreSetter" value="true"/>
</module>References:
UpperEll
内容：longの定数につける末尾の’L’は大文字になっていることの確認
理由：小文字の’l’は1と見間違える可能性があるため
References:
AvoidNestedBlocks
内容：ネストされているブロックが存在しないことの確認
理由：無駄にブロックで囲むと可読性が低くなったり、勘違いしやすくなるため
References:
議論の余地が小さいルール、個人による好みの差があまりないルール
議論の余地が小さいため、適用したほうがよさそうなルールを集めました。
直接的な品質の改善効果は弱いかもしれませんが、可読性向上が見込めそうなものです。
FileTabCharacter
内容：ファイル中にタブ文字が含まれていないかどうか
理由：環境によるレイアウト崩れにつながるため。また、エディタの機能で容易に解決可能であるため
References:
EmptyStatement
内容：空の文がないことの確認。セミコロンが2連続している、など
理由：議論の余地がなく、直し方も明らかであるため
References:
OneStatementPerLine, MultipleVariableDeclarations
内容：一行に一文しか書かれていないことの確認と、複数の変数が同時に宣言されていないことの確認
理由：このルールを適用したほうが編集しやすい他、禁止してもあまり弊害がないため
References:
UnusedImports, RedundantImport
内容：使っていないor冗長なインポートが定義されていないことの確認
理由：除外したほうがよいことは明らかである他、インポートはエディタの機能で自動整形できるため
References:
IllegalTokenText
内容：不必要に分かりにくい表現ユニコード表現などが使われていないことの確認。
理由：可読性の向上。わざわざ分かりにくい表記を使う必要はないため
google_checks.xmlでは以下のように指定されている。
<module name="IllegalTokenText">
<property name="tokens" value="STRING_LITERAL, CHAR_LITERAL"/>
<property name="format"
value="\\u00(09|0(a|A)|0(c|C)|0(d|D)|22|27|5(C|c))|\\(0(10|11|12|14|15|42|47)|134)"/>
<property name="message"
value="Consider using special escape sequence instead of octal value or Unicode escaped value."/>
</module>全部確認したわけではないが、おそらく¥¥u000Dや¥¥u000Aなどの表現は使わず、¥rや¥nを使うべきということだと思います。
References:
PackageName
内容：パッケージ名が命名規則に沿っていることの確認
理由：パッケージ名には特にこだわりはないことが多く、特に弊害もないため
google_checks.xmlではアンダースコアを許していないが、アンダースコアくらいは使ってもよいかもしれない…。
ただしパッケージ名はディレクトリ名として使われるため、アルファベットは大文字を使わず小文字に統一することは守ったほうがよい。1
References:
InnerAssignment
内容：条件式や引数指定における代入を行っていないことの確認
理由：可読性が低くなるため。Javaではboolean型があるため、このようなことをするメリットがほとんどない
References:
NoLineWrap
内容：インポート文などの途中で改行されていないことの確認
理由：インポート文などは改行するメリットがないと思われるため
References:
ModifierOrder
内容：public static finalなどの修飾子の順番が正しいことの確認
理由：慣例として修飾子の順番は決まっているし、特に強いこだわりのある人もいないと思うので
References:
ピックアップしなかったもの
以下のようなものはピックアップしませんでした。
- インデントについてのルール
- 表記ゆれが多そうなルール
- 空白文字に関するポリシー
- 括弧に関するポリシー
- 一文が複数行に渡るときの改行や演算子に関するポリシー
- メソッド名、型名、パラメータ名などの命名規則
- アノテーションやコメントの位置に関するポリシー
- JavaDocコメントに関するポリシー
命名規則あたりは比較的浸透しているし、直し方も明確かもしれませんが、フィールド名やメソッド名に日本語を使っているパターンも一部見かけたので、議論の余地ありということで外しました。2
まとめ
というわけで、28個のルールをピックアップしました。
Checkstyleはマイルールを整備しておくと、新たにプロジェクトを立ち上げる時にもすぐに適用できてよさそうですね。
またよさそうなルールが見つかったら適宜追加していこうと思います。