The usual convention for Object.clone() according to Oracle's Javadoc is that:

• x.clone() != x
• x.clone().getClass() == x.getClass()
• x.clone().equals(x)

Obtaining the object that will be returned by calling super.clone() helps to satisfy those invariants:

• super.clone() returns a new object instance
• super.clone() returns an object of the same type as the one clone() was called on
• Object.clone() performs a shallow copy of the object's state

For example, the following code:

class BaseClass implements Cloneable {
  @Override
  public Object clone() throws CloneNotSupportedException {    // Non-Compliant - should return the super.clone() instance
    return new BaseClass();
  }
}

class DerivedClass extends BaseClass implements Cloneable {
  /* Does not override clone() */

  public void sayHello() {
    System.out.println("Hello, world!");
  }
}

class Application {
  public static void main(String[] args) throws Exception {
    DerivedClass instance = new DerivedClass();
    ((DerivedClass) instance.clone()).sayHello();              // Throws a ClassCastException because invariant #2 is violated
  }
}

should be refactored into:

class BaseClass implements Cloneable {
  @Override
  public Object clone() throws CloneNotSupportedException {    // Compliant
    return super.clone();
  }
}

class DerivedClass extends BaseClass implements Cloneable {
  /* Does not override clone() */

  public void sayHello() {
    System.out.println("Hello, world!");
  }
}

class Application {
  public static void main(String[] args) throws Exception {
    DerivedClass instance = new DerivedClass();
    ((DerivedClass) instance.clone()).sayHello();              // Displays "Hello, world!" as expected. Invariant #2 is satisfied
  }
}

Overriding the Object.finalize() method must be done with caution to dispose some system resources. Calling the super.finalize() at the end of this method implementation is highly recommended in case parent implementations must also dispose some system resources.

The following code snippet illustrates this rule:

protected void finalize() {            // Non-Compliant
  releaseSomeResources();
}

protected void finalize() {
  super.finalize();                    // Non-Compliant
  releaseSomeResources();
}

protected void finalize() {
  releaseSomeResources();
  super.finalize();                    // Compliant
}

When the execution is not explicitly terminated at the end of a switch case, it continues to execute the statements of the following case. While this is sometimes intentional, it often is a mistake which leads to unexpected behavior.

This rule doesn't apply to empty cases. Indeed those empty cases allow you to specify the same behavior for a group of cases.

The following code snippet illustrates this rule:

switch (myVariable) {
  case 0:                                // Compliant
  case 1:                                // Compliant
    doSomething();
    break;
  case 2:                                // Compliant
    return;
  case 3:                                // Compliant
    throw new IllegalStateException();
  case 4:                                // Compliant
    continue;
  case 5:                                // Non-Compliant - both 'doSomething()' and 'doSomethingElse()' will be executed
    doSomething();
  default:                               // Non-Compliant
    doSomethingElse();
}

Switch cases should remain small to keep the overall switch compact and readable.

The following code snippet illustrates this rule with the default threshold of 5:

switch (myVariable) {
  case 0:                     // Compliant - 5 lines till following case
    System.out.println("");
    System.out.println("");
    System.out.println("");
    break;
  default:                    // Non-Compliant - 6 lines till switch end
    System.out.println("");
    System.out.println("");
    System.out.println("");
    System.out.println("");
    break;
}

The requirement for a final default clause is defensive programming. This clause should either take appropriate action or contain a suitable comment as to why no action is taken.

The following code snippet illustrates this rule:

switch (state) {                       // Non-Compliant - must have a default case
  case 0:
  case 1:
    System.out.println("0 or 1!");
    break;
}

switch (state) {
  default:                             // Non-Compliant - must be last for better readability
    throw new IllegalStateException();
  case 0:
  case 1:
    System.out.println("0 or 1!");
    break;
}

switch (state) {
  case 0:
  case 1:
    System.out.println("0 or 1!");
    break;
  default:                             // Compliant
    throw new IllegalStateException();

Early classes of the Java API, such as Vector, Hashtable and StringBuffer, were synchronized to make them thread-safe. Unfortunately, synchronization has a big negative impact on performance, even when using these collections from a single thread.

It is better to use their new unsynchronized replacements:

• ArrayList or LinkedList instead of Vector
• Deque instead of Stack
• HashMap instead of Hashtable
• StringBuilder instead of StringBuffer

Noncompliant Code Example

Vector cats = new Vector();

Compliant Solution

ArrayList cats = new ArrayList(); 

Exceptions

Use of those synchronized classes is allowed in method signatures when overriding an existing method.

@Override
public Vector getCats() {...} 

Calling System.exit(int status) or Rutime.getRuntime().exit(int status) leads to the shut down of the entire Java virtual machine. It should be used with care, when stopping the whole Java process is meant. For instance, it should not be called from applications running in a J2EE container.

Noncompliant Code Example

System.exit(0);
Runtime.getRuntime().exit(0);

Two important requirements must be fulfilled when logging messages:

• The user must be able to easily retrieve the logs.
• The format of all messages must be uniform to enable users to easily browse them.

If a program directly writes to the standard output, there is absolutely no way to comply with these requirements. That's why defining and using a dedicated logger is highly recommended.

The following code snippet illustrates this rule:

System.out.println("My Message");  // Non-Compliant

logger.log("My Message");          // Compliant

Developers should not need to configure the tab width of their text editors in order to be able to read source code. So the use of tabulation character must be banned.

According to the Java Language Specification:

Unnamed packages are provided by the Java platform principally for convenience when developing small or temporary applications or when just beginning development.

To enforce this best practice, classes located in default package can no longer be access from named ones since Java 1.4.

The following piece of code:

public class MyClass { /* ... */ }

should be refactored into:

package org.example;

public class MyClass{ /* ... */ }

According to the official Java documentation, Object.finalize() is called by the garbage collector on an object when garbage collection determines that there are no more references to the object. Calling this method explicitly breaks this contract and so is misleading.

The following code snippet illustrates this rule:

public void dispose() throws Throwable {
  this.finalize();                       // Non-Compliant
}

This Object.finalize() method is called by the garbage collector on an object when garbage collection determines that there are no more references to the object. But there is absolutely no warranty that this method will be called AS SOON AS the last references to the object are removed. It can be few microseconds to few minutes later. So when some system resources need to be disposed by an object it's better to not rely on this asynchronous mechanism to dispose them.

The following piece of code illustrates this rule:

public class MyClass {

  protected void finalize() {
    releaseSomeResources();     // Non-Compliant
  }

}

Throwable is the superclass of all errors and exceptions in Java. Error is the superclass of all errors which are not meant to be caught by applications.

Catching either Throwable or Error will also catch OutOfMemoryError or InternalError from which an application should not attempt to recover.

Only Exception and its subclasses should be caught.

Noncompliant Code Example

try { /* ... */ } catch (Throwable t) { /* ... */ }
try { /* ... */ } catch (Error e) { /* ... */ } 

Compliant Solution

try { /* ... */ } catch (Exception e) { /* ... */ }  
try { /* ... */ } catch (RuntimeException e) { /* ... */ }  
try { /* ... */ } catch (MyException e) { /* ... */ }  

Throwable.printStackTrace(...) prints a throwable and its stack trace to some stream.

Loggers should be used instead to print throwables, as they have many advantages:

• Users are able to easily retrieve the logs.
• The format of log messages is uniform and allow users to browse the logs easily.

The following code:

try {
  /* ... */
} catch(Exception e) {
  e.printStackTrace();        // Non-Compliant
}

should be refactored into:

try {
  /* ... */
} catch(Exception e) {
  LOGGER.log("context", e);   // Compliant
}

An exception in a throws declaration in Java is redundant if:

• It is listed multiple times
• It is a subclass of another listed exception
• It is a RuntimeException, or one of its descendants

The following code:

void foo() throws MyException, MyException {}  // Non-Compliant - should be listed once
void bar() throws Throwable, Exception {}      // Non-Compliant - Exception is a subclass of Throwable
void baz() throws RuntimeException {}          // Non-Compliant - RuntimeException can always be thrown

should be refactored into:

void foo() throws MyException {}               // Compliant
void bar() throws Throwable {}                 // Compliant
void baz() {}                                  // Compliant

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
  }
}

Avoid unnecessarily creating local variables

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.