Wednesday, December 28, 2011

Sonar Review

Overview

In short, Sonar provides great convenience in source code analysis and unit testing. Sonar works as a facade to various open source tools aiming at code qualities, such as Findbugs, PMD, Checkstyle, Cobertura, Clover, Surefire, etc. The Sonar-Maven integration is excellent. Executing a single Maven goal sonar will run all code analysis and tests, and will store the results in a database.

A dashboard displays summary of various analysis. The summary can be either per project or per Java package. From there one can drill down into various issues with details like where in the source code the issues occur. Often the code with issues is visually marked out in its context displayed on the GUI. For each rule violation, Sonar also provides a brief explanation on why it is an issue. For programmers who do not have any idea about the potential harm of the code with the issue, the explanation is a good starting point to learn.


Features

Rule Violations

This is one of the most useful features of Sonar. It lists the total number of rule violations, and the subtotal numbers in the groups: blocker, critical, major, minor, and info (they are in the descendant order of severity). It also gives the percentage of rule compliance. Below are two bugs caught by Sonar in real world enterprise projects that I saw in person.

Bug 1 – An Infinite Loop

public Table() {
    Table myTable = new Table();
}

Sonar's remark: Correctness - An apparent infinite recursive loop.


The constructor calls itself recursively and forms an infinite loop.




Bug 2 – Mistaking & for &&

if ( books != null & !books.isEmpty() ) {
     return books.toArray(new Book[]{});
} 

Sonar's remark: Correctness - Possible null pointer dereference

The code author intended to prevent NullpointerException but the actually code does not do what he meant. & is actually bit shift rather than logic “and”. The author should have used &&, which is logic “and” instead of &.


Duplications

Sonar is quite good at identifying code duplications. Not just identical code blocks are identified as duplications, but also code blocks similar enough.

The duplication check is very helpful for programmers to minimize code duplications. For an excellent explanation of the harm of code duplications, please see The Evils of Duplication in the must-read The Pragmatic Programmer: From Journeyman to Master by Andrew Hunt and David Thomas.

Complexity

The complexity is measured by the cyclomatic complexity number for the classes and methods. Cyclomatic complexity is a mature and well-accept software quality metric, backed by empirical data. This feature is very helpful to identify hotspots where the code is too complex.

Package tangle index

It is said that there is a package dependency from Package A to Package B if some Java classes in Package A depend on Java classes in Package B. The package tangle index indicates the severity of cyclical package dependency, for example Package A depends on Package B, and meanwhile Package B also depends on Package A.

The layer architecture pattern is the most fundamental architecture pattern. (For an excellent discussion of the layer architecture pattern, see Layer in Pattern-Oriented Software Architecture Volume 1 by Frank Bushchmann and others).  Classes responsible for different layers should be placed in different packages. A cyclical package dependency may indicate that a lower layer  depends on a upper layer, or even worse, the software does not have well defined layers. We may call it anti-layer pattern.

Some cyclical package dependencies can be eliminated by simply moving classes from one package to another.

Comments

In the comments section, sonar shows the percentage of API with javadoc and how many public classes or public methods do not have any companion javadoc. The GUI allows user to drill down into the source code to see which classes/methods are not documented.

Sonar, however, cannot evaluate the quality of the javadoc. It means that one may have 100% API documented but the document may be totally trash.

On the other hand, Sonar cannot tell that certain methods like getters and setters do not really need javadoc.
The analysis score of comments must be taken with a grain of salt.

Code coverage

Sonar provides a summary on the percentage of code and branches covered by unit tests. The GUI also makes it easy to locate code that is not covered by unit tests. However, one must be aware that a 100% code coverage is far from a 100% correctness. 100% coverage only means that 100% code is executed during unit tests. Bugs won't be caught by just executing the code where they reside.

Lack of cohesion of methods (LCOM4) and Response for Class (RFC)


These are two of the six metrics in the Chidamber & Kemerer metrics suite. (LCOM4 is an improved versio. The the original LCOM of the Chidamber & Kemerer metrics suite is usually referred as LCOM1).

Personally, I have not yet seen much value of these two metrics.



Issues

Performance

Sonar is quite slow. Some time it is very slow.

Data Corruption

A bug frequently corrupts data in database and causes sonar execution (analysis and testing) to fail.

Example - Error Message from Failing Sonar Execution (via Maven)

[INFO] ------------------------------------------------------------------------
[ERROR] BUILD ERROR
[INFO] ------------------------------------------------------------------------
[INFO] Can not execute Sonar

Embedded error: PicoLifecycleException: method 'public void org.sonar.batch.ProjectTree.start() throws java.io.IOException', instance 'org.sonar.batch.ProjectTr
ee@a9be37, java.lang.RuntimeException: wrapper
result returns more than one elements
[INFO] ------------------------------------------------------------------------
[INFO] For more information, run Maven with the -e switch
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 1 minute 17 seconds
[INFO] Finished at: Fri Sep 16 10:43:21 EDT 2011
[INFO] Final Memory: 103M/119M
[INFO] ------------------------------------------------------------------------

Sonar expects any two records in the snapshots table with 1 as the value of the islast field to have different project_id. The bug causes more than one record to have the same project_id while the value of the islast field is 1. When it happens, Sonar execution (analysis and testing) will fail.

To resolve the problem, one has to run a certain SQL script in the database to delete the problematic snapshots records. It will only temporarily solve the problem, which will re-occur again.

Documentation

Sonar documentation is pretty good.
  1. Sonar documentation at the home site