New Feature: @NonNull on parameter? Null check!

[Reinier Zwitserloot]

We just pushed to master (and released on edge: http://projectlombok.org/download-edge.html ) the following new feature:

You can now put @lombok.NonNull on any method / constructor parameter. If it's there, we will inject a nullcheck unless you wrote one yourself. Careful: If you have explicit this() / super() calls, we will insert the check after your explicit constructor call.

The null check is unchanged from @Data and such:

if (paramName == null) throw new NullPointerException("paramName");

There are no configurable options for @NonNull (it'll always be an NPE, and the message will always be the param name).

(This closes feature request issue 514: https://code.google.com/p/projectlombok/issues/detail?id=514 ).

[Fabrizio Giudici]

On Fri, 31 May 2013 01:08:42 +0200, Reinier Zwitserloot
<rein...@gmail.com> wrote:

> You can now put @lombok.NonNull on any method / constructor parameter.
> If it's there, we will inject a nullcheck unless you wrote one yourself.
> Careful: If you have explicit this() / super() calls, we will insert the
> check after your explicit constructor call.

Perhaps it's because I just woke up... but wasn't this feature already
present with javax.annotation.NotNull?

--
Fabrizio Giudici - Java Architect @ Tidalwave s.a.s.
"We make Java work. Everywhere."
http://tidalwave.it/fabrizio/blog - fabrizio...@tidalwave.it

[Leon Blakey]

IIRC javax.annotation.NotNull only a hint for IDE's, not a language level feature

--
You received this message because you are subscribed to the Google Groups "Project Lombok" group.
To unsubscribe from this group and stop receiving emails from it, send an email to project-lombok+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[Fabrizio Giudici]

On Fri, 31 May 2013 10:14:41 +0200, Leon Blakey <lord.qu...@gmail.com>
wrote:

> IIRC javax.annotation.NotNull only a hint for IDE's, not a language level
> feature

Sure, but Lombok enforces it with @Setter, for instance:

import javax.annotation.Nonnull;
import lombok.Setter;

public class App {
@Setter @Nonnull
public String value;

public static void main(String[] args) {
System.out.println("Hello World!");
new App().setValue(null);
}
}

[exec:exec]
Hello World!
Exception in thread "main" java.lang.NullPointerException: value
at it.tidalwave.thesefoolishthings.lomboktest.App.setValue(App.java)
at it.tidalwave.thesefoolishthings.lomboktest.App.main(App.java:18)

In fact I seem to recall a discussion about NullPointerException vs
IllegalArgumentException (I was a supporter of the latter) already
happening here in the past.

So, the problem was that I previously understood only half of the
Reinier's message, and I associated it with setters. Ok, so Lombok is
extending this behaviour to any method with annotated params, which is
good news. But why only with a lombok.* annotation, and not with JSR 305?
Or does the new feature work with JSR 305 too?

[Fabrizio Giudici]

I've received a private follow up on this - I don't know whether it's
intentional or not (sometimes we hit the wrong reply button). In any case,
without quoting the original author, I'm resuming the point and making my
observations.

Point #1: introducing a check for every param with @NonNull might be
unexpected
Point #2: if one wants to have both FindBugs and Lombok checks, he has to
write @NotNull @NonNull each time.

IMHO this is bad. And there's definitely an inconsistency between the use
of the two annotations.

Let's focus on the "unexpected by most users". Why? If you pass null to a
@NonNull method, it's a mistake (Findbugs signals it with a high priority
warning). The point is that if I start without Lombok with code such as:

@NonNull private String foo;

public void setFoo (String foo) { this.foo = foo; }

and by introducing Lombok I refactor to:

@NonNull @Setter private String foo;

any call setFoo(null) will throw NPE, while previously they worked. Is it
unexpected? Probably yes. But now, what? It's not a bug introduced by
Lombok, it's just that Lombok makes evident a previous bug of mine: if I
declared @NonNull on a field that can be null, either I was wrong with the
annotation, so I have to remove it, or I have a bug.

The only valid complain I can see is about performance - e.g. introducing
useless checks in private methods that could be called often. But if we
just used a common approach, that is to check and throw NPE in public
methods, and use assert in private methods, most of the performance stuff
should be ok.

To me, this is not a secondary point.

[Reinier Zwitserloot]

@NotNull is also a hibernate thing, and treating it as nonnull is the wrong action.

We can expand the list to more known "they actually mean what we think it means" annotations, though.

So, we need a list of full package name and exact desired behavior in the form:

1. @Data and co: copy annotation to setter / constructor parameter and getter method. Add null check. Treat as "required" for @RequiredArgsConstructor. (Alternative: as above but don't copy annotation)

2. Inject null check if present on method or constructor parameter.

Wasn't the jsr305 annotation some really silly design such as @Nullable(when=Nullity.NEVER) or am I misremembering?