Skip to content

Commit c2ec7e1

Browse files
nedtwiggnedtwigg
authored
bugfix: reorder import in GJF using Google Style (#1680)
2 parents 94ba918 + 64ab314 commit c2ec7e1

File tree

7 files changed

+276
-3
lines changed

7 files changed

+276
-3
lines changed

‎CHANGES.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (
1515
* 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))
1616
### Fixed
1717
* When P2 download fails, indicate the responsible formatter. ([#1698](https://github.com/diffplug/spotless/issues/1698))
18+
* Fixed a regression which changed the import sorting order in `googleJavaFormat` introduced in `2.38.0`. ([#1680](https://github.com/diffplug/spotless/pull/1680))
1819
### Changes
1920
* Bump default sortpom version to latest `3.0.0` -> `3.2.1`. ([#1675](https://github.com/diffplug/spotless/pull/1675))
2021

‎lib/src/googleJavaFormat/java/com/diffplug/spotless/glue/java/GoogleJavaFormatFormatterFunc.java

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import com.google.googlejavaformat.java.FormatterException;
2424
import com.google.googlejavaformat.java.ImportOrderer;
2525
import com.google.googlejavaformat.java.JavaFormatterOptions;
26+
import com.google.googlejavaformat.java.JavaFormatterOptions.Style;
2627
import com.google.googlejavaformat.java.RemoveUnusedImports;
2728
import com.google.googlejavaformat.java.StringWrapper;
2829

@@ -37,13 +38,13 @@ public class GoogleJavaFormatFormatterFunc implements FormatterFunc {
3738
private final String version;
3839

3940
@Nonnull
40-
private final JavaFormatterOptions.Style formatterStyle;
41+
private final Style formatterStyle;
4142

4243
private final boolean reflowStrings;
4344

4445
public GoogleJavaFormatFormatterFunc(@Nonnull String version, @Nonnull String style, boolean reflowStrings) {
4546
this.version = Objects.requireNonNull(version);
46-
this.formatterStyle = JavaFormatterOptions.Style.valueOf(Objects.requireNonNull(style));
47+
this.formatterStyle = Style.valueOf(Objects.requireNonNull(style));
4748
this.reflowStrings = reflowStrings;
4849

4950
this.formatter = new Formatter(JavaFormatterOptions.builder()
@@ -56,7 +57,10 @@ public GoogleJavaFormatFormatterFunc(@Nonnull String version, @Nonnull String st
5657
public String apply(@Nonnull String input) throws Exception {
5758
String formatted = formatter.formatSource(input);
5859
String removedUnused = RemoveUnusedImports.removeUnusedImports(formatted);
59-
String sortedImports = ImportOrderer.reorderImports(removedUnused, formatterStyle);
60+
// Issue #1679: we used to call ImportOrderer.reorderImports(String) here, but that is deprecated.
61+
// Replacing the call with (the correct) reorderImports(String, Style) causes issues for Style.AOSP,
62+
// so we force the style to GOOGLE for now (which is what the deprecated method did)
63+
String sortedImports = ImportOrderer.reorderImports(removedUnused, Style.GOOGLE);
6064
return reflowLongStrings(sortedImports);
6165
}
6266

‎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
### Fixed
99
* Added `@DisableCachingByDefault` to `RegisterDependenciesTask`. ([#1666](https://github.com/diffplug/spotless/pull/1666))
1010
* When P2 download fails, indicate the responsible formatter. ([#1698](https://github.com/diffplug/spotless/issues/1698))
11+
* Fixed a regression which changed the import sorting order in `googleJavaFormat` introduced in `6.18.0`. ([#1680](https://github.com/diffplug/spotless/pull/1680))
1112
### Changes
1213
* Bump default sortpom version to latest `3.0.0` -> `3.2.1`. ([#1675](https://github.com/diffplug/spotless/pull/1675))
1314

‎plugin-maven/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
### Fixed
99
* `palantir` step now accepts a `style` parameter, which is documentation had already claimed to do. ([#1694](https://github.com/diffplug/spotless/pull/1694))
1010
* When P2 download fails, indicate the responsible formatter. ([#1698](https://github.com/diffplug/spotless/issues/1698))
11+
* Fixed a regression which changed the import sorting order in `googleJavaFormat` introduced in `2.36.0`. ([#1680](https://github.com/diffplug/spotless/pull/1680))
1112
### Changes
1213
* Bump default sortpom version to latest `3.0.0` -> `3.2.1`. ([#1675](https://github.com/diffplug/spotless/pull/1675))
1314

‎testlib/src/main/resources/combined/issue1679.clean

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
/*
2+
* Copyright 2021-2022 Creek Contributors (https://github.com/creek-service)
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package io.github.creek.service.basic.kafka.streams.demo.services;
18+
19+
// formatting:off
20+
// begin-snippet: includes-1
21+
import static io.github.creek.service.basic.kafka.streams.demo.internal.TopicConfigBuilder.withPartitions;
22+
import static io.github.creek.service.basic.kafka.streams.demo.internal.TopicDescriptors.inputTopic;
23+
import static io.github.creek.service.basic.kafka.streams.demo.internal.TopicDescriptors.outputTopic;
24+
// end-snippet
25+
import java.time.Duration;
26+
import java.util.ArrayList;
27+
import java.util.Collection;
28+
import java.util.List;
29+
// begin-snippet: includes-2
30+
import org.creekservice.api.kafka.metadata.OwnedKafkaTopicInput;
31+
import org.creekservice.api.kafka.metadata.OwnedKafkaTopicOutput;
32+
// end-snippet
33+
import org.creekservice.api.platform.metadata.ComponentInput;
34+
import org.creekservice.api.platform.metadata.ComponentInternal;
35+
import org.creekservice.api.platform.metadata.ComponentOutput;
36+
import org.creekservice.api.platform.metadata.ServiceDescriptor;
37+
// formatting:on
38+
39+
// begin-snippet: class-name
40+
public final class HandleOccurrenceServiceDescriptor implements ServiceDescriptor {
41+
// end-snippet
42+
private static final List<ComponentInput> INPUTS = new ArrayList<>();
43+
private static final List<ComponentInternal> INTERNALS = new ArrayList<>();
44+
private static final List<ComponentOutput> OUTPUTS = new ArrayList<>();
45+
46+
// formatting:off
47+
// begin-snippet: topic-resources
48+
// Define the tweet-text input topic, conceptually owned by this service:
49+
public static final OwnedKafkaTopicInput<Long, String> TweetTextStream =
50+
register(
51+
inputTopic(
52+
"twitter.tweet.text", // Topic name
53+
Long.class, // Topic key: Tweet id
54+
String.class, // Topic value: Tweet text
55+
withPartitions(5))); // Topic config
56+
57+
// Define the output topic, again conceptually owned by this service:
58+
public static final OwnedKafkaTopicOutput<String, Integer> TweetHandleUsageStream =
59+
register(outputTopic(
60+
"twitter.handle.usage",
61+
String.class, // Twitter handle
62+
Integer.class, // Usage count
63+
withPartitions(6)
64+
.withRetentionTime(Duration.ofHours(12))
65+
));
66+
// end-snippet
67+
// formatting:on
68+
69+
public HandleOccurrenceServiceDescriptor() {}
70+
71+
@Override
72+
public String dockerImage() {
73+
return "ghcr.io/creek-service/basic-kafka-streams-demo-handle-occurrence-service";
74+
}
75+
76+
@Override
77+
public Collection<ComponentInput> inputs() {
78+
return List.copyOf(INPUTS);
79+
}
80+
81+
@Override
82+
public Collection<ComponentInternal> internals() {
83+
return List.copyOf(INTERNALS);
84+
}
85+
86+
@Override
87+
public Collection<ComponentOutput> outputs() {
88+
return List.copyOf(OUTPUTS);
89+
}
90+
91+
private static <T extends ComponentInput> T register(final T input) {
92+
INPUTS.add(input);
93+
return input;
94+
}
95+
96+
// Uncomment if needed:
97+
// private static <T extends ComponentInternal> T register(final T internal) {
98+
// INTERNALS.add(internal);
99+
// return internal;
100+
// }
101+
102+
private static <T extends ComponentOutput> T register(final T output) {
103+
OUTPUTS.add(output);
104+
return output;
105+
}
106+
}

‎testlib/src/main/resources/combined/issue1679.dirty

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
/*
2+
* Copyright 2021-2022 Creek Contributors (https://github.com/creek-service)
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package io.github.creek.service.basic.kafka.streams.demo.services;
18+
19+
// formatting:off
20+
// begin-snippet: includes-1
21+
import static io.github.creek.service.basic.kafka.streams.demo.internal.TopicConfigBuilder.withPartitions;
22+
import static io.github.creek.service.basic.kafka.streams.demo.internal.TopicDescriptors.inputTopic;
23+
import static io.github.creek.service.basic.kafka.streams.demo.internal.TopicDescriptors.outputTopic;
24+
// end-snippet
25+
import java.time.Duration;
26+
import java.util.ArrayList;
27+
import java.util.Collection;
28+
import java.util.List;
29+
// begin-snippet: includes-2
30+
import org.creekservice.api.kafka.metadata.OwnedKafkaTopicInput;
31+
import org.creekservice.api.kafka.metadata.OwnedKafkaTopicOutput;
32+
// end-snippet
33+
import org.creekservice.api.platform.metadata.ComponentInput;
34+
import org.creekservice.api.platform.metadata.ComponentInternal;
35+
import org.creekservice.api.platform.metadata.ComponentOutput;
36+
import org.creekservice.api.platform.metadata.ServiceDescriptor;
37+
// formatting:on
38+
39+
// begin-snippet: class-name
40+
public final class HandleOccurrenceServiceDescriptor implements ServiceDescriptor {
41+
// end-snippet
42+
private static final List<ComponentInput> INPUTS = new ArrayList<>();
43+
private static final List<ComponentInternal> INTERNALS = new ArrayList<>();
44+
private static final List<ComponentOutput> OUTPUTS = new ArrayList<>();
45+
46+
// formatting:off
47+
// begin-snippet: topic-resources
48+
// Define the tweet-text input topic, conceptually owned by this service:
49+
public static final OwnedKafkaTopicInput<Long, String> TweetTextStream =
50+
register(
51+
inputTopic(
52+
"twitter.tweet.text", // Topic name
53+
Long.class, // Topic key: Tweet id
54+
String.class, // Topic value: Tweet text
55+
withPartitions(5))); // Topic config
56+
57+
// Define the output topic, again conceptually owned by this service:
58+
public static final OwnedKafkaTopicOutput<String, Integer> TweetHandleUsageStream =
59+
register(outputTopic(
60+
"twitter.handle.usage",
61+
String.class, // Twitter handle
62+
Integer.class, // Usage count
63+
withPartitions(6)
64+
.withRetentionTime(Duration.ofHours(12))
65+
));
66+
// end-snippet
67+
// formatting:on
68+
69+
public HandleOccurrenceServiceDescriptor() {}
70+
71+
@Override
72+
public String dockerImage() {
73+
return "ghcr.io/creek-service/basic-kafka-streams-demo-handle-occurrence-service";
74+
}
75+
76+
@Override
77+
public Collection<ComponentInput> inputs() { return List.copyOf(INPUTS); }
78+
79+
@Override
80+
public Collection<ComponentInternal> internals() {
81+
return List.copyOf(INTERNALS);
82+
}
83+
84+
@Override
85+
public Collection<ComponentOutput> outputs() {
86+
return List.copyOf(OUTPUTS);
87+
}
88+
89+
private static <T extends ComponentInput> T register(final T input) {
90+
INPUTS.add(input);
91+
return input;
92+
}
93+
94+
// Uncomment if needed:
95+
// private static <T extends ComponentInternal> T register(final T internal) {
96+
// INTERNALS.add(internal);
97+
// return internal;
98+
// }
99+
100+
private static <T extends ComponentOutput> T register(final T output) {
101+
OUTPUTS.add(output);
102+
return output;
103+
}
104+
}

‎testlib/src/test/java/com/diffplug/spotless/combined/CombinedJavaFormatStepTest.java

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
/*
2+
* Copyright 2023 DiffPlug
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.diffplug.spotless.combined;
17+
18+
import static com.diffplug.spotless.TestProvisioner.mavenCentral;
19+
20+
import org.junit.jupiter.api.Test;
21+
22+
import com.diffplug.spotless.FormatterStep;
23+
import com.diffplug.spotless.ResourceHarness;
24+
import com.diffplug.spotless.StepHarness;
25+
import com.diffplug.spotless.generic.EndWithNewlineStep;
26+
import com.diffplug.spotless.generic.IndentStep;
27+
import com.diffplug.spotless.generic.PipeStepPair;
28+
import com.diffplug.spotless.generic.TrimTrailingWhitespaceStep;
29+
import com.diffplug.spotless.java.GoogleJavaFormatStep;
30+
import com.diffplug.spotless.java.ImportOrderStep;
31+
import com.diffplug.spotless.java.RemoveUnusedImportsStep;
32+
33+
public class CombinedJavaFormatStepTest extends ResourceHarness {
34+
35+
@Test
36+
void checkIssue1679() {
37+
FormatterStep gjf = GoogleJavaFormatStep.create("1.15.0", "AOSP", mavenCentral());
38+
FormatterStep indentWithSpaces = IndentStep.Type.SPACE.create();
39+
FormatterStep importOrder = ImportOrderStep.forJava().createFrom();
40+
FormatterStep removeUnused = RemoveUnusedImportsStep.create(mavenCentral());
41+
FormatterStep trimTrailing = TrimTrailingWhitespaceStep.create();
42+
FormatterStep endWithNewLine = EndWithNewlineStep.create();
43+
PipeStepPair toggleOffOnPair = PipeStepPair.named(PipeStepPair.defaultToggleName()).openClose("formatting:off", "formatting:on").buildPair();
44+
try (StepHarness formatter = StepHarness.forSteps(
45+
toggleOffOnPair.in(),
46+
gjf,
47+
indentWithSpaces,
48+
importOrder,
49+
removeUnused,
50+
trimTrailing,
51+
endWithNewLine,
52+
toggleOffOnPair.out())) {
53+
formatter.testResource("combined/issue1679.dirty", "combined/issue1679.clean");
54+
}
55+
}
56+
}

0 commit comments

Comments
 (0)