Address MinAndMax generics warnings

[javanna]

The MinAndMax encapsulates min and max values for a shard. It uses generics to make sure that the values are of the same type and are also comparable. Though there are warnings whenever this class is currently used, which are addressed with this commit.

Relates to #49092

[elasticmachine]

Pinging @elastic/es-search (:Search/Search)

[jpountz]

Some part of the change makes sense to me, like replacing Comparable with Comparable<T> but I wonder that some other changes are not needed like replacing T extends Comparable<? super T> with T extends Comparable<T> as the former is usually the right way to use generics with wildcards that need to extend Comparable.

[jpountz]

should it be a Function<byte[], ? extends T> to avoid the unchecked cast?

[javanna]

great point, I missed this

[jpountz]

In general this is the right way to use generics with Comparable?

[javanna]

thanks @jpountz for your feedback.

When I was working on this change I asked myself even if generics were really useful in this case.
I think the main benefit of generics in MinAndMax is to enforce that the two values (min and max) are of the same type. Currently we don't though as we have unsafe casts whenever we create an instance of MinAndMax. I modified the logic wherever we create an instance of MinAndMax so that we are more explicit and we always create MinAndMax safely.

I agree with you, T extends Comparable<? super T> is usually the way to go and gives more freedom to users, as what we need is a Comparable that does not necessarily has to only compare to self, it can compare to anything really. That though causes problems as we end up having to create a Comparator<MinAndMax<?>> and the following does not compile unless I replace T extends Comparable<? super T> with T extends Comparable<T>:

Comparator<MinAndMax<?>> cmp = order == SortOrder.ASC  ?
            Comparator.comparing(MinAndMax::getMin) : Comparator.comparing(MinAndMax::getMax);

At this point I wonder if all the flexibility given by <? super T> is needed. This is internal code, and we only ever instantiate this class providing either Long, Integer, Float, Double, String or BytesRef which are all comparable to self only.

[javanna]

run elasticsearch-ci/2

[javanna]

turns out that the compile errors that I was seeing are shown only in IntelliJ. They were weird so it is not surprising that this compiles from command line. I did not understand yet what's causing the error in IntelliJ either, I use the same compiler, and the project itself compiles but when I open MinAndMax there's a line that's marked error where a generics error is shown (it is not a warning).

I think this is ready then, I went back to <T extends Comparable<? super T>> which I agree is the best way to go.

[javanna]

run elasticsearch-ci/bwc

[javanna]

Build is green, this should be finally ready ;)

[jpountz]

Thanks Luca, this LGTM.

[javanna]

thank you very much @jpountz ;)