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

Question: Finding compatible Checkstyle version for a config file #359

Closed
baron1405 opened this issue Nov 14, 2017 · 3 comments
Closed

Question: Finding compatible Checkstyle version for a config file #359

baron1405 opened this issue Nov 14, 2017 · 3 comments

Comments

@baron1405
Copy link
Contributor

baron1405 commented Nov 14, 2017

Hi, this is a question, not an issue. I am writing an IntelliJ plugin that wants to set the correct version of Checkstyle when a project is loaded. The correct version would be determined by picking the highest version of Checkstyle that is compatible with the user's configuration file. I have something almost working which does the following on project open.

Iterate over the Checkstyle versions from the highest to the lowest doing the following:

  1. Invalidate the checker cache
  2. Set the active Checkstyle version using CheckstyleProjectService.activateCheckstyleVersion
  3. Create a checker using CheckstyleActions.createChecker
  4. If a CheckstyleToolException is thrown, assume the version is incompatible with the configuration file.
  5. If an exception is not thrown, assume the version is compatible

The first question is whether there is a simpler way to achieve this? The problem I am having is that the Checkstyle IDEA plugin is reporting that the configuration file cannot be read during this above iteration. Is there a way to silence it during my iteration, if this is the correct approach?

Thanks in advance for any help!

Baron

@tsjensen
Copy link
Contributor

tsjensen commented Nov 14, 2017

Well, such an approach might get you a version of Checkstyle which can parse your configuration file without errors. However, it may still not get you the "correct" version. Example:

  • You have the check which requires all parameters to be final.
  • You also have the RedundantModifier check.
  • This is fine for all versions of Checkstyle, as long as you didn't set any properties.

However, newer versions of Checkstyle will spam the developers with issues that final is redundant in interfaces (so should be removed), and the other check will tell them to add final modifiers.

So, I think you should approach this the other way around. Decide on the Checkstyle version you want to use, make the proper config file for that, and configure all your tools to use that same version. This would also be consistent with Checkstyle's idea of a "coding standard" that it is designed to enforce. Such a standard would not be worth much if it adjusted to the individual user.

@jshiell
Copy link
Owner

jshiell commented Nov 14, 2017

Hi. Full marks for adventurousness! To be honest, it's never been a use case I've considered.

Checkstyle itself has never offered an API as such - they have said they're targeted as a command-line tool, and the fact that you can use it as a backend is incidentally, which is why they don't really worry about bug reports such as raw exceptions coming out of Checkstyle.

This, in turn, informs the design on our side - it's very much attempting to capture and sanity feedback from the Checkstyle API. It doesn't offer any abstraction layer - it's pretty tightly coupled to the IDEA UI at present. At the time it was expedient; were I to do it all again I'd probably abstract just to improve testability. If wishes were horses...

So the honest answer at present is I can't think of a more simple way to accomplish the task, and to fix the alerts you'd probably need to modify the core plugin to introduce an abstraction for error feedback. There's already a start of one there in the listener interface for scan feedback, but it's incomplete, and the error handling could do with a tidy-up to make it more consistent. So probably not a 5-minute job I'm afraid.

And, as the good @tsjensen says, it's a good way to catch the nastier exceptions but there are subtleties that may well be missed. So I fear it's a bit of a Pandora's box in general.

Sorry to be the bearer of bad news.

@baron1405
Copy link
Contributor Author

baron1405 commented Nov 15, 2017

Thank you @tsjensen and @jshiell for your quick and thoughtful replies. I did get my solution to work once I realized that changing the scanning behavior also required calling the service activateCheckstyleVersion since I had been changing it out from under the UI. As you folks point out, properties are a potential gotcha for a general solution. The good news is that the set of properties used in my org is very small and controlled so I just add those in when doing my compatibility testing. It would sure be nice if the plugin offered an auto-detect mode but I understand the limitations. Perhaps this issue can be converted from a question to a feature request if you think it is a useful and practical idea.

Once again thanks for your insights.

@jshiell jshiell closed this as completed Apr 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants