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: No violation before or after changes for static import list #5065

Closed
rnveach opened this Issue Sep 3, 2017 · 2 comments

Comments

Projects
None yet
2 participants
@rnveach
Member

rnveach commented Sep 3, 2017

Identified at #5053 (comment) ,

specifically at the code 28e618b#diff-f784beebc72b92982be3042b568387cfR22

With empty line:

$ cat TestClass.java
import static com.puppycrawl.tools.checkstyle.internal.XpathUtil.getXpathItems;

import static org.junit.Assert.assertEquals;

public class TestClass {
    void method() {
    }
}

$ cat TestConfig.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">
    <property name="charset" value="UTF-8"/>

    <module name="TreeWalker">
    <module name="ImportOrder">
      <property name="groups" value="/^java\./,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>

$ java -jar checkstyle-8.2-all.jar -c TestConfig.xml TestClass.java
Starting audit...
Audit done.

Without empty line:

$ cat TestClass.java
import static com.puppycrawl.tools.checkstyle.internal.XpathUtil.getXpathItems;
import static org.junit.Assert.assertEquals;

public class TestClass {
    void method() {
    }
}

$ cat TestConfig.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">
    <property name="charset" value="UTF-8"/>

    <module name="TreeWalker">
    <module name="ImportOrder">
      <property name="groups" value="/^java\./,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>

$ java -jar checkstyle-8.2-all.jar -c TestConfig.xml TestClass.java
Starting audit...
Audit done.

There was no violation with an empty line after import static com.puppycrawl.tools.checkstyle.internal.XpathUtil.getXpathItems; or when the empty line was removed in the PR.
I expected ImportOrder to enforce a specific behavior in this regards that all users should follow. I expected a violation in one of the examples, not both.

djydewang added a commit to djydewang/checkstyle that referenced this issue Sep 8, 2017

djydewang added a commit to djydewang/checkstyle that referenced this issue Sep 8, 2017

djydewang added a commit to djydewang/checkstyle that referenced this issue Sep 8, 2017

djydewang added a commit to djydewang/checkstyle that referenced this issue Sep 9, 2017

djydewang added a commit to djydewang/checkstyle that referenced this issue Sep 9, 2017

djydewang added a commit to djydewang/checkstyle that referenced this issue Sep 10, 2017

djydewang added a commit to djydewang/checkstyle that referenced this issue Sep 12, 2017

djydewang added a commit to djydewang/checkstyle that referenced this issue Sep 12, 2017

djydewang added a commit to djydewang/checkstyle that referenced this issue Sep 13, 2017

djydewang added a commit to djydewang/checkstyle that referenced this issue Sep 14, 2017

djydewang added a commit to djydewang/checkstyle that referenced this issue Sep 15, 2017

romani added a commit that referenced this issue Sep 21, 2017

@romani romani added this to the 8.3 milestone Sep 21, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Sep 21, 2017

Member

fix is merged.

Member

romani commented Sep 21, 2017

fix is merged.

@romani romani closed this Sep 21, 2017

ssp added a commit to ssp/checkstyle that referenced this issue Oct 8, 2017

@romani romani changed the title from No violation before or after changes for static import list to ImportOrder: No violation before or after changes for static import list Mar 18, 2018

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Mar 18, 2018

Member

fix for this issue is hard to simply revert, as extra fix is based on it #5176.
Conflict of interests of users demands become bigger, we should go forward with fixes (no revert).

Member

romani commented Mar 18, 2018

fix for this issue is hard to simply revert, as extra fix is based on it #5176.
Conflict of interests of users demands become bigger, we should go forward with fixes (no revert).

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