Implement AOSP import ordering.

eaftan · ronshapiro · commit d40d0d69b89c · 2019-05-05T12:33:23.000-04:00
Fixes google#319 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=246145469
diff --git a/core/src/main/java/com/google/googlejavaformat/java/FormatFileCallable.java b/core/src/main/java/com/google/googlejavaformat/java/FormatFileCallable.java
@@ -55,7 +55,7 @@ private String fixImports(String input) throws FormatterException {
       input = RemoveUnusedImports.removeUnusedImports(input);
     }
     if (parameters.sortImports()) {
-      input = ImportOrderer.reorderImports(input);
+      input = ImportOrderer.reorderImports(input, options.style());
     }
     return input;
   }
diff --git a/core/src/main/java/com/google/googlejavaformat/java/Formatter.java b/core/src/main/java/com/google/googlejavaformat/java/Formatter.java
@@ -216,7 +216,7 @@ public String formatSource(String input) throws FormatterException {
    *     Google Java Style Guide - 3.3.3 Import ordering and spacing</a>
    */
   public String formatSourceAndFixImports(String input) throws FormatterException {
-    input = ImportOrderer.reorderImports(input);
+    input = ImportOrderer.reorderImports(input, options.style());
     input = RemoveUnusedImports.removeUnusedImports(input);
     String formatted = formatSource(input);
     formatted = StringWrapper.wrap(formatted);
diff --git a/core/src/main/java/com/google/googlejavaformat/java/ImportOrderer.java b/core/src/main/java/com/google/googlejavaformat/java/ImportOrderer.java
@@ -14,30 +14,94 @@
 package com.google.googlejavaformat.java;
 
 import static com.google.common.collect.Iterables.getLast;
+import static com.google.common.primitives.Booleans.trueFirst;
 
 import com.google.common.base.CharMatcher;
 import com.google.common.base.Preconditions;
+import com.google.common.base.Splitter;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSortedSet;
 import com.google.googlejavaformat.Newlines;
+import com.google.googlejavaformat.java.JavaFormatterOptions.Style;
 import com.google.googlejavaformat.java.JavaInput.Tok;
 import java.util.ArrayList;
+import java.util.Comparator;
 import java.util.List;
 import java.util.Optional;
+import java.util.function.BiFunction;
+import java.util.stream.Stream;
 import org.openjdk.tools.javac.parser.Tokens.TokenKind;
 
 /** Orders imports in Java source code. */
 public class ImportOrderer {
+
+  private static final Splitter DOT_SPLITTER = Splitter.on('.');
+
   /**
    * Reorder the inputs in {@code text}, a complete Java program. On success, another complete Java
    * program is returned, which is the same as the original except the imports are in order.
    *
    * @throws FormatterException if the input could not be parsed.
    */
-  public static String reorderImports(String text) throws FormatterException {
+  public static String reorderImports(String text, Style style) throws FormatterException {
     ImmutableList<Tok> toks = JavaInput.buildToks(text, CLASS_START);
-    return new ImportOrderer(text, toks).reorderImports();
+    return new ImportOrderer(text, toks, style).reorderImports();
+  }
+
+  /**
+   * Reorder the inputs in {@code text}, a complete Java program, in Google style. On success,
+   * another complete Java program is returned, which is the same as the original except the imports
+   * are in order.
+   *
+   * @deprecated Use {@link #reorderImports(String, Style)} instead
+   * @throws FormatterException if the input could not be parsed.
+   */
+  @Deprecated
+  public static String reorderImports(String text) throws FormatterException {
+    return reorderImports(text, Style.GOOGLE);
+  }
+
+  private String reorderImports() throws FormatterException {
+    int firstImportStart;
+    Optional<Integer> maybeFirstImport = findIdentifier(0, IMPORT_OR_CLASS_START);
+    if (!maybeFirstImport.isPresent() || !tokenAt(maybeFirstImport.get()).equals("import")) {
+      // No imports, so nothing to do.
+      return text;
+    }
+    firstImportStart = maybeFirstImport.get();
+    int unindentedFirstImportStart = unindent(firstImportStart);
+
+    ImportsAndIndex imports = scanImports(firstImportStart);
+    int afterLastImport = imports.index;
+
+    // Make sure there are no more imports before the next class (etc) definition.
+    Optional<Integer> maybeLaterImport = findIdentifier(afterLastImport, IMPORT_OR_CLASS_START);
+    if (maybeLaterImport.isPresent() && tokenAt(maybeLaterImport.get()).equals("import")) {
+      throw new FormatterException("Imports not contiguous (perhaps a comment separates them?)");
+    }
+
+    StringBuilder result = new StringBuilder();
+    String prefix = tokString(0, unindentedFirstImportStart);
+    result.append(prefix);
+    if (!prefix.isEmpty() && Newlines.getLineEnding(prefix) == null) {
+      result.append(lineSeparator).append(lineSeparator);
+    }
+    result.append(reorderedImportsString(imports.imports));
+
+    List<String> tail = new ArrayList<>();
+    tail.add(CharMatcher.whitespace().trimLeadingFrom(tokString(afterLastImport, toks.size())));
+    if (!toks.isEmpty()) {
+      Tok lastTok = getLast(toks);
+      int tailStart = lastTok.getPosition() + lastTok.length();
+      tail.add(text.substring(tailStart));
+    }
+    if (tail.stream().anyMatch(s -> !s.isEmpty())) {
+      result.append(lineSeparator);
+      tail.forEach(result::append);
+    }
+
+    return result.toString();
   }
 
   /**
@@ -55,45 +119,127 @@ public static String reorderImports(String text) throws FormatterException {
   private static final ImmutableSet<String> IMPORT_OR_CLASS_START =
       ImmutableSet.of("import", "class", "interface", "enum");
 
+  /**
+   * A {@link Comparator} that orders {@link Import}s by Google Style, defined at
+   * https://google.github.io/styleguide/javaguide.html#s3.3.3-import-ordering-and-spacing.
+   */
+  private static final Comparator<Import> GOOGLE_IMPORT_COMPARATOR =
+      Comparator.comparing(Import::isStatic, trueFirst()).thenComparing(Import::imported);
+
+  /**
+   * A {@link Comparator} that orders {@link Import}s by AOSP Style, defined at
+   * https://source.android.com/setup/contribute/code-style#order-import-statements and implemented
+   * in IntelliJ at
+   * https://android.googlesource.com/platform/development/+/master/ide/intellij/codestyles/AndroidStyle.xml.
+   */
+  private static final Comparator<Import> AOSP_IMPORT_COMPARATOR =
+      Comparator.comparing(Import::isStatic, trueFirst())
+          .thenComparing(Import::isAndroid, trueFirst())
+          .thenComparing(Import::isThirdParty, trueFirst())
+          .thenComparing(Import::isJava, trueFirst())
+          .thenComparing(Import::imported);
+
+  /**
+   * Determines whether to insert a blank line between the {@code prev} and {@code curr} {@link
+   * Import}s based on Google style.
+   */
+  private static boolean shouldInsertBlankLineGoogle(Import prev, Import curr) {
+    return prev.isStatic() && !curr.isStatic();
+  }
+
+  /**
+   * Determines whether to insert a blank line between the {@code prev} and {@code curr} {@link
+   * Import}s based on AOSP style.
+   */
+  private static boolean shouldInsertBlankLineAosp(Import prev, Import curr) {
+    if (prev.isStatic() && !curr.isStatic()) {
+      return true;
+    }
+    // insert blank line between "com.android" from "com.anythingelse"
+    if (prev.isAndroid() && !curr.isAndroid()) {
+      return true;
+    }
+    return !prev.topLevel().equals(curr.topLevel());
+  }
+
   private final String text;
   private final ImmutableList<Tok> toks;
   private final String lineSeparator;
+  private final Comparator<Import> importComparator;
+  private final BiFunction<Import, Import, Boolean> shouldInsertBlankLineFn;
 
-  private ImportOrderer(String text, ImmutableList<Tok> toks) throws FormatterException {
+  private ImportOrderer(String text, ImmutableList<Tok> toks, Style style) {
     this.text = text;
     this.toks = toks;
     this.lineSeparator = Newlines.guessLineSeparator(text);
+    if (style.equals(Style.GOOGLE)) {
+      this.importComparator = GOOGLE_IMPORT_COMPARATOR;
+      this.shouldInsertBlankLineFn = ImportOrderer::shouldInsertBlankLineGoogle;
+    } else if (style.equals(Style.AOSP)) {
+      this.importComparator = AOSP_IMPORT_COMPARATOR;
+      this.shouldInsertBlankLineFn = ImportOrderer::shouldInsertBlankLineAosp;
+    } else {
+      throw new IllegalArgumentException("Unsupported code style: " + style);
+    }
   }
 
   /** An import statement. */
-  private class Import implements Comparable<Import> {
+  class Import {
+    private final String imported;
+    private final boolean isStatic;
+    private final String trailing;
+
+    Import(String imported, String trailing, boolean isStatic) {
+      this.imported = imported;
+      this.trailing = trailing;
+      this.isStatic = isStatic;
+    }
+
     /** The name being imported, for example {@code java.util.List}. */
-    final String imported;
+    String imported() {
+      return imported;
+    }
+
+    /** True if this is {@code import static}. */
+    boolean isStatic() {
+      return isStatic;
+    }
+
+    /** The top-level package of the import. */
+    String topLevel() {
+      return DOT_SPLITTER.split(imported()).iterator().next();
+    }
+
+    /** True if this is an Android import per AOSP style. */
+    boolean isAndroid() {
+      return Stream.of("android.", "androidx.", "dalvik.", "libcore.", "com.android.")
+          .anyMatch(imported::startsWith);
+    }
+
+    /** True if this is a Java import per AOSP style. */
+    boolean isJava() {
+      switch (topLevel()) {
+        case "java":
+        case "javax":
+          return true;
+        default:
+          return false;
+      }
+    }
 
     /**
      * The {@code //} comment lines after the final {@code ;}, up to and including the line
      * terminator of the last one. Note: In case two imports were separated by a space (which is
      * disallowed by the style guide), the trailing whitespace of the first import does not include
      * a line terminator.
      */
-    final String trailing;
-
-    /** True if this is {@code import static}. */
-    final boolean isStatic;
-
-    Import(String imported, String trailing, boolean isStatic) {
-      this.imported = imported;
-      this.trailing = trailing;
-      this.isStatic = isStatic;
+    String trailing() {
+      return trailing;
     }
 
-    // This is how the sorting happens, including sorting static imports before non-static ones.
-    @Override
-    public int compareTo(Import that) {
-      if (this.isStatic != that.isStatic) {
-        return this.isStatic ? -1 : +1;
-      }
-      return this.imported.compareTo(that.imported);
+    /** True if this is a third-party import per AOSP style. */
+    public boolean isThirdParty() {
+      return !(isAndroid() || isJava());
     }
 
     // One or multiple lines, the import itself and following comments, including the line
@@ -102,61 +248,19 @@ public int compareTo(Import that) {
     public String toString() {
       StringBuilder sb = new StringBuilder();
       sb.append("import ");
-      if (isStatic) {
+      if (isStatic()) {
         sb.append("static ");
       }
-      sb.append(imported).append(';');
-      if (trailing.trim().isEmpty()) {
+      sb.append(imported()).append(';');
+      if (trailing().trim().isEmpty()) {
         sb.append(lineSeparator);
       } else {
-        sb.append(trailing);
+        sb.append(trailing());
       }
       return sb.toString();
     }
   }
 
-  private String reorderImports() throws FormatterException {
-    int firstImportStart;
-    Optional<Integer> maybeFirstImport = findIdentifier(0, IMPORT_OR_CLASS_START);
-    if (!maybeFirstImport.isPresent() || !tokenAt(maybeFirstImport.get()).equals("import")) {
-      // No imports, so nothing to do.
-      return text;
-    }
-    firstImportStart = maybeFirstImport.get();
-    int unindentedFirstImportStart = unindent(firstImportStart);
-
-    ImportsAndIndex imports = scanImports(firstImportStart);
-    int afterLastImport = imports.index;
-
-    // Make sure there are no more imports before the next class (etc) definition.
-    Optional<Integer> maybeLaterImport = findIdentifier(afterLastImport, IMPORT_OR_CLASS_START);
-    if (maybeLaterImport.isPresent() && tokenAt(maybeLaterImport.get()).equals("import")) {
-      throw new FormatterException("Imports not contiguous (perhaps a comment separates them?)");
-    }
-
-    StringBuilder result = new StringBuilder();
-    String prefix = tokString(0, unindentedFirstImportStart);
-    result.append(prefix);
-    if (!prefix.isEmpty() && Newlines.getLineEnding(prefix) == null) {
-      result.append(lineSeparator).append(lineSeparator);
-    }
-    result.append(reorderedImportsString(imports.imports));
-
-    List<String> tail = new ArrayList<>();
-    tail.add(CharMatcher.whitespace().trimLeadingFrom(tokString(afterLastImport, toks.size())));
-    if (!toks.isEmpty()) {
-      Tok lastTok = getLast(toks);
-      int tailStart = lastTok.getPosition() + lastTok.length();
-      tail.add(text.substring(tailStart));
-    }
-    if (tail.stream().anyMatch(s -> !s.isEmpty())) {
-      result.append(lineSeparator);
-      tail.forEach(result::append);
-    }
-
-    return result.toString();
-  }
-
   private String tokString(int start, int end) {
     StringBuilder sb = new StringBuilder();
     for (int i = start; i < end; i++) {
@@ -191,7 +295,7 @@ private static class ImportsAndIndex {
    */
   private ImportsAndIndex scanImports(int i) throws FormatterException {
     int afterLastImport = i;
-    ImmutableSortedSet.Builder<Import> imports = ImmutableSortedSet.naturalOrder();
+    ImmutableSortedSet.Builder<Import> imports = ImmutableSortedSet.orderedBy(importComparator);
     // JavaInput.buildToks appends a zero-width EOF token after all tokens. It won't match any
     // of our tests here and protects us from running off the end of the toks list. Since it is
     // zero-width it doesn't matter if we include it in our string concatenation at the end.
@@ -258,20 +362,18 @@ private ImportsAndIndex scanImports(int i) throws FormatterException {
   private String reorderedImportsString(ImmutableSortedSet<Import> imports) {
     Preconditions.checkArgument(!imports.isEmpty(), "imports");
 
-    Import firstImport = imports.iterator().next();
-
-    // Pretend that the first import was preceded by another import of the same kind
-    // (static or non-static), so we don't insert a newline there.
-    boolean lastWasStatic = firstImport.isStatic;
+    // Pretend that the first import was preceded by another import of the same kind, so we don't
+    // insert a newline there.
+    Import prevImport = imports.iterator().next();
 
     StringBuilder sb = new StringBuilder();
-    for (Import thisImport : imports) {
-      if (lastWasStatic && !thisImport.isStatic) {
+    for (Import currImport : imports) {
+      if (shouldInsertBlankLineFn.apply(prevImport, currImport)) {
         // Blank line between static and non-static imports.
         sb.append(lineSeparator);
       }
-      lastWasStatic = thisImport.isStatic;
-      sb.append(thisImport);
+      sb.append(currImport);
+      prevImport = currImport;
     }
     return sb.toString();
   }
diff --git a/core/src/main/java/com/google/googlejavaformat/java/JavaFormatterOptions.java b/core/src/main/java/com/google/googlejavaformat/java/JavaFormatterOptions.java
@@ -30,7 +30,6 @@
 public class JavaFormatterOptions {
 
   public enum Style {
-
     /** The default Google Java Style configuration. */
     GOOGLE(1),
 
@@ -54,11 +53,16 @@ private JavaFormatterOptions(Style style) {
     this.style = style;
   }
 
-  /** Returns the multiplier for the unit of indent */
+  /** Returns the multiplier for the unit of indent. */
   public int indentationMultiplier() {
     return style.indentationMultiplier();
   }
 
+  /** Returns the code style. */
+  public Style style() {
+    return style;
+  }
+
   /** Returns the default formatting options. */
   public static JavaFormatterOptions defaultOptions() {
     return builder().build();
diff --git a/core/src/test/java/com/google/googlejavaformat/java/FormatterTest.java b/core/src/test/java/com/google/googlejavaformat/java/FormatterTest.java
diff --git a/core/src/test/java/com/google/googlejavaformat/java/ImportOrdererTest.java b/core/src/test/java/com/google/googlejavaformat/java/ImportOrdererTest.java

Original file line number	Diff line number	Diff line change
`@@ -55,7 +55,7 @@ private String fixImports(String input) throws FormatterException {`
`55`	`55`	`input = RemoveUnusedImports.removeUnusedImports(input);`
`56`	`56`	`}`
`57`	`57`	`if (parameters.sortImports()) {`
`58`		`- input = ImportOrderer.reorderImports(input);`
	`58`	`+ input = ImportOrderer.reorderImports(input, options.style());`
`59`	`59`	`}`
`60`	`60`	`return input;`
`61`	`61`	`}`