Skip to content

Commit

Permalink
Support for group of imports without blank lines between groups
Browse files Browse the repository at this point in the history 
To keep compatibility with existing configuration, a special delimiter `|` is introduced to separate subgroup of imports inside a group.
For example "java|javax" will not generate a blank line between the java group and the javax group.

The notion of ImportsGroup has been introduced for clarification, allowing code has been simplification.

Fixes #1375
  • Loading branch information
@murdos
murdos committed Nov 20, 2022
1 parent db3798b commit 35a56ea
Show file tree
Hide file tree
Showing 7 changed files with 119 additions and 105 deletions.
1 change: 1 addition & 0 deletions CHANGES.md
View file
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 (
### Fixed
* Don't treat `@Value` as a type annotation [#1367](https://github.com/diffplug/spotless/pull/1367)
* Support `ktlint_disabled_rules` in `ktlint` 0.47.x [#1378](https://github.com/diffplug/spotless/pull/1378)
* ImportOrder: support groups of imports without blank lines [#1401](https://github.com/diffplug/spotless/pull/1401)

## [2.30.0] - 2022-09-14
### Added
Expand Down
174 changes: 75 additions & 99 deletions lib/src/main/java/com/diffplug/spotless/java/ImportSorterImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

import java.io.Serializable;
import java.util.*;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import javax.annotation.Nullable;

Expand All @@ -25,12 +27,41 @@
// which itself is licensed under the Apache 2.0 license.
final class ImportSorterImpl {

private final List<String> template = new ArrayList<>();
private static final String CATCH_ALL_SUBGROUP = "";
private static final String STATIC_KEYWORD = "static ";
private static final String STATIC_SYMBOL = "\\#";
private static final String SUBGROUP_SEPARATOR = "|";

private final List<ImportsGroup> importsGroups;
private final Map<String, List<String>> matchingImports = new HashMap<>();
private final List<String> notMatching = new ArrayList<>();
private final Set<String> allImportOrderItems = new HashSet<>();
private final Comparator<String> ordering;

// An ImportsGroup is a group of imports ; each group is separated by blank lines.
// A group is composed of subgroups : imports are sorted by subgroup.
private static class ImportsGroup {

private final List<String> subGroups;

public ImportsGroup(String importOrder) {
this.subGroups = Stream.of(importOrder.split("\\" + SUBGROUP_SEPARATOR))
.map(this::normalizeStatic)
.collect(Collectors.toList());
}

private String normalizeStatic(String subgroup) {
if (subgroup.startsWith(STATIC_SYMBOL)) {
return subgroup.replace(STATIC_SYMBOL, STATIC_KEYWORD);
}
return subgroup;
}

public List<String> getSubGroups() {
return subGroups;
}
}

static List<String> sort(List<String> imports, List<String> importsOrder, boolean wildcardsLast, String lineFormat) {
ImportSorterImpl importsSorter = new ImportSorterImpl(importsOrder, wildcardsLast);
return importsSorter.sort(imports, lineFormat);
Expand All @@ -40,43 +71,42 @@ private List<String> sort(List<String> imports, String lineFormat) {
filterMatchingImports(imports);
mergeNotMatchingItems(false);
mergeNotMatchingItems(true);
mergeMatchingItems();
List<String> sortedImported = mergeMatchingItems();

return getResult(lineFormat);
return getResult(sortedImported, lineFormat);
}

private ImportSorterImpl(List<String> importOrder, boolean wildcardsLast) {
List<String> importOrderCopy = new ArrayList<>(importOrder);
normalizeStaticOrderItems(importOrderCopy);
putStaticItemIfNotExists(importOrderCopy);
template.addAll(importOrderCopy);
importsGroups = importOrder.stream().filter(Objects::nonNull).map(ImportsGroup::new).collect(Collectors.toList());
putStaticItemIfNotExists(importsGroups);
putCatchAllGroupIfNotExists(importsGroups);

ordering = new OrderingComparator(wildcardsLast);
this.allImportOrderItems.addAll(importOrderCopy);

List<String> subgroups = importsGroups.stream().map(ImportsGroup::getSubGroups).flatMap(Collection::stream).collect(Collectors.toList());
this.allImportOrderItems.addAll(subgroups);
}

private static void putStaticItemIfNotExists(List<String> allImportOrderItems) {
boolean contains = false;
private void putStaticItemIfNotExists(List<ImportsGroup> importsGroups) {
boolean catchAllSubGroupExist = importsGroups.stream().anyMatch(group -> group.getSubGroups().contains(STATIC_KEYWORD));
if (catchAllSubGroupExist) {
return;
}

int indexOfFirstStatic = 0;
for (int i = 0; i < allImportOrderItems.size(); i++) {
String allImportOrderItem = allImportOrderItems.get(i);
if (allImportOrderItem.equals("static ")) {
contains = true;
}
if (allImportOrderItem.startsWith("static ")) {
for (int i = 0; i < importsGroups.size(); i++) {
boolean subgroupMatch = importsGroups.get(i).getSubGroups().stream().anyMatch(subgroup -> subgroup.startsWith(STATIC_KEYWORD));
if (subgroupMatch) {
indexOfFirstStatic = i;
}
}
if (!contains) {
allImportOrderItems.add(indexOfFirstStatic, "static ");
}
importsGroups.add(indexOfFirstStatic, new ImportsGroup(STATIC_KEYWORD));
}

private static void normalizeStaticOrderItems(List<String> allImportOrderItems) {
for (int i = 0; i < allImportOrderItems.size(); i++) {
String s = allImportOrderItems.get(i);
if (s.startsWith("\\#")) {
allImportOrderItems.set(i, s.replace("\\#", "static "));
}
private void putCatchAllGroupIfNotExists(List<ImportsGroup> importsGroups) {
boolean catchAllSubGroupExist = importsGroups.stream().anyMatch(group -> group.getSubGroups().contains(CATCH_ALL_SUBGROUP));
if (!catchAllSubGroupExist) {
importsGroups.add(new ImportsGroup(CATCH_ALL_SUBGROUP));
}
}

Expand All @@ -87,9 +117,7 @@ private void filterMatchingImports(List<String> imports) {
for (String anImport : imports) {
String orderItem = getBestMatchingImportOrderItem(anImport);
if (orderItem != null) {
if (!matchingImports.containsKey(orderItem)) {
matchingImports.put(orderItem, new ArrayList<>());
}
matchingImports.computeIfAbsent(orderItem, key -> new ArrayList<>());
matchingImports.get(orderItem).add(anImport);
} else {
notMatching.add(anImport);
Expand All @@ -116,34 +144,14 @@ private void filterMatchingImports(List<String> imports) {
* not matching means it does not match any order item, so it will be appended before or after order items
*/
private void mergeNotMatchingItems(boolean staticItems) {
sort(notMatching);

int firstIndexOfOrderItem = getFirstIndexOfOrderItem(notMatching, staticItems);
int indexOfOrderItem = 0;
for (String notMatchingItem : notMatching) {
if (!matchesStatic(staticItems, notMatchingItem)) {
continue;
}
boolean isOrderItem = isOrderItem(notMatchingItem, staticItems);
if (isOrderItem) {
indexOfOrderItem = template.indexOf(notMatchingItem);
} else {
if (indexOfOrderItem == 0 && firstIndexOfOrderItem != 0) {
// insert before alphabetically first order item
template.add(firstIndexOfOrderItem, notMatchingItem);
firstIndexOfOrderItem++;
} else if (firstIndexOfOrderItem == 0) {
// no order is specified
if (template.size() > 0 && (template.get(template.size() - 1).startsWith("static"))) {
// insert N after last static import
template.add(ImportSorter.N);
}
template.add(notMatchingItem);
} else {
// insert after the previous order item
template.add(indexOfOrderItem + 1, notMatchingItem);
indexOfOrderItem++;
}
if (!isOrderItem) {
matchingImports.computeIfAbsent(CATCH_ALL_SUBGROUP, key -> new ArrayList<>());
matchingImports.get(CATCH_ALL_SUBGROUP).add(notMatchingItem);
}
}
}
Expand All @@ -153,76 +161,44 @@ private boolean isOrderItem(String notMatchingItem, boolean staticItems) {
return contains && matchesStatic(staticItems, notMatchingItem);
}

/**
* gets first order item from sorted input list, and finds out it's index in template.
*/
private int getFirstIndexOfOrderItem(List<String> notMatching, boolean staticItems) {
int firstIndexOfOrderItem = 0;
for (String notMatchingItem : notMatching) {
if (!matchesStatic(staticItems, notMatchingItem)) {
continue;
}
boolean isOrderItem = isOrderItem(notMatchingItem, staticItems);
if (isOrderItem) {
firstIndexOfOrderItem = template.indexOf(notMatchingItem);
break;
}
}
return firstIndexOfOrderItem;
}

private static boolean matchesStatic(boolean staticItems, String notMatchingItem) {
boolean isStatic = notMatchingItem.startsWith("static ");
boolean isStatic = notMatchingItem.startsWith(STATIC_KEYWORD);
return (isStatic && staticItems) || (!isStatic && !staticItems);
}

private void mergeMatchingItems() {
for (int i = 0; i < template.size(); i++) {
String item = template.get(i);
if (allImportOrderItems.contains(item)) {
// find matching items for order item
List<String> strings = matchingImports.get(item);
private List<String> mergeMatchingItems() {
List<String> template = new ArrayList<>();
for (ImportsGroup group : importsGroups) {
boolean groupIsNotEmpty = false;
for (String subgroup : group.getSubGroups()) {
List<String> strings = matchingImports.get(subgroup);
if (strings == null || strings.isEmpty()) {
// if there is none, just remove order item
template.remove(i);
i--;
continue;
}
groupIsNotEmpty = true;
List<String> matchingItems = new ArrayList<>(strings);
sort(matchingItems);

// replace order item by matching import statements
// this is a mess and it is only a luck that it works :-]
template.remove(i);
if (i != 0 && !template.get(i - 1).equals(ImportSorter.N)) {
template.add(i, ImportSorter.N);
i++;
}
if (i + 1 < template.size() && !template.get(i + 1).equals(ImportSorter.N)
&& !template.get(i).equals(ImportSorter.N)) {
template.add(i, ImportSorter.N);
}
template.addAll(i, matchingItems);
if (i != 0 && !template.get(i - 1).equals(ImportSorter.N)) {
template.add(i, ImportSorter.N);
}

template.addAll(matchingItems);
}
if (groupIsNotEmpty) {
template.add(ImportSorter.N);
}
}
// if there is \n on the end, remove it
if (template.size() > 0 && template.get(template.size() - 1).equals(ImportSorter.N)) {
if (!template.isEmpty() && template.get(template.size() - 1).equals(ImportSorter.N)) {
template.remove(template.size() - 1);
}
return template;
}

private void sort(List<String> items) {
items.sort(ordering);
}

private List<String> getResult(String lineFormat) {
private List<String> getResult(List<String> sortedImported, String lineFormat) {
List<String> strings = new ArrayList<>();

for (String s : template) {
for (String s : sortedImported) {
if (s.equals(ImportSorter.N)) {
strings.add(s);
} else {
Expand Down
4 changes: 2 additions & 2 deletions plugin-gradle/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,8 @@ spotless {
// Use the default importOrder configuration
importOrder()
// optional: you can specify import groups directly
// note: you can use an empty string for all the imports you didn't specify explicitly, and '\\#` prefix for static imports
importOrder('java', 'javax', 'com.acme', '', '\\#com.acme', '\\#')
// note: you can use an empty string for all the imports you didn't specify explicitly, '|' to join group without blank line, and '\\#` prefix for static imports
importOrder('java|javax', 'com.acme', '', '\\#com.acme', '\\#')
// optional: instead of specifying import groups directly you can specify a config file
// export config file: https://github.com/diffplug/spotless/blob/main/ECLIPSE_SCREENSHOTS.md#creating-spotlessimportorder
importOrderFile('eclipse-import-order.txt') // import order file as exported from eclipse
Expand Down
8 changes: 4 additions & 4 deletions plugin-maven/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,8 @@ any other maven phase (i.e. compile) then it can be configured as below;
<importOrder /> <!-- standard import order -->
<importOrder> <!-- or a custom ordering -->
<wildcardsLast>false</wildcardsLast> <!-- Optional, default false. Sort wildcard import after specific imports -->
<order>java,javax,org,com,com.diffplug,,\\#com.diffplug,\\#</order> <!-- or use <file>${project.basedir}/eclipse.importorder</file> -->
<!-- you can use an empty string for all the imports you didn't specify explicitly, and '\\#` prefix for static imports. -->
<order>java|javax,org,com,com.diffplug,,\\#com.diffplug,\\#</order> <!-- or use <file>${project.basedir}/eclipse.importorder</file> -->
<!-- you can use an empty string for all the imports you didn't specify explicitly, '|' to join group without blank line, and '\\#` prefix for static imports. -->
</importOrder>

<removeUnusedImports /> <!-- self-explanatory -->
Expand Down Expand Up @@ -286,8 +286,8 @@ These mechanisms already exist for the Gradle plugin.

<importOrder /> <!-- standard import order -->
<importOrder> <!-- or a custom ordering -->
<order>java,javax,org,com,com.diffplug,,\\#com.diffplug,\\#</order> <!-- or use <file>${project.basedir}/eclipse.importorder</file> -->
<!-- you can use an empty string for all the imports you didn't specify explicitly, and '\\#` prefix for static imports -->
<order>java|javax,org,com,com.diffplug,,\\#com.diffplug,\\#</order> <!-- or use <file>${project.basedir}/eclipse.importorder</file> -->
<!-- you can use an empty string for all the imports you didn't specify explicitly, '|' to join group without blank line, and '\\#` prefix for static imports. -->
</importOrder>

<greclipse /> <!-- has its own section below -->
Expand Down
16 changes: 16 additions & 0 deletions testlib/src/main/resources/java/importsorter/JavaCodeSortedImportsSubgroups.test
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import java.awt.*;
import java.lang.Runnable;
import java.lang.Thread;
import java.util.*;
import java.util.List;
import javax.annotation.Nullable;
import javax.inject.Inject;

import org.dooda.Didoo;
import static com.foo.Bar;
import static com.github.tomakehurst.wiremock.client.WireMock.*;
import static com.github.tomakehurst.wiremock.client.WireMock.equalTo;

import static java.lang.Exception.*;
import static java.lang.Runnable.*;
import static org.hamcrest.Matchers.*;
15 changes: 15 additions & 0 deletions testlib/src/main/resources/java/importsorter/JavaCodeUnsortedImportsSubgroups.test
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import static java.lang.Exception.*;
import static com.github.tomakehurst.wiremock.client.WireMock.*;
import org.dooda.Didoo;
import java.util.List;
import javax.inject.Inject;
import java.lang.Thread;
import java.util.*;
import java.lang.Runnable;
import static org.hamcrest.Matchers.*;
import javax.annotation.Nullable;

import static java.lang.Runnable.*;
import static com.github.tomakehurst.wiremock.client.WireMock.equalTo;
import static com.foo.Bar
import java.awt.*;
6 changes: 6 additions & 0 deletions testlib/src/test/java/com/diffplug/spotless/java/ImportOrderStepTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ void sortImportsFromArray() throws Throwable {
assertOnResources(step, "java/importsorter/JavaCodeUnsortedImports.test", "java/importsorter/JavaCodeSortedImports.test");
}

@Test
void sortImportsFromArrayWithSubgroups() throws Throwable {
FormatterStep step = ImportOrderStep.forJava().createFrom("java|javax", "org|\\#com", "\\#");
assertOnResources(step, "java/importsorter/JavaCodeUnsortedImportsSubgroups.test", "java/importsorter/JavaCodeSortedImportsSubgroups.test");
}

@Test
void sortImportsFromFile() throws Throwable {
FormatterStep step = ImportOrderStep.forJava().createFrom(createTestFile("java/importsorter/import.properties"));
Expand Down

0 comments on commit 35a56ea

Please sign in to comment.