Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add semantics-aware sorting for Java imports #1709

Merged
merged 5 commits into from
May 22, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
39 changes: 29 additions & 10 deletions lib/src/main/java/com/diffplug/spotless/java/ImportOrderStep.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -39,6 +40,9 @@

public final class ImportOrderStep {
private static final boolean WILDCARDS_LAST_DEFAULT = false;
private static final boolean SEMANTIC_SORT_DEFAULT = true;
nedtwigg marked this conversation as resolved.
Show resolved Hide resolved
private static final Set<String> TREAT_AS_PACKAGE_DEFAULT = null;
private static final Set<String> TREAT_AS_CLASS_DEFAULT = null;

private final String lineFormat;

Expand All @@ -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<String> treatAsPackage,
Set<String> treatAsClass, String... importOrder) {
// defensive copying and null checking
List<String> 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<String> treatAsPackage,
Set<String> 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<List<String>> importOrder) {
private FormatterStep createFrom(boolean wildcardsLast, boolean semanticSort, Set<String> treatAsPackage,
Set<String> treatAsClass, Supplier<List<String>> 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);
}

Expand Down Expand Up @@ -106,15 +117,23 @@ private static final class State implements Serializable {
private final List<String> importOrder;
private final String lineFormat;
private final boolean wildcardsLast;
private final boolean semanticSort;
private final Set<String> treatAsPackage;
private final Set<String> treatAsClass;
nedtwigg marked this conversation as resolved.
Show resolved Hide resolved

State(List<String> importOrder, String lineFormat, boolean wildcardsLast) {
State(List<String> importOrder, String lineFormat, boolean wildcardsLast, boolean semanticSort,
Set<String> treatAsPackage, Set<String> 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);
}
}
}
13 changes: 11 additions & 2 deletions lib/src/main/java/com/diffplug/spotless/java/ImportSorter.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Scanner;
import java.util.Set;

/**
* @author Vojtech Krasa
Expand All @@ -30,10 +31,17 @@ final class ImportSorter {

private final List<String> importsOrder;
private final boolean wildcardsLast;
private final boolean semanticSort;
private final Set<String> treatAsPackage;
private final Set<String> treatAsClass;

ImportSorter(List<String> importsOrder, boolean wildcardsLast) {
ImportSorter(List<String> importsOrder, boolean wildcardsLast, boolean semanticSort, Set<String> treatAsPackage,
Set<String> treatAsClass) {
this.importsOrder = new ArrayList<>(importsOrder);
this.wildcardsLast = wildcardsLast;
this.semanticSort = semanticSort;
this.treatAsPackage = treatAsPackage;
this.treatAsClass = treatAsClass;
}

String format(String raw, String lineFormat) {
Expand Down Expand Up @@ -81,7 +89,8 @@ String format(String raw, String lineFormat) {
}
scanner.close();

List<String> sortedImports = ImportSorterImpl.sort(imports, importsOrder, wildcardsLast, lineFormat);
List<String> sortedImports = ImportSorterImpl.sort(imports, importsOrder, wildcardsLast, semanticSort,
treatAsPackage, treatAsClass, lineFormat);
return applyImportsToDocument(raw, firstImportLine, lastImportLine, sortedImports);
}

Expand Down
203 changes: 186 additions & 17 deletions lib/src/main/java/com/diffplug/spotless/java/ImportSorterImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,10 @@ public List<String> getSubGroups() {
}
}

static List<String> sort(List<String> imports, List<String> importsOrder, boolean wildcardsLast, String lineFormat) {
ImportSorterImpl importsSorter = new ImportSorterImpl(importsOrder, wildcardsLast);
static List<String> sort(List<String> imports, List<String> importsOrder, boolean wildcardsLast,
boolean semanticSort, Set<String> treatAsPackage, Set<String> treatAsClass, String lineFormat) {
ImportSorterImpl importsSorter = new ImportSorterImpl(importsOrder, wildcardsLast, semanticSort, treatAsPackage,
treatAsClass);
return importsSorter.sort(imports, lineFormat);
}

Expand All @@ -76,12 +78,17 @@ private List<String> sort(List<String> imports, String lineFormat) {
return getResult(sortedImported, lineFormat);
}

private ImportSorterImpl(List<String> importOrder, boolean wildcardsLast) {
private ImportSorterImpl(List<String> importOrder, boolean wildcardsLast, boolean semanticSort,
Set<String> treatAsPackage, Set<String> 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<String> subgroups = importsGroups.stream().map(ImportsGroup::getSubGroups).flatMap(Collection::stream).collect(Collectors.toList());
this.allImportOrderItems.addAll(subgroups);
Expand Down Expand Up @@ -233,30 +240,192 @@ private List<String> getResult(List<String> sortedImported, String lineFormat) {
return null;
}

private static class OrderingComparator implements Comparator<String>, 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<String>, 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<String>, Serializable {
private static final long serialVersionUID = 1;

private final boolean wildcardsLast;
private final Set<String> treatAsPackage;
private final Set<String> treatAsClass;

private SemanticOrderingComparator(boolean wildcardsLast, Set<String> treatAsPackage,
Set<String> 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);
}

}
}
1 change: 1 addition & 0 deletions plugin-gradle/CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
Loading