Showing 716 of 716 total issues
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 "projectName" private field. Open
private String projectName;
- 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.
Either re-interrupt this method or rethrow the "InterruptedException" that can be caught here. Open
} catch (IOException | InterruptedException e) {
- Read upRead up
- Exclude checks
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
- MITRE, CWE-391 - Unchecked Error Condition
- Dealing with InterruptedException
This block of commented-out lines of code should be removed. Open
{
- 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.
Provide the parametrized type for this generic. Open
Optional opt = tmpConfig.getParameters().stream().filter(p -> TestsToRunConverterBuilder.TESTS_TO_RUN_PARAMETER.equals(p.getName())).findFirst();
- 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;
This block of commented-out lines of code should be removed. Open
//MBT needs to know real path to tests and not ${workspace}
- 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.
Define a constant instead of duplicating this literal "build '" 6 times. Open
logger.error("build '" + jobId + " #" + buildId + "' not found");
- 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 "tagTypeName" 4 times. Open
.setName(jsonObject.getString("tagTypeName")))
- 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 "DELETE" 3 times. Open
if (changeType.equals("DELETE")) {
- 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.
Catch Exception instead of Throwable. Open
} catch (Throwable throwable) {
- 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)
Define a constant instead of duplicating this literal "uploadSSC section was not found" 3 times. Open
logger.warn("uploadSSC section was not found");
- 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 "project" 3 times. Open
json.getString("project"),
- 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.
Make this anonymous inner class a lambda (sonar.java.source not set. Assuming 8 or greater.) Open
new HostnameVerifier(){
- Read upRead up
- Exclude checks
Before Java 8, the only way to partially support closures in Java was by using anonymous inner classes. But the syntax of anonymous classes may seem unwieldy and unclear.
With Java 8, most uses of anonymous inner classes should be replaced by lambdas to highly increase the readability of the source code.
Note that this rule is automatically disabled when the project's sonar.java.source
is lower than 8
.
Noncompliant Code Example
myCollection.stream().map(new Mapper<String,String>() { public String map(String input) { return new StringBuilder(input).reverse().toString(); } }); Predicate<String> isEmpty = new Predicate<String> { boolean test(String myString) { return myString.isEmpty(); } }
Compliant Solution
myCollection.stream().map(input -> new StringBuilder(input).reverse().toString()); Predicate<String> isEmpty = myString -> myString.isEmpty();
Rename this constant name to match the regular expression '^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$'. Open
Alm, FileSystem, LoadRunner
- 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; }
This block of commented-out lines of code should be removed. Open
return true;//FreeStyleProject.class.isAssignableFrom(jobType);
- 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.
This block of commented-out lines of code should be removed. Open
public void setRunFromAlmModel(RunFromAlmModel runFromAlmModel){
- 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.
This block of commented-out lines of code should be removed. Open
//CommonLoggerContextUtil.configureLogger(CIJenkinsServicesImpl.getAllowedStorageFile());
- 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.
This accessibility update should be removed. Open
field.setAccessible(true);
- Read upRead up
- Exclude checks
This rule raises an issue when reflection is used to change the visibility of a class, method or field, and when it is used to directly update a field value. Altering or bypassing the accessibility of classes, methods, or fields violates the encapsulation principle and could lead to run-time errors.
Noncompliant Code Example
public void makeItPublic(String methodName) throws NoSuchMethodException { this.getClass().getMethod(methodName).setAccessible(true); // Noncompliant } public void setItAnyway(String fieldName, int value) { this.getClass().getDeclaredField(fieldName).setInt(this, value); // Noncompliant; bypasses controls in setter }
See
- CERT, SEC05-J. - Do not use reflection to increase accessibility of classes, methods, or fields
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 re-interrupt this method or rethrow the "InterruptedException" that can be caught here. Open
} catch (InterruptedException ex) {
- Read upRead up
- Exclude checks
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
- MITRE, CWE-391 - Unchecked Error Condition
- Dealing with InterruptedException