Add SpotBugs to our build

[jasonaowen]

Add the static analysis tool SpotBugs to our Java subprojects, so that we can use it to find errors in our code.

From the website:

SpotBugs is a program which uses static analysis to look for bugs in Java code.

SpotBugs is the spiritual successor of FindBugs, carrying on from the point where it left off with support of its community.

It adds spotbugs{SourceSetName} tasks in gradle via the gradle plugin. In practice, since we use Groovy for our tests and don't have any other source sets, that means you can invoke it like so:

$ ./gradlew spotbugsMain

It creates XML reports in psm-app/<subproject>/build/reports/spotbugs/main.xml.

Currently, there are many problems in our code that SpotBugs can detect, so these checks don't pass; I add it now as an aspirational tool that we can use to find small things to fix in between larger features.

See also the list of bugs it can detect.

Issue #915 Use static analysis to find bugs

[jasonaowen]

Is there a way to configure it so that it only runs for certain packages, or is suppressed by packages? That way, we can start using it to prevent regressions for all the code that luckily passes right now. Then, if it's on jenkins and in our workflow, it won't be forgotten.

It looks like we could write a filter file and adding a gradle configuration directive to use it.

Right now it's finding bugs in every subproject; I haven't yet looked to see which packages. My plan was to configure suppressions after picking some of the low-hanging fruit and researching some of the oft-repeated problems it finds that I either don't understand or don't agree with. I worry that suppressing warnings too soon makes it harder to see those warnings and less likely that we fix the existing problems.

Hm... what if we add empty filters in this PR, and then we could set up CI with filters to ignore all our existing errors and enforce no new errors? I feel like that might address both our concerns... what do you think, @frankduncan?

[frankduncan]

I think that would be awesome. Originally I was thinking of just generating a filter file with all classes that show up with errors, and then over time the goal is to keep removing those until non are left. It kind of becomes a code cleanup todo list. I was planning on doing this with the 90 column wide checkstyle.

After looking at that filter specification, it would be really cool if you can generate matches for all current class/error pairs, so that each item on this new todo list is very narrow. I don't know how annoying generating that filter file would be.

I'll mark this is approve for now if you want to punt on putting that work into an issue to be resolved when time allows.

[jasonaowen]

Further research reveals that we have three ways of filtering:

• include filters
• exclude filters
• baseline exclusion filters

The first two are documented in the aforementioned link, but the baseline filter (also know as the excludeBugsFilter) is not as well documented. It seems to accept the output of a previous run of SpotBugs, and excludes everything listed in that report. It can be specified in gradle like so:

spotbugs {
    excludeBugsFilter = file("$project.projectDir/spotbugs-baseline.xml")
}

Unfortunately, it crashes on an empty file, and so I see two paths to updating it as we go:

• by hand, or
• edit build.gradle, comment out that line, run the reports, copy/move them over the old report, and uncomment that line

While I appreciate the utility of having SpotBugs hooked up to CI sooner rather than later, I think I want to spend a little more time investigating or fixing the problems it's reporting before setting up exclusions; ideally, we'd be able to get rid of all of them ( or at least decide we don't want to deal with some categories of bugs and ignore them appropriately), and never need to commit a baseline report.