« ehcache 1.0 released | Main | AMD64 Redux: Sun 40Z on fire! »

October 23, 2004

Adventures in CheckStyle

I blogged recently about the new features in checkstyle 3.4 and their application to an open source project of mine, ehcache.

Applying the changes to ehcache was relatively straightforward. This week I applied them to a large project that has been going for 18 months. So far it has taken me all week to do so. Interestingly the same 5-10 classes kept coming up as requiring suppression of a CheckStyle check. We always knew those classes smelt but now the smell is overpowering.

DeclarationOrder

The DeclarationOrder check has proven to be quite troublesome. Not surprisingly most of our classes were loosely in conformance with the Sun standard for Java source file organisation. The idea here is that it supports code standards and collective code ownership.

Jalopy

I tried out some tools for automating the rearrangement. First I tried jalopy. There is an IntelliJ plugin which seems to work fine. I then tried the ant plugin to convert the entire code base. With jalopy you can export a configuration and get the ant plugin to apply it. Try as I might I could not get it to do rearrangement. Jalopy has gone commercial now. The last open source version is 1 beta 10. The commercial version is 1.3. It looks like bugs are not being fixed in the open source version, which is a pity.

Jacobe

I then tried jacobe which supports a long list of formatting options. Unfortunately one of those is not declaration order.

Rearranger

Finally I tried another IntelliJ plugin - Rearranger. This tool mainly does what it says: rearranges files in accordance with your declaration order. You can load a default and then tweak it from there. Overall this tool worked very well. I opened each file needing rearrangement and then rearranged each one. I then tweaked the rules until I got it to agree with CheckStyle.

A few words of warning though. Firstly on one file Rearranger inexplicably deleted two methods. This was repeatable. Bring the original back, hit Rearranger and it would delete the methods again. A more serious problem was with some scrappy typesafe enum classes we had. In these and a few other classes we had constants which were public and some which were private with the public ones initialising from the private ones. According to CheckStyle the public ones should be declared first. Due to the Java restriction on forward references during initialization this was not possible without a compile time error. There was also a case where the rearrangement on one innocent looking class caused a ClassNotDefError in our EJB tier which eluded all debugger based efforts to locate it. I finally found it by rolling all the changes back and forth in a binary search. I am still not clear what the problem was,

Automating Code Beatification on Commit

We discussed whether to discard the checkstyle rules and allow a beautifier to do its work at CVS commit time. The problems I had with the initial rearrangement cost me almost a day. Far better to require the code to be in order, then get CheckStyle to check it then run all the tests.

Empty JavaDoc

CheckStyle has a JavaDocStyle check which can include a checkEmptyJavadoc property.

The empty check finds empty lines e.g.

/** * This is

 * a JavaDoc comment with an empty line in the middle of it.
 */

This is not that useful. It does not find:

/** */

or multiline empty JavaDoc.

In addition it would be useful to find comments with say less than 5 characters in them. These comments are as good as empty.

I have written such a check called EmptyJavaDoc. I will submit it as a patch to CheckStyle.

Posted by gluck at October 23, 2004 04:40 AM

Comments