Showing 716 of 716 total issues
Refactor this method to reduce its Cognitive Complexity from 22 to the 15 allowed. Open
private List<String> importExternalTestSet(List<AlmTestSet> testsets, String tester, int testsetFolderId, int testFolderId, String testingTool, String subversion, String jobName, String buildUrl ) throws ExternalEntityUploadException{
- 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
Refactor this method to reduce its Cognitive Complexity from 19 to the 15 allowed. Open
public synchronized void getUpdatedData() {
- 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
This block of commented-out lines of code should be removed. Open
// public Collection<? extends Action> getProjectActions() {
- Read upRead up
- Exclude checks
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.
Add a nested comment explaining why this method is empty, throw an UnsupportedOperationException or complete the implementation. Open
public ObjectFactory() {
- Read upRead up
- Exclude checks
There are several reasons for a method not to have a method body:
- It is an unintentional omission, and should be fixed to prevent an unexpected behavior in production.
- 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.
Noncompliant Code Example
public void doSomething() { } public void doSomethingElse() { }
Compliant Solution
@Override public void doSomething() { // Do nothing because of X and Y. } @Override public void doSomethingElse() { throw new UnsupportedOperationException(); }
Exceptions
Default (no-argument) constructors are ignored when there are other constructors in the class, as are empty methods in abstract classes.
public abstract class Animal { void speak() { // default implementation ignored } }
Disable access to external entities in XML parsing. Open
TransformerFactory transformerFactory = TransformerFactory.newInstance();
- Read upRead up
- Exclude checks
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
- OWASP Top 10 2017 Category A4 - XML External Entities (XXE)
- OWASP XXE Prevention Cheat Sheet
- MITRE, CWE-611 - Information Exposure Through XML External Entity Reference
- MITRE, CWE-827 - Improper Control of Document Type Definition
Disable access to external entities in XML parsing. Open
DocumentBuilder dBuilder = dbFactory.newDocumentBuilder();
- Read upRead up
- Exclude checks
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
- OWASP Top 10 2017 Category A4 - XML External Entities (XXE)
- OWASP XXE Prevention Cheat Sheet
- MITRE, CWE-611 - Information Exposure Through XML External Entity Reference
- MITRE, CWE-827 - Improper Control of Document Type Definition
Add a private constructor to hide the implicit public one. Open
public class Base64Encoder {
- Read upRead up
- Exclude checks
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.
Remove this unused method parameter "postRunAction". Open
String postRunAction,
- Read upRead up
- Exclude checks
Unused parameters are misleading. Whatever the values passed to such parameters, the behavior will be the same.
Noncompliant Code Example
void doSomething(int a, int b) { // "b" is unused compute(a); }
Compliant Solution
void doSomething(int a) { compute(a); }
Exceptions
The rule will not raise issues for unused parameters:
- that are annotated with
@javax.enterprise.event.Observes
- in overrides and implementation methods
- in interface
default
methods - in non-private methods that only
throw
or that have empty bodies - in annotated methods, unless the annotation is
@SuppressWarning("unchecked")
or@SuppressWarning("rawtypes")
, in which case the annotation will be ignored - in overridable methods (non-final, or not member of a final class, non-static, non-private), if the parameter is documented with a proper javadoc.
@Override void doSomething(int a, int b) { // no issue reported on b compute(a); } public void foo(String s) { // designed to be extended but noop in standard case } protected void bar(String s) { //open-closed principle } public void qix(String s) { throw new UnsupportedOperationException("This method should be implemented in subclasses"); } /** * @param s This string may be use for further computation in overriding classes */ protected void foobar(int a, String s) { // no issue, method is overridable and unused parameter has proper javadoc compute(a); }
See
- CERT, MSC12-C. - Detect and remove code that has no effect or is never executed
Catch Exception instead of Throwable. Open
} catch (Throwable cause) {
- Read upRead up
- Exclude checks
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
- MITRE, CWE-396 - Declaration of Catch for Generic Exception
- C++ Core Guidelines E.14 - Use purpose-designed user-defined types as exceptions (not built-in types)
Refactor this method to reduce its Cognitive Complexity from 43 to the 15 allowed. Open
public void doNotify(StaplerRequest req, StaplerResponse res) throws IOException {
- 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
Extract this nested try block into a separate method. Open
try {
- Read upRead up
- Exclude checks
Nesting try
/catch
blocks severely impacts the readability of source code because it makes it too difficult to understand
which block will catch which exception.
Rename this constant name to match the regular expression '^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$'. Open
AverageThroughput, TotalThroughput, AverageHitsPerSecond, TotalHits,
- Read upRead up
- Exclude checks
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; }
Add a private constructor to hide the implicit public one. Open
public class UftConstants {
- Read upRead up
- Exclude checks
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.
Add a nested comment explaining why this method is empty, throw an UnsupportedOperationException or complete the implementation. Open
public AvgTransactionResponseTime() {
- Read upRead up
- Exclude checks
There are several reasons for a method not to have a method body:
- It is an unintentional omission, and should be fixed to prevent an unexpected behavior in production.
- 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.
Noncompliant Code Example
public void doSomething() { } public void doSomethingElse() { }
Compliant Solution
@Override public void doSomething() { // Do nothing because of X and Y. } @Override public void doSomethingElse() { throw new UnsupportedOperationException(); }
Exceptions
Default (no-argument) constructors are ignored when there are other constructors in the class, as are empty methods in abstract classes.
public abstract class Animal { void speak() { // default implementation ignored } }
Catch Exception instead of Throwable. Open
} catch (Throwable e) {
- Read upRead up
- Exclude checks
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
- MITRE, CWE-396 - Declaration of Catch for Generic Exception
- C++ Core Guidelines E.14 - Use purpose-designed user-defined types as exceptions (not built-in types)
Remove this unused "projectActionList" private field. Open
private final List<TestResultProjectAction> projectActionList;
- Read upRead up
- Exclude checks
If a private
field is declared but not used in the program, it can be considered dead code and should therefore be removed. This will
improve maintainability because developers will not wonder what the variable is used for.
Note that this rule does not take reflection into account, which means that issues will be raised on private
fields that are only
accessed using the reflection API.
Noncompliant Code Example
public class MyClass { private int foo = 42; public int compute(int a) { return a * 42; } }
Compliant Solution
public class MyClass { public int compute(int a) { return a * 42; } }
Exceptions
The Java serialization runtime associates with each serializable class a version number, called serialVersionUID
, which is used during
deserialization to verify that the sender and receiver of a serialized object have loaded classes for that object that are compatible with respect to
serialization.
A serializable class can declare its own serialVersionUID
explicitly by declaring a field named serialVersionUID
that
must be static, final, and of type long. By definition those serialVersionUID
fields should not be reported by this rule:
public class MyClass implements java.io.Serializable { private static final long serialVersionUID = 42L; }
Moreover, this rule doesn't raise any issue on annotated fields.
Replace this "Map.containsKey()" with a call to "Map.computeIfAbsent()". Open
if (!projectTransactionsData.containsKey(transactionName)) {
- Read upRead up
- Exclude checks
It's a common pattern to test the result of a java.util.Map.get()
against null
or calling
java.util.Map.containsKey()
before proceeding with adding or changing the value in the map. However the java.util.Map
API
offers a significantly better alternative in the form of the computeIfPresent()
and computeIfAbsent()
methods. Using these
instead leads to cleaner and more readable code.
Note that this rule is automatically disabled when the project's sonar.java.source
is not 8.
Noncompliant Code Example
V value = map.get(key); if (value == null) { // Noncompliant value = V.createFor(key); if (value != null) { map.put(key, value); } } if (!map.containsKey(key)) { // Noncompliant value = V.createFor(key); if (value != null) { map.put(key, value); } } return value;
Compliant Solution
return map.computeIfAbsent(key, k -> V.createFor(k));
Exceptions
This rule will not raise an issue when trying to add null
to a map, because computeIfAbsent
will not add the entry if the
value returned by the function is null
.
See also
- {rule:java:S6104} - Map "computeIfAbsent()" and "computeIfPresent()" should not be used to add "null" values.
Replace this "Map.containsKey()" with a call to "Map.computeIfAbsent()". Open
if (!vUserPerRun.containsKey(runNumber)) {
- Read upRead up
- Exclude checks
It's a common pattern to test the result of a java.util.Map.get()
against null
or calling
java.util.Map.containsKey()
before proceeding with adding or changing the value in the map. However the java.util.Map
API
offers a significantly better alternative in the form of the computeIfPresent()
and computeIfAbsent()
methods. Using these
instead leads to cleaner and more readable code.
Note that this rule is automatically disabled when the project's sonar.java.source
is not 8.
Noncompliant Code Example
V value = map.get(key); if (value == null) { // Noncompliant value = V.createFor(key); if (value != null) { map.put(key, value); } } if (!map.containsKey(key)) { // Noncompliant value = V.createFor(key); if (value != null) { map.put(key, value); } } return value;
Compliant Solution
return map.computeIfAbsent(key, k -> V.createFor(k));
Exceptions
This rule will not raise an issue when trying to add null
to a map, because computeIfAbsent
will not add the entry if the
value returned by the function is null
.
See also
- {rule:java:S6104} - Map "computeIfAbsent()" and "computeIfPresent()" should not be used to add "null" values.
Provide the parametrized type for this generic. Open
List<String> conditions = new LinkedList();
- Read upRead up
- Exclude checks
Generic types shouldn't be used raw (without type parameters) in variable declarations or return values. Doing so bypasses generic type checking, and defers the catch of unsafe code to runtime.
Noncompliant Code Example
List myList; // Noncompliant Set mySet; // Noncompliant
Compliant Solution
List<String> myList; Set<? extends Number> mySet;
Either remove or fill this block of code. Open
}catch (Exception e) {
- Read upRead up
- Exclude checks
Most of the time a block of code is empty when a piece of code is really missing. So such empty block must be either filled or removed.
Noncompliant Code Example
for (int i = 0; i < 42; i++){} // Empty on purpose or missing piece of code ?
Exceptions
When a block contains a comment, this block is not considered to be empty unless it is a synchronized
block. synchronized
blocks are still considered empty even with comments because they can still affect program flow.