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

ImportOrder: make static imports ordering as in Eclipse #3101

Closed
romani opened this Issue Apr 14, 2016 · 5 comments

Comments

Projects
None yet
3 participants
@romani
Member

romani commented Apr 14, 2016

base on https://bugs.eclipse.org/bugs/show_bug.cgi?id=473629

/var/tmp $ java -jar checkstyle-6.17-SNAPSHOT-all.jar -c Test.xml TestClass.java 
Starting audit...
[ERROR] /var/tmp/TestClass.java:4: Wrong order for 'HttpHeaders.Names.DATE' import. [ImportOrder]
Audit done.
Checkstyle ends with 1 errors.

/var/tmp $ cat TestClass.java 
import static HttpConstants.COLON;
import static HttpHeaders.addHeader;
import static HttpHeaders.setHeader;
import static HttpHeaders.Names.DATE;

public class TestClass {

}

/var/tmp $ cat Test.xml 
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
          "-//Puppy Crawl//DTD Check Configuration 1.3//EN"
          "http://www.puppycrawl.com/dtds/configuration_1_3.dtd">
<module name = "Checker">
    <module name="TreeWalker">
      <module name="ImportOrder">
        <property name="groups" value="/^javax?\./,org"/>
        <property name="ordered" value="true"/>
        <property name="separated" value="true"/>
        <property name="option" value="top"/>
        <property name="sortStaticImportsAlphabetically" value="true"/>
      </module>
    </module>
</module>

Expected, no violations for Eclipse users.

investigate sorting behavior for static imports in Idea and NetBeans.

If the same in all 3 IDEs - just follow them
If different - we need new properties with explanations.

@romani romani added the approved label Apr 14, 2016

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Apr 15, 2016

Member

NetBeans default, Looks like it is alphabetical order (upper before lower), ignoring static/nonstatic.

import static test3101.HttpConstants.COLON;
import test3101.HttpHeaders;
import static test3101.HttpHeaders.Names.DATE;
import static test3101.HttpHeaders.addHeader;
import static test3101.HttpHeaders.setHeader;

IntelliJ default, non-static first, then static. Rest seems similar to NetBeans.

import test3101.HttpHeaders;

import static test3101.HttpConstants.COLON;
import static test3101.HttpHeaders.Names.DATE;
import static test3101.HttpHeaders.addHeader;
import static test3101.HttpHeaders.setHeader;

Eclipse default, static first, looks complete alphabetical order afterwards (lower before upper).

import static test3101.HttpConstants.COLON;
import static test3101.HttpHeaders.addHeader;
import static test3101.HttpHeaders.setHeader;
import static test3101.HttpHeaders.Names.DATE;
import test3101.HttpHeaders;
Member

rnveach commented Apr 15, 2016

NetBeans default, Looks like it is alphabetical order (upper before lower), ignoring static/nonstatic.

import static test3101.HttpConstants.COLON;
import test3101.HttpHeaders;
import static test3101.HttpHeaders.Names.DATE;
import static test3101.HttpHeaders.addHeader;
import static test3101.HttpHeaders.setHeader;

IntelliJ default, non-static first, then static. Rest seems similar to NetBeans.

import test3101.HttpHeaders;

import static test3101.HttpConstants.COLON;
import static test3101.HttpHeaders.Names.DATE;
import static test3101.HttpHeaders.addHeader;
import static test3101.HttpHeaders.setHeader;

Eclipse default, static first, looks complete alphabetical order afterwards (lower before upper).

import static test3101.HttpConstants.COLON;
import static test3101.HttpHeaders.addHeader;
import static test3101.HttpHeaders.setHeader;
import static test3101.HttpHeaders.Names.DATE;
import test3101.HttpHeaders;
@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Apr 15, 2016

Member

@rnveach ,
thanks a lot for quick reaction on this issue.
Main problem is that Eclipse use non ASCII (http://www.asciitable.com/) approach for sorting of static imports (described in Eclipse bugzilla issue). NetBeans and Idea - ASCII .

We need new option for this Check - "useContainerOrderingForStatic" = "true/false". Default is false (as 2 of 3 IDEs use this approach).

Member

romani commented Apr 15, 2016

@rnveach ,
thanks a lot for quick reaction on this issue.
Main problem is that Eclipse use non ASCII (http://www.asciitable.com/) approach for sorting of static imports (described in Eclipse bugzilla issue). NetBeans and Idea - ASCII .

We need new option for this Check - "useContainerOrderingForStatic" = "true/false". Default is false (as 2 of 3 IDEs use this approach).

@MEZk

This comment has been minimized.

Show comment
Hide comment
@MEZk

MEZk Jul 1, 2016

Contributor

@romani
Should we take into account the value of 'caseSensitive' option when we compare static import containers?

Contributor

MEZk commented Jul 1, 2016

@romani
Should we take into account the value of 'caseSensitive' option when we compare static import containers?

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jul 1, 2016

Member

Should we take into account the value of 'caseSensitive' option when we compare static import containers?

yes

Member

romani commented Jul 1, 2016

Should we take into account the value of 'caseSensitive' option when we compare static import containers?

yes

MEZk added a commit to MEZk/checkstyle that referenced this issue Jul 3, 2016

MEZk added a commit to MEZk/checkstyle that referenced this issue Jul 3, 2016

MEZk added a commit to MEZk/checkstyle that referenced this issue Jul 3, 2016

MEZk added a commit to MEZk/checkstyle that referenced this issue Jul 4, 2016

MEZk added a commit to MEZk/checkstyle that referenced this issue Jul 6, 2016

romani added a commit that referenced this issue Jul 6, 2016

@romani romani added this to the 7.1 milestone Jul 6, 2016

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jul 6, 2016

Member

Fix is merged

Member

romani commented Jul 6, 2016

Fix is merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment