palantir / gradle-baseline

A set of Gradle plugins that configure default code quality tools for developers.
Apache License 2.0
307 stars 134 forks source link

Enable checkstyle rule: ConstantName #2455

Open ash211 opened 2 years ago

ash211 commented 2 years ago

What happened?

A code review flagged that this line should be in UPPER_SNAKE_CASE:

static final String emptyContents = "";

If we care about this, it should be enforced as a lint rule instead of comment.

What did you want to happen?

Enable https://checkstyle.sourceforge.io/config_naming.html#ConstantName in the default checkstyle file.

ash211 commented 2 years ago

Probably need to handle log/logger constants as exceptions, since those are rarely capitalized.

An internal repo has the rule enabled like this:

<module name="ConstantName">
    <property name="format" value="^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*|log|logger$"/>
</module>
j-baker commented 2 years ago

https://github.com/palantir/gradle-baseline/blob/develop/docs/java-style-guide/readme.md#constant-names - personally I prefer this guidance.

ash211 commented 2 years ago

that constant vs non-constant distinction sounds very similar to immutable vs mutable. Is there a difference between constant-ness and immutability?

For programmatic enforcement of the constant naming scheme, we could look into requiring members whose types are annotated @Immutable be named according to the constant naming scheme in UPPER_SNAKE_CASE. That immutability is verified with https://errorprone.info/bugpattern/Immutable

I'm not sure this is worth the effort though, just for naming consistency.