From e1c06a8ef55ef1ceb98b566459c298c0521cecf9 Mon Sep 17 00:00:00 2001 From: Patrick Schmidt Date: Mon, 22 May 2023 19:44:09 +0200 Subject: [PATCH 1/5] Introduce semantics-aware Java import ordering. (fixes #522) --- .../spotless/java/ImportOrderStep.java | 39 +++- .../diffplug/spotless/java/ImportSorter.java | 13 +- .../spotless/java/ImportSorterImpl.java | 203 ++++++++++++++++-- .../gradle/spotless/JavaExtension.java | 40 +++- .../spotless/maven/java/ImportOrder.java | 34 ++- .../JavaCodeSortedLexicographic.test | 13 ++ .../JavaCodeSortedSemanticSort.test | 13 ++ .../JavaCodeUnsortedSemanticSort.test | 15 ++ .../spotless/java/ImportOrderStepTest.java | 16 +- 9 files changed, 351 insertions(+), 35 deletions(-) create mode 100644 testlib/src/main/resources/java/importsorter/JavaCodeSortedLexicographic.test create mode 100644 testlib/src/main/resources/java/importsorter/JavaCodeSortedSemanticSort.test create mode 100644 testlib/src/main/resources/java/importsorter/JavaCodeUnsortedSemanticSort.test diff --git a/lib/src/main/java/com/diffplug/spotless/java/ImportOrderStep.java b/lib/src/main/java/com/diffplug/spotless/java/ImportOrderStep.java index 686c9cfcaf..7f100e0adc 100644 --- a/lib/src/main/java/com/diffplug/spotless/java/ImportOrderStep.java +++ b/lib/src/main/java/com/diffplug/spotless/java/ImportOrderStep.java @@ -28,6 +28,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Set; import java.util.function.Supplier; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -39,6 +40,9 @@ public final class ImportOrderStep { private static final boolean WILDCARDS_LAST_DEFAULT = false; + private static final boolean SEMANTIC_SORT_DEFAULT = true; + private static final Set TREAT_AS_PACKAGE_DEFAULT = null; + private static final Set TREAT_AS_CLASS_DEFAULT = null; private final String lineFormat; @@ -55,27 +59,34 @@ private ImportOrderStep(String lineFormat) { } public FormatterStep createFrom(String... importOrder) { - return createFrom(WILDCARDS_LAST_DEFAULT, importOrder); + return createFrom(WILDCARDS_LAST_DEFAULT, SEMANTIC_SORT_DEFAULT, TREAT_AS_PACKAGE_DEFAULT, + TREAT_AS_CLASS_DEFAULT, importOrder); } - public FormatterStep createFrom(boolean wildcardsLast, String... importOrder) { + public FormatterStep createFrom(boolean wildcardsLast, boolean semanticSort, Set treatAsPackage, + Set treatAsClass, String... importOrder) { // defensive copying and null checking List importOrderList = requireElementsNonNull(Arrays.asList(importOrder)); - return createFrom(wildcardsLast, () -> importOrderList); + return createFrom(wildcardsLast, semanticSort, treatAsPackage, treatAsClass, () -> importOrderList); } public FormatterStep createFrom(File importsFile) { - return createFrom(WILDCARDS_LAST_DEFAULT, importsFile); + return createFrom(WILDCARDS_LAST_DEFAULT, SEMANTIC_SORT_DEFAULT, TREAT_AS_PACKAGE_DEFAULT, + TREAT_AS_CLASS_DEFAULT, importsFile); } - public FormatterStep createFrom(boolean wildcardsLast, File importsFile) { + public FormatterStep createFrom(boolean wildcardsLast, boolean semanticSort, Set treatAsPackage, + Set treatAsClass, File importsFile) { Objects.requireNonNull(importsFile); - return createFrom(wildcardsLast, () -> getImportOrder(importsFile)); + return createFrom(wildcardsLast, semanticSort, treatAsPackage, treatAsClass, () -> getImportOrder(importsFile)); } - private FormatterStep createFrom(boolean wildcardsLast, Supplier> importOrder) { + private FormatterStep createFrom(boolean wildcardsLast, boolean semanticSort, Set treatAsPackage, + Set treatAsClass, Supplier> importOrder) { return FormatterStep.createLazy("importOrder", - () -> new State(importOrder.get(), lineFormat, wildcardsLast), + () -> new State(importOrder.get(), lineFormat, wildcardsLast, semanticSort, + treatAsPackage == null ? Set.of() : treatAsPackage, + treatAsClass == null ? Set.of() : treatAsClass), State::toFormatter); } @@ -106,15 +117,23 @@ private static final class State implements Serializable { private final List importOrder; private final String lineFormat; private final boolean wildcardsLast; + private final boolean semanticSort; + private final Set treatAsPackage; + private final Set treatAsClass; - State(List importOrder, String lineFormat, boolean wildcardsLast) { + State(List importOrder, String lineFormat, boolean wildcardsLast, boolean semanticSort, + Set treatAsPackage, Set treatAsClass) { this.importOrder = importOrder; this.lineFormat = lineFormat; this.wildcardsLast = wildcardsLast; + this.semanticSort = semanticSort; + this.treatAsPackage = treatAsPackage; + this.treatAsClass = treatAsClass; } FormatterFunc toFormatter() { - return raw -> new ImportSorter(importOrder, wildcardsLast).format(raw, lineFormat); + return raw -> new ImportSorter(importOrder, wildcardsLast, semanticSort, treatAsPackage, treatAsClass) + .format(raw, lineFormat); } } } diff --git a/lib/src/main/java/com/diffplug/spotless/java/ImportSorter.java b/lib/src/main/java/com/diffplug/spotless/java/ImportSorter.java index edfc948487..d6da23f254 100644 --- a/lib/src/main/java/com/diffplug/spotless/java/ImportSorter.java +++ b/lib/src/main/java/com/diffplug/spotless/java/ImportSorter.java @@ -18,6 +18,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Scanner; +import java.util.Set; /** * @author Vojtech Krasa @@ -30,10 +31,17 @@ final class ImportSorter { private final List importsOrder; private final boolean wildcardsLast; + private final boolean semanticSort; + private final Set treatAsPackage; + private final Set treatAsClass; - ImportSorter(List importsOrder, boolean wildcardsLast) { + ImportSorter(List importsOrder, boolean wildcardsLast, boolean semanticSort, Set treatAsPackage, + Set treatAsClass) { this.importsOrder = new ArrayList<>(importsOrder); this.wildcardsLast = wildcardsLast; + this.semanticSort = semanticSort; + this.treatAsPackage = treatAsPackage; + this.treatAsClass = treatAsClass; } String format(String raw, String lineFormat) { @@ -81,7 +89,8 @@ String format(String raw, String lineFormat) { } scanner.close(); - List sortedImports = ImportSorterImpl.sort(imports, importsOrder, wildcardsLast, lineFormat); + List sortedImports = ImportSorterImpl.sort(imports, importsOrder, wildcardsLast, semanticSort, + treatAsPackage, treatAsClass, lineFormat); return applyImportsToDocument(raw, firstImportLine, lastImportLine, sortedImports); } diff --git a/lib/src/main/java/com/diffplug/spotless/java/ImportSorterImpl.java b/lib/src/main/java/com/diffplug/spotless/java/ImportSorterImpl.java index 1f014ba95e..828b5ef8f0 100644 --- a/lib/src/main/java/com/diffplug/spotless/java/ImportSorterImpl.java +++ b/lib/src/main/java/com/diffplug/spotless/java/ImportSorterImpl.java @@ -62,8 +62,10 @@ public List getSubGroups() { } } - static List sort(List imports, List importsOrder, boolean wildcardsLast, String lineFormat) { - ImportSorterImpl importsSorter = new ImportSorterImpl(importsOrder, wildcardsLast); + static List sort(List imports, List importsOrder, boolean wildcardsLast, + boolean semanticSort, Set treatAsPackage, Set treatAsClass, String lineFormat) { + ImportSorterImpl importsSorter = new ImportSorterImpl(importsOrder, wildcardsLast, semanticSort, treatAsPackage, + treatAsClass); return importsSorter.sort(imports, lineFormat); } @@ -76,12 +78,17 @@ private List sort(List imports, String lineFormat) { return getResult(sortedImported, lineFormat); } - private ImportSorterImpl(List importOrder, boolean wildcardsLast) { + private ImportSorterImpl(List importOrder, boolean wildcardsLast, boolean semanticSort, + Set treatAsPackage, Set treatAsClass) { importsGroups = importOrder.stream().filter(Objects::nonNull).map(ImportsGroup::new).collect(Collectors.toList()); putStaticItemIfNotExists(importsGroups); putCatchAllGroupIfNotExists(importsGroups); - ordering = new OrderingComparator(wildcardsLast); + if (semanticSort) { + ordering = new SemanticOrderingComparator(wildcardsLast, treatAsPackage, treatAsClass); + } else { + ordering = new LexicographicalOrderingComparator(wildcardsLast); + } List subgroups = importsGroups.stream().map(ImportsGroup::getSubGroups).flatMap(Collection::stream).collect(Collectors.toList()); this.allImportOrderItems.addAll(subgroups); @@ -233,30 +240,192 @@ private List getResult(List sortedImported, String lineFormat) { return null; } - private static class OrderingComparator implements Comparator, Serializable { + private static int compareWithWildcare(String string1, String string2, boolean wildcardsLast) { + int string1WildcardIndex = string1.indexOf('*'); + int string2WildcardIndex = string2.indexOf('*'); + boolean string1IsWildcard = string1WildcardIndex >= 0; + boolean string2IsWildcard = string2WildcardIndex >= 0; + if (string1IsWildcard == string2IsWildcard) { + return string1.compareTo(string2); + } + int prefixLength = string1IsWildcard ? string1WildcardIndex : string2WildcardIndex; + boolean samePrefix = string1.regionMatches(0, string2, 0, prefixLength); + if (!samePrefix) { + return string1.compareTo(string2); + } + return (string1IsWildcard == wildcardsLast) ? 1 : -1; + } + + private static class LexicographicalOrderingComparator implements Comparator, Serializable { private static final long serialVersionUID = 1; private final boolean wildcardsLast; - private OrderingComparator(boolean wildcardsLast) { + private LexicographicalOrderingComparator(boolean wildcardsLast) { this.wildcardsLast = wildcardsLast; } @Override public int compare(String string1, String string2) { - int string1WildcardIndex = string1.indexOf('*'); - int string2WildcardIndex = string2.indexOf('*'); - boolean string1IsWildcard = string1WildcardIndex >= 0; - boolean string2IsWildcard = string2WildcardIndex >= 0; - if (string1IsWildcard == string2IsWildcard) { - return string1.compareTo(string2); + return compareWithWildcare(string1, string2, wildcardsLast); + } + } + + private static class SemanticOrderingComparator implements Comparator, Serializable { + private static final long serialVersionUID = 1; + + private final boolean wildcardsLast; + private final Set treatAsPackage; + private final Set treatAsClass; + + private SemanticOrderingComparator(boolean wildcardsLast, Set treatAsPackage, + Set treatAsClass) { + this.wildcardsLast = wildcardsLast; + this.treatAsPackage = treatAsPackage; + this.treatAsClass = treatAsClass; + } + + @Override + public int compare(String string1, String string2) { + /* + * Ordering uses semantics of the import string by splitting it into package, + * class name(s) and static member (for static imports) and then comparing by + * each of those three substrings in sequence. + * + * When comparing static imports, the last segment in the dot-separated string + * is considered to be the member (field, method, type) name. + * + * The first segment starting with an upper case letter is considered to be the + * (first) class name. Since this comparator has no actual type information, + * this auto-detection will fail for upper case package names and lower case + * class names. treatAsPackage and treatAsClass can be used respectively to + * provide hints to the auto-detection. + */ + if (string1.startsWith(STATIC_KEYWORD)) { + String[] split = splitFqcnAndMember(string1); + String fqcn1 = split[0]; + String member1 = split[1]; + + split = splitFqcnAndMember(string2); + String fqcn2 = split[0]; + String member2 = split[1]; + + int result = compareFullyQualifiedClassName(fqcn1, fqcn2); + if (result != 0) + return result; + + return compareWithWildcare(member1, member2, wildcardsLast); + } else { + return compareFullyQualifiedClassName(string1, string2); + } + } + + /** + * Compares two fully qualified class names by splitting them into package and + * (nested) class names. + */ + private int compareFullyQualifiedClassName(String fqcn1, String fqcn2) { + String[] split = splitPackageAndClasses(fqcn1); + String p1 = split[0]; + String c1 = split[1]; + + split = splitPackageAndClasses(fqcn2); + String p2 = split[0]; + String c2 = split[1]; + + int result = p1.compareTo(p2); + if (result != 0) + return result; + + return compareWithWildcare(c1, c2, wildcardsLast); + } + + /** + * Splits the provided static import string into fully qualified class name and + * the imported static member (field, method or type). + */ + private String[] splitFqcnAndMember(String importString) { + String s = importString.substring(STATIC_KEYWORD.length()).trim(); + + String fqcn; + String member; + + int dot = s.lastIndexOf("."); + if (!Character.isUpperCase(s.charAt(dot + 1))) { + fqcn = s.substring(0, dot); + member = s.substring(dot + 1); + } else { + fqcn = s; + member = null; } - int prefixLength = string1IsWildcard ? string1WildcardIndex : string2WildcardIndex; - boolean samePrefix = string1.regionMatches(0, string2, 0, prefixLength); - if (!samePrefix) { - return string1.compareTo(string2); + + return new String[] { fqcn, member }; + } + + /** + * Splits the fully qualified class name into package and class name(s). + */ + private String[] splitPackageAndClasses(String fqcn) { + String packageNames = null; + String classNames = null; + + /* + * The first segment that starts with an upper case letter starts the class + * name(s), unless it matches treatAsPackage (then it's explicitly declared as + * package via configuration). If no segment starts with an upper case letter + * then the last segment must be a class name (unless the method input is + * garbage). + */ + int dot = fqcn.indexOf('.'); + while (dot > -1) { + int nextDot = fqcn.indexOf('.', dot + 1); + if (nextDot > -1) { + if (Character.isUpperCase(fqcn.charAt(dot + 1))) { + // if upper case, check if should be treated as package nonetheless + if (!treatAsPackage(fqcn.substring(0, nextDot))) { + packageNames = fqcn.substring(0, dot); + classNames = fqcn.substring(dot + 1); + break; + } + } else { + // if lower case, check if should be treated as class nonetheless + if (treatAsClass(fqcn.substring(0, nextDot))) { + packageNames = fqcn.substring(0, dot); + classNames = fqcn.substring(dot + 1); + break; + } + } + } + + dot = nextDot; } - return (string1IsWildcard == wildcardsLast) ? 1 : -1; + + if (packageNames == null) { + int i = fqcn.lastIndexOf("."); + packageNames = fqcn.substring(0, i); + classNames = fqcn.substring(i + 1); + } + + return new String[] { packageNames, classNames }; } + + /** + * Returns whether the provided prefix matches any entry of + * {@code treatAsPackage}. + */ + private boolean treatAsPackage(String prefix) { + // This would be the place to introduce wild cards or even regex matching. + return treatAsPackage.contains(prefix); + } + + /** + * Returns whether the provided prefix name matches any entry of + * {@code treatAsClass}. + */ + private boolean treatAsClass(String prefix) { + // This would be the place to introduce wild cards or even regex matching. + return treatAsClass.contains(prefix); + } + } } diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JavaExtension.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JavaExtension.java index d8dd77a09c..3016a94413 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JavaExtension.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JavaExtension.java @@ -19,10 +19,13 @@ import java.io.File; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Set; import javax.inject.Inject; @@ -74,6 +77,9 @@ public class ImportOrderConfig { final File importOrderFile; boolean wildcardsLast = false; + boolean semanticSort = true; + Set treatAsPackage = Set.of(); + Set treatAsClass = Set.of(); ImportOrderConfig(String[] importOrder) { this.importOrder = importOrder; @@ -98,12 +104,42 @@ public ImportOrderConfig wildcardsLast(boolean wildcardsLast) { return this; } + public ImportOrderConfig semanticSort() { + return semanticSort(true); + } + + public ImportOrderConfig semanticSort(boolean semanticSort) { + this.semanticSort = semanticSort; + replaceStep(createStep()); + return this; + } + + public ImportOrderConfig treatAsPackage(String... treatAsPackage) { + return treatAsPackage(Arrays.asList(treatAsPackage)); + } + + public ImportOrderConfig treatAsPackage(Collection treatAsPackage) { + this.treatAsPackage = new HashSet<>(treatAsPackage); + replaceStep(createStep()); + return this; + } + + public ImportOrderConfig treatAsClass(String... treatAsClass) { + return treatAsClass(Arrays.asList(treatAsClass)); + } + + public ImportOrderConfig treatAsClass(Collection treatAsClass) { + this.treatAsClass = new HashSet<>(treatAsClass); + replaceStep(createStep()); + return this; + } + private FormatterStep createStep() { ImportOrderStep importOrderStep = ImportOrderStep.forJava(); return importOrderFile != null - ? importOrderStep.createFrom(wildcardsLast, getProject().file(importOrderFile)) - : importOrderStep.createFrom(wildcardsLast, importOrder); + ? importOrderStep.createFrom(wildcardsLast, semanticSort, treatAsPackage, treatAsClass, getProject().file(importOrderFile)) + : importOrderStep.createFrom(wildcardsLast, semanticSort, treatAsPackage, treatAsClass, importOrder); } } diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/java/ImportOrder.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/java/ImportOrder.java index 8476b83733..94a89478bb 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/java/ImportOrder.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/java/ImportOrder.java @@ -16,6 +16,8 @@ package com.diffplug.spotless.maven.java; import java.io.File; +import java.util.HashSet; +import java.util.Set; import org.apache.maven.plugins.annotations.Parameter; @@ -34,17 +36,43 @@ public class ImportOrder implements FormatterStepFactory { @Parameter private boolean wildcardsLast = false; + /** + * Whether imports should be sorted based on semantics (i.e. sorted by package, + * class and then static member). This considers treatAsPackage and + * treatAsClass, and assumes default upper and lower case + * conventions otherwise. When turned off, imports are sorted purely + * lexicographically. + */ + @Parameter + private boolean semanticSort = true; + + /** + * The prefixes that should be treated as packages for + * semanticSort. Useful for upper case package names. + */ + @Parameter + private Set treatAsPackage = new HashSet<>(); + + /** + * The prefixes that should be treated as classes for + * semanticSort. Useful for lower case class names. + */ + @Parameter + private Set treatAsClass = new HashSet<>(); + @Override public FormatterStep newFormatterStep(FormatterStepConfig config) { if (file != null ^ order != null) { if (file != null) { File importsFile = config.getFileLocator().locateFile(file); - return ImportOrderStep.forJava().createFrom(wildcardsLast, importsFile); + return ImportOrderStep.forJava().createFrom(wildcardsLast, semanticSort, treatAsPackage, treatAsClass, + importsFile); } else { - return ImportOrderStep.forJava().createFrom(wildcardsLast, order.split(",", -1)); + return ImportOrderStep.forJava().createFrom(wildcardsLast, semanticSort, treatAsPackage, treatAsClass, + order.split(",", -1)); } } else if (file == null && order == null) { - return ImportOrderStep.forJava().createFrom(wildcardsLast); + return ImportOrderStep.forJava().createFrom(wildcardsLast, semanticSort, treatAsPackage, treatAsClass); } else { throw new IllegalArgumentException("Must specify exactly one of 'file' or 'order'."); } diff --git a/testlib/src/main/resources/java/importsorter/JavaCodeSortedLexicographic.test b/testlib/src/main/resources/java/importsorter/JavaCodeSortedLexicographic.test new file mode 100644 index 0000000000..731cbb5878 --- /dev/null +++ b/testlib/src/main/resources/java/importsorter/JavaCodeSortedLexicographic.test @@ -0,0 +1,13 @@ +import static com.example.Test.*; +import static com.example.Test.A_CONST; +import static com.example.Test.Nested.B_CONST; +import static com.example.Test.Z_CONST; + +import com.example.Test; +import com.example.Test.*; +import com.example.Test.Nested; +import com.example.a.A; +import com.example.b; +import com.example.c.C; +import com.sun.jna.platform.win32.COM.Unknown; +import com.sun.jna.platform.win32.Guid.CLSID; diff --git a/testlib/src/main/resources/java/importsorter/JavaCodeSortedSemanticSort.test b/testlib/src/main/resources/java/importsorter/JavaCodeSortedSemanticSort.test new file mode 100644 index 0000000000..aaab4c1b86 --- /dev/null +++ b/testlib/src/main/resources/java/importsorter/JavaCodeSortedSemanticSort.test @@ -0,0 +1,13 @@ +import static com.example.Test.*; +import static com.example.Test.A_CONST; +import static com.example.Test.Nested.B_CONST; +import static com.example.Test.Z_CONST; + +import com.example.Test; +import com.example.Test.*; +import com.example.Test.Nested; +import com.example.b; +import com.example.a.A; +import com.example.c.C; +import com.sun.jna.platform.win32.Guid.CLSID; +import com.sun.jna.platform.win32.COM.Unknown; diff --git a/testlib/src/main/resources/java/importsorter/JavaCodeUnsortedSemanticSort.test b/testlib/src/main/resources/java/importsorter/JavaCodeUnsortedSemanticSort.test new file mode 100644 index 0000000000..766322936d --- /dev/null +++ b/testlib/src/main/resources/java/importsorter/JavaCodeUnsortedSemanticSort.test @@ -0,0 +1,15 @@ +import static com.example.Test.*; +import static com.example.Test.A_CONST; +import static com.example.Test.Z_CONST; +import static com.example.Test.Nested.B_CONST; + +import com.example.Test.Nested; +import com.example.Test; +import com.example.Test.*; + +import com.sun.jna.platform.win32.COM.Unknown; +import com.sun.jna.platform.win32.Guid.CLSID; + +import com.example.a.A; +import com.example.b; +import com.example.c.C; diff --git a/testlib/src/test/java/com/diffplug/spotless/java/ImportOrderStepTest.java b/testlib/src/test/java/com/diffplug/spotless/java/ImportOrderStepTest.java index ca6bd39825..57b1401554 100644 --- a/testlib/src/test/java/com/diffplug/spotless/java/ImportOrderStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/java/ImportOrderStepTest.java @@ -21,6 +21,7 @@ import com.diffplug.spotless.ResourceHarness; import com.diffplug.spotless.SerializableEqualityTester; import com.diffplug.spotless.StepHarness; +import java.util.Set; class ImportOrderStepTest extends ResourceHarness { @Test @@ -61,7 +62,7 @@ void sortImportsUnmatched() { @Test void sortImportsWildcardsLast() { - FormatterStep step = ImportOrderStep.forJava().createFrom(true); + FormatterStep step = ImportOrderStep.forJava().createFrom(true, true, null, null); StepHarness.forStep(step).testResource("java/importsorter/JavaCodeUnsortedImports.test", "java/importsorter/JavaCodeSortedImportsWildcardsLast.test"); } @@ -89,6 +90,19 @@ void empty() { StepHarness.forStep(step).testResource("java/importsorter/JavaCodeEmptyFile.test", "java/importsorter/JavaCodeEmptyFile.test"); } + @Test + void lexicographicSort() { + FormatterStep step = ImportOrderStep.forJava().createFrom(false, false, null, null, createTestFile("java/importsorter/import.properties")); + StepHarness.forStep(step).testResource("java/importsorter/JavaCodeUnsortedSemanticSort.test", "java/importsorter/JavaCodeSortedLexicographic.test"); + } + + @Test + void semanticSort() { + FormatterStep step = ImportOrderStep.forJava().createFrom(false, true, Set.of("com.sun.jna.platform.win32.COM"), + Set.of("com.example.b"), createTestFile("java/importsorter/import.properties")); + StepHarness.forStep(step).testResource("java/importsorter/JavaCodeUnsortedSemanticSort.test", "java/importsorter/JavaCodeSortedSemanticSort.test"); + } + @Test void groovyImports() { FormatterStep step = ImportOrderStep.forGroovy().createFrom(createTestFile("java/importsorter/import.properties")); From 7deb02e52e6a45f888658028f0a1003c3f3eb19e Mon Sep 17 00:00:00 2001 From: Patrick Schmidt Date: Mon, 22 May 2023 19:46:10 +0200 Subject: [PATCH 2/5] Update changelog and readme. --- CHANGES.md | 1 + plugin-gradle/CHANGES.md | 1 + plugin-maven/CHANGES.md | 1 + plugin-maven/README.md | 7 +++++++ 4 files changed, 10 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 9d65dfd9b5..873bbbe84e 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -13,6 +13,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( ### Added * `Jvm.Support` now accepts `-SNAPSHOT` versions, treated as the non`-SNAPSHOT`. ([#1583](https://github.com/diffplug/spotless/issues/1583)) * Support Rome as a formatter for JavaScript and TypeScript code. Adds a new `rome` step to `javascript` and `typescript` formatter configurations. ([#1663](https://github.com/diffplug/spotless/pull/1663)) +* Add semantics-aware Java import ordering (i.e. sort by package, then class, then member). ([#522](https://github.com/diffplug/spotless/issues/522)) ### Fixed * When P2 download fails, indicate the responsible formatter. ([#1698](https://github.com/diffplug/spotless/issues/1698)) * Fixed a regression which changed the import sorting order in `googleJavaFormat` introduced in `2.38.0`. ([#1680](https://github.com/diffplug/spotless/pull/1680)) diff --git a/plugin-gradle/CHANGES.md b/plugin-gradle/CHANGES.md index d064173af6..7b1b401f14 100644 --- a/plugin-gradle/CHANGES.md +++ b/plugin-gradle/CHANGES.md @@ -5,6 +5,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( ## [Unreleased] ### Added * Support Rome as a formatter for JavaScript and TypeScript code. Adds a new `rome` step to `javascript` and `typescript` formatter configurations. ([#1663](https://github.com/diffplug/spotless/pull/1663)) +* Add semantics-aware Java import ordering (i.e. sort by package, then class, then member). ([#522](https://github.com/diffplug/spotless/issues/522)) ### Fixed * Added `@DisableCachingByDefault` to `RegisterDependenciesTask`. ([#1666](https://github.com/diffplug/spotless/pull/1666)) * When P2 download fails, indicate the responsible formatter. ([#1698](https://github.com/diffplug/spotless/issues/1698)) diff --git a/plugin-maven/CHANGES.md b/plugin-maven/CHANGES.md index 5777493c41..139b0fa265 100644 --- a/plugin-maven/CHANGES.md +++ b/plugin-maven/CHANGES.md @@ -5,6 +5,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( ## [Unreleased] ### Added * Support Rome as a formatter for JavaScript and TypeScript code. Adds a new `rome` step to `javascript` and `typescript` formatter configurations. ([#1663](https://github.com/diffplug/spotless/pull/1663)) +* Add semantics-aware Java import ordering (i.e. sort by package, then class, then member). ([#522](https://github.com/diffplug/spotless/issues/522)) ### Fixed * `palantir` step now accepts a `style` parameter, which is documentation had already claimed to do. ([#1694](https://github.com/diffplug/spotless/pull/1694)) * When P2 download fails, indicate the responsible formatter. ([#1698](https://github.com/diffplug/spotless/issues/1698)) diff --git a/plugin-maven/README.md b/plugin-maven/README.md index b11e2c164d..7facd99a0d 100644 --- a/plugin-maven/README.md +++ b/plugin-maven/README.md @@ -196,6 +196,13 @@ any other maven phase (i.e. compile) then it can be configured as below; false java|javax,org,com,com.diffplug,,\#com.diffplug,\# + false + + com.example.MyPackage + + + com.example.myClass + From 10610f8a8d12bfdc8303645daafc4d68c17c9165 Mon Sep 17 00:00:00 2001 From: Patrick Schmidt Date: Mon, 22 May 2023 21:03:16 +0200 Subject: [PATCH 3/5] fixup! Introduce semantics-aware Java import ordering. (fixes #522) --- .../diffplug/spotless/java/ImportOrderStep.java | 16 ++++++++-------- .../diffplug/spotless/java/ImportSorterImpl.java | 4 ++-- .../diffplug/gradle/spotless/JavaExtension.java | 2 +- .../spotless/maven/java/ImportOrder.java | 2 +- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/java/ImportOrderStep.java b/lib/src/main/java/com/diffplug/spotless/java/ImportOrderStep.java index 7f100e0adc..e986875a48 100644 --- a/lib/src/main/java/com/diffplug/spotless/java/ImportOrderStep.java +++ b/lib/src/main/java/com/diffplug/spotless/java/ImportOrderStep.java @@ -29,6 +29,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.TreeSet; import java.util.function.Supplier; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -40,7 +41,7 @@ public final class ImportOrderStep { private static final boolean WILDCARDS_LAST_DEFAULT = false; - private static final boolean SEMANTIC_SORT_DEFAULT = true; + private static final boolean SEMANTIC_SORT_DEFAULT = false; private static final Set TREAT_AS_PACKAGE_DEFAULT = null; private static final Set TREAT_AS_CLASS_DEFAULT = null; @@ -84,9 +85,8 @@ public FormatterStep createFrom(boolean wildcardsLast, boolean semanticSort, Set private FormatterStep createFrom(boolean wildcardsLast, boolean semanticSort, Set treatAsPackage, Set treatAsClass, Supplier> importOrder) { return FormatterStep.createLazy("importOrder", - () -> new State(importOrder.get(), lineFormat, wildcardsLast, semanticSort, - treatAsPackage == null ? Set.of() : treatAsPackage, - treatAsClass == null ? Set.of() : treatAsClass), + () -> new State(importOrder.get(), lineFormat, wildcardsLast, semanticSort, treatAsPackage, + treatAsClass), State::toFormatter); } @@ -118,8 +118,8 @@ private static final class State implements Serializable { private final String lineFormat; private final boolean wildcardsLast; private final boolean semanticSort; - private final Set treatAsPackage; - private final Set treatAsClass; + private final TreeSet treatAsPackage; + private final TreeSet treatAsClass; State(List importOrder, String lineFormat, boolean wildcardsLast, boolean semanticSort, Set treatAsPackage, Set treatAsClass) { @@ -127,8 +127,8 @@ private static final class State implements Serializable { this.lineFormat = lineFormat; this.wildcardsLast = wildcardsLast; this.semanticSort = semanticSort; - this.treatAsPackage = treatAsPackage; - this.treatAsClass = treatAsClass; + this.treatAsPackage = treatAsPackage == null ? null : new TreeSet<>(treatAsPackage); + this.treatAsClass = treatAsClass == null ? null : new TreeSet<>(treatAsClass); } FormatterFunc toFormatter() { diff --git a/lib/src/main/java/com/diffplug/spotless/java/ImportSorterImpl.java b/lib/src/main/java/com/diffplug/spotless/java/ImportSorterImpl.java index 828b5ef8f0..a6d5af675c 100644 --- a/lib/src/main/java/com/diffplug/spotless/java/ImportSorterImpl.java +++ b/lib/src/main/java/com/diffplug/spotless/java/ImportSorterImpl.java @@ -415,7 +415,7 @@ private String[] splitPackageAndClasses(String fqcn) { */ private boolean treatAsPackage(String prefix) { // This would be the place to introduce wild cards or even regex matching. - return treatAsPackage.contains(prefix); + return treatAsPackage != null && treatAsPackage.contains(prefix); } /** @@ -424,7 +424,7 @@ private boolean treatAsPackage(String prefix) { */ private boolean treatAsClass(String prefix) { // This would be the place to introduce wild cards or even regex matching. - return treatAsClass.contains(prefix); + return treatAsClass != null && treatAsClass.contains(prefix); } } diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JavaExtension.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JavaExtension.java index 3016a94413..ecb519bd6e 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JavaExtension.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JavaExtension.java @@ -77,7 +77,7 @@ public class ImportOrderConfig { final File importOrderFile; boolean wildcardsLast = false; - boolean semanticSort = true; + boolean semanticSort = false; Set treatAsPackage = Set.of(); Set treatAsClass = Set.of(); diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/java/ImportOrder.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/java/ImportOrder.java index 94a89478bb..fe6dc10fc8 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/java/ImportOrder.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/java/ImportOrder.java @@ -44,7 +44,7 @@ public class ImportOrder implements FormatterStepFactory { * lexicographically. */ @Parameter - private boolean semanticSort = true; + private boolean semanticSort = false; /** * The prefixes that should be treated as packages for From ff38088bc57fd2a9b15d5b33abfecec6f7de25a0 Mon Sep 17 00:00:00 2001 From: Patrick Schmidt Date: Mon, 22 May 2023 21:29:55 +0200 Subject: [PATCH 4/5] fixup! Introduce semantics-aware Java import ordering. (fixes #522) --- .../java/com/diffplug/spotless/java/ImportOrderStep.java | 2 +- .../java/com/diffplug/spotless/java/ImportSorter.java | 2 +- .../java/com/diffplug/spotless/java/ImportSorterImpl.java | 8 ++++---- .../java/com/diffplug/gradle/spotless/JavaExtension.java | 4 ++-- .../com/diffplug/spotless/maven/java/ImportOrder.java | 2 +- .../com/diffplug/spotless/java/ImportOrderStepTest.java | 3 ++- 6 files changed, 11 insertions(+), 10 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/java/ImportOrderStep.java b/lib/src/main/java/com/diffplug/spotless/java/ImportOrderStep.java index e986875a48..6f0f2a0e8f 100644 --- a/lib/src/main/java/com/diffplug/spotless/java/ImportOrderStep.java +++ b/lib/src/main/java/com/diffplug/spotless/java/ImportOrderStep.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2021 DiffPlug + * Copyright 2016-2023 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/lib/src/main/java/com/diffplug/spotless/java/ImportSorter.java b/lib/src/main/java/com/diffplug/spotless/java/ImportSorter.java index d6da23f254..6cd7dd4978 100644 --- a/lib/src/main/java/com/diffplug/spotless/java/ImportSorter.java +++ b/lib/src/main/java/com/diffplug/spotless/java/ImportSorter.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2021 DiffPlug + * Copyright 2016-2023 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/lib/src/main/java/com/diffplug/spotless/java/ImportSorterImpl.java b/lib/src/main/java/com/diffplug/spotless/java/ImportSorterImpl.java index a6d5af675c..a3fe621e13 100644 --- a/lib/src/main/java/com/diffplug/spotless/java/ImportSorterImpl.java +++ b/lib/src/main/java/com/diffplug/spotless/java/ImportSorterImpl.java @@ -291,10 +291,10 @@ public int compare(String string1, String string2) { * Ordering uses semantics of the import string by splitting it into package, * class name(s) and static member (for static imports) and then comparing by * each of those three substrings in sequence. - * + * * When comparing static imports, the last segment in the dot-separated string * is considered to be the member (field, method, type) name. - * + * * The first segment starting with an upper case letter is considered to be the * (first) class name. Since this comparator has no actual type information, * this auto-detection will fail for upper case package names and lower case @@ -359,7 +359,7 @@ private String[] splitFqcnAndMember(String importString) { member = null; } - return new String[] { fqcn, member }; + return new String[]{fqcn, member}; } /** @@ -406,7 +406,7 @@ private String[] splitPackageAndClasses(String fqcn) { classNames = fqcn.substring(i + 1); } - return new String[] { packageNames, classNames }; + return new String[]{packageNames, classNames}; } /** diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JavaExtension.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JavaExtension.java index ecb519bd6e..13fe674168 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JavaExtension.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JavaExtension.java @@ -127,13 +127,13 @@ public ImportOrderConfig treatAsPackage(Collection treatAsPackage) { public ImportOrderConfig treatAsClass(String... treatAsClass) { return treatAsClass(Arrays.asList(treatAsClass)); } - + public ImportOrderConfig treatAsClass(Collection treatAsClass) { this.treatAsClass = new HashSet<>(treatAsClass); replaceStep(createStep()); return this; } - + private FormatterStep createStep() { ImportOrderStep importOrderStep = ImportOrderStep.forJava(); diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/java/ImportOrder.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/java/ImportOrder.java index fe6dc10fc8..88f47a2a64 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/java/ImportOrder.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/java/ImportOrder.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2021 DiffPlug + * Copyright 2016-2023 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/testlib/src/test/java/com/diffplug/spotless/java/ImportOrderStepTest.java b/testlib/src/test/java/com/diffplug/spotless/java/ImportOrderStepTest.java index 57b1401554..75cd984f08 100644 --- a/testlib/src/test/java/com/diffplug/spotless/java/ImportOrderStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/java/ImportOrderStepTest.java @@ -15,13 +15,14 @@ */ package com.diffplug.spotless.java; +import java.util.Set; + import org.junit.jupiter.api.Test; import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.ResourceHarness; import com.diffplug.spotless.SerializableEqualityTester; import com.diffplug.spotless.StepHarness; -import java.util.Set; class ImportOrderStepTest extends ResourceHarness { @Test From d72a11ce0305b45a8b4a54afdde861280434595c Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 22 May 2023 14:58:11 -0700 Subject: [PATCH 5/5] Fix spotbugs. --- .../main/java/com/diffplug/spotless/java/ImportOrderStep.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/java/ImportOrderStep.java b/lib/src/main/java/com/diffplug/spotless/java/ImportOrderStep.java index 6f0f2a0e8f..e1e63458d9 100644 --- a/lib/src/main/java/com/diffplug/spotless/java/ImportOrderStep.java +++ b/lib/src/main/java/com/diffplug/spotless/java/ImportOrderStep.java @@ -42,8 +42,8 @@ public final class ImportOrderStep { private static final boolean WILDCARDS_LAST_DEFAULT = false; private static final boolean SEMANTIC_SORT_DEFAULT = false; - private static final Set TREAT_AS_PACKAGE_DEFAULT = null; - private static final Set TREAT_AS_CLASS_DEFAULT = null; + private static final Set TREAT_AS_PACKAGE_DEFAULT = Set.of(); + private static final Set TREAT_AS_CLASS_DEFAULT = Set.of(); private final String lineFormat;