[CASSANDRA-17925] Java source code should have sorted imports as defined in the codestyle

• Type: Improvement
• Status: Patch Available
• Priority: Normal
• Resolution: Unresolved
• Fix Version/s: 5.x
• Component/s: Build
• Labels: code-polishing
• Epic Link: Code Style & Static Analysis
• Change Category: Code Clarity
• Complexity: Normal
• Platform: All

Test and Documentation Plan

The documentation must be updated once an agreement on this issue will be reached.

To test do the following:

$ git fetch upstream pull/2108/head:cassandra-17925 && git checkout cassandra-17925
$ ant jar && ant сheck

• Assignee: Maxim Muzafarov
• Reporter: Stefan Miklosovic

Authors

Maxim Muzafarov

• Created: 27/Sep/22 10:48
• Updated: 06/Oct/23 10:36

Description

After we cleaned all unused imports in CASSANDRA-17876, there is one more task remaining to be done - to add checkstyle for imports order and enforce this on build time.

When the project is imported into IDEA, there is a helper target on Ant called "generate-idea-files". ide/idea/codeStyleSettings.xml contains this:

        <option name="IMPORT_LAYOUT_TABLE">
          <value>
            <package name="java" withSubpackages="true" static="false" />
            <package name="javax" withSubpackages="true" static="false" />
            <emptyLine />
            <package name="com.google.common" withSubpackages="true" static="false" />
            <package name="org.apache.log4j" withSubpackages="true" static="false" />
            <package name="org.apache.commons" withSubpackages="true" static="false" />
            <package name="org.cliffc.high_scale_lib" withSubpackages="true" static="false" />
            <package name="org.junit" withSubpackages="true" static="false" />
            <package name="org.slf4j" withSubpackages="true" static="false" />
            <emptyLine />
            <package name="" withSubpackages="true" static="false" />
            <emptyLine />
            <package name="" withSubpackages="true" static="true" />
          </value>
        </option>

This code style is also mentioned in the web page here (minus some details which are present in above configuration snippet but not on the web page): https://cassandra.apache.org/_/development/code_style.html (at the very bottom).

However, when one runs "Optimise imports" in the context menu after right-clicking on org.cassandra.apache package, it will refactor the imports and it results with hundreds of changes.

This means that the source code, as-is, does not adhere to the self-imposed code style we ship for IDEA.

If we fix this, we should add checkstyle for it like this:

    <module name="ImportOrder">
      <property name="groups" value="/(^java\.|javax)/,/(com\.google\.common|org\.apache\.log4j|org\.apache\.commons|org\.cliffc\.high_scale_lib|org\.junit|org\.slf4j)/"/>
      <property name="ordered" value="true"/>
      <property name="separated" value="true"/>
      <property name="option" value="bottom"/>
      <property name="separatedStaticGroups" value="false"/>
      <property name="sortStaticImportsAlphabetically" value="true"/>
    </module>

This checkstyle on import order will pass on the source code we run the import optimization in the context menu on.

There is also no enforcement on "all star" imports (org.some.pkg.*). Checkstyle has specific module for this: https://checkstyle.org/config_imports.html#AvoidStarImport 

I propose we should stop to use all-star imports. Same argument holds as described there: Rationale: Importing all classes from a package or static members from a class leads to tight coupling between packages or classes and might lead to problems when a new version of a library introduces name clashes.

This should be applied to test checktyle as well and the source code should be refactored on imports too.

This should be done on cassandra-4.1 as well as for trunk.

Issue Links

• is related to: CASSANDRA-18089 The source code must obey the avoid star import checkstyle rule
• links to: Discussion: Dev-List Import Order
• links to: GitHub PullRequest: 2108