eclipse-cs icon indicating copy to clipboard operation
eclipse-cs copied to clipboard

Make used checkstyle version configurable (and per project)

Open elmuerte opened this issue 4 years ago • 3 comments

With breaking changes being made to the configuration file (like https://github.com/checkstyle/checkstyle/issues/7417 ) this plugin can effectively no longer be used when two projects use configuration files which are not compatible with the one specific version this plugin ships with.

elmuerte avatar Aug 25 '21 10:08 elmuerte

@elmuerte, please share a user story of your checkstyle usage. To let us better understand why migration is problematic, it is defenetly pain, but should be not that complicated to update config.

romani avatar Aug 26 '21 04:08 romani

I can only guess, but I assume the following: When using eclipse with many plugins, the two most used strategies by developers are:

  • never upgrade anything after you have installed eclipse
  • always upgrade all plugins to the latest version

Basically this depends on whether automatic updates are enabled or not. For those developers that use the "always latest plugins" strategy, eclipse-cs will suddenly break, if the new plugin version assumes another configuration format now. And if you then upgrade the configuration file to make it compatible again, you break it for all the remaining developers of your team which have not yet upgraded. (There is a possibility to enforce the same eclipse plugin versions via Eclipse Oomph, but most teams are not even aware of this mechanism). So while you can upgrade all team members at the same time for Eclipse Checkstyle Maven by just changing the pom.xml in version control, you typically cannot control this easily for a big team (or for a big set of projects).

As I have mentioned in another issue of the checkstyle main project, I believe it's a big design fault to require the configuration file to reflect the class hierarchy of the implementation. That's simply not necessary. All the checks can be identified by their name alone, independent of whether they are nested in the TreeWalker or not (which is the typical thing that breaks compatibility). So just don't nest modules in each other in the config file. Rather just have one list of checks without any hierarchy of modules. There is absolutely no benefit in being able to configure some properties on multiple levels. No normal user needs that.

Bananeweizen avatar Aug 26 '21 18:08 Bananeweizen

The problem is that people can contribute to different projects, from different organizations. These projects/organizations have their own style guides and also own checkstyle configurations. They also often use specific versions of checkstyle due to significant differences in rule behavior either through bugs or explicit changes which are not configuration driven.

Getting around the breaking configuration change can be achieved by creating a local copy of the project/organization's config and make the necessary adjustments so that a newer checkstyle also accepts the config. This adds some extra effort to keep the configuration in sync.

But the remaining problem about about differences in how checkstyle rule work between various versions is not easy to get around when the Eclipse plugin uses a specific checkstyle version. The only way for this would be to use different Eclipse installations.

elmuerte avatar Aug 27 '21 12:08 elmuerte

We will not be able to change the way this works, because the Eclipse plugin needs to be able to load the main Checkstyle classes correctly. Since there are breaking changes all the time, supporting ranges is way too much effort. Generally speaking, it's recommended to have a separate Eclipse installation per project anyway (not just different workspaces for one single installation). Otherwise you will end with an Eclipse installation that has too many plugins, with conflicting dependencies etc. I run more than 50 Eclipse installations on my system and the additional disk space usage is neglectible.

Bananeweizen avatar Sep 17 '23 17:09 Bananeweizen