diff --git a/CHANGES.md b/CHANGES.md index 1151dd8430..5af6d41b63 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -10,6 +10,10 @@ This document is intended for Spotless developers. We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `1.27.0`). ## [Unreleased] +### Added +* `FileSignature.Promised` and `JarState.Promised` to facilitate round-trip serialization for the Gradle configuration cache. ([#1945](https://github.com/diffplug/spotless/pull/1945)) +### Removed +* **BREAKING** Remove `JarState.getMavenCoordinate(String prefix)`. ([#1945](https://github.com/diffplug/spotless/pull/1945)) ### Fixed * Ignore system git config when running tests ([#1990](https://github.com/diffplug/spotless/issues/1990)) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5fce5a9460..27e6221e76 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -94,10 +94,31 @@ Here's a checklist for creating a new step for Spotless: - [ ] Class name ends in Step, `SomeNewStep`. - [ ] Class has a public static method named `create` that returns a `FormatterStep`. -- [ ] Has a test class named `SomeNewStepTest`. +- [ ] Has a test class named `SomeNewStepTest` that uses `StepHarness` or `StepHarnessWithFile` to test the step. + - [ ] Start with `StepHarness.forStep(myStep).supportsRoundTrip(false)`, and then add round trip support as described in the next section. - [ ] Test class has test methods to verify behavior. - [ ] Test class has a test method `equality()` which tests equality using `StepEqualityTester` (see existing methods for examples). +### Serialization roundtrip + +In order to support Gradle's configuration cache, all `FormatterStep` must be round-trip serializable. This is a bit tricky because step equality is based on the serialized form of the state, and `transient` is used to take absolute paths out of the equality check. To make this work, roundtrip compatible steps actually have *two* states: + +- `RoundtripState` which must be roundtrip serializable but has no equality constraints + - `FileSignature.Promised` for settings files and `JarState.Promised` for the classpath +- `EqualityState` which will never be reserialized and its serialized form is used for equality / hashCode checks + - `FileSignature` for settings files and `JarState` for the classpath + +```java +FormatterStep create(String name, + RoundtripState roundTrip, + SerializedFunction equalityFunc, + SerializedFunction formatterFunc) +FormatterStep createLazy(String name, + Supplier roundTrip, + SerializedFunction equalityFunc, + SerializedFunction formatterFunc) +``` + ### Third-party dependencies via reflection or compile-only source sets Most formatters are going to use some kind of third-party jar. Spotless integrates with many formatters, some of which have incompatible transitive dependencies. To address this, we resolve third-party dependencies using [`JarState`](https://github.com/diffplug/spotless/blob/b26f0972b185995d7c6a7aefa726c146d24d9a82/lib/src/main/java/com/diffplug/spotless/kotlin/KtfmtStep.java#L118). To call methods on the classes in that `JarState`, you can either use reflection or a compile-only source set. See [#524](https://github.com/diffplug/spotless/issues/524) for examples of both approaches. diff --git a/lib-extra/src/main/java/com/diffplug/spotless/extra/EclipseBasedStepBuilder.java b/lib-extra/src/main/java/com/diffplug/spotless/extra/EclipseBasedStepBuilder.java index 4f48a5ed74..f22d0a6806 100644 --- a/lib-extra/src/main/java/com/diffplug/spotless/extra/EclipseBasedStepBuilder.java +++ b/lib-extra/src/main/java/com/diffplug/spotless/extra/EclipseBasedStepBuilder.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. @@ -24,7 +24,6 @@ import java.util.ArrayList; import java.util.List; import java.util.Objects; -import java.util.Optional; import java.util.Properties; import com.diffplug.common.base.Errors; @@ -187,12 +186,6 @@ public Properties getPreferences() { return preferences.getProperties(); } - /** Returns first coordinate from sorted set that starts with a given prefix.*/ - public Optional getMavenCoordinate(String prefix) { - return jarState.getMavenCoordinates().stream() - .filter(coordinate -> coordinate.startsWith(prefix)).findFirst(); - } - /** * Load class based on the given configuration of JAR provider and Maven coordinates. * Different class loader instances are provided in the following scenarios: diff --git a/lib/src/main/java/com/diffplug/spotless/FileSignature.java b/lib/src/main/java/com/diffplug/spotless/FileSignature.java index 59893f26e6..d6a23a8fd1 100644 --- a/lib/src/main/java/com/diffplug/spotless/FileSignature.java +++ b/lib/src/main/java/com/diffplug/spotless/FileSignature.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. @@ -33,6 +33,7 @@ import java.util.Locale; import java.util.Map; +import edu.umd.cs.findbugs.annotations.Nullable; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; /** Computes a signature for any needed files. */ @@ -43,6 +44,8 @@ public final class FileSignature implements Serializable { * Transient because not needed to uniquely identify a FileSignature instance, and also because * Gradle only needs this class to be Serializable so it can compare FileSignature instances for * incremental builds. + * + * We don't want these absolute paths to screw up buildcache keys. */ @SuppressFBWarnings("SE_TRANSIENT_FIELD_NOT_RESTORED") private final transient List files; @@ -93,6 +96,31 @@ private FileSignature(final List files) throws IOException { } } + /** A view of `FileSignature` which can be safely roundtripped. */ + public static class Promised implements Serializable { + private static final long serialVersionUID = 1L; + private final List files; + @SuppressFBWarnings("SE_TRANSIENT_FIELD_NOT_RESTORED") + private transient @Nullable FileSignature cached; + + private Promised(List files, @Nullable FileSignature cached) { + this.files = files; + this.cached = cached; + } + + public FileSignature get() { + if (cached == null) { + // null when restored via serialization + cached = ThrowingEx.get(() -> new FileSignature(files)); + } + return cached; + } + } + + public Promised asPromise() { + return new Promised(files, this); + } + /** Returns all of the files in this signature, throwing an exception if there are more or less than 1 file. */ public Collection files() { return Collections.unmodifiableList(files); diff --git a/lib/src/main/java/com/diffplug/spotless/Formatter.java b/lib/src/main/java/com/diffplug/spotless/Formatter.java index 68d9c443bf..fc6fe32afd 100644 --- a/lib/src/main/java/com/diffplug/spotless/Formatter.java +++ b/lib/src/main/java/com/diffplug/spotless/Formatter.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. @@ -304,6 +304,8 @@ public void close() { for (FormatterStep step : steps) { if (step instanceof FormatterStepImpl.Standard) { ((FormatterStepImpl.Standard) step).cleanupFormatterFunc(); + } else if (step instanceof FormatterStepEqualityOnStateSerialization) { + ((FormatterStepEqualityOnStateSerialization) step).cleanupFormatterFunc(); } } } diff --git a/lib/src/main/java/com/diffplug/spotless/FormatterStep.java b/lib/src/main/java/com/diffplug/spotless/FormatterStep.java index ca37303b1f..26bc1e56e5 100644 --- a/lib/src/main/java/com/diffplug/spotless/FormatterStep.java +++ b/lib/src/main/java/com/diffplug/spotless/FormatterStep.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. @@ -29,7 +29,7 @@ */ public interface FormatterStep extends Serializable { /** The name of the step, for debugging purposes. */ - public String getName(); + String getName(); /** * Returns a formatted version of the given content. @@ -43,7 +43,8 @@ public interface FormatterStep extends Serializable { * if the formatter step doesn't have any changes to make * @throws Exception if the formatter step experiences a problem */ - public @Nullable String format(String rawUnix, File file) throws Exception; + @Nullable + String format(String rawUnix, File file) throws Exception; /** * Returns a new FormatterStep which will only apply its changes @@ -54,7 +55,7 @@ public interface FormatterStep extends Serializable { * @return FormatterStep */ @Deprecated - public default FormatterStep filterByContentPattern(String contentPattern) { + default FormatterStep filterByContentPattern(String contentPattern) { return filterByContent(OnMatch.INCLUDE, contentPattern); } @@ -68,7 +69,7 @@ public default FormatterStep filterByContentPattern(String contentPattern) { * java regular expression used to filter in or out files which content contain pattern * @return FormatterStep */ - public default FormatterStep filterByContent(OnMatch onMatch, String contentPattern) { + default FormatterStep filterByContent(OnMatch onMatch, String contentPattern) { return new FilterByContentPatternFormatterStep(this, onMatch, contentPattern); } @@ -78,7 +79,7 @@ public default FormatterStep filterByContent(OnMatch onMatch, String contentPatt *

* The provided filter must be serializable. */ - public default FormatterStep filterByFile(SerializableFileFilter filter) { + default FormatterStep filterByFile(SerializableFileFilter filter) { return new FilterByFileFormatterStep(this, filter); } @@ -104,6 +105,51 @@ public final String format(String rawUnix, File file) throws Exception { } } + /** + * @param name + * The name of the formatter step. + * @param roundtripInit + * If the step has any state, this supplier will calculate it lazily. The supplier doesn't + * have to be serializable, but the result it calculates needs to be serializable. + * @param equalityFunc + * A pure serializable function (method reference recommended) which takes the result of `roundtripInit`, + * and returns a serializable object whose serialized representation will be used for `.equals` and + * `.hashCode` of the FormatterStep. + * @param formatterFunc + * A pure serializable function (method reference recommended) which takes the result of `equalityFunc`, + * and returns a `FormatterFunc` which will be used for the actual formatting. + * @return A FormatterStep which can be losslessly roundtripped through the java serialization machinery. + */ + static FormatterStep createLazy( + String name, + ThrowingEx.Supplier roundtripInit, + SerializedFunction equalityFunc, + SerializedFunction formatterFunc) { + return new FormatterStepSerializationRoundtrip<>(name, roundtripInit, equalityFunc, formatterFunc); + } + + /** + * @param name + * The name of the formatter step. + * @param roundTrip + * The roundtrip serializable state of the step. + * @param equalityFunc + * A pure serializable function (method reference recommended) which takes the result of `roundTrip`, + * and returns a serializable object whose serialized representation will be used for `.equals` and + * `.hashCode` of the FormatterStep. + * @param formatterFunc + * A pure serializable function (method reference recommended) which takes the result of `equalityFunc`, + * and returns a `FormatterFunc` which will be used for the actual formatting. + * @return A FormatterStep which can be losslessly roundtripped through the java serialization machinery. + */ + static FormatterStep create( + String name, + RoundtripState roundTrip, + SerializedFunction equalityFunc, + SerializedFunction formatterFunc) { + return createLazy(name, () -> roundTrip, equalityFunc, formatterFunc); + } + /** * @param name * The name of the formatter step @@ -115,7 +161,7 @@ public final String format(String rawUnix, File file) throws Exception { * only the state supplied by state and nowhere else. * @return A FormatterStep */ - public static FormatterStep createLazy( + static FormatterStep createLazy( String name, ThrowingEx.Supplier stateSupplier, ThrowingEx.Function stateToFormatter) { @@ -132,7 +178,7 @@ public static FormatterStep createLazy( * only the state supplied by state and nowhere else. * @return A FormatterStep */ - public static FormatterStep create( + static FormatterStep create( String name, State state, ThrowingEx.Function stateToFormatter) { @@ -149,7 +195,7 @@ public static FormatterStep create( * @return A FormatterStep which will never report that it is up-to-date, because * it is not equal to the serialized representation of itself. */ - public static FormatterStep createNeverUpToDateLazy( + static FormatterStep createNeverUpToDateLazy( String name, ThrowingEx.Supplier functionSupplier) { return new FormatterStepImpl.NeverUpToDate(name, functionSupplier); @@ -163,7 +209,7 @@ public static FormatterStep createNeverUpToDateLazy( * @return A FormatterStep which will never report that it is up-to-date, because * it is not equal to the serialized representation of itself. */ - public static FormatterStep createNeverUpToDate( + static FormatterStep createNeverUpToDate( String name, FormatterFunc function) { Objects.requireNonNull(function, "function"); diff --git a/lib/src/main/java/com/diffplug/spotless/FormatterStepEqualityOnStateSerialization.java b/lib/src/main/java/com/diffplug/spotless/FormatterStepEqualityOnStateSerialization.java new file mode 100644 index 0000000000..0ff279c5f5 --- /dev/null +++ b/lib/src/main/java/com/diffplug/spotless/FormatterStepEqualityOnStateSerialization.java @@ -0,0 +1,87 @@ +/* + * 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.Arrays; + +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + +/** + * Standard implementation of FormatterStep which cleanly enforces + * separation of a lazily computed "state" object whose serialized form + * is used as the basis for equality and hashCode, which is separate + * from the serialized form of the step itself, which can include absolute paths + * and such without interfering with buildcache keys. + */ +@SuppressFBWarnings("SE_TRANSIENT_FIELD_NOT_RESTORED") +abstract class FormatterStepEqualityOnStateSerialization implements FormatterStep, Serializable { + private static final long serialVersionUID = 1L; + + protected abstract State stateSupplier() throws Exception; + + protected abstract FormatterFunc stateToFormatter(State state) throws Exception; + + private transient FormatterFunc formatter; + private transient State stateInternal; + private transient byte[] serializedStateInternal; + + @Override + public String format(String rawUnix, File file) throws Exception { + if (formatter == null) { + formatter = stateToFormatter(state()); + } + return formatter.apply(rawUnix, file); + } + + @Override + public boolean equals(Object o) { + if (o == null) { + return false; + } else if (getClass() != o.getClass()) { + return false; + } else { + return Arrays.equals(serializedState(), ((FormatterStepEqualityOnStateSerialization) o).serializedState()); + } + } + + @Override + public int hashCode() { + return Arrays.hashCode(serializedState()); + } + + void cleanupFormatterFunc() { + if (formatter instanceof FormatterFunc.Closeable) { + ((FormatterFunc.Closeable) formatter).close(); + formatter = null; + } + } + + private State state() throws Exception { + if (stateInternal == null) { + stateInternal = stateSupplier(); + } + return stateInternal; + } + + private byte[] serializedState() { + if (serializedStateInternal == null) { + serializedStateInternal = ThrowingEx.get(() -> LazyForwardingEquality.toBytes(state())); + } + return serializedStateInternal; + } +} diff --git a/lib/src/main/java/com/diffplug/spotless/FormatterStepSerializationRoundtrip.java b/lib/src/main/java/com/diffplug/spotless/FormatterStepSerializationRoundtrip.java new file mode 100644 index 0000000000..3af89083fc --- /dev/null +++ b/lib/src/main/java/com/diffplug/spotless/FormatterStepSerializationRoundtrip.java @@ -0,0 +1,72 @@ +/* + * Copyright 2023-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.IOException; +import java.io.ObjectStreamException; +import java.io.Serializable; + +import edu.umd.cs.findbugs.annotations.Nullable; + +class FormatterStepSerializationRoundtrip extends FormatterStepEqualityOnStateSerialization { + private static final long serialVersionUID = 1L; + private final String name; + private final transient ThrowingEx.Supplier initializer; + private @Nullable RoundtripState roundtripStateInternal; + private final SerializedFunction equalityStateExtractor; + private final SerializedFunction equalityStateToFormatter; + + public FormatterStepSerializationRoundtrip(String name, ThrowingEx.Supplier initializer, SerializedFunction equalityStateExtractor, SerializedFunction equalityStateToFormatter) { + this.name = name; + this.initializer = initializer; + this.equalityStateExtractor = equalityStateExtractor; + this.equalityStateToFormatter = equalityStateToFormatter; + } + + @Override + public String getName() { + return name; + } + + @Override + protected EqualityState stateSupplier() throws Exception { + if (roundtripStateInternal == null) { + roundtripStateInternal = initializer.get(); + } + return equalityStateExtractor.apply(roundtripStateInternal); + } + + @Override + protected FormatterFunc stateToFormatter(EqualityState equalityState) throws Exception { + return equalityStateToFormatter.apply(equalityState); + } + + // override serialize output + private void writeObject(java.io.ObjectOutputStream out) throws IOException { + if (roundtripStateInternal == null) { + roundtripStateInternal = ThrowingEx.get(initializer::get); + } + out.defaultWriteObject(); + } + + private void readObject(java.io.ObjectInputStream in) throws IOException, ClassNotFoundException { + in.defaultReadObject(); + } + + private void readObjectNoData() throws ObjectStreamException { + throw new UnsupportedOperationException(); + } +} diff --git a/lib/src/main/java/com/diffplug/spotless/JarState.java b/lib/src/main/java/com/diffplug/spotless/JarState.java index 48686c49f8..8680932b9e 100644 --- a/lib/src/main/java/com/diffplug/spotless/JarState.java +++ b/lib/src/main/java/com/diffplug/spotless/JarState.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. @@ -17,6 +17,7 @@ import java.io.File; import java.io.IOException; +import java.io.ObjectStreamException; import java.io.Serializable; import java.net.URI; import java.net.URL; @@ -25,7 +26,6 @@ import java.util.NoSuchElementException; import java.util.Objects; import java.util.Set; -import java.util.TreeSet; import java.util.stream.Collectors; /** @@ -37,14 +37,54 @@ * catch changes in a SNAPSHOT version. */ public final class JarState implements Serializable { + /** A lazily evaluated JarState, which becomes a set of files when serialized. */ + public static class Promised implements Serializable { + private static final long serialVersionUID = 1L; + private final transient ThrowingEx.Supplier supplier; + private FileSignature.Promised cached; + + public Promised(ThrowingEx.Supplier supplier) { + this.supplier = supplier; + } + + public JarState get() { + try { + if (cached == null) { + JarState result = supplier.get(); + cached = result.fileSignature.asPromise(); + return result; + } + return new JarState(cached.get()); + } catch (Exception e) { + throw ThrowingEx.asRuntime(e); + } + } + + // override serialize output + private void writeObject(java.io.ObjectOutputStream out) + throws IOException { + get(); + out.defaultWriteObject(); + } + + private void readObject(java.io.ObjectInputStream in) throws IOException, ClassNotFoundException { + in.defaultReadObject(); + } + + private void readObjectNoData() throws ObjectStreamException { + throw new UnsupportedOperationException(); + } + } + + public static Promised promise(ThrowingEx.Supplier supplier) { + return new Promised(supplier); + } + private static final long serialVersionUID = 1L; - @Deprecated - private final Set mavenCoordinates; private final FileSignature fileSignature; - private JarState(Collection mavenCoordinates, FileSignature fileSignature) { - this.mavenCoordinates = new TreeSet<>(mavenCoordinates); + private JarState(FileSignature fileSignature) { this.fileSignature = fileSignature; } @@ -71,13 +111,13 @@ private static JarState provisionWithTransitives(boolean withTransitives, Collec throw new NoSuchElementException("Resolved to an empty result: " + mavenCoordinates.stream().collect(Collectors.joining(", "))); } FileSignature fileSignature = FileSignature.signAsSet(jars); - return new JarState(mavenCoordinates, fileSignature); + return new JarState(fileSignature); } /** Wraps the given collection of a files as a JarState, maintaining the order in the Collection. */ public static JarState preserveOrder(Collection jars) throws IOException { FileSignature fileSignature = FileSignature.signAsList(jars); - return new JarState(Collections.emptySet(), fileSignature); + return new JarState(fileSignature); } URL[] jarUrls() { @@ -107,10 +147,4 @@ public ClassLoader getClassLoader() { public ClassLoader getClassLoader(Serializable key) { return SpotlessCache.instance().classloader(key, this); } - - /** Returns unmodifiable view on sorted Maven coordinates */ - @Deprecated - public Set getMavenCoordinates() { - return Collections.unmodifiableSet(mavenCoordinates); - } } diff --git a/lib/src/main/java/com/diffplug/spotless/SerializedFunction.java b/lib/src/main/java/com/diffplug/spotless/SerializedFunction.java new file mode 100644 index 0000000000..d4d36edfb4 --- /dev/null +++ b/lib/src/main/java/com/diffplug/spotless/SerializedFunction.java @@ -0,0 +1,25 @@ +/* + * Copyright 2023-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.Serializable; + +@FunctionalInterface +public interface SerializedFunction extends Serializable, ThrowingEx.Function { + static SerializedFunction identity() { + return t -> t; + } +} diff --git a/lib/src/main/java/com/diffplug/spotless/generic/IndentStep.java b/lib/src/main/java/com/diffplug/spotless/generic/IndentStep.java index 4281df736c..91155b1a79 100644 --- a/lib/src/main/java/com/diffplug/spotless/generic/IndentStep.java +++ b/lib/src/main/java/com/diffplug/spotless/generic/IndentStep.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2022 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,18 +16,24 @@ package com.diffplug.spotless.generic; import java.io.Serializable; -import java.util.Objects; import com.diffplug.spotless.FormatterFunc; import com.diffplug.spotless.FormatterStep; +import com.diffplug.spotless.SerializedFunction; /** Simple step which checks for consistent indentation characters. */ -public final class IndentStep { +public final class IndentStep implements Serializable { + private static final long serialVersionUID = 1L; - private static final int DEFAULT_NUM_SPACES_PER_TAB = 4; + final Type type; + final int numSpacesPerTab; - // prevent direct instantiation - private IndentStep() {} + private IndentStep(Type type, int numSpacesPerTab) { + this.type = type; + this.numSpacesPerTab = numSpacesPerTab; + } + + private static final int DEFAULT_NUM_SPACES_PER_TAB = 4; public enum Type { TAB, SPACE; @@ -49,32 +55,21 @@ public FormatterStep create(int numSpacesPerTab) { /** Creates a step which will indent with the given type of whitespace, converting between tabs and spaces at the given ratio. */ public static FormatterStep create(Type type, int numSpacesPerTab) { - Objects.requireNonNull(type, "type"); return FormatterStep.create("indentWith" + type.tabSpace("Tabs", "Spaces"), - new State(type, numSpacesPerTab), State::toFormatter); + new IndentStep(type, numSpacesPerTab), SerializedFunction.identity(), + IndentStep::startFormatting); } - private static class State implements Serializable { - private static final long serialVersionUID = 1L; - - final Type type; - final int numSpacesPerTab; - - State(Type type, int numSpacesPerTab) { - this.type = type; - this.numSpacesPerTab = numSpacesPerTab; - } - - FormatterFunc toFormatter() { - return new Runtime(this)::format; - } + private FormatterFunc startFormatting() { + var runtime = new Runtime(this); + return runtime::format; } static class Runtime { - final State state; + final IndentStep state; final StringBuilder builder = new StringBuilder(); - Runtime(State state) { + Runtime(IndentStep state) { this.state = state; } diff --git a/lib/src/main/java/com/diffplug/spotless/kotlin/DiktatStep.java b/lib/src/main/java/com/diffplug/spotless/kotlin/DiktatStep.java index afd479b4c5..425786d64e 100644 --- a/lib/src/main/java/com/diffplug/spotless/kotlin/DiktatStep.java +++ b/lib/src/main/java/com/diffplug/spotless/kotlin/DiktatStep.java @@ -1,5 +1,5 @@ /* - * Copyright 2021-2023 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. @@ -25,11 +25,20 @@ import com.diffplug.spotless.*; -/** Wraps up diktat as a FormatterStep. */ -public class DiktatStep { - - // prevent direct instantiation - private DiktatStep() {} +/** Wraps up diktat as a FormatterStep. */ +public class DiktatStep implements Serializable { + private static final long serialVersionUID = 1L; + private final JarState.Promised jarState; + private final String versionDiktat; + private final boolean isScript; + private final @Nullable FileSignature.Promised config; + + private DiktatStep(JarState.Promised jarState, String versionDiktat, boolean isScript, @Nullable FileSignature config) { + this.jarState = jarState; + this.versionDiktat = versionDiktat; + this.isScript = isScript; + this.config = config != null ? config.asPromise() : null; + } private static final String MIN_SUPPORTED_VERSION = "1.2.1"; @@ -62,29 +71,32 @@ public static FormatterStep create(String versionDiktat, Provisioner provisioner } Objects.requireNonNull(versionDiktat, "versionDiktat"); Objects.requireNonNull(provisioner, "provisioner"); - return FormatterStep.createLazy(NAME, - () -> new DiktatStep.State(versionDiktat, provisioner, isScript, config), - DiktatStep.State::createFormat); + final String diktatCoordinate; + if (BadSemver.version(versionDiktat) >= BadSemver.version(PACKAGE_RELOCATED_VERSION)) { + diktatCoordinate = MAVEN_COORDINATE + versionDiktat; + } else { + diktatCoordinate = MAVEN_COORDINATE_PRE_2_0_0 + versionDiktat; + } + return FormatterStep.create(NAME, + new DiktatStep(JarState.promise(() -> JarState.from(diktatCoordinate, provisioner)), versionDiktat, isScript, config), + DiktatStep::equalityState, State::createFormat); } - static final class State implements Serializable { + private State equalityState() throws Exception { + return new State(jarState.get(), versionDiktat, isScript, config != null ? config.get() : null); + } + static final class State implements Serializable { private static final long serialVersionUID = 1L; - private final String versionDiktat; + final JarState jar; + final String versionDiktat; /** Are the files being linted Kotlin script files. */ private final boolean isScript; private final @Nullable FileSignature config; - final JarState jar; - State(String versionDiktat, Provisioner provisioner, boolean isScript, @Nullable FileSignature config) throws IOException { - final String diktatCoordinate; - if (BadSemver.version(versionDiktat) >= BadSemver.version(PACKAGE_RELOCATED_VERSION)) { - diktatCoordinate = MAVEN_COORDINATE + versionDiktat; - } else { - diktatCoordinate = MAVEN_COORDINATE_PRE_2_0_0 + versionDiktat; - } - this.jar = JarState.from(diktatCoordinate, provisioner); + State(JarState jar, String versionDiktat, boolean isScript, @Nullable FileSignature config) throws IOException { + this.jar = jar; this.versionDiktat = versionDiktat; this.isScript = isScript; this.config = config; diff --git a/testlib/src/main/java/com/diffplug/spotless/SerializableEqualityTester.java b/testlib/src/main/java/com/diffplug/spotless/SerializableEqualityTester.java index 096edf62e2..726597a323 100644 --- a/testlib/src/main/java/com/diffplug/spotless/SerializableEqualityTester.java +++ b/testlib/src/main/java/com/diffplug/spotless/SerializableEqualityTester.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. @@ -71,7 +71,7 @@ public void areDifferentThan() { } @SuppressWarnings("unchecked") - private static T reserialize(T input) { + static T reserialize(T input) { byte[] asBytes = LazyForwardingEquality.toBytes(input); ByteArrayInputStream byteInput = new ByteArrayInputStream(asBytes); try (ObjectInputStream objectInput = new ObjectInputStream(byteInput)) { diff --git a/testlib/src/main/java/com/diffplug/spotless/StepHarness.java b/testlib/src/main/java/com/diffplug/spotless/StepHarness.java index c611cc738a..0f6ddc9ad9 100644 --- a/testlib/src/main/java/com/diffplug/spotless/StepHarness.java +++ b/testlib/src/main/java/com/diffplug/spotless/StepHarness.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. @@ -21,17 +21,14 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Paths; import java.util.Arrays; -import java.util.Objects; import org.assertj.core.api.AbstractStringAssert; import org.assertj.core.api.Assertions; /** An api for testing a {@code FormatterStep} that doesn't depend on the File path. DO NOT ADD FILE SUPPORT TO THIS, use {@link StepHarnessWithFile} if you need that. */ -public class StepHarness implements AutoCloseable { - private final Formatter formatter; - +public class StepHarness extends StepHarnessBase { private StepHarness(Formatter formatter) { - this.formatter = Objects.requireNonNull(formatter); + super(formatter); } /** Creates a harness for testing steps which don't depend on the file. */ @@ -57,14 +54,14 @@ public static StepHarness forFormatter(Formatter formatter) { /** Asserts that the given element is transformed as expected, and that the result is idempotent. */ public StepHarness test(String before, String after) { - String actual = formatter.compute(LineEnding.toUnix(before), new File("")); + String actual = formatter().compute(LineEnding.toUnix(before), new File("")); assertEquals(after, actual, "Step application failed"); return testUnaffected(after); } /** Asserts that the given element is idempotent w.r.t the step under test. */ public StepHarness testUnaffected(String idempotentElement) { - String actual = formatter.compute(LineEnding.toUnix(idempotentElement), new File("")); + String actual = formatter().compute(LineEnding.toUnix(idempotentElement), new File("")); assertEquals(idempotentElement, actual, "Step is not idempotent"); return this; } @@ -88,7 +85,7 @@ public AbstractStringAssert testResourceExceptionMsg(String resourceBefore) { public AbstractStringAssert testExceptionMsg(String before) { try { - formatter.compute(LineEnding.toUnix(before), Formatter.NO_FILE_SENTINEL); + formatter().compute(LineEnding.toUnix(before), Formatter.NO_FILE_SENTINEL); throw new SecurityException("Expected exception"); } catch (Throwable e) { if (e instanceof SecurityException) { @@ -105,9 +102,4 @@ public AbstractStringAssert testExceptionMsg(String before) { } } } - - @Override - public void close() { - formatter.close(); - } } diff --git a/testlib/src/main/java/com/diffplug/spotless/StepHarnessBase.java b/testlib/src/main/java/com/diffplug/spotless/StepHarnessBase.java new file mode 100644 index 0000000000..8624972c4e --- /dev/null +++ b/testlib/src/main/java/com/diffplug/spotless/StepHarnessBase.java @@ -0,0 +1,69 @@ +/* + * Copyright 2023-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.util.Objects; + +import org.assertj.core.api.Assertions; + +class StepHarnessBase> implements AutoCloseable { + private final Formatter formatter; + private Formatter roundTripped; + private boolean supportsRoundTrip = false; + + protected StepHarnessBase(Formatter formatter) { + this.formatter = Objects.requireNonNull(formatter); + if (formatter.getSteps().size() == 1) { + // our goal is for everything to be roundtrip serializable + // the steps to get there are + // - make every individual step round-trippable + // - make the other machinery (Formatter, LineEnding, etc) round-trippable + // - done! + // + // Right now, we're still trying to get each and every single step to be round-trippable. + // You can help by add a test below, make sure that the test for that step fails, and then + // make the test pass. `FormatterStepEqualityOnStateSerialization` is a good base class for + // easily converting a step to round-trip serialization while maintaining easy and concise + // equality code. + String onlyStepName = formatter.getSteps().get(0).getName(); + if (onlyStepName.startsWith("indentWith")) { + supportsRoundTrip = true; + } else if (onlyStepName.equals("diktat")) { + supportsRoundTrip = true; + } + } + } + + protected Formatter formatter() { + if (!supportsRoundTrip) { + return formatter; + } else { + if (roundTripped == null) { + roundTripped = SerializableEqualityTester.reserialize(formatter); + Assertions.assertThat(roundTripped).isEqualTo(formatter); + } + return roundTripped; + } + } + + @Override + public void close() { + formatter.close(); + if (roundTripped != null) { + roundTripped.close(); + } + } +} diff --git a/testlib/src/main/java/com/diffplug/spotless/StepHarnessWithFile.java b/testlib/src/main/java/com/diffplug/spotless/StepHarnessWithFile.java index 9a9eb4042b..ae2f22db1e 100644 --- a/testlib/src/main/java/com/diffplug/spotless/StepHarnessWithFile.java +++ b/testlib/src/main/java/com/diffplug/spotless/StepHarnessWithFile.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. @@ -26,13 +26,12 @@ import org.assertj.core.api.Assertions; /** An api for testing a {@code FormatterStep} that depends on the File path. */ -public class StepHarnessWithFile implements AutoCloseable { - private final Formatter formatter; +public class StepHarnessWithFile extends StepHarnessBase { private final ResourceHarness harness; private StepHarnessWithFile(ResourceHarness harness, Formatter formatter) { + super(formatter); this.harness = Objects.requireNonNull(harness); - this.formatter = Objects.requireNonNull(formatter); } /** Creates a harness for testing steps which do depend on the file. */ @@ -53,19 +52,25 @@ public static StepHarnessWithFile forFormatter(ResourceHarness harness, Formatte } /** Asserts that the given element is transformed as expected, and that the result is idempotent. */ - public StepHarnessWithFile test(File file, String before, String after) { - String actual = formatter.compute(LineEnding.toUnix(before), file); + public StepHarnessWithFile test(String filename, String before, String after) { + File file = harness.setFile(filename).toContent(before); + String actual = formatter().compute(LineEnding.toUnix(before), file); assertEquals(after, actual, "Step application failed"); return testUnaffected(file, after); } /** Asserts that the given element is idempotent w.r.t the step under test. */ public StepHarnessWithFile testUnaffected(File file, String idempotentElement) { - String actual = formatter.compute(LineEnding.toUnix(idempotentElement), file); + String actual = formatter().compute(LineEnding.toUnix(idempotentElement), file); assertEquals(idempotentElement, actual, "Step is not idempotent"); return this; } + /** Asserts that the given element is idempotent w.r.t the step under test. */ + public StepHarnessWithFile testUnaffected(String file, String idempotentElement) { + return testUnaffected(harness.setFile(file).toContent(idempotentElement), idempotentElement); + } + /** Asserts that the given elements in the resources directory are transformed as expected. */ public StepHarnessWithFile testResource(String resourceBefore, String resourceAfter) { return testResource(resourceBefore, resourceBefore, resourceAfter); @@ -73,8 +78,7 @@ public StepHarnessWithFile testResource(String resourceBefore, String resourceAf public StepHarnessWithFile testResource(String filename, String resourceBefore, String resourceAfter) { String contentBefore = ResourceHarness.getTestResource(resourceBefore); - File file = harness.setFile(filename).toContent(contentBefore); - return test(file, contentBefore, ResourceHarness.getTestResource(resourceAfter)); + return test(filename, contentBefore, ResourceHarness.getTestResource(resourceAfter)); } /** Asserts that the given elements in the resources directory are transformed as expected. */ @@ -96,7 +100,7 @@ public AbstractStringAssert testResourceExceptionMsg(String filename, String public AbstractStringAssert testExceptionMsg(File file, String before) { try { - formatter.compute(LineEnding.toUnix(before), file); + formatter().compute(LineEnding.toUnix(before), file); throw new SecurityException("Expected exception"); } catch (Throwable e) { if (e instanceof SecurityException) { @@ -110,9 +114,4 @@ public AbstractStringAssert testExceptionMsg(File file, String before) { } } } - - @Override - public void close() { - formatter.close(); - } } diff --git a/testlib/src/test/java/com/diffplug/spotless/generic/LicenseHeaderStepTest.java b/testlib/src/test/java/com/diffplug/spotless/generic/LicenseHeaderStepTest.java index 92c6794643..b5e08d2232 100644 --- a/testlib/src/test/java/com/diffplug/spotless/generic/LicenseHeaderStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/generic/LicenseHeaderStepTest.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. @@ -268,7 +268,7 @@ void should_preserve_year_for_license_with_address() throws Throwable { void should_apply_license_containing_filename_token() throws Exception { FormatterStep step = LicenseHeaderStep.headerDelimiter(header(HEADER_WITH_$FILE), package_).build(); StepHarnessWithFile.forStep(this, step) - .test(new File("Test.java"), getTestResource(FILE_NO_LICENSE), hasHeaderFileName(HEADER_WITH_$FILE, "Test.java")) + .test("Test.java", getTestResource(FILE_NO_LICENSE), hasHeaderFileName(HEADER_WITH_$FILE, "Test.java")) .testUnaffected(new File("Test.java"), hasHeaderFileName(HEADER_WITH_$FILE, "Test.java")); } @@ -276,8 +276,7 @@ void should_apply_license_containing_filename_token() throws Exception { void should_update_license_containing_filename_token() throws Exception { FormatterStep step = LicenseHeaderStep.headerDelimiter(header(HEADER_WITH_$FILE), package_).build(); StepHarnessWithFile.forStep(this, step) - .test( - new File("After.java"), + .test("After.java", hasHeaderFileName(HEADER_WITH_$FILE, "Before.java"), hasHeaderFileName(HEADER_WITH_$FILE, "After.java")); } @@ -286,12 +285,10 @@ void should_update_license_containing_filename_token() throws Exception { void should_apply_license_containing_YEAR_filename_token() throws Exception { FormatterStep step = LicenseHeaderStep.headerDelimiter(header(HEADER_WITH_$YEAR_$FILE), package_).build(); StepHarnessWithFile.forStep(this, step) - .test( - new File("Test.java"), + .test("Test.java", getTestResource(FILE_NO_LICENSE), hasHeaderYearFileName(HEADER_WITH_$YEAR_$FILE, currentYear(), "Test.java")) - .testUnaffected( - new File("Test.java"), + .testUnaffected("Test.java", hasHeaderYearFileName(HEADER_WITH_$YEAR_$FILE, currentYear(), "Test.java")); } diff --git a/testlib/src/test/java/com/diffplug/spotless/npm/EslintFormatterStepTest.java b/testlib/src/test/java/com/diffplug/spotless/npm/EslintFormatterStepTest.java index 631a4beaa7..ab54b11bee 100644 --- a/testlib/src/test/java/com/diffplug/spotless/npm/EslintFormatterStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/npm/EslintFormatterStepTest.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. @@ -56,7 +56,6 @@ void formattingUsingRulesetsFile(String ruleSetName) throws Exception { // final File eslintRc = setFile(buildDir().getPath() + "/.eslintrc.js").toResource(filedir + ".eslintrc.js"); final String dirtyFile = filedir + "javascript-es6.dirty"; - File dirtyFileFile = setFile(testDir + "test.js").toResource(dirtyFile); final String cleanFile = filedir + "javascript-es6.clean"; final FormatterStep formatterStep = EslintFormatterStep.create( @@ -69,7 +68,7 @@ void formattingUsingRulesetsFile(String ruleSetName) throws Exception { new EslintConfig(eslintRc, null)); try (StepHarnessWithFile stepHarness = StepHarnessWithFile.forStep(this, formatterStep)) { - stepHarness.test(dirtyFileFile, ResourceHarness.getTestResource(dirtyFile), ResourceHarness.getTestResource(cleanFile)); + stepHarness.test("test.js", ResourceHarness.getTestResource(dirtyFile), ResourceHarness.getTestResource(cleanFile)); } } } @@ -100,7 +99,6 @@ void formattingUsingRulesetsFile(String ruleSetName) throws Exception { tsconfigFile = setFile(testDir + "tsconfig.json").toResource(filedir + "tsconfig.json"); } final String dirtyFile = filedir + "typescript.dirty"; - File dirtyFileFile = setFile(testDir + "test.ts").toResource(dirtyFile); final String cleanFile = filedir + "typescript.clean"; final FormatterStep formatterStep = EslintFormatterStep.create( @@ -113,7 +111,7 @@ void formattingUsingRulesetsFile(String ruleSetName) throws Exception { new EslintTypescriptConfig(eslintRc, null, tsconfigFile)); try (StepHarnessWithFile stepHarness = StepHarnessWithFile.forStep(this, formatterStep)) { - stepHarness.test(dirtyFileFile, ResourceHarness.getTestResource(dirtyFile), ResourceHarness.getTestResource(cleanFile)); + stepHarness.test(testDir + "test.ts", ResourceHarness.getTestResource(dirtyFile), ResourceHarness.getTestResource(cleanFile)); } } } @@ -158,7 +156,6 @@ void formattingUsingInlineXoConfig() throws Exception { File tsconfigFile = setFile(testDir + "tsconfig.json").toResource(filedir + "tsconfig.json"); final String dirtyFile = filedir + "typescript.dirty"; - File dirtyFileFile = setFile(testDir + "test.ts").toResource(dirtyFile); final String cleanFile = filedir + "typescript.clean"; final FormatterStep formatterStep = EslintFormatterStep.create( @@ -171,7 +168,7 @@ void formattingUsingInlineXoConfig() throws Exception { new EslintTypescriptConfig(null, esLintConfig, tsconfigFile)); try (StepHarnessWithFile stepHarness = StepHarnessWithFile.forStep(this, formatterStep)) { - stepHarness.test(dirtyFileFile, ResourceHarness.getTestResource(dirtyFile), ResourceHarness.getTestResource(cleanFile)); + stepHarness.test(testDir + "test.ts", ResourceHarness.getTestResource(dirtyFile), ResourceHarness.getTestResource(cleanFile)); } } }