Felipe Gaucho's Blog

FindBugs and PMD applied on J2EE 5

After few months of a fun learning curve - coding WSDL-first Web Services based on EJB 3 and JPA - I found some time to include automated quality tasks in my project with ant scripts of FindBugs and PMD. As expected, the first round of quality assurance returned me a long list of bugs, most of them trivial mistakes like non public fields or unused methods. After the first cleanup, some bugs remained in the report, and after few quality review cycles I got a set of tricky bugs - the ones you can't imagine the solution and the ones that definitely don't seem a bug. In the next sections I will unveil this tricky bugs and the workarounds I adopted to eliminate them. I hope you agree with my strategy, and I would appreciate a feedback in case you disagree.

Why using PMD and FindBugs?

A good starting point of software quality assurance is to check if the code is working as designed, what can be done through software testing. Despite the usefulness of the tests, they only offer you a good indicator that the code will do the job, but tests miss the point about the fine grain code inspection (show me the code). Are you using the correct syntax and data structures? Is there redundancy in the code?

A soundness analysis of the code requires a person to remember all details about the Java language while keeping an eye on the performance tuning - an impossible mission if tried manually. Fortunately, tools like PMD and FindBugs come to help in finding code problems and also offering good hints on code optimization - the tools unveil the most commons code problems, saving your time to more important tasks. If you never used those tools before, or if you never paid the proper attention to them, you can assume a simple goal: the goal is to minimize the number of reported errors about your code. The closer you get to the zero errors report, the more sound is your code. Yes, it is linear, with some interpretation due to the different levels and types of bugs covered by the automated tools (keep in mind that the tools don't think, they are unnamed robots). The installation and configuration of PMD and FindBugs are fully exploited on the Internet and, instead of publishing one more blog about that, I will briefly comment about the recent experiments we have done in the Cejug-Classifieds Project.

Applying PMD and FindBugs against the Cejug-Classifieds Project

Cejug-Classifieds is an open source J2EE application based on WSDL-first Web Services, EJB 3 and JPA, what means a project full of wsimport generated code, annotations and generics. The combination of those modern Java features and technologies generated controversial bugs on the reports, but first let's checkout the code and run the quality tools. An important detail on our ant task is about its Glassfish dependency. Since our project depends on J2EE types and annotations, the environment variable AS_HOME should be present in your operational system in order to allow you to execute the code compilation task. The steps to checkout the code and run the quality tasks are: Install Glassfish and configure the environment variable AS_HOME Checkout the code from SVN repository: svn checkout https://cejug-classifieds.dev.java.net/svn/cejug-classifieds/trunk cejug-classifieds --username your.java.net.login After connecting the repository, please checkout the cejug-classifieds-server module, the one I will use as example. Run the ant quality tasks: ant server,pmd,FindBugs

Phantom Blot prefers the mind games of FindBugs

The ant tasks will generate two reports you can find in the folder /build/quality - those are standard PMD and FindBugs reports. Other important detail: FindBugs will fail in case of any error, so a successful task execution means a clean code regarding FindBugs. PMD is not 100% clean yet, because there are pending issues on the report - those ones I will discuss below.

Special tips for IDE users: the project is pre-configured to Eclipse and Netbeans, but it can also be compiled and tested from line commands if you prefer that way.

Customization required to produce zero-errors code according to FindBugs

1. Changing XMLGregorianCalendar to Calendar: WSIMPORT uses JAXB to generate classes from the XSD Schemas documents and by default JAXB generates XMLGregorianCalendar from xsd:dateTime attributes definition. Since XMLGregorian calendar is not a Serializable type, FindBugs complains against that. We can workaround that applying a JAXB Customization in our XSD documents, as described in this link.

   
       <xsd:annotation>
          <xsd:appinfo>
             <jaxb:globalBindings>
                <jaxb:javaType name="java.util.Calendar"
                   xmlType="xsd:dateTime"
                   parseMethod="javax.xml.bind.DatatypeConverter.parseDate"
                   printMethod="javax.xml.bind.DatatypeConverter.printDate" />
             </jaxb:globalBindings>
          </xsd:appinfo>
       </xsd:annotation>

2. @EJB remote interface requires Serializable classes: WSIMPORT generates non-serializable code from WSDL documents, but since I am using @Remote interfaces of EJB, the RMI messages requires their contents to be Serializable. The workaround of that was again another annotation in the XSD Schema:

   
       <xsd:annotation>
          <xsd:appinfo>
             <jaxb:globalBindings>
                <xjc:serializable uid="-6026937020915831338" />
                <jaxb:javaType name="java.util.Calendar"
                   xmlType="xsd:dateTime"
                   parseMethod="javax.xml.bind.DatatypeConverter.parseDate"
                   printMethod="javax.xml.bind.DatatypeConverter.printDate" />
             </jaxb:globalBindings>
          </xsd:appinfo>
       </xsd:annotation>

And that's it, after this minor changes the FindBugs stopped to bother me about bugs and I got my 100% clean code following FindBugs, including the generated code. FindBugs checks the byte code of your classes, it uses a more sophisticated strategy on checking how your code works instead of how it is written as .java document. A clean FindBugs code indicates the generated byte code is sound - a good feeling for developers and better sleeping nights for project managers.

Customization required to produce zero-errors code according to PMD

PMD was a bit more complicated to satisfy since it is based on java code analysis instead of the byte code. A bit more pedantic that FindBugs, PMD is based on heuristics about the code statistics. If a code is lengthy, PMD assumes it is buggy, and if there are too many public methods, PMD also assumes it is buggy. That attempt to identify problems analyzing code metrics is controversial but PMD was one of the first quality tool I learned to use few years ago, so I decided to customize its rules set in order to continue on using it in my J2EE 5 code. You can find the resultant rule set configuration file here. Be aware that Cejug-Classifieds is a work under progress and the contents of that file are only our beliefs about our own code quality. After the PMD customization I got a zero-errors report but since PMD covers five different levels of problems, I noticed the report of some High Warnings on the code. I am still fighting against them, specially the following ones:

Beagle Boys prefer brute force against bugs.

1. BeanMembersShouldSerialize: that one is hard, since I have a secondary issue related to that warning. One of the GUIs consuming our services uses FLEX, and it seems FLEX uses reflection to set values in the Java beans. The problem is, JAXB doesn't generate setters methods to attributes that contains maxOccurs='unbounded' (Collections). It causes problems of FLEX consuming SOAP 1.1 service exposed by JAXWS, and we are still looking for a solution for that. Actually, the PMD warning points to another direction, but it keeps a helpful mark to the FLEX problem in our service code.
2. AvoidFieldNameMatchingTypeName: one of the elements of my XSD has a ref to itself, because it models Categories and Sub-Categories. Since I am using reference instead of creating a new attribute, the name of the field is compiled as the same name of the class. I agree with PMD that it is weird, so I will look for a solution asap. Until finding the solution, the warning will remain active.
3. AbstractClassWithoutAbstractMethod: since we are using a JPA persistence façade, we use an AbstractEntity class to share entities fields. That class hasn't any method, since it is used only as superclass to the entities and therefore doesn't make any sense to force this class to be concrete nor to have methods.
4. UncommentedEmptyConstructor: I finally opted to remove this rule since JAXB generates empty constructor for all elements on the meta-data. Not a big deal in my opinion, empty constructors are useless in most part of cases but they never cause problems at all, so the rule seems too pedantic to remain active.
5. UnnecessaryParentheses: the same as the previous example, unnecessary parenthesis causes no damage to the code since they will be removed by the compiler. It only happens on generated classes and shouldn't be part of a serious code analysis.

Conclusion: I strongly recommend you to use PMD and FindBugs on your J2EE 5 code

Generated code and specially Generics requires an extra effort to pass through the code analysis since it is not pure classical Java code. Specially PMD claims more attention since it is based on heuristics instead of byte code analysis and it is older than FindBugs. Many of the suppositions ruled by PMD were created before the generics support on Java, and some of them simply doesn't apply anymore. Despite that, we realized in our experiments that both tools can be used together with good results and no conflict at all. If you aren't using PMD and FindBugs on your project, try them today - you will be surprised on how fast you can enhance the quality of your code. Actually I bet you will also get addicted to their quality metrics, specially FindBugs.

Acknowledge: the experiments and the effort to achieve the current quality results were shared with Rodrigo Lopes, one of the main commiters of Cejug-Classifieds.

Comments

This work is licensed under a Creative Commons License.