Add documentation, tests, and hit a wall as mutators are not detected

Author: blacelle

README.md

@@ -84,6 +84,7 @@ lib('java.PalantirJavaFormatStep')               +'{{yes}}       | {{yes}}
 lib('java.RemoveUnusedImportsStep')              +'{{yes}}       | {{yes}}      | {{yes}}      | {{no}}  |',
 extra('java.EclipseJdtFormatterStep')            +'{{yes}}       | {{yes}}      | {{yes}}      | {{no}}  |',
 lib('java.FormatAnnotationsStep')                +'{{yes}}       | {{yes}}      | {{no}}       | {{no}}  |',
+lib('java.CleanthatJavaStep')                    +'{{no}}       | {{yes}}      | {{no}}       | {{no}}  |',
 lib('json.gson.GsonStep')                        +'{{yes}}       | {{yes}}      | {{no}}       | {{no}}  |',
 lib('json.JacksonJsonStep')                      +'{{yes}}       | {{yes}}      | {{no}}       | {{no}}  |',
 lib('json.JsonSimpleStep')                       +'{{yes}}       | {{yes}}      | {{no}}       | {{no}}  |',
@@ -131,6 +132,7 @@ lib('yaml.JacksonYamlStep')                      +'{{yes}}       | {{yes}}
 | [`java.RemoveUnusedImportsStep`](lib/src/main/java/com/diffplug/spotless/java/RemoveUnusedImportsStep.java) | :+1:       | :+1:      | :+1:      | :white_large_square:  |
 | [`java.EclipseJdtFormatterStep`](lib-extra/src/main/java/com/diffplug/spotless/extra/java/EclipseJdtFormatterStep.java) | :+1:       | :+1:      | :+1:      | :white_large_square:  |
 | [`java.FormatAnnotationsStep`](lib/src/main/java/com/diffplug/spotless/java/FormatAnnotationsStep.java) | :+1:       | :+1:      | :white_large_square:       | :white_large_square:  |
+| [`java.CleanthatJavaStep`](lib/src/main/java/com/diffplug/spotless/java/CleanthatJavaStep.java) | :white_large_square:       | :+1:      | :white_large_square:       | :white_large_square:  |
 | [`json.gson.GsonStep`](lib/src/main/java/com/diffplug/spotless/json/gson/GsonStep.java) | :+1:       | :+1:      | :white_large_square:       | :white_large_square:  |
 | [`json.JacksonJsonStep`](lib/src/main/java/com/diffplug/spotless/json/JacksonJsonStep.java) | :+1:       | :+1:      | :white_large_square:       | :white_large_square:  |
 | [`json.JsonSimpleStep`](lib/src/main/java/com/diffplug/spotless/json/JsonSimpleStep.java) | :+1:       | :+1:      | :white_large_square:       | :white_large_square:  |

lib/src/cleanthat/java/com/diffplug/spotless/glue/java/JavaCleanthatRefactorerFunc.java

@@ -19,6 +19,9 @@
 import java.util.Collections;
 import java.util.List;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import com.diffplug.spotless.FormatterFunc;
 
 import eu.solven.cleanthat.config.pojo.CleanthatEngineProperties;
@@ -28,6 +31,8 @@
 import eu.solven.cleanthat.formatter.LineEnding;
 
 public class JavaCleanthatRefactorerFunc implements FormatterFunc {
+	private static final Logger LOGGER = LoggerFactory.getLogger(JavaCleanthatRefactorerFunc.class);
+
 	private String jdkVersion;
 	private List<String> included;
 	private List<String> excluded;
@@ -53,6 +58,9 @@ public String apply(String input) throws Exception {
 
 		JavaRefactorer refactorer = new JavaRefactorer(engineProperties, refactorerProperties);
 
+		LOGGER.debug("Processing sourceJdk={} included={} excluded={}", jdkVersion, included, excluded);
+		LOGGER.debug("Available mutators: {}", JavaRefactorer.getAllIncluded());
+
 		// Spotless calls steps always with LF eol.
 		return refactorer.doFormat(input, LineEnding.LF);
 	}

lib/src/main/java/com/diffplug/spotless/java/CleanthatStepFactory.java renamed to lib/src/main/java/com/diffplug/spotless/java/CleanthatJavaStep.java

@@ -34,15 +34,15 @@
  * @author Benoit Lacelle
  */
 // https://github.com/diffplug/spotless/blob/main/CONTRIBUTING.md#how-to-add-a-new-formatterstep
-public final class CleanthatStepFactory {
+public final class CleanthatJavaStep {
 
 	private static final String NAME = "cleanthat";
-	private static final String MAVEN_COORDINATE = "io.github.solven-eu.cleanthat:java:";
+	private static final String MAVEN_COORDINATE = "io.github.solven-eu.cleanthat:java";
 
 	private static final Jvm.Support<String> JVM_SUPPORT = Jvm.<String> support(NAME).add(11, "2.0");
 
 	// prevent direct instantiation
-	private CleanthatStepFactory() {}
+	private CleanthatJavaStep() {}
 
 	/** Creates a step which apply default CleanThat mutators. */
 	public static FormatterStep create(Provisioner provisioner) {
@@ -83,7 +83,7 @@ public static FormatterStep create(String groupArtifact,
 			Provisioner provisioner) {
 		Objects.requireNonNull(groupArtifact, "groupArtifact");
 		if (groupArtifact.chars().filter(ch -> ch == ':').count() != 1) {
-			throw new IllegalArgumentException("groupArtifact must be in the form 'groupId:artifactId'");
+			throw new IllegalArgumentException("groupArtifact must be in the form 'groupId:artifactId'. it was: " + groupArtifact);
 		}
 		Objects.requireNonNull(version, "version");
 		Objects.requireNonNull(provisioner, "provisioner");

plugin-maven/README.md

@@ -39,7 +39,7 @@ user@machine repo % mvn spotless:check
   - [Requirements](#requirements)
   - [Binding to maven phase](#binding-to-maven-phase)
 - **Languages**
-  - [Java](#java) ([google-java-format](#google-java-format), [eclipse jdt](#eclipse-jdt), [prettier](#prettier), [palantir-java-format](#palantir-java-format), [formatAnnotations](#formatAnnotations))
+  - [Java](#java) ([google-java-format](#google-java-format), [eclipse jdt](#eclipse-jdt), [prettier](#prettier), [palantir-java-format](#palantir-java-format), [formatAnnotations](#formatAnnotations), [cleanthat](#cleanthat))
   - [Groovy](#groovy) ([eclipse groovy](#eclipse-groovy))
   - [Kotlin](#kotlin) ([ktfmt](#ktfmt), [ktlint](#ktlint), [diktat](#diktat), [prettier](#prettier))
   - [Scala](#scala) ([scalafmt](#scalafmt))
@@ -182,6 +182,8 @@ any other maven phase (i.e. compile) then it can be configured as below;
       <include>src/test/java/**/*.java</include>
     </includes>
 
+    <cleanthat />        <!-- has its own section below -->
+
     <googleJavaFormat /> <!-- has its own section below -->
     <eclipse />          <!-- has its own section below -->
     <prettier />         <!-- has its own section below -->
@@ -274,6 +276,25 @@ list of well-known type annotations.  You can make a pull request to add new one
 In the future there will be mechanisms to add/remove annotations from the list.
 These mechanisms already exist for the Gradle plugin.
 
+### Cleanthat
+
+[homepage](https://github.com/solven-eu/cleanthat). CleanThat enables automatic refactoring of Java code
+
+```xml
+<cleanthat>
+  <version>2.0</version>                          <!-- optional version of Cleanthat -->
+  <sourceJdk>${maven.compiler.source}</sourceJdk> <!-- optional. Default to ${maven.compiler.source} else '1.7' -->
+  <mutators>
+    <mutator>*</mutator> <!-- optional. Default to '*' to include all mutators -->
+  </mutators>
+  <mutators> <!-- List of mutators: https://github.com/solven-eu/cleanthat/tree/master/java/src/main/java/eu/solven/cleanthat/engine/java/refactorer/mutators -->
+    <mutator>LiteralsFirstInComparisons</mutator> <!-- You may alternatively list the requested mutators -->
+  </mutators>
+  <excludedMutators>
+    <excludedMutator>OptionalNotEmpty</excludedMutator> <!-- You can discard specific rules -->
+  </excludedMutators>
+</cleanthat>
+```
 
 ## Groovy
 

plugin-maven/src/main/java/com/diffplug/spotless/maven/java/CleanthatJava.java

@@ -20,7 +20,7 @@
 import org.apache.maven.plugins.annotations.Parameter;
 
 import com.diffplug.spotless.FormatterStep;
-import com.diffplug.spotless.java.CleanthatStepFactory;
+import com.diffplug.spotless.java.CleanthatJavaStep;
 import com.diffplug.spotless.maven.FormatterStepConfig;
 import com.diffplug.spotless.maven.FormatterStepFactory;
 
@@ -33,19 +33,19 @@ public class CleanthatJava implements FormatterStepFactory {
 
 	// https://maven.apache.org/plugins/maven-compiler-plugin/compile-mojo.html#source
 	@Parameter(property = "maven.compiler.source")
-	private String sourceJdk = CleanthatStepFactory.defaultJdkVersion();
+	private String sourceJdk = CleanthatJavaStep.defaultJdkVersion();
 
 	@Parameter
-	private List<String> mutators = CleanthatStepFactory.defaultMutators();
+	private List<String> mutators = CleanthatJavaStep.defaultMutators();
 
 	@Parameter
-	private List<String> excludedMutators = CleanthatStepFactory.defaultExcludedMutators();
+	private List<String> excludedMutators = CleanthatJavaStep.defaultExcludedMutators();
 
 	@Override
 	public FormatterStep newFormatterStep(FormatterStepConfig config) {
-		String groupArtifact = this.groupArtifact != null ? this.groupArtifact : CleanthatStepFactory.defaultGroupArtifact();
-		String version = this.version != null ? this.version : CleanthatStepFactory.defaultVersion();
+		String groupArtifact = this.groupArtifact != null ? this.groupArtifact : CleanthatJavaStep.defaultGroupArtifact();
+		String version = this.version != null ? this.version : CleanthatJavaStep.defaultVersion();
 
-		return CleanthatStepFactory.create(groupArtifact, version, sourceJdk, mutators, excludedMutators, config.getProvisioner());
+		return CleanthatJavaStep.create(groupArtifact, version, sourceJdk, mutators, excludedMutators, config.getProvisioner());
 	}
 }

plugin-maven/src/main/java/com/diffplug/spotless/maven/java/Java.java

@@ -79,6 +79,10 @@ public void addFormatAnnotations(FormatAnnotations formatAnnotations) {
 		addStepFactory(formatAnnotations);
 	}
 
+	public void addCleanthat(CleanthatJava cleanthat) {
+		addStepFactory(cleanthat);
+	}
+
 	private static String fileMask(Path path) {
 		String dir = path.toString();
 		if (!dir.endsWith(File.separator)) {

plugin-maven/src/test/java/com/diffplug/spotless/maven/java/CleanthatJavaRefactorerTest.java

@@ -0,0 +1,76 @@
+/*
+ * Copyright 2022-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.maven.java;
+
+import org.assertj.core.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.diffplug.spotless.maven.MavenIntegrationHarness;
+
+class CleanthatJavaRefactorerTest extends MavenIntegrationHarness {
+	private static final Logger LOGGER = LoggerFactory.getLogger(CleanthatJavaRefactorerTest.class);
+
+	@Test
+	void testLiteralsFirstInComparisons() throws Exception {
+		writePomWithJavaSteps(
+				"<cleanthat>",
+				"</cleanthat>");
+
+		runTest("LiteralsFirstInComparisons.dirty.java", "LiteralsFirstInComparisons.clean.java");
+	}
+
+	@Test
+	void testMultipleMutators() throws Exception {
+		writePomWithJavaSteps(
+				"<cleanthat>",
+				"</cleanthat>");
+
+		runTest("MultipleMutators.dirty.java", "MultipleMutators.clean.java");
+	}
+
+	@Test
+	void testExcludeOptionalNotEmpty() throws Exception {
+		writePomWithJavaSteps(
+				"<cleanthat>",
+				"  <excludedMutators>",
+				"    <excludedMutator>OptionalNotEmpty</excludedMutator>",
+				"  </excludedMutators>",
+				"</cleanthat>");
+
+		runTest("MultipleMutators.dirty.java", "MultipleMutators.clean.onlyLiteralsFirst.java");
+	}
+
+	@Test
+	void testIncludeOnlyLiteralsFirstInComparisons() throws Exception {
+		writePomWithJavaSteps(
+				"<cleanthat>",
+				"  <mutators>",
+				"    <mutator>LiteralsFirstInComparisons</mutator>",
+				"  </mutators>",
+				"</cleanthat>");
+
+		runTest("MultipleMutators.dirty.java", "MultipleMutators.clean.onlyLiteralsFirst.java");
+	}
+
+	private void runTest(String dirtyPath, String cleanPath) throws Exception {
+		String path = "src/main/java/test.java";
+		setFile(path).toResource("java/cleanthat/" + dirtyPath);
+		Assertions.assertThat(mavenRunner().withArguments("spotless:apply -X").runNoError().stdOutUtf8()).isEmpty();
+		assertFile(path).sameAsResource("java/cleanthat/" + cleanPath);
+	}
+}

testlib/src/main/resources/java/cleanthat/LiteralsFirstInComparisons.clean.java

@@ -0,0 +1,8 @@
+package eu.solven.cleanthat.engine.java.refactorer.cases.do_not_format_me;
+
+public class LiteralsFirstInComparisonsCases {
+
+	public boolean isHardcoded(String input) {
+		return "hardcoded".equals(input);
+	}
+}

testlib/src/main/resources/java/cleanthat/LiteralsFirstInComparisons.dirty.java

@@ -0,0 +1,8 @@
+package eu.solven.cleanthat.engine.java.refactorer.cases.do_not_format_me;
+
+public class LiteralsFirstInComparisonsCases {
+
+	public boolean isHardcoded(String input) {
+		return input.equals("hardcoded");
+	}
+}

testlib/src/main/resources/java/cleanthat/MultipleMutators.clean.java

@@ -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();
+	}
+}