Skip to content

bugfix: reorder import in GJF using Google Style #1680

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

Merged
merged 4 commits into from
May 17, 2023
Merged
Show file tree
Hide file tree
Changes from all 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 @@ -15,6 +15,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (
* 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))
### 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))
### Changes
* Bump default sortpom version to latest `3.0.0` -> `3.2.1`. ([#1675](https://github.com/diffplug/spotless/pull/1675))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import com.google.googlejavaformat.java.FormatterException;
import com.google.googlejavaformat.java.ImportOrderer;
import com.google.googlejavaformat.java.JavaFormatterOptions;
import com.google.googlejavaformat.java.JavaFormatterOptions.Style;
import com.google.googlejavaformat.java.RemoveUnusedImports;
import com.google.googlejavaformat.java.StringWrapper;

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

@Nonnull
private final JavaFormatterOptions.Style formatterStyle;
private final Style formatterStyle;

private final boolean reflowStrings;

public GoogleJavaFormatFormatterFunc(@Nonnull String version, @Nonnull String style, boolean reflowStrings) {
this.version = Objects.requireNonNull(version);
this.formatterStyle = JavaFormatterOptions.Style.valueOf(Objects.requireNonNull(style));
this.formatterStyle = Style.valueOf(Objects.requireNonNull(style));
this.reflowStrings = reflowStrings;

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

Expand Down
1 change: 1 addition & 0 deletions plugin-gradle/CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (
### 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))
* Fixed a regression which changed the import sorting order in `googleJavaFormat` introduced in `6.18.0`. ([#1680](https://github.com/diffplug/spotless/pull/1680))
### Changes
* Bump default sortpom version to latest `3.0.0` -> `3.2.1`. ([#1675](https://github.com/diffplug/spotless/pull/1675))

Expand Down
1 change: 1 addition & 0 deletions plugin-maven/CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (
### Fixed
* `palantir` step now accepts a `style` parameter, which is documentation had already claimed to do. ([#1694](https://github.com/diffplug/spotless/pull/1694))
* 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.36.0`. ([#1680](https://github.com/diffplug/spotless/pull/1680))
### Changes
* Bump default sortpom version to latest `3.0.0` -> `3.2.1`. ([#1675](https://github.com/diffplug/spotless/pull/1675))

Expand Down
106 changes: 106 additions & 0 deletions testlib/src/main/resources/combined/issue1679.clean
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
/*
* Copyright 2021-2022 Creek Contributors (https://github.com/creek-service)
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.github.creek.service.basic.kafka.streams.demo.services;

// formatting:off
// begin-snippet: includes-1
import static io.github.creek.service.basic.kafka.streams.demo.internal.TopicConfigBuilder.withPartitions;
import static io.github.creek.service.basic.kafka.streams.demo.internal.TopicDescriptors.inputTopic;
import static io.github.creek.service.basic.kafka.streams.demo.internal.TopicDescriptors.outputTopic;
// end-snippet
import java.time.Duration;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
// begin-snippet: includes-2
import org.creekservice.api.kafka.metadata.OwnedKafkaTopicInput;
import org.creekservice.api.kafka.metadata.OwnedKafkaTopicOutput;
// end-snippet
import org.creekservice.api.platform.metadata.ComponentInput;
import org.creekservice.api.platform.metadata.ComponentInternal;
import org.creekservice.api.platform.metadata.ComponentOutput;
import org.creekservice.api.platform.metadata.ServiceDescriptor;
// formatting:on

// begin-snippet: class-name
public final class HandleOccurrenceServiceDescriptor implements ServiceDescriptor {
// end-snippet
private static final List<ComponentInput> INPUTS = new ArrayList<>();
private static final List<ComponentInternal> INTERNALS = new ArrayList<>();
private static final List<ComponentOutput> OUTPUTS = new ArrayList<>();

// formatting:off
// begin-snippet: topic-resources
// Define the tweet-text input topic, conceptually owned by this service:
public static final OwnedKafkaTopicInput<Long, String> TweetTextStream =
register(
inputTopic(
"twitter.tweet.text", // Topic name
Long.class, // Topic key: Tweet id
String.class, // Topic value: Tweet text
withPartitions(5))); // Topic config

// Define the output topic, again conceptually owned by this service:
public static final OwnedKafkaTopicOutput<String, Integer> TweetHandleUsageStream =
register(outputTopic(
"twitter.handle.usage",
String.class, // Twitter handle
Integer.class, // Usage count
withPartitions(6)
.withRetentionTime(Duration.ofHours(12))
));
// end-snippet
// formatting:on

public HandleOccurrenceServiceDescriptor() {}

@Override
public String dockerImage() {
return "ghcr.io/creek-service/basic-kafka-streams-demo-handle-occurrence-service";
}

@Override
public Collection<ComponentInput> inputs() {
return List.copyOf(INPUTS);
}

@Override
public Collection<ComponentInternal> internals() {
return List.copyOf(INTERNALS);
}

@Override
public Collection<ComponentOutput> outputs() {
return List.copyOf(OUTPUTS);
}

private static <T extends ComponentInput> T register(final T input) {
INPUTS.add(input);
return input;
}

// Uncomment if needed:
// private static <T extends ComponentInternal> T register(final T internal) {
// INTERNALS.add(internal);
// return internal;
// }

private static <T extends ComponentOutput> T register(final T output) {
OUTPUTS.add(output);
return output;
}
}
104 changes: 104 additions & 0 deletions testlib/src/main/resources/combined/issue1679.dirty
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
/*
* Copyright 2021-2022 Creek Contributors (https://github.com/creek-service)
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.github.creek.service.basic.kafka.streams.demo.services;

// formatting:off
// begin-snippet: includes-1
import static io.github.creek.service.basic.kafka.streams.demo.internal.TopicConfigBuilder.withPartitions;
import static io.github.creek.service.basic.kafka.streams.demo.internal.TopicDescriptors.inputTopic;
import static io.github.creek.service.basic.kafka.streams.demo.internal.TopicDescriptors.outputTopic;
// end-snippet
import java.time.Duration;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
// begin-snippet: includes-2
import org.creekservice.api.kafka.metadata.OwnedKafkaTopicInput;
import org.creekservice.api.kafka.metadata.OwnedKafkaTopicOutput;
// end-snippet
import org.creekservice.api.platform.metadata.ComponentInput;
import org.creekservice.api.platform.metadata.ComponentInternal;
import org.creekservice.api.platform.metadata.ComponentOutput;
import org.creekservice.api.platform.metadata.ServiceDescriptor;
// formatting:on

// begin-snippet: class-name
public final class HandleOccurrenceServiceDescriptor implements ServiceDescriptor {
// end-snippet
private static final List<ComponentInput> INPUTS = new ArrayList<>();
private static final List<ComponentInternal> INTERNALS = new ArrayList<>();
private static final List<ComponentOutput> OUTPUTS = new ArrayList<>();

// formatting:off
// begin-snippet: topic-resources
// Define the tweet-text input topic, conceptually owned by this service:
public static final OwnedKafkaTopicInput<Long, String> TweetTextStream =
register(
inputTopic(
"twitter.tweet.text", // Topic name
Long.class, // Topic key: Tweet id
String.class, // Topic value: Tweet text
withPartitions(5))); // Topic config

// Define the output topic, again conceptually owned by this service:
public static final OwnedKafkaTopicOutput<String, Integer> TweetHandleUsageStream =
register(outputTopic(
"twitter.handle.usage",
String.class, // Twitter handle
Integer.class, // Usage count
withPartitions(6)
.withRetentionTime(Duration.ofHours(12))
));
// end-snippet
// formatting:on

public HandleOccurrenceServiceDescriptor() {}

@Override
public String dockerImage() {
return "ghcr.io/creek-service/basic-kafka-streams-demo-handle-occurrence-service";
}

@Override
public Collection<ComponentInput> inputs() { return List.copyOf(INPUTS); }

@Override
public Collection<ComponentInternal> internals() {
return List.copyOf(INTERNALS);
}

@Override
public Collection<ComponentOutput> outputs() {
return List.copyOf(OUTPUTS);
}

private static <T extends ComponentInput> T register(final T input) {
INPUTS.add(input);
return input;
}

// Uncomment if needed:
// private static <T extends ComponentInternal> T register(final T internal) {
// INTERNALS.add(internal);
// return internal;
// }

private static <T extends ComponentOutput> T register(final T output) {
OUTPUTS.add(output);
return output;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/*
* Copyright 2023 DiffPlug
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.diffplug.spotless.combined;

import static com.diffplug.spotless.TestProvisioner.mavenCentral;

import org.junit.jupiter.api.Test;

import com.diffplug.spotless.FormatterStep;
import com.diffplug.spotless.ResourceHarness;
import com.diffplug.spotless.StepHarness;
import com.diffplug.spotless.generic.EndWithNewlineStep;
import com.diffplug.spotless.generic.IndentStep;
import com.diffplug.spotless.generic.PipeStepPair;
import com.diffplug.spotless.generic.TrimTrailingWhitespaceStep;
import com.diffplug.spotless.java.GoogleJavaFormatStep;
import com.diffplug.spotless.java.ImportOrderStep;
import com.diffplug.spotless.java.RemoveUnusedImportsStep;

public class CombinedJavaFormatStepTest extends ResourceHarness {

@Test
void checkIssue1679() {
FormatterStep gjf = GoogleJavaFormatStep.create("1.15.0", "AOSP", mavenCentral());
FormatterStep indentWithSpaces = IndentStep.Type.SPACE.create();
FormatterStep importOrder = ImportOrderStep.forJava().createFrom();
FormatterStep removeUnused = RemoveUnusedImportsStep.create(mavenCentral());
FormatterStep trimTrailing = TrimTrailingWhitespaceStep.create();
FormatterStep endWithNewLine = EndWithNewlineStep.create();
PipeStepPair toggleOffOnPair = PipeStepPair.named(PipeStepPair.defaultToggleName()).openClose("formatting:off", "formatting:on").buildPair();
try (StepHarness formatter = StepHarness.forSteps(
toggleOffOnPair.in(),
gjf,
indentWithSpaces,
importOrder,
removeUnused,
trimTrailing,
endWithNewLine,
toggleOffOnPair.out())) {
formatter.testResource("combined/issue1679.dirty", "combined/issue1679.clean");
}
}
}