かんがるーさんの日記

最近自分が興味をもったものを調べた時の手順等を書いています。今は Spring Boot をいじっています。

Spring Boot 1.4.x の Web アプリを 1.5.x へバージョンアップする ( その12 )( build.gradle への PMD の導入2 )

概要

記事一覧はこちらです。

Spring Boot 1.4.x の Web アプリを 1.5.x へバージョンアップする ( その11 )( build.gradle への PMD の導入 ) の続きです。

  • 今回の手順で確認できるのは以下の内容です。
    • 前回 PMD の設定をしたので、今回は指摘を受けた箇所を修正します。
    • PMD のバージョンも 5.7.0 → 5.8.0 → 5.8.1 へバージョンアップします。

参照したサイト・書籍

目次

  1. PMD を 5.8.0 へ上げて build タスクを実行する
  2. 指摘を受けた箇所を修正する
    1. rulesets/java/comments.xml/CommentRequired
    2. rulesets/java/design.xml
    3. rulesets/java/imports.xml
    4. rulesets/java/logging-java.xml
    5. rulesets/java/naming.xml
    6. rulesets/java/optimizations.xml
    7. rulesets/java/strings.xml
    8. rulesets/java/unnecessary.xml
    9. rulesets/java/unusedcode.xml
  3. clean タスク → Rebuild Project → build タスクを実行する
  4. メモ書き
    1. コンソールに出力されるメッセージがどの RuleSet のどの Rule なのかをどうやって探すのか?
    2. RuleSet を複数適用してメッセージが大量に出力されたらどうするか?
  5. PMD を 5.8.0 → 5.8.1 へ上げて build タスクを実行する
  6. 最後に

手順

PMD を 5.8.0 へ上げて build タスクを実行する

build.gradle の以下の点を変更します。

pmd {
    toolVersion = "5.8.0"
    sourceSets = [project.sourceSets.main]
    ignoreFailures = true
    consoleOutput = true
    ruleSetFiles = rootProject.files("/config/pmd/pmd-project-rulesets.xml")
    ruleSets = []
}
  • toolVersion = "5.7.0"toolVersion = "5.8.0" に変更します。

clean タスク → Rebuild Project → build タスク の順に実行します。

f:id:ksby:20170701065604p:plain

199 PMD rule violations were found. という結果でした。5.7.0 の時は 219 PMD rule violations were found. でしたので、バージョンを上げたら 20 も減りましたね。。。?

減ったところを確認したら、以下のメッセージの部分でした。このメッセージが全て出なくなっているのではなく、一部だけのようです。

  • Private field ... could be made final; it is only initialized in the declaration or constructor.

5.7.0 で指摘を受けた箇所を見てみましたが、final を付けるよう指摘されてもおかしくはないようなところが指摘されなくなっているようでした。出なくなった理由は分かりませんが、そのまま進めます。

指摘を受けた箇所を修正する

rulesets/java/comments.xml/CommentRequired

  • CommentRequired の headerCommentRequirement(headerCommentRequirement Required)は、今回は指摘を受けたクラスに以下のコメントを追加して、指摘が出ないようにだけします。
/**
 * ???
 */

rulesets/java/design.xml

  • UseVarargs(Consider using varargs for methods or constructors which take an array the last parameter.)は、メソッドの最後の引数を Object[] argsObject... args のように変更します。。。と思いましたが、指摘された2箇所はどちらも可変引数ではなく Object[] で受けないといけないところでした。特に null が渡されることがあるところを Object... に変更すると、呼び出し元を null(Object) null に変更しないといけなくなるので、さすがにそれは避けたいと思いました。@SuppressWarnings({"PMD.UseVarargs"}) を付けて指摘されないようにします。
  • UnnecessaryLocalBeforeReturn(Consider simply returning the value vs storing it in local variable ...)は、途中に変数を入れていたところを入れないように修正します。ただし、spring-retry の RetryTemplate#execute を使用している部分はコードを修正せず @SuppressWarnings({"PMD.UnnecessaryLocalBeforeReturn"}) を付加してメッセージが出力されないようにします。
    public MessageConverter messageConverter() {
        // 変更前
        // Jackson2JsonMessageConverter converter = new Jackson2JsonMessageConverter();
        // return converter;

        // 変更後
        return new Jackson2JsonMessageConverter();
    }
  • UncommentedEmptyConstructor(Document empty constructor)は、指摘された箇所のコンストラクタを削除できないことに気付いたので、pmd-project-rulesets.xml<exclude name="UncommentedEmptyConstructor"/> を追加して、この rule を外すことにします。
  • ImmutableField(Private field ... could be made final; it is only initialized in the declaration or constructor.)は、フィールド変数に final を付けるよう変更します。
  • AccessorMethodGeneration(Avoid autogenerated methods to access private fields and methods of inner / outer classes)はインナークラスからアクセスしているアウタークラスのフィールド変数のアクセス識別子を private → public へ変更します。でも、これはアウタークラスのフィールド変数にアクセスしていること自体が問題なんだろうな、という気がしています。。。
  • PreserveStackTrace(New exception is thrown in catch block, original stack trace may be lost)は、catch 句の中で throw している例外クラスの引数に catch した例外クラスのインスタンスを追加するよう変更します。
  • ClassWithOnlyPrivateConstructorsShouldBeFinal(A class which only has private constructors should be final)は、private コンストラクタのみのクラスに final を付けるよう変更します。
  • AvoidDeeplyNestedIfStmts(Deeply nested if..then statements are hard to read)は、指摘を付けた箇所の if 文内の処理を private メソッドへ分離して if 文のネストの深さを減らすように変更します。

rulesets/java/imports.xml

  • UnnecessaryFullyQualifiedName(Unnecessary use of fully qualified name ... due to existing import ...)は外部のライブラリの存在チェックをしている @ConditionalOnClass(com.fasterxml.jackson.dataformat.xml.XmlMapper.class) のところで指摘されたのですが、ここはパッケージ込みで記述しておきたいので @SuppressWarnings({"PMD.UnnecessaryFullyQualifiedName"}) を付けて指摘されないようにします。

rulesets/java/logging-java.xml

  • LoggerIsNotStaticFinal(The Logger variable declaration does not contain the static and final modifiers)は、Logger の変数に static を付けていなかったので static を付けるよう変更します。

rulesets/java/naming.xml

  • VariableNamingConventions(Variables that are final and static should be all capitals, ... is not all capitals.)は private static final Logger logger = LoggerFactory.getLogger(~.class); の logger を大文字&アンダーバーに変更するよう指摘されることに気付いたので、pmd-project-rulesets.xml<exclude name="VariableNamingConventions"/> を追加して、この rule を外すことにします。
  • SuspiciousConstantFieldName(The field name indicates a constant but its modifiers do not)は定数のフィールド変数に final を付けていなかったので final を付けるよう変更します。

rulesets/java/optimizations.xml

  • PrematureDeclaration(Avoid declaring a variable if it is unreferenced before a possible exit point.)は、変数を定義する位置と最初に使用している位置が離れていたので、定義する位置を使用する位置の直前に移動します。
  • AvoidInstantiatingObjectsInLoops(Avoid instantiating new objects inside loops)は指摘箇所を修正しようと思ったのですが、ループの外でインスタンスを生成するよう修正できなかったので、pmd-project-rulesets.xml<exclude name="AvoidInstantiatingObjectsInLoops"/> を追加して、この rule を外すことにします。

rulesets/java/strings.xml

  • ConsecutiveAppendsShouldReuse(StringBuffer (or StringBuilder).append is called consecutively without reusing the target variable.)は、指摘通り StringBuilder#append をチェーンするよう変更します。
  • AppendCharacterWithChar(Avoid appending characters as strings in StringBuffer.append.)は、StringBuilder#append の引数が1文字だけの場合には "a"'a' のようにシングルクォーテーションに変更します。
  • AvoidDuplicateLiterals(The String literal "..." appears ... times in this file; the first occurrence is on line ...)は、フィールド定数を定義して、それを使用するよう変更します。

rulesets/java/unnecessary.xml

  • UnnecessaryModifier(Avoid modifiers which are implied by the context)は、インターフェースに定義したメソッドに public を付けていたので削除します。

rulesets/java/unusedcode.xml

  • UnusedPrivateMethod(Avoid unused private methods such as ...)は、@SuppressWarnings({"PMD.UnusedPrivateMethod"}) を付加してメッセージが出力されないようにします。
  • UnusedLocalVariable(Avoid unused local variables such as ...)は、未使用の変数が必要のないよう処理を変更するか、未使用の変数を削除します。

clean タスク → Rebuild Project → build タスクを実行する

上の修正では /config/pmd/pmd-project-rulesets.xml に記載した ReleSet を一旦全てコメントアウトしてから、1つずつコメントアウトを解除して適用しながら確認していました。最後に全て適用した形で build タスクを実行します。

リンク先の内容 が最終版の /config/pmd/pmd-project-rulesets.xml です。

clean タスク → Rebuild Project → build タスク の順に実行し、"BUILD SUCCESSFUL" が出力されることを確認します。

f:id:ksby:20170708093657p:plain

メモ書き

コンソールに出力されるメッセージがどの RuleSet のどの Rule なのかをどうやって探すのか?

例えば rulesets/java/design.xml だけ適用してチェックしていた時に Consider using varargs for methods or constructors which take an array the last parameter. のメッセージが出力された場合、以下の手順で該当の Rule を探しました。

  1. https://github.com/pmd/pmd/tree/master/pmd-java/src/main/resources/rulesets/java の下の design.xml をクリックする。
  2. ページ内で Consider using varargs for methods or constructors which take an array the last parameter. の文字列を検索する。message="Consider using varargs for methods or constructors which take an array the last parameter." がヒットして、これが UseVarargs の Rule であることが分かる。
  3. design.xml の UseVarargs の description を読んだり、https://pmd.github.io/pmd-5.8.0/pmd-java/rules/java/design.html の UseVarargs の説明を読んで変更方法を考える。

適用している RuleSet の xml を開いて、出力されているメッセージの文字列で検索する方法になります。複数の RuleSet を適用している場合には、https://github.com/pmd/pmd/tree/master/pmd-java/src/main/resources/rulesets/java の一番上に表示されている GitHub の検索フィールドに文字列を入力して検索してもいいかもしれません。

ただし、この方法では Rule が見つからない場合があります。

  • メッセージ内のメソッド名や変数名等動的に変わる部分はそのまま検索しても当然ヒットしないので、それ以外の部分で検索すること。
  • メッセージで動的な部分以外で検索しても全然ヒットしない場合があります。例えば naming.xml の VariableNamingConventions(Variables that are final and static should be all capitals, ... is not all capitals.)は “final and static” で検索してもヒットしませんでした。結局 “final” だけで検索してヒットした Rule を1つずつ確認して判断しています。

    メッセージが出力されている Rule の RuleSet が分からず、メッセージで検索してもヒットしない状態だとすごく探しづらいと思います。分からない場合にはまず1つずつ RuleSet を適用していきながら、メッセージが出力されている Rule が含まれる RuleSet を探すところから始めましょう(たぶん)。

RuleSet を複数適用してメッセージが大量に出力されたらどうするか?

一番最初がこの状態でしたが、この場合には RuleSet を1つずつ適用しながら(他の RuleSet はコメントアウトしながら)対応方法を考えるのが良いと思います。

PMD を 5.8.0 → 5.8.1 へ上げて build タスクを実行する

ここまで書いてから PMD のバージョンを確認したところ、5.8.1 がリリースされていました。5.8.1 はバージョンアップしておきます。

build.gradle の以下の点を変更します。

pmd {
    toolVersion = "5.8.1"
    sourceSets = [project.sourceSets.main]
    ignoreFailures = true
    consoleOutput = true
    ruleSetFiles = rootProject.files("/config/pmd/pmd-project-rulesets.xml")
    ruleSets = []
}
  • toolVersion = "5.8.0"toolVersion = "5.8.1" に変更します。

clean タスク → Rebuild Project → build タスク の順に実行します。5.8.1 にバージョンアップしても追加で指摘されたところはありませんでした。

f:id:ksby:20170708114354p:plain

※上の実行前に試しに1度実行したので、PMD 5.8.1 のモジュールのダウンロード処理は出力されていません。

最後に

PMD は checkstyleFindBugs とは別の観点から指摘してくれる(一部重複するところはある)ようで、これも入れた方が良いと思いました。ただし全ての RuleSet を無条件に適用するのは無理があるので(不要と思う RuleSet あるいは Rule は必ず出て来ます)、プロジェクトにあったものだけ適用するよう調整が必要です。

ソースコード

pmd-project-rulesets.xml

<?xml version="1.0" encoding="UTF-8"?>
<ruleset name="mybraces"
         xmlns="http://pmd.sourceforge.net/ruleset/2.0.0"
         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
         xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 http://pmd.sourceforge.net/ruleset_2_0_0.xsd">
    <description>project rulesets</description>

    <!--
        rulesets の種類・説明は 以下の URL 参照
        https://github.com/pmd/pmd/tree/master/pmd-java/src/main/resources/rulesets/java
        https://pmd.github.io/pmd-5.8.0/pmd-java/rules/index.html
        ※"pmd-5.8.0" の部分は適用しているバージョンに変更すること。
    -->
    <rule ref="rulesets/java/basic.xml"/>
    <rule ref="rulesets/java/braces.xml"/>
    <rule ref="rulesets/java/clone.xml"/>
    <rule ref="rulesets/java/codesize.xml">
        <exclude name="TooManyMethods"/>
    </rule>
    <rule ref="rulesets/java/comments.xml/CommentRequired">
        <properties>
            <property name="fieldCommentRequirement" value="Ignored"/>
            <property name="publicMethodCommentRequirement" value="Ignored"/>
            <property name="protectedMethodCommentRequirement" value="Ignored"/>
            <property name="enumCommentRequirement" value="Ignored"/>
        </properties>
    </rule>
    <rule ref="rulesets/java/design.xml">
        <exclude name="UseUtilityClass"/>
        <exclude name="UncommentedEmptyMethodBody"/>
        <exclude name="UncommentedEmptyConstructor"/>
        <exclude name="MissingStaticMethodInNonInstantiatableClass"/>
    </rule>
    <rule ref="rulesets/java/empty.xml"/>
    <rule ref="rulesets/java/finalizers.xml"/>
    <rule ref="rulesets/java/imports.xml"/>
    <rule ref="rulesets/java/logging-java.xml"/>
    <rule ref="rulesets/java/naming.xml">
        <exclude name="ShortVariable"/>
        <exclude name="LongVariable"/>
        <exclude name="ShortMethodName"/>
        <exclude name="VariableNamingConventions"/>
        <exclude name="ShortClassName"/>
    </rule>
    <rule ref="rulesets/java/optimizations.xml">
        <exclude name="LocalVariableCouldBeFinal"/>
        <exclude name="AvoidInstantiatingObjectsInLoops"/>
        <exclude name="MethodArgumentCouldBeFinal"/>
        <exclude name="UseStringBufferForStringAppends"/>
        <exclude name="RedundantFieldInitializer"/>
    </rule>
    <rule ref="rulesets/java/strictexception.xml">
        <exclude name="SignatureDeclareThrowsException"/>
        <exclude name="AvoidThrowingRawExceptionTypes"/>
        <exclude name="AvoidCatchingGenericException"/>
    </rule>
    <rule ref="rulesets/java/strings.xml"/>
    <rule ref="rulesets/java/sunsecure.xml"/>
    <rule ref="rulesets/java/typeresolution.xml">
        <exclude name="SignatureDeclareThrowsException"/>
    </rule>
    <rule ref="rulesets/java/unnecessary.xml">
        <exclude name="UselessParentheses"/>
    </rule>
    <rule ref="rulesets/java/unusedcode.xml"/>
</ruleset>

履歴

2017/07/08
初版発行。