Update Checkstyle and associated rules #71
Conversation
README.md
Outdated
| @@ -52,6 +52,8 @@ See [the Intellij documentation](https://www.jetbrains.com/help/idea/copying-cod | |||
| See [the eclipse documentation](http://help.eclipse.org/kepler/index.jsp?topic=%2Forg.eclipse.jdt.doc.user%2Freference%2Fpreferences%2Fjava%2Fcodestyle%2Fref-preferences-formatter.htm) for how to import the files into your IDE. | |||
| In addition, html indentation settings are not exported, but can be set to 2 spaces from Preferences -> Web -> HTML Files -> Editor -> "Indent using spaces". [Screenshot of the eclipse config here.](https://s3.amazonaws.com/uploads.hipchat.com/18157/88592/RSkQhq8UYnxf81Z/upload.png) | |||
|
|
|||
| Note that Eclipse's _Organize Imports_ feature does not group imports by 1st party, 3rd party, etc. as defined by Checkstyle rules. | |||
There was a problem hiding this comment.
If you cannot configure all major IDEs employed by our developers to meet the rules that Checkstyle seeks to enforce and fail on, then those rules have reached the end of their usefulness.
There was a problem hiding this comment.
I'm inclined to agree 43a3701
I tested a couple of ways in Eclipse, and whether you configure 0 or 1 blank lines between groups, it will mess up the grouping in an existing file when you do an automatic import that comes at the alphabetical beginning or end of a group 🤷
|
@paulerickson Were you not able to create a branch on this project. We do have branch builds at https://jenkins.ci.sonatype.dev/job/bnr/job/tools/job/codestyle/job/feature-snapshots/ |
|
@ajdranse This seems to be a superset of your PR. Perhaps you can review and maybe close yours? |
Sure thing, will do that today. |
Oops, I hadn't even noticed my origin was a fork, but I do have permissions now, so I created https://github.com/sonatype/codestyle/pull/72/files to replace this |
Our version of Checkstyle is out of date, does not enforce certain rules very well, and completely chokes on newer language elements such as switch expressions.
Unfortunately, that project uses sentimental versioning and broke some things without any clear migration guides 😒
Config changes that don't change behavior
The LineLength check only moved from one place to another
Javadoc
allowMissingThrowsTagsandallowThrowsTagsForSubclassesare no longer supported. Since we allowed these, it should be no change.Javadoc
allowMissingJavadocandminLineCountmoved to a new check. Since we allow missing, I have not added that check.Javadoc
scopeapparently got renamed toaccessModifiersUnchanged config that changes behavior
Mainly I would highlight import groups. Checkstyle defines a fixed number of logical groups to be separated by a blank line; we have
It enforces that there are blank lines between those groups and no additional blank lines within groups, whereas it used to permit extra blank lines. IMHO, this way of grouping and avoiding excess blank lines is good style and consistent with the existing rule, so I updated the IDEA config to be consistent. Unfortunately, Eclipse only groups packages, so its auto-organize import feature is fundamentally different.
EDIT: so I removed this check
Related to this,
javaxleft the JDK in version 11, so whether it is 3rd party or standard is not a matter of style but JDK version. I updated the IDE configs to treat it as 3rd party, since that is current and consistent with Checkstyle.The rule against unnecessary parentheses is enforced more rigidly now, and I had one or two other new violations with HDS, which seemed consistent with the existing rules.