TODO tags are commonly used to mark places where some more code is required, but which the developer wants to implement later. Sometimes the developer will not have the time or will simply forget to get back to that tag. This rule is meant to track those tags, and ensure that they do not go unnoticed.

The following code illustrates this rule:

void doSomething() {
  // TODO
}

Nesting try-catch blocks severely impacts the readability of source code because it makes it to difficult to understand which block will catch which exception.

The following code:

try {
  try {                                     // Non-Compliant
    doSomething();
  } catch (RuntimeException e) {
    /* Ignore */
  }

  doSomethingElse();
} catch (Exception e) {
  /* ... */
}

should be refactored into:

try {
  dedicatedMethod();                        // Compliant
  doSomethingElse();
} catch (Exception e) {
  /* ... */
}

/* ... */

private void dedicatedMethod() {
  try {                                     // Compliant
    doSomething();
  } catch (RuntimeException e) {
    /* Ignore */
  }
}

Sharing some naming conventions is a key point to make it possible for a team to efficiently collaborate. This rule allows to check that all type parameter names match a provided regular expression.

The following code snippet illustrates this rule when the regular expression value is "^[A-Z]$":

class MyClass<TYPE> { // Non-Compliant
  <TYPE> void addAll(Collection<TYPE> c) { // Non-compliant
  }
}

public class MyClass<T> { // Compliant
  <T> void addAll(Collection<T> c) { // Compliant
  }
}

This code performs an unchecked cast of the return value of a method. The code might be calling the method in such a way that the cast is guaranteed to be safe, but FindBugs is unable to verify that the cast is safe. Check that your program logic ensures that this cast will not fail.

Avoid unnecessarily creating local variables

This field is never read. The field is public or protected, so perhaps it is intended to be used with classes not seen as part of the analysis. If not, consider removing it from the class.

Avoid passing parameters to methods or constructors and then not using those parameters.

Detects when a local variable is declared and/or assigned, but not used.

This rule is deprecated, use squid:S1481 instead.

Fields in interfaces are automatically public static final, and methods are public abstract. Classes or interfaces nested in an interface are automatically public and static (all nested interfaces are automatically static). For historical reasons, modifiers which are implied by the context are accepted by the compiler, but are superfluous.

After checking an object reference for null, you should invoke equals() on that object rather than passing it to another object's equals() method.

Detects when a private field is declared and/or assigned a value, but not used.

This rule is deprecated, use squid:S1068 instead.

Private methods that are never executed are dead code. Dead code means unnecessary, inoperative code that should be removed. This helps in maintenance by decreasing the maintained code size, making it easier to understand the program and preventing bugs from being introduced.

In the following two cases, private methods are not considered as dead code by SonarQube :

• Private empty constructors that are intentionally used to prevent any direct instantiation of a class.
• Private methods : readObject(...), writeObject(...), writeReplace(...), readResolve(...) which can contractually be used when implementing the Serializable interface.

This field is never used. The field is public or protected, so perhaps it is intended to be used with classes not seen as part of the analysis. If not, consider removing it from the class.

No writes were seen to this public/protected field. All reads of it will return the default value. Check for errors (should it have been initialized?), or remove it if it is useless.

The class java.util.Arrays has a asList method that should be use when you want to create a new List from an array of objects. It is faster than executing a loop to cpy all the elements of the array one by one

Use String.indexOf(char) when checking for the index of a single character; it executes faster.

Use StringBuffer.length() to determine StringBuffer length rather than using StringBuffer.toString().equals() or StringBuffer.toString().length() ==.

The imports part of a file should be handled by the Integrated Development Environment (IDE), not manually by the developer. Unused and useless imports should not occur if that is the case. Leaving them in reduces the code's readability, since their presence can be confusing.

The following code snippet illustrates this rule:

package my.company;

import java.lang.String;        // Non-Compliant - java.lang classes are always implicitly imported
import my.company.SomeClass;    // Non-Compliant - same package files are always implicitly imported
import java.io.File;            // Non-Compliant - File is not used

import my.company2.SomeType;
import my.company2.SomeType;    // Non-Compliant - 'SomeType' is already imported

class ExampleClass {

  public String someString;
  public SomeType something;

}

An operation on an Immutable object (BigDecimal or BigInteger) won't change the object itself. The result of the operation is a new object. Therefore, ignoring the operation result is an error.

Useless parentheses can sometimes be misleading and so should be removed.

The following code snippet illustrates this rule:

return 3;             // Compliant
return (x);           // Non-Compliant
return (x + 1);       // Non-Compliant
int x = (y / 2 + 1);  // Non-Compliant
int y = (4+X) * y;    // Compliant

This method calls wait(), notify() or notifyAll() on an object that also provides an await(), signal(), signalAll() method (such as util.concurrent Condition objects). This probably isn't what you want, and even if you do want it, you should consider changing your design, as other developers will find it exceptionally confusing.

Utility classes, which are a collection of static members, are not meant to be instantiated. They should therefore not have public constructors.

Java adds an implicit public constructor to every class which does not define at least one explicitly. Hence, at least one non-public constructor should be defined.

The following code:

class StringUtils { // Non-Compliant

  public static String concatenate(String s1, String s2) {
    return s1 + s2;
  }

}

should be refactored into:

class StringUtils { // Compliant

  private StringUtils {
  }

  public static String concatenate(String s1, String s2) {
    return s1 + s2;
  }

}