Peer code reviews have been proven to be a cost-effective software defect detection technique. This is because defects are detected early in the stage of a project when it is less expensive to fix them.
We think code reviews can be even more powerful when they are assisted by tools — i.e. by static code analysis tools. Because we really like SonarQube which is one of the best tools to integrate the various static code analyzers and to manage code quality in our opinion, and we of course are using Atlassian’s Stash for our code reviews based on pull requests, we started early to integrate these two
This has the advantage that we can focus on verifying the application logic during code reviews , instead of hunting programming bugs where static code analyzers are better in finding them anyway. And we never have to leave Stash, because all the issues, code coverage and code duplication information are displayed in the diffs of our pull requests. In case a static code analyzer reported a questionable issue or suggested a fix that is simply wrong in the given context, Stash allows the reviewer team to discuss it with its pull request comments. And if an issue found is really a serious problem, a JIRA issue or a Stash task can be created so that fixing it is not forgotten.
Hunting bugs in a COBOL codebaseWith this blog post we want to show you a couple of real-world examples to see how this works. We have chosen a small COBOL code base because we want to show that SonarQube is able to analyze even rather old and difficult to parse programming languages and that even for mature programming languages with an otherwise rather scarce tooling, modern programming practices like tool-supported code reviews can be applied with the right tool-set behind.
Let’s get practical. The following examples show you various diffs in pull requests of Stash. We use Sonar for Stash as an integration of SonarQube which shows us all the important information about the two branches of a pull request. The first example we want to show you is an issue in the code where a value is moved to a field which is too small leading to data truncation.
This is definitely an issue which is very hard to find by a human during a code review — especially in COBOL where the declaration of variables and their usage is often by design in different sections of the program. This is where static code analysis tools really shine.
Sonar for Stash lets you comment a found code issue, create a task for it, but also to mark it as a false positive. And it also shows the technical debt calculated by SonarQube for the specific issue:
Example of a detected issue in a COBOL code base.
How to find dead codeAnother great problem for maintenance in legacy systems is dead code. SonarQube is able to detect unused variables, unused paragraphs or even unused copybooks (something similar like header files in C/C++). The following screenshot shows two examples of unused code in Stash’s pull request diff view:
Two code issues in a COBOL code base: the first is an unused data item, the second an unused paragraph.
Analyze inline SQLAnother important part of COBOL programs are inline SQL statements. SonarQube is able to analyze them and has over 30 rules integrated to report typical coding problems. Famous examples are LIKE conditions starting with a wildcard and the usage of DELETE statements without a WHERE clause (although TRUNCATE might be better suited anyway). Here’s an example of the mentioned issue with LIKE and wildcards:
A SQL issue in a COBOL code base.
Don’t let bad code get into your production branchSonar for Stash also shows the most important statistics of the two involved branches of a pull request on Stash’s overview page:
Sonar statistics shown by the Sonar for Stash plug-in in a Stash pull request.
The yellow exclamation mark at the top of the screenshot means that this pull request cannot be merged because the defined quality thresholds (e.g., maximum increase of technical debt) are exceeded. If you hover over it, Sonar for Stash shows the details why this pull request is not allowed to be merged. With this feature, bad code will never be merged again into your master branch!
What we have not shown in this blog post is the integration of SonarQube into your build system. This is a necessary precondition to let SonarQube analyze your projects. There are already various plug-ins for the major continuous integration tools. For the Atlassian ecosystem, there is Sonar for Bamboo which supports Bamboo’s plan branch concept to create a Sonar project for every new branch automatically. This is necessary in order to use them with pull requests. And to not get stuck with potentially thousands of SonarQube projects for every feature branch, Sonar for Stash will automatically delete a Sonar project when its associated feature branch is deleted (e.g., when a pull request is merged and the source branch gets deleted).
That’s how we do code reviews with SonarQube and Stash. We would really like to hear how you do them. Just comment below!