Core Java

Nobody Expects the SpotBugs Inquisition

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.

Our chief weapon is surprise, surprise and pedantry, our TWO main weapons…

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 @SuppressFBWarnings is not a good solution
  • My current approach of a filter file that ignores all POJO model objects AND all .*Service and .*Controller objects is not a great solution either, but it is limited to the EI and EI2 warnings 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.

Ashley Frieze

Software developer, stand-up comedian, musician, writer, jolly big cheer-monkey, skeptical thinker, Doctor Who fan, lover of fine sounds
Subscribe
Notify of
guest

This site uses Akismet to reduce spam. Learn how your comment data is processed.

1 Comment
Oldest
Newest Most Voted
Inline Feedbacks
View all comments
Lisa Powell
Lisa Powell
2 years ago

Really helpful, keep sharing mate.

Back to top button