JSR 305: a silver bullet or not a bullet at all?

on Mar 30, 09 • by Mikhail Ksenzov • with 9 Comments

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...

Home » Static Analysis » JSR 305: a silver bullet or not a bullet at all?

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:

  1. 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.
  2. During the analysis of ‘isComplete’, the tool records the fact that the incoming argument ‘spec’ is dereferenced.
  3. 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.

Related Posts

9 Responses to JSR 305: a silver bullet or not a bullet at all?

  1. Adam Gent says:

    I think the real benefit to having something like @NonNull is better documentation.
    Annotations can be included in Java Doc.
    I can’t tell you how many times have written java doc stating whether or not parameters can be null or whether the return value will be null.

    @NonNull would provide a consistent way of documenting this.
    Otherwise if you are looking for better static analysis perhaps a language such as Scala or F# would be better suited. Typing final in-front of every object is just to much of a pain in the butt.

  2. Andreas Krey says:

    It is quite funny: The exact arguments against @nonnull could also be used against specifying the parameter type in methods (and elsewhere). Those can just as well be inferred by global analysis. After all, @nonnull only exists because the java designer allowed null to be used in places where an Whatever is expected even though null clearly does not honor the contract of any Whatever. When allowed nullness of a value would have to be declared, NullPointerException could easily be a checked exception.

  3. I completely disagree with your suggestion that whole program analysis can take the place of annotations. Whole program analysis cannot reason about JNI code or code that is provided at run time, and it cannot say anything about future changes. Furthermore, any automatic analysis is only a conservative guess, whereas explicit annotations are a contract. Having a contract makes it a lot easier for the compiler to check – doing full-program analysis is most certainly a huge task in the presence of large libraries. I look forward to having @Nonnull available (especially since they added the @Default annotation).

  4. Mikhail Ksenzov says:

    Thank you for your comments guys. Here is a couple of thoughts:

    About maintenance cost:

    I tend to agree that it is a good idea to invest time and effort in automated testing (such as junit tests) or automated verification (such as adding some metadata with annotations). However it makes sense only in the cases when the testing and verification are the only way to check your code. There is no reason to add tests for the defects which would be caught by compiler. You can think of the static analysis tool as a compiler on steroids. Most NPEs can be detected by modern tools w/o any human effort, if the tools are doing their jobs.

    In the following posts I would like to show when using the annotations for defect detection makes sense and where it does not.

    About final values:

    Even if you declare all the variables and parameters as final ones it would not fix the NPE problems in the snippets above. Saying that I think that using final values where possible is the good practice, though totally unrelated to the blog post.

  5. Ex Boyfriend says:

    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.

  6. 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 ;-)

  7. Mikko Kauppila says:

    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.

Leave a Reply

Your email address will not be published. Required fields are marked *

*

You may use these HTML tags and attributes: <a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <strike> <strong>

Scroll to top