@NonNullByDefault and HashMap

Development Misc
1

This is really just a Java question, but it’s got me stumped, and internet searches are finding the same terms from other languages, so haven’t been helpful. Please let me know if there’s a better, more generic place to ask.

I’m working on a binding. The code style guidelines say all classes need to be defined with @NonNullByDefault. I’m porting some old code which didn’t do this, and adding it adds a bunch of warnings. Most of them are pretty manageable, though, and I’m able to work through them.

One of the ones that’s left me stumped is that when the code uses a HashMap, all the methods on the HashMap also get defined as @NonNull, such as HashMap’s get() method.

This is wrong.

Get() is explicitly documented to return null when an item isn’t found, and all the code that exists in what I’m porting that carefully checks the return of get() for null is flagged as dead code, because get() can’t return null.

Since HashMap is defined in java.util, I can’t easily mark it up with @Nullable.

How do I prevent this warning correctly? I’d rather not just disable it for the whole method, as it’s valuable, but I haven’t figured out a way to annotate a specific spot to say, “No, really, this is (or is not) nullable right here, trust me, I checked.”

Any guidance? Pointers to docs I missed?

Do I need to implement a HashMap that’s marked Nullable somehow? Can I just override “get” to be nullable?

2

You should not override the get() or any other method. It should work by adding the @Nullable annotation to the type of the value:

    private final Map<String, @Nullable String> myMap = new HashMap<>();
3

Annotating the value type of maps with @Nullable is actually a workaround to silence the false positives generated by the compiler. The proper way would be to use Eclipse External Annotations (EEA). These would add the proper information so the compiler can deduce that Map.get(..) can return null values.

See also:

There’s also an openHAB Core issue to start using these:

4

Thanks, both of you! One of these is a work-around that will let me address some warnings, although with a bit of a nasty side effect of allowing nulls in the hash. The other is the Right Way to fix it, and shows that I’m not the only one seeing this, and it’s nothing obvious I’ve missed.

Wouter, it looks like there are maintainers who want to work on this bit that there isn’t consensus on the best way to handle it. I read some of the EEA docs and it wasn’t obviously clear to me how they would be defined, so I don’t feel like I can jump in and offer to help.

I do agree that it’s an issue that would be good to address, and that some of the JRE definitions should probably be defined someplace central for more than just OH to use.

5

Alright. But what should we do as long as EEAs are not integrated? Ignore such warnings? Adding “wrong” annotations just to remove that warnings doesn’t make sense. Does it? I guess adding @SuppressWarnings("null") isn’t an option too.

6

I did use that option when I ran into it back then because it is in line with the Code Style Guidelines:

Code MUST not show any warnings. Warnings that cannot be circumvented should be suppressed by using the @SuppressWarnings annotation.

7

I would be happier with suppressing the warning if I could suppress it for smaller blocks than the whole method. It is what I will do to get free of warnings.

Re-reading the issues on GitHub, I see two possible solutions. One was deemed inappropriate because binding developers would have to have the extra project, and the other stalled for reasons I don’t quite follow.

Having an extra project open would not bother me at all; they were set up by the installer and it would be one more thing I don’t know what it is. If I understood why the other solution stalled, I might offer to help, but I wasn’t clear on the issue there.