Get Started
- CodeAnt AI
- Control Center
- Pull Request Review
- IDE
- Compliance
- Anti-Patterns
- Code Governance
- Infrastructure Security Database
- Application Security Database
Kotlin
There are two types of sequence and stream operations:
Intermediate operations, which return another sequence or stream
Terminal operations, which return something else
Intermediate operations are lazy, meaning they aren’t actually executed until and unless a terminal sequence operation is performed on their results. Consequently, if the result of an intermediate sequence operation is not fed to a terminal operation, it serves no purpose, which is almost certainly an error.
Sometimes there is the need to cancel the execution of a coroutine after a given period of time. You can do this manually by combining the delay() and cancel() functions. However, this technique is verbose and error-prone. An easier way to manage timeouts is using the function withTimeout() or withTimeoutOrNull().
The withTimeout function will throw a TimeoutCancellationException when the timeout is reached, while withTimeoutOrNull will simply return null instead.
This rule raises an issue if timeout mechanisms are implemented manually instead of using appropriate built-in functions.
Kotlin features language support for the delegator pattern using by clauses. Because this is a built-in language feature, it should be used as an idiom instead of resorting to custom idioms.
It is a common pattern to validate required preconditions at the beginning of a function or block. There are two different kinds of preconditions:
Preconditions about argument values. An example is the assertion that a function argument lies within a specific range. An IllegalArgumentException should be thrown if these preconditions are violated.
Preconditions about the state of the owner or the execution context of the function. An example is when a specific method, such as open, init or prepare, must be called before the current method can be executed. An IllegalStateException should be thrown if these preconditions are violated.
The Kotlin standard library provides the functions check(), require(), checkNotNull() and requireNotNull() for this purpose. They should be used instead of directly throwing an IllegalArgumentException or an IllegalStateException.
The logic of a task’s actions, like doFirst and doLast, is only executed when the task is executed. Configuration logic (everything outside the action blocks) is executed in the configuration phase for every build, whether the task will be executed or not.
Code that could also be moved into the action blocks but resides in the configuration logic can inflict a performance penalty.
The actions performed by a task are often too complex or specific, and the task name cannot describe the task’s action sufficiently well. Moreover, build scripts usually consist of many tasks - whether explicitly defined or implicitly by the applied plugins - so a structure is required to help the user navigate through existing tasks.
Therefore, tasks should define a description and group to provide documentation and categorization for the user.
In Kotlin, you must override either both or neither of the equals(Any?) and hashCode() methods in order to keep the contract between the two:
Whenever the hashCode method is invoked on the same object more than once, it must consistently return the same integer, provided no information used in equals comparisons on the object is modified.
If two objects are equal according to the equals method, then calling the hashCode method on each of the two objects must produce the same integer result.
By overriding only one of the two methods with a non-trivial implementation, this contract is almost certainly broken.
The expression can be simplified using one of the functions isEmpty, isNotEmpty or isNullOrEmpty.
Kotlin supports getters and setters for properties. Because this is a built-in language feature, it should be the idiom used to implement the getter and setter pattern instead of using custom idioms.
An ObjectOutputStream writes primitive data types and graphs of Java objects to an OutputStream. The objects can be read (reconstituted) using an ObjectInputStream.
When ObjectOutputStream is used with files opened in append mode, it can cause data corruption and unexpected behavior. This is because when ObjectOutputStream is created, it writes metadata to the output stream, which can conflict with the existing metadata when the file is opened in append mode. This can lead to errors and data loss.
When used with serialization, an ObjectOutputStream first writes the serialization stream header. This header should appear once per file at the beginning. When you’re trying to read your object(s) back from the file, only the first one will be read successfully, and a StreamCorruptedException will be thrown after that.
Kotlin provides the operators as and as? to cast an expression to a specific type, and is to check the assignment compatibility with a type. These operators are used for downcasts, smart casts, and run-time type checking.
In case the as or as? operator is used for upcasting from a subtype to a supertype, the cast is redundant as it has no effect and can never fail. If a specific type is expected, an expression of a subtype can always be inserted without casting (Substitution Principle and Assignment Compatibility).
Likewise, the is operator is redundant and will always return true if the type of the expression on the left side is assignment compatible with the type on the right.
The functions find(predicate), findLast(predicate), firstOrNull(predicate) and “lastOrNull(predicate) can be improperly used to check the presence of an element that matches the given predicate. In such cases the code is more difficult to read and understand than it would be with the functions any(predicate), none(predicate) or contains(element).
Dependencies should be grouped by their destination, otherwise, it becomes hard for the reader to see which destination has what dependencies.
When accessing Map elements by key, you should keep in mind that value might not be present. If value is not present, null will be returned. To make it possible, the type of returned value is nullable. In Kotlin, it’s not usually convenient to operate with nullable types, so developers usually try to avoid them or convert them to non-nullable types. One of the possible solutions is to use !! (non-null assertion operator). If during the runtime the actual value applied to non-null asserrion operator was null, then NullPointerException will be thrown. While in some cases it could still be legitimate to use !!, accesing Map values is not one of them. Usage of a !! when accesing Map values is a bad practice and can lead to NullPointerException in Kotlin and potential crashes, if Java interop was involved.
The Kotlin when statement can not only be used to compare a selector against a list of values, but also to select from a list of conditions.
This is a more readable and elegant alternative to a chain of if statements and should be used instead.
It is common for code to have unnecessary variable assignments. Unnecessary variable assignments make code harder to read and maintain. By avoiding these redundant assignments, your code will be more efficient and understandable.
In order to extend the functionalities of your Gradle build you can add plugins with id(“plugin-id”). Gradle maintains some plugins that are known as core plugins. These core plugins can be identified by a short name.
When adding core plugins, it is a good practice to use their short name because it is more concise, readable, and less prone to typing mistakes.
In Kotlin, Flow represents a cold stream concept. Similar to Stream in Java or Sequence in Kotlin, we can manipulate the data inside the flow (filter, transform, collect, etc). The Flow API, just like Stream and Sequence, offers two types of operations: intermediate and terminal. Intermediate operations again return a Flow instance, all other operations are considered terminal. As flows are naturally lazy, no operations will actually be started until a terminal operation is called.
This rule reports an issue when the result of an intermediate operation on Flow is left unused.
The filter(predicate) function is used to extract a subset of elements from a collection that match a given predicate. Many collection functions such as any(), count(), first(), and more, come with an optional condition predicate.
It is not recommended to invoke the filter(predicate) function prior to these terminal operations. Instead, the predicate variant of the terminal operation should be used as a replacement.
Kotlin coroutines follow the principle of structured concurrency. This helps in preventing resource leaks and ensures that scopes are only exited once all child coroutines have exited. Hence, structured concurrency enables developers to build concurrent applications while having to worry less about cleaning up concurrent tasks manually.
It is possible to break this concept of structured concurrency in various ways. Generally, this is not advised, as it can open the door to coroutines being leaked or lost. Ask yourself if breaking structured concurrency here is really necessary for the application’s business logic, or if it could be avoided by refactoring parts of the code.
This rule raises an issue when it detects that the structured concurrency principles are violated. It avoids reporting on valid use cases and in situations where developers have consciously opted into using delicate APIs (e.g. by using the @OptIn annotation) and hence should be aware of the possible pitfalls.
While `hashCode and toString are available on arrays, they are largely useless. hashCode returns the array’s “identity hash code”, and toString returns nearly the same value. Neither method’s output actually reflects the array’s contents. Furthermore, contentHashCode() and contentToString() are also useless on arrays of array.
Instead, you should use:
On array of objects or arrays: contentDeepHashCode() and contentDeepToString()
On array of primitives: contentHashCode() and contentToString()`
The suspend function modifier is used to mark a function which might take some time to execute and could suspend the caller coroutine. The location where such function is called is a “suspension point”. Functions marked as suspend (“suspending functions”) should themselves contain at least one suspension point, otherwise it makes no sense to mark it as suspend
This rule reports an issue if a function with suspend modifier has no calls to other suspend functions inside its body.
If a mutable collection type is used but no mutating functions such as add or remove are ever called, and the collection instance does not leave the scope of the function, it can be replaced with the corresponding immutable collection type.
This is similar to why val should be used instead of var for local variables that are never re-assigned.
In Kotlin, object declarations and object expressions are the designed way to implement the singleton pattern. Because this is a built-in language feature, it should be used as an idiom instead of resorting to custom idioms.
An alternative is to declare private constructors and to provide a single class instance within a companion object. However, this idiom is adopted from other languages like Java which do not feature the direct declaration of objects. It should not be used in Kotlin.
settings.gradle.kts or settings.gradle are used to define general project properties; see the following sample file:
Multi-line string literals representing a big chunk of text should not be used directly in complex expressions, as it decreases the code’s readability. It is preferred to extract the string literal and store it in a variable or a field that can be accessed in the context of the complex expression.
This rule reports an issue when a multi-line string literal consists of more lines than specified in the rule’s parameter and is directly used within a lambda literal.
Jump statements (return, break and continue
) move control flow out of the current code block. So any statements that come after a jump are dead code.
PreparedStatement is an object that represents a precompiled SQL statement, that can be used to execute the statement multiple times efficiently.
ResultSet is the Java representation of the result set of a database query obtained from a Statement object. A default ResultSet object is not updatable and has a cursor that moves forward only.
The parameters in PreparedStatement and ResultSet are indexed beginning at 1, not 0. When an invalid index is passed to the PreparedStatement or ResultSet methods, an IndexOutOfBoundsException is thrown. This can cause the program to crash or behave unexpectedly, leading to a poor user experience.
This rule raises an issue for the get methods in PreparedStatement and the set methods in ResultSet.
The size of a collection or an array is always greater than or equal to zero. Testing it doesn’t make sense, since the result is always true.
The Any#equals(other: Any?) method is used to compare two objects to see if they are equal.
The other parameter’s type is Any?, this means that an object of any type, as well as null, can be passed as an argument to this method.
Any class overriding Any#equals(other: Any?) should respect this contract, accept any object as an argument, and return false when the argument’s type differs from the expected type. The other parameter’s type can be checked using the is operator or by comparing the javaClass field:
For example, with the default provided regular expression ^[a-zA-Z][a-zA-Z0-9]*$
, the function:
Functional interfaces can be instantiated from lambda expressions directly. If the only purpose of a class or a singleton object is to implement a functional interface, that class or object is redundant and should be replaced with a lambda expression.
MutableStateFlow and MutableSharedFlow are very convenient for storing and adding updates of some data structures in event-driven paradigm. This is widely used in Android Views for handling updates. While it’s extremely useful to manage such objects inside some class, it’s not recommended to expose them outside of the class.
When properties of the types MutableStateFlow or MutableSharedFlow are accessible from outside of a class, data updates cannot be verified properly anymore. It is generally recommended to have only one class responsible for updating these flows, otherwise inconsistency issues and problems with maintainability, as well as increased error-proneness may be introduced.
To restrict write access, StateFlow or SharedFlow should be used together with private MutableStateFlow or MutableSharedFlow fields.
This rule raises an issue when encountering a public or internal property of the type MutableStateFlow or MutableSharedFlow.
When using POSIX classes like \p{Alpha} without the (?U) to include Unicode characters or when using hard-coded character classes like “[a-zA-Z]”, letters outside of the ASCII range, such as umlauts, accented letters or letter from non-Latin languages, won’t be matched. This may cause code to incorrectly handle input containing such letters.
To correctly handle non-ASCII input, it is recommended to use Unicode classes like \p{IsAlphabetic}. When using POSIX classes, Unicode support should be enabled by using (?U) inside the regex.
Generally speaking, main threads should be available to allow user-facing parts of an application to remain responsive. Long-running blocking operations can significantly reduce threads’ availability and are best executed on a designated thread pool.
As a consequence, suspending functions should not block main threads and instead move any long-running blocking tasks off the main thread. This can be done conveniently by using withContext with an appropriate dispatcher. Alternatively, coroutine builders such as launch and async accept an optional CoroutineContext. An appropriate dispatcher could be Dispatchers.IO for long-running blocking IO operations, which can create and shutdown threads on demand.
For some blocking tasks and APIs there may already be suspending alternatives available. When available, these alternatives should be used instead of their blocking counterparts.
This rule raises an issue when the call of a long-running blocking function is detected within a suspending function without the use of an appropriate dispatcher. If non-blocking alternatives to the called function are known, they may be suggested (e.g. use delay(…) instead of Thread.sleep(…)).
In Kotlin, == means structural equality and != structural inequality and both map to the left-side term’s equals() function. It is, therefore, redundant to call equals() as a function. Also, == and != are more general than equals() and !equals() because it allows either of both operands to be null.
Developers using equals() instead of == or != is often the result of adapting styles from other languages like Java, where == means reference equality and != means reference inequality.
If a local variable is never reassigned, it should be declared val to make it a constant within its scope. This makes the code easier to read and protects the variable from accidental re-assignments in future code changes.
The public API of a framework, plugin, or library is the way its provider intended it to be used. API stability and compatibility (within the same major version number) of a library are guaranteed only for its public API.
Internal APIs are mere implementation details and are prone to breaking changes as the implementation of the library changes. No guarantees are being made about them. Therefore, users should not use internal APIs, even when visible.
Kotlin uses the data type Unit to represent the absence of a value in a function or an expression. It corresponds to the type void in Java or unknown in JavaScript. While Void is available in Kotlin, it is a Java platform type and not equivalent to Java void but java.lang.Void.
Use Unit instead of Void because it represents the absence of a value in Kotlin.
In Kotlin, nullability is a part of the type system. By default, any given type T is non-nullable. If you append a ”?” to the type, it becomes nullable: T?.
When accessing properties or functions of a nullable type, you need to handle the case when the target is null. However, while accessing a non-nullable type, it is redundant to test for null, as the compiler statically ensures that the value can never be null. So all the nullability checks on the non-nullable types are considered code smells.
On the other hand, performing a null-check on a value that is always null is equally as redundant.
Here is an example of a non-nullable variable. s is of a type String and cannot be null.
settings.gradle.kts or settings.gradle are used to define general project properties.
It is a good practice to specify the rootProject.name property to set your project name and avoid inconsistencies. By default, Gradle uses the project directory name as the project name and this is not guaranteed to always be the desired behavior.
Passing a collection as an argument to the collection’s own method is either an error - some other argument was intended - or simply nonsensical code.
Further, because some methods require that the argument remain unmodified during the execution, passing a collection to itself can result in undefined behavior.
The when
statement should be used only to clearly define some new branches in the control flow. As soon as a case clause contains too many statements this highly decreases the readability of the overall control flow statement. In such case, the content of the case clause should be extracted into a dedicated function.
Dispatchers should not be hardcoded when using withContext or creating new coroutines using launch or async. Injectable dispatchers ease testing by allowing tests to inject more deterministic dispatchers.
You can use default values for the dispatcher constructor arguments to eliminate the need to specify them explicitly in the production caller contexts.
This rule raises an issue when it finds a hard-coded dispatcher being used in withContext or when starting new coroutines.
Kotlin provides the operators as and as? to cast an expression to a specific type, and is to check the assignment compatibility with a type. These operators are used for downcasts, smart casts, and run-time type checking.
In case the as or as? operator is used for casting between incompatible types, that is, distinct types and neither being a subtype of the other, the cast will never succeed but always throw a ClassCastException. This results in dead code and is likely a symptom of wrong program logic.
Likewise, the is operator is redundant and will never return true if the type of the expression on the left side is incompatible with the type on the right.
Having all branches of a when or if chain with the same implementation indicates a problem.
In the following code:
The is construction is a preferred way to check whether a variable can be cast to some type statically because a compile-time error will occur in case of incompatible types. The isInstance() functions from kotlin.reflect.KClass and java.lang.Class work differently and type check at runtime only. Incompatible types will therefore not be detected as early during development, potentially resulting in dead code. isInstance() function calls should only be used in dynamic cases when the is operator can’t be used.
This rule raises an issue when isInstance() is used and could be replaced with an is check.
There are two ways to define asynchronous functions in Kotlin:
using the modifier suspend in the function declaration
creating an extension function on CoroutineScope (or passing it as a parameter)
The suspend modifier is generally used for functions that might take some time to complete. The caller coroutine might be potentially suspended.
Functions that return results immediately but start a coroutine in the background should be written as extension functions on CoroutineScope. At the same time, these functions should not be declared suspend, as suspending functions should not leave running background tasks behind.
The primary purpose of classes that implement indexed access operators is that of element access, like this is the case for arrays, lists, maps, or sets. When a class implements indexed access operators, they should be used as operators instead of calling them as functions, because this is the intention of the designer of the API.
Data classes have autogenerated implementations of equals, hashcode, toString, copy and componentN. Although the former three methods can still be overridden, there is no use to do so if no special logic is required. The latter two methods cannot be overridden and cause a compile-time error if attempted.
This rule reports an issue on simple equals and hashCode implementations with no additional logic beyond the default behaviour.
In general hard-coded values is a well known bad practice that affects maintainability. In dependency management, this issue is even more critical because there is the risk of accidentally having different versions for the same dependency in your configuration.
Keeping hard-coded dependency versions increases the cost of maintainability and complicates the update process.
Custom tasks and plugins should be moved from the build script to the buildSrc directory to encapsulate custom build logic and separate it against its application in the build script.
This helps to keep the build script clean and easy to understand.
In Kotlin, if and when statements are expressions that return a value. This allows for a more concise and functional programming style with less cognitive complexity, because it results in fewer return points and fewer variable assignments in a function.
If both branches of an if statement end with a return statement, the if statement should be used instead as an expression for a return statement.
If all branches of an exhaustive when statement end with a return statement, the when statement should be used instead as an expression for a return statement. A when statement is exhaustive when it covers all elements of an enum or features an else clause.
An infinite loop is one that will never end while the program is running, i.e., you have to kill the program to get out of the loop. Whether it is by meeting the loop’s end condition or via a break
, every loop should have an end condition.
By convention, calls to suspending functions should not block the thread, eliminating the requirement of calling suspending functions on background threads. Any long-running blocking operations should be moved to background threads within the suspending function that performs these operations. If suspending functions do block, this is breaking the Kotlin coroutines conventions (also see S6307).
As suspending functions can generally be called directly within other suspending functions, there is no need to move such a call to a background thread. This does not bring much benefit while adding complexity and making the code harder to understand.
In fact, various libraries explicitly provide suspending APIs for otherwise long-running blocking operations. The complexity of moving the appropriate tasks to background threads is already taken care of within the library and does not have to be considered when calling the library’s suspending API.
Note also, however, that suspending functions do not block by convention. This behavior is not enforced on a technical level, leaving it to the developer to ensure the conventions are actually followed. If a suspending library function not under the control of a developer is actually blocking, then calling it in a different thread can work around the issues caused by the poorly written suspending function. It should be preferred to fix the callee, however, and not make such workarounds common practice.
This rule raises an issue when a suspending function is called in a way that executes the call in a new thread.
Views should not be responsible for directly triggering coroutines. Hence, ViewModel classes should prefer creating coroutines instead of exposing suspending functions to perform some piece of business logic. This allows for easier testing of your application, as ViewModel classes can be unit tested, whereas views require instrumentation tests.
Please refer to the Android docs for more advanced examples and mechanisms of updating the views with data generated asynchronously.
This rule raises an issue when suspending functions are exposed by classes inheriting from ViewModel.
Some functions return kotlinx.coroutines.Deferred to eventually communicate the result of an asynchronous operation. This is necessary, as callers do not wait for the result of an operation when it is launched asynchronously. Even if the result may be relevant later on, other tasks can be completed while waiting for the asynchronous operation’s result to become available. Hence, to avoid blocking the caller, functions can trigger a task to be run and then immediately return a Deferred instance, which will contain the result once the background task is complete.
Deferred (aka Future, Promise, etc) provides an await() function, which suspends the caller coroutine until the asynchronous task is complete and returns the result of the execution. By not using the Deferred return value, the result of the corresponding asynchronously launched task is lost. This could point to an issue in the code, where data is not passed along as intended.
For instance, the Kotlin coroutines API provides both the functions async and launch as ways to launch asynchronous work. The key difference here is their return type. launch starts a new coroutine without blocking the current thread and returns a reference to the coroutine as a Job. This function is not designed to return a result, i.e. follows the idea of “fire and forget”. async creates a coroutine and returns its future result as an implementation of Deferred.
Ask yourself whether:
You really need whatever result is calculated by the asynchronous operation. If not, you may be better off using a function that does not return Deferred.
You should be using the Deferred return value and fetching some data from it later on, for instance by calling await() on it.
This rule raises an issue when a function returning the type kotlinx.coroutines.Deferred is used without the result of the operation being retrieved.
There are two ways to define asynchronous functions in Kotlin:
using the modifier suspend in the function declaration
creating an extension function on CoroutineScope
The suspend modifier is generally used for functions that might take some time to complete. The caller coroutine might potentially be suspended.
Functions that start a coroutine in the background and return before said coroutine has completed running should be extension functions on CoroutineScope. This helps to clarify the intention of such a function. Further, such functions should not be suspending, as suspending functions should only return once all the work they are designed to perform is complete.
Functions returning Flow or Channel should return the result immediately and may start a new coroutine in the background. As a consequence, such functions should not be suspending and if they launch a coroutine in the background, they should be declared as extension functions on CoroutineScope.
In data classes, the default behavior of the equals() method is to check the equality by field values. This works well for primitive fields or fields, whose type overrides equals(), but this behavior doesn’t work as expected for array fields.
By default, array fields are compared by their reference, so overriding equals() is highly recommended to ensure a deep equality check. The same applies to the hashcode() method.
This rule reports an issue if a record class has an array field and is not overriding equals() or hashcode() methods.
Nested `when structures are difficult to understand because you can easily confuse the cases of an inner when as belonging to an outer statement. Therefore nested when statements should be avoided.
Specifically, you should structure your code to avoid the need for nested when statements, but if you cannot, then consider moving the inner when` to another function.
The Kotlin collections API has methods that allow developers to overcome type-safety restriction of the parameter, such as Iterable.contains. When the actual type of the object provided to these methods is not consistent with the target collection’s actual type, those methods will always return false or null. This is most likely unintended and hides a design problem.
This rule raises an issue when the type of the argument of the following APIs is unrelated to the type used for the collection declaration:
MutableCollection.remove
MutableCollection.removeAll
MutableCollection.retainAll
Array.contains
Array.indexOf
Array.lastIndexOf
Collection.containsAll
Iterable.contains
Iterable.indexOf
Iterable.lastIndexOf
List.indexOf
List.lastIndexOf
Map.contains
Map.containsKey
Map.containsValue
Map.get
MutableMap.remove
Using break, continue, return and throw inside of a finally block suppresses the propagation of any unhandled Throwable thrown in the try or catch block.
This rule raises an issue when a jump statement (break, continue, return, throw) would force control flow to leave a finally block.
An interface that declares only a single function should be marked as function interface. Function interfaces can be instantiated from lambda expressions directly and are, therefore, more comfortable to use.
Also, consider using a function type instead of a function interface. In many situations, a function type is sufficient. A function interface is only required when the function must not be anonymous or when an object should implement multiple function interfaces at once.
Using the Kotlin Gradle DSL, a task can be defined in several ways:
tasks.create(…) will eagerly configure the task, regardless of whether it is required.
tasks.register(…) lazily configures the task only when it is required. This happens when it is located using query methods such as TaskCollection.getByName(java.lang.String), when it is added to the task graph for execution, or when Provider.get() is called on the return value of this method.
It is generally more efficient to use tasks.register(…) instead of tasks.create(…) as the task will not be configured if it is not needed.
Overly complicated regular expressions are hard to read and to maintain and can easily cause hard-to-find bugs. If a regex is too complicated, you should consider replacing it or parts of it with regular code or splitting it apart into multiple patterns at least.
The complexity of a regular expression is determined as follows:
Each of the following operators increases the complexity by an amount equal to the current nesting level and also increases the current nesting level by one for its arguments:
`| - when multiple | operators are used together, the subsequent ones only increase the complexity by 1
&& (inside character classes) - when multiple && operators are used together, the subsequent ones only increase the complexity by 1
Quantifiers (*, +, ?, {n,m}, {n,} or {n})
Non-capturing groups that set flags (such as (?i:some_pattern) or (?i)some_pattern`)
Lookahead and lookbehind assertions
Additionally, each use of the following features increase the complexity by 1 regardless of nesting:
character classes
back references
If a regular expression is split among multiple variables, the complexity is calculated for each variable individually, not for the whole regular expression. If a regular expression is split over multiple lines, each line is treated individually if it is accompanied by a comment (either a Java comment or a comment within the regular expression), otherwise the regular expression is analyzed as a whole.
Storing data locally is a common task for mobile applications. Such data includes preferences or authentication tokens for external services, among other things. There are many convenient solutions that allow storing data persistently, for example SQLiteDatabase, SharedPreferences, and Realm. By default these systems store the data unencrypted, thus an attacker with physical access to the device can read them out easily. Access to sensitive data can be harmful for the user of the application, for example when the device gets stolen.
It is needlessly complex to invert the result of a boolean comparison. The opposite comparison should be made instead.
Regular expressions have their own syntax that is understood by regular expression engines. Those engines will throw an exception at runtime if they are given a regular expression that does not conform to that syntax.
To avoid syntax errors, special characters should be escaped with backslashes when they are intended to be matched literally and references to capturing groups should use the correctly spelled name or number of the group.
WebViews can be used to display web content as part of a mobile application. A browser engine is used to render and display the content. Like a web application, a mobile application that uses WebViews can be vulnerable to Cross-Site Scripting if untrusted code is rendered.
If malicious JavaScript code in a WebView is executed this can leak the contents of sensitive files when access to local files is enabled.
Putting multiple statements on a single line lowers the code readability and makes debugging the code more complex.
Unresolved directive in <stdin> - include::{noncompliant}[]
Write one statement per line to improve readability.
Unresolved directive in <stdin> - include::{compliant}[]
The complexity of an expression is defined by the number of &&, || and condition ? ifTrue : ifFalse
operators it contains.
A single expression’s complexity should not become too high to keep the code readable.
Development tools and frameworks usually have options to make debugging easier for developers. Although these features are useful during development, they should never be enabled for applications deployed in production. Debug instructions or error messages can leak detailed information about the system, like the application’s path or file names.
A boolean literal can be represented in two different ways: {true} or {false}. They can be combined with logical operators ({ops}) to produce logical expressions that represent truth values. However, comparing a boolean literal to a variable or expression that evaluates to a boolean value is unnecessary and can make the code harder to read and understand. The more complex a boolean expression is, the harder it will be for developers to understand its meaning and expected behavior, and it will favour the introduction of new bugs.
This rule is meant to be used as a way to track code which is marked as being deprecated. Deprecated code should eventually be removed.
Using pseudorandom number generators (PRNGs) is security-sensitive. For example, it has led in the past to the following vulnerabilities:
When software generates predictable values in a context requiring unpredictability, it may be possible for an attacker to guess the next value that will be generated, and use this guess to impersonate another user or access sensitive information.
Nested code - blocks of code inside blocks of code - is eventually necessary, but increases complexity. This is why keeping the code as flat as possible, by avoiding unnecessary nesting, is considered a good practice.
Merging if statements when possible will decrease the nesting of the code and improve its readability.
An empty {operationName} is generally considered bad practice and can lead to confusion, readability, and maintenance issues. Empty {operationName}s bring no functionality and are misleading to others as they might think the {operationName} implementation fulfills a specific and identified requirement.
There are several reasons for a {operationName} not to have a 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 exception should be thrown.
The method is an intentionally-blank override. In this case a nested comment should explain the reason for the blank override.
One way to test for empty lines is to use the regex `”^$”, which can be extremely handy when filtering out empty lines from collections of Strings, for instance. With regard to this, the Javadoc for Pattern (Line Terminators) states the following:
As emphasized, ^ is not going to match at the end of an input, and the end of the input is necessarily included in the empty string, which might lead to completely missing empty lines, while it would be the initial reason for using such regex.
Therefore, when searching for empty lines using a multi-line regular expression, you should also check whether the string is empty.
This rule is raising an issue every time a pattern that can match the empty string is used with MULTILINE flag and without calling isEmpty()` on the string.
Clear-text protocols such as `ftp, telnet, or http lack encryption of transported data, as well as the capability to build an authenticated connection. It means that an attacker able to sniff traffic from the network can read, modify, or corrupt the transported content. These protocols are not secure as they expose applications to an extensive range of risks:
sensitive data exposure
traffic redirected to a malicious endpoint
malware-infected software update or installer
execution of client-side code
corruption of critical information
Even in the context of isolated networks like offline environments or segmented cloud environments, the insider threat exists. Thus, attacks involving communications being sniffed or tampered with can still happen.
For example, attackers could successfully compromise prior security layers by:
bypassing isolation mechanisms
compromising a component of the network
getting the credentials of an internal IAM account (either from a service account or an actual person)
In such cases, encrypting communications would decrease the chances of attackers to successfully leak data or steal credentials from other network components. By layering various security practices (segmentation and encryption, for example), the application will follow the defense-in-depth principle.
Note that using the http` protocol is being deprecated by major web browsers.
In the past, it has led to the following vulnerabilities:
When a cookie is configured with the HttpOnly attribute set to true, the browser guaranties that no client-side script will be able to read it. In most cases, when a cookie is created, the default value of HttpOnly is false and it’s up to the developer to decide whether or not the content of the cookie can be read by the client-side script. As a majority of Cross-Site Scripting (XSS) attacks target the theft of session-cookies, the HttpOnly
attribute can help to reduce their impact as it won’t be possible to exploit the XSS vulnerability to steal session-cookies.
The use of parentheses, even those not required to enforce a desired order of operations, can clarify the intent behind a piece of code. However, redundant pairs of parentheses could be misleading and should be removed.
Storing data locally is a common task for mobile applications. Such data includes files among other things. One convenient way to store files is to use the external file storage which usually offers a larger amount of disc space compared to internal storage.
Files created on the external storage are globally readable and writable. Therefore, a malicious application having the permissions WRITE_EXTERNAL_STORAGE or READ_EXTERNAL_STORAGE could try to read sensitive information from the files that other applications have stored on the external storage.
External storage can also be removed by the user (e.g when based on SD card) making the files unavailable to the application.
When placing Unicode Grapheme Clusters (characters which require to be encoded in multiple Code Points) inside a character class of a regular expression, this will likely lead to unintended behavior.
For instance, the grapheme cluster c̈ requires two code points: one for ‘c’, followed by one for the umlaut modifier ‘\u{0308}‘. If placed within a character class, such as [c̈], the regex will consider the character class being the enumeration [c\u{0308}] instead. It will, therefore, match every ‘c’
and every umlaut that isn’t expressed as a single codepoint, which is extremely unlikely to be the intended behavior.
This rule raises an issue every time Unicode Grapheme Clusters are used within a character class of a regular expression.
There is no reason to re-assign a variable to itself. Either this statement is redundant and should be removed, or the re-assignment is a mistake and some other value or variable was intended for the assignment instead.
This rule applies whenever an `if statement is followed by one or more else if statements; the final else if should be followed by an else statement.
The requirement for a final else statement is defensive programming.
The else statement should either take appropriate action or contain a suitable comment as to why no action is taken. This is consistent with the requirement to have a final default clause in a switch` statement.
Android applications can receive broadcasts from the system or other applications. Receiving intents is security-sensitive. For example, it has led in the past to the following vulnerabilities:
Receivers can be declared in the manifest or in the code to make them context-specific. If the receiver is declared in the manifest Android will start the application if it is not already running once a matching broadcast is received. The receiver is an entry point into the application.
Other applications can send potentially malicious broadcasts, so it is important to consider broadcasts as untrusted and to limit the applications that can send broadcasts to the receiver.
Permissions can be specified to restrict broadcasts to authorized applications. Restrictions can be enforced by both the sender and receiver of a broadcast. If permissions are specified when registering a broadcast receiver, then only broadcasters who were granted this permission can send a message to the receiver.
This rule raises an issue when a receiver is registered without specifying any broadcast permission.
When a cookie is protected with the secure
attribute set to true it will not be send by the browser over an unencrypted HTTP request and thus cannot be observed by an unauthorized person during a man-in-the-middle attack.
Android comes with Android KeyStore, a secure container for storing key materials. It’s possible to define certain keys to be unlocked when users authenticate using biometric credentials. This way, even if the application process is compromised, the attacker cannot access keys, as presence of the authorized user is required.
These keys can be used, to encrypt, sign or create a message authentication code (MAC) as proof that the authentication result has not been tampered with. This protection defeats the scenario where an attacker with physical access to the device would try to hook into the application process and call the `onAuthenticationSucceeded method directly. Therefore he would be unable to extract the sensitive data or to perform the critical operations protected by the biometric authentication.
Ask Yourself Whether
The application contains:
Cryptographic keys / sensitive information that need to be protected using biometric authentication.
There is a risk if you answered yes to this question.
Recommended Secure Coding Practices
It’s recommended to tie the biometric authentication to a cryptographic operation by using a CryptoObject` during authentication.
`java.util.concurrent.ScheduledThreadPoolExecutor’s pool is sized with corePoolSize, so setting corePoolSize to zero means the executor will have no threads and run nothing.
This rule detects instances where corePoolSize` is set to zero, via either its setter or the object constructor.
Duplicated string literals make the process of refactoring complex and error-prone, as any change would need to be propagated on all occurrences.
A value that is incremented or decremented and then not stored is at best wasted code and at worst a bug.
Code is sometimes annotated as deprecated by developers maintaining libraries or APIs to indicate that the method, class, or other programming element is no longer recommended for use. This is typically due to the introduction of a newer or more effective alternative. For example, when a better solution has been identified, or when the existing code presents potential errors or security risks.
Deprecation is a good practice because it helps to phase out obsolete code in a controlled manner, without breaking existing software that may still depend on it. It is a way to warn other developers not to use the deprecated element in new code, and to replace it in existing code when possible.
Deprecated classes, interfaces, and their members should not be used, inherited or extended because they will eventually be removed. The deprecation period allows you to make a smooth transition away from the aging, soon-to-be-retired technology.
Check the documentation or the deprecation message to understand why the code was deprecated and what the recommended alternative is.
WebViews can be used to display web content as part of a mobile application. A browser engine is used to render and display the content. Like a web application, a mobile application that uses WebViews can be vulnerable to Cross-Site Scripting if untrusted code is rendered. In the context of a WebView, JavaScript code can exfiltrate local files that might be sensitive or even worse, access exposed functions of the application that can result in more severe vulnerabilities such as code injection. Thus JavaScript support should not be enabled for WebViews unless it is absolutely necessary and the authenticity of the web resources can be guaranteed.
Two {func_name}s having the same implementation are suspicious. It might be that something else was intended. Or the duplication is intentional, which becomes a maintenance burden.
Hardcoding IP addresses is security-sensitive. It has led in the past to the following vulnerabilities:
Today’s services have an ever-changing architecture due to their scaling and redundancy needs. It is a mistake to think that a service will always have the same IP address. When it does change, the hardcoded IP will have to be modified too. This will have an impact on the product development, delivery, and deployment:
The developers will have to do a rapid fix every time this happens, instead of having an operation team change a configuration file.
It misleads to use the same address in every environment (dev, sys, qa, prod).
Last but not least it has an effect on application security. Attackers might be able to decompile the code and thereby discover a potentially sensitive address. They can perform a Denial of Service attack on the service, try to get access to the system, or try to spoof the IP address to bypass security checks. Such attacks can always be possible, but in the case of a hardcoded IP address solving the issue will take more time, which will increase an attack’s impact.
Developers often use TODO tags to mark areas in the code where additional work or improvements are needed but are not implemented immediately. However, these TODO tags sometimes get overlooked or forgotten, leading to incomplete or unfinished code. This rule aims to identify and address unattended TODO tags to ensure a clean and maintainable codebase. This description explores why this is a problem and how it can be fixed to improve the overall code quality.
Storing files locally is a common task for mobile applications. Files that are stored unencrypted can be read out and modified by an attacker with physical access to the device. Access to sensitive data can be harmful for the user of the application, for example when the device gets stolen.
Character classes in regular expressions are a convenient way to match one of several possible characters by listing the allowed characters or ranges of characters. If the same character is listed twice in the same character class or if the character class contains overlapping ranges, this has no effect.
Thus duplicate characters in a character class are either a simple oversight or a sign that a range in the character class matches more than is intended or that the author misunderstood how character classes work and wanted to match more than one character. A common example of the latter mistake is trying to use a range like [0-99] to match numbers of up to two digits, when in fact it is equivalent to [0-9]. Another common cause is forgetting to escape the - character, creating an unintended range that overlaps with other characters in the character class.
This rule raises an issue when a {visibility} {operationName} is never referenced in the code.
`if statements with conditions that are always false have the effect of making blocks of code non-functional. if statements with conditions that are always true are completely redundant, and make the code less readable.
There are three possible causes for the presence of such code:
An if statement was changed during debugging and that debug code has been committed.
Some value was left unset.
Some logic is not doing what the programmer thought it did.
In any of these cases, unconditional if` statements should be removed.
An empty code block is confusing. It will require some effort from maintainers to determine if it is intentional or indicates the implementation is incomplete.
Unresolved directive in <stdin> - include::{example}[]
Removing or filling the empty code blocks takes away ambiguity and generally results in a more straightforward and less surprising code.
Enabling runFinalizersOnExit is unsafe as it might result in erratic behavior and deadlocks on application exit.
Indeed, finalizers might be force-called on live objects while other threads are concurrently manipulating them.
Instead, if you want to execute something when the virtual machine begins its shutdown sequence, you should attach a shutdown hook.
Although they don’t affect the runtime behavior of the application after compilation, removing them will:
Improve the readability and maintainability of the code.
Help avoid potential naming conflicts.
Improve the build time, as the compiler has fewer lines to read and fewer types to resolve.
Reduce the number of items the code editor will show for auto-completion, thereby showing fewer irrelevant suggestions.
Cryptographic hash algorithms such as MD2, MD4, MD5, MD6, HAVAL-128, HMAC-MD5, DSA (which uses SHA-1), RIPEMD, RIPEMD-128, RIPEMD-160, HMACRIPEMD160 and SHA-1 are no longer considered secure, because it is possible to have collisions
(little computational effort is enough to find two or more different inputs that produce the same hash).
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.
Because it is easy to extract strings from an application source code or binary, credentials should not be hard-coded. This is particularly true for applications that are distributed or that are open-source.
In the past, it has led to the following vulnerabilities:
Credentials should be stored outside of the code in a configuration file, a database, or a management service for secrets.
This rule flags instances of hard-coded credentials used in database and LDAP connections. It looks for hard-coded credentials in connection strings, and for variable names that match any of the patterns from the provided list.
It’s recommended to customize the configuration of this rule with additional credential words such as “oauthToken”, “secret”, …
In Android applications, broadcasting intents is security-sensitive. For example, it has led in the past to the following vulnerability:
By default, broadcasted intents are visible to every application, exposing all sensitive information they contain.
This rule raises an issue when an intent is broadcasted without specifying any “receiver permission”.
Android KeyStore is a secure container for storing key materials, in particular it prevents key materials extraction, i.e. when the application process is compromised, the attacker cannot extract keys but may still be able to use them. It’s possible to enable an Android security feature, user authentication, to restrict usage of keys to only authenticated users. The lock screen has to be unlocked with defined credentials (pattern/PIN/password, biometric).
An unused local variable is a variable that has been declared but is not used anywhere in the block of code where it is defined. It is dead code, contributing to unnecessary complexity and leading to confusion when reading the code. Therefore, it should be removed from your code to maintain clarity and efficiency.
A naming convention in software development is a set of guidelines for naming code elements like variables, functions, and classes. {identifier_capital_plural} hold the meaning of the written code. Their names should be meaningful and follow a consistent and easily recognizable pattern. Adhering to a consistent naming convention helps to make the code more readable and understandable, which makes it easier to maintain and debug. It also ensures consistency in the code, especially when multiple developers are working on the same project.
This rule checks that {identifier} names match a provided regular expression.
Having two clauses in a when statement or two branches in an if chain with the same implementation is at best duplicate code, and at worst a coding error.