JMRI: Static Analysis with FindBugs
FindBugs is a tool that can examine code to find possible problems. It does a "static analysis", looking through the code to find certain "known bad ideas": Things that are likely to cause occasional/intermittent problems, poor performance, etc. Because those kinds of problems are hard to find with tests, finding them by inspection is often the only realistic approach. Having a tool that can do those inspections automatically, so that they can be done every time something is changed, keeps the code from slowly getting worse and worse without anybody noticing until it's too late.For more information on FindBugs, see the FindBugs home page.
We routinely run FindBugs as part of our continuous integration process. You can see the results of the most recent build online here.
If FindBugs is finding a false positive, a bug that's not really a bug, you can turn it off with an annotation such as:
@edu.umd.cs.findbugs.annotations.SuppressWarnings("FE_FLOATING_POINT_EQUALITY","OK to compare floats, as even tiny differences should trigger update")
The second "justification" argument is optional, but highly recommended.
Explaining why you've added this annotation to suppress a message will help
whoever comes after you and is trying to understand the code.
It will also help make sure you properly understand the cause of the underlying
bug report: Sometimes what seems a false positive really isn't.
If you need to put more than one message type in an annotation, use array syntax:
@edu.umd.cs.findbugs.annotations.SuppressWarnings("{type1},{type2}","why both are needed")
There are also FindBugs annotations that can help it better understand your code. Sometimes they'll give it enough understanding of e.g. when a variable can be null, that it'll no longer make false-positive mistakes. For more on this, see the Findbugs annotation page.
It can be useful to mark classes with one of the following annotations so that FindBugs does a good job of reasoning about it:
- edu.umd.cs.findbugs.annotations.CheckForNull - a variable or parameter may have a null value, so all uses should appropriately handle that.
- edu.umd.cs.findbugs.annotations.CheckReturnValue - this method has no side-effects, so there's no point in calling it without checking it's return value
- net.jcip.annotations.Immutable - An object of this class can't be changed after it's created. This allows both better checking of logic, and also simplifies use by your colleagues, so it's good to tag classes with this property.
- net.jcip.annotations.NotThreadSafe - a class that isn't thread-safe, so shouldn't be checked for concurrency issues. Often used for Swing-based classes, but note that some Swing components (e.g. monitor windows, classes with listeners) do have to accept input from other threads.
- net.jcip.annotations.ThreadSafe - classes that do have to be usable for multiple threads. FindBugs generally assumes this, but it's good to put it on a class that's intended to be thread-safe as a reminder to future developers.
Simon White added FindBugs support to our Ant-based build chain during the development of JMRI 2.5.5. His note on this follows.
As per feature request 1716873, I've added an ant task to run Findbugs. This will produce a report in HTML in the same location as the JMRI jar (i.e. the top level project dir). Note the new task first runs ant dist because FindBugs examines the jmri.jar file.
To run the task:
- Install Findbugs (for me I put this in C:/findbugs-1.3.8)
- Copy findbugs-ant.jar from the findbugs lib directory to the java/lib directory
- either run
ant -Dfindbugs.home=C:/findbugs-1.3.8 findbugs (replacing the parameter with your install location)
or edit the build.xml and modified the current commented out parameter findbugs.home to your install location and then runant findbugs
Note that in the build.xml I have set the maximum memory -Xmx setting for the findbugs task to 1024m. If your system has more memory, boosting this may speed things up.
Running this on JMRI 2.5.4 produced the following:
| Bad practice Warnings | 164 |
| Correctness Warnings | 77 |
| Experimental Warnings | 7 |
| Malicious code vulnerability Warnings | 221 |
| Multithreaded correctness Warnings | 90 |
| Performance Warnings | 459 |
| Dodgy Warnings | 304 |
| Total | 1322 |
|---|
A lot of work has gone into JMRI during the 2.12 release cycle to bring the bug count down to zero. The Continous Integration server runs FindBugs twice a day, which helps developers see the results of their coding without having to set up to run findbugs themselves.
If you are checking code in to the JMRI Subversion repository, please check the CI server and make sure that your changes do not introduce new errors.