How to find the source of nullable data, tracing and analyzing data flow in IntelliJ IDEA for Kotlin
If you remember, in one of my posts I was talking about null checks in terms of the contract between data producer and the data consumer. There I made a point that it is much better to check the data right away upon retrieval in order to make a decision whether or not the data provided is eligible for processing.
If it’s not — there is no point in dragging it around, as it will be dropped out when it comes to processing at some point of the workflow. Or even better approach — if there is any control over data provider, it should be requested only for the data that could be processed.
It’s just that easy.
This design decision can save you a lot of null checks and headache with NullPointerException
s if you follow it from the beginning. But, if you are dealing with some existing code, you will need some tools to track down the origin of each value under question and check whether the null check is really necessary in each concrete case.
Luckily, we are no longer coding in notepads as our grandparents used to do. We are privileged to use some of the latest tools with a countless number of helper features. Let’s appreciate that.
As you might have guessed already, I am talking about IntelliJ IDEA. And namely about “Analyze Data Flow to Here” feature:
This analysis is executed on method argument and allows you to trace back concrete parameter to the place of its definition. Pretty cool, hah?
And that’s what we need really. After all — there is a new statement for every object in your program. Let’s find all those places where the object under question is created and find out whether we need a null check really.
First thing I did after cloning the project, I counted usages of org.jetbrains.annotations.Nullable
:
2270 usages in production files and 2382 usages overall! That’s a lot. Let’s check whether all of them are necessary.
Maybe some of those could be replaced by introducing better contracts with data producer. Or some of them are redundant because there already was a null-check somewhere along with the flow. But since there is no rule to check data as soon as it’s retrieved — we can’t be sure whether we can operate with the value provided.
First example.
There is a static method in the org.jetbrains.kotlin.codegen.CallReceiver
class defined as follows:
CallReceiver
As we can see — there are a couple of nullable method arguments, namely extensionReceiver
and callableMethod
.
These nullable variables concern me for a number of reasons:
- in case
extensionReceiver
is null the call doesn’t make any sense, the method returns immediately; so why would we drag this null value for who knows how many method calls just to figure out we can’t process the data at the topmost level of the stack? - this method not only just returns in case of
extensionReceive == null
, it itself returns null; to return null in a project that has like 9000 nullables and null checks you should be some kind of a rebel really; additionally, to that, someone on the receiving end would have to check for that null as well. callableMethod
just affects return type, so there could be just two methods one that acceptscallableMethod
and other that simply doesn’t have this parameter at all; why would we drag this empty glass for no reason?
But fist things first, let’s trace down the source of this extensionReceiver
parameter:
extensionReceiver
Data flow for the value shows us that this parameter is originally received by the public static method for which it is marked as optional as well. Unfortunately, it was as far as I could get with this data analysis tool. I was not able to see all the consumers of this public static method inside the project.
But in any case the question arises: wouldn’t it be better to introduce another public method without the parameter that could be null?
But what is even more interesting — is the method itself:
There is already a null check for extensionReceiverParameter
! So the check-in method calcExtensionReceiverType
is redundant. And this null return value we noticed from the beginning (and that is complete nonsense) is never used, which is a good thing. Bad thing is, it’s probably covered with some test for the project to have good condition coverage.
What about callableMethod
parameter — it also originates as an input to this public method. If we check all the usages of this parameter in the method generateCallReceiver
we’ll see the following:
So, as we can see this parameter is used in the following methods: calcExtensionReceiverType
, calcDispatchReceiverType
and isLocalFunCall
. Well, we started from calcExtensionReceiverType
, but let’s check again how it is used there:
Here is a snippet from calcDispatchReceiverType
:
And finally from isLocalFunCall
:
Alright, I can see a pattern here.
First of all, every method starts with a null check.
Second of all, every method needs some specific info from this callableMethod
parameter:
ExtensionReceiverType
andExtensionReceiverKotlinType
in the first case;DispatchReceiverType
andDispatchReceiverKotlinType
in the second;GenerateCalleeType
in the third.
So, apart from the possibility of being null itself, callableMethod
the object is expected to provide different information based on the use case. And I really doubt that it can contain both ExtensionReceiverType
and DispatchReceiverType
for instance at the same time. That to me seems like a bad API design.
The called of this public method generateCallReceiver
probably knows whether it’s dealing with ExtensionReceiver
, DispatchReceiver
or GenerateCalle
; or that it has no callable method altogether. So all those four seem like four different cases that should be handled separately for clearer code.
Here, we’ve just saved Kotlin guys like seven nullable annotations and null checks. 👏
The second example, the simpler one.
We’ll mostly use the data flow analysis tool output and will not dive into code itself so much in this example.
Let’s take a look now at the private constructor of org.jetbrains.kotlin.resolve.calls.util.ExpressionValueArgument
, which is used by three public static methods to construct this object:
The only nullable here is the parameter expression
. Let’s track all possible data flows for it:
Whoa, this seems like a lot. But worry not, check all roots first. We can easily skip the last three, as the expression parameter is marked @NotNull
there. So there was already a null check enforced before. So it’s only these top two we need to check on possible nullability of the value.
Both these two origins deal with a list of KtExpression
s. And it’s the makeValueArgument
call that deals with the concrete values from the list.
Just from this window already we can see that the result of makeValueArgument
is added to another list. So my assumption is that this whole method chain could have been one operation on a stream of KtExpression
s.
But even if it’s not, an element of the list is considered to be nullable in this flow. This already seems like a code smell to me. I frankly don’t remember any data provider I had to deal with that would supply a list of elements mixed with nulls, so during processing, I would need to check every single one of them not to be null.
But even if we assume that there is such data provider, wouldn’t it be better to filter out all nulls once we received this data set and then just proceed with the flow that doesn’t require null checks?
That’s erotesis.
Frankly, I was very surprised by the approach was taken by Kotlin developers. Famous Kotlin’s null safety aims to eliminate the danger of null references from code.
But the code I’ve seen so far in Kotlin’s sources is more like paranoic null checks all over the place. Even if one was performed before, even if it’s not possible to have a null value at this point.
There is no approach and no idea how to make code they write null safe, only a postulate to check everything everywhere. For a couple of times. I guess you can’t be to cautious about null checks, right?
Even more surprising was to see constructions like:
This makes the code somewhat of a mixture of paranoid null checks and careless return null
statements. This just doesn’t seem right.
As a programmer, you are in control of your application flow. And even if you haven’t done it from the beginning — there are some great tools to trace your data flow.
And all you need from there is just a systematic approach to stick with.
Because I believe that there is a place for a null check in Java. And this place is there consumer meets producer.