r4fterman/pdf.forms

Overriding or shadowing a variable declared in an outer scope can strongly impact the readability, and therefore the maintainability, of a piece of code. Further, it could lead maintainers to introduce bugs because they think they're using one variable but are really using another.

Noncompliant Code Example

class Foo {
  public int myField;

  public void doSomething() {
    int myField = 0;
    ...
  }
}

See

• CERT, DCL01-C. - Do not reuse variable names in subscopes
• CERT, DCL51-J. - Do not shadow or obscure identifiers in subscopes

A dead store happens when a local variable is assigned a value that is not read by any subsequent instruction. Calculating or retrieving a value only to then overwrite it or throw it away, could indicate a serious error in the code. Even if it's not an error, it is at best a waste of resources. Therefore all calculated values should be used.

Noncompliant Code Example

i = a + b; // Noncompliant; calculation result not used before value is overwritten
i = compute();

Compliant Solution

i = a + b;
i += compute();

Exceptions

This rule ignores initializations to -1, 0, 1, null, true, false and "".

See

• MITRE, CWE-563 - Assignment to Variable without Use ('Unused Variable')
• CERT, MSC13-C. - Detect and remove unused values
• CERT, MSC56-J. - Detect and remove superfluous code and values

Programmers should not comment out code as it bloats programs and reduces readability.

Unused code should be deleted and can be retrieved from source control history if required.

There are several reasons for a method not to have a method body:

• It is an unintentional omission, and should be fixed to prevent an unexpected behavior in production.
• It is not yet, or never will be, supported. In this case an UnsupportedOperationException should be thrown.
• The method is an intentionally-blank override. In this case a nested comment should explain the reason for the blank override.

Noncompliant Code Example

public void doSomething() {
}

public void doSomethingElse() {
}

Compliant Solution

@Override
public void doSomething() {
  // Do nothing because of X and Y.
}

@Override
public void doSomethingElse() {
  throw new UnsupportedOperationException();
}

Exceptions

Default (no-argument) constructors are ignored when there are other constructors in the class, as are empty methods in abstract classes.

public abstract class Animal {
  void speak() {  // default implementation ignored
  }
}

Programmers should not comment out code as it bloats programs and reduces readability.

Unused code should be deleted and can be retrieved from source control history if required.

Fields in a Serializable class must themselves be either Serializable or transient even if the class is never explicitly serialized or deserialized. For instance, under load, most J2EE application frameworks flush objects to disk, and an allegedly Serializable object with non-transient, non-serializable data members could cause program crashes, and open the door to attackers. In general a Serializable class is expected to fulfil its contract and not have an unexpected behaviour when an instance is serialized.

This rule raises an issue on non-Serializable fields, and on collection fields when they are not private (because they could be assigned non-Serializable values externally), and when they are assigned non-Serializable types within the class.

Noncompliant Code Example

public class Address {
  //...
}

public class Person implements Serializable {
  private static final long serialVersionUID = 1905122041950251207L;

  private String name;
  private Address address;  // Noncompliant; Address isn't serializable
}

Compliant Solution

public class Address implements Serializable {
  private static final long serialVersionUID = 2405172041950251807L;
}

public class Person implements Serializable {
  private static final long serialVersionUID = 1905122041950251207L;

  private String name;
  private Address address;
}

Exceptions

The alternative to making all members serializable or transient is to implement special methods which take on the responsibility of properly serializing and de-serializing the object. This rule ignores classes which implement the following methods:

 private void writeObject(java.io.ObjectOutputStream out)
     throws IOException
 private void readObject(java.io.ObjectInputStream in)
     throws IOException, ClassNotFoundException;

See

• MITRE, CWE-594 - Saving Unserializable Objects to Disk
• Oracle Java 6, Serializable
• Oracle Java 7, Serializable

A for loop stop condition should test the loop counter against an invariant value (i.e. one that is true at both the beginning and ending of every loop iteration). Ideally, this means that the stop condition is set to a local variable just before the loop begins.

Stop conditions that are not invariant are slightly less efficient, as well as being difficult to understand and maintain, and likely lead to the introduction of errors in the future.

This rule tracks three types of non-invariant stop conditions:

• When the loop counters are updated in the body of the for loop
• When the stop condition depend upon a method call
• When the stop condition depends on an object property, since such properties could change during the execution of the loop.

Noncompliant Code Example

for (int i = 0; i < 10; i++) {
  ...
  i = i - 1; // Noncompliant; counter updated in the body of the loop
  ...
}

Compliant Solution

for (int i = 0; i < 10; i++) {...}

Fields in a Serializable class must themselves be either Serializable or transient even if the class is never explicitly serialized or deserialized. For instance, under load, most J2EE application frameworks flush objects to disk, and an allegedly Serializable object with non-transient, non-serializable data members could cause program crashes, and open the door to attackers. In general a Serializable class is expected to fulfil its contract and not have an unexpected behaviour when an instance is serialized.

This rule raises an issue on non-Serializable fields, and on collection fields when they are not private (because they could be assigned non-Serializable values externally), and when they are assigned non-Serializable types within the class.

Noncompliant Code Example

public class Address {
  //...
}

public class Person implements Serializable {
  private static final long serialVersionUID = 1905122041950251207L;

  private String name;
  private Address address;  // Noncompliant; Address isn't serializable
}

Compliant Solution

public class Address implements Serializable {
  private static final long serialVersionUID = 2405172041950251807L;
}

public class Person implements Serializable {
  private static final long serialVersionUID = 1905122041950251207L;

  private String name;
  private Address address;
}

Exceptions

The alternative to making all members serializable or transient is to implement special methods which take on the responsibility of properly serializing and de-serializing the object. This rule ignores classes which implement the following methods:

 private void writeObject(java.io.ObjectOutputStream out)
     throws IOException
 private void readObject(java.io.ObjectInputStream in)
     throws IOException, ClassNotFoundException;

See

• MITRE, CWE-594 - Saving Unserializable Objects to Disk
• Oracle Java 6, Serializable
• Oracle Java 7, Serializable

Programmers should not comment out code as it bloats programs and reduces readability.

Unused code should be deleted and can be retrieved from source control history if required.

Programmers should not comment out code as it bloats programs and reduces readability.

Unused code should be deleted and can be retrieved from source control history if required.

Deprecation should be marked with both the @Deprecated annotation and @deprecated Javadoc tag. The annotation enables tools such as IDEs to warn about referencing deprecated elements, and the tag can be used to explain when it was deprecated, why, and how references should be refactored.

Further, Java 9 adds two additional arguments to the annotation:

• since allows you to describe when the deprecation took place
• forRemoval, indicates whether the deprecated element will be removed at some future date

If your compile level is Java 9 or higher, you should be using one or both of these arguments.

Noncompliant Code Example

class MyClass {

  @Deprecated
  public void foo1() {
  }

  /**
    * @deprecated
    */
  public void foo2() {    // Noncompliant
  }

}

Compliant Solution

class MyClass {

  /**
    * @deprecated (when, why, refactoring advice...)
    */
  @Deprecated
  public void foo1() {
  }

  /**
    * Java >= 9
    * @deprecated (when, why, refactoring advice...)
    */
  @Deprecated(since="5.1")
  public void foo2() {
  }

  /**
    * Java >= 9
    * @deprecated (when, why, refactoring advice...)
    */
  @Deprecated(since="4.2", forRemoval=true)
  public void foo3() {
  }

}

Exceptions

The members and methods of a deprecated class or interface are ignored by this rule. The classes and interfaces themselves are still subject to it.

/**
 * @deprecated (when, why, etc...)
 */
@Deprecated
class Qix  {

  public void foo() {} // Compliant; class is deprecated

}

/**
 * @deprecated (when, why, etc...)
 */
@Deprecated
interface Plop {

  void bar();

}

Programmers should not comment out code as it bloats programs and reduces readability.

Unused code should be deleted and can be retrieved from source control history if required.

Duplicated string literals make the process of refactoring error-prone, since you must be sure to update all occurrences.

On the other hand, constants can be referenced from many places, but only need to be updated in a single place.

Noncompliant Code Example

With the default threshold of 3:

public void run() {
  prepare("action1");                              // Noncompliant - "action1" is duplicated 3 times
  execute("action1");
  release("action1");
}

@SuppressWarning("all")                            // Compliant - annotations are excluded
private void method1() { /* ... */ }
@SuppressWarning("all")
private void method2() { /* ... */ }

public String method3(String a) {
  System.out.println("'" + a + "'");               // Compliant - literal "'" has less than 5 characters and is excluded
  return "";                                       // Compliant - literal "" has less than 5 characters and is excluded
}

Compliant Solution

private static final String ACTION_1 = "action1";  // Compliant

public void run() {
  prepare(ACTION_1);                               // Compliant
  execute(ACTION_1);
  release(ACTION_1);
}

Exceptions

To prevent generating some false-positives, literals having less than 5 characters are excluded.

There are several reasons for a method not to have a method body:

• It is an unintentional omission, and should be fixed to prevent an unexpected behavior in production.
• It is not yet, or never will be, supported. In this case an UnsupportedOperationException should be thrown.
• The method is an intentionally-blank override. In this case a nested comment should explain the reason for the blank override.

Noncompliant Code Example

public void doSomething() {
}

public void doSomethingElse() {
}

Compliant Solution

@Override
public void doSomething() {
  // Do nothing because of X and Y.
}

@Override
public void doSomethingElse() {
  throw new UnsupportedOperationException();
}

Exceptions

Default (no-argument) constructors are ignored when there are other constructors in the class, as are empty methods in abstract classes.

public abstract class Animal {
  void speak() {  // default implementation ignored
  }
}

There are several reasons for a method not to have a method body:

• It is an unintentional omission, and should be fixed to prevent an unexpected behavior in production.
• It is not yet, or never will be, supported. In this case an UnsupportedOperationException should be thrown.
• The method is an intentionally-blank override. In this case a nested comment should explain the reason for the blank override.

Noncompliant Code Example

public void doSomething() {
}

public void doSomethingElse() {
}

Compliant Solution

@Override
public void doSomething() {
  // Do nothing because of X and Y.
}

@Override
public void doSomethingElse() {
  throw new UnsupportedOperationException();
}

Exceptions

Default (no-argument) constructors are ignored when there are other constructors in the class, as are empty methods in abstract classes.

public abstract class Animal {
  void speak() {  // default implementation ignored
  }
}

Unused parameters are misleading. Whatever the values passed to such parameters, the behavior will be the same.

Noncompliant Code Example

void doSomething(int a, int b) {     // "b" is unused
  compute(a);
}

Compliant Solution

void doSomething(int a) {
  compute(a);
}

Exceptions

The rule will not raise issues for unused parameters:

• that are annotated with @javax.enterprise.event.Observes
• in overrides and implementation methods
• in interface default methods
• in non-private methods that only throw or that have empty bodies
• in annotated methods, unless the annotation is @SuppressWarning("unchecked") or @SuppressWarning("rawtypes"), in which case the annotation will be ignored
• in overridable methods (non-final, or not member of a final class, non-static, non-private), if the parameter is documented with a proper javadoc.

@Override
void doSomething(int a, int b) {     // no issue reported on b
  compute(a);
}

public void foo(String s) {
  // designed to be extended but noop in standard case
}

protected void bar(String s) {
  //open-closed principle
}

public void qix(String s) {
  throw new UnsupportedOperationException("This method should be implemented in subclasses");
}

/**
 * @param s This string may be use for further computation in overriding classes
 */
protected void foobar(int a, String s) { // no issue, method is overridable and unused parameter has proper javadoc
  compute(a);
}

See

• CERT, MSC12-C. - Detect and remove code that has no effect or is never executed

Programmers should not comment out code as it bloats programs and reduces readability.

Unused code should be deleted and can be retrieved from source control history if required.

private methods that are never executed are dead code: unnecessary, inoperative code that should be removed. Cleaning out dead code decreases the size of the maintained codebase, making it easier to understand the program and preventing bugs from being introduced.

Note that this rule does not take reflection into account, which means that issues will be raised on private methods that are only accessed using the reflection API.

Noncompliant Code Example

public class Foo implements Serializable
{
  private Foo(){}     //Compliant, private empty constructor intentionally used to prevent any direct instantiation of a class.
  public static void doSomething(){
    Foo foo = new Foo();
    ...
  }
  private void unusedPrivateMethod(){...}
  private void writeObject(ObjectOutputStream s){...}  //Compliant, relates to the java serialization mechanism
  private void readObject(ObjectInputStream in){...}  //Compliant, relates to the java serialization mechanism
}

Compliant Solution

public class Foo implements Serializable
{
  private Foo(){}     //Compliant, private empty constructor intentionally used to prevent any direct instantiation of a class.
  public static void doSomething(){
    Foo foo = new Foo();
    ...
  }

  private void writeObject(ObjectOutputStream s){...}  //Compliant, relates to the java serialization mechanism

  private void readObject(ObjectInputStream in){...}  //Compliant, relates to the java serialization mechanism
}

Exceptions

This rule doesn't raise any issue on annotated methods.

private methods that are never executed are dead code: unnecessary, inoperative code that should be removed. Cleaning out dead code decreases the size of the maintained codebase, making it easier to understand the program and preventing bugs from being introduced.

Note that this rule does not take reflection into account, which means that issues will be raised on private methods that are only accessed using the reflection API.

Noncompliant Code Example

public class Foo implements Serializable
{
  private Foo(){}     //Compliant, private empty constructor intentionally used to prevent any direct instantiation of a class.
  public static void doSomething(){
    Foo foo = new Foo();
    ...
  }
  private void unusedPrivateMethod(){...}
  private void writeObject(ObjectOutputStream s){...}  //Compliant, relates to the java serialization mechanism
  private void readObject(ObjectInputStream in){...}  //Compliant, relates to the java serialization mechanism
}

Compliant Solution

public class Foo implements Serializable
{
  private Foo(){}     //Compliant, private empty constructor intentionally used to prevent any direct instantiation of a class.
  public static void doSomething(){
    Foo foo = new Foo();
    ...
  }

  private void writeObject(ObjectOutputStream s){...}  //Compliant, relates to the java serialization mechanism

  private void readObject(ObjectInputStream in){...}  //Compliant, relates to the java serialization mechanism
}

Exceptions

This rule doesn't raise any issue on annotated methods.

Duplicated string literals make the process of refactoring error-prone, since you must be sure to update all occurrences.

On the other hand, constants can be referenced from many places, but only need to be updated in a single place.

Noncompliant Code Example

With the default threshold of 3:

public void run() {
  prepare("action1");                              // Noncompliant - "action1" is duplicated 3 times
  execute("action1");
  release("action1");
}

@SuppressWarning("all")                            // Compliant - annotations are excluded
private void method1() { /* ... */ }
@SuppressWarning("all")
private void method2() { /* ... */ }

public String method3(String a) {
  System.out.println("'" + a + "'");               // Compliant - literal "'" has less than 5 characters and is excluded
  return "";                                       // Compliant - literal "" has less than 5 characters and is excluded
}

Compliant Solution

private static final String ACTION_1 = "action1";  // Compliant

public void run() {
  prepare(ACTION_1);                               // Compliant
  execute(ACTION_1);
  release(ACTION_1);
}

Exceptions

To prevent generating some false-positives, literals having less than 5 characters are excluded.