The local build configuration for otr4j

Sat, Aug 25, 2018 ❝A description of the current build configuration for otr4j.❞

Contents

otr4j, the Java implementation of Off-the-Record protocol (OTR), is a library for use by chat clients to include OTR protocol support. My recent effort has been focused on implementing OTRv4 (draft) in otr4j. This is a follow up to previous efforts in refactoring otr4j. I have been able to benefit greatly from the design and code structure improvements. As otr4j is used by other software, it is important to provide a robust library with a good API.

Being able to deliver a good API and implementation relies for a significant part on developer skills and experience. However, there are plenty of aspects that can be off-loaded to automated checking tools, such as syntactical correctness, appropriate use of language features (e.g. avoid features that exist only for backwards-compatibility), appropriate use of logic compositions (e.g. bit manipulation logic), code styling and formatting, extensive static analysis. By introducing these checks at build-time, we prevent “bad” code from being introduced unknowingly.

The general idea: set a quality threshold and signal if you drop below this threshold. I chose to set a lower, feasible threshold - meaning I disabled checks that may be sensible but that cannot be met at this time - instead of a high, infeasible threshold. Years of software development have shown that developers are not impressed by a flood of warnings. However, by setting a threshold that we meet now and then fail the build if we drop below the threshold, we have immediate benefit. The ability to suppress warnings for individual cases allows us to make exceptions. This is especially beneficial for “foreign” (mathematic, cryptographic etc.) concepts which use specific naming conventions. The exceptions will be localized to that particular class. All in all, the biggest benefit of this is that developers can focus on one thing. The developer mind is freed to focus on the item he is working on, getting that to work. The attention for the finishing touches can come later. Note that this is already the case. However, now we get a reminder as the build fails.

In addition, I should note that this is based on a single, important assumption: the build-time checks do not interfere with the developer’s ability to do work. The developer needs to be able to build the software without being blocked. In Java, the IDE is able to do this. For example, IntelliJ will build the code without blocking on compiler warnings. Therefore, the build failures can function as last-stage signals instead of early annoyances. This is an important property if we want to be productive as well as thorough.

Current build-time checks

So let’s have a look at the build-time checks that are currently available.

Compiler warnings

The compiler warnings are there to warn the developer about trivial mistakes, like the forgotten generic types and the old-style language constructs. These are trivial to correct.

By default, maven not only ignores compiler warnings but hides them as well. Now we fail the build on compiler warnings.

Javadoc warnings

As otr4j is a library that we expected to be used by multiple clients, we should add a decent amount of javadoc to explain the API. However, this is not the only reason.

We enable the Javadoc warnings for the public-facing API, which helps as a reminder to developers to consider whether something really needs to be part of the public API. We should prefer a minimal API. If the method isn’t documented, is it even really needed to be exposed?

Undeclared / Unused declared dependencies

In order for the build system to do its work, it needs to have accurate information about dependencies. By warning about used-but-not-declared and declared-but-not-used dependencies, we can weed out forgotten configuration changes.

Unit tests

No need to explain this.

pmd static analysis

pmd provides a plethora of rules for checking Java code against common bugs as well as identify “dubious” constructs that should be reconsidered. pmd is not always right (pmd#1316, pmd#1251, pmd#1285), but it quite often is.

checkstyle analysis

I prefer to use pmd for static analysis. checkstyle is introduced here for the remaining aspects. It checks only the star-imports, code style and formatting. Checking the code style, however, is very valuable. It ensures that there a single consistent coding style to read and write.

Note that the checkstyle configuration doubles as IDE configuration for the coding style, which allows for easy set-up on most IDEs.

SpotBugs analysis

SpotBugs, like pmd, performs a static analysis of the source code. There is some overlap between pmd and SpotBugs, but that is not a problem. In addition, SpotBugs offers interesting features to extend the capabilities of static analysis that I intend to use in the future.

Future improvements

Current analysis capabilities have proven useful. There are more options available.

Re-evaluate and enable existing exclusions/disabled rules

Some of the disabled rules are good and valuable, but could not be enabled as it would fail the build. These checks should be re-evaluated at a later time and their findings corrected.

SpotBugs-annotations for extended static analysis, resource management

SpotBugs offers annotations. Whenever these annotations are used, SpotBugs is able to extend its analysis to a larger scope. Especially the annotations that support resource management are interesting, due to the fact that we have an interest in clearing sensitive data after use. (This also fits perfectly with otr4j’s goal to encapsulate sensitive data and corresponding operations.)

Note that it would be interesting to know if SpotBugs annotations are supported by IntelliJ and other IDEs. Right now, otr4j uses JSR-305 annotations for null-analysis. These can be replaced, but I would prefer to not trade-off one kind of analysis for another.

Animal-sniffer, for verifying the public API

When we release our first new version of otr4j with the OTRv4 support, it would be good to guard against (unintended) changes to the API.

The animal-sniffer build plug-in can build up a signature for a library and then set up to check at build-time if the signature is still valid. This ensures that minor bug fixes do not break the public API unintentionally. This plug-in is limited to checking syntax only.

In addition to checking the library, it is able to verify the code base against any JDK version’s public API. This helps with ensuring compatibility with a certain minimum supported Java version.

SonarQube

SonarQube has many static analysis capabilities, as well as capabilities to evaluate the structure and design of the library. I am less interested in the former, as there will be significant overlap with pmd and/or SpotBugs. It would be interesting to see the internal structure of the library in order to identify those improvements that are not directly obvious from looking at the code.

Conclusions

All-in-all these build improvements are very nice. They allow focus on the work at hand. There are further improvements possible, as there always are, but these are not urgent. Continuous Integration is not a big win at this point, but it will be beneficial in the future to help with PRs.