Spotless: Spotless sorts imports with uppercase package names different from eclipse

[staffanf]

HI again!
I'm seeing some reorganized imports between eclipse and spotless.
Due to some legacy third party code, we have import packages starting with upper case. These seems to be sorted differently between Eclipse and Spotless ImportSorter.
I saw that this code was reimplemented from the IntelliJs plugin EclipseCodeFormatter. The latest version of Eclipse Code Formatter seems to do this correctly.

I created a minimum viable example here, https://github.com/staffanf/spotless-tests

[nedtwigg]

Good find! There are pros and cons to fixing this.

Pro: We will match Eclipse's current behavior.
Con: We will introduce noise into projects that have been using Spotless so far.

I think the best approach would be this:

spotless {
    java {
        importOrderLegacy() // the old behavior
        importOrder() // the new behavior
...

This means that new projects get the behavior the "correct" behavior, and old projects can keep the behavior they've had to-date, if they like.

The code in question is here:

https://github.com/diffplug/spotless/tree/master/lib/src/main/java/com/diffplug/spotless/java

Implementing this won't make it to the top of my todo-list, but I'm happy to merge a PR for it.

[jbduncan]

Reminder (prompted by https://github.com/diffplug/spotless/pull/223#discussion_r174295043):

• When implementing new importOrder(), create an option to allow wildcard imports to be converted to full imports, e.g.:
  gradle importOrder().expandingWildcardImports()

[jbduncan]

As an alternative to https://github.com/diffplug/spotless/issues/167#issuecomment-372849937, consider adding an option to the new importOrder() that just outright disallows wildcard imports. (Think Checkstyle's AvoidStarImport option.)

[DJViking]

Would be great to be able to fail the build if wildcard imports are used. However it does seem strange to have validation for Check that cannot be fixed with Apply.

[jbduncan]

@DJViking Indeed... So with hindsight, I think my comment above where I suggested we introduce a mechanism that fails but has no associated fix, is not the right way to go about fixing this.

However, @nedtwigg has mentioned a few times in other issues that it's impossible for Spotless to recognise and and expand wildcard imports without a huge architectural change (i.e., moving away from representing formatting steps as simple Function<String, String>s internally).

So I wonder if in your case, using a tool like Checkstyle is the best that can be done presently... (One can tell it to report an error if it finds wildcard imports, but just to report it, not to automatically fix things as well.)

[DJViking]

Yes, I think Checkstyle would be the best way to do it.

[nedtwigg]

This wouldn't be the only place where spotlessApply fails. ktlint catches lot of errors that it can't fix. Checkstyle is a good way to do it, but I still think it makes sense to implement errorOnWildcard().