Checkstyleで汎用的に使えそうなルールをピックアップしてみた

Checkstyleを導入しようと思って色々試したところ、色々なルールがあって精査するのに苦労しました。
そこで、汎用的に使えそうなものを今後のためにまとめておきます。

ルールの選定基準

Checkstyleの全てのルールはここにあります。
…が、これを全て精査するのは絶望的です。

そこで、google_checks.xmlsun_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はマイルールを整備しておくと、新たにプロジェクトを立ち上げる時にもすぐに適用できてよさそうですね。

またよさそうなルールが見つかったら適宜追加していこうと思います。

References


  1. Windows環境ではファイルシステム上で大文字と小文字が区別されないため、Linuxでは別パッケージだったのにWindowsに移したら一つのパッケージにに統合されてしまう…といったことが起こりうる(未検証) ↩︎

  2. 日本語のフィールド名やメソッド名は賛否ありそうですが、どうしても日本語でしか表現しづらい業務知識の場合はありだと思います。無理に英語で書いて伝わらなかったり、勘違いしてバグになったりしたら悲しいので ↩︎

関連記事


  1. 既存コードへのCheckstyle導入におけるルールの選定
  2. JaCoCoでJavaのテストカバレッジのレポートを出力する
  3. 他の人には読めない形式でGitHubのSecretsの値を読み出す
  4. GitHub Actionsでプルリクのコメントに複数行テキストを投稿する
  5. GitHub Actionsでエラーの時だけ特定の処理を実行する
  6. MavenでOSSのライセンス一覧を出力する
  7. アノテーションを活用した影響調査にトライしてみた

comments powered by Disqus