[ 
https://issues.apache.org/jira/browse/CASSANDRA-17925?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17679968#comment-17679968
 ] 

Maxim Muzafarov commented on CASSANDRA-17925:
---------------------------------------------

This is the discussion on the dev-list about the classes import order
[https://lists.apache.org/thread/d7wgg5hf4khzkwyd3hgnooowxl5b8dg1]
----
While preparing the code style configuration for the Eclipse IDE, I discovered 
that there was no easy way to have complex grouping options for the set of 
packages. So we need to add extra blank lines between each group of packages so 
that all the configurations for Eclipse, NetBeans, IntelliJ IDEA and checkstyle 
are aligned. I should have checked this earlier for sure, but I only did it for 
static imports and some groups, my bad. The resultant configuration looks like 
this:
{code:java}
java.*
[blank line]
javax.*
[blank line]
com.*
[blank line]
net.*
[blank line]
org.*
[blank line]
org.apache.cassandra.*
[blank line]
all other imports
[blank line]
static all other imports 
{code}
The changes include:
 * The AvoidStarImport and ImportOrder rules were added to checkstyle.xml and 
checkstyle_test.xml;
 * The codeStyleSettings.xml migrated to a new IntelliJ IDEA format. This 
configuration file is deprecated since IntelliJ IDEA 2017.3 and is not used 
anymore. All code style settings are located under 
<PROJECT_ROOT>/.idea/codeStyles directory and consist of the following files: 
codeStyleConfig.xml, Project.xml;
 * The Eclipse code style configuration was added from the link mentioned in 
the documentation pages: [https://github.com/tjake/cassandra-style-eclipse]

The pull request is here:
[GitHub PullRequest: 2108|https://github.com/apache/cassandra/pull/2108]

The configuration-related changes are placed in a dedicated commit, so it 
should be easy to make a review:
[84e292ddc9671a0be76ceb9304b2b9a051c2d52a|https://github.com/apache/cassandra/pull/2108/commits/84e292ddc9671a0be76ceb9304b2b9a051c2d52a]

The Jenkins build is here:
[https://ci-cassandra.apache.org/view/patches/job/Cassandra-devbranch/2215/]
----
Another important thing to mention is that the total amount of changes for 
organising imports is really big (more than 2000 files!), so we need to decide 
the right time to merge this PR. Although rebasing or merging changes to 
development branches should become much easier ("Accept local" + "Organize 
imports"), we still need to pay extra attention here to minimise the impact on 
major patches for the next release.

> Java source code should have sorted imports as defined in the codestyle
> -----------------------------------------------------------------------
>
>                 Key: CASSANDRA-17925
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-17925
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: Build
>            Reporter: Stefan Miklosovic
>            Assignee: Maxim Muzafarov
>            Priority: Normal
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> 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:
> {code:java}
>         <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>
> {code}
> 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:
> {code:java}
>     <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>
> {code}
> 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.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org

Reply via email to