Skip to content

Commit 23c9341

Browse files
authored
Fix: importOrder() with single catch-all group strips blank lines added by greclipse(), causing non-idempotent formatting (#2914)
2 parents 9aeda7b + 530a093 commit 23c9341

8 files changed

Lines changed: 51 additions & 4 deletions

File tree

CHANGES.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (
1212
## [Unreleased]
1313
### Added
1414
- Add `javaparserVersion` option to the Cleanthat step, allowing callers to override the JavaParser version pulled in transitively by Cleanthat. ([#2903](https://github.com/diffplug/spotless/pull/2903))
15+
### Fixed
16+
- Fix non-idempotent formatting when `importOrder()` is combined with `greclipse()`: a single catch-all group no longer strips blank lines that `greclipse()` independently inserted between import groups. ([#2914](https://github.com/diffplug/spotless/pull/2914))
1517
### Changes
1618
- Bump default `cleanthat` version `2.24` -> `2.25`. ([#2903](https://github.com/diffplug/spotless/pull/2903))
1719
- Bump default `eclipse-jdt` version from `4.35` to `4.39`. ([#2912](https://github.com/diffplug/spotless/pull/2912))

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2016-2023 DiffPlug
2+
* Copyright 2016-2026 DiffPlug
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -19,6 +19,7 @@
1919
import java.util.List;
2020
import java.util.Scanner;
2121
import java.util.Set;
22+
import java.util.stream.Collectors;
2223

2324
/**
2425
* @author Vojtech Krasa
@@ -91,6 +92,15 @@ String format(String raw, String lineFormat) {
9192

9293
List<String> sortedImports = ImportSorterImpl.sort(imports, importsOrder, wildcardsLast, semanticSort,
9394
treatAsPackage, treatAsClass, lineFormat);
95+
boolean sortedHasNoBlanks = sortedImports.stream().noneMatch(N::equals);
96+
if (sortedHasNoBlanks && firstImportLine > 0) {
97+
List<String> originalFormatted = imports.stream()
98+
.map(i -> lineFormat.formatted(i) + N)
99+
.collect(Collectors.toList());
100+
if (sortedImports.equals(originalFormatted)) {
101+
return raw;
102+
}
103+
}
94104
return applyImportsToDocument(raw, firstImportLine, lastImportLine, sortedImports);
95105
}
96106

plugin-gradle/CHANGES.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (
88
- Add `javaparserVersion(...)` to `cleanthat`, allowing users to override the JavaParser version pulled in transitively by Cleanthat. ([#2903](https://github.com/diffplug/spotless/pull/2903))
99
### Fixed
1010
- Fix `tableTestFormatter` editorconfig cache not honoring `.editorconfig` changes across Gradle daemon runs due to a shared static `EditorConfigProvider`. ([#2893](https://github.com/diffplug/spotless/pull/2893))
11+
- Fix non-idempotent formatting when `importOrder()` is combined with `greclipse()`: a single catch-all group no longer strips blank lines that `greclipse()` independently inserted between import groups. ([#2914](https://github.com/diffplug/spotless/pull/2914))
1112
### Changes
1213
- Bump default `cleanthat` version `2.24` -> `2.25`. ([#2903](https://github.com/diffplug/spotless/pull/2903))
1314
- Bump default `eclipse-jdt` version from `4.35` to `4.39`. ([#2912](https://github.com/diffplug/spotless/pull/2912))

plugin-maven/CHANGES.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (
55
## [Unreleased]
66
### Added
77
- Add `<javaparserVersion>` option to `<cleanthat>`, allowing users to override the JavaParser version pulled in transitively by Cleanthat. ([#2903](https://github.com/diffplug/spotless/pull/2903))
8+
### Fixed
9+
- Fix non-idempotent formatting when `importOrder()` is combined with `greclipse()`: a single catch-all group no longer strips blank lines that `greclipse()` independently inserted between import groups. ([#2914](https://github.com/diffplug/spotless/pull/2914))
810
### Changes
911
- Bump default `cleanthat` version `2.24` -> `2.25`. ([#2903](https://github.com/diffplug/spotless/pull/2903))
1012
- Bump default `eclipse-jdt` version from `4.35` to `4.39`. ([#2912](https://github.com/diffplug/spotless/pull/2912))
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import java.lang.Runnable
2+
import java.lang.Thread
3+
import org.dooda.Didoo
4+
5+
public class NotDeletedByFormatter {
6+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import java.lang.Runnable
2+
import java.lang.Thread
3+
4+
import org.dooda.Didoo
5+
6+
public class NotDeletedByFormatter {
7+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import org.dooda.Didoo
2+
3+
import java.lang.Thread
4+
import java.lang.Runnable
5+
6+
public class NotDeletedByFormatter {
7+
}

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

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2016-2024 DiffPlug
2+
* Copyright 2016-2026 DiffPlug
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -117,12 +117,24 @@ void groovyImports() {
117117
}
118118

119119
@Test
120-
void doesntThrowIfImportOrderIsntSerializable() {
120+
void groovyImportsPreservesBlankLinesBetweenGroups() {
121+
FormatterStep step = ImportOrderStep.forGroovy().createFrom();
122+
StepHarness.forStep(step).testResourceUnaffected("java/importsorter/GroovyCodeSortedImportsWithBlankLines.test");
123+
}
124+
125+
@Test
126+
void groovySortImportsStripsInterleavedBlankLines() {
127+
FormatterStep step = ImportOrderStep.forGroovy().createFrom();
128+
StepHarness.forStep(step).testResource("java/importsorter/GroovyCodeUnsortedImportsWithBlankLines.test", "java/importsorter/GroovyCodeSortedImportsNoBlankLines.test");
129+
}
130+
131+
@Test
132+
void doesntThrowIfImportOrderIsNotSerializable() {
121133
ImportOrderStep.forJava().createFrom("java", "javax", "org", "\\#com");
122134
}
123135

124136
@Test
125-
void equality() throws Exception {
137+
void equality() {
126138
new SerializableEqualityTester() {
127139
String[] imports = {};
128140

0 commit comments

Comments
 (0)