search

spring-io / spring-javaformat

Apache License 2.0
795 stars 110 forks source link

Compatibility with Checkstyle 10 #395

Closed wilkinsona closed 8 months ago

wilkinsona commented 9 months ago

Java Format's Checkstyle extensions are not currently fully compatible with Checkstyle 10 due to some breaking API and behavioral changes in Checkstyle. We should try to improve our compatibility with Checkstyle 10 while, since it requires Java 11, also maintaining compatibility with Checkstyle 9 for the benefit of projects that are still being built with Java 8.

wilkinsona commented 9 months ago

https://github.com/checkstyle/checkstyle/issues/5337 breaks the loading of our checks. They're no longer instances of AutomaticBean so we don't call contextualize and configure on them. This leaves them in an unconfigured state and causes subsequent failures. We can't depend directly on AbstractAutomaticBean (the replacement for the deprecated AutomaticBean) as it is only available in Checkstyle 10.9.3+. We should be able to use Contextualizable and Configurable instead which both classes implement.

wilkinsona commented 9 months ago

Checkstyle 10.3 contains a breaking change to the AnnotationUtil API with the signature of containsAnnotation changing from containsAnnotation(DetailAST ast, List<String> annotations) to containsAnnotation(DetailAST ast, Set<String> annotations). This may improve in the future but to maximise compatibility we can catch the NoSuchMethodError that's thrown when calling the List<String> variant that we have compiled against and try calling the Set<String> variant instead.

wilkinsona commented 8 months ago

containsAnnotation(DetailAST ast, List<String> annotations) is not going to be reintroduced so we'll need to accommodate this difference, irrespective of the version of Checkstyle that's in use.

wilkinsona commented 8 months ago

Since 10.3.2, AnnotationUtil.containsAnnotation no longer matches when given a simple name and the AST contains a fully-qualified name. This change in behavior was introduced in https://github.com/checkstyle/checkstyle/commit/2282a7eca66073d838ace295d1d6874818323fab and the previous behavior is not going to be restored. We can either implement our own replacement for containsAnnotation or make changes on the calling side to pass in both the simple name and the fully-qualified name.