Even the best developers make simple mistakes from time to time and I welcome any tool that minimizes the risk of introducing new bugs to a system. I recently browsed through Google research papers [1] and noticed a couple of articles referencing a tool called Error Prone [2]. Interested to find out more, I took the tool to a test round, and found it rather useful.
What is Error Prone
All Java developers should be familiar with the Java programming language compiler a.k.a javac. It reads class and interface definitions and produces bytecode class files. If syntax or static type checking has issues, a compilation error is raised. Error Prone does all this, but in addition to this it checks for common mistakes that could be present in the source code and forces them to be fixed at compile time. It does this without interfering a developer’s basic development cycle and without any significant performance impact (9.0% time overhead [3]).
How Does it Work?
Since the release of JSR 199 (Java Compiler API) it has been possible to compile Java inside a Java program. The different compilation phases are exposed in JavaCompiler class and Error Prone actually extends this class in order to insert hooks into the compilation pipeline. To be more specific, it adds custom error checking after the flow phase of the default compiler. It uses AST (Abstract Syntax Tree) related APIs such as TreeScanner to visit the AST nodes and with each node, it runs proper validation code depending on the node type.
Below is an example of a bug checker that ensures that the codebase does not contain a division by integer literal zero:
What Kind of Errors Are Actually Caught?
Instead of writing a sample code snippet myself, I think it is more interesting to see what Error Prone finds in real software projects. I picked Elasticsearch [4] as it is an active open source project with a large contributor pool. A cloned the project and replaced the compiler configuration to the following in the pom.xml in order to active Error Prone:
After that I ran ”mvn clean compile” and the following output was generated:
https://gist.github.com/sebastianmonte/2e41126958fe1a76687d#file-output
From the output we can see that 8 warnings and 13 errors exist and that the code did not compile successfully (it compiles fine without Error Prone). Warnings are not as severe as errors and they do not result in compilattion failure.
Let’s look at the first error in detail:
https://gist.github.com/sebastianmonte/707b4afd4d00245e8c02#file-checkreturnvalue
The error states that we are not checking the return value of a method that has been annotated with a @CheckReturnValue annotation (in this case the method ’removalListener’). As stated in JSR 305, ignoring the return value of such a method is likely incorrect.
The javadoc from CacheBuilder.removalListener states clearly that the method should not be used as it has been used in Elasticsearch:
”Warning: after invoking this method, do not continue to use this cache builder reference; instead use the reference this method returns. At runtime, these point to the same instance, but only the returned reference has the correct generic type information so as to ensure type safety. For best results, use the standard method-chaining idiom illustrated in the class documentation above, configuring a builder and building your cache in a single statement. Failure to heed this advice can result in a ClassCastException being thrown by a cache operation at some undefined point in the future.” [5]
Error Prone suggests a fix for the error which we should apply to the code so the code takes the following form:
We could even shorten this fix to a one liner:
Once that is out of the way, let’s take a look at another error:
https://gist.github.com/sebastianmonte/6b2c9a723052b434ec17#file-staticaccessedfrominstanc
The above error is a common one in my opinion and I have seen it in many code bases. It does not break anything, but it makes the code unclear about what is actually a static class method or constant. Again, a fix is suggested and easily fixed with the following code:
It is not useful for me to go over all the errors found. A full list of the errors detectable by Error Prone can be found at ”Bug patterns detected” page [6]. I’m always happy to give something back to the community, so I created a pull request to Elasticsearch that fixes all the reported errors [7].
In addition to using existing bug patterns, the process of adding new ones is quite straightforward. Error Prone is available in Github [8] so if you feel that there is a bug pattern missing, contributions seem to be welcome! There seems to be only upsides in using Error Prone, so I see no reason why this shouldn’t be the de facto standard of compiling Java code.
[1] http://research.google.com/pubs/papers.html
[2] http://errorprone.info/
[3] http://static.googleusercontent.com/media/research.google.com/en//pubs/archive/38275.pdf
[4] https://github.com/elasticsearch/elasticsearch
[5] http://docs.guava-libraries.googlecode.com/git-history/release/javadoc/com/google/common/cache/CacheBuilder.html#removalListener(com.google.common.cache.RemovalListener)
[6] http://errorprone.info/bugpatterns
[7] https://github.com/elastic/elasticsearch/pull/9817
[8] https://github.com/google/error-prone
