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 does not work #377

Open
appraveen opened this issue Feb 8, 2018 · 19 comments
Open

customImportOrder does not work #377

appraveen opened this issue Feb 8, 2018 · 19 comments
Labels
code-style-importer Related to the code style import functionality

Comments

@appraveen
Copy link

Thank you @mqzry for the feature. There is a bug from the pull #363 which prevents customImportOrder to work properly.

private void parseImportOrderRules(@NotNull String value) {
    String[] groups = value.split(value);
    customImportOrder = Arrays.stream(groups).map(ImportGroup::valueOf).collect(Collectors.toList());
}

Using value.split("###") might fix the issue. [Spec].(http://checkstyle.sourceforge.net/apidocs/com/puppycrawl/tools/checkstyle/checks/imports/CustomImportOrderCheck.html)

@mqzry
Copy link
Contributor

mqzry commented Feb 8, 2018

This is already fixed by @yeoji in #372

@appraveen
Copy link
Author

Looks like #372 change is already released in 5.16.3. But, the below configuration is not working in Intellij IDEA 2017.3.3

 <module name="CustomImportOrder">
            <property name="standardPackageRegExp" value="^(java|javax)\."/>
            <property name="specialImportsRegExp" value="^org\."/>
            <property name="sortImportsInGroupAlphabetically" value="true"/>
            <property name="separateLineBetweenGroups" value="true"/>
            <property name="customImportOrderRules" value="STANDARD_JAVA_PACKAGE###SPECIAL_IMPORTS###THIRD_PARTY_PACKAGE###STATIC"/>
        </module>

@yeoji
Copy link
Contributor

yeoji commented Feb 8, 2018

@appraveen Is there an error? What doesn't work?

I just tested using the configuration you have provided and it seems alright. Imported correctly and I had no errors. I am on IntelliJ 2017.2.5 though, not sure if that matters.

@appraveen
Copy link
Author

I am not getting any error, when I import, it says, "Checkstyle configuration settings were imported to Default copy". I don't see the order reflected in the dialog and reformatting the code does not pick up the order mentioned in checkstyle.xml

Checstyle -IDEA
image

Code Style Preference

image

@yeoji
Copy link
Contributor

yeoji commented Feb 9, 2018

@appraveen Hmm this is what I got after importing and it's correct.
screen shot 2018-02-09 at 7 59 10 pm

Is your plugin updated to the latest version? Does the import layout table change at all after importing?

@e-tyryshkin
Copy link

@yeoji Same issue for 2017.3.4. Doesn't import counts to use "*" as well. Maybe something had changed in the API since 2017.2?

@yeoji
Copy link
Contributor

yeoji commented Feb 21, 2018

@appraveen @e-tyryshkin You're right, I just upgraded to 2017.3.4 to test this out and it doesn't import properly! Hmm since the plugin is developed with 2016.1, I'm not too sure how to go about figuring out what has changed/fixing this. I'll try and see if I can find anything out though.

@jshiell
Copy link
Owner

jshiell commented Feb 23, 2018

This is a standard problem, I fear - sometime new versions can do odd things. But people tend to get a bit miffed if we follow the release, as a lot of large orgs don't or can't upgrade immediately.

I'd try switching the version referenced in build.gradle to match the current release and see if anything shows up. Even if we can't release with it built against that SDK, perhaps the results will be illuminating.

@appraveen
Copy link
Author

This should not be a problem. Looking at the Intellij Version 2017.2.5 to 2017.3.4, API seem to be broken between their minor version upgrade. I don't know have any knowledge on their API. Perhaps, you can create an issue with Intellij to check why this is broken on 2017.3.4

If something works in 2017.2.5 i would expect it to work on all 2017.x.y versions where x>=2 (y>=5 when x==2, otherwise y can be '*')

@mmoayyed
Copy link

mmoayyed commented Jul 13, 2018

I am using the plugin version: 5.20.0 with Checkstyle 8.11 in IDEA 2018.2:

IntelliJ IDEA 2018.2 EAP (Ultimate Edition)
Build #IU-182.3684.2, built on July 10, 2018
JRE: 1.8.0_152-release-1248-b8 x86_64
JVM: OpenJDK 64-Bit Server VM by JetBrains s.r.o
macOS 10.13.5

Here is an a snippet that demonstrates a CustomImportOrder check:

<module name="CustomImportOrder">
            <property name="severity" value="error"/>
            <property name="customImportOrderRules"
                      value="SAME_PACKAGE(3)###THIRD_PARTY_PACKAGE###SPECIAL_IMPORTS###STANDARD_JAVA_PACKAGE###STATIC"/>
            <property name="specialImportsRegExp" value="^javax\."/>
            <property name="standardPackageRegExp" value="^java\."/>
            <property name="sortImportsInGroupAlphabetically" value="true"/>
            <property name="separateLineBetweenGroups" value="false"/>
</module>

Full example available here

Importing the configuration from Preferences / Code Style / Java presents:

image

Then, I changed the rule to the following, taking out severity:

<module name="CustomImportOrder">
            <property name="customImportOrderRules"
                      value="SAME_PACKAGE(3)###THIRD_PARTY_PACKAGE###SPECIAL_IMPORTS###STANDARD_JAVA_PACKAGE###STATIC"/>
            <property name="specialImportsRegExp" value="^javax\."/>
            <property name="standardPackageRegExp" value="^java\."/>
            <property name="sortImportsInGroupAlphabetically" value="true"/>
            <property name="separateLineBetweenGroups" value="false"/>
</module>

Second error presents itself complaining about missing enum constant for SAME_PACKAGE(3). So I switched the rule to:

<module name="CustomImportOrder">
            <property name="customImportOrderRules"
                      value="SAME_PACKAGE###THIRD_PARTY_PACKAGE###SPECIAL_IMPORTS###STANDARD_JAVA_PACKAGE###STATIC"/>
            <property name="specialImportsRegExp" value="^javax\."/>
            <property name="standardPackageRegExp" value="^java\."/>
            <property name="sortImportsInGroupAlphabetically" value="true"/>
            <property name="separateLineBetweenGroups" value="false"/>
</module>

In summary and to confirm:

  1. Seems like severity is not supported by the plugin?
  2. Seems like SAME_PACKAGE(n) is not supported by the plugin?

PS Happy to open up a separate issue, if it helps further.

@jshiell
Copy link
Owner

jshiell commented Jul 13, 2018

@mmoayyed thanks for the report. What version of Checkstyle are you using with this config? At first glance that exception seems to be coming from Checkstyle, not the plugin. In general the plugin just concerns itself with munging the file paths - most of the work, including the majority of parsing, is still done by the underlying Checkstyle instance.

@mmoayyed
Copy link

Apologies. I should have mentioned that I use Checkstyle 8.11. (Corrected the original post).

It's worth noting that with the original CustomImportOrder rule (with severity and such defined), the plugin is able to perfectly recognize the Checkstyle rules and examine the code, reporting back faulty import orders. That bit works just fine. The issue seems to only present itself when the rules are about to be imported into IDEA as a Code Style profile.

@jshiell
Copy link
Owner

jshiell commented Jul 14, 2018

@mmoayyed thanks - and another thank you for your excellent bug report, it's much appreciated!

The good news: I can reproduce it. The file works fine for addition & scanning, but fails when imported as a code style. The bad news: the code style work was a contribution, so it's a bit of a mystery. But I'll see what I can dig up 😄

@jshiell
Copy link
Owner

jshiell commented Jul 14, 2018

I've added some fixes - severity is now supported (and ignored), and SAME_PACKAGE(n) should now work (it wasn't checking for the (n) previously). Released as 5.21.0.

@mmoayyed
Copy link

Outstanding. Thank you very much for your time. I'll give the new release a try and will report back.

@mmoayyed
Copy link

I should report back the latest version of the plugin that is 5.21.0 resolves this issue. Once again, thank you very much for your help. I hope to get better at this gradually and start contributing more :)

@chriswhite199
Copy link

chriswhite199 commented Sep 22, 2018

I'm seeing a similar issue with 5.22.1:

screen shot 2018-09-21 at 8 29 31 pm

<module name="CustomImportOrder">
    <property name="sortImportsInGroupAlphabetically" value="true"/>
    <property name="separateLineBetweenGroups" value="false"/>
    <property name="customImportOrderRules" value="STATIC###THIRD_PARTY_PACKAGE"/>
</module>

So the checkstyle.xml is saying static first, but the Import Layout section of the Code Style after importing (with no errors reported) is still showing the default IntelliJ order (statics last and separate java & javax sections).

The Checkstyle plugin is configured with the right version (8.12).

Taking another look - i did find that you need to make sure the Scheme selected in the Code Style -> Java preferences is set to Project (and not anything related to the IDE) - in which case it half gets the Import Layout right (the java & javax sections are the issue now, they shouldn't be present):

screen shot 2018-09-21 at 10 02 35 pm

@romandvoskin-circle
Copy link

romandvoskin-circle commented Aug 5, 2021

This is still an issue in 5.54.0. Here's a snippet of the imports section I was using

<module name="CustomImportOrder">
            <property name="customImportOrderRules"
                      value="THIRD_PARTY_PACKAGE###SPECIAL_IMPORTS###STANDARD_JAVA_PACKAGE###STATIC"/>
            <property name="specialImportsRegExp" value="^javax\."/>
            <property name="standardPackageRegExp" value="^java\."/>
            <property name="sortImportsInGroupAlphabetically" value="true"/>
            <property name="separateLineBetweenGroups" value="false"/>
</module>

I found a workaround:

  1. Import the checkstyle settings into Code Style | Java | Scheme IDE | Gear | Import Scheme
  2. Manually set the imports tab | Import Layout until the CheckStyle passes
  3. Gear | Export | Editorconfig file
    This exports the resulting settings as .editorconfig (to help share with your team)

@Haarolean
Copy link

Haarolean commented Sep 7, 2021

This is still an issue in 2021 (5.55.1).

This is still an issue in 5.54.0. Here's a snippet of the imports section I was using

<module name="CustomImportOrder">
            <property name="customImportOrderRules"
                      value="THIRD_PARTY_PACKAGE###SPECIAL_IMPORTS###STANDARD_JAVA_PACKAGE###STATIC"/>
            <property name="specialImportsRegExp" value="^javax\."/>
            <property name="standardPackageRegExp" value="^java\."/>
            <property name="sortImportsInGroupAlphabetically" value="true"/>
            <property name="separateLineBetweenGroups" value="false"/>
</module>

I found a workaround:

  1. Import the checkstyle settings into Code Style | Java | Scheme IDE | Gear | Import Scheme
  2. Manually set the imports tab | Import Layout until the CheckStyle passes
  3. Gear | Export | Editorconfig file
    This exports the resulting settings as .editorconfig (to help share with your team)

The huge downside is that once you edit the checkstyle again you have to edit the order again.

@jshiell jshiell added the code-style-importer Related to the code style import functionality label Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code-style-importer Related to the code style import functionality
Projects
None yet
Development

No branches or pull requests

9 participants