Hi,
I did a quick look over the checkstyle-suppressions.xml
to initially get rid of unneeded stuff. While doing so, I noticed that some checks are in my opinion a bit too broad and cover up some minor issues. E.g. the following suppressions:
<suppress files="[\\/]autoconfigure[\\/]" checks="JavadocType" />
<suppress files="[\\/]autoconfigure[\\/]" checks="JavadocVariable" />
cause DispatcherServletAutoConfiguration#DEFAULT_DISPATCHER_SERVLET_BEAN_NAME to have no comment although it's just not using the correct style for the field.
It's mostly missing comments on enums that the above checks suppress. I would volunteer to put a comment everywhere (or fix their style). The same would apply for the Ansi.*
suppressions. And if they really don't make sense, I would change them to suppress specific files instead.
But as this means a bit tedious work, I wanted to know your opinion first ;-)
Cheers, Christoph
Comment From: philwebb
Those do look a bit broad and I'm trying to remember why they might be there. I have a feeling it might be for @ConfigurationProperties
classes where we break the javadoc rules to generate IDE meta-data. It would certainly be better if we could be a little more surgical for those.
Comment From: dreis2211
Do you have an example where the rules are broken for @ConfigurationProperties
on purpose?
Comment From: philwebb
Sorry, I don't remember any specific examples. I was just trying to remember why we might have put those suppression rules in place. Perhaps I'm misremembering.
Comment From: snicoll
I've started to look at it and the exclude on autoconfigure
feels a bit wide at first indeed. I've tried to fix those and it makes some area of the code more dense than I'd like them to be. More importantly, checkstyle is complaining about parameters in enum:
[ant:checkstyle] [ERROR] /Users/snicoll/workspace/work/spring-boot/2.3.x/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jms/JmsProperties.java:379:17: Missing a Javadoc comment. [JavadocVariable]
Maybe limiting on specific classes could be better, I don't know.
Comment From: philwebb
I remember why we did it now. For @ConfigurationProperties
we generate IDE meta-data by reading the javadoc from the field. We don't really want getter/setter docs, hence the suppression.
I'll limit the scope to just *Properties
classes.