Restricting the number of break and continue statements in a loop is done in the interest of good structured programming.

One break or continue statement is acceptable in a loop, since this allows optimal coding. In most cases, they can either be combined together, or merged with the loop's condition.

The following code:

for (int i = 1; i <= 10; i++) {     // Non-Compliant - 2 continue - one might be tempted to add some logic in between
  if (i % 2 == 0) {
    continue;
  }

  if (i % 3 == 0) {
    continue;
  }

  System.out.println("i = " + i);
}

should be refactored into:

for (int i = 1; i <= 10; i++) {     // Compliant
  if (i % 2 == 0 || i % 3 == 0) {
    continue;
  }

  System.out.println("i = " + i);
}

Avoid using implementation types (i.e., HashSet); use the interface (i.e, Set) instead

This rule is deprecated, use squid:S1319 instead.

A final static field references an array and can be accessed by malicious code or by accident from another package. This code can freely modify the contents of the array.

A final static field references a Hashtable and can be accessed by malicious code or by accident from another package. This code can freely modify the contents of the Hashtable.

A mutable static field could be changed by malicious code or by accident from another package. Unfortunately, the way the field is used doesn't allow any easy fix to this problem.

A mutable static field could be changed by malicious code or by accident from another package. The field could be made final to avoid this vulnerability.

A mutable static field could be changed by malicious code or by accident from another package. The field could be made package protected and/or made final to avoid this vulnerability.

A final static field that is defined in an interface references a mutable object such as an array or hashtable. This mutable object could be changed by malicious code or by accident from another package. To solve this, the field needs to be moved to a class and made package protected to avoid this vulnerability.

A mutable static field could be changed by malicious code or by accident. The field could be made package protected to avoid this vulnerability.

A class's finalize() method should have protected access, not public.

This code stores a reference to an externally mutable object into the internal representation of the object.  If instances are accessed by untrusted code, and unchecked changes to the mutable object would compromise security or other important properties, you will need to do something different. Storing a copy of the object is better approach in many situations.

Returning a reference to a mutable object value stored in one of the object's fields exposes the internal representation of the object.  If instances are accessed by untrusted code, and unchecked changes to the mutable object would compromise security or other important properties, you will need to do something different. Returning a new copy of the object is better approach in many situations.

This code stores a reference to an externally mutable object into a static field. If unchecked changes to the mutable object would compromise security or other important properties, you will need to do something different. Storing a copy of the object is better approach in many situations.

A public static method returns a reference to an array that is part of the static state of the class. Any code that calls this method can freely modify the underlying array. One fix is to return a copy of the array.

This code calls a method and ignores the return value. The return value is the same type as the type the method is invoked on, and from our analysis it looks like the return value might be important (e.g., like ignoring the return value of String.toLowerCase()).

We are guessing that ignoring the return value might be a bad idea just from a simple analysis of the body of the method. You can use a @CheckReturnValue annotation to instruct FindBugs as to whether ignoring the return value of this method is important or acceptable.

Please investigate this closely to decide whether it is OK to ignore the return value.

This method may fail to clean up (close, dispose of) a stream, database object, or other resource requiring an explicit cleanup operation.
In general, if a method opens a stream or other resource, the method should use a try/finally block to ensure that the stream or resource is cleaned up before the method returns.

This bug pattern is essentially the same as the OS_OPEN_STREAM and ODR_OPEN_DATABASE_RESOURCE bug patterns, but is based on a different (and hopefully better) static analysis technique. See Weimer and Necula, Finding and Preventing Run-Time Error Handling Mistakes, for a description of the analysis technique. .

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 function/method names match a provided regular expression.

The following code snippet illustrates this rule when the regular expression value is "^[a-z][a-zA-Z0-9]*$":

public class MyClass {
  private int myProperty;

  public int getMyProperty() { // Compliant
    return myProperty;
  }

  public void SetMyProperty(int value) { // Non-Compliant
    myProperty = value;
  }
}

"equals" as a method name should be exclusively used to override Object.equals(Object) to prevent any confusion.

It is tempting to overload the method to take a specific class instead of Object as parameter, to save class comparison check. However, this will not work as expected.

For example:

class MyClass {
  private int foo = 1;

  public boolean equals(MyClass o) {                    // Non-Compliant - "equals" method which does not override Object.equals(Object)
    return o != null && o.foo == this.foo;
  }

  public static void main(String[] args) {
    MyClass o1 = new MyClass();
    Object o2 = new MyClass();
    System.out.println(o1.equals(o2));                  // Will display "false" because "o2" is of type "Object" and not "MyClass"
  }
}

should be refactored into:

class MyClass {
  private int foo = 1;

  @Override
  public boolean equals(Object o) {                     // Compliant - overrides Object.equals(Object)
    if (o == null || !(o instanceof MyClass)) {
      return false;
    }

    MyClass other = (MyClass)o;
    return this.foo == other.foo;
  }

  /* ... */
}

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

• It is an unintentional omission, and should be fixed.
• 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.

The following code snippet:

// Non-Compliant
public void doSomething() {
}

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

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

Empty methods not having any nested comments are tolerated in Abstract classes as those empty methods are usual when implementing the visitor pattern.

Naming a method hashcode() is either:

• A bug. Overriding Object.hashCode() was meant and the application does not behave as expected.
• Done on purpose. The name however will confuse every other developer, who will think it is a bug.

In both cases, the method should be renamed.

The following code:

public int hashcode() { /* ... */ }    // Non-Compliant

should be refactored into:

@Override
public int hashCode() { /* ... */ }    // Compliant

The Cyclomatic Complexity is measured by the number of (&&, ||) operators and (if, while, do, for, ?:, catch, switch, case, return, throw) statements in the body of a class plus one for each constructor, method (but not getter/setter), static initializer, or instance initializer in the class. The last return stament in method, if exists, is not taken into account.

Even when the Cyclomatic Complexity of a class is very high, this complexity might be well distributed among all methods. Nevertheless, most of the time, a very complex class is a class which breaks the Single Responsibility Principle and which should be re-factored to be split in several classes.

A long parameter list can indicate that a new structure should be created to wrap the numerous parameters or that the method is doing too many things.

A class that has private constructors and does not have any static methods or fields cannot be used.

The Java Language Specification recommends listing modifiers in the following order:

• Annotations
• public
• protected
• private
• abstract
• static
• final
• transient
• volatile
• synchronized
• native
• strictfp

Not following this convention has no technical impact, but will reduce the code's readability because most developers are used to the standard order.

The following code:

static public void main(String[] args) {   // Non-Compliant
}

should be refactored into:

public static void main(String[] args) {   // Compliant
}

This method creates a thread without specifying a run method either by deriving from the Thread class, or by passing a Runnable object. This thread, then, does nothing but waste time.