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

After upgrade 5.41 -> 5.42 formatter adds additional \n between static and normal imports #500

Closed
DorianOlympia opened this issue Sep 14, 2020 · 3 comments

Comments

@DorianOlympia
Copy link

DorianOlympia commented Sep 14, 2020

After upgrade 5.41 -> 5.42 formatter adds two \n between static and normal imports (according to xml checkstyle i should be once):

import static X;
import Y;"

is refactored to:

import static X;


import Y;

even though according to checkstyle xml it should be:

import static X;

import Y;

This leads to inconsistency between checkstyle and formatter rules.

@jshiell
Copy link
Owner

jshiell commented Sep 15, 2020

Curious. There have been no changes in the formatter for some releases, so no idea what has triggered this.

Now the bad bit: the formatter was a contribution, so I don't know the code well, nor TBH do I have much interest in it. So the chances of this getting fixed by me aren't high. Sorry.

@dominicboeck
Copy link

I took a look at the source code since this is a rather annoying issue. I hope I can offer a bit more context and a workaround with this comment.

I am using the following rule in my checkstyle configuration (by default this is actually part of the google_checks.xml):

<module name="CustomImportOrder">
    <property name="sortImportsInGroupAlphabetically" value="true"/>
    <property name="separateLineBetweenGroups" value="true"/>
    <property name="customImportOrderRules" value="STATIC###THIRD_PARTY_PACKAGE"/>
    <property name="tokens" value="IMPORT, STATIC_IMPORT, PACKAGE_DEF"/>
</module>

Upon importing this checkstyle configuration in IntelliJ the formatter creates the following Import Layout table (IntelliJ settings > Editor > Code Style > Java > Imports > Import Layout):

Static Package With Subpackages
import static all other imports
<blank line>
import .*
<blank line>
import all other imports

However, after closing and reopening the IntelliJ settings the Import Layout table changes as follows:

Static Package With Subpackages
import static all other imports
<blank line>
<blank line>
import all other imports

This is exactly the behavior mentioned by @DorianOlympia. I did not look into the actual reason why this is happening, but as a workaround it is possible to remove THIRD_PARTY_PACKAGE from the customImportOrderRules property:

<module name="CustomImportOrder">
    <property name="sortImportsInGroupAlphabetically" value="true"/>
    <property name="separateLineBetweenGroups" value="true"/>
    <property name="customImportOrderRules" value="STATIC"/>
    <property name="tokens" value="IMPORT, STATIC_IMPORT, PACKAGE_DEF"/>
</module>

Looking into the checkstyle documentation for CustomImportOrder this actually makes sense, due to the default value of THIRD_PARTY_PACKAGE. Combined with the program flow starting from line 93 in CustomImportOrderImporter.java this causes the creation of import .* followed by <blank line>. Removing THIRD_PARTY_PACKAGE therefore leads to the expected Import Layout:

Static Package With Subpackages
import static all other imports
<blank line>
import all other imports

@jshiell
Copy link
Owner

jshiell commented Apr 6, 2021

Thanks @dominicboeck - lovely work!

I've made a change so that THIRD_PARTY_PACKAGE/SPECIAL_IMPORTS will be ignored if thirdPartyPackageRegExp/specialImportsRegExp are undefined, which I believe should avoid this nastiness.

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

3 participants