Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Start making each FormatterStep roundtrip serializable. #1945

Merged
merged 27 commits into from
Jan 23, 2024
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
eb0df83
StepHarness (and WithFile) now have a `supportsRoundTrip` method so w…
nedtwigg Dec 4, 2023
e3843cd
`StepHarnessBase` is now embarked on a methodical journey to make eve…
nedtwigg Dec 4, 2023
cb346a4
Introduce a new `FormatterStep` base class for the serialized roundtr…
nedtwigg Dec 4, 2023
decbd1b
Convert `IndentStep` to be roundtrip serializable.
nedtwigg Dec 4, 2023
317b2ac
Remove the long-deprecated and completely unused `Set<String> mavenCo…
nedtwigg Dec 4, 2023
e4d6c49
Introduce `FileSignature.RoundTrippable` and `JarState.Promised` so t…
nedtwigg Dec 5, 2023
6f7edd3
Make `DiktatStep` round-trippable.
nedtwigg Dec 5, 2023
9fa1da3
Fix spotbugs warnings.
nedtwigg Dec 5, 2023
cce20d4
Rename `FileSignature.RoundTrippable` to `FileSignature.Promised` to …
nedtwigg Dec 5, 2023
56bb67e
More improvements to `StepHarnessWithFile`.
nedtwigg Dec 6, 2023
c56968a
Add a roundtrip-serializable method to `FormatterStep`.
nedtwigg Dec 6, 2023
46635c6
Fixup the `FormatterStep` api.
nedtwigg Dec 6, 2023
f5f6836
Use the new API.
nedtwigg Dec 6, 2023
44b3126
Fixup spotbugs warnings.
nedtwigg Dec 6, 2023
55481fa
`FileSignature.Promised` and `JarState.Promised` should both have `ge…
nedtwigg Dec 6, 2023
ab1452d
Streamline the `FileSignature` api.
nedtwigg Dec 6, 2023
4e54fc2
Update the `CONTRIBUTING.md` for our new roundtrip serializable world.
nedtwigg Dec 6, 2023
5a05beb
Merge branch 'main' into serializable-refactor
nedtwigg Dec 15, 2023
005113e
Remove unneeded `public` declarations.
nedtwigg Dec 15, 2023
ce34f89
Fix various typos found by @jbduncan
nedtwigg Jan 23, 2024
d102709
merge main (DiktatStep conflict)
nedtwigg Jan 23, 2024
3bd4243
Copyright updates.
nedtwigg Jan 23, 2024
19aff77
Merge branch 'main' into serializable-refactor
nedtwigg Jan 23, 2024
4fdb178
Remove unused method.
nedtwigg Jan 23, 2024
14991e4
Update changelog.
nedtwigg Jan 23, 2024
4b15b39
Fix an oversight from the merge conflict earlier.
nedtwigg Jan 23, 2024
93eb099
Merge branch 'main' into serializable-refactor
nedtwigg Jan 23, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2016-2021 DiffPlug
* 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.
Expand All @@ -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;
Expand Down Expand Up @@ -187,12 +186,6 @@ public Properties getPreferences() {
return preferences.getProperties();
}

/** Returns first coordinate from sorted set that starts with a given prefix.*/
public Optional<String> getMavenCoordinate(String prefix) {
return jarState.getMavenCoordinates().stream()
.filter(coordinate -> coordinate.startsWith(prefix)).findFirst();
}

nedtwigg marked this conversation as resolved.
Show resolved Hide resolved
/**
* Load class based on the given configuration of JAR provider and Maven coordinates.
* Different class loader instances are provided in the following scenarios:
Expand Down
34 changes: 33 additions & 1 deletion lib/src/main/java/com/diffplug/spotless/FileSignature.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2016-2021 DiffPlug
* 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.
Expand Down Expand Up @@ -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. */
Expand All @@ -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<File> files;
Expand Down Expand Up @@ -93,6 +96,35 @@ private FileSignature(final List<File> 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<File> files;
nedtwigg marked this conversation as resolved.
Show resolved Hide resolved
@SuppressFBWarnings("SE_TRANSIENT_FIELD_NOT_RESTORED")
private transient @Nullable FileSignature cached;

private Promised(List<File> 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 static Promised promise(Iterable<File> files) {
return new Promised(MoreIterables.toNullHostileList(files), null);
}
nedtwigg marked this conversation as resolved.
Show resolved Hide resolved

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<File> files() {
return Collections.unmodifiableList(files);
Expand Down
2 changes: 2 additions & 0 deletions lib/src/main/java/com/diffplug/spotless/Formatter.java
Original file line number Diff line number Diff line change
Expand Up @@ -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();
nedtwigg marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
Expand Down
45 changes: 45 additions & 0 deletions lib/src/main/java/com/diffplug/spotless/FormatterStep.java
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,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.
nedtwigg marked this conversation as resolved.
Show resolved Hide resolved
*/
static <RoundtripState extends Serializable, EqualityState extends Serializable> FormatterStep createLazy(
String name,
ThrowingEx.Supplier<RoundtripState> roundtripInit,
SerializedFunction<RoundtripState, EqualityState> equalityFunc,
SerializedFunction<EqualityState, FormatterFunc> 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.
nedtwigg marked this conversation as resolved.
Show resolved Hide resolved
*/
static <RoundtripState extends Serializable, EqualityState extends Serializable> FormatterStep create(
String name,
RoundtripState roundTrip,
SerializedFunction<RoundtripState, EqualityState> equalityFunc,
SerializedFunction<EqualityState, FormatterFunc> formatterFunc) {
return createLazy(name, () -> roundTrip, equalityFunc, formatterFunc);
}

/**
* @param name
* The name of the formatter step
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
/*
* 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;

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<State extends Serializable> 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;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/*
* 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;

import java.io.IOException;
import java.io.ObjectStreamException;
import java.io.Serializable;

import edu.umd.cs.findbugs.annotations.Nullable;

class FormatterStepSerializationRoundtrip<RoundtripState extends Serializable, EqualityState extends Serializable> extends FormatterStepEqualityOnStateSerialization<EqualityState> {
private static final long serialVersionUID = 1L;
private final String name;
private final transient ThrowingEx.Supplier<RoundtripState> initializer;
private @Nullable RoundtripState roundtripStateInternal;
private final SerializedFunction<RoundtripState, EqualityState> equalityStateExtractor;
private final SerializedFunction<EqualityState, FormatterFunc> equalityStateToFormatter;

public FormatterStepSerializationRoundtrip(String name, ThrowingEx.Supplier<RoundtripState> initializer, SerializedFunction<RoundtripState, EqualityState> equalityStateExtractor, SerializedFunction<EqualityState, FormatterFunc> 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();
}
}
Loading
Loading