Spotless build integration plugin

[DanVanAtta]

Following up with spotless from format discussion: #4488 (comment)

My first impression is positive. Having a task that auto-fixes any problems detected is rad 👍

I did hit an error though running the eclipse format file that has an eclipse installation dependency.

Caused by: java.lang.IllegalStateException: Workspace is closed.
        at org.eclipse.core.resources.ResourcesPlugin.getWorkspace(ResourcesPlugin.java:412)
        at org.eclipse.jdt.internal.formatter.DefaultCodeFormatter.createParser(DefaultCodeFormatter.java:332)
        at org.eclipse.jdt.internal.formatter.DefaultCodeFormatter.parseSourceCode(DefaultCodeFormatter.java:317)
        at org.eclipse.jdt.internal.formatter.DefaultCodeFormatter.prepareFormattedCode(DefaultCodeFormatter.java:213)

For reference that was with this config:

  spotless {
        java {
            importOrderFile rootProject.file('.eclipse/format/triplea.importorder')
            eclipseFormatFile rootProject.file('.eclipse/format/triplea_java_eclipse_format_style.xml')

            trimTrailingWhitespace()
            endWithNewline()

            custom 'Lambda fix', { it.replace('} )', '})').replace('} ,', '},') }
        }
    }

Either of these two configs worked, and output is attached below.

    spotless {
        java {
            googleJavaFormat()
        }
    }

    spotless {
        java {
            googleJavaFormat()
            importOrderFile rootProject.file('.eclipse/format/triplea.importorder')
        }
    }

Trying out google-java-format config, it seemed to have worked well and had reasonable output. I pushed a POC to: d89f394

The output looked reasonable:

---

Opening this ticket to talk about Spotless specifically and keep the discussion threads clean. Regardless of !4488 this looks like something we want.

[RoiEXLab]

👍

[ssoloff]

The IllegalStateException is due to diffplug/spotless#191. It can be fixed by using the latest version of the Eclipse formatter supported by Spotless (currently 4.8.0 for Spotless Gradle plugin 3.16.0; 3.17.0 will support 4.9.0):

eclipse('4.8.0').configFile rootProject.file('.eclipse/format/triplea_java_eclipse_format_style.xml')

[DanVanAtta]

Great, that removes that as a dealbreaker (did not try it, but looks promising). I suppose we are waiting on #4488 now to know which formatter to configure.

[ssoloff]

Already verified it when I posted that comment--seems to work as expected.

However, I had to add a blank entry to the import order file (i.e. 4=) in order for Spotless to correctly place "all other" imports after the existing entries. That seems to be a slight difference in behavior between the command-line formatter and the IDE, as the IDE appears to do that by default. I did re-import the modified import order file into the IDE, and it shows the blank entry as * - all unmatched type imports. So, after making that change, there shouldn't be any differences between Spotless and the IDE (at least for Eclipse).

[DanVanAtta]

@ssoloff if you'd like to submit a PR that adds the spotless config, please go for it. We can examine later when we should turn it to 'enforcing' mode to actually fail builds, in the meantime it is very valuable to have a command that would auto-fix what would otherwise be build problems.

If you're short on time, let me know and I can follow-up instead.

[ssoloff]

I found an issue with Spotless related to import ordering.

Basically, Spotless sorts imports without an implicit dot (.) after each token, while Eclipse (and presumably IDEA) sorts imports with an implicit dot after each token. For example, consider the following imports:

import java.util.Collection;

import org.junit.jupiter.api.Nested;

import javafx.stage.Stage;

Using Eclipse and our current import order file, those imports would remain unchanged when running Organize Imports. However, using Spotless, they get reordered as follows:

import java.util.Collection;
import javafx.stage.Stage;

import org.junit.jupiter.api.Nested;

One can get Spotless to follow the Eclipse order by either:

1. Add a trailing dot to each entry in the import order file. Note, however, that this renders the import order file un-importable by Eclipse.
2. Manually specify the import order in the Gradle configuration using importOrder (with trailing dots) instead of importOrderFile. This has the disadvantage that the import order is maintained in two different files (one for Gradle and one for IDEs).
3. Add new entries to the import order file to ensure packages that start with java, javax, org, and com are placed at the end of the import list. This will slightly change the format by adding a newline between these groups where none existed before. This will also require an update to the Checkstyle rules to accommodate the new import groups.

It really seems that this is a bug in Spotless because every other tool I've come across treats the import order tokens as a complete, individual package name, not simply a substring to match at the beginning of the fully-qualified package name. However, I was unable to find any open or closed issues in the Spotless tracker related to this behavior. It's possible there's some other configuration option that changes this behavior to match the IDE, but I couldn't find it.

[ssoloff]

Another possibility to get Spotless and the IDEs to match might be to read the import order file, append a period to each non-empty value, convert the values to a list sorted by key and pass that to importOrder. Something like:

spotless {
    importOrder spotlessImportOrderFromFile(rootProject.file('.eclipse/format/triplea.importorder'))
}

where spotlessImportOrderFromFile() is a function we have to write.

[ssoloff]

@ssoloff if you'd like to submit a PR that adds the spotless config, please go for it.

#4569

[DanVanAtta]

Do you know if Spotless has a default import order it can enforce? @ssoloff

[ssoloff]

Do you know if Spotless has a default import order it can enforce?

I removed the importOrder configuration and messed up a few imports in one file. Running spotlessCheck produces no warnings and running spotlessApply produces no changes.

If I change the configuration to importOrder() (i.e. with no arguments), running spotlessCheck raises an error that states "You have a misbehaving rule which can't make up its mind," while running spotlessApply succeeds and places all imports into a single group ordered lexicographically. If I run spotlessCheck at this point, it produces no messages and the build succeeds.

[ssoloff]

As reported in #4586 (comment), it appears the Spotless Gradle plugin automatically adds the spotlessCheck task as a dependency of the check task. Therefore, while unintentional, Spotless is now integrated with our build process, and Travis builds will fail if code is not properly formatted. Given that that appears to have been the goal of this issue, I'm going to close it now.