Modernize codebase with Java improvements

[gnodet]

This PR includes several improvements to modernize the codebase:

1. Replace IllegalArgumentException with ClassCastException for type cast failures
2. Replace IllegalArgumentException with NullPointerException for null checks
3. Use modern Java collections API (toList() instead of collect(Collectors.toList()))
4. Replace custom null checks with Objects.requireNonNull
5. Add @nonnull and @nullable annotations throughout the codebase

These changes make the code more maintainable, safer, and aligned with modern Java practices.

[kwin]

Regarding point 2. (and the related point 4.): I consider the IAE more concise than an NPE in case a method argument is passed with null (although not allowed). Compare also with https://discuss.kotlinlang.org/t/why-does-requirenotnull-throw-an-illegalargumentexception/7617/4

[desruisseaux]

I think that the main argument in favour of NullPointerException is consistency. Not all methods perform explicit null checks. Especially since JEP 358: Helpful NullPointerExceptions provides even more helpful messages than many explicit null checks. Therefore, if Objects.requireNonNull was throwing IllegalArgumentException for a null argument, we would be at risk of having sometime an IllegalArgumentException, and sometime a NullPointerException, depending on whether the method chooses to perform explicit null checks or not. It would also be a risk of compatibility break if the method implementation changes its strategy over time.

[Pankraz76]

• Use modern Java collections API (toList() instead of collect(Collectors.toList()))

Hi folks, please check out attached openrewrite link as its related and some can be done by computer.

This could be supplemented in dedicated PR as there is a migration for this:

• https://docs.openrewrite.org/recipes/staticanalysis/replacestreamtolistwithcollect

Maybe we can team up with as its kind of related. The migration is "free" as done by one click so we can reproduce anytime.

• https://docs.openrewrite.org/recipes/java/migrate/upgradetojava21
• [POC] Migrate to Java 21 - code #2281

[gnodet]

its nice to fix this once, but without a computer checking and enforcing this by some tool like checkstyle or rewrite, its not completed.

I don't see where it's done in your PR as it only provides the result of executing the transformation.

[Pankraz76]

Current State of NullPointerException Handling

The current implementation around NullPointerException is not ideal, but in context, it's clear what's happening.

Options:

1. Keep it as-is
   • Works but lacks elegance.
2. Remove all such checks
   • Simplifies but loses explicit null-check messaging.
3. Refactor into a dedicated layer
   • Separate the "cannot be null" concern to avoid:
     • DRY violations (repetition)
     • Feature envy (fluff and copy-paste)

---

Proposal: Dedicated Logging Layer

If improved logging is desired, this concern should be moved to a dedicated utility layer, similar to Apache Maven's approach:

maven/impl/maven-impl/src/main/java/org/apache/maven/impl/ImplUtils.java

Line 37 in d65c375

throw new IllegalArgumentException(name + " cannot be null");

Benefits:

• Contextual clarity: Right now, the check is just a short "non-null" hint.
• Better debugging: Explicit messaging improves NullPointerException readability.
• Consistency: Centralized handling avoids duplication.

This change is already reflected in PR #2290.

---

[Pankraz76]

Current State of NullPointerException Handling

The current implementation around NullPointerException is not ideal, but in context, it's clear what's happening.

Options:

1. Keep it as-is
   • Works but lacks elegance.
2. Remove all such checks
   • Simplifies but loses explicit null-check messaging.
3. Refactor into a dedicated layer
   • Separate the "cannot be null" concern to avoid:
     • DRY violations (repetition)
     • Feature envy (fluff and copy-paste)

---

Proposal: Dedicated Logging Layer

If improved logging is desired, this concern should be moved to a dedicated utility layer, similar to Apache Maven's approach:

maven/impl/maven-impl/src/main/java/org/apache/maven/impl/ImplUtils.java

Line 37 in d65c375

throw new IllegalArgumentException(name + " cannot be null");

Benefits:

• Contextual clarity: Right now, the check is just a short "non-null" hint.
• Better debugging: Explicit messaging improves NullPointerException readability.
• Consistency: Centralized handling avoids duplication.

This change is already reflected in PR #2290.

---

[gnodet]

@Pankraz76 I'm not sure what you aim for with your PRs. How do they differ from this one ? It's fine very fine that you used openrewrite rules, but isn't the result the same at the end ?

[elharo]

"Use modern Java collections API (toList() instead of collect(Collectors.toList()))" is riskier than the other changes because of the move of mutable to immutable lists. Nine times out of ten this is fine. The tenth time it's a bear to debug. These changes should be pulled out into a separate PR.

The rest could be split, but maybe don't have to be. Nuulable and Nonnull might also call for a separate PR.

[Pankraz76]

Regarding point 2. (and the related point 4.): I consider the IAE more concise than an NPE in case a method argument is passed with null (although not allowed). Compare also with https://discuss.kotlinlang.org/t/why-does-requirenotnull-throw-an-illegalargumentexception/7617/4

Java and Lombok prefer NPE as well.

https://docs.oracle.com/javase/8/docs/api/java/util/Objects.html#requireNonNull-T-

https://projectlombok.org/features/NonNull

both valid points. Ideally its just one config-switch to adjust:

[gnodet]

There 's no plan to use Lombok at this point.

[Pankraz76]

this is dedicated in:

• [chore] Modernize codebase with Java improvements - Replace custom null checks with Objects.requireNonNull #2290

if possible lets merge this first as this is kind of huge.