ImportSorter eats comments

[jochenberger]

The ImportSorter removes comments inside the imports block, i.e.:
before:

package org.example;

import java.util.List;
// this is very important
import java.util.Set;

public class Test {

}

after:

package org.example;

import java.util.List;
import java.util.Set;

public class Test {

}

[nedtwigg]

If a comment is to the right of an import, it's clear who it should get sorted with. But for your example, should it count as above its target or below? One possible option:

importSorter(COMMENT_SORTWITH_ABOVE)
importSorter(COMMENT_SORTWITH_BELOW)
importSorter(COMMENT_REMOVE) // the current behavior
importSorter(COMMENT_ERROR)  // a safer default

IMO "no comments in the imports" is a reasonable styleguide. Our importsorter has several issues, but I wonder how Eclipse or IntelliJ handle this. Happy to take a PR to improve this behavior, but it's extremely low on my todo list.

[jochenberger]

Yes, I can see how one would consider comments in between import statements bad style or practice, but in our case, we need them as markers during code generation.
The current behavior however is a problem in my opinion, because a sort step should not remove any code, not even comments. It seems as if you agree, since you added the bug label. :-)
Most (if not all) of the code I know seems to treat single-line comments as belonging to the line (or, more generally speaking, code) below.
However, it just came to my mind that considering the comment to belong to the line below wouldn't actually help with our current use case.
I already said that we generate code. There's a mechanism to be able to add custom code that is left alone when re-generating. Our imports look like this:

package org.example;

import java.util.List;
import java.util.Set;
// begin user-defined imports
import java.util.ArrayList;
import org.example.PersonUtilities;
// end user-defined imports

public class Test {
  ....
}

Just treating the comments as a two-line import statement and using the regular import sorting won't help because it would move the List and Set imports into the user-defined imports.
So, I can think of three solutions that would help us here, ordered from the (IMO) worst to best:

• being able to specify different steps for different targets, i.e. exclude the import sorter for our generated sources
• being able to specify importSorter(COMMENT_NOOP) to just skip the sorting if comments are found
• Separating the FormatterStep from the sorter, i.e. extract the actual sorting into a separate class that you can override, either with a class on the classpath (importSorterCustom(org.example.CustomSorter)) or even inline (importSorterCustom({List<String> lines-> lines.reverse()} as com.diffplug.spotless.java.IImportSorter))

I can imaging us helping with a PR if we settle on a desired solution.

[nedtwigg]

being able to specify different steps for different targets, i.e. exclude the import sorter for our generated sources

This is very easy, just one piece missing. If we make this method public:

spotless/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtension.java

Line 217 in 608af5c

private <T extends FormatExtension> void configure(String name, Class<T> clazz, Action<T> configure) {

then you could do this:

spotless {
  java {
    targetExclude 'src/main/java-autogenerated/**'
    googleJavaFormat()
    importSorter()
  }
  configure 'javaAutogenerated', com.diffplug.gradle.spotless.JavaExtension, {
    target 'src/main/java-autogenerated/**'
    googleJavaFormat()
  }
}

Should probably rename "configure" to "format".

being able to specify importSorter(COMMENT_NOOP) to just skip the sorting if comments are found

COMMENT_NOOP is a good idea - easier to implement than COMMENT_SORTWITH_ABOVE/SORTWITH_BELOW.

Separating the FormatterStep from the sorter, i.e. extract the actual sorting into a separate class that you can override, either with a class on the classpath

I think it would be better to just copy-paste the existing sorter and modify to make a custom formatter step, rather than burden the import sorter with a public API to maintain for a very niche usecase.

A PR for any viable solution is welcome.

[jochenberger]

This issue is no longer relevant to us, so feel free to close it.