PMD – Designルール・セット


コードデザインに関するルールセットです。
※PMD 3.9に対応しています。

UseSingleton

staticメソッドしか持たないクラスでSingleton化されていないものがないかチェックします。

public class MaybeASingleton
{
    public static void foo()
    {
        // …
    }

    public static void bar()
    {
        // …
    }
}

SimplifyBooleanReturns

booleanを返却するためだけにif..elseステートメントが使われていないかチェックします。

public class Foo
{
    private int bar =2;

    public boolean isBarEqualsTo(int x)
    {
        // booleanを返却するだけに
        // if..elseステートメントが使われているのでNG!
        if (bar == x)
        {
            return true;
        }
        else
        {
            return false;
        }

        // 上記はこう書ける
        // return bar == x;
    }
}

SimplifyBooleanExpressions

単純化できるboolean値の比較がないかチェックします。

public class Bar
{
    // 単純に bar = isFoo(); と書けるのでNG!
    private boolean bar = (isFoo() == true);

    public isFoo()
    {
        return false;
    }
}

SwitchStmtsShouldHaveDefault

switch文にdefaultが指定されているかチェックします。

public class Foo
{
    public void bar()
    {
        int x = 2;

        // switch文にdefaultがないのでNG!
        switch (x)
        {
            case 2:
                int j = 8;
        }
    }
}

AvoidDeeplyNestedIfStmts

ネストの深いifステートメントがないかチェックします。
チェック対象外とするネストの最小値を指定する場合は
<property name=”problemDepth” value=”3″ />
を指定します。
デフォルト値は3です。

public class Foo
{
    public void bar(int x, int y, int z)
    {
        if (x < y)
        {
            if (y < z)
            {
                if (z == x)
                {
                    // …
                }
            }
        }
    }
}

AvoidReassigningParameters

パラメータに値を再設定していないかチェックします。

public class Foo
{
    private void foo(String bar)
    {
        // パラメータに値を再設定しているのでNG!
        bar = "something else";
    }
}

SwitchDensity

switch文のcaseブロックに多くの処理が記述されていないかチェックします。
チェック対象外とするステートメント数の最小値を指定する場合は
<property name=”minimum” value=”10″ />
を指定します。
デフォルト値は10です。

public class Foo
{
    public void bar(int x)
    {
        switch (x)
        {
            case 1:
                //多くの処理
                break;
            case 2:
                // 多くの処理
                break;
        }
    }
}

ConstructorCallsOverridableMethod

コンストラクタ内でオーバーライドされているメソッドがないかチェックします。

public class SeniorClass
{
    public SeniorClass()
    {
        // オーバーライドしていると
        // NullPointerExceptionが発生するかもしれない
        toString();
    }

    public String toString()
    {
        return "IAmSeniorClass";
    }
}

public class JuniorClass extends SeniorClass
{
    private String name; public JuniorClass()
    {
        // nameが設定されていないのでNullPointerExceptionが発生する
        super();
        name = "JuniorClass";
    }

    public String toString()
    {
        return name.toUpperCase();
    }
}

AccessorClassGeneration

privateコンストラクタが呼び出されていないかチェックします。
ファクトリーメソッドを使用すべきです。

public class Outer
{
    void method()
    {
        Inner ic = new Inner();
    }

    public class Inner
    {
        private Inner() {
            // …
        }
    }
}

FinalFieldCouldBeStatic

finalフィールドでstatic宣言されていないフィールドがないかチェックします。

public class Foo
{
    // staticで宣言すべきである
    public final int BAR = 42;
}

CloseResource

閉じられていないリソースがないかチェックします。
チェック対象とするリソースを指定する場合は
<property name=”types” value=”Connection,Statement,ResultSet″ />
を指定します。
デフォルト値はConnection,Statement,ResultSetです。

public class Bar
{
    public void foo()
    {
        Connection c = pool.getConnection();
        try
        {
            // …
        }
        catch (SQLException ex)
        {
            // …
        }
        finally
        {
            // c.close();
        }
    }
}

NonStaticInitializer

staticでないイニシャライザがないかチェックします。

public class MyClass
{
    // インスタンスが生成される度に実行されてしまう
    {
        System.out.println("I am about to construct myself");
    }
}

DefaultLabelNotLastInSwitchStmt

switch文のdefaultが最後にあるかチェックします。

public class Foo
{
    void bar(int a)
    {
        switch (a)
        {
            case 1:
                // …
                break;
            default:
                // defaultがswitch文の最後でないのでNG!
                break;
            case 2:
                break;
        }
    }
}

NonCaseLabelInSwitchStatement

switch文でcaseラベル以外が記述されていないかチェックします。

public class Foo
{
    void bar(int a)
    {
        switch (a)
        {
            case 1:
                // …
                break;
            mylabel:
                // …
                break;
            default:
                break;
        }
    }
}

OptimizableToArrayCall

Collection.toArray()が必要なサイズの配列で呼び出されているかチェックします。

class Foo
{
    void bar(Collection x)
    {
        x.toArray(new Foo[0]);

        // こっちの方が効率がよい
        x.toArray(new Foo[x.size()]);
    }
}

BadComparison

Double.NaNで比較されていないかチェックします。

public class Bar
{
    boolean x = (y == Double.NaN);
}

EqualsNull

equals()でnullと比較されていないかチェックします。

class Bar
{
    void foo()
    {
        String x = "foo";
        if (x.equals(null))
        {
            // …
        }
    }
}

ConfusingTernary

if…elseステートメントの条件に否定が使われていないかチェックします。

public class Foo
{
    boolean bar(int x, int y)
    {
        return (x != y) ? diff : same;
        // こっちの方が分かりやすい
        // return (x == y) ? same : diff;
    }
}

InstantiationToGetClass

getClass()でクラスが取得されていないかチェックします。

public class Foo
{
    Class c = new String().getClass();

    // こっちの方がよい
    // Class c = String.class;
}

IdempotentOperations

無意味な代入がないかチェックします。

public class Foo
{
    public void bar()
    {
        int x = 2;
        // 無意味
        x = x;
    }
}

SimpleDateFormatNeedsLocale

SimpleDateFormat生成時にロケールが指定されているかチェックします。

public class Foo
{
    // ロケールを指定すべきです
    private SimpleDateFormat sdf = new SimpleDateFormat("pattern");
}

ImmutableField

finalで宣言されていない不変なフィールドがないかチェックします。

public class Foo
{
    // 不変なのでfinalで宣言すべきです
    private int x;

    public Foo()
    {
        x = 7;
    }

    public void foo()
    {
        int a = x + 2;
    }
}

UseLocaleWithCaseConversions

String.toLowerCase()メソッド/toUpperCase()メソッドでロケールが指定されているかチェックします。
ロケールを指定しない場合、トルコ語などで問題があります。

class Foo
{
    // ロケールが指定されていないのでNG!
    String z1 = a.toLowerCase();

    // ロケールが指定されているのでOK!
    String z2 = a.toLowerCase(Locale.EN);
}

AvoidProtectedFieldInFinalClass

finalクラスでprotectedフィールドが宣言されていないかチェックします。

public final class Bar
{
    private int x;

    // finalクラスでprotected宣言されている
    // サブクラスを作れないので無意味です
    protected int y;

    Bar()
    {
        // …
    }
}

AssignmentToNonFinalStatic

staticフィールドへ代入されていないかチェックします。

public class StaticField
{
    static int x;

    public FinalFields(int y)
    {
        // staticフィールドへ代入されているのでNG!
        x = y;
    }
}

MissingStaticMethodInNonInstantiatableClass

コンストラクタがprivateでstaticメソッドがないクラスがないかチェックします。

public class Foo
{
    private Foo()
    {
        // …
    }

    void foo()
    {
        // …
    }
}

AvoidSynchronizedAtMethodLevel

メソッドレベルの同期(synchronized)がないかチェックします。

public class Foo
{
    // メソッドレベルの同期はNG!
    synchronized void foo()
    {
    }

    void bar()
    {
        // ブロックレベルの同期はOK!
        synchronized(this) {
        }
    }
}

MissingBreakInSwitch

break文が記述されていないswitch文がないかチェックします。

public class Foo
{
    public void bar(int status)
    {
        switch(status)
        {
            case CANCELLED:
                doCancelled();
                //break;
            case NEW:
                doNew();
            case REMOVED:
                doRemoved();
        }
    }
}

UseNotifyAllInsteadOfNotify

Thread.notify()メソッドが使用されていないかチェックします。
Thread.notifyAll()メソッドを使用すべきです。

public class Foo
{
    void bar()
    {
        x.notify();

        // こっちを使うべきです
        x.notifyAll();
    }
}

AvoidInstanceofChecksInCatchClause

catchした例外をそのcatchブロックの中で処理しているかチェックします。

    // こっちの方はよくない
    try
    {
        // …
    }
    catch (Exception ee)
    {
        if (ee instanceof IOException)
        {
            cleanup();
        }
    }

    // こっちの方がよい
    try
    {
        // …
    }
    catch (IOException ee)
    {
        cleanup();
    }

AbstractClassWithoutAbstractMethod

abstractクラスにabstractメソッドが記述されているかチェックします。

public abstract class Foo
{
    void int method1()
    {
       // …
    }

    void int method2()
    {
        // …
    }
}

SimplifyConditional

instanceofの前に不要なnullチェックがないかチェックします。

class Foo
{
    void bar(Object x)
    {
        if (x != null && x instanceof Bar)
        {
            // just drop the "x != null" check
        }
    }
}

CompareObjectsWithEquals

オブジェクトを==で比較していないかチェックします。
オブジェクトの比較にはequals()メソッドを使用すべきです。

class Foo
{
    boolean bar(String a, String b)
    {
        return a == b;
    }
}

PositionLiteralsFirstInComparisons

左辺がオブジェクト、右辺が定数で文字列を比較していないかチェックします。

class Foo
{
    boolean bar(String x)
    {
        // xがnullの場合、NullPointerExceptionが発生するので、
        // "2".equals(x)とすべきです
        return x.equals("2");
    }
}

UnnecessaryLocalBeforeReturn

不必要なローカル変数がないかチェックします。

public class Foo
{
    public int foo()
    {
        int x = doSomething();
        // 単にreturn doSomething();でよい
        return x;
    }
}

NonThreadSafeSingleton

スレッドセーフになっていないSingletonクラスがないかチェックします。

private static Foo foo = null;

public static Foo getFoo()
{
    if (foo==null) foo = new Foo();
    return foo;
}

UncommentedEmptyMethod

コメントがない空のメソッドがないかチェックします。

public void doSomething()
{
}

UncommentedEmptyConstructor

コメントがない空のコンストラクタがないかチェックします。

public Foo()
{
    super();
}

AvoidConstantsInterface

定数のみのインターフェイスがないかチェックします。

public interface ConstantsInterface
{
    public static final int CONSTANT1 = 0;
    public static final String CONSTANT2 = "1";
}

UnsynchronizedStaticDateFormatter

同期化していないSimpleDateFormatがないかチェックします。

public class Foo
{
    private static final SimpleDateFormat sdf = new SimpleDateFormat();

    void bar()
    {
        // こっちの方はよくない
        sdf.format();
    }

    synchronized void foo()
    {
        // こっちの方はよい
        sdf.format();
    }
}

PreserveStackTrace

スタックトレースが捨てられていないかチェックします。

public class Foo
{
    void good()
    {
        try
        {
            Integer.parseInt("a");
        }
        catch (Exception e)
        {
            throw new Exception(e);
        }
    }

    void bad()
    {
        try
        {
            Integer.parseInt("a");
        }
        catch (Exception e)
        {
            throw new Exception(e.getMessage());
        }
    }
}

UseCollectionIsEmpty

コレクションの判定でsize()==0が使われていないかチェックします。
isEmpty()を使用すべきです。

public class Foo
{
    void good()
    {
        List foo = getList();
        if (foo.isEmpty())
        {
            // …
        }
    }

    void bad()
    {
        List foo = getList();
        if (foo.size() == 0)
        {
            // …
        }
    }
}

関連記事

  1. コメント 0

  1. トラックバック 0

return top