Skip to content
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

CustomImportOrder check broken #6386

Closed
ringmaster217 opened this issue Jan 17, 2019 · 3 comments
Closed

CustomImportOrder check broken #6386

ringmaster217 opened this issue Jan 17, 2019 · 3 comments

Comments

@ringmaster217
Copy link

When using the google java formatter with the google_checks.xml checkstyle configuration, checkstyle incorrectly flags import statements as being out of order. It appears that the CustomImportOrder module is incorrectly attempting to validate more import groups than are configured.

For example, this import block

package com.example;

import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.DeserializationContext;
import com.fasterxml.jackson.databind.JsonDeserializer;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import java.io.IOException;
import java.net.URI;
import java.time.Instant;
import java.time.ZoneId;
import java.util.Arrays;
import java.util.Objects;
import java.util.Optional;
import org.springframework.http.HttpEntity;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpMethod;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.http.ResponseEntity;
import org.springframework.lang.Nullable;
import org.springframework.web.client.HttpStatusCodeException;
import org.springframework.web.client.RestTemplate;

produces this checkstyle output

[WARN] /home/user1/java-workspace/java-project/src/main/java/com/example/WsClient.java:13: 'java.io.IOException' should be separated from previous import group. [CustomImportOrder]
[WARN] /home/user1/java-workspace/java-project/src/main/java/com/example/WsClient.java:20: Import statement for 'org.springframework.http.HttpEntity' is in the wrong order. Should be in the 'THIRD_PARTY_PACKAGE' group, expecting not assigned imports on this line. [CustomImportOrder]
[WARN] /home/user1/java-workspace/java-project/src/main/java/com/example/WsClient.java:21: Import statement for 'org.springframework.http.HttpHeaders' is in the wrong order. Should be in the 'THIRD_PARTY_PACKAGE' group, expecting not assigned imports on this line. [CustomImportOrder]
[WARN] /home/user1/java-workspace/java-project/src/main/java/com/example/WsClient.java:22: Import statement for 'org.springframework.http.HttpMethod' is in the wrong order. Should be in the 'THIRD_PARTY_PACKAGE' group, expecting not assigned imports on this line. [CustomImportOrder]
[WARN] /home/user1/java-workspace/java-project/src/main/java/com/example/WsClient.java:23: Import statement for 'org.springframework.http.HttpStatus' is in the wrong order. Should be in the 'THIRD_PARTY_PACKAGE' group, expecting not assigned imports on this line. [CustomImportOrder]
[WARN] /home/user1/java-workspace/java-project/src/main/java/com/example/WsClient.java:24: Import statement for 'org.springframework.http.MediaType' is in the wrong order. Should be in the 'THIRD_PARTY_PACKAGE' group, expecting not assigned imports on this line. [CustomImportOrder]
[WARN] /home/user1/java-workspace/java-project/src/main/java/com/example/WsClient.java:25: Import statement for 'org.springframework.http.ResponseEntity' is in the wrong order. Should be in the 'THIRD_PARTY_PACKAGE' group, expecting not assigned imports on this line. [CustomImportOrder]
[WARN] /home/user1/java-workspace/java-project/src/main/java/com/example/WsClient.java:26: Import statement for 'org.springframework.lang.Nullable' is in the wrong order. Should be in the 'THIRD_PARTY_PACKAGE' group, expecting not assigned imports on this line. [CustomImportOrder]
[WARN] /home/user1/java-workspace/java-project/src/main/java/com/example/WsClient.java:27: Import statement for 'org.springframework.web.client.HttpStatusCodeException' is in the wrong order. Should be in the 'THIRD_PARTY_PACKAGE' group, expecting not assigned imports on this line. [CustomImportOrder]

It looks like the CustomImportOrder module is picking up the java.* imports as the STANDARD_JAVA_PACKAGE group, even though only STATIC and THIRD_PARTY_PACKAGE are configured in google_checks.xml. Packages imported after the java.* packages are then incorrectly flagged as being in the wrong group.

@rnveach
Copy link
Member

rnveach commented Jan 18, 2019

@ringmaster217 I cannot reproduce your violations with the check from the google_config in master (https://github.com/checkstyle/checkstyle/blob/master/src/main/resources/google_checks.xml#L193-L197).

$ cat TestClass.java
package com.example;

import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.DeserializationContext;
import com.fasterxml.jackson.databind.JsonDeserializer;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import java.io.IOException;
import java.net.URI;
import java.time.Instant;
import java.time.ZoneId;
import java.util.Arrays;
import java.util.Objects;
import java.util.Optional;
import org.springframework.http.HttpEntity;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpMethod;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.http.ResponseEntity;
import org.springframework.lang.Nullable;
import org.springframework.web.client.HttpStatusCodeException;
import org.springframework.web.client.RestTemplate;

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="CustomImportOrder">
            <property name="sortImportsInGroupAlphabetically" value="true"/>
            <property name="separateLineBetweenGroups" value="true"/>
            <property name="customImportOrderRules" value="STATIC###THIRD_PARTY_PACKAGE"/>
        </module>
    </module>
</module>

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

Please see https://checkstyle.org/report_issue.html#How_to_report_a_bug.3F .

When using the google java formatter

We verify our configuration against the google style guide and not the formatter.

@rnveach
Copy link
Member

rnveach commented Jan 18, 2019

I was able to reproduce the first post results with the google config from 2 years ago.

81ad459#diff-c55cd603ef6597463971db3f33f4f4c8

$ cat TestClass.java
package com.example;

import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.DeserializationContext;
import com.fasterxml.jackson.databind.JsonDeserializer;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import java.io.IOException;
import java.net.URI;
import java.time.Instant;
import java.time.ZoneId;
import java.util.Arrays;
import java.util.Objects;
import java.util.Optional;
import org.springframework.http.HttpEntity;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpMethod;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.http.ResponseEntity;
import org.springframework.lang.Nullable;
import org.springframework.web.client.HttpStatusCodeException;
import org.springframework.web.client.RestTemplate;

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="CustomImportOrder">
<property name="specialImportsRegExp" value="com.google"/>
            <property name="sortImportsInGroupAlphabetically" value="true"/>
            <property name="customImportOrderRules" value="STATIC###SPECIAL_IMPORTS###THIRD_PARTY_PACKAGE###STANDARD_JAVA_PACKAGE"/>
        </module>
    </module>
</module>

$ java -jar checkstyle-8.16-all.jar -c TestConfig.xml TestClass.java
Starting audit...
[ERROR] TestClass.java:10: 'java.io.IOException' should be separated from previous import group by one line. [CustomImportOrder]
[ERROR] TestClass.java:17: Import statement for 'org.springframework.http.HttpEntity' is in the wrong order. Should be in the 'THIRD_PARTY_PACKAGE' group, expecting not assigned imports on this line. [CustomImportOrder]
[ERROR] TestClass.java:18: Import statement for 'org.springframework.http.HttpHeaders' is in the wrong order. Should be in the 'THIRD_PARTY_PACKAGE' group, expecting not assigned imports on this line. [CustomImportOrder]
[ERROR] TestClass.java:19: Import statement for 'org.springframework.http.HttpMethod' is in the wrong order. Should be in the 'THIRD_PARTY_PACKAGE' group, expecting not assigned imports on this line. [CustomImportOrder]
[ERROR] TestClass.java:20: Import statement for 'org.springframework.http.HttpStatus' is in the wrong order. Should be in the 'THIRD_PARTY_PACKAGE' group, expecting not assigned imports on this line. [CustomImportOrder]
[ERROR] TestClass.java:21: Import statement for 'org.springframework.http.MediaType' is in the wrong order. Should be in the 'THIRD_PARTY_PACKAGE' group, expecting not assigned imports on this line. [CustomImportOrder]
[ERROR] TestClass.java:22: Import statement for 'org.springframework.http.ResponseEntity' is in the wrong order. Should be in the 'THIRD_PARTY_PACKAGE' group, expecting not assigned imports on this line. [CustomImportOrder]
[ERROR] TestClass.java:23: Import statement for 'org.springframework.lang.Nullable' is in the wrong order. Should be in the 'THIRD_PARTY_PACKAGE' group, expecting not assigned imports on this line. [CustomImportOrder]
[ERROR] TestClass.java:24: Import statement for 'org.springframework.web.client.HttpStatusCodeException' is in the wrong order. Should be in the 'THIRD_PARTY_PACKAGE' group, expecting not assigned imports on this line. [CustomImportOrder]
[ERROR] TestClass.java:25: Import statement for 'org.springframework.web.client.RestTemplate' is in the wrong order. Should be in the 'THIRD_PARTY_PACKAGE' group, expecting not assigned imports on this line. [CustomImportOrder]
Audit done.
Checkstyle ends with 10 errors.

@rnveach
Copy link
Member

rnveach commented Jan 18, 2019

Because this has already been fixed for 2 years now I am closing the issue.
Issue can be reopened if new information is provided.

@rnveach rnveach closed this as completed Jan 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants