Showing 3,943 of 3,943 total issues
Define a constant instead of duplicating this literal "ASSIGNED_WORKER" 5 times. Open
rs.getString("ASSIGNED_WORKER"),
- Read upRead up
- Exclude checks
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.
Define a constant instead of duplicating this literal " q.CREATE_TIME " 4 times. Open
" q.CREATE_TIME " +
- Read upRead up
- Exclude checks
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.
Define a constant instead of duplicating this literal " FROM OO_EXECUTION_QUEUES qq " 3 times. Open
" FROM OO_EXECUTION_QUEUES qq " +
- Read upRead up
- Exclude checks
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.
Add a nested comment explaining why this method is empty, throw an UnsupportedOperationException or complete the implementation. Open
public void deleteInterrupts(Long executionId, Map<String, Set<String>> interrupts) {
- 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 } }
Remove the non-escaped \u0009 character from this literal. Open
" (s.ACTIVE = 1) AND " +
- Read upRead up
- Exclude checks
Non-encoded control characters and whitespace characters are often injected in the source code because of a bad manipulation. They are either
invisible or difficult to recognize, which can result in bugs when the string is not what the developer expects. If you actually need to use a control
character use their encoded version (ex: ASCII \n,\t,
... or Unicode U+000D, U+0009,
...).
This rule raises an issue when the following characters are seen in a literal string:
- ASCII control character. (character index < 32 or = 127)
- Unicode whitespace characters.
- Unicode C0 control characters
- Unicode characters
U+200B, U+200C, U+200D, U+2060, U+FEFF, U+2028, U+2029
No issue will be raised on the simple space character. Unicode U+0020
, ASCII 32.
Noncompliant Code Example
String tabInside = "A B"; // Noncompliant, contains a tabulation String zeroWidthSpaceInside = "foobar"; // Noncompliant, it contains a U+200B character inside char tab = ' ';
Compliant Solution
String tabInside = "A\tB"; // Compliant, uses escaped value String zeroWidthSpaceInside = "foo\u200Bbar"; // Compliant, uses escaped value char tab = '\t';
Exceptions
Text Blocks string literals (java 13 three double-quote marks) can contain tabulations to allow indentation using tabulations.
Add a nested comment explaining why this method is empty, throw an UnsupportedOperationException or complete the implementation. Open
public void deletePauseData(String executionId, String branchId) {
- 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 } }
Define a constant instead of duplicating this literal "runEnv" 3 times. Open
eventData.put(EventConstants.SCORE_RUN_ENV, execution.getContexts().get("runEnv"));
- Read upRead up
- Exclude checks
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 this "try" to a try-with-resources. (sonar.java.source not set. Assuming 7 or greater.) Open
try {
- Read upRead up
- Exclude checks
Java 7 introduced the try-with-resources statement, which guarantees that the resource in question will be closed. Since the new syntax is closer
to bullet-proof, it should be preferred over the older try
/catch
/finally
version.
This rule checks that close
-able resources are opened in a try-with-resources statement.
Note that this rule is automatically disabled when the project's sonar.java.source
is lower than 7
.
Noncompliant Code Example
FileReader fr = null; BufferedReader br = null; try { fr = new FileReader(fileName); br = new BufferedReader(fr); return br.readLine(); } catch (...) { } finally { if (br != null) { try { br.close(); } catch(IOException e){...} } if (fr != null ) { try { br.close(); } catch(IOException e){...} } }
Compliant Solution
try ( FileReader fr = new FileReader(fileName); BufferedReader br = new BufferedReader(fr) ) { return br.readLine(); } catch (...) {}
or
try (BufferedReader br = new BufferedReader(new FileReader(fileName))) { // no need to name intermediate resources if you don't want to return br.readLine(); } catch (...) {}
See
- CERT, ERR54-J. - Use a try-with-resources statement to safely handle closeable resources
Define a constant instead of duplicating this literal "no worker was found by the specified UUID:" 7 times. Open
throw new IllegalStateException("no worker was found by the specified UUID:" + uuid);
- Read upRead up
- Exclude checks
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.
Remove this "clone" implementation; use a copy constructor or copy factory instead. Open
public Object clone() {
- Read upRead up
- Exclude checks
Many consider clone
and Cloneable
broken in Java, largely because the rules for overriding clone
are tricky
and difficult to get right, according to Joshua Bloch:
Object's clone method is very tricky. It's based on field copies, and it's "extra-linguistic." It creates an object without calling a constructor. There are no guarantees that it preserves the invariants established by the constructors. There have been lots of bugs over the years, both in and outside Sun, stemming from the fact that if you just call super.clone repeatedly up the chain until you have cloned an object, you have a shallow copy of the object. The clone generally shares state with the object being cloned. If that state is mutable, you don't have two independent objects. If you modify one, the other changes as well. And all of a sudden, you get random behavior.
A copy constructor or copy factory should be used instead.
This rule raises an issue when clone
is overridden, whether or not Cloneable
is implemented.
Noncompliant Code Example
public class MyClass { // ... public Object clone() { // Noncompliant //... } }
Compliant Solution
public class MyClass { // ... MyClass (MyClass source) { //... } }
See
See Also
- {rule:java:S2157} - "Cloneables" should implement "clone"
- {rule:java:S1182} - Classes that override "clone" should be "Cloneable" and call "super.clone()"
Define a constant instead of duplicating this literal "Group name is empty or null" 4 times. Open
Validate.notEmpty(groupName, "Group name is empty or null");
- Read upRead up
- Exclude checks
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.
Remove usage of generic wildcard type. Open
public Map<String, ?> getNavigationData() {
- Read upRead up
- Exclude checks
It is highly recommended not to use wildcard types as return types. Because the type inference rules are fairly complex it is unlikely the user of that API will know how to use it correctly.
Let's take the example of method returning a "List<? extends Animal>". Is it possible on this list to add a Dog, a Cat, ... we simply don't know. And neither does the compiler, which is why it will not allow such a direct use. The use of wildcard types should be limited to method parameters.
This rule raises an issue when a method returns a wildcard type.
Noncompliant Code Example
List<? extends Animal> getAnimals(){...}
Compliant Solution
List<Animal> getAnimals(){...}
or
List<Dog> getAnimals(){...}
Refactor this method to reduce its Cognitive Complexity from 19 to the 15 allowed. Open
private boolean shouldBeRolled(PartitionGroup partitionGroup){
- 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
Add a nested comment explaining why this method is empty, throw an UnsupportedOperationException or complete the implementation. Open
public void stop() {
- 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 } }
Remove this call to "wait" or move it into a "while" loop. Open
lock.wait(10000);
- Read upRead up
- Exclude checks
According to the documentation of the Java Condition
interface:
When waiting upon a
Condition
, a "spurious wakeup" is permitted to occur, in general, as a concession to the underlying platform semantics. This has little practical impact on most application programs as a Condition should always be waited upon in a loop, testing the state predicate that is being waited for. An implementation is free to remove the possibility of spurious wakeups but it is recommended that applications programmers always assume that they can occur and so always wait in a loop.
The same advice is also found for the Object.wait(...)
method:
waits should always occur in loops, like this one:
synchronized (obj) { while (<condition does not hold>){ obj.wait(timeout); } ... // Perform action appropriate to condition }
Noncompliant Code Example
synchronized (obj) { if (!suitableCondition()){ obj.wait(timeout); //the thread can wake up even if the condition is still false } ... // Perform action appropriate to condition }
Compliant Solution
synchronized (obj) { while (!suitableCondition()){ obj.wait(timeout); } ... // Perform action appropriate to condition }
See
- CERT THI03-J. - Always invoke wait() and await() methods inside a loop
Rename this constant name to match the regular expression '^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$'. Open
private static final int capacity = getInteger("metrics.collection.sampleCount", 10);
- 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; }
Define a constant instead of duplicating this literal "Failed to execute script {%s}" 3 times. Open
logger.error(String.format("Failed to execute script {%s}", exception));
- Read upRead up
- Exclude checks
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.
Rename this constant name to match the regular expression '^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$'. Open
private static final int nullStringLength = "null".length();
- 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; }
Rename this constant name to match the regular expression '^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$'. Open
private static final boolean disabled = getBoolean("worker.monitoring.disable");
- 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 nested comment explaining why this method is empty, throw an UnsupportedOperationException or complete the implementation. Open
public void allocate() {
- 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 } }