Java: @Nonnull used with boxed primitive type

Clean Code New rules / language support
1

Consider a Java field declaration like this:

@Nonnull private Integer id;

That is inefficient, since there is no need for a boxed primitive type here if we are also saying the field cannot be null. The boxing will just slow things down, consume more memory, increase load on garbage collector, etc.

So, my suggestion is, a rule that flags use of @Nonnull (and equivalent annotations such as @NotNull and @NonNull) on a field/parameter/return type with a boxed primitive (Boolean, Byte, Short, Character, Integer, Long, Float, Double). In most of those cases, one wants to use the unboxed primitive type instead.

(There are some exceptions; e.g., sometimes implementing a generic interface, or extending from a generic superclass, forces you to use a boxed instead of unboxed type, and in that case annotating it as @Nonnull can still be useful – to avoid false positives in those cases, it might make sense to exclude the rule from applying to inherited/overridden methods.)

2

There is a rule RSPEC-4682 “@CheckForNull” or “@Nullable” should not be used on primitive types. It would be nice to just extend it :slight_smile:

4

@agabrys I believe the purpose and fix for the rule is slightly different : one is about useless annotation. The suggested rule is about changing the type of the field to primitive.

@skissane Before we make the move to transform this suggestion in a dedicated rule, may I ask if you encountered this somewhere in a real code base ?
I am a bit worried about raising many false positives for cases where the annotation is here for validation of beans after being filled with value from DB after instanciation. (and in this kind of cases it might make sense to have boxed value : null is absent which indicates an anomaly).

1 Like
5

I have encountered this on method parameter declarations.

6

@bduderstadt can you post an example of such usage? I am not sure I follow what do you mean.

7

It was something like

public void foo(
    @NonNull String aString, 
    @NonNull MyThing anObject, 
    @NonNull Integer aNumber) {
    // ...
}

I assume the author was not familiar with the differences between primitives and boxed types. There was no need for a @NonNull Integer in the method, it should have been int. So the code should have been:

public void foo(
    @NonNull String aString,
    @NonNull MyThing anObject,
    int aNumber) {
   // ...
}
8

@bduderstadt I believe this still holds true. Maybe we could implement this rule but not activate it by default as it may be controversial in some code.

9

I see what you mean. It might be true, because these Nonnull annotations are not very standardized and serve very different purposes. Some are merely informational, some give a warning in the IDE when you try to assign null, others produce an NPE at runtime. I’d prefer that a Nonnull annotation should indicate an invariant. After instantiation, the annotations should always hold and you should be able to rely on them. And if data from the database is loaded into the fields, why not fail early with an NPE right away? What’s the use of carrying around data that violates its annotations?

However, my point of view seems to be in contrast to the use case of validators like http://hibernate.org/validator/

Perhaps it would make sense to differentiate here and allow certain annotations as exception?
So using Hibernate Validators @NotNull annotation with boxed types would be compliant, because it’s for bean validation, but using others, e.g. Lombok’s @NonNull with boxed types should trigger the suggestion to use a primitive. Does that sound reasonable?

10

To be honest, this sounds like maintenance hell. I am pretty sure that once this is implemented we’ll get a thread on this very forum with some opinions/arguments going against this.

2 Likes