Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compatibility with Checkstyle 10 #395

Closed
wilkinsona opened this issue Dec 12, 2023 · 4 comments
Closed

Compatibility with Checkstyle 10 #395

wilkinsona opened this issue Dec 12, 2023 · 4 comments
Assignees
Milestone

Comments

@wilkinsona
Copy link
Contributor

wilkinsona commented Dec 12, 2023

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
Copy link
Contributor Author

checkstyle/checkstyle#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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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 checkstyle/checkstyle@2282a7e 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.

@wilkinsona wilkinsona modified the milestones: 0.0.x, 0.0.41 Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant