aws-powertools
Maintenance: Checkstyle improvements
Summary
Checkstyle's great, but we should make it easier to work with.
Why is this needed?
If contributors can't easily push changes because of checkstyle lints we are raising the barrier to entry to contribution.
Which area does this relate to?
No response
Solution
- some way of CLI-applying lints - so we’re not tied to intellij, and so we can use that in the build if needed
- fail the build if lints aren’t applied - as it stands, some lints do this, but not all of the formatting changes checkstyle applies. If we want to enforce the formatting, we should fail if it is not there
- enforce javadoc on public interfaces putting Checkstyle in place lets us ensure other nice properties of the codebase hold - like full coverage of public interface documentation
I'm not sure how far we can get with the checkstyle plugin for these points, but would expect there is some solution out there!
Acknowledgment
- [X] This request meets Powertools for AWS Lambda (Java) Tenets
- [ ] Should this be considered in other Powertools for AWS Lambda (Java) languages? i.e. Python, TypeScript
the build already seems to run checkstyle
https://github.com/aws-powertools/powertools-lambda-java/actions/runs/5975367102/job/16211188408#step:5:11642
[INFO] You have 0 Checkstyle violations.
are you looking to update above processing?
Hey @gliptak , there are two problems. 1/ it is difficult to run checkstyle locally - our instructions are for intellij-only (which makes it hard to work with the repo in any other editor), and experience has shown that intellij plugin version changes have resulted in different checkstyle changes applied to the code. Optimally there would be some standard IDE-independent way of applying the checkstyle changes - e.g. maven target.
2/ We expect checkstyle to be applied in this intellij fashion, which includes formatting changes which do not fail the build. This puts us in a position where we have to come back to users and say "you need to fix this", even though the build is not failing.
would
mvn install checkstyle:checkstyle
find -name checkstyle.html
find -name checkstyle-result.xml
work locally?
Does checkstyle:checkstyle only produce output if no issues are found? If so that might work for the second point above! If not, we'd have to parse the XML to check for empty lint sets, which probably isn't too difficult.
For the first - we still need a way to apply the formatting from the CLI, which the maven checkstyle plugin doesn't support.
We can potentially move to Spotless: https://github.com/diffplug/spotless/tree/main/plugin-maven.
It can both apply and check formatting using maven. Gradle has official IDE integrations for VSCode and IntelliJ. Unfortunately not for Maven though.
While working on this PR I also found that Checkstyle uses a non-approved license. I took the opportunity to remove the dependency from the project until we have replaced it in v2 with an alternative formatter. As linter, we are already using PMD.
The PR #1621 was closed and checkstyle was removed as dependency.
Next steps:
- Introduce alternative formatter (such as spotless)
- Remove checkstyle.xml which was kept for backwards compatibility of IDE integrations