Showing 518 of 532 total issues
TODO found Open
// TODO: To migrate https://developer.android.com/about/versions/10/privacy/changes#configure-wifi
- Exclude checks
TODO found Open
// TODO: Migrate to SAF
- Exclude checks
TODO found Open
// TODO: Deprecated. Migrate to https://developers.google.com/ml-kit/vision/barcode-scanning/android
- Exclude checks
TODO found Open
//takes just the first signature, TODO: handle multi signed apks
- Exclude checks
TODO found Open
// TODO: Do permission check for bluetooth in the activity
- Exclude checks
TODO found Open
// TODO: Revamp this as it does not work well in dark mode. We are using a legacy theme to mitigate for now
- Exclude checks
TODO found Open
<!-- TODO: We have requestLegacyExternalStorage for now until we migrate to SAF -->
- Exclude checks
Provide the parametrized type for this generic. Open
private void toggleConnectivitySwitch(int startIntent, int endIntent, String name, SwitchCompat mSwitch, ConnectivityPeriod period, Class className) {
- 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;
Add a default case to this switch. Open
switch (exercise.toLowerCase()) {
- Read upRead up
- Exclude checks
The requirement for a final default
clause is defensive programming. The clause should either take appropriate action, or contain a
suitable comment as to why no action is taken.
Noncompliant Code Example
switch (param) { //missing default clause case 0: doSomething(); break; case 1: doSomethingElse(); break; } switch (param) { default: // default clause should be the last one error(); break; case 0: doSomething(); break; case 1: doSomethingElse(); break; }
Compliant Solution
switch (param) { case 0: doSomething(); break; case 1: doSomethingElse(); break; default: error(); break; }
Exceptions
If the switch
parameter is an Enum
and if all the constants of this enum are used in the case
statements,
then no default
clause is expected.
Example:
public enum Day { SUNDAY, MONDAY } ... switch(day) { case SUNDAY: doSomething(); break; case MONDAY: doSomethingElse(); break; }
See
- MITRE, CWE-478 - Missing Default Case in Switch Statement
- CERT, MSC01-C. - Strive for logical completeness
Rename this variable to not match a restricted identifier. Open
private Record migrateRecord(Record record) {
- Read upRead up
- Exclude checks
Even if it is technically possible, Restricted Identifiers should not be used as identifiers. This is only possible for compatibility reasons, using it in Java code is confusing and should be avoided.
Note that this applies to any version of Java, including the one where these identifiers are not yet restricted, to avoid future confusion.
This rule reports an issue when restricted identifiers:
- var
- yield
- record
are used as identifiers.
Noncompliant Code Example
var var = "var"; // Noncompliant: compiles but this code is confusing var = "what is this?"; int yield(int i) { // Noncompliant return switch (i) { case 1: yield(0); // This is a yield from switch expression, not a recursive call. default: yield(i-1); }; } String record = "record"; // Noncompliant
Compliant Solution
var myVariable = "var"; int minusOne(int i) { return switch (i) { case 1: yield(0); default: yield(i-1); }; } String myRecord = "record";
See
End this switch case with an unconditional break, return or throw statement. Open
case 1:
- Read upRead up
- Exclude checks
When the execution is not explicitly terminated at the end of a switch case, it continues to execute the statements of the following case. While this is sometimes intentional, it often is a mistake which leads to unexpected behavior.
Noncompliant Code Example
switch (myVariable) { case 1: foo(); break; case 2: // Both 'doSomething()' and 'doSomethingElse()' will be executed. Is it on purpose ? doSomething(); default: doSomethingElse(); break; }
Compliant Solution
switch (myVariable) { case 1: foo(); break; case 2: doSomething(); break; default: doSomethingElse(); break; }
Exceptions
This rule is relaxed in the following cases:
switch (myVariable) { case 0: // Empty case used to specify the same behavior for a group of cases. case 1: doSomething(); break; case 2: // Use of return statement return; case 3: // Use of throw statement throw new IllegalStateException(); case 4: // Use of continue statement continue; default: // For the last case, use of break statement is optional doSomethingElse(); }
See
- MITRE, CWE-484 - Omitted Break Statement in Switch
- CERT, MSC17-C. - Finish every set of statements associated with a case label with a break statement
- CERT, MSC52-J. - Finish every set of statements associated with a case label with a break statement
Make the enclosing method "static" or remove this set. Open
useAppColor = false;
- Read upRead up
- 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 } }
Merge this if statement with the enclosing one. Open
if (isValidMove(gameBoard, 4))
- 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(); }
Provide the parametrized type for this generic. Open
Class classObj = Class.forName(className);
- 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;
Add a default case to this switch. Open
switch (msg.what) {
- Read upRead up
- Exclude checks
The requirement for a final default
clause is defensive programming. The clause should either take appropriate action, or contain a
suitable comment as to why no action is taken.
Noncompliant Code Example
switch (param) { //missing default clause case 0: doSomething(); break; case 1: doSomethingElse(); break; } switch (param) { default: // default clause should be the last one error(); break; case 0: doSomething(); break; case 1: doSomethingElse(); break; }
Compliant Solution
switch (param) { case 0: doSomething(); break; case 1: doSomethingElse(); break; default: error(); break; }
Exceptions
If the switch
parameter is an Enum
and if all the constants of this enum are used in the case
statements,
then no default
clause is expected.
Example:
public enum Day { SUNDAY, MONDAY } ... switch(day) { case SUNDAY: doSomething(); break; case MONDAY: doSomethingElse(); break; }
See
- MITRE, CWE-478 - Missing Default Case in Switch Statement
- CERT, MSC01-C. - Strive for logical completeness
Add a nested comment explaining why this method is empty, throw an UnsupportedOperationException or complete the implementation. Open
public void onNothingSelected(AdapterView<?> adapterView) {
- 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 } }
Refactor this method to reduce its Cognitive Complexity from 16 to the 15 allowed. Open
public void onReceive(Context context, Intent intent) {
- 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
Define a constant instead of duplicating this literal "SafetyNet request: success
" 3 times. Open
b.append("SafetyNet request: success\n");
- 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.
Use try-with-resources or close this "RandomAccessFile" in a "finally" clause. Open
load = new RandomAccessFile("/sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_min_freq", "r").readLine();
- Read upRead up
- 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
Define a constant instead of duplicating this literal "CPU-READ" 5 times. Open
LogHelper.e("CPU-READ", "Error reading file: " + ex.getLocalizedMessage());
- 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.