jenkinsci/hpe-application-automation-tools-plugin

Just because you can do something, doesn't mean you should, and that's the case with nested ternary operations. Nesting ternary operators results in the kind of code that may seem clear as day when you write it, but six months later will leave maintainers (or worse - future you) scratching their heads and cursing.

Instead, err on the side of clarity, and use another line to express the nested operation as a separate statement.

Noncompliant Code Example

public String getReadableStatus(Job j) {
  return j.isRunning() ? "Running" : j.hasErrors() ? "Failed" : "Succeeded";  // Noncompliant
}

Compliant Solution

public String getReadableStatus(Job j) {
  if (j.isRunning()) {
    return "Running";
  }
  return j.hasErrors() ? "Failed" : "Succeeded";
}

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

Shared coding conventions allow teams to collaborate efficiently. This rule checks that all constant names match a provided regular expression.

Noncompliant Code Example

With the default regular expression ^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$:

public class MyClass {
  public static final int first = 1;
}

public enum MyEnum {
  first;
}

Compliant Solution

public class MyClass {
  public static final int FIRST = 1;
}

public enum MyEnum {
  FIRST;
}

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.

Because printf-style format strings are interpreted at runtime, rather than validated by the compiler, they can contain errors that result in the wrong strings being created. This rule statically validates the correlation of printf-style format strings to their arguments when calling the format(...) methods of java.util.Formatter, java.lang.String, java.io.PrintStream, MessageFormat, and java.io.PrintWriter classes and the printf(...) methods of java.io.PrintStream or java.io.PrintWriter classes.

Noncompliant Code Example

String.format("First {0} and then {1}", "foo", "bar");  //Noncompliant. Looks like there is a confusion with the use of {{java.text.MessageFormat}}, parameters "foo" and "bar" will be simply ignored here
String.format("Display %3$d and then %d", 1, 2, 3);   //Noncompliant; the second argument '2' is unused
String.format("Too many arguments %d and %d", 1, 2, 3);  //Noncompliant; the third argument '3' is unused
String.format("First Line\n");   //Noncompliant; %n should be used in place of \n to produce the platform-specific line separator
String.format("Is myObject null ? %b", myObject);   //Noncompliant; when a non-boolean argument is formatted with %b, it prints true for any nonnull value, and false for null. Even if intended, this is misleading. It's better to directly inject the boolean value (myObject == null in this case)
String.format("value is " + value); // Noncompliant
String s = String.format("string without arguments"); // Noncompliant

MessageFormat.format("Result '{0}'.", value); // Noncompliant; String contains no format specifiers. (quote are discarding format specifiers)
MessageFormat.format("Result {0}.", value, value);  // Noncompliant; 2nd argument is not used
MessageFormat.format("Result {0}.", myObject.toString()); // Noncompliant; no need to call toString() on objects

java.util.Logger logger;
logger.log(java.util.logging.Level.SEVERE, "Result {0}.", myObject.toString()); // Noncompliant; no need to call toString() on objects
logger.log(java.util.logging.Level.SEVERE, "Result.", new Exception()); // compliant, parameter is an exception
logger.log(java.util.logging.Level.SEVERE, "Result '{0}'", 14); // Noncompliant - String contains no format specifiers.
logger.log(java.util.logging.Level.SEVERE, "Result " + param, exception); // Noncompliant; Lambda should be used to differ string concatenation.

org.slf4j.Logger slf4jLog;
org.slf4j.Marker marker;

slf4jLog.debug(marker, "message {}");
slf4jLog.debug(marker, "message", 1); // Noncompliant - String contains no format specifiers.

org.apache.logging.log4j.Logger log4jLog;
log4jLog.debug("message", 1); // Noncompliant - String contains no format specifiers.

Compliant Solution

String.format("First %s and then %s", "foo", "bar");
String.format("Display %2$d and then %d", 1, 3);
String.format("Too many arguments %d %d", 1, 2);
String.format("First Line%n");
String.format("Is myObject null ? %b", myObject == null);
String.format("value is %d", value);
String s = "string without arguments";

MessageFormat.format("Result {0}.", value);
MessageFormat.format("Result '{0}'  =  {0}", value);
MessageFormat.format("Result {0}.", myObject);

java.util.Logger logger;
logger.log(java.util.logging.Level.SEVERE, "Result {0}.", myObject);
logger.log(java.util.logging.Level.SEVERE, "Result {0}'", 14);
logger.log(java.util.logging.Level.SEVERE, exception, () -> "Result " + param);

org.slf4j.Logger slf4jLog;
org.slf4j.Marker marker;

slf4jLog.debug(marker, "message {}");
slf4jLog.debug(marker, "message {}", 1);

org.apache.logging.log4j.Logger log4jLog;
log4jLog.debug("message {}", 1);

See

• CERT, FIO47-C. - Use valid format strings

Cognitive Complexity is a measure of how hard the control flow of a method is to understand. Methods with high Cognitive Complexity will be difficult to maintain.

See

• Cognitive Complexity

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

Noncompliant Code Example

With a maximum number of 4 parameters:

public void doSomething(int param1, int param2, int param3, String param4, long param5) {
...
}

Compliant Solution

public void doSomething(int param1, int param2, int param3, String param4) {
...
}

Exceptions

Methods annotated with :

• Spring's @RequestMapping (and related shortcut annotations, like @GetRequest)
• JAX-RS API annotations (like @javax.ws.rs.GET)
• Bean constructor injection with @org.springframework.beans.factory.annotation.Autowired
• CDI constructor injection with @javax.inject.Inject
• @com.fasterxml.jackson.annotation.JsonCreator

may have a lot of parameters, encapsulation being possible. Such methods are therefore ignored.

Correctly updating a static field from a non-static method is tricky to get right and could easily lead to bugs if there are multiple class instances and/or multiple threads in play. Ideally, static fields are only updated from synchronized static methods.

This rule raises an issue each time a static field is updated from a non-static method.

Noncompliant Code Example

public class MyClass {

  private static int count = 0;

  public void doSomething() {
    //...
    count++;  // Noncompliant
  }
}

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

Noncompliant Code Example

With a maximum number of 4 parameters:

public void doSomething(int param1, int param2, int param3, String param4, long param5) {
...
}

Compliant Solution

public void doSomething(int param1, int param2, int param3, String param4) {
...
}

Exceptions

Methods annotated with :

• Spring's @RequestMapping (and related shortcut annotations, like @GetRequest)
• JAX-RS API annotations (like @javax.ws.rs.GET)
• Bean constructor injection with @org.springframework.beans.factory.annotation.Autowired
• CDI constructor injection with @javax.inject.Inject
• @com.fasterxml.jackson.annotation.JsonCreator

may have a lot of parameters, encapsulation being possible. Such methods are therefore ignored.

InterruptedExceptions should never be ignored in the code, and simply logging the exception counts in this case as "ignoring". The throwing of the InterruptedException clears the interrupted state of the Thread, so if the exception is not handled properly the fact that the thread was interrupted will be lost. Instead, InterruptedExceptions should either be rethrown - immediately or after cleaning up the method's state - or the thread should be re-interrupted by calling Thread.interrupt() even if this is supposed to be a single-threaded application. Any other course of action risks delaying thread shutdown and loses the information that the thread was interrupted - probably without finishing its task.

Similarly, the ThreadDeath exception should also be propagated. According to its JavaDoc:

If ThreadDeath is caught by a method, it is important that it be rethrown so that the thread actually dies.

Noncompliant Code Example

public void run () {
  try {
    while (true) {
      // do stuff
    }
  }catch (InterruptedException e) { // Noncompliant; logging is not enough
    LOGGER.log(Level.WARN, "Interrupted!", e);
  }
}

Compliant Solution

public void run () {
  try {
    while (true) {
      // do stuff
    }
  }catch (InterruptedException e) {
    LOGGER.log(Level.WARN, "Interrupted!", e);
    // Restore interrupted state...
    Thread.currentThread().interrupt();
  }
}

See

• MITRE, CWE-391 - Unchecked Error Condition
• Dealing with InterruptedException

Cognitive Complexity is a measure of how hard the control flow of a method is to understand. Methods with high Cognitive Complexity will be difficult to maintain.

See

• Cognitive Complexity

Cognitive Complexity is a measure of how hard the control flow of a method is to understand. Methods with high Cognitive Complexity will be difficult to maintain.

See

• Cognitive Complexity

The underlying implementation of String::replaceAll calls the java.util.regex.Pattern.compile() method each time it is called even if the first argument is not a regular expression. This has a significant performance cost and therefore should be used with care.

When String::replaceAll is used, the first argument should be a real regular expression. If it’s not the case, String::replace does exactly the same thing as String::replaceAll without the performance drawback of the regex.

This rule raises an issue for each String::replaceAll used with a String as first parameter which doesn’t contains special regex character or pattern.

Noncompliant Code Example

String init = "Bob is a Bird... Bob is a Plane... Bob is Superman!";
String changed = init.replaceAll("Bob is", "It's"); // Noncompliant
changed = changed.replaceAll("\\.\\.\\.", ";"); // Noncompliant

Compliant Solution

String init = "Bob is a Bird... Bob is a Plane... Bob is Superman!";
String changed = init.replace("Bob is", "It's");
changed = changed.replace("...", ";");

Or, with a regex:

String init = "Bob is a Bird... Bob is a Plane... Bob is Superman!";
String changed = init.replaceAll("\\w*\\sis", "It's");
changed = changed.replaceAll("\\.{3}", ";");

See

• {rule:java:S4248} - Regex patterns should not be created needlessly

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.

The underlying implementation of String::replaceAll calls the java.util.regex.Pattern.compile() method each time it is called even if the first argument is not a regular expression. This has a significant performance cost and therefore should be used with care.

When String::replaceAll is used, the first argument should be a real regular expression. If it’s not the case, String::replace does exactly the same thing as String::replaceAll without the performance drawback of the regex.

This rule raises an issue for each String::replaceAll used with a String as first parameter which doesn’t contains special regex character or pattern.

Noncompliant Code Example

String init = "Bob is a Bird... Bob is a Plane... Bob is Superman!";
String changed = init.replaceAll("Bob is", "It's"); // Noncompliant
changed = changed.replaceAll("\\.\\.\\.", ";"); // Noncompliant

Compliant Solution

String init = "Bob is a Bird... Bob is a Plane... Bob is Superman!";
String changed = init.replace("Bob is", "It's");
changed = changed.replace("...", ";");

Or, with a regex:

String init = "Bob is a Bird... Bob is a Plane... Bob is Superman!";
String changed = init.replaceAll("\\w*\\sis", "It's");
changed = changed.replaceAll("\\.{3}", ";");

See

• {rule:java:S4248} - Regex patterns should not be created needlessly

Cognitive Complexity is a measure of how hard the control flow of a method is to understand. Methods with high Cognitive Complexity will be difficult to maintain.

See

• Cognitive Complexity

When two methods have the same implementation, either it was a mistake - something else was intended - or the duplication was intentional, but may be confusing to maintainers. In the latter case, one implementation should invoke the other. Numerical and string literals are not taken into account.

Noncompliant Code Example

private final static String CODE = "bounteous";

public String calculateCode() {
  doTheThing();
  return CODE;
}

public String getName() {  // Noncompliant
  doTheThing();
  return CODE;
}

Compliant Solution

private final static String CODE = "bounteous";

public String getCode() {
  doTheThing();
  return CODE;
}

public String getName() {
  return getCode();
}

Exceptions

Methods that are not accessors (getters and setters), with fewer than 2 statements are ignored.

Encryption operation mode and the padding scheme should be chosen appropriately to guarantee data confidentiality, integrity and authenticity:

• For block cipher encryption algorithms (like AES):
  • The GCM (Galois Counter Mode) mode which works internally with zero/no padding scheme, is recommended, as it is designed to provide both data authenticity (integrity) and confidentiality. Other similar modes are CCM, CWC, EAX, IAPM and OCB.
  • The CBC (Cipher Block Chaining) mode by itself provides only data confidentiality, it's recommended to use it along with Message Authentication Code or similar to achieve data authenticity (integrity) too and thus to prevent padding oracle attacks.
  • The ECB (Electronic Codebook) mode doesn't provide serious message confidentiality: under a given key any given plaintext block always gets encrypted to the same ciphertext block. This mode should not be used.
• For RSA encryption algorithm, the recommended padding scheme is OAEP.

Noncompliant Code Example

Cipher c1 = Cipher.getInstance("AES"); // Noncompliant: by default ECB mode is chosen
Cipher c2 = Cipher.getInstance("AES/ECB/NoPadding"); // Noncompliant: ECB doesn't provide serious message confidentiality

Cipher c3 = Cipher.getInstance("RSA/NONE/NoPadding"); // Noncompliant: RSA without OAEP padding scheme is not recommanded

Compliant Solution

// Recommended for block ciphers
Cipher c1 = Cipher.getInstance("AES/GCM/NoPadding"); // Compliant

// Recommended for RSA
Cipher c2= Cipher.getInstance("RSA/None/OAEPWithSHA-1AndMGF1Padding"); // Compliant
Cipher c3 = Cipher.getInstance("RSA/None/OAEPWITHSHA-256ANDMGF1PADDING"); // Compliant

See

• OWASP Top 10 2017 Category A6 - Security Misconfiguration
• MITRE, CWE-327 - Use of a Broken or Risky Cryptographic Algorithm
• CERT, MSC61-J. - Do not use insecure or weak cryptographic algorithms
• SANS Top 25 - Porous Defenses

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.

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.