Why use static analysis? Exploring an error from Akka.NET
Use static analysis regularly, not just before releases…
The earlier you find errors, the cheaper they are to fix…
You probably heard this a hundred times. Today we’ll answer the “Why?” question once again. An error from the Akka.NET project will assist us.
The error
We’ll start with a task. Find a defect in the code below:
Let’s examine the code above and see what the problem is.
The _shards variable is of type HashSet<ShardId>. The code above calls several methods that change the state of this set.
HashSet<T>.Add:
_shards.Add(shardId);HashSet<T>.UnionWith:
_shards.UnionWith(state.Shards.Keys.Union(state.UnallocatedShards));However, one of these calls is incorrect:
_shards.Union(state.Shards);It does not change the state of the _shards object. Enumerable.Union is a LINQ extension method that does not change the original collection and returns a modified collection instead. This means, the method call’s result must be saved somewhere or used somehow. We do not see that in the code.
The PVS-Studio analyzer issued the following warning: V3010 The return value of function ‘Union’ is required to be utilized. Akka.Cluster.Sharding EventSourcedRememberEntitiesCoordinatorStore.cs 123
By the way, here’s what the fixed code looks like:
_shards.UnionWith(state.Shards);How we found the error — or talk number 101 on the benefits of static analysis
Every night our server runs static analysis for several open-source projects. These include Akka.NET. Why do we do it? This practice offers a few benefits:
- it provides an extra way to test the analyzer;
- it helps us write notes like this one and provides us with interesting examples that demonstrate the benefits of static analysis.
We wrote more about it here.
And now a few words on our case at hand — how the error appeared and how it was fixed.
April 20, 2022:
- code with an error is committed to the Akka.NET project’s dev branch (a link to the specific line of code);
April 21, 2022:
- our server analyzes the code and sends me an email with warnings;
- I investigate the problem and create an issue on GitHub;
- the developers fix the error. A link to the commit.
I think that was some pretty smooth collaboration! Thanks to the developers for the prompt fix.
And now to the important question — how long would this error have existed in the code if events had taken a different turn? Here I’ll leave some room for your imagination.
So what can you do right now to avoid mistakes like this one?
