Skip to content

Update to NullAway 0.14.0 and fix new warnings - #37188

Open
msridhar wants to merge 1 commit into
spring-projects:mainfrom
msridhar:nullaway-0.14.0
Open

Update to NullAway 0.14.0 and fix new warnings#37188
msridhar wants to merge 1 commit into
spring-projects:mainfrom
msridhar:nullaway-0.14.0

Conversation

@msridhar

Copy link
Copy Markdown
Contributor

Most of the new warnings concern making upper bounds of type variables @Nullable for consistency, and the fixes are straightforward. There is one case where we suppress a warning, as I'm unsure of the desired fix; I will add a more detailed PR comment on that change.

Signed-off-by: Manu Sridharan <msridhar@gmail.com>
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Aug 21, 2026
* @param conversionService the conversion service
* @param targetType the target type
*/
@SuppressWarnings("NullAway") // Retain support for comparators that handle a null conversion result

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is Codex's explanation for this warning suppression. I read through it and it makes sense to me, but I'm not an expert on this code. I'm not sure what is the best fix (that is not a suppression).


The suppression covers a real mismatch between the legacy runtime behavior and the declared generic types.

The constructor adapts a ConversionService into a Converter:

public ConvertingComparator(
        Comparator<T> comparator,
        ConversionService conversionService,
        Class<? extends T> targetType) {

    this(comparator,
            new ConversionServiceConverter<>(conversionService, targetType));
}

The delegated constructor expects:

ConvertingComparator(Comparator<T>, Converter<S, T>)

But ConversionService.convert(...) is explicitly nullable:

<T> @Nullable T convert(@Nullable Object source, Class<T> targetType);

Consequently, the adapter is:

Converter<S, @Nullable T>

NullAway correctly observes that this cannot safely become Converter<S, T>. During comparison, the result is passed directly to the comparator:

T converted = this.converter.convert(source);
return this.comparator.compare(converted1, converted2);

If conversion returns null:

  • A null-aware comparator such as Comparator.nullsFirst(...) handles it correctly.
  • A comparator that does not support null will likely throw NullPointerException.

The existing API allows both possibilities and leaves responsibility with the supplied comparator. That relationship is not accurately represented by the constructor’s Comparator<T> type.

The comment therefore means:

Do not force the conversion result to be non-null, because existing callers may intentionally supply a comparator that supports null.

For example, adding this would satisfy NullAway:

return Objects.requireNonNull(
        this.conversionService.convert(source, this.targetType));

But it would change behavior by rejecting null before a null-aware comparator could process it.

Accurately modeling this would require a larger API redesign, likely requiring this constructor to produce a ConvertingComparator<S, @Nullable T> and accept a comparator whose input type is nullable. Constructors cannot independently change the enclosing class’s type argument, so a static factory or internal class redesign would probably be needed.

Thus, this suppression is:

  • Narrowly scoped to the affected constructor.
  • Preserving established runtime behavior.
  • Not claiming that NullAway is wrong.
  • Documenting that null handling is delegated to the caller-provided comparator.

}

nullability {
nullAwayVersion = "0.14.0"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternately, we could wait for a new nullability plugin release that defaults to 0.14.0. The other changes would remain valid.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: waiting-for-triage An issue we've not yet triaged or decided on

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants