diff --git a/lib-extra/src/groovy/java/com/diffplug/spotless/extra/glue/groovy/GrEclipseFormatterStepImpl.java b/lib-extra/src/groovy/java/com/diffplug/spotless/extra/glue/groovy/GrEclipseFormatterStepImpl.java index 9549847d74..7c29209157 100644 --- a/lib-extra/src/groovy/java/com/diffplug/spotless/extra/glue/groovy/GrEclipseFormatterStepImpl.java +++ b/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 errors; + private final List errors; public GroovyErrorListener() { /* * We need a synchronized list here, in case multiple instantiations * run in parallel. */ - errors = Collections.synchronizedList(new ArrayList()); + 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); + } } } diff --git a/lib-extra/src/main/java/com/diffplug/spotless/extra/EquoBasedStepBuilder.java b/lib-extra/src/main/java/com/diffplug/spotless/extra/EquoBasedStepBuilder.java index cc12a938ad..db2e6674db 100644 --- a/lib-extra/src/main/java/com/diffplug/spotless/extra/EquoBasedStepBuilder.java +++ b/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 settingsFiles = new ArrayList<>(); private Map 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 stateToFormatter) { + this(formatterName, mavenProvisioner, null, stateToFormatter); + } + + /** Initialize valid default configuration, taking latest version */ + public EquoBasedStepBuilder(String formatterName, Provisioner mavenProvisioner, @Nullable String defaultVersion, ThrowingEx.Function stateToFormatter) { this.formatterName = formatterName; this.mavenProvisioner = mavenProvisioner; + this.formatterVersion = defaultVersion; this.stateToFormatter = stateToFormatter; } diff --git a/lib-extra/src/main/java/com/diffplug/spotless/extra/cpp/EclipseCdtFormatterStep.java b/lib-extra/src/main/java/com/diffplug/spotless/extra/cpp/EclipseCdtFormatterStep.java index 1a83389eaa..9f9d591c3c 100644 --- a/lib-extra/src/main/java/com/diffplug/spotless/extra/cpp/EclipseCdtFormatterStep.java +++ b/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(); diff --git a/lib-extra/src/main/java/com/diffplug/spotless/extra/groovy/GrEclipseFormatterStep.java b/lib-extra/src/main/java/com/diffplug/spotless/extra/groovy/GrEclipseFormatterStep.java index 08c28889f9..fd3c8b7d8b 100644 --- a/lib-extra/src/main/java/com/diffplug/spotless/extra/groovy/GrEclipseFormatterStep.java +++ b/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.")) { diff --git a/lib-extra/src/main/java/com/diffplug/spotless/extra/java/EclipseJdtFormatterStep.java b/lib-extra/src/main/java/com/diffplug/spotless/extra/java/EclipseJdtFormatterStep.java index cd9314406f..4a90a40832 100644 --- a/lib-extra/src/main/java/com/diffplug/spotless/extra/java/EclipseJdtFormatterStep.java +++ b/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(); diff --git a/lib-extra/src/test/java/com/diffplug/spotless/extra/groovy/GrEclipseFormatterStepSpecialCaseTest.java b/lib-extra/src/test/java/com/diffplug/spotless/extra/groovy/GrEclipseFormatterStepSpecialCaseTest.java new file mode 100644 index 0000000000..8f832b89f0 --- /dev/null +++ b/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 ? "" : "$parm"} + * works: ${parm == null ? "" : "parm"} + */ + @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"); + } +} diff --git a/lib-extra/src/test/java/com/diffplug/spotless/extra/java/EclipseJdtFormatterStepSpecialCaseTest.java b/lib-extra/src/test/java/com/diffplug/spotless/extra/java/EclipseJdtFormatterStepSpecialCaseTest.java new file mode 100644 index 0000000000..eb93cf1f09 --- /dev/null +++ b/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"); + } +} diff --git a/testlib/src/main/resources/groovy/greclipse/format/SomeClass.fixed b/testlib/src/main/resources/groovy/greclipse/format/SomeClass.fixed new file mode 100644 index 0000000000..8c93ca4ad0 --- /dev/null +++ b/testlib/src/main/resources/groovy/greclipse/format/SomeClass.fixed @@ -0,0 +1,12 @@ +package com.somepackage + +class SomeClass { + + def func(parm) { + """ + ${parm == null ? "" : "parm"} + ${parm == null ? "" : "parm"} + + """ + } +} diff --git a/testlib/src/main/resources/groovy/greclipse/format/SomeClass.test b/testlib/src/main/resources/groovy/greclipse/format/SomeClass.test new file mode 100644 index 0000000000..62d4df0fc5 --- /dev/null +++ b/testlib/src/main/resources/groovy/greclipse/format/SomeClass.test @@ -0,0 +1,12 @@ +package com.somepackage + +class SomeClass { + + def func(parm) { + """ + ${parm == null ? "" : "$parm"} + ${parm == null ? "" : "$parm"} + + """ + } +} diff --git a/testlib/src/main/resources/java/eclipse/AbstractType.clean b/testlib/src/main/resources/java/eclipse/AbstractType.clean new file mode 100644 index 0000000000..475b1186fd --- /dev/null +++ b/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)"; + } + +} diff --git a/testlib/src/main/resources/java/eclipse/AbstractType.test b/testlib/src/main/resources/java/eclipse/AbstractType.test new file mode 100644 index 0000000000..d4753b2cfc --- /dev/null +++ b/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)"; + } + + +}