Support for group of imports without blank lines between groups (#1401)

Author: nedtwigg

CHANGES.md

@@ -10,6 +10,8 @@ This document is intended for Spotless developers.
 We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `1.27.0`).
 
 ## [Unreleased]
+### Added
+* `importOrder` now support groups of imports without blank lines ([#1401](https://github.com/diffplug/spotless/pull/1401))
 ### 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)

lib/src/main/java/com/diffplug/spotless/java/ImportSorterImpl.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2016-2021 DiffPlug
+ * Copyright 2016-2022 DiffPlug
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -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;
 
@@ -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);
@@ -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));
 		}
 	}
 
@@ -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);
@@ -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);
 			}
 		}
 	}
@@ -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 {

plugin-gradle/CHANGES.md

@@ -3,6 +3,8 @@
 We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `3.27.0`).
 
 ## [Unreleased]
+### Added
+* `importOrder` now support groups of imports without blank lines ([#1401](https://github.com/diffplug/spotless/pull/1401))
 ### 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)

plugin-gradle/README.md

@@ -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

plugin-maven/CHANGES.md

@@ -3,6 +3,8 @@
 We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `1.27.0`).
 
 ## [Unreleased]
+### Added
+* `importOrder` now support groups of imports without blank lines ([#1401](https://github.com/diffplug/spotless/pull/1401))
 ### 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)

plugin-maven/README.md

@@ -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 -->
@@ -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 -->

testlib/src/main/resources/java/importsorter/JavaCodeSortedImportsSubgroups.test

@@ -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.*;

testlib/src/main/resources/java/importsorter/JavaCodeUnsortedImportsSubgroups.test

@@ -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.*;

testlib/src/test/java/com/diffplug/spotless/java/ImportOrderStepTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2016-2021 DiffPlug
+ * Copyright 2016-2022 DiffPlug
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -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"));