PMD Java Rules (ver 7.0.0-rc4)- Best Practices ①

PMDバージョン: 7.0.0-rc4

AbstractClassWithoutAbstractMethod

公式ドキュメント: AbstractClassWithoutAbstractMethod

Since: PMD 3.0

Priority: Medium (3)

Description:

抽象メソッドを含まない抽象クラスを検出する。

抽象クラスは不完全な実装を示唆しており、抽象メソッドを実装したサブクラスによって完成される。
クラスが(直接インスタンス化されないように)基底クラスとしてのみ使用されることを意図している場合、抽象クラスにするのではなく、直接インスタンス化されないようにするために可視性を「protected」としたコンストラクタを作成するのが良い。

Configuration:

<!-- プロパティ設定なし -->
<rule ref="category/java/bestpractices.xml/AbstractClassWithoutAbstractMethod" />

Example:

// NG 
// 抽象クラスでは抽象メソッドを使用するか、abstract修飾子を外してprotectedなコンストラクタを追加した方が良い
public abstract class Foo {
  void int method1() { ... }
  void int method2() { ... }
}

// OK
public class Foo {
  protected Foo(){}

  void int method1() { ... }
  void int method2() { ... }
}

// OK
public abstract class Foo {
  void int method1() { ... }
  void int method2() { ... }
  abstract void int method3();
}

AccessorClassGeneration

公式ドキュメント: AccessorClassGeneration

Since: PMD 1.0.4

Priority: Medium (3)

Maximum Language Version: Java 10

Description:

コンストラクタのクラス外から可視性が「private」なコンストラクタを使用してインスタンス化すると、アクセサ・クラスが生成されることがある。
このような状況はファクトリーメソッドを使用するか、コンストラクタの可視性を「private」以外にすることで回避できる。

生成されるクラスファイルは実際にはインターフェースである。これはインターフェースを補助パラメータとして受け取る、可視性が「package-private」なコンストラクタを呼び出す機能をアクセスするクラスに与える。
これによって、可視性が「private」なコンストラクタを事実上「package-private」なコンストラクタに変えてしまう。

※このルールはJava 10以下でのみ実行される。Java 11以降では、JEP 181:ネスト・ベースのアクセス制御が実装され、アクセサ・クラスは生成されなくなった。

Configuration:

<!-- プロパティ設定なし -->
<rule ref="category/java/bestpractices.xml/AccessorClassGeneration" />

Example:

public class Outer {
  void method(){
    Inner ic = new Inner(); // アクセサ・クラスの生成
  }


  public class Inner {
    // アクセサ・クラスを作成しないために、可視性を「package-private」にすると良い
    private Inner(){}
  }
}

AccessorMethodGeneration

公式ドキュメント: AccessorMethodGeneration

アクセサ・メソッドとは:
アクセサメソッドとは、特定のオブジェクト内のクラスやフィールドを操作する際に使用されるメソッドのこと。ゲッター、セッターとも呼ばれる。

Since: PMD 5.5.4

Priority: Medium (3)

Description:

他のクラスから可視性が「private」なフィールドもしくはメソッドにアクセスする場合、Javaコンパイラーは可視性が「package-private」なアクセサ・メソッドを生成する。これはオーバーヘッドになりAndroidのdexメソッド数に加算される。
この状況は、フィールドもしくはメソッドの可視性を「private」から「package-private」に変更することで回避できる。

※このルールはJava 10以下でのみ実行される。Java 11以降では、JEP 181:ネスト・ベースのアクセス制御が実装され、アクセサ・クラスは生成されなくなった。

Configuration:

<!-- プロパティ設定なし -->
<rule ref="category/java/bestpractices.xml/AccessorMethodGeneration" />

Example:

public class OuterClass {
  // 他のクラスから直接アクセスされる際にアクセサ・メソッドが生成されてしまうため、可視性を「package-private」にすると良い
  private int counter;
  /* package */ int id;

  public class InnerClass {
    InnerClass() {
      OuterClass.this.counter++;  // アクセサ・メソッドが生成される
    }

    public int getOuterClassId() {
      return OuterClass.this.id;
    }
  }
}

ArrayIsStoredDirectly

公式ドキュメント: ArrayIsStoredDirectly

Since: PMD 2.2

Priority: Medium (3)

Description:

配列を受け取るコンストラクタやメソッドは、オブジェクトをクローンし、そのコピーを保存しなければならない。
そうすることで、コンストラクタやメソッド内で行われた配列に対する変更が元の配列に影響するのを防ぐことができる。

※引数で渡した配列やリストは、同じメモリ上のものを参照し続けているので、メソッドの処理の中で要素の追加・削除をした場合、メソッドの呼び元の配列やリストの要素も変更される。
要素の変更が意図したものであれば問題ないが、そうではない場合、意図しない挙動を示す可能性がある。

Property:

名前 デフォルト値 説明
allowPrivate true trueの場合、privateなメソッド/コンストラクタで配列を直接格納できるようにする。

Configuration:

<!-- プロパティ設定なし -->
<rule ref="category/java/bestpractices.xml/ArrayIsStoredDirectly" />

<!-- プロパティ設定あり -->
<rule ref="category/java/bestpractices.xml/ArrayIsStoredDirectly">
    <properties>
        <property name="allowPrivate" value="false" />
    </properties>
</rule>

Example:

public class Foo {
  private String[] x;

  public void foo (String [] param) {
    // NG 直接代入はしない
    this.x = param;
  }
}

public class Foo {
  private String[] x;

  public void foo (String[] param) {
    // OK
    this.x = Arrays.copyOf(param, param.length);
  }
}

PMD6からの変更点:

違反はパラメータではなく、代入部分で検出されるようになった。
ルール違反が報告される行番号はおそらく移動するでしょう。


AvoidMessageDigestField

公式ドキュメント: AvoidMessageDigestField

Since: PMD 6.18.0

Priority: Medium (3)

Description:

MessageDigest のインスタンスをフィールドとして宣言すると、このインスタンスを複数のスレッドで直接利用できるようになる。このようなMessageDigestインスタンスの共有は、アクセスが正しく同期されないと誤った結果につながるため、可能な限り避けるべきである。
新しいインスタンスを作成し、必要な場所でローカルで使用するのを推奨する。新しいインスタンスの作成は、共有インスタンスへのアクセスを同期するよりも簡単に実装可能である。

Configuration:

<!-- プロパティ設定なし -->
<rule ref="category/java/bestpractices.xml/AvoidMessageDigestField" />

Example:

import java.security.MessageDigest;

public class AvoidMessageDigestFieldExample {

  private final MessageDigest sharedMd;

  public AvoidMessageDigestFieldExample() throws Exception {
    sharedMd = MessageDigest.getInstance("SHA-256");
  }

  // NG
  public byte[] calculateHashShared(byte[] data) {
    // アクセスを同期させずにフィールドでMessageDigestを共有すると、間違った結果につながる可能性がある。
    sharedMd.reset();
    sharedMd.update(data);
    return sharedMd.digest();
  }

  // OK
  public byte[] calculateHash(byte[] data) throws Exception {
    // 必要な時にインスタンスを生成して使用することで、マルチスレッドでも正しい結果を返却可能
    MessageDigest md = MessageDigest.getInstance("SHA-256");
    md.update(data);
    return md.digest();
  }
}

AvoidPrintStackTrace

公式ドキュメント: AvoidPrintStackTrace

Since: PMD 3.2

Priority: Medium (3)

Description:

printStackTrace()は避け、代わりにロガー呼び出しを使用する。

避けるべき理由等について、以下の記事に詳しく記載されている: 段階的に理解する Java 例外処理

Configuration:

<!-- プロパティ設定なし -->
<rule ref="category/java/bestpractices.xml/AvoidPrintStackTrace" />

Example:

class Foo {
  void bar() {
    try {
      // 何か処理
    } catch (Exception e) {
      // NG printStackTraceを使用しないで、ログ出力するようにした方が良い
      e.printStackTrace();
    }
  }
}

AvoidReassigningCatchVariables

公式ドキュメント: AvoidReassigningCatchVariables

Since: PMD 6.27.0

Priority: Medium (3)

Description:

catch文でキャッチした例外変数の再代入は、以下の理由から避けるべきである:

  1. 必要であればマルチキャッチを簡単に追加できる
    ⇒再代入するのであれば、マルチキャッチにクラスを追加した方が良い
  2. catch文でキャッチされた例外がtryブロックでスローされる例外であることを確認したい
    ⇒再代入すると、tryブロックでスローされた例外の情報を消してしまうことになるので、例外の原因の特定が困難になる

Configuration:

<!-- プロパティ設定なし -->
<rule ref="category/java/bestpractices.xml/AvoidReassigningCatchVariables" />

Example:

public class Foo {
  public void foo() {
    try {
      // 何か処理
    } catch (Exception e) {
      // NG catch変数の再代入はなるべく避ける
      e = new NullPointerException();
    }

    try {
      // 何か処理
    } catch (MyException | ServerException e) {
      // NG マルチキャッチの場合、再代入はコンパイルエラーとなる
      e = new RuntimeException();
    }
  }
}

AvoidReassigningLoopVariables

公式ドキュメント: AvoidReassigningLoopVariables

Since: PMD 6.11.0

Priority: Medium (3)

Description:

ループ変数の再割り当ては見つけにくいバグにつながる可能性がある。
これらの変数が変更されるのを防ぐか、制限する。

foreach ループでは、foreachReassign プロパティで設定する:

  • deny:ループ本体でループ変数の再割り当てを報告する。デフォルト
  • allow:ループ変数をチェックしない
  • firstOnly:ループ変数の再代入は、ループ本体の最初のステートメントを除いて報告する。これは、使用前の値の正規化やクリーンアップが許可されるが、それ以外の変数の変更は許可されない場合に使用する。

forループでは、forReassignプロパティで設定する:

  • deny:ループ本体でループ変数の再割り当てを報告する。デフォルト
  • allow:ループ変数をチェックしない
  • skip:条件付きインクリメント/デクリメント(++, --, +=, -=)を除く、コントロール変数の再割り当てを報告する。これにより、制御変数の偶発的な再割り当てや無条件のインクリメントを防ぐことができる

Property:

名前 デフォルト値 説明
foreachReassign deny foreachのループ変数の再割り当て制御方法
forReassign deny forのループ変数の再割り当て制御方法

Configuration:

<!-- プロパティ設定なし -->
<rule ref="category/java/bestpractices.xml/AvoidReassigningLoopVariables" />

<!-- プロパティ設定あり -->
<rule ref="category/java/bestpractices.xml/AvoidReassigningLoopVariables">
    <properties>
        <property name="foreachReassign" value="deny" />
        <property name="forReassign" value="deny" />
    </properties>
</rule>

Example:

public class Foo {
  private void foo() {
    for (String s : listOfStrings()) {
      // 「foreachReassign」が「firstOnly」もしくは「allow」の場合OK
      s = s.trim(); 
      doSomethingWith(s);

      // 「foreachReassign」が「allow」の場合OK
      s = s.toUpper();
      doSomethingElseWith(s);
    }

    for (int i=0; i < 10; i++) {
      if (check(i)) {
        // 「forReassign」が「skip」もしくは「allow」の場合OK
        i++;
      }

      // 「forReassign」が「allow」の場合OK
      i = 5;

      doSomethingWith(i);
    }
  }
}

PMD6からの変更点:

このルールは、forReassignプロパティがskipに設定されている場合、forループ内の制御変数のすべての再割り当てを報告しなくなる可能性がある。
詳細はissue #4500参照。


AvoidReassigningParameters

公式ドキュメント: AvoidReassigningParameters

Since: PMD 1.0

Priority: Medium High (2)

Description:

コードは入力パラメータの値は変化しないという前提で読まれることが多いため、メソッドやコンストラクタの入力パラメータへの再代入はコードの理解が難しくなってしまう。
メソッドのJavadocにパラメータについて説明が記載されていて、再代入された値が元の文書化された内容と異なる場合に特に問題となる。

パラメータへの再代入の代替手段として、ローカル変数を使うことによって新しい名前を割り当てることができ、コードがより理解しやすくなる。

このルールはメソッドとコンストラクタの両方を考慮することに注意する。入力パラメータに対する複数回の代入がある場合は、最初の代入のみが報告される。

Configuration:

<!-- プロパティ設定なし -->
<rule ref="category/java/bestpractices.xml/AvoidReassigningParameters" />

Example:

public class Hello {
  private void greet(String name) {
    // NG パラメータに別の値を再代入するのはなるべく避ける
    name = name.trim();
    System.out.println("Hello " + name);
  }

  private void greet(String name) {
    // OK 別で変数を定義して値を代入する
    String trimmedName = name.trim();
    System.out.println("Hello " + trimmedName);
  }
}

AvoidStringBufferField

公式ドキュメント: AvoidStringBufferField

Since: PMD 4.2

Priority: Medium (3)

Description:

StringBuffer/StringBuilderはかなり大きくなる可能性があるため、寿命の長いオブジェクト内に保持するとメモリリークの原因になる可能性がある。

StringBufferとStringBuilder、Stringの違いについては下記の記事参照:
StringBuilderとStringBufferの違い
Java言語 StringとStringBufferとStringBuilder

Configuration:

<!-- プロパティ設定なし -->
<rule ref="category/java/bestpractices.xml/AvoidStringBufferField" />

Example:

public class Foo {
     // NG インスタンス変数としてメモリリークの可能性がある
    // 必要な時に適宜インスタンスを生成した方が良い
    private StringBuffer buffer; 
}

AvoidUsingHardCodedIP

公式ドキュメント: AvoidUsingHardCodedIP

Since: PMD 4.1

Priority: Medium (3)

Description:

IPアドレスがハードコードされたアプリケーションは、場合によっては導入が不可能になる。
IPアドレス環境変数や外部ファイルに記載するのが望ましい。

Property:

名前 デフォルト値 説明
checkAddressTypes IPv4 , IPv6 , IPv4 mapped IPv6 IPアドレスの種類を確認する

Configuration:

<!-- プロパティ設定なし -->
<rule ref="category/java/bestpractices.xml/AvoidUsingHardCodedIP" />

<!-- プロパティ設定あり -->
<rule ref="category/java/bestpractices.xml/AvoidUsingHardCodedIP">
    <properties>
        <property name="checkAddressTypes" value="IPv4,IPv6" />
    </properties>
</rule>

Example:

public class Foo {
    // NG IPアドレスをハードコーディングするのは望ましくない
    // 環境変数や外部設定ファイルに記載する方が望ましい
    private String ip = "127.0.0.1";
}

PMD 7.0のルールについて、以下の記事も書いています。

HTML Rule:

olafnosuke.hatenablog.com

JavaScript Rule - Best Practices:

olafnosuke.hatenablog.com

JavaScript Rule - Code Style:

olafnosuke.hatenablog.com

JavaScript Rule - Error Prone:

olafnosuke.hatenablog.com