Fix ActionHandler nullness annotation

[Nadahar]

This started with #4922 (comment) while I was looking for the cause for #4965.

In short, results are inserted into the "result map" of ScriptActionHandler.execute() even if the result in null. This conflicts with the null annotation in the method signature, which is inherited from ActionHandler.execute(). @florian-h05 claimed that the null value mean to be inserted, and after some "investigation" I agree. The map stores the "outputs" from the Action, and the output should be included even if the value is null. This means that the nullness annotations are wrong.

This leads to incorrect warnings for nullness, but that's not the main reason I think it's worth fixing. nulls in Maps in Java is "problematic" because it's not supported by all Map implementations. Popular implementations like ConcurrentHashMap doesn't support it, moreover it can easily be misinterpreted because map.get() returns null for these entries, just like it does for non-existing keys. "Popular functions" like computeIfAbsent() can't handle it either, and Maps with nulls are probably generally difficult to use with streams. More details about this can be found here.

As a consequence, we rarely see null in Maps in Java these days. It's just too many pitfalls, so it is usually avoided. This is a legitimate case for storing nulls in Maps, but the combination of the wrong annotation and the fact that we rarely see it, means that chances are great that a developer will assume that nulls won't be encountered when dealing with these Maps. I think not correcting the annotation is "a bug waiting to happen".

Unfortunately, correcting the annotation leads to changes in quite a few files, but fortunately, not a lot of code actually interacts with these Maps.

I have built all add-ons against a core with the modified annotations, and two add-ons must also have their annotations updated. I will create a separate PR with these changes.

[Nadahar]

"Companion PR" for add-ons created: openhab/openhab-addons#19230

[holgerfriedrich]

This currently contradicts the approach I tried in #4859, see may comment in the description.
#4859 regards all map entries as NonNull.

I was before 5.0 when I came up with this approach, and I did not have time recently to finish it.

[Nadahar]

@holgerfriedrich I'm not sure that I understand how you think it contradicts. I absolutely agree that "the normal" is that all map entries are non-null, but in this case they aren't, and it has a legitimate reason. Things work as the code is today, null values are used, but the annotation claims otherwise, with leads to warnings and potential "developer confusion".

The only alternative I can think of is to use something like Optional instead, but that's a lot of overhead just to be allowed to keep it "in line" with a practice. This goes out the REST API, so it would also have to be converted to actual nulls in the DTO.

[florian-h05]

Thanks for correcting the annotations according to the JavaDoc and the code.

[kaikreuzer]

@holgerfriedrich Is this ok to merge? I am also not sure where you see a contradiction to your work in #4859 - after all, here, the values can be null, so the nullable annotations will anyhow be required, won't they?

[holgerfriedrich]

@kaikreuzer I think it should be fine to merge. I seem to have mixed this up with property maps I tried to force to non-null in my PR.