leangen / geantyref

Advanced generic type reflection library with support for working with AnnotatedTypes (for Java 8+)
Apache License 2.0
99 stars 15 forks source link

Current trunk does not build for JDK 11 and has test failures for JDK 8 #25

Closed hgschmie closed 7 months ago

hgschmie commented 7 months ago

With JDK 11:

[ERROR] /Users/henning/oss/geantyref/src/main/java/io/leangen/geantyref/AnnotatedArrayTypeImpl.java:[13,1] io.leangen.geantyref.AnnotatedArrayTypeImpl is not abstract and does not override abstract method getAnnotatedOwnerType() in java.lang.reflect.AnnotatedArrayType
[ERROR] /Users/henning/oss/geantyref/src/main/java/io/leangen/geantyref/AnnotatedTypeVariableImpl.java:[13,1] io.leangen.geantyref.AnnotatedTypeVariableImpl is not abstract and does not override abstract method getAnnotatedOwnerType() in java.lang.reflect.AnnotatedTypeVariable
[ERROR] /Users/henning/oss/geantyref/src/main/java/io/leangen/geantyref/AnnotatedParameterizedTypeImpl.java:[15,1] io.leangen.geantyref.AnnotatedParameterizedTypeImpl is not abstract and does not override abstract method getAnnotatedOwnerType() in java.lang.reflect.AnnotatedParameterizedType
[ERROR] /Users/henning/oss/geantyref/src/main/java/io/leangen/geantyref/AnnotatedWildcardTypeImpl.java:[16,1] io.leangen.geantyref.AnnotatedWildcardTypeImpl is not abstract and does not override abstract method getAnnotatedOwnerType() in java.lang.reflect.AnnotatedWildcardType

With JDK 8, the current code compiles but shows errors:

kaqqao commented 7 months ago

For the time being, I intend to keep this JDK8 compatible, so it has to be built with 8 as the target. But with that, I have no problems building it with JDK17. Not sure why you're having issues.

hgschmie commented 7 months ago

Makes sense. I tried mvn clean install with JDK 8, 11, 17 and 21 and 11-21 all show the errors above.

When I replace the <source>1.8</source> and <target>1.8</target> configuration in the pom with <release>8</release>, then the code does compile with 11, 17 and 21 (but no longer with Java 8, because the java 8 compiler does not have a --release command line option.

With Java 11 and 21, I still see the errors described above. With Java 17 (!), I only see the Issue17 (so the problem with TestIssue20 and TypeFactoryTest do not show up with Java 17)

hgschmie commented 7 months ago

If you are interested, I can get you some github action CI scripts (basically the same that we run for Jdbi) to test for this.

kaqqao commented 7 months ago

Ok, I see the issue with JDK21. Seems like the toString implementation has yet again changed to now use canonical type names instead of the regular names. This logic wasn't touched since Java 5, but now changes every release for whatever reason... At this point, I think it's pointless to try chasing toString compatibility with Java, so I'll drop/rewrite those tests.

kaqqao commented 7 months ago

I don't get the following error with 21 though:

[ERROR] Issue20Test.testRecursiveTypes:60 array lengths differed, expected.length=5 actual.length=2; arrays first differed at element [0]; expected:<interface io.leangen.geantyref.Annotations$A1> but was:<interface io.leangen.geantyref.Annotations$A2>

kaqqao commented 7 months ago

Ah, ok, I get the error above with JDK11. I think this is sort of expected. JDKs before ~15 had quite a few bugs in annotation parsing (you'll find strange mitigations for this all over the codebase). I'm OK with the project requiring JDK17+ to build, as long as JDK8 compatibility is kept for a little longer, but I'm also OK with dropping that soon.

kaqqao commented 7 months ago

Ok, moving the inner interfaces to the upper level was enough of a mitigation to make JDK11 pass as well. So I'll keep it that way.

kaqqao commented 7 months ago

If you are interested, I can get you some github action CI scripts (basically the same that we run for Jdbi) to test for this.

Yes, please, this would indeed be helpful 🙏

kaqqao commented 7 months ago

Fixed this in https://github.com/leangen/geantyref/commit/70fc31fc54b093468a225a0a78d7893c6831284c

JDK11 is the minimum required to build.