Showing 356 of 356 total issues
Wrong lexicographical order for 'com.fasterxml.jackson.dataformat.yaml.YAMLGenerator.Feature' import. Should be before 'org.springframework.web.servlet.config.annotation.WebMvcConfigurerAdapter'. Open
import com.fasterxml.jackson.dataformat.yaml.YAMLGenerator.Feature;
- Read upRead up
- Exclude checks
Checks that the groups of import declarations appear in the order specifiedby the user. If there is an import but its group is not specified in theconfiguration such an import should be placed at the end of the import list.
This documentation is written and maintained by the Checkstyle community and is covered under the same license as the Checkstyle project.
Wrong lexicographical order for 'com.github.damianwajser.model.details.DetailField' import. Should be before 'java.util.List'. Open
import com.github.damianwajser.model.details.DetailField;
- Read upRead up
- Exclude checks
Checks that the groups of import declarations appear in the order specifiedby the user. If there is an import but its group is not specified in theconfiguration such an import should be placed at the end of the import list.
This documentation is written and maintained by the Checkstyle community and is covered under the same license as the Checkstyle project.
Wrong lexicographical order for 'com.github.damianwajser.utils.JsonSchemmaUtils' import. Should be before 'java.util.List'. Open
import com.github.damianwajser.utils.JsonSchemmaUtils;
- Read upRead up
- Exclude checks
Checks that the groups of import declarations appear in the order specifiedby the user. If there is an import but its group is not specified in theconfiguration such an import should be placed at the end of the import list.
This documentation is written and maintained by the Checkstyle community and is covered under the same license as the Checkstyle project.
Wrong lexicographical order for 'com.fasterxml.jackson.core.JsonParser' import. Should be before 'org.springframework.web.servlet.config.annotation.WebMvcConfigurerAdapter'. Open
import com.fasterxml.jackson.core.JsonParser;
- Read upRead up
- Exclude checks
Checks that the groups of import declarations appear in the order specifiedby the user. If there is an import but its group is not specified in theconfiguration such an import should be placed at the end of the import list.
This documentation is written and maintained by the Checkstyle community and is covered under the same license as the Checkstyle project.
These nested if statements could be combined Open
if (ParameterizedType.class.isAssignableFrom(returnType.get().getClass()))
returnType = ReflectionUtils.getGenericType((ParameterizedType) returnType.get());
- Read upRead up
- Exclude checks
CollapsibleIfStatements
Since: PMD 3.1
Priority: Medium
Categories: Style
Remediation Points: 50000
Sometimes two consecutive 'if' statements can be consolidated by separating their conditions with a boolean short-circuit operator.
Example:
void bar() {
if (x) { // original implementation
if (y) {
// do stuff
}
}
}
void bar() {
if (x && y) { // optimized implementation
// do stuff
}
}
Double checked locking is not thread safe in Java. Open
public static synchronized ResourcesBuilder getInstance() {
if (instance == null) {
synchronized (ResourcesBuilder.class) {
if (instance == null) {
instance = new ResourcesBuilder();
- Read upRead up
- Exclude checks
DoubleCheckedLocking
Since: PMD 1.04
Priority: High
Categories: Style
Remediation Points: 50000
Partially created objects can be returned by the Double Checked Locking pattern when used in Java. An optimizing JRE may assign a reference to the baz variable before it calls the constructor of the object the reference points to. Note: With Java 5, you can make Double checked locking work, if you declare the variable to be volatile
. For more details refer to: http://www.javaworld.com/javaworld/jw-02-2001/jw-0209-double.html or http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html
Example:
public class Foo {
/*volatile */ Object baz = null; // fix for Java5 and later: volatile
Object bar() {
if (baz == null) { // baz may be non-null yet not fully created
synchronized(this) {
if (baz == null) {
baz = new Object();
}
}
}
return baz;
}
}
Replace this call to "replaceAll()" by a call to the "replace()" method. Open
params.get(i).setName(param.replaceAll("\\{", "").replaceAll("\\}", ""));
- Read upRead up
- Exclude checks
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
Merge this if statement with the enclosing one. Open
if (ParameterizedType.class.isAssignableFrom(returnType.get().getClass()))
- Read upRead up
- Exclude checks
Merging collapsible if
statements increases the code's readability.
Noncompliant Code Example
if (file != null) { if (file.isFile() || file.isDirectory()) { /* ... */ } }
Compliant Solution
if (file != null && isFileOrDirectory(file)) { /* ... */ } private static boolean isFileOrDirectory(File file) { return file.isFile() || file.isDirectory(); }
Change the visibility of this constructor to "protected". Open
public Validator(Annotation annotation) {
- Read upRead up
- Exclude checks
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 DetailFieldStrategy(Type type) {
- Read upRead up
- Exclude checks
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 Body(Method m, Class<?> controllerClass) {
- Read upRead up
- Exclude checks
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 } }
Define and throw a dedicated exception instead of using a generic one. Open
throw new RuntimeException(e);
- Read upRead up
- Exclude checks
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
Rename "resources" which hides the field declared at line 22. Open
Map<String, Resource> resources = new HashMap<>();
- Read upRead up
- Exclude checks
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
Replace this "Map.containsKey()" with a call to "Map.computeIfAbsent()". Open
if (!resource.containsKey(key)) {
- 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 call to "replaceAll()" by a call to the "replace()" method. Open
params.get(i).setName(param.replaceAll("\\{", "").replaceAll("\\}", ""));
- Read upRead up
- Exclude checks
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
Change the visibility of this constructor to "protected". Open
public Body() {
- Read upRead up
- Exclude checks
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 } }