When logging a message there are several important requirements which must be fulfilled:
The user must be able to easily retrieve the logs
The format of all logged message must be uniform to allow the user to easily read the log
Logged data must actually be recorded
Sensitive data must only be logged securely
If a program directly writes to the standard outputs, there is absolutely no way to comply with those requirements. That's why defining and using a
dedicated logger is highly recommended.
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
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.
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.
Using such generic exceptions as Error, RuntimeException, Throwable, and Exception prevents
calling methods from handling true, system-generated exceptions differently than application-generated errors.
Noncompliant Code Example
public void foo(String bar) throws Throwable { // Noncompliant
throw new RuntimeException("My Message"); // Noncompliant
}
Compliant Solution
public void foo(String bar) {
throw new MyOwnRuntimeException("My Message");
}
Exceptions
Generic exceptions in the signatures of overriding methods are ignored, because overriding method has to follow signature of the throw declaration
in the superclass. The issue will be raised on superclass declaration of the method (or won't be raised at all if superclass is not part of the
analysis).
@Override
public void myMethod() throws Exception {...}
Generic exceptions are also ignored in the signatures of methods that make calls to methods that throw generic exceptions.
public void myOtherMethod throws Exception {
doTheThing(); // this method throws Exception
}
See
MITRE, CWE-397 - Declaration of Throws for Generic Exception
CERT, ERR07-J. - Do not throw RuntimeException, Exception, or Throwable
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
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.
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
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
When logging a message there are several important requirements which must be fulfilled:
The user must be able to easily retrieve the logs
The format of all logged message must be uniform to allow the user to easily read the log
Logged data must actually be recorded
Sensitive data must only be logged securely
If a program directly writes to the standard outputs, there is absolutely no way to comply with those requirements. That's why defining and using a
dedicated logger is highly recommended.
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.
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
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
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.
public String getReadableStatus(Job j) {
if (j.isRunning()) {
return "Running";
}
return j.hasErrors() ? "Failed" : "Succeeded";
}
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.
public String getReadableStatus(Job j) {
if (j.isRunning()) {
return "Running";
}
return j.hasErrors() ? "Failed" : "Succeeded";
}
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.
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
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.
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