JSR-305 is a Java Specification Request intended to improve the effectiveness of static analysis tools operating in Java 5+ environments. The idea here is that one can use special purpose annotations in order to provide static analysis tools with hints regarding the behaviour and side effects of methods.
An example of such annotations can be found in the presentation ‘Annotations for Software Defect Detection’ by William Pugh, who is masterminding the whole spec. Here we go:
1: void test() {
2: if (spec != null) fFragments.add(spec);
3: if (isComplete(spec)) fPreferences.add(spec);
4: }
6:
5: boolean isComplete(AnnotationPreferences spec) {
6: return spec.getColorPreferenceKey() != null
7: && spec.getColorPreferenceValue() != null
8: && spec.getTextPreferenceKey() != null
9: && spec.getOverviewRulerPreferenceKey() != null;
10: }What’s wrong with the snippet above? Well, the check for null on line 2 shows that the developer expects that the value of ‘spec’ can potentially be null, but it is still passed to method ‘isComplete’. Later, ‘isComplete’ attempts to dereference the value, which causes a NullPointerException.
According to Dr. Pugh, the best way to detect this issue statically is to force a developer to add the annotation @Nonnull to the method signature like this:
5: boolean isComplete(@Nonnull AnnotationPreferences spec) {
In this way, a basic static analysis tool that can minimally track ‘spec’ as a potential suspect for ‘null’ can issue a warning when the @Nonnull annotation is contradicted (that is, when ‘spec’ is passed to ‘isComplete’ as a parameter).
There are two problems with this approach:
- it forces the developer to do work that rightly should be performed by a static analysis engine
- it takes time to write the annotation for static analysis, but it takes even more effort to maintain the annotations and the actual code base in a consistent state.
In reality, the proposals behind JSR-305 exist to enable a tool intended for single function analysis (so-called intra-procedural analysis) to act as if it were performing whole program analysis by requiring the developer to state expected behaviour up front (whether or not that behaviour is actually expressed correctly in the developer’s code).
In contrast, this same scenario is supported by a whole program static analysis tool (so-called inter-procedural analysis) without developer intervention:
- First, a complete call-graph of the system is built, and then all the methods are ordered so that called methods are processed prior to callers — such an ordering allows the tool to generate all the necessary information about, in this instance, the method ‘isComplete’ by the time the analysis of the method ‘test’ begins.
- During the analysis of ‘isComplete’, the tool records the fact that the incoming argument ‘spec’ is dereferenced.
- Next, the method ‘test’ is analyzed. In this method, the variable ‘spec’ is checked for null, so it is tracked as a potential suspect for an exception. Using the information generated about ‘isComplete’ the tool can reliably issue a warning on line 3, since it already knows that ‘isComplete’ dereferences the incoming argument.
So that example applies to a simple unconditional dereference scenario. In more complicated cases, Dr. Pugh proposes to use the annotation parameter ‘when’, with one of the following values:
- ALWAYS
- NEVER
- MAYBE
- UNKNOWN
For example: ‘@Nonnull(when=When.NEVER)’ means that a value is always null in the given context.
This specification seems to be a compromise between the amount of information provided by a developer to a static analysis tool and the amount of effort a developer has to put into it, a compromise that does not seem to be a particularly good solution here. First of all, the amount of information provided in such a manner is insufficient to provide accurate analysis, and secondly this seems to be too much work for the developer, especially when this work can be avoided.
Let’s examine how conditional value dereferencing is supported by whole program static analysis tools:
1: void test() {
2: entity.qualifiedName = null;
3: saveName(entity, false);
4: }
5:
6: boolean saveName(Entity entity, boolean qualified) {
7: String name;
8: if (qualified)
9: name = entity.qualifiedName.trim();
10: else
11: name = entity.name.trim()
12:
13: save(name);
14: }In this example, an inter-procedural static analysis tool would first analyze the method ‘saveName’. A good analysis engine should be able to record the fact that this method only dereferences ‘entity.qualifiedName’ if the second parameter, ‘qualified’, is set to ‘true’. This, it would appear, is a deal more detailed than one can practically achieve by adding @Nonnull(when=When.XXX) annotations, even with all the work the annotation implies for the developer.
Next, the method ‘test’ would be analyzed. A good static analysis tool will naturally keep track of ‘entity.qualifiedName’ because of the assignment to ‘null’ on line 2 and its therefore potential use in an exception causing context. However, given that the actual call to ‘saveName’ on line 3 uses ‘false’ as its second argument, such a tool will not issue a warning that would in reality be a false positive, since the knowledge gained from analyzing ‘saveName’ disqualifies any potential warning due to the conditional relationship between arguments.
In summary, JSR-305 proposes a whole roster of interesting ideas for using annotations to enhance static analysis of Java code, and NPE detection seems to be only one aspect of this specification request. In upcoming blog posts, we shall continue the discussion of proposed annotations as well as offering our own ideas about how and when annotations should be used in static analysis.
The tragedy is that Java designers chose to make all variables non-final and nullable by default. This makes inter-procedural analysis difficult since the analyzer cannot assume pretty much anything. Also, classes are non-final by default, which makes inter-procedural analysis even harder (since dynamic dispatch due to inheritance may occur).
I disagree about the maintenance cost associated with annotations. This is akin to saying that unit tests are bad since they need to be updated whenever contracts change. Maintaining the annotations — or unit tests, for that matter — means that you’re managing the correctness of your code, which is only right.
The last snippet (with methods “test” and “saveName”) you present is indeed hard to analyze. However, it can be argued that it duplicates information: whether the entity has a qualified name is a property of the entity itself, and should not be passed as a separate parameter (“boolean qualified”). My humble opinion is that programmers should strive to write code that’s easy to analyze.
Otherwise, your post was good and informative.
I agree with Mikko’s second paragraph: indeed I think the time spent to put annotations and ensure they are consistent is very well spent, as it forces you to be precise with your design. Also, if you adopt standard practices such as using mostly @NotNull (e.g. by using NullObject or SpecialCase patterns) the amount of work is really reduced.
The fact that things in Java are not final by default is no excuse for developers not using the “final” keyword as much as possible ;-)
Not that I’m impressed a lot, but this is a lot more than I expected for when I stumpled upon a link on SU telling that the info is quite decent. Thanks.