Showing 14,752 of 14,752 total issues
Use try-with-resources or close this "ObjectInputStream" in a "finally" clause. Open
ObjectInputStream ois = new ObjectInputStream(in);
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
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
- MITRE, CWE-459 - Incomplete Cleanup
- MITRE, CWE-772 - Missing Release of Resource after Effective Lifetime
- CERT, FIO04-J. - Release resources when they are no longer needed
- CERT, FIO42-C. - Close files when they are no longer needed
- Try With Resources
Use try-with-resources or close this "ObjectInputStream" in a "finally" clause. Open
ObjectInputStream ois = new ObjectInputStream(in);
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
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
- MITRE, CWE-459 - Incomplete Cleanup
- MITRE, CWE-772 - Missing Release of Resource after Effective Lifetime
- CERT, FIO04-J. - Release resources when they are no longer needed
- CERT, FIO42-C. - Close files when they are no longer needed
- Try With Resources
Add a nested comment explaining why this method is empty, throw an UnsupportedOperationException or complete the implementation. Open
public void setHideInWizard(boolean hide) {}
- Read upRead up
- Create a ticketCreate a ticket
- 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 "sourceParam <- fromJSON('" 4 times. Open
rawJsonData = "sourceParam <- fromJSON('"
- Read upRead up
- Create a ticketCreate a ticket
- 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 mousePressed(MouseEvent e) {
- Read upRead up
- Create a ticketCreate a ticket
- 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 } }
Add a nested comment explaining why this method is empty, throw an UnsupportedOperationException or complete the implementation. Open
public void editingCanceled(ChangeEvent e) {
- Read upRead up
- Create a ticketCreate a ticket
- 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 } }
Refactor this method to reduce its Cognitive Complexity from 35 to the 15 allowed. Open
public ChartSelectionPanel(List<String> ids, boolean selectionsExclusive,
- Read upRead up
- Create a ticketCreate a ticket
- 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 36 to the 15 allowed. Open
public Class<?> getColumnClass(int column) {
- Read upRead up
- Create a ticketCreate a ticket
- 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
Define a constant instead of duplicating this literal "UTF-8" 4 times. Open
FileUtils.writeStringToFile(rPackagesFile, packageInfoList, "UTF-8");
- Read upRead up
- Create a ticketCreate a ticket
- 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.
Refactor this method to reduce its Cognitive Complexity from 18 to the 15 allowed. Open
private void resetParameterIdForObjectsFromJSON(FskPortObject outObj1, int suffixIndex) {
- Read upRead up
- Create a ticketCreate a ticket
- 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
Define a constant instead of duplicating this literal "UTF-8" 11 times. Open
IOUtils.write(portObject.model, out, "UTF-8");
- Read upRead up
- Create a ticketCreate a ticket
- 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.
Use already-defined constant 'WORKSPACE' instead of duplicating its value here. Open
final Path workspacePath = FileUtil.createTempFile("workspace", ".r").toPath();
- Read upRead up
- Create a ticketCreate a ticket
- 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.
Replace this call to "replaceAll()" by a call to the "replace()" method. Open
+ tempData.getAbsolutePath().replaceAll("\\\\", "/") + "')";
- Read upRead up
- Create a ticketCreate a ticket
- 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
Make the enclosing method "static" or remove this set. Open
plugin = this;
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
Correctly updating a static
field from a non-static method is tricky to get right and could easily lead to bugs if there are multiple
class instances and/or multiple threads in play. Ideally, static
fields are only updated from synchronized static
methods.
This rule raises an issue each time a static
field is updated from a non-static method.
Noncompliant Code Example
public class MyClass { private static int count = 0; public void doSomething() { //... count++; // Noncompliant } }
Add a nested comment explaining why this method is empty, throw an UnsupportedOperationException or complete the implementation. Open
protected void reset() {}
- Read upRead up
- Create a ticketCreate a ticket
- 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 } }
Use try-with-resources or close this "ObjectInputStream" in a "finally" clause. Open
ObjectInputStream ois = new ObjectInputStream(in);
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
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
- MITRE, CWE-459 - Incomplete Cleanup
- MITRE, CWE-772 - Missing Release of Resource after Effective Lifetime
- CERT, FIO04-J. - Release resources when they are no longer needed
- CERT, FIO42-C. - Close files when they are no longer needed
- Try With Resources
Refactor this method to reduce its Cognitive Complexity from 27 to the 15 allowed. Open
protected PortObject[] performExecute(PortObject[] inObjects, ExecutionContext exec)
- Read upRead up
- Create a ticketCreate a ticket
- 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
Use try-with-resources or close this "ObjectInputStream" in a "finally" clause. Open
final ObjectInputStream ois = new ObjectInputStream(in);
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
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
- MITRE, CWE-459 - Incomplete Cleanup
- MITRE, CWE-772 - Missing Release of Resource after Effective Lifetime
- CERT, FIO04-J. - Release resources when they are no longer needed
- CERT, FIO42-C. - Close files when they are no longer needed
- Try With Resources
Rename this class. Open
public class TimeSeriesSchema extends de.bund.bfr.knime.pmm.common.pmmtablemodel.TimeSeriesSchema {
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
While it's perfectly legal to give a class the same simple name as a class in another package that it extends or interface it implements, it's confusing and could cause problems in the future.
Noncompliant Code Example
package my.mypackage; public class Foo implements a.b.Foo { // Noncompliant
Compliant Solution
package my.mypackage; public class FooJr implements a.b.Foo {
Make the enclosing method "static" or remove this set. Open
plugin = null;
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
Correctly updating a static
field from a non-static method is tricky to get right and could easily lead to bugs if there are multiple
class instances and/or multiple threads in play. Ideally, static
fields are only updated from synchronized static
methods.
This rule raises an issue each time a static
field is updated from a non-static method.
Noncompliant Code Example
public class MyClass { private static int count = 0; public void doSomething() { //... count++; // Noncompliant } }