Checkstyle: Remove properties related to class loading from JavadocMethod

Created on 6 Dec 2019  路  6Comments  路  Source: checkstyle/checkstyle

part of #7116 .

We need to remove from Check any attempt to use classloader to validate source code.
Checkstyle cannot make this reliably and it is whole tool limitation for now - https://checkstyle.org/writingchecks.html#Limitations

Migration Notes:
properties allowMissingThrowsTags, allowUndeclaredRTE and allowThrowsTagsForSubclasses, logLoadErrors, suppressLoadErrors should be just removed from configs.
Behavior will change as there will be less violations in code, such properties did not work well so their removal should be ok, as there will be less false-positives. Throws will not be validated by default !!
If you need to validate @throws please enable property validateThrows but please read documentation as there some limitations - https://checkstyle.org/config_javadoc.html#JavadocMethod .
Old documentation is at https://checkstyle.sourceforge.io/version/8.27/config_javadoc.html#JavadocMethod

approved breaking compatibility

Most helpful comment

Well, actually I periodically bump version numbers of some of my projects to get bugfixes. Sometimes, it does not goes well, surely, but most of times, it simply works.

Yesterday, it was Checkstyle that didn't went very well. After having some work to get the actual error message (some build tools eat them up and just tell that it failed without telling why), Checkstyle was complaining that some of my properties didn't exist. Looking for them in documentation, I found nothing. Huh? What the heck is going on? Googling for allowUndeclaredRTE didn't really helped to figure out what was going on.

It took me a lot of time to figure out that the property simply does not exists anymore, since the documentation is silent about them and I had to compare documentations of different versions and scan changelogs to finally be able to understand what was going on, something that occasional users shouldn't need to do. Further, finding where the old versions of the documentation are is not a very easy task for people who are not used to it.

Fortunately, once I found the cause of the problem and understood it, it was very easy to fix. But to help our users, at least a message (even if it is an error) saying that allowUndeclaredRTE was removed with suggestions and orientations about what to do would be much more clear and informative than an error message saying that allowUndeclaredRTE doesn't exist as if I it was some random gibberish as a property name.


I could make a parallel about the Java philosophy of not removing anything from its API has a reason to exist (and that the exceptions are only the most extremate problematic cases, and even then, they give prior warning and do a migration plan and impact study).

The reason is that people might be locked out in old versions of software due to the inability or difficulty of upgrading. This in turn, make future migrations even harder and harder and make bugs and secure issues pile up. Java itself suffers from it, with large chunks of the market stuck with Java 7 or Java 8 era due to (mostly accidental and unintended) difficulties and incompatibilities introduced preventing the migration (including many issues with Android).

This problem could be so dire that I even know a case so severe (and got a bit paranoid about it) that eventually led to the death of some people because the hospital had buggy software locked out in ancient decaying versions that occasionally screwed up the databases and the medical records and they were unable to upgrade it due to breaking changes with the following versions. Ok, you might argue that this is very far from the case of checkstyle (and it really is), but you see the point: sudden unexplained breaking changes can be really bad and some persons (myself included) can have some strong reasons to be paranoid about them.

All 6 comments

fix is merged

Removing the "allowUndeclaredRTE" broken one script of mine in a big project. Including sudden breaking changes with few to no forewarning is not a good thing. At least, you could deprecate it first and remove it only at a later version.

Also, it would be a good idea to keep in the documentation that those properties once existed and where removed. The last version of the documentation is completely silent about them as if they never existed. Users should have no obligation to scan release change logs or github issues to figure out why something that used to work just broken mysteriously after a small minor version bump.

it would be a good idea to keep in the documentation that those properties once existed and where removed.

https://checkstyle.org/#Previous_Version_Documentation

Including sudden breaking changes with few to no forewarning is not a good thing.

Please share problem, we might help.
Nothing is forcing you to upgrade version of checkstyle. If you are affected, sorry, please report problems, if possible we might help to ease or provide quick fix.

could deprecate it first and remove it only at a later version.

We do not follow semi version, our minor version do mean new or breaking is done.
More details at https://checkstyle.org/#Development_Cycle

Well, actually I periodically bump version numbers of some of my projects to get bugfixes. Sometimes, it does not goes well, surely, but most of times, it simply works.

Yesterday, it was Checkstyle that didn't went very well. After having some work to get the actual error message (some build tools eat them up and just tell that it failed without telling why), Checkstyle was complaining that some of my properties didn't exist. Looking for them in documentation, I found nothing. Huh? What the heck is going on? Googling for allowUndeclaredRTE didn't really helped to figure out what was going on.

It took me a lot of time to figure out that the property simply does not exists anymore, since the documentation is silent about them and I had to compare documentations of different versions and scan changelogs to finally be able to understand what was going on, something that occasional users shouldn't need to do. Further, finding where the old versions of the documentation are is not a very easy task for people who are not used to it.

Fortunately, once I found the cause of the problem and understood it, it was very easy to fix. But to help our users, at least a message (even if it is an error) saying that allowUndeclaredRTE was removed with suggestions and orientations about what to do would be much more clear and informative than an error message saying that allowUndeclaredRTE doesn't exist as if I it was some random gibberish as a property name.


I could make a parallel about the Java philosophy of not removing anything from its API has a reason to exist (and that the exceptions are only the most extremate problematic cases, and even then, they give prior warning and do a migration plan and impact study).

The reason is that people might be locked out in old versions of software due to the inability or difficulty of upgrading. This in turn, make future migrations even harder and harder and make bugs and secure issues pile up. Java itself suffers from it, with large chunks of the market stuck with Java 7 or Java 8 era due to (mostly accidental and unintended) difficulties and incompatibilities introduced preventing the migration (including many issues with Android).

This problem could be so dire that I even know a case so severe (and got a bit paranoid about it) that eventually led to the death of some people because the hospital had buggy software locked out in ancient decaying versions that occasionally screwed up the databases and the medical records and they were unable to upgrade it due to breaking changes with the following versions. Ok, you might argue that this is very far from the case of checkstyle (and it really is), but you see the point: sudden unexplained breaking changes can be really bad and some persons (myself included) can have some strong reasons to be paranoid about them.

@victorwss , thanks a lot for feedback.

I wish one day checksyle will have same amount of contributors as java .... so we can make more reliable Checks and better design. In addition to 5K issue ... we do have problem with deprecation process, as Checkstyle can not put some message as warning for migration. All we can do is put same violation on all your files in your project. So as you mentioned (hard to find actual error message in build system) we do not know where checkstyle is embedded. So it very hard for us to make something that will be easily seen by user, as plugins are not updating that quick and usually bump inner version of checkstyle. One day we will address this .... if we have more contributors in library and in plugins.

I do not see right now any good solution to not over burn contributors with tons of documentation and in the same time keep contribution easy process.
Probably "stackoverflow" way to share problem (stacktrace or error message) and get answer and later on all other users see/found it in google it the good approach.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

Vampire picture Vampire  路  8Comments

DaGeRe picture DaGeRe  路  5Comments

rnveach picture rnveach  路  5Comments

Fernal73 picture Fernal73  路  7Comments

crm416 picture crm416  路  3Comments