Refactor this method to reduce its Cognitive Complexity from 16 to the 15 allowed. Open
public JobResult process(Job job) {
- Read upRead up
- Exclude checks
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
Change this condition so that it does not always evaluate to "false" Open
if (packagesRemoved > 0) {
- Read upRead up
- Exclude checks
Conditional expressions which are always true
or false
can lead to dead code. Such code is always buggy and should never
be used in production.
Noncompliant Code Example
a = false; if (a) { // Noncompliant doSomething(); // never executed } if (!a || b) { // Noncompliant; "!a" is always "true", "b" is never evaluated doSomething(); } else { doSomethingElse(); // never executed }
Exceptions
This rule will not raise an issue in either of these cases:
- When the condition is a single
final boolean
final boolean debug = false; //... if (debug) { // Print something }
- When the condition is literally
true
orfalse
.
if (true) { // do something }
In these cases it is obvious the code is as intended.
See
- MITRE, CWE-570 - Expression is Always False
- MITRE, CWE-571 - Expression is Always True
- CERT, MSC12-C. - Detect and remove code that has no effect or is never executed
"NullPointerException" will be thrown when invoking method "getPackageDescription()". Open
String packageDescription = getPackageDescription(definition);
- Read upRead up
- Exclude checks
A reference to null
should never be dereferenced/accessed. Doing so will cause a NullPointerException
to be thrown. At
best, such an exception will cause abrupt program termination. At worst, it could expose debugging information that would be useful to an attacker, or
it could allow an attacker to bypass security measures.
Note that when they are present, this rule takes advantage of @CheckForNull
and @Nonnull
annotations defined in JSR-305 to understand which values are and are not nullable except when @Nonnull
is used
on the parameter to equals
, which by contract should always work with null.
Noncompliant Code Example
@CheckForNull String getName(){...} public boolean isNameEmpty() { return getName().length() == 0; // Noncompliant; the result of getName() could be null, but isn't null-checked }
Connection conn = null; Statement stmt = null; try{ conn = DriverManager.getConnection(DB_URL,USER,PASS); stmt = conn.createStatement(); // ... }catch(Exception e){ e.printStackTrace(); }finally{ stmt.close(); // Noncompliant; stmt could be null if an exception was thrown in the try{} block conn.close(); // Noncompliant; conn could be null if an exception was thrown }
private void merge(@Nonnull Color firstColor, @Nonnull Color secondColor){...} public void append(@CheckForNull Color color) { merge(currentColor, color); // Noncompliant; color should be null-checked because merge(...) doesn't accept nullable parameters }
void paint(Color color) { if(color == null) { System.out.println("Unable to apply color " + color.toString()); // Noncompliant; NullPointerException will be thrown return; } ... }
See
- MITRE, CWE-476 - NULL Pointer Dereference
- CERT, EXP34-C. - Do not dereference null pointers
- CERT, EXP01-J. - Do not use a null in a case where an object is required
Make this method "synchronized" to match the parent class implementation. Open
public RepositoryException getCause() {
- Read upRead up
- Exclude checks
When @Overrides
of synchronized
methods are not themselves synchronized
, the result can be improper
synchronization as callers rely on the thread-safety promised by the parent class.
Noncompliant Code Example
public class Parent { synchronized void foo() { //... } } public class Child extends Parent { @Override public void foo () { // Noncompliant // ... super.foo(); } }
Compliant Solution
public class Parent { synchronized void foo() { //... } } public class Child extends Parent { @Override synchronized void foo () { // ... super.foo(); } }
See
- CERT, TSM00-J - Do not override thread-safe methods with methods that are not thread-safe