bugfix: reorder import in GJF using Google Style

[simschla]

This fixes#1679.

In the origin GJF reflection code (before #1630), we used to call reorderImports(String) which internally expanded to reorderImports(String, Style.GOOGLE) no matter what style the user had configured.

The report in #1679 uses the AOSP style, so the code after #1630 called reorderImports(String, Style.AOSP) (instead of the reorderImports(String) (which is an alias for reorderImports(String, Style.GOOGLE)).

Changing that, resolves the issue. (This PR)

@nedtwigg Underlying, however, is another issue: Calling the actual reorderImports(String, Style.AOSP) on the example file results in a Situation where spotless reorders imports even though they are in the formatting:off and formatting:on section of the file. You can see that if you change the checkIssue1679() test method, to only include the formatters (toggleOffOnPair.in(), gjf, toggleOffOnPair.out()). I did not investigate that further, but could be that there is a general issue with those toggles in the import section?

[big-andy-coates]

Thanks for fixing @simschla!

I did not investigate that further, but could be that there is a general issue with those toggles in the import section?

Is is worth raising a new bug to track this secondary issue?

[simschla]

Is is worth raising a new bug to track this secondary issue?

I think it's worth keeping in mind (and as such raise an issue for that) - but I'm not very at home in that part of the code, so maybe @nedtwigg should weigh in here whenever he finds the time.

[simschla]

BTW @big-andy-coates are you OK with me using your "problem file" in testlib/src/main/resources/combined/issue1679.clean and testlib/src/main/resources/combined/issue1679.dirty directly or would you rather have me create an artificial file?

[big-andy-coates]

Yep, no worries - its OSS anyway

[nedtwigg]

Thanks very much for reporting your issue @big-andy-coates, and thanks for tracking down the very subtle cause @simschla.

As for ignoring the formatter:off tag - if a step removes those tags, then we can't restore their content. We rely on intermediate steps keeping those tags inside the source code.

The real "problem" here is (until recently) we don't actually respect AOSP style when using gjf to do imports. But that has been our behavior for so long that it's essentially a regression for existing users.

If we wanted to respect style when doing GJF import order, we would have to add a second flag for import style, e.g.
.style(AOSP).importOrder(AOSP). Back-compat for existing users is the most important thing, I'm open to supporting alternative import styles as an "enhancement" if somebody wants to contribute that, but I don't think it's very important.

[nedtwigg]

Released in plugin-gradle 6.19.0 and plugin-maven 2.37.0.

[bjrke]

we adapted 6.18 and the new import order inherited from google-java-format since it perfectly works with the gjf plugin of IntelliJ which also changed its import order with the update.
adding importOrder("AOSP") doesn't really help, because you have to declare the import order manually like importOrder('java|javax', 'com.acme', '', '\\#com.acme', '\\#') (just copied from the docs)
an importOrder(AOSP) doesn't exist in the gradle dsl and I don't see a trick to achieve it.

[simschla]

@bjrke I think you misunderstood. the importOrder("AOSP") API does not exist, that was a suggestion of @nedtwigg if we wanted to go the way of exposing these different gjf import orderings.

As of now, you can upgrade to 6.19.0 and get the original gjf import order back, or set a custom import order using importOrder('java', 'com', ...) if you'd like to override it to something else

[bjrke]

are there any plans (different issue or PR) to introduce such a possibility? For the moment we just don't upgrade, but in the long term it would be very useful to have this.

[simschla]

@bjrke AFAIK, there is nothing planned (yet). I suggest you create an Issue describing what the changes are, that you are proposing. Adding them to the API should be quite straightforward, and I'll gladly implement it as soon as I have some time to spare. (Or maybe someone from the community or yourself is willing to step-in in the meantime?)