JSR-308 : Clarification on Nonnull, ParameterAreNonnullByDefault and the way Eclipse Kepler detect it

Question

I decided to edit my question, after seeing that 1 year after, I've changed how I work with nulls:

• I don't use Eclipse builtin null checks, because I found it rather primitive (and perhaps a little tricky to understand)
• I use @Nullable to tell a value can be null. After all, there should be less null-values than non-null values.
• I'm using Java 8 and I tend to use Optional, thus allowing the following: Optional.ofNullable(value).orElseGet(() -> 1);. It does not beat the ?: and ?. operator of Groovy, but Optional give some nice tool like map, filter, and so on.

And, as for my code:

• constructors checks for nulls using Objects.requireNonNull, like this:

  public Foobar(String a) { this.a = Objects.requireNonNull(a, "a"); }

• methods checks for nulls using Preconditions.checkNotNull from Guava framework whenever I use it in my projects or Objects.requireNonNull:

  public void foobar(String a) { Preconditions.checkNotNull(a, "a"); }

Using the one or the other depends on if I reuse the value.

I don't check for method parameters every time, but rather mostly in the public methods. The idea is not to replace the default runtime check that throws NullPointerException more efficiently than I can do.

---

I am currently using @Nonnull or @Nullable annotation on all my parameters, fields, method result (return), but I'm wondering what is truly the best :

• How could I tell that my field and method result are non null by default ? (the @ParameterAreNonnullByDefault does not work for them). I'd like a portable way (I've read here that I could create my own annotations, with specific names, and that would work for findbugs)
• If I annotate package com.foobar with @ParameterAreNonnullByDefault, does it apply to com.foobar.example as well ?
• Should I check every parameters (I am currently checking constructor parameters) when annotated by @Nonnull ?

Also, since Eclipse 3.8, there is annotation based null checks. But I have problem with some "simple" case :

@ParameterAreNonnullByDefault
class Foobar<E extends Throwable> {
  @Nullable private Constructor<E> one;
  @Nullable private Constructor<E> two;

  public Foobar(Constructor<E> one, @Nullable Constructor<E> two) {
    this.one = Objects.requireNonNull(one, "one");
    this.two = two;
  }

  // don't care about exceptions.
  public E getInstance(String msg, Throwable t) { 
    if (null == two) {
      return (E)one.newInstance(msg).initCause(t);
    } 
    return two.newInstance(msg, t);
  }
}

Why is telling me that two is nullable at that position, and why he is warning me about potential null access to two ?

Answer 1

As far as the warning on the two variable in getInstance is concerned the null analysis is not clever enough to work out that the field cannot be null. You can work around this by using a local variable:

public E getInstance(String msg, Throwable t) { 
  final Constructor<E> localTwo = two;
  if (null == localTwo) {
    return (E)one.newInstance(msg).initCause(t);
  }
  return localTwo.newInstance(msg, t);
}

There is a setting Enable syntactic null analysis for fields in Preferences > Java > Compiler > Errors/Warnings > Null analysis which allows code like:

if (two != null) {
  return two.newInstance(msg, t);
}

without a warning.

Answer 2

Here's how all your requests can be realized using Eclipse:

Non null by default for fields and method returns

Since Eclipse Kepler, the @NonNullByDefault annotation affects all these locations. Starting with Luna, using Java 8 and type annotations, the effect can be further fine tuned.

As you ask about a portable solution: it's not difficult to use the Eclipse compiler also in CI builds, see the JDT FAQ, and even IntelliJ can be told to use the Eclipse compiler.

Per-Package defaults

Java does not have a concept of sub-packages. If package names share a common prefix this has no semantic implications. Hence, sadly, each package has to be annotated individually.

Once you are ready to apply the default to all packages, Eclipse can be told to warn you / or even raise an error if you missed a default on a package. The option is called: "Missing '@NonNullByDefault' annotation on package".

Runtime checks

In an ideal world, all @NonNull arguments will be fully checked by the compiler and you would not need any runtime checks. It's a matter of how much safety you need, to add these checks, but if you do, I don't see much reason for treating constructor arguments differently from method arguments.

Perhaps, if you have final @NonNull fields, constructors have special responsibility and deserve more checks, but that's basically a matter of your code style and safety requirements.

Unexpected warning in Eclipse

Eclipse performs full flow analysis only for local variables. It is carefully designed this way in order to protect you from the risks of side-effects, aliasing and concurrency, each of which could cause NPE despite the prior null check. The so-called "syntactic analysis" for fields has been introduced as a (limited) compromise, but the real solution must be, to fetch the value into a local variable, where it can be fully analysed. Eclipse even offers a quick fix for this change.

Answer 3

I'll give answers to your questions, using an external tool, the Checker Framework's Nullness Checker. It has an Eclipse plug-in and it reads the annotations used by Eclipse and FindBugs. Its analysis is better than the built-in Eclipse nullness analysis, but its IDE integration is not as nice as Eclipse's. Both are worthwhile tools, and you can decide which one is best for you.

How could I tell that my field and method result are non null by default?

That is the default. You don't have to do anything.

If I annotate package com.foobar with @ParameterAreNonnullByDefault, does it apply to com.foobar.example as well?

You don't have to change the defaults at all (though you can, if for some reason you want to use @Nullable as the default in some or all code locations).

Should I check every parameters (I am currently checking constructor parameters) when annotated by @Nonnull ?

You only need to write run-time checks if you are worried that the nullness analysis is unsound — that is, the analysis gives no error even though your code can throw a null pointer exception — or if you are not running the analysis on your entire program. Both Eclipse's and FindBugs's nullness analyses are unsound by design. The Checker Framework is sound by design. Of course, any tool may contain bugs.

Also, since Eclipse 3.8, there is annotation based null checks. But I have problem with some "simple" case:

The Checker Framework handles this simple case precisely -- exactly as you wish it to.

More specifically, the Checker Framework does so when it is checking single-threaded code.

If your code is multi-threaded, then you can supply the -AconcurrentSemantics command-line option. In this case, the Checker Framework will issue a warning because another thread might reset two between the check and the use. If your code is multi-threaded and other threads are modifying fields concurrently, then you probably want to use locks. Copying the value into a local variable, as suggested in another answer, is a bad idea. Not only does it degrades code readability, but it doesn't necessarily behave as you wish; for example, the other thread could violate representation invariants and cause your code to crash in other ways.