From f6928642ec38c4fdaadade3bee6221c2d5e549d3 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sun, 15 Jan 2017 16:15:10 -0800 Subject: [PATCH 01/11] Renamed ThrowingEx.unwrapAsRuntimeOrRethrow to unwrapCause. As discussed at https://github.com/diffplug/spotless/commit/d8f5eeec376fb6a918bb2f5835e2c299739a0d47#commitcomment-20480600 --- lib/src/main/java/com/diffplug/spotless/ThrowingEx.java | 4 ++-- .../main/java/com/diffplug/spotless/kotlin/KtLintStep.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/ThrowingEx.java b/lib/src/main/java/com/diffplug/spotless/ThrowingEx.java index c960ce776d..be7ac4b91c 100644 --- a/lib/src/main/java/com/diffplug/spotless/ThrowingEx.java +++ b/lib/src/main/java/com/diffplug/spotless/ThrowingEx.java @@ -94,11 +94,11 @@ public static RuntimeException asRuntime(Exception e) { * try { * doSomething(); * } catch (Throwable e) { - * throw unwrapAsRuntimeOrRethrow(e); + * throw unwrapCause(e); * } * ``` */ - public static RuntimeException unwrapAsRuntimeOrRethrow(Throwable e) { + public static RuntimeException unwrapCause(Throwable e) { Throwable cause = e.getCause(); if (cause == null) { return ifErrorRethrowElseAsRuntime(e); diff --git a/lib/src/main/java/com/diffplug/spotless/kotlin/KtLintStep.java b/lib/src/main/java/com/diffplug/spotless/kotlin/KtLintStep.java index 2295205dcc..31ddef5473 100644 --- a/lib/src/main/java/com/diffplug/spotless/kotlin/KtLintStep.java +++ b/lib/src/main/java/com/diffplug/spotless/kotlin/KtLintStep.java @@ -97,7 +97,7 @@ FormatterFunc createFormat() throws Exception { String formatted = (String) formatterMethod.invoke(ktlint, input, ruleSets, formatterCallback); return formatted; } catch (InvocationTargetException e) { - throw ThrowingEx.unwrapAsRuntimeOrRethrow(e); + throw ThrowingEx.unwrapCause(e); } }; } From 1b8483e69b4e972ed2900d50d194e8a27de17e16 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sun, 15 Jan 2017 16:21:18 -0800 Subject: [PATCH 02/11] Added a FormatExceptionPolicy so that Formatter can become more strict about exceptions in its formatters. Default behavior matches the existing behavior. --- .../spotless/FormatExceptionPolicy.java | 41 ++++++++++++++++++ .../spotless/FormatExceptionPolicyLegacy.java | 42 +++++++++++++++++++ .../java/com/diffplug/spotless/Formatter.java | 30 ++++++++----- 3 files changed, 102 insertions(+), 11 deletions(-) create mode 100644 lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicy.java create mode 100644 lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyLegacy.java diff --git a/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicy.java b/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicy.java new file mode 100644 index 0000000000..c048f60869 --- /dev/null +++ b/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicy.java @@ -0,0 +1,41 @@ +/* + * Copyright 2016 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.nio.file.Path; + +/** A policy for handling exceptions in the format. */ +public interface FormatExceptionPolicy extends Serializable { + /** Called for every error in the formatter. */ + void handleError(Throwable e, FormatterStep step, File file, Path rootDir); + + /** + * Returns a byte array representation of everything inside this `FormatExceptionPolicy`. + * + * The main purpose of this method is to ensure one can't instantiate this class with lambda + * expressions, which are notoriously difficult to serialize and deserialize properly. + */ + public byte[] toBytes(); + + /** + * A policy which rethrows subclasses of `Error` and logs other kinds of Exception. + */ + public static FormatExceptionPolicy failOnlyOnError() { + return new FormatExceptionPolicyLegacy(); + } +} diff --git a/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyLegacy.java b/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyLegacy.java new file mode 100644 index 0000000000..f87e4b1af3 --- /dev/null +++ b/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyLegacy.java @@ -0,0 +1,42 @@ +/* + * Copyright 2016 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.nio.file.Path; +import java.util.logging.Level; +import java.util.logging.Logger; + +class FormatExceptionPolicyLegacy implements FormatExceptionPolicy { + private static final long serialVersionUID = 1L; + + private static final Logger logger = Logger.getLogger(Formatter.class.getName()); + + @Override + public void handleError(Throwable e, FormatterStep step, File file, Path rootDir) { + if (e instanceof Error) { + logger.severe("Step '" + step.getName() + "' found problem in '" + rootDir.relativize(file.toPath()) + "':\n" + e.getMessage()); + throw ((Error) e); + } else { + logger.log(Level.WARNING, "Unable to apply step '" + step.getName() + "' to '" + rootDir.relativize(file.toPath()), e); + } + } + + @Override + public byte[] toBytes() { + return LazyForwardingEquality.toBytes(this); + } +} diff --git a/lib/src/main/java/com/diffplug/spotless/Formatter.java b/lib/src/main/java/com/diffplug/spotless/Formatter.java index ea322a1d04..827f6e4e21 100644 --- a/lib/src/main/java/com/diffplug/spotless/Formatter.java +++ b/lib/src/main/java/com/diffplug/spotless/Formatter.java @@ -25,8 +25,6 @@ import java.util.Arrays; import java.util.List; import java.util.Objects; -import java.util.logging.Level; -import java.util.logging.Logger; import javax.annotation.Nullable; @@ -36,14 +34,14 @@ public final class Formatter { private final Charset encoding; private final Path rootDir; private final List steps; + private final FormatExceptionPolicy exceptionPolicy; - private static final Logger logger = Logger.getLogger(Formatter.class.getName()); - - private Formatter(LineEnding.Policy lineEndingsPolicy, Charset encoding, Path rootDirectory, List steps) { + private Formatter(LineEnding.Policy lineEndingsPolicy, Charset encoding, Path rootDirectory, List steps, FormatExceptionPolicy exceptionPolicy) { this.lineEndingsPolicy = Objects.requireNonNull(lineEndingsPolicy, "lineEndingsPolicy"); this.encoding = Objects.requireNonNull(encoding, "encoding"); this.rootDir = Objects.requireNonNull(rootDirectory, "rootDir"); this.steps = new ArrayList<>(Objects.requireNonNull(steps, "steps")); + this.exceptionPolicy = Objects.requireNonNull(exceptionPolicy, "exceptionPolicy"); } public LineEnding.Policy getLineEndingsPolicy() { @@ -62,6 +60,10 @@ public List getSteps() { return steps; } + public FormatExceptionPolicy getExceptionPolicy() { + return exceptionPolicy; + } + public static Formatter.Builder builder() { return new Formatter.Builder(); } @@ -72,6 +74,7 @@ public static class Builder { private Charset encoding; private Path rootDir; private List steps; + private FormatExceptionPolicy exceptionPolicy; private Builder() {} @@ -95,8 +98,14 @@ public Builder steps(List steps) { return this; } + public Builder exceptionPolicy(FormatExceptionPolicy exceptionPolicy) { + this.exceptionPolicy = exceptionPolicy; + return this; + } + public Formatter build() { - return new Formatter(lineEndingsPolicy, encoding, rootDir, steps); + return new Formatter(lineEndingsPolicy, encoding, rootDir, steps, + exceptionPolicy == null ? FormatExceptionPolicy.failOnlyOnError() : exceptionPolicy); } } @@ -183,11 +192,8 @@ public String compute(String unix, File file) throws Error { // Should already be unix-only, but some steps might misbehave. unix = LineEnding.toUnix(formatted); } - } catch (Error e) { - logger.severe("Step '" + step.getName() + "' found problem in '" + rootDir.relativize(file.toPath()) + "':\n" + e.getMessage()); - throw e; } catch (Throwable e) { - logger.log(Level.WARNING, "Unable to apply step '" + step.getName() + "' to '" + rootDir.relativize(file.toPath()), e); + exceptionPolicy.handleError(e, step, file, rootDir); } } return unix; @@ -201,6 +207,7 @@ public int hashCode() { result = prime * result + lineEndingsPolicy.hashCode(); result = prime * result + rootDir.hashCode(); result = prime * result + steps.hashCode(); + result = prime * result + exceptionPolicy.hashCode(); return result; } @@ -219,6 +226,7 @@ public boolean equals(Object obj) { return encoding.equals(other.encoding) && lineEndingsPolicy.equals(other.lineEndingsPolicy) && rootDir.equals(other.rootDir) && - steps.equals(other.steps); + steps.equals(other.steps) && + exceptionPolicy.equals(other.exceptionPolicy); } } From e36ddab627578bdbcc1d40c0c3303f40887f5a66 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sun, 15 Jan 2017 16:38:08 -0800 Subject: [PATCH 03/11] FormatExceptionPolicy implementations need to implement Serializable and equals/hashCode with the same constraints as SerializableFileFilter. Refactored the `toBytes()` method into the interface `NoLambda`, and moved `SerializableFileFilter.EqualityBasedOnSerialization` to `NoLambda.EqualityBasedOnSerialization`. Technically a breaking change, but will affect exactly zero real-world client code. --- .../spotless/FormatExceptionPolicy.java | 2 +- .../spotless/FormatExceptionPolicyLegacy.java | 7 +- .../java/com/diffplug/spotless/NoLambda.java | 71 +++++++++++++++++++ .../spotless/SerializableFileFilter.java | 40 +---------- .../spotless/SerializableFileFilterImpl.java | 2 +- 5 files changed, 75 insertions(+), 47 deletions(-) create mode 100644 lib/src/main/java/com/diffplug/spotless/NoLambda.java diff --git a/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicy.java b/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicy.java index c048f60869..10f8d4c0e5 100644 --- a/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicy.java +++ b/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicy.java @@ -20,7 +20,7 @@ import java.nio.file.Path; /** A policy for handling exceptions in the format. */ -public interface FormatExceptionPolicy extends Serializable { +public interface FormatExceptionPolicy extends Serializable, NoLambda { /** Called for every error in the formatter. */ void handleError(Throwable e, FormatterStep step, File file, Path rootDir); diff --git a/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyLegacy.java b/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyLegacy.java index f87e4b1af3..cb8a40a1f4 100644 --- a/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyLegacy.java +++ b/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyLegacy.java @@ -20,7 +20,7 @@ import java.util.logging.Level; import java.util.logging.Logger; -class FormatExceptionPolicyLegacy implements FormatExceptionPolicy { +class FormatExceptionPolicyLegacy extends NoLambda.EqualityBasedOnSerialization implements FormatExceptionPolicy { private static final long serialVersionUID = 1L; private static final Logger logger = Logger.getLogger(Formatter.class.getName()); @@ -34,9 +34,4 @@ public void handleError(Throwable e, FormatterStep step, File file, Path rootDir logger.log(Level.WARNING, "Unable to apply step '" + step.getName() + "' to '" + rootDir.relativize(file.toPath()), e); } } - - @Override - public byte[] toBytes() { - return LazyForwardingEquality.toBytes(this); - } } diff --git a/lib/src/main/java/com/diffplug/spotless/NoLambda.java b/lib/src/main/java/com/diffplug/spotless/NoLambda.java new file mode 100644 index 0000000000..6ce14d914d --- /dev/null +++ b/lib/src/main/java/com/diffplug/spotless/NoLambda.java @@ -0,0 +1,71 @@ +/* + * Copyright 2016 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; +import java.util.Arrays; + +/** + * Marker interface to prevent lambda implementations of + * single-method interfaces that require serializability. + * + * In order for Spotless to support up-to-date checks, all + * of its parameters must be {@link Serializable} so that + * entries can be written to file, and they must implement + * equals and hashCode correctly. + * + * This interface and its standard implementation, + * {@link EqualityBasedOnSerialization}, are a quick way + * to accomplish these goals. + */ +public interface NoLambda extends Serializable { + /** + * Returns a byte array representation of everything inside this `SerializableFileFilter`. + * + * The main purpose of this method is to ensure one can't instantiate this class with lambda + * expressions, which are notoriously difficult to serialize and deserialize properly. (See + * `SerializableFileFilterImpl.SkipFilesNamed` for an example of how to make a serializable + * subclass.) + */ + public byte[] toBytes(); + + /** An implementation of NoLambda in which equality is based on the serialized representation of itself. */ + public static abstract class EqualityBasedOnSerialization implements NoLambda { + private static final long serialVersionUID = 1733798699224768949L; + + @Override + public byte[] toBytes() { + return LazyForwardingEquality.toBytes(this); + } + + @Override + public int hashCode() { + return Arrays.hashCode(toBytes()); + } + + @Override + public boolean equals(Object otherObj) { + if (otherObj == null) { + return false; + } else if (otherObj.getClass().equals(this.getClass())) { + EqualityBasedOnSerialization other = (EqualityBasedOnSerialization) otherObj; + return Arrays.equals(toBytes(), other.toBytes()); + } else { + return false; + } + } + } +} diff --git a/lib/src/main/java/com/diffplug/spotless/SerializableFileFilter.java b/lib/src/main/java/com/diffplug/spotless/SerializableFileFilter.java index c6325af62c..27561758fe 100644 --- a/lib/src/main/java/com/diffplug/spotless/SerializableFileFilter.java +++ b/lib/src/main/java/com/diffplug/spotless/SerializableFileFilter.java @@ -17,49 +17,11 @@ import java.io.FileFilter; import java.io.Serializable; -import java.util.Arrays; /** A file filter with full support for serialization. */ -public interface SerializableFileFilter extends FileFilter, Serializable { +public interface SerializableFileFilter extends FileFilter, Serializable, NoLambda { /** Creates a FileFilter which will accept all files except files with the given name. */ public static SerializableFileFilter skipFilesNamed(String name) { return new SerializableFileFilterImpl.SkipFilesNamed(name); } - - /** - * Returns a byte array representation of everything inside this `SerializableFileFilter`. - * - * The main purpose of this method is to ensure one can't instantiate this class with lambda - * expressions, which are notoriously difficult to serialize and deserialize properly. (See - * `SerializableFileFilterImpl.SkipFilesNamed` for an example of how to make a serializable - * subclass.) - */ - public byte[] toBytes(); - - /** An implementation of SerializableFileFilter in which equality is based on the serialized representation. */ - public static abstract class EqualityBasedOnSerialization implements SerializableFileFilter { - private static final long serialVersionUID = 1733798699224768949L; - - @Override - public byte[] toBytes() { - return LazyForwardingEquality.toBytes(this); - } - - @Override - public int hashCode() { - return Arrays.hashCode(toBytes()); - } - - @Override - public boolean equals(Object otherObj) { - if (otherObj == null) { - return false; - } else if (otherObj.getClass().equals(this.getClass())) { - EqualityBasedOnSerialization other = (EqualityBasedOnSerialization) otherObj; - return Arrays.equals(toBytes(), other.toBytes()); - } else { - return false; - } - } - } } diff --git a/lib/src/main/java/com/diffplug/spotless/SerializableFileFilterImpl.java b/lib/src/main/java/com/diffplug/spotless/SerializableFileFilterImpl.java index d46aee6e6a..82b935839f 100644 --- a/lib/src/main/java/com/diffplug/spotless/SerializableFileFilterImpl.java +++ b/lib/src/main/java/com/diffplug/spotless/SerializableFileFilterImpl.java @@ -19,7 +19,7 @@ import java.util.Objects; class SerializableFileFilterImpl { - static class SkipFilesNamed extends SerializableFileFilter.EqualityBasedOnSerialization { + static class SkipFilesNamed extends NoLambda.EqualityBasedOnSerialization implements SerializableFileFilter { private static final long serialVersionUID = 1L; private final String nameToSkip; From 56aa1b9c97f54e8dec68454d1ab6825ccadcd75e Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sun, 15 Jan 2017 16:58:34 -0800 Subject: [PATCH 04/11] LineEnding.Policy is now marked with NoLambda, which exposed equality issues we had in the non-GIT_ATTRIBUTES policies. Since GitAttributesLineEndings.Policy uses LazyForwardingEquality, it required LazyForwardingEquality to implement NoLambda. --- .../spotless/LazyForwardingEquality.java | 13 +++++++---- .../com/diffplug/spotless/LineEnding.java | 23 +++++++++++++++---- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/LazyForwardingEquality.java b/lib/src/main/java/com/diffplug/spotless/LazyForwardingEquality.java index 195be858fd..05e758d586 100644 --- a/lib/src/main/java/com/diffplug/spotless/LazyForwardingEquality.java +++ b/lib/src/main/java/com/diffplug/spotless/LazyForwardingEquality.java @@ -31,7 +31,7 @@ * of lazily-computed state. The state's serialized form is used to implement * equals() and hashCode(), so you don't have to. */ -public abstract class LazyForwardingEquality implements Serializable { +public abstract class LazyForwardingEquality implements Serializable, NoLambda { private static final long serialVersionUID = 1L; /** Lazily initialized - null indicates that the state has not yet been set. */ @@ -80,13 +80,18 @@ private void readObjectNoData() throws ObjectStreamException { throw new UnsupportedOperationException(); } + @Override + public byte[] toBytes() { + return toBytes(state()); + } + @Override public final boolean equals(Object other) { if (other == null) { return false; } else if (getClass().equals(other.getClass())) { - Serializable otherState = ((LazyForwardingEquality) other).state(); - return Arrays.equals(toBytes(otherState), toBytes(state())); + LazyForwardingEquality otherCast = (LazyForwardingEquality) other; + return Arrays.equals(otherCast.toBytes(), toBytes()); } else { return false; } @@ -94,7 +99,7 @@ public final boolean equals(Object other) { @Override public final int hashCode() { - return Arrays.hashCode(toBytes(state())); + return Arrays.hashCode(toBytes()); } static byte[] toBytes(Serializable obj) { diff --git a/lib/src/main/java/com/diffplug/spotless/LineEnding.java b/lib/src/main/java/com/diffplug/spotless/LineEnding.java index a05c0ecb01..ea70664e0f 100644 --- a/lib/src/main/java/com/diffplug/spotless/LineEnding.java +++ b/lib/src/main/java/com/diffplug/spotless/LineEnding.java @@ -76,10 +76,25 @@ public Policy createPolicy() { } } - private static final Policy WINDOWS_POLICY = file -> WINDOWS.str(); - private static final Policy UNIX_POLICY = file -> UNIX.str(); + static class ConstantLineEndingPolicy extends NoLambda.EqualityBasedOnSerialization implements Policy { + private static final long serialVersionUID = 1L; + + final String lineEnding; + + ConstantLineEndingPolicy(String lineEnding) { + this.lineEnding = lineEnding; + } + + @Override + public String getEndingFor(File file) { + return lineEnding; + } + } + + private static final Policy WINDOWS_POLICY = new ConstantLineEndingPolicy(WINDOWS.str()); + private static final Policy UNIX_POLICY = new ConstantLineEndingPolicy(UNIX.str()); private static final String _platformNative = System.getProperty("line.separator"); - private static final Policy _platformNativePolicy = file -> _platformNative; + private static final Policy _platformNativePolicy = new ConstantLineEndingPolicy(_platformNative); /** Returns the standard line ending for this policy. */ public String str() { @@ -93,7 +108,7 @@ public String str() { // @formatter:on /** A policy for line endings which can vary based on the specific file being requested. */ - public interface Policy extends Serializable { + public interface Policy extends Serializable, NoLambda { /** Returns the line ending appropriate for the given file. */ String getEndingFor(File file); From 241033c8f7f217a64ba0a375741875e45e679226 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 17 Jan 2017 15:44:30 -0800 Subject: [PATCH 05/11] Formatter now implements Serializable in addition to equals() and hashCode(), which required some serialization tricks because Charset and Path are not serializable. --- .../java/com/diffplug/spotless/Formatter.java | 44 +++++++++-- .../diffplug/spotless/StepEqualityTester.java | 3 +- .../com/diffplug/spotless/FormatterTest.java | 76 +++++++++++++++++++ 3 files changed, 116 insertions(+), 7 deletions(-) create mode 100644 testlib/src/test/java/com/diffplug/spotless/FormatterTest.java diff --git a/lib/src/main/java/com/diffplug/spotless/Formatter.java b/lib/src/main/java/com/diffplug/spotless/Formatter.java index 827f6e4e21..3cab185480 100644 --- a/lib/src/main/java/com/diffplug/spotless/Formatter.java +++ b/lib/src/main/java/com/diffplug/spotless/Formatter.java @@ -17,9 +17,14 @@ import java.io.File; import java.io.IOException; +import java.io.ObjectInputStream; +import java.io.ObjectOutputStream; +import java.io.ObjectStreamException; +import java.io.Serializable; import java.nio.charset.Charset; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.Paths; import java.nio.file.StandardOpenOption; import java.util.ArrayList; import java.util.Arrays; @@ -29,12 +34,14 @@ import javax.annotation.Nullable; /** Formatter which performs the full formatting. */ -public final class Formatter { - private final LineEnding.Policy lineEndingsPolicy; - private final Charset encoding; - private final Path rootDir; - private final List steps; - private final FormatExceptionPolicy exceptionPolicy; +public final class Formatter implements Serializable { + private static final long serialVersionUID = 1L; + + private LineEnding.Policy lineEndingsPolicy; + private Charset encoding; + private Path rootDir; + private List steps; + private FormatExceptionPolicy exceptionPolicy; private Formatter(LineEnding.Policy lineEndingsPolicy, Charset encoding, Path rootDirectory, List steps, FormatExceptionPolicy exceptionPolicy) { this.lineEndingsPolicy = Objects.requireNonNull(lineEndingsPolicy, "lineEndingsPolicy"); @@ -44,6 +51,31 @@ private Formatter(LineEnding.Policy lineEndingsPolicy, Charset encoding, Path ro this.exceptionPolicy = Objects.requireNonNull(exceptionPolicy, "exceptionPolicy"); } + // override serialize output + private void writeObject(ObjectOutputStream out) throws IOException { + out.writeObject(lineEndingsPolicy); + out.writeObject(encoding.name()); + out.writeObject(rootDir.toString()); + out.writeObject(steps); + out.writeObject(exceptionPolicy); + } + + // override serialize input + @SuppressWarnings("unchecked") + private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException { + lineEndingsPolicy = (LineEnding.Policy) in.readObject(); + encoding = Charset.forName((String) in.readObject()); + rootDir = Paths.get((String) in.readObject()); + steps = (List) in.readObject(); + exceptionPolicy = (FormatExceptionPolicy) in.readObject(); + } + + // override serialize input + @SuppressWarnings("unused") + private void readObjectNoData() throws ObjectStreamException { + throw new UnsupportedOperationException(); + } + public LineEnding.Policy getLineEndingsPolicy() { return lineEndingsPolicy; } diff --git a/testlib/src/main/java/com/diffplug/spotless/StepEqualityTester.java b/testlib/src/main/java/com/diffplug/spotless/StepEqualityTester.java index 7ff94b8af7..f528a811b0 100644 --- a/testlib/src/main/java/com/diffplug/spotless/StepEqualityTester.java +++ b/testlib/src/main/java/com/diffplug/spotless/StepEqualityTester.java @@ -15,6 +15,7 @@ */ package com.diffplug.spotless; +import java.io.Serializable; import java.util.ArrayList; import java.util.List; @@ -22,7 +23,7 @@ import com.diffplug.common.testing.EqualsTester; public abstract class StepEqualityTester { - protected abstract FormatterStep create(); + protected abstract Serializable create(); protected abstract void setupTest(API api) throws Exception; diff --git a/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java b/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java new file mode 100644 index 0000000000..d4d15dfdab --- /dev/null +++ b/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java @@ -0,0 +1,76 @@ +/* + * Copyright 2016 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.nio.charset.Charset; +import java.nio.charset.StandardCharsets; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.List; + +import org.junit.Test; + +import com.diffplug.common.base.StandardSystemProperty; +import com.diffplug.spotless.generic.EndWithNewlineStep; + +public class FormatterTest { + @Test + public void equality() { + new StepEqualityTester() { + private LineEnding.Policy lineEndingsPolicy = LineEnding.UNIX.createPolicy(); + private Charset encoding = StandardCharsets.UTF_8; + private Path rootDir = Paths.get(StandardSystemProperty.USER_DIR.value()); + private List steps = new ArrayList<>(); + private FormatExceptionPolicy exceptionPolicy = FormatExceptionPolicy.failOnlyOnError(); + + @Override + protected void setupTest(API api) throws Exception { + api.assertThisEqualToThis(); + api.areDifferentThan(); + + lineEndingsPolicy = LineEnding.WINDOWS.createPolicy(); + api.assertThisEqualToThis(); + api.areDifferentThan(); + + encoding = StandardCharsets.UTF_16; + api.assertThisEqualToThis(); + api.areDifferentThan(); + + rootDir = rootDir.getParent(); + api.assertThisEqualToThis(); + api.areDifferentThan(); + + steps.add(EndWithNewlineStep.create()); + api.assertThisEqualToThis(); + api.areDifferentThan(); + + // TODO: create some second exception policy to test this + } + + @Override + protected Formatter create() { + return Formatter.builder() + .lineEndingsPolicy(lineEndingsPolicy) + .encoding(encoding) + .rootDir(rootDir) + .steps(steps) + .exceptionPolicy(exceptionPolicy) + .build(); + } + }.testEquals(); + } +} From d126ed54b55c1b1c62302cccd4ec745a0cdb0b5c Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 17 Jan 2017 15:54:53 -0800 Subject: [PATCH 06/11] StepEqualityTester.API had three methods, but really only needed one. Refactored to only have one. --- .../extra/java/EclipseFormatterStepTest.java | 2 -- .../diffplug/spotless/StepEqualityTester.java | 26 +++++++------------ .../FilterByFileFormatterStepTest.java | 4 --- .../com/diffplug/spotless/FormatterTest.java | 5 ---- .../generic/EndWithNewlineStepTest.java | 1 - .../spotless/generic/IndentStepTest.java | 4 --- .../generic/LicenseHeaderStepTest.java | 4 --- .../TrimTrailingWhitespaceStepTest.java | 1 - .../java/GoogleJavaFormatStepTest.java | 2 -- .../spotless/java/ImportOrderStepTest.java | 4 --- .../java/RemoveUnusedImportsStepTest.java | 1 - .../spotless/kotlin/KtLintStepTest.java | 2 -- .../spotless/markdown/FreshMarkStepTest.java | 3 --- .../spotless/scala/ScalaFmtStepTest.java | 4 --- 14 files changed, 9 insertions(+), 54 deletions(-) diff --git a/lib-extra/src/test/java/com/diffplug/spotless/extra/java/EclipseFormatterStepTest.java b/lib-extra/src/test/java/com/diffplug/spotless/extra/java/EclipseFormatterStepTest.java index 4202ac2c9e..0ebdc4532c 100644 --- a/lib-extra/src/test/java/com/diffplug/spotless/extra/java/EclipseFormatterStepTest.java +++ b/lib-extra/src/test/java/com/diffplug/spotless/extra/java/EclipseFormatterStepTest.java @@ -60,11 +60,9 @@ public void equality() throws IOException { @Override protected void setupTest(API api) { settingsFile = xmlFile; - api.assertThisEqualToThis(); api.areDifferentThan(); settingsFile = propFile; - api.assertThisEqualToThis(); api.areDifferentThan(); } diff --git a/testlib/src/main/java/com/diffplug/spotless/StepEqualityTester.java b/testlib/src/main/java/com/diffplug/spotless/StepEqualityTester.java index f528a811b0..8ec61cdabb 100644 --- a/testlib/src/main/java/com/diffplug/spotless/StepEqualityTester.java +++ b/testlib/src/main/java/com/diffplug/spotless/StepEqualityTester.java @@ -28,10 +28,6 @@ public abstract class StepEqualityTester { protected abstract void setupTest(API api) throws Exception; public interface API { - void assertThis(); - - void assertThisEqualToThis(); - void areDifferentThan(); } @@ -39,21 +35,17 @@ public void testEquals() { List> allGroups = new ArrayList<>(); Box> currentGroup = Box.of(new ArrayList<>()); API api = new API() { - @Override - public void assertThis() { - currentGroup.get().add(create()); - } - - @Override - public void assertThisEqualToThis() { - assertThis(); - assertThis(); - } - @Override public void areDifferentThan() { - allGroups.add(currentGroup.get()); - currentGroup.set(new ArrayList<>()); + currentGroup.modify(current -> { + // create two instances, and add them to the group + current.add(create()); + current.add(create()); + // add this group to the list of all groups + allGroups.add(current); + // and return a new blank group for the next call + return new ArrayList<>(); + }); } }; try { diff --git a/testlib/src/test/java/com/diffplug/spotless/FilterByFileFormatterStepTest.java b/testlib/src/test/java/com/diffplug/spotless/FilterByFileFormatterStepTest.java index afe1b1709f..314b1b51b0 100644 --- a/testlib/src/test/java/com/diffplug/spotless/FilterByFileFormatterStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/FilterByFileFormatterStepTest.java @@ -43,19 +43,15 @@ protected void setupTest(API api) { // no filter, standard state state = "state"; filter = null; - api.assertThisEqualToThis(); api.areDifferentThan(); // same state, but now with a filter filter = "a"; - api.assertThisEqualToThis(); api.areDifferentThan(); // same state, but now with a filter filter = "b"; - api.assertThisEqualToThis(); api.areDifferentThan(); // same filter, but the state has changed state = "otherState"; - api.assertThisEqualToThis(); api.areDifferentThan(); } diff --git a/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java b/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java index d4d15dfdab..ccf930c08e 100644 --- a/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java @@ -39,23 +39,18 @@ public void equality() { @Override protected void setupTest(API api) throws Exception { - api.assertThisEqualToThis(); api.areDifferentThan(); lineEndingsPolicy = LineEnding.WINDOWS.createPolicy(); - api.assertThisEqualToThis(); api.areDifferentThan(); encoding = StandardCharsets.UTF_16; - api.assertThisEqualToThis(); api.areDifferentThan(); rootDir = rootDir.getParent(); - api.assertThisEqualToThis(); api.areDifferentThan(); steps.add(EndWithNewlineStep.create()); - api.assertThisEqualToThis(); api.areDifferentThan(); // TODO: create some second exception policy to test this diff --git a/testlib/src/test/java/com/diffplug/spotless/generic/EndWithNewlineStepTest.java b/testlib/src/test/java/com/diffplug/spotless/generic/EndWithNewlineStepTest.java index c0d9155616..fd933a655e 100644 --- a/testlib/src/test/java/com/diffplug/spotless/generic/EndWithNewlineStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/generic/EndWithNewlineStepTest.java @@ -38,7 +38,6 @@ public void equality() throws Exception { new StepEqualityTester() { @Override protected void setupTest(API api) { - api.assertThisEqualToThis(); api.areDifferentThan(); } diff --git a/testlib/src/test/java/com/diffplug/spotless/generic/IndentStepTest.java b/testlib/src/test/java/com/diffplug/spotless/generic/IndentStepTest.java index feb2aa12e7..db8fb19a36 100644 --- a/testlib/src/test/java/com/diffplug/spotless/generic/IndentStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/generic/IndentStepTest.java @@ -65,19 +65,15 @@ public void equality() { @Override protected void setupTest(API api) { - api.assertThisEqualToThis(); api.areDifferentThan(); numSpacesPerTab = 4; - api.assertThisEqualToThis(); api.areDifferentThan(); type = IndentStep.Type.TAB; - api.assertThisEqualToThis(); api.areDifferentThan(); numSpacesPerTab = 2; - api.assertThisEqualToThis(); api.areDifferentThan(); } 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 92552b1ff7..e1ebf2433a 100644 --- a/testlib/src/test/java/com/diffplug/spotless/generic/LicenseHeaderStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/generic/LicenseHeaderStepTest.java @@ -82,19 +82,15 @@ public void equality() { @Override protected void setupTest(API api) { - api.assertThisEqualToThis(); api.areDifferentThan(); delimiter = "crate"; - api.assertThisEqualToThis(); api.areDifferentThan(); header = "APACHE"; - api.assertThisEqualToThis(); api.areDifferentThan(); delimiter = "package"; - api.assertThisEqualToThis(); api.areDifferentThan(); } diff --git a/testlib/src/test/java/com/diffplug/spotless/generic/TrimTrailingWhitespaceStepTest.java b/testlib/src/test/java/com/diffplug/spotless/generic/TrimTrailingWhitespaceStepTest.java index d39234f552..f0a41af736 100644 --- a/testlib/src/test/java/com/diffplug/spotless/generic/TrimTrailingWhitespaceStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/generic/TrimTrailingWhitespaceStepTest.java @@ -54,7 +54,6 @@ public void equality() throws Exception { new StepEqualityTester() { @Override protected void setupTest(API api) { - api.assertThisEqualToThis(); api.areDifferentThan(); } diff --git a/testlib/src/test/java/com/diffplug/spotless/java/GoogleJavaFormatStepTest.java b/testlib/src/test/java/com/diffplug/spotless/java/GoogleJavaFormatStepTest.java index 868b0b649c..ac1e4a7afc 100644 --- a/testlib/src/test/java/com/diffplug/spotless/java/GoogleJavaFormatStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/java/GoogleJavaFormatStepTest.java @@ -44,11 +44,9 @@ public void equality() throws Exception { @Override protected void setupTest(API api) { // same version == same - api.assertThisEqualToThis(); api.areDifferentThan(); // change the version, and it's different version = "1.0"; - api.assertThisEqualToThis(); api.areDifferentThan(); } diff --git a/testlib/src/test/java/com/diffplug/spotless/java/ImportOrderStepTest.java b/testlib/src/test/java/com/diffplug/spotless/java/ImportOrderStepTest.java index 5ebb8ec1d4..87cd6b4c47 100644 --- a/testlib/src/test/java/com/diffplug/spotless/java/ImportOrderStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/java/ImportOrderStepTest.java @@ -63,19 +63,15 @@ public void equality() throws Exception { @Override protected void setupTest(API api) { // same version == same - api.assertThisEqualToThis(); api.areDifferentThan(); // change the version, and it's different imports = ImmutableList.of("a"); - api.assertThisEqualToThis(); api.areDifferentThan(); // change the version, and it's different imports = ImmutableList.of("b"); - api.assertThisEqualToThis(); api.areDifferentThan(); // change the version, and it's different imports = ImmutableList.of("a", "b"); - api.assertThisEqualToThis(); api.areDifferentThan(); } diff --git a/testlib/src/test/java/com/diffplug/spotless/java/RemoveUnusedImportsStepTest.java b/testlib/src/test/java/com/diffplug/spotless/java/RemoveUnusedImportsStepTest.java index 2bba77cc37..3697da713a 100644 --- a/testlib/src/test/java/com/diffplug/spotless/java/RemoveUnusedImportsStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/java/RemoveUnusedImportsStepTest.java @@ -38,7 +38,6 @@ public void equality() throws Exception { new StepEqualityTester() { @Override protected void setupTest(API api) { - api.assertThisEqualToThis(); api.areDifferentThan(); } diff --git a/testlib/src/test/java/com/diffplug/spotless/kotlin/KtLintStepTest.java b/testlib/src/test/java/com/diffplug/spotless/kotlin/KtLintStepTest.java index 497e6bf456..dba1c15e1f 100644 --- a/testlib/src/test/java/com/diffplug/spotless/kotlin/KtLintStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/kotlin/KtLintStepTest.java @@ -43,11 +43,9 @@ public void equality() throws Exception { @Override protected void setupTest(API api) { // same version == same - api.assertThisEqualToThis(); api.areDifferentThan(); // change the version, and it's different version = "0.2.1"; - api.assertThisEqualToThis(); api.areDifferentThan(); } diff --git a/testlib/src/test/java/com/diffplug/spotless/markdown/FreshMarkStepTest.java b/testlib/src/test/java/com/diffplug/spotless/markdown/FreshMarkStepTest.java index 99ff9667c7..2b894b82b4 100644 --- a/testlib/src/test/java/com/diffplug/spotless/markdown/FreshMarkStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/markdown/FreshMarkStepTest.java @@ -44,15 +44,12 @@ public void equality() throws Exception { @Override protected void setupTest(API api) { // same version and props == same - api.assertThisEqualToThis(); api.areDifferentThan(); // change the version, and it's different version = "1.3.0"; - api.assertThisEqualToThis(); api.areDifferentThan(); // change the props, and it's different props.put("1", "2"); - api.assertThisEqualToThis(); api.areDifferentThan(); } diff --git a/testlib/src/test/java/com/diffplug/spotless/scala/ScalaFmtStepTest.java b/testlib/src/test/java/com/diffplug/spotless/scala/ScalaFmtStepTest.java index b08a53c0d7..2fa9b58270 100644 --- a/testlib/src/test/java/com/diffplug/spotless/scala/ScalaFmtStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/scala/ScalaFmtStepTest.java @@ -50,19 +50,15 @@ public void equality() throws Exception { @Override protected void setupTest(API api) throws IOException { // same version == same - api.assertThisEqualToThis(); api.areDifferentThan(); // change the version, and it's different version = "0.5.0"; - api.assertThisEqualToThis(); api.areDifferentThan(); // add a config file, and its different configFile = createTestFile("scala/scalafmt/scalafmt.conf"); - api.assertThisEqualToThis(); api.areDifferentThan(); // change the config file and its different configFile = createTestFile("scala/scalafmt/scalafmt2.conf"); - api.assertThisEqualToThis(); api.areDifferentThan(); } From 7ed828c0276cd7b2161f4c1d19d363f3f351313d Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 17 Jan 2017 16:14:39 -0800 Subject: [PATCH 07/11] StepEqualityTester now tests that equality is preserved across serialization. Turns out that it wasn't for FreshMarkStep, which has now been fixed. --- .../spotless/markdown/FreshMarkStep.java | 8 ++++++-- .../diffplug/spotless/StepEqualityTester.java | 17 +++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/markdown/FreshMarkStep.java b/lib/src/main/java/com/diffplug/spotless/markdown/FreshMarkStep.java index 95e65e2f69..4e7871003c 100644 --- a/lib/src/main/java/com/diffplug/spotless/markdown/FreshMarkStep.java +++ b/lib/src/main/java/com/diffplug/spotless/markdown/FreshMarkStep.java @@ -18,7 +18,9 @@ import java.io.Serializable; import java.lang.reflect.Method; import java.util.Map; +import java.util.NavigableMap; import java.util.Objects; +import java.util.TreeMap; import java.util.function.Consumer; import java.util.logging.Logger; @@ -60,11 +62,13 @@ private static class State implements Serializable { /** The jar that contains the eclipse formatter. */ final JarState jarState; - final Map properties; + final NavigableMap properties; State(JarState jarState, Map properties) { this.jarState = Objects.requireNonNull(jarState); - this.properties = Objects.requireNonNull(properties); + // because equality is computed based on serialization, it's important to order the properties + // before writing them + this.properties = new TreeMap<>(Objects.requireNonNull(properties)); } FormatterFunc createFormat() throws Exception { diff --git a/testlib/src/main/java/com/diffplug/spotless/StepEqualityTester.java b/testlib/src/main/java/com/diffplug/spotless/StepEqualityTester.java index 8ec61cdabb..6c5ada5068 100644 --- a/testlib/src/main/java/com/diffplug/spotless/StepEqualityTester.java +++ b/testlib/src/main/java/com/diffplug/spotless/StepEqualityTester.java @@ -15,6 +15,9 @@ */ package com.diffplug.spotless; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.ObjectInputStream; import java.io.Serializable; import java.util.ArrayList; import java.util.List; @@ -41,6 +44,9 @@ public void areDifferentThan() { // create two instances, and add them to the group current.add(create()); current.add(create()); + // create two instances using a serialization roundtrip, and add them to the group + current.add(reserialize(create())); + current.add(reserialize(create())); // add this group to the list of all groups allGroups.add(current); // and return a new blank group for the next call @@ -63,4 +69,15 @@ public void areDifferentThan() { } tester.testEquals(); } + + @SuppressWarnings("unchecked") + private static T reserialize(T input) { + byte[] asBytes = LazyForwardingEquality.toBytes(input); + ByteArrayInputStream byteInput = new ByteArrayInputStream(asBytes); + try (ObjectInputStream objectInput = new ObjectInputStream(byteInput)) { + return (T) objectInput.readObject(); + } catch (IOException | ClassNotFoundException e) { + throw ThrowingEx.asRuntime(e); + } + } } From 5f5efed7ce7981f736727670196c7d27f1d86aa0 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 17 Jan 2017 18:47:25 -0800 Subject: [PATCH 08/11] Renamed StepEqualityTester to SerializableEqualityTester, now that we're using it for any serializable object. --- .../spotless/extra/java/EclipseFormatterStepTest.java | 4 ++-- ...tepEqualityTester.java => SerializableEqualityTester.java} | 2 +- .../com/diffplug/spotless/FilterByFileFormatterStepTest.java | 2 +- .../src/test/java/com/diffplug/spotless/FormatterTest.java | 2 +- .../com/diffplug/spotless/generic/EndWithNewlineStepTest.java | 4 ++-- .../java/com/diffplug/spotless/generic/IndentStepTest.java | 4 ++-- .../com/diffplug/spotless/generic/LicenseHeaderStepTest.java | 4 ++-- .../spotless/generic/TrimTrailingWhitespaceStepTest.java | 4 ++-- .../com/diffplug/spotless/java/GoogleJavaFormatStepTest.java | 4 ++-- .../java/com/diffplug/spotless/java/ImportOrderStepTest.java | 4 ++-- .../diffplug/spotless/java/RemoveUnusedImportsStepTest.java | 4 ++-- .../java/com/diffplug/spotless/kotlin/KtLintStepTest.java | 4 ++-- .../com/diffplug/spotless/markdown/FreshMarkStepTest.java | 4 ++-- .../java/com/diffplug/spotless/scala/ScalaFmtStepTest.java | 4 ++-- 14 files changed, 25 insertions(+), 25 deletions(-) rename testlib/src/main/java/com/diffplug/spotless/{StepEqualityTester.java => SerializableEqualityTester.java} (98%) diff --git a/lib-extra/src/test/java/com/diffplug/spotless/extra/java/EclipseFormatterStepTest.java b/lib-extra/src/test/java/com/diffplug/spotless/extra/java/EclipseFormatterStepTest.java index 0ebdc4532c..e3c9182af2 100644 --- a/lib-extra/src/test/java/com/diffplug/spotless/extra/java/EclipseFormatterStepTest.java +++ b/lib-extra/src/test/java/com/diffplug/spotless/extra/java/EclipseFormatterStepTest.java @@ -22,7 +22,7 @@ import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.ResourceHarness; -import com.diffplug.spotless.StepEqualityTester; +import com.diffplug.spotless.SerializableEqualityTester; import com.diffplug.spotless.StepHarness; import com.diffplug.spotless.TestProvisioner; @@ -54,7 +54,7 @@ public void longLiteralProblem() throws Throwable { public void equality() throws IOException { File xmlFile = createTestFile("java/eclipse/format/formatter.xml"); File propFile = createTestFile("java/eclipse/format/formatter.properties"); - new StepEqualityTester() { + new SerializableEqualityTester() { File settingsFile; @Override diff --git a/testlib/src/main/java/com/diffplug/spotless/StepEqualityTester.java b/testlib/src/main/java/com/diffplug/spotless/SerializableEqualityTester.java similarity index 98% rename from testlib/src/main/java/com/diffplug/spotless/StepEqualityTester.java rename to testlib/src/main/java/com/diffplug/spotless/SerializableEqualityTester.java index 6c5ada5068..096edf62e2 100644 --- a/testlib/src/main/java/com/diffplug/spotless/StepEqualityTester.java +++ b/testlib/src/main/java/com/diffplug/spotless/SerializableEqualityTester.java @@ -25,7 +25,7 @@ import com.diffplug.common.base.Box; import com.diffplug.common.testing.EqualsTester; -public abstract class StepEqualityTester { +public abstract class SerializableEqualityTester { protected abstract Serializable create(); protected abstract void setupTest(API api) throws Exception; diff --git a/testlib/src/test/java/com/diffplug/spotless/FilterByFileFormatterStepTest.java b/testlib/src/test/java/com/diffplug/spotless/FilterByFileFormatterStepTest.java index 314b1b51b0..78155b67b3 100644 --- a/testlib/src/test/java/com/diffplug/spotless/FilterByFileFormatterStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/FilterByFileFormatterStepTest.java @@ -34,7 +34,7 @@ public void behavior() throws Exception { @Test public void equality() throws Exception { - new StepEqualityTester() { + new SerializableEqualityTester() { String state; String filter; diff --git a/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java b/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java index ccf930c08e..8526d92fe5 100644 --- a/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java @@ -30,7 +30,7 @@ public class FormatterTest { @Test public void equality() { - new StepEqualityTester() { + new SerializableEqualityTester() { private LineEnding.Policy lineEndingsPolicy = LineEnding.UNIX.createPolicy(); private Charset encoding = StandardCharsets.UTF_8; private Path rootDir = Paths.get(StandardSystemProperty.USER_DIR.value()); diff --git a/testlib/src/test/java/com/diffplug/spotless/generic/EndWithNewlineStepTest.java b/testlib/src/test/java/com/diffplug/spotless/generic/EndWithNewlineStepTest.java index fd933a655e..d16e88ab05 100644 --- a/testlib/src/test/java/com/diffplug/spotless/generic/EndWithNewlineStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/generic/EndWithNewlineStepTest.java @@ -18,7 +18,7 @@ import org.junit.Test; import com.diffplug.spotless.FormatterStep; -import com.diffplug.spotless.StepEqualityTester; +import com.diffplug.spotless.SerializableEqualityTester; import com.diffplug.spotless.StepHarness; public class EndWithNewlineStepTest { @@ -35,7 +35,7 @@ public void behavior() throws Exception { @Test public void equality() throws Exception { - new StepEqualityTester() { + new SerializableEqualityTester() { @Override protected void setupTest(API api) { api.areDifferentThan(); diff --git a/testlib/src/test/java/com/diffplug/spotless/generic/IndentStepTest.java b/testlib/src/test/java/com/diffplug/spotless/generic/IndentStepTest.java index db8fb19a36..30043d6f8f 100644 --- a/testlib/src/test/java/com/diffplug/spotless/generic/IndentStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/generic/IndentStepTest.java @@ -22,7 +22,7 @@ import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.ResourceHarness; -import com.diffplug.spotless.StepEqualityTester; +import com.diffplug.spotless.SerializableEqualityTester; import com.diffplug.spotless.generic.IndentStep; public class IndentStepTest extends ResourceHarness { @@ -59,7 +59,7 @@ public void doesntClipNewlines() throws Throwable { @Test public void equality() { - new StepEqualityTester() { + new SerializableEqualityTester() { IndentStep.Type type = IndentStep.Type.SPACE; int numSpacesPerTab = 2; 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 e1ebf2433a..d66d37ecad 100644 --- a/testlib/src/test/java/com/diffplug/spotless/generic/LicenseHeaderStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/generic/LicenseHeaderStepTest.java @@ -23,7 +23,7 @@ import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.ResourceHarness; -import com.diffplug.spotless.StepEqualityTester; +import com.diffplug.spotless.SerializableEqualityTester; import com.diffplug.spotless.generic.LicenseHeaderStep; public class LicenseHeaderStepTest extends ResourceHarness { @@ -76,7 +76,7 @@ public void sanitizerDoesntGoTooFar() throws Throwable { @Test public void equality() { - new StepEqualityTester() { + new SerializableEqualityTester() { String header = "LICENSE"; String delimiter = "package"; diff --git a/testlib/src/test/java/com/diffplug/spotless/generic/TrimTrailingWhitespaceStepTest.java b/testlib/src/test/java/com/diffplug/spotless/generic/TrimTrailingWhitespaceStepTest.java index f0a41af736..15d9415f9d 100644 --- a/testlib/src/test/java/com/diffplug/spotless/generic/TrimTrailingWhitespaceStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/generic/TrimTrailingWhitespaceStepTest.java @@ -19,7 +19,7 @@ import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.ResourceHarness; -import com.diffplug.spotless.StepEqualityTester; +import com.diffplug.spotless.SerializableEqualityTester; import com.diffplug.spotless.StepHarness; public class TrimTrailingWhitespaceStepTest extends ResourceHarness { @@ -51,7 +51,7 @@ public void trimTrailingWhitespace() throws Exception { @Test public void equality() throws Exception { - new StepEqualityTester() { + new SerializableEqualityTester() { @Override protected void setupTest(API api) { api.areDifferentThan(); diff --git a/testlib/src/test/java/com/diffplug/spotless/java/GoogleJavaFormatStepTest.java b/testlib/src/test/java/com/diffplug/spotless/java/GoogleJavaFormatStepTest.java index ac1e4a7afc..e5e9a9a6af 100644 --- a/testlib/src/test/java/com/diffplug/spotless/java/GoogleJavaFormatStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/java/GoogleJavaFormatStepTest.java @@ -21,7 +21,7 @@ import com.diffplug.common.base.StringPrinter; import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.ResourceHarness; -import com.diffplug.spotless.StepEqualityTester; +import com.diffplug.spotless.SerializableEqualityTester; import com.diffplug.spotless.StepHarness; import com.diffplug.spotless.TestProvisioner; @@ -38,7 +38,7 @@ public void behavior() throws Exception { @Test public void equality() throws Exception { - new StepEqualityTester() { + new SerializableEqualityTester() { String version = "1.1"; @Override diff --git a/testlib/src/test/java/com/diffplug/spotless/java/ImportOrderStepTest.java b/testlib/src/test/java/com/diffplug/spotless/java/ImportOrderStepTest.java index 87cd6b4c47..dc174d689f 100644 --- a/testlib/src/test/java/com/diffplug/spotless/java/ImportOrderStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/java/ImportOrderStepTest.java @@ -23,7 +23,7 @@ import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.NonSerializableList; import com.diffplug.spotless.ResourceHarness; -import com.diffplug.spotless.StepEqualityTester; +import com.diffplug.spotless.SerializableEqualityTester; public class ImportOrderStepTest extends ResourceHarness { @Test @@ -57,7 +57,7 @@ public void doesntThrowIfImportOrderIsntSerializable() { @Test public void equality() throws Exception { - new StepEqualityTester() { + new SerializableEqualityTester() { ImmutableList imports = ImmutableList.of(); @Override diff --git a/testlib/src/test/java/com/diffplug/spotless/java/RemoveUnusedImportsStepTest.java b/testlib/src/test/java/com/diffplug/spotless/java/RemoveUnusedImportsStepTest.java index 3697da713a..db068e318e 100644 --- a/testlib/src/test/java/com/diffplug/spotless/java/RemoveUnusedImportsStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/java/RemoveUnusedImportsStepTest.java @@ -18,7 +18,7 @@ import org.junit.Test; import com.diffplug.spotless.FormatterStep; -import com.diffplug.spotless.StepEqualityTester; +import com.diffplug.spotless.SerializableEqualityTester; import com.diffplug.spotless.StepHarness; import com.diffplug.spotless.TestProvisioner; @@ -35,7 +35,7 @@ public void behavior() throws Exception { @Test public void equality() throws Exception { - new StepEqualityTester() { + new SerializableEqualityTester() { @Override protected void setupTest(API api) { api.areDifferentThan(); diff --git a/testlib/src/test/java/com/diffplug/spotless/kotlin/KtLintStepTest.java b/testlib/src/test/java/com/diffplug/spotless/kotlin/KtLintStepTest.java index dba1c15e1f..951d8838dd 100644 --- a/testlib/src/test/java/com/diffplug/spotless/kotlin/KtLintStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/kotlin/KtLintStepTest.java @@ -19,7 +19,7 @@ import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.ResourceHarness; -import com.diffplug.spotless.StepEqualityTester; +import com.diffplug.spotless.SerializableEqualityTester; import com.diffplug.spotless.StepHarness; import com.diffplug.spotless.TestProvisioner; @@ -37,7 +37,7 @@ public void behavior() throws Exception { @Test public void equality() throws Exception { - new StepEqualityTester() { + new SerializableEqualityTester() { String version = "0.2.2"; @Override diff --git a/testlib/src/test/java/com/diffplug/spotless/markdown/FreshMarkStepTest.java b/testlib/src/test/java/com/diffplug/spotless/markdown/FreshMarkStepTest.java index 2b894b82b4..6f8a804a8a 100644 --- a/testlib/src/test/java/com/diffplug/spotless/markdown/FreshMarkStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/markdown/FreshMarkStepTest.java @@ -21,7 +21,7 @@ import org.junit.Test; import com.diffplug.spotless.FormatterStep; -import com.diffplug.spotless.StepEqualityTester; +import com.diffplug.spotless.SerializableEqualityTester; import com.diffplug.spotless.StepHarness; import com.diffplug.spotless.TestProvisioner; @@ -37,7 +37,7 @@ public void behavior() throws Exception { @Test public void equality() throws Exception { - new StepEqualityTester() { + new SerializableEqualityTester() { String version = "1.3.1"; Map props = new HashMap<>(); diff --git a/testlib/src/test/java/com/diffplug/spotless/scala/ScalaFmtStepTest.java b/testlib/src/test/java/com/diffplug/spotless/scala/ScalaFmtStepTest.java index 2fa9b58270..21b4372f70 100644 --- a/testlib/src/test/java/com/diffplug/spotless/scala/ScalaFmtStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/scala/ScalaFmtStepTest.java @@ -22,7 +22,7 @@ import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.ResourceHarness; -import com.diffplug.spotless.StepEqualityTester; +import com.diffplug.spotless.SerializableEqualityTester; import com.diffplug.spotless.StepHarness; import com.diffplug.spotless.TestProvisioner; @@ -43,7 +43,7 @@ public void behaviorCustomConfig() throws Exception { @Test public void equality() throws Exception { - new StepEqualityTester() { + new SerializableEqualityTester() { String version = "0.5.1"; File configFile = null; From b47b12dff410700bcf797a2449767daee5118a62 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 17 Jan 2017 18:21:56 -0800 Subject: [PATCH 09/11] Added a strict FormatExceptionPolicy. --- .../spotless/FormatExceptionPolicyLegacy.java | 12 +++- .../spotless/FormatExceptionPolicyStrict.java | 57 +++++++++++++++++++ .../com/diffplug/spotless/ThrowingEx.java | 18 ++++-- .../com/diffplug/spotless/FormatterTest.java | 14 ++++- 4 files changed, 94 insertions(+), 7 deletions(-) create mode 100644 lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyStrict.java diff --git a/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyLegacy.java b/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyLegacy.java index cb8a40a1f4..a1131709df 100644 --- a/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyLegacy.java +++ b/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyLegacy.java @@ -28,10 +28,18 @@ class FormatExceptionPolicyLegacy extends NoLambda.EqualityBasedOnSerialization @Override public void handleError(Throwable e, FormatterStep step, File file, Path rootDir) { if (e instanceof Error) { - logger.severe("Step '" + step.getName() + "' found problem in '" + rootDir.relativize(file.toPath()) + "':\n" + e.getMessage()); + error(e, step, file, rootDir); throw ((Error) e); } else { - logger.log(Level.WARNING, "Unable to apply step '" + step.getName() + "' to '" + rootDir.relativize(file.toPath()), e); + warning(e, step, file, rootDir); } } + + static void error(Throwable e, FormatterStep step, File file, Path rootDir) { + logger.severe("Step '" + step.getName() + "' found problem in '" + rootDir.relativize(file.toPath()) + "':\n" + e.getMessage()); + } + + static void warning(Throwable e, FormatterStep step, File file, Path rootDir) { + logger.log(Level.WARNING, "Unable to apply step '" + step.getName() + "' to '" + rootDir.relativize(file.toPath()), e); + } } diff --git a/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyStrict.java b/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyStrict.java new file mode 100644 index 0000000000..4ca2561b8a --- /dev/null +++ b/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyStrict.java @@ -0,0 +1,57 @@ +/* + * Copyright 2016 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.nio.file.Path; +import java.util.Set; +import java.util.TreeSet; + +/** + * A policy for handling exceptions in the format. Any exceptions will + * halt the build except for a specifically excluded path or step. + */ +public class FormatExceptionPolicyStrict extends NoLambda.EqualityBasedOnSerialization implements FormatExceptionPolicy { + private static final long serialVersionUID = 1L; + + private final Set excludeSteps = new TreeSet<>(); + private final Set excludePaths = new TreeSet<>(); + + /** Adds a step name to exclude. */ + public void excludeStep(String stepName) { + excludeSteps.add(stepName); + } + + /** Adds a realtive pathx to exclude. */ + public void excludePath(String relativePath) { + excludePaths.add(relativePath); + } + + @Override + public void handleError(Throwable e, FormatterStep step, File file, Path rootDir) { + if (excludeSteps.contains(step.getName())) { + FormatExceptionPolicyLegacy.warning(e, step, file, rootDir); + } else { + String path = rootDir.relativize(file.toPath()).toString(); + if (excludePaths.contains(path)) { + FormatExceptionPolicyLegacy.warning(e, step, file, rootDir); + } else { + FormatExceptionPolicyLegacy.error(e, step, file, rootDir); + throw ThrowingEx.asRuntimeRethrowError(e); + } + } + } +} diff --git a/lib/src/main/java/com/diffplug/spotless/ThrowingEx.java b/lib/src/main/java/com/diffplug/spotless/ThrowingEx.java index be7ac4b91c..d92dce90a2 100644 --- a/lib/src/main/java/com/diffplug/spotless/ThrowingEx.java +++ b/lib/src/main/java/com/diffplug/spotless/ThrowingEx.java @@ -101,14 +101,24 @@ public static RuntimeException asRuntime(Exception e) { public static RuntimeException unwrapCause(Throwable e) { Throwable cause = e.getCause(); if (cause == null) { - return ifErrorRethrowElseAsRuntime(e); + return asRuntimeRethrowError(e); } else { - return ifErrorRethrowElseAsRuntime(cause); + return asRuntimeRethrowError(cause); } } - /** Rethrows errors, wraps and returns everything else as a runtime exception. */ - private static RuntimeException ifErrorRethrowElseAsRuntime(Throwable e) { + /** + * Rethrows errors, wraps and returns everything else as a runtime exception. + * + * try { + * doSomething(); + * } catch (Throwable e) { + * throw asRuntimeRethrowError(e); + * } + * ``` + * + * */ + static RuntimeException asRuntimeRethrowError(Throwable e) { if (e instanceof Error) { throw (Error) e; } else if (e instanceof RuntimeException) { diff --git a/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java b/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java index 8526d92fe5..02089058fa 100644 --- a/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java @@ -53,7 +53,19 @@ protected void setupTest(API api) throws Exception { steps.add(EndWithNewlineStep.create()); api.areDifferentThan(); - // TODO: create some second exception policy to test this + { + FormatExceptionPolicyStrict standard = new FormatExceptionPolicyStrict(); + standard.excludePath("path"); + exceptionPolicy = standard; + api.areDifferentThan(); + } + + { + FormatExceptionPolicyStrict standard = new FormatExceptionPolicyStrict(); + standard.excludeStep("step"); + exceptionPolicy = standard; + api.areDifferentThan(); + } } @Override From 672051d8b00d86ae01e513d9372b402a461fcf0e Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 17 Jan 2017 18:22:58 -0800 Subject: [PATCH 10/11] FormatExtension now has methods `ignoreErrorForStep` and `ignoreErrorForPath`, but by default any exceptions will kill the build. --- .../diffplug/gradle/spotless/FormatExtension.java | 14 ++++++++++++++ .../com/diffplug/gradle/spotless/SpotlessTask.java | 14 ++++++++++++++ 2 files changed, 28 insertions(+) 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 16e3522183..5a3b27e0c6 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 @@ -33,6 +33,7 @@ import org.gradle.api.file.FileCollection; import org.gradle.api.internal.file.UnionFileCollection; +import com.diffplug.spotless.FormatExceptionPolicyStrict; import com.diffplug.spotless.FormatterFunc; import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.LazyForwardingEquality; @@ -105,6 +106,18 @@ public void setEncoding(Charset charset) { encoding = Objects.requireNonNull(charset); } + FormatExceptionPolicyStrict exceptionPolicy = new FormatExceptionPolicyStrict(); + + /** Ignores errors in the given step. */ + public void ignoreErrorForStep(String stepName) { + exceptionPolicy.excludeStep(stepName); + } + + /** Ignores errors for the given relative path. */ + public void ignoreErrorForPath(String relativePath) { + exceptionPolicy.excludePath(relativePath); + } + /** Sets encoding to use (defaults to {@link SpotlessExtension#getEncoding()}). */ public void encoding(String charset) { setEncoding(charset); @@ -340,6 +353,7 @@ public void licenseHeaderFile(Object licenseHeaderFile, String delimiter) { protected void setupTask(SpotlessTask task) { task.setPaddedCell(paddedCell); task.setEncoding(getEncoding().name()); + task.setExceptionPolicy(exceptionPolicy); task.setTarget(target); task.setSteps(steps); task.setLineEndingsPolicy(getLineEndings().createPolicy(getProject().getProjectDir(), () -> task.target)); diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java index 166969d34e..fb4bd85769 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java @@ -24,6 +24,7 @@ import java.util.Collections; import java.util.List; import java.util.Locale; +import java.util.Objects; import org.gradle.api.DefaultTask; import org.gradle.api.GradleException; @@ -33,6 +34,8 @@ import org.gradle.api.tasks.TaskAction; import org.gradle.api.tasks.incremental.IncrementalTaskInputs; +import com.diffplug.spotless.FormatExceptionPolicy; +import com.diffplug.spotless.FormatExceptionPolicyStrict; import com.diffplug.spotless.Formatter; import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.LineEnding; @@ -75,6 +78,17 @@ public void setPaddedCell(boolean paddedCell) { this.paddedCell = paddedCell; } + @Input + protected FormatExceptionPolicy exceptionPolicy = new FormatExceptionPolicyStrict(); + + public void setExceptionPolicy(FormatExceptionPolicy exceptionPolicy) { + this.exceptionPolicy = Objects.requireNonNull(exceptionPolicy); + } + + public FormatExceptionPolicy getExceptionPolicy() { + return exceptionPolicy; + } + @InputFiles @SkipWhenEmpty protected Iterable target; From e279153e5112a61f10b349a5e26e922fb7ee381b Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 18 Jan 2017 08:53:05 -0800 Subject: [PATCH 11/11] FormatExceptionPolicy now takes (String relativePath) rather than (File file, Path rootDir). --- .../diffplug/spotless/FormatExceptionPolicy.java | 4 +--- .../spotless/FormatExceptionPolicyLegacy.java | 16 +++++++--------- .../spotless/FormatExceptionPolicyStrict.java | 13 +++++-------- .../java/com/diffplug/spotless/Formatter.java | 3 ++- .../gradle/spotless/ErrorShouldRethrow.java | 14 ++------------ 5 files changed, 17 insertions(+), 33 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicy.java b/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicy.java index 10f8d4c0e5..4d3555854b 100644 --- a/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicy.java +++ b/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicy.java @@ -15,14 +15,12 @@ */ package com.diffplug.spotless; -import java.io.File; import java.io.Serializable; -import java.nio.file.Path; /** A policy for handling exceptions in the format. */ public interface FormatExceptionPolicy extends Serializable, NoLambda { /** Called for every error in the formatter. */ - void handleError(Throwable e, FormatterStep step, File file, Path rootDir); + void handleError(Throwable e, FormatterStep step, String relativePath); /** * Returns a byte array representation of everything inside this `FormatExceptionPolicy`. diff --git a/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyLegacy.java b/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyLegacy.java index a1131709df..df95542a44 100644 --- a/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyLegacy.java +++ b/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyLegacy.java @@ -15,8 +15,6 @@ */ package com.diffplug.spotless; -import java.io.File; -import java.nio.file.Path; import java.util.logging.Level; import java.util.logging.Logger; @@ -26,20 +24,20 @@ class FormatExceptionPolicyLegacy extends NoLambda.EqualityBasedOnSerialization private static final Logger logger = Logger.getLogger(Formatter.class.getName()); @Override - public void handleError(Throwable e, FormatterStep step, File file, Path rootDir) { + public void handleError(Throwable e, FormatterStep step, String relativePath) { if (e instanceof Error) { - error(e, step, file, rootDir); + error(e, step, relativePath); throw ((Error) e); } else { - warning(e, step, file, rootDir); + warning(e, step, relativePath); } } - static void error(Throwable e, FormatterStep step, File file, Path rootDir) { - logger.severe("Step '" + step.getName() + "' found problem in '" + rootDir.relativize(file.toPath()) + "':\n" + e.getMessage()); + static void error(Throwable e, FormatterStep step, String relativePath) { + logger.log(Level.SEVERE, "Step '" + step.getName() + "' found problem in '" + relativePath + "':\n" + e.getMessage(), e); } - static void warning(Throwable e, FormatterStep step, File file, Path rootDir) { - logger.log(Level.WARNING, "Unable to apply step '" + step.getName() + "' to '" + rootDir.relativize(file.toPath()), e); + static void warning(Throwable e, FormatterStep step, String relativePath) { + logger.log(Level.WARNING, "Unable to apply step '" + step.getName() + "' to '" + relativePath + "'", e); } } diff --git a/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyStrict.java b/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyStrict.java index 4ca2561b8a..ec01ecf7d0 100644 --- a/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyStrict.java +++ b/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyStrict.java @@ -15,8 +15,6 @@ */ package com.diffplug.spotless; -import java.io.File; -import java.nio.file.Path; import java.util.Set; import java.util.TreeSet; @@ -41,15 +39,14 @@ public void excludePath(String relativePath) { } @Override - public void handleError(Throwable e, FormatterStep step, File file, Path rootDir) { + public void handleError(Throwable e, FormatterStep step, String relativePath) { if (excludeSteps.contains(step.getName())) { - FormatExceptionPolicyLegacy.warning(e, step, file, rootDir); + FormatExceptionPolicyLegacy.warning(e, step, relativePath); } else { - String path = rootDir.relativize(file.toPath()).toString(); - if (excludePaths.contains(path)) { - FormatExceptionPolicyLegacy.warning(e, step, file, rootDir); + if (excludePaths.contains(relativePath)) { + FormatExceptionPolicyLegacy.warning(e, step, relativePath); } else { - FormatExceptionPolicyLegacy.error(e, step, file, rootDir); + FormatExceptionPolicyLegacy.error(e, step, relativePath); throw ThrowingEx.asRuntimeRethrowError(e); } } diff --git a/lib/src/main/java/com/diffplug/spotless/Formatter.java b/lib/src/main/java/com/diffplug/spotless/Formatter.java index 3cab185480..101fdb4084 100644 --- a/lib/src/main/java/com/diffplug/spotless/Formatter.java +++ b/lib/src/main/java/com/diffplug/spotless/Formatter.java @@ -225,7 +225,8 @@ public String compute(String unix, File file) throws Error { unix = LineEnding.toUnix(formatted); } } catch (Throwable e) { - exceptionPolicy.handleError(e, step, file, rootDir); + String relativePath = rootDir.relativize(file.toPath()).toString(); + exceptionPolicy.handleError(e, step, relativePath); } } return unix; diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ErrorShouldRethrow.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ErrorShouldRethrow.java index 0969d0c629..2eddba7fea 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ErrorShouldRethrow.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ErrorShouldRethrow.java @@ -15,7 +15,6 @@ */ package com.diffplug.gradle.spotless; -import java.io.File; import java.util.List; import java.util.stream.Collectors; @@ -33,7 +32,7 @@ public class ErrorShouldRethrow extends GradleIntegrationTest { @Test public void noSwearing() throws Exception { - File build = write("build.gradle", + write("build.gradle", "plugins {", " id 'com.diffplug.gradle.spotless'", "}", @@ -53,16 +52,7 @@ public void noSwearing() throws Exception { String expectedToStartWith = StringPrinter.buildStringFromLines( ":spotlessMiscStep 'no swearing' found problem in 'README.md':", "No swearing!", - " FAILED", - "", - "FAILURE: Build failed with an exception.", - "", - "* Where:", - "Build file '" + build + "' line: 9", - "", - "* What went wrong:", - "Execution failed for task ':spotlessMisc'.", - "> No swearing!"); + "java.lang.AssertionError: No swearing!").trim(); int numNewlines = CharMatcher.is('\n').countIn(expectedToStartWith); List actualLines = Splitter.on('\n').splitToList(LineEnding.toUnix(result.getOutput())); String actualStart = actualLines.subList(0, numNewlines + 1).stream().collect(Collectors.joining("\n"));