[CASSANDRA-19276] Add 'LeftCurly' checkstyle rule to enforce braces on next line on build

• Type: Task
• Status: Open
• Priority: Low
• Resolution: Unresolved
• Component/s: Build
• Change Category: Code Clarity
• Complexity: Normal
• Platform: All
• Assignee: Andy Tolbert
• Reporter: Andy Tolbert

Authors

Andy Tolbert

• Created: 18/Jan/24 16:49
• Updated: 23/Jan/24 16:50

Description

It came up in a review that I had missed some style changes in CASSANDRA-18857 (https://github.com/apache/cassandra/pull/2969#pullrequestreview-1810999533). Chatting with smiklosovic we agreed that it would be nice if we could enforce this in checkstyle, so we wouldn't need to be dependent on this being caught in review.

The change for this is effectively:

index 8b81f21281..9bd22dc1ac 100644
--- a/.build/checkstyle.xml
+++ b/.build/checkstyle.xml
@@ -179,6 +179,10 @@
       <message key="matchxpath.match"
                value="'Deprecated annotation must provide 'since' value."/>
     </module>
+    <module name="LeftCurly">
+        <property name="option" value="nl"/>
+        <property name="tokens" value="ANNOTATION_DEF,CLASS_DEF,CTOR_DEF,ENUM_CONSTANT_DEF,INTERFACE_DEF,LITERAL_CASE,LITERAL_CATCH,LITERAL_DEFAULT,LITERAL_DO,LITERAL_ELSE,LITERAL_FINALLY,LITERAL_FOR,LITERAL_IF,LITERAL_SWITCH,LITERAL_SYNCHRONIZED,LITERAL_TRY,LITERAL_WHILE,METHOD_DEF,OBJBLOCK,STATIC_INIT,RECORD_DEF,COMPACT_CTOR_DEF"/>
+    </module>
   </module>

 </module>

Notably, we would allow braces on the same lines for lambdas as per the project guidelines on code formatting:

} and { are placed on a new line except when empty or opening a multi-line lambda expression. Braces may be elided to a depth of one if the condition or loop guards a single expression.

There are 594 violations and 211 source files that would need to be adjusted. I may play with the rules a little bit more to get this right.

I would like to propose that we change all files that are not imported from other projects (there are a few, such as src/java/org/apache/cassandra/utils/obs/BitUtil.java, which we can suppress changes for), but we would change others.

I'll make a patch with the changes send a small proposal to the mailing list as it could be disruptive to make a bunch of tiny changes, and depending on timing there may be a better time to make a change like this.

We should make this change only on trunk.