Skip to content
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

Serializable Groovy, Java, Kotlin, and Scala steps #2033

Merged
merged 14 commits into from
Feb 13, 2024
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import com.diffplug.spotless.FormatterStep;
import com.diffplug.spotless.JarState;
import com.diffplug.spotless.Provisioner;
import com.diffplug.spotless.RoundedStep;
import com.diffplug.spotless.SerializedFunction;

/**
Expand Down Expand Up @@ -126,7 +127,7 @@ public void setPreferences(Iterable<File> settingsFiles) {
this.settingsFiles = settingsFiles;
}

static class EclipseStep implements Serializable {
static class EclipseStep implements RoundedStep {
private static final long serialVersionUID = 1;
private final String semanticVersion;
private final String formatterStepExt;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import com.diffplug.spotless.FormatterStep;
import com.diffplug.spotless.JarState;
import com.diffplug.spotless.Provisioner;
import com.diffplug.spotless.RoundedStep;
import com.diffplug.spotless.SerializedFunction;

import dev.equo.solstice.NestedJars;
Expand Down Expand Up @@ -149,7 +150,7 @@ private P2Model createModelWithMirrors() {
return model;
}

static class EquoStep implements Serializable {
static class EquoStep implements RoundedStep {
private static final long serialVersionUID = 1;
private final String semanticVersion;
private final FileSignature.Promised settingsPromise;
Expand Down
20 changes: 20 additions & 0 deletions lib/src/main/java/com/diffplug/spotless/RoundedStep.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/*
* Copyright 2024 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;

import java.io.Serializable;

public interface RoundedStep extends Serializable {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The intent here is to be a marker interface?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Goooler My YAGNI senses are tingling! I'm wondering if you can clarify why this interface was introduced, and if we can get away with inlining Serializable instead?

Otherwise, this all LGTM. 👍

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking that we can do some checks based on a common step type instead of names here

} else if (Set.of("black", "buf", "clang", "ktlint", "ktfmt", "scalafmt", "palantir-java-format", "google-java-format",
"removeUnusedImports", "cleanthat", "No line break between type annotation and type",
"importOrder", "Remove unnecessary semicolons").contains(onlyStepName)) {
supportsRoundTrip = true;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good idea, but it's hard to get to that state. I figured it was probably meant as a testing thing, we can remove it when we finish #1274

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me either way! 👍

Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,13 @@
*/
package com.diffplug.spotless.generic;

import java.io.Serializable;

import com.diffplug.spotless.FormatterFunc;
import com.diffplug.spotless.FormatterStep;
import com.diffplug.spotless.RoundedStep;
import com.diffplug.spotless.SerializedFunction;

/** Simple step which checks for consistent indentation characters. */
public final class IndentStep implements Serializable {
public final class IndentStep implements RoundedStep {
private static final long serialVersionUID = 1L;

final Type type;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2023 DiffPlug
* Copyright 2023-2024 DiffPlug
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -21,23 +21,25 @@

import com.diffplug.spotless.FormatterFunc;
import com.diffplug.spotless.FormatterStep;
import com.diffplug.spotless.RoundedStep;

/**
* Removes all semicolons from the end of lines.
*
* @author Jose Luis Badano
*/
public final class RemoveSemicolonsStep {
public final class RemoveSemicolonsStep implements RoundedStep {
private static final long serialVersionUID = 1L;
private static final String NAME = "Remove unnecessary semicolons";

private RemoveSemicolonsStep() {
// do not instantiate
}

public static FormatterStep create() {
return FormatterStep.createLazy(NAME,
State::new,
RemoveSemicolonsStep.State::toFormatter);
return FormatterStep.create(NAME,
new State(),
State::toFormatter);
}

private static final class State implements Serializable {
Expand All @@ -64,7 +66,7 @@ FormatterFunc toFormatter() {
* @return the line without the last semicolon
*/
private String removeSemicolon(String line) {
// find last semicolon in a string a remove it
// Find the last semicolon in a string and remove it.
int lastSemicolon = line.lastIndexOf(";");
if (lastSemicolon != -1 && lastSemicolon == line.length() - 1) {
return line.substring(0, lastSemicolon);
Expand Down
91 changes: 51 additions & 40 deletions lib/src/main/java/com/diffplug/spotless/java/CleanthatJavaStep.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2023 DiffPlug
* Copyright 2023-2024 DiffPlug
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -15,7 +15,6 @@
*/
package com.diffplug.spotless.java;

import java.io.IOException;
import java.io.Serializable;
import java.lang.reflect.Constructor;
import java.lang.reflect.Method;
Expand All @@ -27,30 +26,51 @@
import com.diffplug.spotless.JarState;
import com.diffplug.spotless.Jvm;
import com.diffplug.spotless.Provisioner;
import com.diffplug.spotless.RoundedStep;

/**
* Enables CleanThat as a SpotLess step.
*
* @author Benoit Lacelle
*/
// https://github.com/diffplug/spotless/blob/main/CONTRIBUTING.md#how-to-add-a-new-formatterstep
public final class CleanthatJavaStep {

public final class CleanthatJavaStep implements RoundedStep {
private static final long serialVersionUID = 1L;
private static final String NAME = "cleanthat";
private static final String MAVEN_COORDINATE = "io.github.solven-eu.cleanthat:java";

// CleanThat changelog is available at https://github.com/solven-eu/cleanthat/blob/master/CHANGES.MD
/**
* CleanThat changelog is available at <a href="https://github.com/solven-eu/cleanthat/blob/master/CHANGES.MD">here</a>.
*/
private static final Jvm.Support<String> JVM_SUPPORT = Jvm.<String> support(NAME).add(11, "2.16");

// prevent direct instantiation
private CleanthatJavaStep() {}
private final JarState.Promised jarState;
private final String version;
private final String sourceJdkVersion;
private final List<String> included;
private final List<String> excluded;
private final boolean includeDraft;

private CleanthatJavaStep(JarState.Promised jarState,
String version,
String sourceJdkVersion,
List<String> included,
List<String> excluded,
boolean includeDraft) {
this.jarState = jarState;
this.version = version;

this.sourceJdkVersion = sourceJdkVersion;
this.included = included;
this.excluded = excluded;
this.includeDraft = includeDraft;
}

/** Creates a step which apply default CleanThat mutators. */
/** Creates a step that applies default CleanThat mutators. */
public static FormatterStep create(Provisioner provisioner) {
return create(defaultVersion(), provisioner);
}

/** Creates a step which apply default CleanThat mutators. */
/** Creates a step that applies default CleanThat mutators. */
public static FormatterStep create(String version, Provisioner provisioner) {
return create(MAVEN_COORDINATE, version, defaultSourceJdk(), defaultMutators(), defaultExcludedMutators(), defaultIncludeDraft(), provisioner);
}
Expand All @@ -64,7 +84,6 @@ public static String defaultSourceJdk() {

/**
* By default, we include only safe and consensual mutators
* @return
*/
public static List<String> defaultMutators() {
// see ICleanthatStepParametersProperties.SAFE_AND_CONSENSUAL
Expand All @@ -79,7 +98,7 @@ public static boolean defaultIncludeDraft() {
return false;
}

/** Creates a step which apply selected CleanThat mutators. */
/** Creates a step that applies selected CleanThat mutators. */
public static FormatterStep create(String groupArtifact,
String version,
String sourceJdkVersion,
Expand All @@ -93,57 +112,51 @@ public static FormatterStep create(String groupArtifact,
}
Objects.requireNonNull(version, "version");
Objects.requireNonNull(provisioner, "provisioner");
return FormatterStep.createLazy(NAME,
() -> new JavaRefactorerState(NAME, groupArtifact, version, sourceJdkVersion, included, excluded, includeDraft, provisioner),
JavaRefactorerState::createFormat);
return FormatterStep.create(NAME,
new CleanthatJavaStep(JarState.promise(() -> JarState.from(groupArtifact + ":" + version, provisioner)), version, sourceJdkVersion, included, excluded, includeDraft),
CleanthatJavaStep::equalityState,
State::createFormat);
}

/** Get default formatter version */
public static String defaultVersion() {
return JVM_SUPPORT.getRecommendedFormatterVersion();
return Objects.requireNonNull(JVM_SUPPORT.getRecommendedFormatterVersion());
}

public static String defaultGroupArtifact() {
return MAVEN_COORDINATE;
}

static final class JavaRefactorerState implements Serializable {
private static final long serialVersionUID = 1L;

final JarState jarState;
final String stepName;
final String version;
private State equalityState() {
return new State(jarState.get(), version, sourceJdkVersion, included, excluded, includeDraft);
}

final String sourceJdkVersion;
final List<String> included;
final List<String> excluded;
final boolean includeDraft;
private static final class State implements Serializable {
private static final long serialVersionUID = 1L;

JavaRefactorerState(String stepName, String version, Provisioner provisioner) throws IOException {
this(stepName, MAVEN_COORDINATE, version, defaultSourceJdk(), defaultMutators(), defaultExcludedMutators(), defaultIncludeDraft(), provisioner);
}
private final JarState jarState;
private final String version;
private final String sourceJdkVersion;
private final List<String> included;
private final List<String> excluded;
private final boolean includeDraft;

JavaRefactorerState(String stepName,
String groupArtifact,
State(JarState jarState,
String version,
String sourceJdkVersion,
List<String> included,
List<String> excluded,
boolean includeDraft,
Provisioner provisioner) throws IOException {
boolean includeDraft) {
JVM_SUPPORT.assertFormatterSupported(version);
ModuleHelper.doOpenInternalPackagesIfRequired();
this.jarState = JarState.from(groupArtifact + ":" + version, provisioner);
this.stepName = stepName;
this.jarState = jarState;
this.version = version;

this.sourceJdkVersion = sourceJdkVersion;
this.included = included;
this.excluded = excluded;
this.includeDraft = includeDraft;
}

@SuppressWarnings("PMD.UseProperClassLoader")
FormatterFunc createFormat() {
ClassLoader classLoader = jarState.getClassLoader();

Expand All @@ -159,9 +172,7 @@ FormatterFunc createFormat() {
throw new IllegalStateException("Issue executing the formatter", e);
}

return JVM_SUPPORT.suggestLaterVersionOnError(version, input -> {
return (String) formatterMethod.invoke(formatter, input);
});
return JVM_SUPPORT.suggestLaterVersionOnError(version, input -> (String) formatterMethod.invoke(formatter, input));
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2022-2023 DiffPlug
* Copyright 2022-2024 DiffPlug
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -26,6 +26,8 @@

import com.diffplug.spotless.FormatterFunc;
import com.diffplug.spotless.FormatterStep;
import com.diffplug.spotless.RoundedStep;
import com.diffplug.spotless.SerializedFunction;

/**
* Some formatters put every annotation on its own line
Expand All @@ -36,7 +38,8 @@
* <p>
* Note: A type annotation is an annotation that is meta-annotated with {@code @Target({ElementType.TYPE_USE})}.
*/
public final class FormatAnnotationsStep {
public final class FormatAnnotationsStep implements RoundedStep {
private static final long serialVersionUID = 1L;

/**
* Simple names of type annotations.
Expand Down Expand Up @@ -389,14 +392,14 @@ public final class FormatAnnotationsStep {

);

static final String NAME = "No line break between type annotation and type";
private static final String NAME = "No line break between type annotation and type";

public static FormatterStep create() {
return create(Collections.emptyList(), Collections.emptyList());
}

public static FormatterStep create(List<String> addedTypeAnnotations, List<String> removedTypeAnnotations) {
return FormatterStep.create(NAME, new State(addedTypeAnnotations, removedTypeAnnotations), State::toFormatter);
return FormatterStep.create(NAME, new State(addedTypeAnnotations, removedTypeAnnotations), SerializedFunction.identity(), State::toFormatter);
}

private FormatAnnotationsStep() {}
Expand Down Expand Up @@ -432,13 +435,13 @@ private static final class State implements Serializable {
}

FormatterFunc toFormatter() {
return unixStr -> fixupTypeAnnotations(unixStr);
return this::fixupTypeAnnotations;
}

/**
* Removes line break between type annotations and the following type.
*
* @param the text of a Java file
* @param unixStr the text of a Java file
* @return corrected text of the Java file
*/
String fixupTypeAnnotations(String unixStr) {
Expand Down
Loading
Loading