Comment From: wilkinsona

Thanks for the proposal, @nosan. I see you've expanded the JavaVersion enum. That's a good thing to do but I think it should be tackled separately. I've opened https://github.com/spring-projects/spring-boot/issues/17565.

I'm not sure about the move to using reflection here. Our current approach feels cleaner to me, but let's see what the rest of the team thinks. Was there a reason for the change, perhaps because there isn't a class that can be used for the new versions that you've also proposed?

Comment From: philwebb

It looks like Runtime.version() was added in Java 9 so I guess the call isn't available to us without using reflection.

Comment From: wilkinsona

Yeah, we have to use reflection if we want to use it. I'm just not sure that we should. Furthermore, the major() method that this proposal calls reflectively was deprecated in Java 10. feature() is the recommended replacement there.

Comment From: philwebb

I was trying to find a new class that was added in 10 and is in the java.base module so we know it'll always be there. I didn't have a lot of luck. I think using the Runtime.version() method is probably our only option going forward if we want to offer all versions in the enum.

Comment From: nosan

Thanks, @wilkinsona

I have not found a new class that was added in java 10. I believe that JavaVersion enum should be dependent on java.lang.Runtime$Version instead of Class.isPresent(...).

PR has been updated to use either major or feature methods.

Comment From: dreis2211

@philwebb & @nosan If one can trust the following JDK API Diff reports JDK 10 could be handled via java.lang.invoke.ClassSpecializer$Factory I guess. 11 (java.lang.invoke.ConstantBootstraps) and 12 (java.lang.invoke.TypeDescriptor) are also fine, but 13 is missing a new class so far you could check for.

Alternatively, one could check for new methods in very common java.lang classes. It would still involve the reflection ecosystem in the end but less obvious and without invoking the reflective method. E.g.:

JDK Method Note
JDK 8 ClassUtils.hasMethod(Optional.class, "empty")
JDK 9 ClassUtils.hasMethod(Optional.class, "stream")
JDK 10 ClassUtils.hasMethod(Optional.class, "orElseThrow")
JDK 11 ClassUtils.hasMethod(String.class, "strip")
JDK 12 ClassUtils.hasMethod(String.class, "describeConstable")
JDK 13 ClassUtils.hasMethod(String.class, "stripIndent") Deprecated but works

With that JavaVersion would stay mainly unchanged (apart from new enums maybe), .e.g:

    /**
     * Java 1.8.
     */
    EIGHT("1.8", Optional.class, "empty"),

    /**
     * Java 1.9.
     */
    NINE("1.9", Optional.class, "stream");

    private final String name;

    private final boolean available;

    JavaVersion(String name, Class<?> clazz, String methodName) {
        this.name = name;
        this.available = ClassUtils.hasMethod(clazz, methodName);
    }

Obviously, one could choose also methods with parameters with a tiny addition to the constructor. I was just a tiny bit lazy ;-)

Comment From: nosan

Thank you, @dreis2211

looks pretty good 👍

Comment From: dreis2211

Let's see what the team thinks :)

Comment From: philwebb

Thanks for the PR @nosan. I'm going to close this one in favor of the method suggested by @dreis2211. We'll track this in #17565