From 15670fa05ac2b69d4e8bd47ea58e140e8debc8a3 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 29 May 2024 10:51:33 -0700 Subject: [PATCH 1/8] Route all the `FormatterStep.create` through `FormatterStepSerializationRoundtrip`. --- .../java/com/diffplug/spotless/Formatter.java | 6 + .../com/diffplug/spotless/FormatterFunc.java | 4 +- .../com/diffplug/spotless/FormatterStep.java | 28 +--- .../diffplug/spotless/FormatterStepImpl.java | 133 ------------------ 4 files changed, 11 insertions(+), 160 deletions(-) delete mode 100644 lib/src/main/java/com/diffplug/spotless/FormatterStepImpl.java diff --git a/lib/src/main/java/com/diffplug/spotless/Formatter.java b/lib/src/main/java/com/diffplug/spotless/Formatter.java index 9400989ddf..b5cd5f81fc 100644 --- a/lib/src/main/java/com/diffplug/spotless/Formatter.java +++ b/lib/src/main/java/com/diffplug/spotless/Formatter.java @@ -312,4 +312,10 @@ public void close() { /** This Sentinel reference may be used to pass string content to a Formatter or FormatterStep when there is no actual File to format */ public static final File NO_FILE_SENTINEL = new File("NO_FILE_SENTINEL"); + + static void checkNotSentinel(File file) { + if (file == Formatter.NO_FILE_SENTINEL) { + throw new IllegalArgumentException("This step requires the underlying file. If this is a test, use StepHarnessWithFile"); + } + } } diff --git a/lib/src/main/java/com/diffplug/spotless/FormatterFunc.java b/lib/src/main/java/com/diffplug/spotless/FormatterFunc.java index 7aee828b0d..800a553225 100644 --- a/lib/src/main/java/com/diffplug/spotless/FormatterFunc.java +++ b/lib/src/main/java/com/diffplug/spotless/FormatterFunc.java @@ -115,7 +115,7 @@ public void close() { @Override public String apply(String unix, File file) throws Exception { - FormatterStepImpl.checkNotSentinel(file); + Formatter.checkNotSentinel(file); return function.apply(resource, unix, file); } @@ -144,7 +144,7 @@ interface NeedsFile extends FormatterFunc { @Override default String apply(String unix, File file) throws Exception { - FormatterStepImpl.checkNotSentinel(file); + Formatter.checkNotSentinel(file); return applyWithFile(unix, file); } diff --git a/lib/src/main/java/com/diffplug/spotless/FormatterStep.java b/lib/src/main/java/com/diffplug/spotless/FormatterStep.java index 5974151fba..2a5a7d2b2f 100644 --- a/lib/src/main/java/com/diffplug/spotless/FormatterStep.java +++ b/lib/src/main/java/com/diffplug/spotless/FormatterStep.java @@ -70,28 +70,6 @@ default FormatterStep filterByFile(SerializableFileFilter filter) { return new FilterByFileFormatterStep(this, filter); } - /** - * Implements a FormatterStep in a strict way which guarantees correct and lazy implementation - * of up-to-date checks. This maximizes performance for cases where the FormatterStep is not - * actually needed (e.g. don't load eclipse setting file unless this step is actually running) - * while also ensuring that Gradle can detect changes in a step's settings to determine that - * it needs to rerun a format. - */ - abstract class Strict extends LazyForwardingEquality implements FormatterStep { - private static final long serialVersionUID = 1L; - - /** - * Implements the formatting function strictly in terms - * of the input data and the result of {@link #calculateState()}. - */ - protected abstract String format(State state, String rawUnix, File file) throws Exception; - - @Override - public final String format(String rawUnix, File file) throws Exception { - return format(state(), rawUnix, file); - } - } - /** * @param name * The name of the formatter step. @@ -151,8 +129,8 @@ static static FormatterStep createLazy( String name, ThrowingEx.Supplier stateSupplier, - ThrowingEx.Function stateToFormatter) { - return new FormatterStepImpl.Standard<>(name, stateSupplier, stateToFormatter); + SerializedFunction stateToFormatter) { + return createLazy(name, stateSupplier, SerializedFunction.identity(), stateToFormatter); } /** @@ -168,7 +146,7 @@ static FormatterStep createLazy( static FormatterStep create( String name, State state, - ThrowingEx.Function stateToFormatter) { + SerializedFunction stateToFormatter) { Objects.requireNonNull(state, "state"); return createLazy(name, () -> state, stateToFormatter); } diff --git a/lib/src/main/java/com/diffplug/spotless/FormatterStepImpl.java b/lib/src/main/java/com/diffplug/spotless/FormatterStepImpl.java deleted file mode 100644 index 8e2982d6db..0000000000 --- a/lib/src/main/java/com/diffplug/spotless/FormatterStepImpl.java +++ /dev/null @@ -1,133 +0,0 @@ -/* - * Copyright 2016-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.File; -import java.io.Serializable; -import java.util.Objects; -import java.util.Random; - -import com.diffplug.spotless.FormatterStep.Strict; - -import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; - -/** - * Standard implementation of FormatExtension which cleanly enforces - * separation of serializable configuration and a pure format function. - *

- * Not an inner-class of FormatterStep so that it can stay entirely private - * from the API. - */ -@SuppressFBWarnings("SE_TRANSIENT_FIELD_NOT_RESTORED") -abstract class FormatterStepImpl extends Strict { - private static final long serialVersionUID = 1L; - - /** Transient because only the state matters. */ - final transient String name; - - /** Transient because only the state matters. */ - transient ThrowingEx.Supplier stateSupplier; - - FormatterStepImpl(String name, ThrowingEx.Supplier stateSupplier) { - this.name = Objects.requireNonNull(name); - this.stateSupplier = Objects.requireNonNull(stateSupplier); - } - - @Override - public String getName() { - return name; - } - - @Override - protected State calculateState() throws Exception { - // LazyForwardingEquality guarantees that this will only be called once, and keeping toFormat - // causes a memory leak, see https://github.com/diffplug/spotless/issues/1194 - State state = stateSupplier.get(); - stateSupplier = null; - return state; - } - - static final class Standard extends FormatterStepImpl { - private static final long serialVersionUID = 1L; - - final transient ThrowingEx.Function stateToFormatter; - transient FormatterFunc formatter; // initialized lazily - - Standard(String name, ThrowingEx.Supplier stateSupplier, ThrowingEx.Function stateToFormatter) { - super(name, stateSupplier); - this.stateToFormatter = Objects.requireNonNull(stateToFormatter); - } - - @Override - protected String format(State state, String rawUnix, File file) throws Exception { - Objects.requireNonNull(state, "state"); - Objects.requireNonNull(rawUnix, "rawUnix"); - Objects.requireNonNull(file, "file"); - if (formatter == null) { - formatter = stateToFormatter.apply(state()); - } - return formatter.apply(rawUnix, file); - } - - @Override - public void close() throws Exception { - if (formatter instanceof FormatterFunc.Closeable) { - ((FormatterFunc.Closeable) formatter).close(); - formatter = null; - } - } - } - - /** Formatter which is equal to itself, but not to any other Formatter. */ - static class NeverUpToDate extends FormatterStepImpl { - private static final long serialVersionUID = 1L; - - private static final Random RANDOM = new Random(); - - final transient ThrowingEx.Supplier formatterSupplier; - transient FormatterFunc formatter; // initialized lazily - - NeverUpToDate(String name, ThrowingEx.Supplier formatterSupplier) { - super(name, RANDOM::nextInt); - this.formatterSupplier = Objects.requireNonNull(formatterSupplier, "formatterSupplier"); - } - - @Override - protected String format(Integer state, String rawUnix, File file) throws Exception { - if (formatter == null) { - formatter = formatterSupplier.get(); - if (formatter instanceof FormatterFunc.Closeable) { - throw new AssertionError("NeverUpToDate does not support FormatterFunc.Closeable. See https://github.com/diffplug/spotless/pull/284"); - } - } - return formatter.apply(rawUnix, file); - } - - @Override - public void close() throws Exception { - if (formatter instanceof FormatterFunc.Closeable) { - ((FormatterFunc.Closeable) formatter).close(); - formatter = null; - } - } - } - - static void checkNotSentinel(File file) { - if (file == Formatter.NO_FILE_SENTINEL) { - throw new IllegalArgumentException("This step requires the underlying file. If this is a test, use StepHarnessWithFile"); - } - } -} From 116124d75412401cd31e4b8940a96eefccc0a6fd Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 23 May 2024 16:19:42 -0700 Subject: [PATCH 2/8] Fixup FenceStepTest so that we don't need so much `forStepNoRoundtrip`. --- .../spotless/generic/FenceStepTest.java | 70 +++++++++---------- 1 file changed, 32 insertions(+), 38 deletions(-) diff --git a/testlib/src/test/java/com/diffplug/spotless/generic/FenceStepTest.java b/testlib/src/test/java/com/diffplug/spotless/generic/FenceStepTest.java index 682855755e..697a199fd2 100644 --- a/testlib/src/test/java/com/diffplug/spotless/generic/FenceStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/generic/FenceStepTest.java @@ -16,16 +16,11 @@ package com.diffplug.spotless.generic; import java.io.File; -import java.io.Serializable; import java.util.Arrays; -import java.util.Objects; - -import javax.annotation.Nullable; import org.junit.jupiter.api.Test; import com.diffplug.common.base.StringPrinter; -import com.diffplug.spotless.FormatterFunc; import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.ResourceHarness; import com.diffplug.spotless.StepHarness; @@ -34,8 +29,8 @@ class FenceStepTest extends ResourceHarness { @Test void single() { FormatterStep fence = FenceStep.named("fence").openClose("spotless:off", "spotless:on") - .preserveWithin(Arrays.asList(createNeverUpToDateSerializable("lowercase", String::toLowerCase))); - StepHarness harness = StepHarness.forStepNoRoundtrip(fence); + .preserveWithin(Arrays.asList(ToCaseStep.lower())); + StepHarness harness = StepHarness.forStep(fence); harness.test( StringPrinter.buildStringFromLines( "A B C", @@ -54,8 +49,8 @@ void single() { @Test void multiple() { FormatterStep fence = FenceStep.named("fence").openClose("spotless:off", "spotless:on") - .preserveWithin(Arrays.asList(createNeverUpToDateSerializable("lowercase", String::toLowerCase))); - StepHarness harness = StepHarness.forStepNoRoundtrip(fence); + .preserveWithin(Arrays.asList(ToCaseStep.lower())); + StepHarness harness = StepHarness.forStep(fence); harness.test( StringPrinter.buildStringFromLines( "A B C", @@ -88,7 +83,7 @@ void multiple() { @Test void broken() { FormatterStep fence = FenceStep.named("fence").openClose("spotless:off", "spotless:on") - .preserveWithin(Arrays.asList(createNeverUpToDateSerializable("uppercase", String::toUpperCase))); + .preserveWithin(Arrays.asList(ToCaseStep.upper())); // this fails because uppercase turns spotless:off into SPOTLESS:OFF, etc StepHarness.forStepNoRoundtrip(fence).testExceptionMsg(StringPrinter.buildStringFromLines("A B C", "spotless:off", @@ -100,8 +95,8 @@ void broken() { @Test void andApply() { FormatterStep fence = FenceStep.named("fence").openClose("", "") - .applyWithin(Arrays.asList(createNeverUpToDateSerializable("lowercase", String::toLowerCase))); - StepHarness.forStepNoRoundtrip(fence).test( + .applyWithin(Arrays.asList(ToCaseStep.lower())); + StepHarness.forStep(fence).test( StringPrinter.buildStringFromLines( "A B C", "", @@ -116,46 +111,45 @@ void andApply() { "G H I")); } - /** - * @param name - * The name of the formatter step - * @param function - * The function used by the formatter step - * @return A FormatterStep which will never report that it is up-to-date, because - * it is not equal to the serialized representation of itself. - */ - static FormatterStep createNeverUpToDateSerializable( - String name, - T function) { - Objects.requireNonNull(function, "function"); - return new NeverUpToDateSerializable(name, function); - } + static class ToCaseStep implements FormatterStep { + static ToCaseStep upper() { + return new ToCaseStep(true); + } - static class NeverUpToDateSerializable implements FormatterStep, Serializable { - private final String name; - private final T formatterFunc; + static ToCaseStep lower() { + return new ToCaseStep(false); + } + + private final boolean uppercase; - private NeverUpToDateSerializable(String name, T formatterFunc) { - this.name = name; - this.formatterFunc = formatterFunc; + ToCaseStep(boolean uppercase) { + this.uppercase = uppercase; } @Override public String getName() { - return name; + return uppercase ? "uppercase" : "lowercase"; } - @Nullable + @org.jetbrains.annotations.Nullable @Override public String format(String rawUnix, File file) throws Exception { - return formatterFunc.apply(rawUnix, file); + return uppercase ? rawUnix.toUpperCase() : rawUnix.toLowerCase(); } @Override public void close() throws Exception { - if (formatterFunc instanceof FormatterFunc.Closeable) { - ((FormatterFunc.Closeable) formatterFunc).close(); - } + + } + + @Override + public int hashCode() { + return getName().hashCode(); + } + + @Override + public boolean equals(Object obj) { + return obj instanceof ToCaseStep && getName().equals(((ToCaseStep) obj).getName()); } } } From 8bef1adf591683b4c1bf325d875bb4f29dde23bc Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 29 May 2024 11:04:41 -0700 Subject: [PATCH 3/8] TODO: add warning that custom steps require Gradle 8.0+ --- .../spotless/BumpThisNumberIfACustomStepChangesTest.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/BumpThisNumberIfACustomStepChangesTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/BumpThisNumberIfACustomStepChangesTest.java index f959226032..e24c0175d0 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/BumpThisNumberIfACustomStepChangesTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/BumpThisNumberIfACustomStepChangesTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2021 DiffPlug + * Copyright 2016-2024 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,9 +17,14 @@ import java.io.IOException; +import org.gradle.testkit.runner.GradleRunner; import org.junit.jupiter.api.Test; class BumpThisNumberIfACustomStepChangesTest extends GradleIntegrationHarness { + @Override + protected GradleRunner gradleRunner() throws IOException { + return super.gradleRunner().withGradleVersion("8.0"); + } private void writeBuildFile(String toInsert) throws IOException { setFile("build.gradle").toLines( From 83850bfe8e1f5bfc0419c15fd6f841ebcfc0cf04 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 29 May 2024 11:40:10 -0700 Subject: [PATCH 4/8] Resolved TODO: custom steps only allowed on Gradle 8+ --- plugin-gradle/CHANGES.md | 2 ++ .../diffplug/gradle/spotless/FormatExtension.java | 7 +++++++ .../com/diffplug/gradle/spotless/SpotlessPlugin.java | 3 ++- .../gradle/spotless/SpotlessPluginRedirect.java | 10 +++++++--- .../BumpThisNumberIfACustomStepChangesTest.java | 6 ------ .../gradle/spotless/GitRatchetGradleTest.java | 2 +- .../gradle/spotless/GradleIntegrationHarness.java | 12 +++++++++--- 7 files changed, 28 insertions(+), 14 deletions(-) diff --git a/plugin-gradle/CHANGES.md b/plugin-gradle/CHANGES.md index 666221af89..99160a6011 100644 --- a/plugin-gradle/CHANGES.md +++ b/plugin-gradle/CHANGES.md @@ -4,6 +4,8 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( ## [Unreleased] ### Added +* Full, no-asterisk support of Gradle configuration cache. ([#1274](https://github.com/diffplug/spotless/issues/1274), giving up on [#987](https://github.com/diffplug/spotless/issues/987)) + * In order to use `custom`, you must now use Gradle 8.0+. * Respect `.editorconfig` settings for formatting shell via `shfmt` ([#2031](https://github.com/diffplug/spotless/pull/2031)) * Add support for formatting and sorting Maven POMs ([#2082](https://github.com/diffplug/spotless/issues/2082)) ### Fixed diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java index 6d8d85e2f4..b589074915 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java @@ -16,6 +16,7 @@ package com.diffplug.gradle.spotless; import static com.diffplug.gradle.spotless.PluginGradlePreconditions.requireElementsNonNull; +import static com.diffplug.gradle.spotless.SpotlessPluginRedirect.badSemver; import static java.util.Objects.requireNonNull; import java.io.File; @@ -451,6 +452,12 @@ public String apply(String unixNewlines) { */ public void custom(String name, FormatterFunc formatter) { requireNonNull(formatter, "formatter"); + if (badSemver(getProject()) < badSemver(SpotlessPlugin.VER_GRADLE_minVersionForCustom)) { + throw new GradleException("The 'custom' method is only available if you are using Gradle " + + SpotlessPlugin.VER_GRADLE_minVersionForCustom + + " or newer, this is " + + getProject().getGradle().getGradleVersion()); + } addStep(FormatterStep.createLazy(name, () -> globalState, SerializedFunction.alwaysReturns(formatter))); } diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessPlugin.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessPlugin.java index 0043e2b274..407f98dd0d 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessPlugin.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessPlugin.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2023 DiffPlug + * Copyright 2016-2024 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -28,6 +28,7 @@ public class SpotlessPlugin implements Plugin { static final String SPOTLESS_MODERN = "spotlessModern"; static final String VER_GRADLE_min = "6.1.1"; static final String VER_GRADLE_javaPluginExtension = "7.1"; + static final String VER_GRADLE_minVersionForCustom = "8.0"; private static final int MINIMUM_JRE = 11; @Override diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessPluginRedirect.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessPluginRedirect.java index bca8ea7c70..9668042400 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessPluginRedirect.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessPluginRedirect.java @@ -1,5 +1,5 @@ /* - * Copyright 2020-2023 DiffPlug + * Copyright 2020-2024 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -27,7 +27,11 @@ public class SpotlessPluginRedirect implements Plugin { private static final Pattern BAD_SEMVER = Pattern.compile("(\\d+)\\.(\\d+)"); - private static int badSemver(String input) { + static int badSemver(Project project) { + return badSemver(project.getGradle().getGradleVersion()); + } + + static int badSemver(String input) { Matcher matcher = BAD_SEMVER.matcher(input); if (!matcher.find() || matcher.start() != 0) { throw new IllegalArgumentException("Version must start with " + BAD_SEMVER.pattern()); @@ -46,7 +50,7 @@ private static int badSemver(int major, int minor) { static boolean gradleIsTooOld(Project project) { if (gradleIsTooOld == null) { - gradleIsTooOld = badSemver(project.getGradle().getGradleVersion()) < badSemver(SpotlessPlugin.VER_GRADLE_min); + gradleIsTooOld = badSemver(project) < badSemver(SpotlessPlugin.VER_GRADLE_min); } return gradleIsTooOld.booleanValue(); } diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/BumpThisNumberIfACustomStepChangesTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/BumpThisNumberIfACustomStepChangesTest.java index e24c0175d0..28351bd9b7 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/BumpThisNumberIfACustomStepChangesTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/BumpThisNumberIfACustomStepChangesTest.java @@ -17,15 +17,9 @@ import java.io.IOException; -import org.gradle.testkit.runner.GradleRunner; import org.junit.jupiter.api.Test; class BumpThisNumberIfACustomStepChangesTest extends GradleIntegrationHarness { - @Override - protected GradleRunner gradleRunner() throws IOException { - return super.gradleRunner().withGradleVersion("8.0"); - } - private void writeBuildFile(String toInsert) throws IOException { setFile("build.gradle").toLines( "plugins {", diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GitRatchetGradleTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GitRatchetGradleTest.java index 388bbbb8d1..d63efd9e57 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GitRatchetGradleTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GitRatchetGradleTest.java @@ -53,7 +53,7 @@ private Git initRepo() throws IllegalStateException, GitAPIException, IOExceptio @Override protected GradleRunner gradleRunner() throws IOException { - return super.gradleRunner().withGradleVersion(GradleVersionSupport.CONFIGURATION_CACHE.version); + return super.gradleRunner().withGradleVersion(GradleVersionSupport.CUSTOM_STEPS.version); } @ParameterizedTest diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GradleIntegrationHarness.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GradleIntegrationHarness.java index 417bc23b52..8ef71ec020 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GradleIntegrationHarness.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GradleIntegrationHarness.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2023 DiffPlug + * Copyright 2016-2024 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -43,7 +43,7 @@ public class GradleIntegrationHarness extends ResourceHarness { public enum GradleVersionSupport { - JRE_11("5.0"), MINIMUM(SpotlessPlugin.VER_GRADLE_min), + JRE_11("5.0"), MINIMUM(SpotlessPlugin.VER_GRADLE_min), CUSTOM_STEPS(SpotlessPlugin.VER_GRADLE_minVersionForCustom), // technically, this API exists in 6.5, but the flags for it change in 6.6, so we build to that CONFIGURATION_CACHE("6.6"), // https://docs.gradle.org/7.5/userguide/configuration_cache.html#config_cache:stable @@ -116,8 +116,14 @@ void gitAttributes() throws IOException { } protected GradleRunner gradleRunner() throws IOException { + GradleVersionSupport version; + if (newFile("build.gradle").exists() && read("build.gradle").contains("custom")) { + version = GradleVersionSupport.CUSTOM_STEPS; + } else { + version = GradleVersionSupport.MINIMUM; + } return GradleRunner.create() - .withGradleVersion(GradleVersionSupport.MINIMUM.version) + .withGradleVersion(version.version) .withProjectDir(rootFolder()) .withTestKitDir(getTestKitDir()) .withPluginClasspath(); From cffb2f474547e57483baea3fc767ed2937279f12 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 23 May 2024 16:54:43 -0700 Subject: [PATCH 5/8] StepHarness should operate on the post-roundtripped set, not the pre-roundtripped. --- .../src/main/java/com/diffplug/spotless/StepHarnessBase.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/testlib/src/main/java/com/diffplug/spotless/StepHarnessBase.java b/testlib/src/main/java/com/diffplug/spotless/StepHarnessBase.java index 656057cd4e..d86021ef99 100644 --- a/testlib/src/main/java/com/diffplug/spotless/StepHarnessBase.java +++ b/testlib/src/main/java/com/diffplug/spotless/StepHarnessBase.java @@ -27,14 +27,15 @@ enum RoundTrip { private final Formatter formatter; protected StepHarnessBase(Formatter formatter, RoundTrip roundTrip) { - this.formatter = Objects.requireNonNull(formatter); if (roundTrip == RoundTrip.DONT_ROUNDTRIP) { + this.formatter = Objects.requireNonNull(formatter); return; } Formatter roundTripped = SerializableEqualityTester.reserialize(formatter); if (roundTrip == RoundTrip.ASSERT_EQUAL) { Assertions.assertThat(roundTripped).isEqualTo(formatter); } + this.formatter = roundTripped; } protected Formatter formatter() { From 50429a8dff941e0852fd7b8231763dd3ef68f398 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 23 May 2024 16:06:11 -0700 Subject: [PATCH 6/8] Fix round-tripping of NativeCmdStep. --- .../spotless/generic/NativeCmdStep.java | 25 ++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/generic/NativeCmdStep.java b/lib/src/main/java/com/diffplug/spotless/generic/NativeCmdStep.java index 31b30fe56f..3e58c9dbae 100644 --- a/lib/src/main/java/com/diffplug/spotless/generic/NativeCmdStep.java +++ b/lib/src/main/java/com/diffplug/spotless/generic/NativeCmdStep.java @@ -1,5 +1,5 @@ /* - * Copyright 2021 DiffPlug + * Copyright 2021-2024 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -35,24 +35,37 @@ private NativeCmdStep() {} public static FormatterStep create(String name, File pathToExe, List arguments) { Objects.requireNonNull(name, "name"); Objects.requireNonNull(pathToExe, "pathToExe"); - return FormatterStep.createLazy(name, () -> new State(FileSignature.signAsList(pathToExe), arguments), State::toFunc); + return FormatterStep.createLazy(name, () -> new State(FileSignature.promise(pathToExe), arguments), State::toRuntime, Runtime::toFunc); } static class State implements Serializable { - private static final long serialVersionUID = 1L; + private static final long serialVersionUID = 2L; + final FileSignature.Promised pathToExe; + final List arguments; - final FileSignature pathToExe; + State(FileSignature.Promised pathToExe, List arguments) { + this.pathToExe = pathToExe; + this.arguments = arguments; + } + + Runtime toRuntime() { + return new Runtime(pathToExe.get().getOnlyFile(), arguments); + } + } + static class Runtime implements Serializable { + private static final long serialVersionUID = 2L; + final File pathToExe; final List arguments; - State(FileSignature pathToExe, List arguments) { + Runtime(File pathToExe, List arguments) { this.pathToExe = pathToExe; this.arguments = arguments; } String format(ProcessRunner runner, String input) throws IOException, InterruptedException { List argumentsWithPathToExe = new ArrayList<>(); - argumentsWithPathToExe.add(pathToExe.getOnlyFile().getAbsolutePath()); + argumentsWithPathToExe.add(pathToExe.getAbsolutePath()); if (arguments != null) { argumentsWithPathToExe.addAll(arguments); } From 92664ebf87d950c702e9f03b3ea7d673f4f396b5 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 4 Apr 2024 10:35:37 -0700 Subject: [PATCH 7/8] Fix round-tripping of DBeaver step. --- .../spotless/sql/DBeaverSQLFormatterStep.java | 27 ++++++------------- .../sql/DBeaverSQLFormatterStepTest.java | 10 +++---- 2 files changed, 13 insertions(+), 24 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/sql/DBeaverSQLFormatterStep.java b/lib/src/main/java/com/diffplug/spotless/sql/DBeaverSQLFormatterStep.java index fece2ea0fd..e9c7a59e54 100644 --- a/lib/src/main/java/com/diffplug/spotless/sql/DBeaverSQLFormatterStep.java +++ b/lib/src/main/java/com/diffplug/spotless/sql/DBeaverSQLFormatterStep.java @@ -1,5 +1,5 @@ /* - * Copyright 2016 DiffPlug + * Copyright 2016-2024 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,7 +16,6 @@ package com.diffplug.spotless.sql; import java.io.File; -import java.io.Serializable; import com.diffplug.spotless.FileSignature; import com.diffplug.spotless.FormatterFunc; @@ -32,24 +31,14 @@ public class DBeaverSQLFormatterStep { private DBeaverSQLFormatterStep() {} public static FormatterStep create(Iterable files) { - return FormatterStep.createLazy(NAME, - () -> new State(files), - State::createFormat); + return FormatterStep.create(NAME, FileSignature.promise(files), + FileSignature.Promised::get, + DBeaverSQLFormatterStep::createFormat); } - static final class State implements Serializable { - private static final long serialVersionUID = 1L; - - final FileSignature settingsSignature; - - State(final Iterable settingsFiles) throws Exception { - this.settingsSignature = FileSignature.signAsList(settingsFiles); - } - - FormatterFunc createFormat() throws Exception { - FormatterProperties preferences = FormatterProperties.from(settingsSignature.files()); - DBeaverSQLFormatter dbeaverSqlFormatter = new DBeaverSQLFormatter(preferences.getProperties()); - return dbeaverSqlFormatter::format; - } + private static FormatterFunc createFormat(FileSignature settings) { + FormatterProperties preferences = FormatterProperties.from(settings.files()); + DBeaverSQLFormatter dbeaverSqlFormatter = new DBeaverSQLFormatter(preferences.getProperties()); + return dbeaverSqlFormatter::format; } } diff --git a/testlib/src/test/java/com/diffplug/spotless/sql/DBeaverSQLFormatterStepTest.java b/testlib/src/test/java/com/diffplug/spotless/sql/DBeaverSQLFormatterStepTest.java index 58b0115b9c..648ddc6669 100644 --- a/testlib/src/test/java/com/diffplug/spotless/sql/DBeaverSQLFormatterStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/sql/DBeaverSQLFormatterStepTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2021 DiffPlug + * Copyright 2016-2024 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -29,7 +29,7 @@ class DBeaverSQLFormatterStepTest extends ResourceHarness { @Test - void behavior() throws Exception { + void behavior() { FormatterStep step = DBeaverSQLFormatterStep.create(Collections.emptySet()); StepHarness.forStep(step) .testResource("sql/dbeaver/full.dirty", "sql/dbeaver/full.clean") @@ -40,21 +40,21 @@ void behavior() throws Exception { } @Test - void behaviorWithConfigFile() throws Exception { + void behaviorWithConfigFile() { FormatterStep step = DBeaverSQLFormatterStep.create(createTestFiles("sql/dbeaver/sqlConfig.properties")); StepHarness.forStep(step) .testResource("sql/dbeaver/create.dirty", "sql/dbeaver/create.clean"); } @Test - void behaviorWithAlternativeConfigFile() throws Exception { + void behaviorWithAlternativeConfigFile() { FormatterStep step = DBeaverSQLFormatterStep.create(createTestFiles("sql/dbeaver/sqlConfig2.properties")); StepHarness.forStep(step) .testResource("sql/dbeaver/create.dirty", "sql/dbeaver/create.clean.alternative"); } @Test - void equality() throws Exception { + void equality() { List sqlConfig1 = createTestFiles("sql/dbeaver/sqlConfig.properties"); List sqlConfig2 = createTestFiles("sql/dbeaver/sqlConfig2.properties"); new SerializableEqualityTester() { From 5473eb1d0fc99c2790607205b84b51aa9d5a10b3 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 29 May 2024 15:15:14 -0700 Subject: [PATCH 8/8] Add proper roundtrip behavior to the `ForeignExe` steps. --- .../spotless/cpp/ClangFormatStep.java | 12 ++++---- .../diffplug/spotless/go/GofmtFormatStep.java | 28 +++++++++++++++---- .../diffplug/spotless/python/BlackStep.java | 12 ++++---- .../diffplug/spotless/shell/ShfmtStep.java | 28 +++++++++++++++---- 4 files changed, 56 insertions(+), 24 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/cpp/ClangFormatStep.java b/lib/src/main/java/com/diffplug/spotless/cpp/ClangFormatStep.java index 345ac723c3..64a43bf439 100644 --- a/lib/src/main/java/com/diffplug/spotless/cpp/ClangFormatStep.java +++ b/lib/src/main/java/com/diffplug/spotless/cpp/ClangFormatStep.java @@ -64,10 +64,10 @@ public ClangFormatStep withPathToExe(String pathToExe) { } public FormatterStep create() { - return FormatterStep.createLazy(name(), this::createState, RoundtripState::state, State::toFunc); + return FormatterStep.createLazy(name(), this::createRoundtrip, RoundtripState::toEquality, EqualityState::toFunc); } - private RoundtripState createState() throws IOException, InterruptedException { + private RoundtripState createRoundtrip() throws IOException, InterruptedException { String howToInstall = "" + "You can download clang-format from https://releases.llvm.org and " + "then point Spotless to it with {@code pathToExe('/path/to/clang-format')} " + @@ -98,13 +98,13 @@ static class RoundtripState implements Serializable { this.exe = exe; } - private State state() { - return new State(version, style, exe); + private EqualityState toEquality() { + return new EqualityState(version, style, exe); } } @SuppressFBWarnings("SE_TRANSIENT_FIELD_NOT_RESTORED") - static class State implements Serializable { + static class EqualityState implements Serializable { private static final long serialVersionUID = -1825662356883926318L; // used for up-to-date checks and caching final String version; @@ -113,7 +113,7 @@ static class State implements Serializable { // used for executing private transient @Nullable List args; - State(String version, @Nullable String style, ForeignExe pathToExe) { + EqualityState(String version, @Nullable String style, ForeignExe pathToExe) { this.version = version; this.style = style; this.exe = Objects.requireNonNull(pathToExe); diff --git a/lib/src/main/java/com/diffplug/spotless/go/GofmtFormatStep.java b/lib/src/main/java/com/diffplug/spotless/go/GofmtFormatStep.java index 18e388cf5b..ffdc17cd21 100644 --- a/lib/src/main/java/com/diffplug/spotless/go/GofmtFormatStep.java +++ b/lib/src/main/java/com/diffplug/spotless/go/GofmtFormatStep.java @@ -63,10 +63,10 @@ public GofmtFormatStep withGoExecutable(String pathToExe) { } public FormatterStep create() { - return FormatterStep.createLazy(name(), this::createState, GofmtFormatStep.State::toFunc); + return FormatterStep.createLazy(name(), this::createRountrip, RoundtripState::toEquality, EqualityState::toFunc); } - private State createState() throws IOException, InterruptedException { + private RoundtripState createRountrip() throws IOException, InterruptedException { String howToInstall = "gofmt is a part of standard go distribution. If spotless can't discover it automatically, " + "you can point Spotless to the go binary with {@code pathToExe('/path/to/go')}"; final ForeignExe exe = ForeignExe.nameAndVersion("go", version) @@ -76,18 +76,34 @@ private State createState() throws IOException, InterruptedException { .fixWrongVersion( "You can tell Spotless to use the version you already have with {@code gofmt('{versionFound}')}" + "or you can install the currently specified Go version, {version}.\n" + howToInstall); - return new State(this, exe); + return new RoundtripState(version, exe); + } + + static class RoundtripState implements Serializable { + private static final long serialVersionUID = 1L; + + final String version; + final ForeignExe exe; + + RoundtripState(String version, ForeignExe exe) { + this.version = version; + this.exe = exe; + } + + private EqualityState toEquality() { + return new EqualityState(version, exe); + } } @SuppressFBWarnings("SE_TRANSIENT_FIELD_NOT_RESTORED") - static class State implements Serializable { + static class EqualityState implements Serializable { private static final long serialVersionUID = -1825662355363926318L; // used for up-to-date checks and caching final String version; final transient ForeignExe exe; - public State(GofmtFormatStep step, ForeignExe goExecutable) { - this.version = step.version; + public EqualityState(String version, ForeignExe goExecutable) { + this.version = version; this.exe = Objects.requireNonNull(goExecutable); } diff --git a/lib/src/main/java/com/diffplug/spotless/python/BlackStep.java b/lib/src/main/java/com/diffplug/spotless/python/BlackStep.java index 9fe26cdae8..46d00caab2 100644 --- a/lib/src/main/java/com/diffplug/spotless/python/BlackStep.java +++ b/lib/src/main/java/com/diffplug/spotless/python/BlackStep.java @@ -56,10 +56,10 @@ public BlackStep withPathToExe(String pathToExe) { } public FormatterStep create() { - return FormatterStep.createLazy(name(), this::createState, RoundtripState::state, State::toFunc); + return FormatterStep.createLazy(name(), this::createRoundtrip, RoundtripState::toEquality, EqualityState::toFunc); } - private RoundtripState createState() { + private RoundtripState createRoundtrip() { String trackingIssue = "\n github issue to handle this better: https://github.com/diffplug/spotless/issues/674"; ForeignExe exeAbsPath = ForeignExe.nameAndVersion("black", version) .pathToExe(pathToExe) @@ -80,13 +80,13 @@ static class RoundtripState implements Serializable { this.exe = exe; } - private State state() { - return new State(version, exe); + private EqualityState toEquality() { + return new EqualityState(version, exe); } } @SuppressFBWarnings("SE_TRANSIENT_FIELD_NOT_RESTORED") - static class State implements Serializable { + static class EqualityState implements Serializable { private static final long serialVersionUID = -1825662356883926318L; // used for up-to-date checks and caching final String version; @@ -94,7 +94,7 @@ static class State implements Serializable { // used for executing private transient @Nullable String[] args; - State(String version, ForeignExe exeAbsPath) { + EqualityState(String version, ForeignExe exeAbsPath) { this.version = version; this.exe = Objects.requireNonNull(exeAbsPath); } diff --git a/lib/src/main/java/com/diffplug/spotless/shell/ShfmtStep.java b/lib/src/main/java/com/diffplug/spotless/shell/ShfmtStep.java index 6fd83aac80..6630bf32a1 100644 --- a/lib/src/main/java/com/diffplug/spotless/shell/ShfmtStep.java +++ b/lib/src/main/java/com/diffplug/spotless/shell/ShfmtStep.java @@ -60,10 +60,10 @@ public ShfmtStep withPathToExe(String pathToExe) { } public FormatterStep create() { - return FormatterStep.createLazy(name(), this::createState, State::toFunc); + return FormatterStep.createLazy(name(), this::createRoundtrip, RoundtripState::toEquality, EqualityState::toFunc); } - private State createState() throws IOException, InterruptedException { + private RoundtripState createRoundtrip() throws IOException, InterruptedException { String howToInstall = "" + "You can download shfmt from https://github.com/mvdan/sh and " + "then point Spotless to it with {@code pathToExe('/path/to/shfmt')} " + @@ -79,11 +79,27 @@ private State createState() throws IOException, InterruptedException { .fixWrongVersion( "You can tell Spotless to use the version you already have with {@code shfmt('{versionFound}')}" + "or you can download the currently specified version, {version}.\n" + howToInstall); - return new State(this, exe); + return new RoundtripState(version, exe); + } + + static class RoundtripState implements Serializable { + private static final long serialVersionUID = 1L; + + final String version; + final ForeignExe exe; + + RoundtripState(String version, ForeignExe exe) { + this.version = version; + this.exe = exe; + } + + private EqualityState toEquality() { + return new EqualityState(version, exe); + } } @SuppressFBWarnings("SE_TRANSIENT_FIELD_NOT_RESTORED") - static class State implements Serializable { + static class EqualityState implements Serializable { private static final long serialVersionUID = -1825662356883926318L; // used for up-to-date checks and caching @@ -93,8 +109,8 @@ static class State implements Serializable { // used for executing private transient @Nullable List args; - State(ShfmtStep step, ForeignExe pathToExe) { - this.version = step.version; + EqualityState(String version, ForeignExe pathToExe) { + this.version = version; this.exe = Objects.requireNonNull(pathToExe); }