We recently upgraded to the latest version of SpotBugs, which is the successor to FindBugs. Its role is to identify risky areas of code and flag them up.
We also use Sonar, which recently stopped a build because of a bug which had escaped the unit tests, but would have hurt in production. Similarly, I’ve had runtime areas predicted successfully by SpotBugs, so it’s really handy to have tools like this sitting guard over the code making sure invisible errors don’t pass through.
But what if the tool produces lots of false positives, or opinionated results?
We can add suppressions to the code, but then:
When your code is full of tool suppression statements, it starts to lose meaning as code, and becomes coupled to tools that should remain outside of it
Though it’s possible to create external suppression statements, sometimes at least, that’s not quite the point here. Ideally we will create a few general rules, and we’d rather than the tool wasn’t making itself horribly noisy in the first place.
SpotBugs Guilt By Association
Recent changes to SpotBugs seem to be clever, but this comes at a cost. SpotBugs appears to have learned about transitive mutable objects. In other words, it determines if an object you’re storing in one class is effectively immutable or not. If it thinks it’s not immutable, then it raises warnings if you directly store or return that object from another.
This is clever.
Though it does start to feel a bit like a witch hunt.
Worse than that, when you have a Spring project, you often have a series of beans connected to each other, some of which may well either BE mutable, or appear to be so because they come from a library which hasn’t marked them as @Immutable.
For example, in my project, I have a specialist mutable cache, which is used by a few of my beans. It’s a cache of local copies of files, and the important bits are immutable. SpotBugs only knows that anything which uses this bean is exposing its internal implementation to external change.
I’m inclined to think that:
- SpotBugs has a point
- It’s inconvenient, though
- Annotating with
@SuppressFBWarningsis not a good solution - My current approach of a filter file that ignores all POJO model objects AND all
.*Serviceand.*Controllerobjects is not a great solution either, but it is limited to theEIandEI2warnings only, which is probably ok - Overall SpotBugs has documentation that leaves a lot to be desired
The Real Cost of The Tools
Not using tools like this will cost you runtime bugs that could have been prevented.
Using these weakly documented, quirky tools, though enormously powerful, will occasionally lose you:
- Time working through the latest arbitrary pronouncements
- Noise in your code with suppression statements
- Time wasted trying to find a way of expressing a good rule to avoid lots of code noise
Let’s hope things improve.
Published on Java Code Geeks with permission by Ashley Frieze, partner at our JCG program. See the original article here: Nobody Expects the SpotBugs Inquisition Opinions expressed by Java Code Geeks contributors are their own. |
Really helpful, keep sharing mate.