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

Upgrade Cleanthat. Enable draft #1574

Merged
merged 8 commits into from
Feb 23, 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
4 changes: 2 additions & 2 deletions lib/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ dependencies {

gsonCompileOnly 'com.google.code.gson:gson:2.10.1'

cleanthatCompileOnly 'io.github.solven-eu.cleanthat:java:2.2'
compatCleanthat2Dot1CompileAndTestOnly 'io.github.solven-eu.cleanthat:java:2.2'
cleanthatCompileOnly 'io.github.solven-eu.cleanthat:java:2.6'
compatCleanthat2Dot1CompileAndTestOnly 'io.github.solven-eu.cleanthat:java:2.6'
}

// we'll hold the core lib to a high standard
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,17 @@ public class JavaCleanthatRefactorerFunc implements FormatterFunc {
private String jdkVersion;
private List<String> included;
private List<String> excluded;
private boolean includeDraft;

public JavaCleanthatRefactorerFunc(String jdkVersion, List<String> included, List<String> excluded) {
public JavaCleanthatRefactorerFunc(String jdkVersion, List<String> included, List<String> excluded, boolean includeDraft) {
this.jdkVersion = jdkVersion == null ? IJdkVersionConstants.JDK_8 : jdkVersion;
this.included = included == null ? Collections.emptyList() : included;
this.excluded = excluded == null ? Collections.emptyList() : excluded;
this.includeDraft = includeDraft;
}

public JavaCleanthatRefactorerFunc() {
this(IJdkVersionConstants.JDK_8, Arrays.asList(JavaRefactorerProperties.WILDCARD), Arrays.asList());
this(IJdkVersionConstants.JDK_8, Arrays.asList("SafeAndConsensual"), Arrays.asList(), false);
}

@Override
Expand Down Expand Up @@ -79,9 +81,11 @@ private String doApply(String input) throws InterruptedException, IOException {
refactorerProperties.setIncluded(included);
refactorerProperties.setExcluded(excluded);

refactorerProperties.setIncludeDraft(includeDraft);

JavaRefactorer refactorer = new JavaRefactorer(engineProperties, refactorerProperties);

LOGGER.debug("Processing sourceJdk={} included={} excluded={}", jdkVersion, included, excluded);
LOGGER.debug("Processing sourceJdk={} included={} excluded={}", jdkVersion, included, excluded, includeDraft);
LOGGER.debug("Available mutators: {}", JavaRefactorer.getAllIncluded());

// Spotless calls steps always with LF eol.
Expand Down
52 changes: 36 additions & 16 deletions lib/src/main/java/com/diffplug/spotless/java/CleanthatJavaStep.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public final class CleanthatJavaStep {
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
private static final Jvm.Support<String> JVM_SUPPORT = Jvm.<String> support(NAME).add(11, "2.2");
private static final Jvm.Support<String> JVM_SUPPORT = Jvm.<String> support(NAME).add(11, "2.6");

// prevent direct instantiation
private CleanthatJavaStep() {}
Expand All @@ -52,7 +52,7 @@ public static FormatterStep create(Provisioner provisioner) {

/** Creates a step which apply default CleanThat mutators. */
public static FormatterStep create(String version, Provisioner provisioner) {
return create(MAVEN_COORDINATE, version, defaultSourceJdk(), defaultExcludedMutators(), defaultMutators(), provisioner);
return create(MAVEN_COORDINATE, version, defaultSourceJdk(), defaultMutators(), defaultExcludedMutators(), defaultIncludeDraft(), provisioner);
}

public static String defaultSourceJdk() {
Expand All @@ -62,16 +62,21 @@ public static String defaultSourceJdk() {
return "1.7";
}

public static List<String> defaultExcludedMutators() {
return List.of();
}

/**
* By default, we include all available rules
* By default, we include only safe and consensual mutators
* @return
*/
public static List<String> defaultMutators() {
return List.of("eu.solven.cleanthat.engine.java.refactorer.mutators.composite.SafeAndConsensualMutators");
// see ICleanthatStepParametersProperties.SAFE_AND_CONSENSUAL
return List.of("SafeAndConsensual");
}

public static List<String> defaultExcludedMutators() {
return List.of();
}

public static boolean defaultIncludeDraft() {
return false;
}

/** Creates a step which apply selected CleanThat mutators. */
Expand All @@ -80,6 +85,7 @@ public static FormatterStep create(String groupArtifact,
String sourceJdkVersion,
List<String> excluded,
List<String> included,
boolean includeDraft,
Provisioner provisioner) {
Objects.requireNonNull(groupArtifact, "groupArtifact");
if (groupArtifact.chars().filter(ch -> ch == ':').count() != 1) {
Expand All @@ -88,7 +94,7 @@ public static FormatterStep create(String groupArtifact,
Objects.requireNonNull(version, "version");
Objects.requireNonNull(provisioner, "provisioner");
return FormatterStep.createLazy(NAME,
() -> new JavaRefactorerState(NAME, groupArtifact, version, sourceJdkVersion, excluded, included, provisioner),
() -> new JavaRefactorerState(NAME, groupArtifact, version, sourceJdkVersion, excluded, included, includeDraft, provisioner),
JavaRefactorerState::createFormat);
}

Expand All @@ -111,9 +117,10 @@ static final class JavaRefactorerState implements Serializable {
final String sourceJdkVersion;
final List<String> included;
final List<String> excluded;
final boolean includeDraft;

JavaRefactorerState(String stepName, String version, Provisioner provisioner) throws IOException {
this(stepName, MAVEN_COORDINATE, version, defaultSourceJdk(), defaultExcludedMutators(), defaultMutators(), provisioner);
this(stepName, MAVEN_COORDINATE, version, defaultSourceJdk(), defaultExcludedMutators(), defaultMutators(), defaultIncludeDraft(), provisioner);
}

JavaRefactorerState(String stepName,
Expand All @@ -122,8 +129,12 @@ static final class JavaRefactorerState implements Serializable {
String sourceJdkVersion,
List<String> included,
List<String> excluded,
boolean includeDraft,
Provisioner provisioner) throws IOException {
JVM_SUPPORT.assertFormatterSupported(version);
// https://github.com/diffplug/spotless/issues/1583
if (!version.endsWith("-SNAPSHOT")) {
JVM_SUPPORT.assertFormatterSupported(version);
}
ModuleHelper.doOpenInternalPackagesIfRequired();
this.jarState = JarState.from(groupArtifact + ":" + version, provisioner);
this.stepName = stepName;
Expand All @@ -132,6 +143,7 @@ static final class JavaRefactorerState implements Serializable {
this.sourceJdkVersion = sourceJdkVersion;
this.included = included;
this.excluded = excluded;
this.includeDraft = includeDraft;
}

@SuppressWarnings("PMD.UseProperClassLoader")
Expand All @@ -142,16 +154,24 @@ FormatterFunc createFormat() {
Method formatterMethod;
try {
Class<?> formatterClazz = classLoader.loadClass("com.diffplug.spotless.glue.java.JavaCleanthatRefactorerFunc");
Constructor<?> formatterConstructor = formatterClazz.getConstructor(String.class, List.class, List.class);
Constructor<?> formatterConstructor = formatterClazz.getConstructor(String.class, List.class, List.class, boolean.class);

formatter = formatterConstructor.newInstance(sourceJdkVersion, included, excluded);
formatter = formatterConstructor.newInstance(sourceJdkVersion, included, excluded, includeDraft);
formatterMethod = formatterClazz.getMethod("apply", String.class);
} catch (ReflectiveOperationException e) {
throw new IllegalStateException("Issue executing the formatter", e);
}
return JVM_SUPPORT.suggestLaterVersionOnError(version, input -> {
return (String) formatterMethod.invoke(formatter, input);
});

// https://github.com/diffplug/spotless/issues/1583
if (!version.endsWith("-SNAPSHOT")) {
return JVM_SUPPORT.suggestLaterVersionOnError(version, input -> {
return (String) formatterMethod.invoke(formatter, input);
});
} else {
return input -> {
return (String) formatterMethod.invoke(formatter, input);
};
}
}

}
Expand Down
4 changes: 4 additions & 0 deletions plugin-gradle/CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `3.27.0`).

## [Unreleased]
### Added
* Add `includeDraft` option, to include draft mutators from composite mutators ([#1574](https://github.com/diffplug/spotless/pull/1574))
### Changes
* Bump default `cleanthat` version to latest `2.2` -> `2.6` ([#1574](https://github.com/diffplug/spotless/pull/1574))
* Bump default `cleanthat` version to latest `2.1` -> `2.2` ([#1569](https://github.com/diffplug/spotless/pull/1569))

## [6.15.0] - 2023-02-10
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,8 @@ public class CleanthatJavaConfig {

private List<String> excludedMutators = new ArrayList<>(CleanthatJavaStep.defaultExcludedMutators());

private boolean includeDraft = CleanthatJavaStep.defaultIncludeDraft();

CleanthatJavaConfig() {
addStep(createStep());
}
Expand Down Expand Up @@ -341,11 +343,17 @@ public CleanthatJavaConfig excludeMutator(String mutator) {
return this;
}

public CleanthatJavaConfig includeDraft(boolean includeDraft) {
this.includeDraft = includeDraft;
replaceStep(createStep());
return this;
}

private FormatterStep createStep() {
return CleanthatJavaStep.create(
groupArtifact,
version,
sourceJdk, mutators, excludedMutators, provisioner());
sourceJdk, mutators, excludedMutators, includeDraft, provisioner());
}
}

Expand Down
3 changes: 3 additions & 0 deletions plugin-maven/CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@
We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `1.27.0`).

## [Unreleased]
### Added
* Add `includeDraft` option, to include draft mutators from composite mutators ([#XXX](https://github.com/diffplug/spotless/pull/XXX))
### Changes
* Bump default `cleanthat` version to latest `2.2` -> `2.6` ([#1574](https://github.com/diffplug/spotless/pull/1574))
* Bump default `cleanthat` version to latest `2.1` -> `2.2` ([#1569](https://github.com/diffplug/spotless/pull/1569))

## [2.33.0] - 2023-02-10
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,14 @@ public class CleanthatJava implements FormatterStepFactory {
@Parameter
private List<String> excludedMutators = CleanthatJavaStep.defaultExcludedMutators();

@Parameter
private boolean includeDraft = CleanthatJavaStep.defaultIncludeDraft();

@Override
public FormatterStep newFormatterStep(FormatterStepConfig config) {
String groupArtifact = this.groupArtifact != null ? this.groupArtifact : CleanthatJavaStep.defaultGroupArtifact();
String version = this.version != null ? this.version : CleanthatJavaStep.defaultVersion();

return CleanthatJavaStep.create(groupArtifact, version, sourceJdk, mutators, excludedMutators, config.getProvisioner());
return CleanthatJavaStep.create(groupArtifact, version, sourceJdk, mutators, excludedMutators, includeDraft, config.getProvisioner());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,17 @@
class CleanthatJavaRefactorerTest extends MavenIntegrationHarness {
private static final Logger LOGGER = LoggerFactory.getLogger(CleanthatJavaRefactorerTest.class);

@Test
void testEnableDraft() throws Exception {
writePomWithJavaSteps(
"<cleanthat>",
" <sourceJdk>11</sourceJdk>",
" <includeDraft>true</includeDraft>",
"</cleanthat>");

runTest("MultipleMutators.dirty.java", "MultipleMutators.clean.onlyOptionalIsPresent.java");
}

@Test
void testLiteralsFirstInComparisons() throws Exception {
writePomWithJavaSteps(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package eu.solven.cleanthat.engine.java.refactorer.cases.do_not_format_me;

import java.util.Optional;

public class LiteralsFirstInComparisonsCases {

public boolean isHardcoded(String input) {
return input.equals("hardcoded");
}

public boolean isPresent(Optional<?> optional) {
return optional.isPresent();
}
}