Fix null annotations Configuration class

[mherwege]

Identified in review of #5638 (comment)

The Configuration class has incomplete null annotations, leading to wrong conclusions.

This may have an impact on addons compilation, TBC

[holgerfriedrich]

Very much appreciated. I once tried to get #4859 merged, but somehow it got stuck. Changes to configuration class seemed similar.

[mherwege]

But I got stalled on this.

It looks like most of the code assumes the configuration map cannot have null values. So forcing that would make sense. But then I discovered some code that adds null values for configuration parameters with required defaults from config descriptions if not set. So one would have to have these values Nullable. But that's a big undertaking, as you can see. I am not at the end of it in core, and addons will not compile when doing this.

So I am not sure anymore I can reasonably finish this without creating a lot of risk everywhere.

[holgerfriedrich]

I tried the all-in approach, fixing all in core and I also had the PR for add-ons at hand. No chance to have this rebased until it is reviewed 😱

So I would recommend to start with smaller chunks like Configuration (unfortunately it is not really small). Prepare the core PR and the add-ons PR. If you need to many hacks to get add-ons compiling, the approach probably does not work due to different assumptions on both sides. You may get rid of SAT warnings and get compiler warnings instead (or even ugly @ NonNullByDefault({}) annotations).

Maybe you can take parts of my old PR. I am not sure if I always took the right decision, but overall it was at least compiling.

[wborn]

I also tried to add the missing null annotations some time ago and remember it resulted in a lot of conflicts/headaches. 😉

[wborn]

I asked an AI for some additional input on this, since this is a fairly complex topic involving both nullness semantics and compatibility across repositories. The following seems like a potentially workable way to make this incremental.

The main difficulty here seems to be that Configuration currently has two conflicting contracts: at runtime it allows null values, while most consumers effectively treat configuration values as non-null.

Trying to make getProperties() fully correct as Map<String, @Nullable Object> immediately pushes that distinction into every consumer and causes a very large cross-repository change.

A possible way forward would be to make the migration more incremental:

1. Fix the clearly nullable scalar API first. get() can return null for a missing or explicitly null value, while put() and remove() can return null because they return the previous value. Making those return types explicitly @Nullable gives callers useful and accurate nullness information while keeping the impact local to individual value accesses. This should be considerably easier to migrate than immediately changing the generic contract of getProperties() to Map<String, @Nullable Object>.

2. Avoid enabling TYPE_ARGUMENT as part of @NonNullByDefault for this API boundary initially, so existing Map<String, Object> consumers do not all have to change at once. The type arguments can remain unspecified/legacy until those APIs are migrated deliberately.

3. Provide an explicit non-null view/copy for the common case, e.g. getNonNullProperties(), which filters entries with null values. A number of the changes in this PR already effectively do this manually.

4. Keep getProperties() as the exact representation, including nullable values, but avoid tightening its generic null contract immediately. Consumers that actually need to distinguish explicit null values can then be migrated first.

5. Once core and add-ons have been migrated, the generic type annotations can be tightened without needing one large coordinated change.

It may also be worth deciding separately whether Configuration really needs to support an explicit key -> null state at all. ConfigurationNormalizer already treats null largely as "not configured" by either applying a default or removing the property. If no relevant code relies on containsKey(key) == true && get(key) == null, changing put(key, null) to remove the property would simplify the API considerably and allow the internal map to remain Map<String, Object>.

That behavioral change should probably be handled separately though, because the current tests explicitly document that null values are supported.

This seems preferable to propagating Map<String, @Nullable Object> through core and add-ons in one go, while still allowing the API to become fully null-correct incrementally.