Reproduce bugs related to equo formatters (#1660)

lib-extra/src/groovy/java/com/diffplug/spotless/extra/glue/groovy/GrEclipseFormatterStepImpl.java

@@ -104,14 +104,14 @@ public String format(String raw) throws Exception {
 	 * Eclipse Groovy formatter does not signal problems by its return value, but by logging errors.
 	 */
 	private static final class GroovyErrorListener implements ILogListener, IGroovyLogger {
-		private final List<String> errors;
+		private final List<Throwable> errors;
 
 		public GroovyErrorListener() {
 			/*
 			 * We need a synchronized list here, in case multiple instantiations
 			 * run in parallel.
 			 */
-			errors = Collections.synchronizedList(new ArrayList<String>());
+			errors = Collections.synchronizedList(new ArrayList<>());
 			ILog groovyLogger = GroovyCoreActivator.getDefault().getLog();
 			groovyLogger.addLogListener(this);
 			synchronized (GroovyLogManager.manager) {
@@ -121,7 +121,7 @@ public GroovyErrorListener() {
 
 		@Override
 		public void logging(final IStatus status, final String plugin) {
-			errors.add(status.getMessage());
+			errors.add(status.getException());
 		}
 
 		public boolean errorsDetected() {
@@ -141,9 +141,10 @@ public String toString() {
 			} else if (0 == errors.size()) {
 				string.append("Step sucesfully executed.");
 			}
-			for (String error : errors) {
+			for (Throwable error : errors) {
+				error.printStackTrace();
 				string.append(System.lineSeparator());
-				string.append(error);
+				string.append(error.getMessage());
 			}
 
 			return string.toString();
@@ -162,7 +163,11 @@ public boolean isCategoryEnabled(TraceCategory cat) {
 
 		@Override
 		public void log(TraceCategory arg0, String arg1) {
-			errors.add(arg1);
+			try {
+				throw new RuntimeException(arg1);
+			} catch (RuntimeException e) {
+				errors.add(e);
+			}
 		}
 	}
 

lib-extra/src/main/java/com/diffplug/spotless/extra/EquoBasedStepBuilder.java

@@ -22,6 +22,8 @@
 import java.util.Map;
 import java.util.Properties;
 
+import javax.annotation.Nullable;
+
 import com.diffplug.spotless.FileSignature;
 import com.diffplug.spotless.FormatterFunc;
 import com.diffplug.spotless.FormatterProperties;
@@ -46,10 +48,17 @@ public abstract class EquoBasedStepBuilder {
 	private Iterable<File> settingsFiles = new ArrayList<>();
 	private Map<String, String> p2Mirrors = Map.of();
 
-	/** Initialize valid default configuration, taking latest version */
+	/** @deprecated if you use this constructor you *must* call {@link #setVersion(String)} before calling {@link #build()} */
+	@Deprecated
 	public EquoBasedStepBuilder(String formatterName, Provisioner mavenProvisioner, ThrowingEx.Function<State, FormatterFunc> stateToFormatter) {
+		this(formatterName, mavenProvisioner, null, stateToFormatter);
+	}
+
+	/** Initialize valid default configuration, taking latest version */
+	public EquoBasedStepBuilder(String formatterName, Provisioner mavenProvisioner, @Nullable String defaultVersion, ThrowingEx.Function<State, FormatterFunc> stateToFormatter) {
 		this.formatterName = formatterName;
 		this.mavenProvisioner = mavenProvisioner;
+		this.formatterVersion = defaultVersion;
 		this.stateToFormatter = stateToFormatter;
 	}
 

lib-extra/src/main/java/com/diffplug/spotless/extra/cpp/EclipseCdtFormatterStep.java

@@ -45,7 +45,7 @@ public static String defaultVersion() {
 
 	/** Provides default configuration */
 	public static EquoBasedStepBuilder createBuilder(Provisioner provisioner) {
-		return new EquoBasedStepBuilder(NAME, provisioner, EclipseCdtFormatterStep::apply) {
+		return new EquoBasedStepBuilder(NAME, provisioner, defaultVersion(), EclipseCdtFormatterStep::apply) {
 			@Override
 			protected P2Model model(String version) {
 				var model = new P2Model();

lib-extra/src/main/java/com/diffplug/spotless/extra/groovy/GrEclipseFormatterStep.java

@@ -39,7 +39,7 @@ public static String defaultVersion() {
 	}
 
 	public static EquoBasedStepBuilder createBuilder(Provisioner provisioner) {
-		return new EquoBasedStepBuilder(NAME, provisioner, GrEclipseFormatterStep::apply) {
+		return new EquoBasedStepBuilder(NAME, provisioner, defaultVersion(), GrEclipseFormatterStep::apply) {
 			@Override
 			protected P2Model model(String version) {
 				if (!version.startsWith("4.")) {

lib-extra/src/main/java/com/diffplug/spotless/extra/java/EclipseJdtFormatterStep.java

@@ -38,7 +38,7 @@ public static String defaultVersion() {
 	}
 
 	public static EquoBasedStepBuilder createBuilder(Provisioner provisioner) {
-		return new EquoBasedStepBuilder(NAME, provisioner, EclipseJdtFormatterStep::apply) {
+		return new EquoBasedStepBuilder(NAME, provisioner, defaultVersion(), EclipseJdtFormatterStep::apply) {
 			@Override
 			protected P2Model model(String version) {
 				var model = new P2Model();

lib-extra/src/test/java/com/diffplug/spotless/extra/groovy/GrEclipseFormatterStepSpecialCaseTest.java

@@ -0,0 +1,44 @@
+/*
+ * 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.extra.groovy;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+import com.diffplug.spotless.StepHarness;
+import com.diffplug.spotless.TestProvisioner;
+
+public class GrEclipseFormatterStepSpecialCaseTest {
+	/**
+	 * https://github.com/diffplug/spotless/issues/1657
+	 *
+	 * broken: ${parm == null ? "" : "<tag>$parm</tag>"}
+	 *  works: ${parm == null ? "" : "<tag>parm</tag>"}
+	 */
+	@Test
+	public void issue_1657() {
+		Assertions.assertThrows(IllegalArgumentException.class, () -> {
+			StepHarness.forStep(GrEclipseFormatterStep.createBuilder(TestProvisioner.mavenCentral()).build())
+					.testResourceUnaffected("groovy/greclipse/format/SomeClass.test");
+		});
+	}
+
+	@Test
+	public void issue_1657_fixed() {
+		StepHarness.forStep(GrEclipseFormatterStep.createBuilder(TestProvisioner.mavenCentral()).build())
+				.testResourceUnaffected("groovy/greclipse/format/SomeClass.fixed");
+	}
+}

lib-extra/src/test/java/com/diffplug/spotless/extra/java/EclipseJdtFormatterStepSpecialCaseTest.java

@@ -0,0 +1,30 @@
+/*
+ * Copyright 2016-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.extra.java;
+
+import org.junit.jupiter.api.Test;
+
+import com.diffplug.spotless.StepHarness;
+import com.diffplug.spotless.TestProvisioner;
+
+public class EclipseJdtFormatterStepSpecialCaseTest {
+	/** https://github.com/diffplug/spotless/issues/1638 */
+	@Test
+	public void issue_1638() {
+		StepHarness.forStep(EclipseJdtFormatterStep.createBuilder(TestProvisioner.mavenCentral()).build())
+				.testResource("java/eclipse/AbstractType.test", "java/eclipse/AbstractType.clean");
+	}
+}

testlib/src/main/resources/groovy/greclipse/format/SomeClass.fixed

@@ -0,0 +1,12 @@
+package com.somepackage
+
+class SomeClass {
+
+	def func(parm) {
+		"""<tag>
+            ${parm == null ? "" : "<tag>parm</tag>"}
+            ${parm == null ? "" : "<tag>parm</tag>"}
+           </tag>
+        """
+	}
+}

testlib/src/main/resources/groovy/greclipse/format/SomeClass.test

@@ -0,0 +1,12 @@
+package com.somepackage
+
+class SomeClass {
+
+    def func(parm) {
+        """<tag>
+            ${parm == null ? "" : "<tag>$parm</tag>"}
+            ${parm == null ? "" : "<tag>$parm</tag>"}
+           </tag>
+        """
+    }
+}

testlib/src/main/resources/java/eclipse/AbstractType.clean

@@ -0,0 +1,38 @@
+package test;
+
+public abstract class AbstractType {
+
+	private String _typeName;
+
+	AbstractType(String typeName) {
+		_typeName = typeName;
+	}
+
+	private String _type() {
+		String name = getClass().getSimpleName();
+		return name.endsWith("Type") ? name.substring(0, getClass().getSimpleName().length() - 4) : name;
+	}
+
+	AbstractType argument() {
+		throw new UnsupportedOperationException(getClass().getSimpleName());
+	}
+
+	@Override
+	public boolean equals(Object another) {
+		if (this == another) {
+			return true;
+		}
+		return another instanceof AbstractType t && _typeName.equals(t._typeName);
+	}
+
+	@Override
+	public int hashCode() {
+		return _typeName.hashCode();
+	}
+
+	@Override
+	public String toString() {
+		return getClass().getSimpleName() + "(typeName)";
+	}
+
+}

testlib/src/main/resources/java/eclipse/AbstractType.test

@@ -0,0 +1,54 @@
+package test;
+
+
+
+
+public abstract class AbstractType {
+
+
+    private String _typeName;
+
+    AbstractType(String typeName) {
+        _typeName = typeName;
+    }
+
+
+
+    private String _type() {
+        String name = getClass().getSimpleName();
+        return name.endsWith("Type")
+                ? name.substring(0, getClass().getSimpleName().length() - 4)
+                : name;
+    }
+
+    AbstractType argument() {
+        throw new UnsupportedOperationException(getClass().getSimpleName());
+    }
+
+
+    @Override
+    public boolean equals(Object another) {
+        if (this == another) {
+            return true;
+        }
+        return another instanceof AbstractType t
+                && _typeName.equals(t._typeName);
+    }
+
+
+
+    @Override
+    public int hashCode() {
+        return _typeName.hashCode();
+    }
+
+
+
+
+    @Override
+    public String toString() {
+        return getClass().getSimpleName() + "(typeName)";
+    }
+
+
+}