Thursday, July 30, 2009

Distributed Java code inspection

As a requirement of our current project, we wanted to establish a minimum criteria of code quality ahead of formal code inspections. No point in wasting valuable brain capacity at spotting simple problems such as using “==” instead of “equals” and so on.

Static code analysis is a good starting point, though it can be a bit intransigent about false-positives, where the developer had good cause to write code in a way that the rule developer could not have anticipated.

Being that our main development tool is Eclipse, this was the one criteria that made me give up on the excellent FindBugs, in that there was no way to mark up the source code to tell FindBugs to ignore the problem in future scans.

Enter the Eclipse built-in code analysis, which not only allows scanning source code, but also has a simple way of marking false-positives inside the source code using a Java comment, so that further scans will not flag the problem again.

Sharing a rule base

The first step in sharing a rule base is to create one, which can be done from the “Run –> Analysis…” menu.

Set the “Scope” to “Analyze selected projects”, without actually checking any project box. This will allow you to right-click any project and then run the analysis only on the selected project.

Under “Rules”, pick the ones you consider the most important. It took me a couple of scans on pre-existing code to weed out the faulty ones (there are a few) and the overly conservative ones. Once I was satisfied, I clicked on the “Export…” button to save it to a file I could share with my colleagues.

image

Importing a rule base

Starting from the “Run –> Analysis…” menu again, click on the “New launch configuration” button, give it a name. Again, set the “Scope” to “Analyze selected projects”, without actually checking any project box.

In the “Rules” tab, click on the “Import…” button and select the “.dat” file exported in the previous step, which is stored in a location shared with the entire team.

Scanning the code

Right-click a Java project and select “Analysis –> [Name of your launch configuration]”. In a few seconds, the results will be displayed in the “Analysis Results” view.

image

Right-clicking any of the individual results shows a menu with options for a “Quick Fix” (available for some rules) or the selection of “Ignore Result”. The latter option will add a Java comment next to the flagged line of code, instructing the analyzer to not flag the problem in future scans.

image

image

4 comments:

  1. Very nice blog post, though it seems the topic of the post was running a static analysis tool, rather than how to go about having a distributed code inspection. I would like to hear your thoughts on the latter as well.

    ReplyDelete
  2. You are right, I was after static analysis as a pre-requisite for the actual code inspection.

    For a distributed code inspection, I separate it in similar and disparate timezones. My impression is that unless people have a number of hours available for direct interaction, the mutual learning benefit of a code inspection is diminished, in that the inspector does not have the opportunity to hear first-hand about the reasons for a certain coding choice, and the author does not get a chance to explore the advice received.

    That said, there is still some benefit for the inspector to use markers inside the code indicating a question or a problem to the author. The use of Eclipse "Task Tags" (Window -> Preferences -> Java Compiler -> Task Tags" is particularly beneficial. The author can then fix the more obvious problems and chose to reply over email to the questions or suggestions that cannot be implemented.

    Personally, I think it is possible to rely solely on markers, establishing first a review process that arbitrarily remove the possibility of back-forth in the source code (some inspection better than none, better than transforming the code base in a tug of war,) and then second, a protocol of task tags.

    ReplyDelete
  3. I believe findbugs has lets you suppress warnings via annotations. See...
    http://www.nabble.com/Annotations-td2636016.html

    Also, we found PMD to be pretty good too (also has eclipse plugins).

    ReplyDelete
  4. Hi Todd, if FindBugs supports it it is yet to document the feature properly and integrate it properly to its Eclipse. Full disclaimer, have not looked at the docs for the past 6 months.

    I still like the built-in Eclipse plugin better in that it will add the source annotation for you.

    ReplyDelete