GoogleJavaFormat entry point does not sort imports or remove unused

[sormuras]

The current entry point in spotless/java/GoogleJavaFormat is the Formatter.formatSource method which lacks fixing of import order and removal of unused imports.

If the entry point was changed to Main.main, the googleJavaFormat() step would work as expected.

#28 would partly be solved - if one uses googleJavaFormat()

[nedtwigg]

Looks like main calls FormatFileCallable.

Here's an excerpt:

  @Override
  public String call() throws FormatterException {
    if (parameters.fixImportsOnly()) {
      return fixImports(input);
    }

    String formatted =
        new Formatter(options).formatSource(input, characterRanges(input).asRanges());
    formatted = fixImports(formatted);
    return formatted;
  }

  private String fixImports(String input) throws FormatterException {
    input =
        RemoveUnusedImports.removeUnusedImports(
            input,
            parameters.removeJavadocOnlyImports()
                ? JavadocOnlyImports.REMOVE
                : JavadocOnlyImports.KEEP);
    input = ImportOrderer.reorderImports(input);
    return input;
  }

Which points us to this gem in RemoveUnusedImports:

public static String removeUnusedImports(String contents, JavadocOnlyImplorts)

So two action items:

1. It looks like our google-java-format is missing some key pieces, and adding them will change the formatting of files to what users actually expect.
2. We can use removeUnusedImports from google-java-format for the other import ordering step as well.

Here's the tricky bit. Right now, spotless has built-in support for the eclipse formatter (we bundle the necessary class files). google-java-format also uses these eclipse libraries, but uses older ones. To keep the different versions separate, we have to do some classloader gymnastics:

That's the reason why we interact with google-java-format in such a strange way:
https://github.com/diffplug/spotless/blob/master/src/main/java/com/diffplug/gradle/spotless/java/GoogleJavaFormat.java

So, in order to call these methods, we'll have to do more classloader magic. Difficult, but not impossible. Happy to take a PR against master which does this. For now, main focus is on big perf improvements in 3.0 which is likely to take at least 2 more weeks. 3.0 bump is a good time to take on the semi-breaking change that fixing google-java-format represents anyway.

[sormuras]

Maybe, action item 1. can partly be resolved over at google-java-format -- see referenced issue above. If it gets merged, the new entry point in Formatter can be used without doing too much classloader magic.

[jbduncan]

I see that a fix has been implemented in 44b7830 and 75392f8. Well done @nedtwigg!

Have you confirmed that it works with google-java-format-1.0? I believe that 1.0 may not have functionality for reordering imports and removing unused ones.

[nedtwigg]

Haven't confirmed, but I'm fine with setting 1.1 as the lowest supported release.

[jbduncan]

Sounds like a plan to me!

I'm personally happy for this issue to be closed now, but I wonder if @sormuras has any other thoughts before we do so?

[nedtwigg]

I like to close things with "Released in 3.0.0", and we've still got a few loose-ends before we get there, I think :)

[nedtwigg]

Fixed in 3.0.0.BETA2 #63

[243826]

How does it work?

I have these imports...

import java.io.IOException;
import java.security.KeyPair;
import java.security.NoSuchAlgorithmException;
import java.security.spec.InvalidKeySpecException;

import javax.validation.constraints.NotNull;

import com.xxx.common.DAM;

they get reformatted as

import com.xxx.common.DAM;
import java.io.IOException;
import java.security.KeyPair;
import java.security.NoSuchAlgorithmException;
import java.security.spec.InvalidKeySpecException;
import javax.validation.constraints.NotNull;

with the following mvn config:

            <plugin>
              <groupId>com.diffplug.spotless</groupId>
              <artifactId>spotless-maven-plugin</artifactId>
              <version>1.27.0</version>
              <configuration>
                <java>
                  <licenseHeader>
                    <file>${basedir}/toolsettings/license-header</file>
                  </licenseHeader>
                  <googleJavaFormat>
                    <version>1.7</version>
                  </googleJavaFormat>
                  <removeUnusedImports />
                </java>
                <importOrder>
                  <order>java,javax,org,com,com.cisco,com.xxx,com.xxx.common</order>
                </importOrder>
              </configuration>
            </plugin>

Obviously, not what I intended. Could someone comment what am I doing wrong?

[nedtwigg]

I think the <importOrder></...> should be inside the <java>...</java> part. See this example.

[243826]

Thank you! It worked and note to myself to be a little more thorough :-)

…

On Mon, Jan 6, 2020 at 8:26 PM Ned Twigg ***@***.***> wrote: I think the <importOrder></...> should be inside the <java>...</java> part. See this example <https://github.com/diffplug/spotless/tree/master/plugin-maven#applying-to-java-source> . — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#48?email_source=notifications&email_token=AAUT3Q4CYN7TF7FSWWF7D2LQ4QAAPA5CNFSM4CU2WGSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIHUZCQ#issuecomment-571427978>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAUT3Q6CMQMKMF5OF3AYDTDQ4QAAPANCNFSM4CU2WGSA> .