Controversial

問題のありそうなロジックを検出するルールセットです。

UnnecessaryConstructor

不必要なコンストラクタ宣言を検出します。

public class Foo {
  public Foo() {}
}

変数宣言文以外でのnull値の代入を検出します。

Object x = null; // これはOK
x = new Object();
x = null; // これはNG

一般的に、これは悪い形式のコーディングだとされています。
昔の記事にはよく、
「明示的にnullにしないとガーベッジコレクト(以下GC)されないのでnullを代入する事」
などという事が書いてありましたが、これは一般的には
ロジックそのものに何らかの間違いがある事を意味しています。

ただし、一部のソートロジックでは明示的なnull値の代入が
GCの効率を上げるために有用である事もあります。
その事に自信があるのなら、この警告を無視しても構わないでしょう。

メソッドの最後以外にreturn文があるロジックを検出します。
つまりこのルールは、メソッドの最後に唯一のreturn文がある事を期待します。

public void foo(int x) {
  if (x &gt; 0) {
    return "hey";   // メソッドの途中にあるreturn文
  }
  return "hi";
}

不必要な識別子の記述を検出します。

public interface Foo {
  public abstract void bar(); // public abstract は必要無い
  public static final int X = 0; // public static final は必要無い
}

オペランド中の代入ロジックを検出します。

if ((x = getX()) == 10) { // 読みにくいので止めた方がいい
  ...
}

コンストラクタが一つも定義されていないクラスを検出します。
この場合、クラスには引数無のデフォルトコンストラクタが作成されますが
これに頼ったコーディングは、思わぬところで弊害を引き起こす可能性があります。

sun. で始まるimport文を検出します。
これらは通常、明示的にソースに記述すべきではありません。

疑わしい8進数表記を検出します。

s = "\123"; // これは "\123" と見なされる
s = "\128"; // これは "\12" + "8" と見なされる(8進数に8は使えない為)

コンストラクタ内で super または this が呼び出されていないロジックを検出します。

public Foo() {
  this.cnt = 10; // これはNG
}

public Foo() {
  super(); // これはOK
}

public Foo() {
  this(10); // これもOK
}

不必要な括弧を検出します。

return (true); // この括弧は不必要

1箇所からしか使用されていないフィールドを検出します。
通常、これはローカル変数化できます。

private int x; // このフィールドは1箇所からしか使われていない
public void foo(int y) {
   x = y + 5;
   return x;
}

アクセス識別子を指定していない箇所を検出します。
この場合、パッケージprivateとなります。これはあまり推奨されません。

public class Foo {
  int bar; // アクセス識別子が指定されていない
}

最適化されていないBoolean値の反転ロジックを検出します。

b = !b; // これは以下のようにする
b ^= true; // …いや、まぁ確かに。でも正直言ってわかりにくいと思いません？