Skip to content

checkstyle configuration for API #11859

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

Machine-Maker
Copy link
Member

@Machine-Maker Machine-Maker commented Dec 29, 2024

The checkstyle configuration for the API. The server's will come later and be mostly similar with some key differences (like javadocs and excluding decompiled sources).

Has a file that includes all packages in the API that need to be done. Once removed from that list, all files in it will be subject to checkstyle.

Checkstyle Categories

  • Annotations
  • Block Checks
  • Class Design
  • Coding
  • Headers
  • Imports
  • Javadoc Comments
  • Metrics
  • Misc
  • Modifiers
  • Naming Conventions
  • Regexp
  • Size Violations
  • Whitespace

@Machine-Maker Machine-Maker added the status: input wanted Looking for community feedback on this issue. label Dec 29, 2024
@Machine-Maker Machine-Maker changed the title beginning of checkstyle configuration for API checkstyle configuration for API Dec 29, 2024
@Machine-Maker
Copy link
Member Author

Machine-Maker commented Dec 29, 2024

Ok, this is ready to be reviewed. I'm sure there will be things we notice when we start updating all the files it tells us to that we don't want, but we can make those changes as we go along.

This also isn't validated against any formatter yet, so there could be conflicts. We will find those out though as we go.

It also isn't running the custom import order yet, because that will come with the formatter.

@Machine-Maker Machine-Maker marked this pull request as ready for review December 29, 2024 21:44
@Machine-Maker Machine-Maker requested a review from a team as a code owner December 29, 2024 21:44
@Machine-Maker Machine-Maker force-pushed the chore/checkstyle branch 2 times, most recently from 10c4bb0 to e015bb8 Compare December 29, 2024 23:38
@Machine-Maker
Copy link
Member Author

So there's a ton of new logic now in the api's build.gradle. This should probably be moved somewhere, idk if we wanna do the buildSrc thing, or make it part of pw, or what

@Machine-Maker Machine-Maker force-pushed the chore/checkstyle branch 2 times, most recently from 567e547 to 6d981e0 Compare January 4, 2025 22:57
@kennytv
Copy link
Member

kennytv commented Jan 11, 2025

I'd leave out final local variables for now to still be able to instantly find "not clean" code, and this one is easy to do automatically in a single commit through IJ later on as well. Otherwise should be good, anything that looks off we can change when we run into it

@Warriorrrr Warriorrrr added the type: feature Request for a new Feature. label Jan 12, 2025
@Machine-Maker Machine-Maker force-pushed the chore/checkstyle branch 2 times, most recently from ace26c9 to 4993c67 Compare July 12, 2025 14:52
@NonSwag
Copy link
Contributor

NonSwag commented Jul 19, 2025

Some things I spotted that might need fixing:

  • primitives (fields params and returns) require nullability annotations
  • void returns require nullability annotatins
  • @NonNull (nullability information in general) infront of the modifiers is allowed if it is on the same line as the method but should only be allowed AFTER the modifiers - example: @NonNull public static X is allowed but should be public static @NonNull X
  • deprecated (not marked for removal) class are still checked which seems redundant
  • there is no rule about the order of nullability annotation and final in method parameters, should it be @NonNull final or final @NonNull
  • package-private and interface methods should be allowed (or even required) to have the nullability annotation not on the same line as the target

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: input wanted Looking for community feedback on this issue. type: feature Request for a new Feature.
Projects
Status: Awaiting review
Development

Successfully merging this pull request may close these issues.

4 participants