From 48e744d8cfcaf5121e5920802d0400a22ebefbb3 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 29 May 2024 13:51:51 -0700 Subject: [PATCH 01/13] Remove crufty and internally-unused methods from `Formatter`. --- CHANGES.md | 1 + .../java/com/diffplug/spotless/Formatter.java | 65 ------------------- 2 files changed, 1 insertion(+), 65 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 04722f1c2b..716166ee39 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -34,6 +34,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( * **BREAKING** Remove `JarState.getMavenCoordinate(String prefix)`. ([#1945](https://github.com/diffplug/spotless/pull/1945)) * **BREAKING** Replace `PipeStepPair` with `FenceStep`. ([#1954](https://github.com/diffplug/spotless/pull/1954)) * **BREAKING** Fully removed `Rome`, use `Biome` instead. ([#2119](https://github.com/diffplug/spotless/pull/2119)) +* **BREAKING** Removed `isClean`, `applyTo`, and `applyToAndReturnResultIfDirty` from `Formatter` because users should instead use `PaddedCell.check()`. ## [2.45.0] - 2024-01-23 ### Added diff --git a/lib/src/main/java/com/diffplug/spotless/Formatter.java b/lib/src/main/java/com/diffplug/spotless/Formatter.java index b5cd5f81fc..4dea2e3d3d 100644 --- a/lib/src/main/java/com/diffplug/spotless/Formatter.java +++ b/lib/src/main/java/com/diffplug/spotless/Formatter.java @@ -24,17 +24,12 @@ 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; import java.util.List; import java.util.Objects; -import javax.annotation.Nullable; - /** Formatter which performs the full formatting. */ public final class Formatter implements Serializable, AutoCloseable { private static final long serialVersionUID = 1L; @@ -159,66 +154,6 @@ public Formatter build() { } } - /** Returns true iff the given file's formatting is up-to-date. */ - public boolean isClean(File file) throws IOException { - Objects.requireNonNull(file); - - String raw = new String(Files.readAllBytes(file.toPath()), encoding); - String unix = LineEnding.toUnix(raw); - - // check the newlines (we can find these problems without even running the steps) - int totalNewLines = (int) unix.codePoints().filter(val -> val == '\n').count(); - int windowsNewLines = raw.length() - unix.length(); - if (lineEndingsPolicy.isUnix(file)) { - if (windowsNewLines != 0) { - return false; - } - } else { - if (windowsNewLines != totalNewLines) { - return false; - } - } - - // check the other formats - String formatted = compute(unix, file); - - // return true iff the formatted string equals the unix one - return formatted.equals(unix); - } - - /** Applies formatting to the given file. */ - public void applyTo(File file) throws IOException { - applyToAndReturnResultIfDirty(file); - } - - /** - * Applies formatting to the given file. - *

- * Returns null if the file was already clean, or the - * formatted result with unix newlines if it was not. - */ - public @Nullable String applyToAndReturnResultIfDirty(File file) throws IOException { - Objects.requireNonNull(file); - - byte[] rawBytes = Files.readAllBytes(file.toPath()); - String raw = new String(rawBytes, encoding); - String rawUnix = LineEnding.toUnix(raw); - - // enforce the format - String formattedUnix = compute(rawUnix, file); - // enforce the line endings - String formatted = computeLineEndings(formattedUnix, file); - - // write out the file iff it has changed - byte[] formattedBytes = formatted.getBytes(encoding); - if (!Arrays.equals(rawBytes, formattedBytes)) { - Files.write(file.toPath(), formattedBytes, StandardOpenOption.TRUNCATE_EXISTING); - return formattedUnix; - } else { - return null; - } - } - /** Applies the appropriate line endings to the given unix content. */ public String computeLineEndings(String unix, File file) { Objects.requireNonNull(unix, "unix"); From 5843830a142677cf9888e171008d8d498cd451e1 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 29 May 2024 13:52:32 -0700 Subject: [PATCH 02/13] Moved `PaddedCell.DirtyState` to its own top-level class with new methods. --- CHANGES.md | 1 + .../com/diffplug/spotless/DirtyState.java | 141 ++++++++++++++++++ .../com/diffplug/spotless/PaddedCell.java | 111 +------------- .../com/diffplug/gradle/spotless/IdeHook.java | 6 +- .../gradle/spotless/SpotlessTaskImpl.java | 10 +- .../com/diffplug/spotless/maven/IdeHook.java | 6 +- .../spotless/maven/SpotlessApplyMojo.java | 6 +- .../spotless/maven/SpotlessCheckMojo.java | 4 +- 8 files changed, 159 insertions(+), 126 deletions(-) create mode 100644 lib/src/main/java/com/diffplug/spotless/DirtyState.java diff --git a/CHANGES.md b/CHANGES.md index 716166ee39..1de166cd2b 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -34,6 +34,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( * **BREAKING** Remove `JarState.getMavenCoordinate(String prefix)`. ([#1945](https://github.com/diffplug/spotless/pull/1945)) * **BREAKING** Replace `PipeStepPair` with `FenceStep`. ([#1954](https://github.com/diffplug/spotless/pull/1954)) * **BREAKING** Fully removed `Rome`, use `Biome` instead. ([#2119](https://github.com/diffplug/spotless/pull/2119)) +* **BREAKING** Moved `PaddedCell.DirtyState` to its own top-level class with new methods. * **BREAKING** Removed `isClean`, `applyTo`, and `applyToAndReturnResultIfDirty` from `Formatter` because users should instead use `PaddedCell.check()`. ## [2.45.0] - 2024-01-23 diff --git a/lib/src/main/java/com/diffplug/spotless/DirtyState.java b/lib/src/main/java/com/diffplug/spotless/DirtyState.java new file mode 100644 index 0000000000..3d133927c6 --- /dev/null +++ b/lib/src/main/java/com/diffplug/spotless/DirtyState.java @@ -0,0 +1,141 @@ +/* + * Copyright 2022-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.IOException; +import java.io.OutputStream; +import java.nio.file.Files; +import java.util.Arrays; + +import javax.annotation.Nullable; + +/** + * The clean/dirty state of a single file. Intended use: + * - {@link #isClean()} means that the file is is clean, and there's nothing else to say + * - {@link #didNotConverge()} means that we were unable to determine a clean state + * - once you've tested the above conditions and you know that it's a dirty file with a converged state, + * then you can call {@link #writeCanonicalTo(OutputStream)} to get the canonical form of the given file. + */ +public class DirtyState { + @Nullable + private final byte[] canonicalBytes; + + DirtyState(@Nullable byte[] canonicalBytes) { + this.canonicalBytes = canonicalBytes; + } + + public boolean isClean() { + return this == isClean; + } + + public boolean didNotConverge() { + return this == didNotConverge; + } + + private byte[] canonicalBytes() { + if (canonicalBytes == null) { + throw new IllegalStateException("First make sure that {@code !isClean()} and {@code !didNotConverge()}"); + } + return canonicalBytes; + } + + public void writeCanonicalTo(File file) throws IOException { + Files.write(file.toPath(), canonicalBytes()); + } + + public void writeCanonicalTo(OutputStream out) throws IOException { + out.write(canonicalBytes()); + } + + /** Returns the DirtyState which corresponds to {@code isClean()}. */ + public static DirtyState clean() { + return isClean; + } + + static final DirtyState didNotConverge = new DirtyState(null); + static final DirtyState isClean = new DirtyState(null); + + public static Calculation of(Formatter formatter, File file) throws IOException { + return of(formatter, file, Files.readAllBytes(file.toPath())); + } + + public static Calculation of(Formatter formatter, File file, byte[] rawBytes) { + return new Calculation(formatter, file, rawBytes); + } + + public static class Calculation { + private final Formatter formatter; + private final File file; + private final byte[] rawBytes; + private final String raw; + + private Calculation(Formatter formatter, File file, byte[] rawBytes) { + this.formatter = formatter; + this.file = file; + this.rawBytes = rawBytes; + this.raw = new String(rawBytes, formatter.getEncoding()); + // check that all characters were encodable + String encodingError = EncodingErrorMsg.msg(raw, rawBytes, formatter.getEncoding()); + if (encodingError != null) { + throw new IllegalArgumentException(encodingError); + } + } + + /** + * Calculates whether the given file is dirty according to a PaddedCell invocation of the given formatter. + * DirtyState includes the clean state of the file, as well as a warning if we were not able to apply the formatter + * due to diverging idempotence. + */ + public DirtyState calculateDirtyState() { + String rawUnix = LineEnding.toUnix(raw); + + // enforce the format + String formattedUnix = formatter.compute(rawUnix, file); + // convert the line endings if necessary + String formatted = formatter.computeLineEndings(formattedUnix, file); + + // if F(input) == input, then the formatter is well-behaving and the input is clean + byte[] formattedBytes = formatted.getBytes(formatter.getEncoding()); + if (Arrays.equals(rawBytes, formattedBytes)) { + return isClean; + } + + // F(input) != input, so we'll do a padded check + String doubleFormattedUnix = formatter.compute(formattedUnix, file); + if (doubleFormattedUnix.equals(formattedUnix)) { + // most dirty files are idempotent-dirty, so this is a quick-short circuit for that common case + return new DirtyState(formattedBytes); + } + + PaddedCell cell = PaddedCell.check(formatter, file, rawUnix); + if (!cell.isResolvable()) { + return didNotConverge; + } + + // get the canonical bytes + String canonicalUnix = cell.canonical(); + String canonical = formatter.computeLineEndings(canonicalUnix, file); + byte[] canonicalBytes = canonical.getBytes(formatter.getEncoding()); + if (!Arrays.equals(rawBytes, canonicalBytes)) { + // and write them to disk if needed + return new DirtyState(canonicalBytes); + } else { + return isClean; + } + } + } +} diff --git a/lib/src/main/java/com/diffplug/spotless/PaddedCell.java b/lib/src/main/java/com/diffplug/spotless/PaddedCell.java index 910fa940d3..b0cb64ae90 100644 --- a/lib/src/main/java/com/diffplug/spotless/PaddedCell.java +++ b/lib/src/main/java/com/diffplug/spotless/PaddedCell.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. @@ -18,19 +18,14 @@ import static com.diffplug.spotless.LibPreconditions.requireElementsNonNull; import java.io.File; -import java.io.IOException; -import java.io.OutputStream; import java.nio.file.Files; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.Comparator; import java.util.List; import java.util.Objects; import java.util.function.Function; -import javax.annotation.Nullable; - /** * Models the result of applying a {@link Formatter} on a given {@link File} * while characterizing various failure modes (slow convergence, cycles, and divergence). @@ -176,108 +171,4 @@ public String userMessage() { } // @formatter:on } - - /** - * Calculates whether the given file is dirty according to a PaddedCell invocation of the given formatter. - * DirtyState includes the clean state of the file, as well as a warning if we were not able to apply the formatter - * due to diverging idempotence. - */ - public static DirtyState calculateDirtyState(Formatter formatter, File file) throws IOException { - Objects.requireNonNull(formatter, "formatter"); - Objects.requireNonNull(file, "file"); - - byte[] rawBytes = Files.readAllBytes(file.toPath()); - return calculateDirtyState(formatter, file, rawBytes); - } - - public static DirtyState calculateDirtyState(Formatter formatter, File file, byte[] rawBytes) throws IOException { - String raw = new String(rawBytes, formatter.getEncoding()); - // check that all characters were encodable - String encodingError = EncodingErrorMsg.msg(raw, rawBytes, formatter.getEncoding()); - if (encodingError != null) { - throw new IllegalArgumentException(encodingError); - } - String rawUnix = LineEnding.toUnix(raw); - - // enforce the format - String formattedUnix = formatter.compute(rawUnix, file); - // convert the line endings if necessary - String formatted = formatter.computeLineEndings(formattedUnix, file); - - // if F(input) == input, then the formatter is well-behaving and the input is clean - byte[] formattedBytes = formatted.getBytes(formatter.getEncoding()); - if (Arrays.equals(rawBytes, formattedBytes)) { - return isClean; - } - - // F(input) != input, so we'll do a padded check - String doubleFormattedUnix = formatter.compute(formattedUnix, file); - if (doubleFormattedUnix.equals(formattedUnix)) { - // most dirty files are idempotent-dirty, so this is a quick-short circuit for that common case - return new DirtyState(formattedBytes); - } - - PaddedCell cell = PaddedCell.check(formatter, file, rawUnix); - if (!cell.isResolvable()) { - return didNotConverge; - } - - // get the canonical bytes - String canonicalUnix = cell.canonical(); - String canonical = formatter.computeLineEndings(canonicalUnix, file); - byte[] canonicalBytes = canonical.getBytes(formatter.getEncoding()); - if (!Arrays.equals(rawBytes, canonicalBytes)) { - // and write them to disk if needed - return new DirtyState(canonicalBytes); - } else { - return isClean; - } - } - - /** - * The clean/dirty state of a single file. Intended use: - * - {@link #isClean()} means that the file is clean, and there's nothing else to say - * - {@link #didNotConverge()} means that we were unable to determine a clean state - * - once you've tested the above conditions and you know that it's a dirty file with a converged state, - * then you can call {@link #writeCanonicalTo(OutputStream)} to get the canonical form of the given file. - */ - public static class DirtyState { - @Nullable - private final byte[] canonicalBytes; - - private DirtyState(@Nullable byte[] canonicalBytes) { - this.canonicalBytes = canonicalBytes; - } - - public boolean isClean() { - return this == isClean; - } - - public boolean didNotConverge() { - return this == didNotConverge; - } - - private byte[] canonicalBytes() { - if (canonicalBytes == null) { - throw new IllegalStateException("First make sure that {@code !isClean()} and {@code !didNotConverge()}"); - } - return canonicalBytes; - } - - public void writeCanonicalTo(File file) throws IOException { - Files.write(file.toPath(), canonicalBytes()); - } - - public void writeCanonicalTo(OutputStream out) throws IOException { - out.write(canonicalBytes()); - } - } - - /** Returns the DirtyState which corresponds to {@code isClean()}. */ - public static DirtyState isClean() { - return isClean; - } - - private static final DirtyState didNotConverge = new DirtyState(null); - private static final DirtyState isClean = new DirtyState(null); } diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/IdeHook.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/IdeHook.java index 8d24e2230e..4c198dd42c 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/IdeHook.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/IdeHook.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. @@ -21,8 +21,8 @@ import com.diffplug.common.base.Errors; import com.diffplug.common.io.ByteStreams; +import com.diffplug.spotless.DirtyState; import com.diffplug.spotless.Formatter; -import com.diffplug.spotless.PaddedCell; class IdeHook { final static String PROPERTY = "spotlessIdeHook"; @@ -55,7 +55,7 @@ static void performHook(SpotlessTaskImpl spotlessTask) { } else { bytes = Files.readAllBytes(file.toPath()); } - PaddedCell.DirtyState dirty = PaddedCell.calculateDirtyState(formatter, file, bytes); + DirtyState dirty = DirtyState.of(formatter, file, bytes).calculateDirtyState(); if (dirty.isClean()) { dumpIsClean(); } else if (dirty.didNotConverge()) { diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java index 04951a4577..cc694942c1 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.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. @@ -37,8 +37,8 @@ import com.diffplug.common.annotations.VisibleForTesting; import com.diffplug.common.base.StringPrinter; +import com.diffplug.spotless.DirtyState; import com.diffplug.spotless.Formatter; -import com.diffplug.spotless.PaddedCell; import com.diffplug.spotless.extra.GitRatchet; @CacheableTask @@ -97,12 +97,12 @@ public void performAction(InputChanges inputs) throws Exception { void processInputFile(@Nullable GitRatchet ratchet, Formatter formatter, File input) throws IOException { File output = getOutputFile(input); getLogger().debug("Applying format to {} and writing to {}", input, output); - PaddedCell.DirtyState dirtyState; + DirtyState dirtyState; if (ratchet != null && ratchet.isClean(getProjectDir().get().getAsFile(), getRootTreeSha(), input)) { - dirtyState = PaddedCell.isClean(); + dirtyState = DirtyState.clean(); } else { try { - dirtyState = PaddedCell.calculateDirtyState(formatter, input); + dirtyState = DirtyState.of(formatter, input).calculateDirtyState(); } catch (IOException e) { throw new IOException("Issue processing file: " + input, e); } catch (RuntimeException e) { diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/IdeHook.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/IdeHook.java index 0001046871..5289af4756 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/IdeHook.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/IdeHook.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,8 +21,8 @@ import com.diffplug.common.base.Errors; import com.diffplug.common.io.ByteStreams; +import com.diffplug.spotless.DirtyState; import com.diffplug.spotless.Formatter; -import com.diffplug.spotless.PaddedCell; class IdeHook { @@ -49,7 +49,7 @@ static void performHook(Iterable projectFiles, Formatter formatter, String } else { bytes = Files.readAllBytes(file.toPath()); } - PaddedCell.DirtyState dirty = PaddedCell.calculateDirtyState(formatter, file, bytes); + DirtyState dirty = DirtyState.of(formatter, file, bytes).calculateDirtyState(); if (dirty.isClean()) { dumpIsClean(); } else if (dirty.didNotConverge()) { diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessApplyMojo.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessApplyMojo.java index 07cc2cbc85..317c61f80c 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessApplyMojo.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessApplyMojo.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. @@ -22,8 +22,8 @@ import org.apache.maven.plugins.annotations.Mojo; import org.apache.maven.plugins.annotations.Parameter; +import com.diffplug.spotless.DirtyState; import com.diffplug.spotless.Formatter; -import com.diffplug.spotless.PaddedCell; import com.diffplug.spotless.maven.incremental.UpToDateChecker; /** @@ -60,7 +60,7 @@ protected void process(Iterable files, Formatter formatter, UpToDateChecke } try { - PaddedCell.DirtyState dirtyState = PaddedCell.calculateDirtyState(formatter, file); + DirtyState dirtyState = DirtyState.of(formatter, file).calculateDirtyState(); if (!dirtyState.isClean() && !dirtyState.didNotConverge()) { getLog().info(String.format("Writing clean file: %s", file)); dirtyState.writeCanonicalTo(file); diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java index b326767c3c..22d91de1cf 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java @@ -27,8 +27,8 @@ import org.apache.maven.plugins.annotations.Parameter; import org.sonatype.plexus.build.incremental.BuildContext; +import com.diffplug.spotless.DirtyState; import com.diffplug.spotless.Formatter; -import com.diffplug.spotless.PaddedCell; import com.diffplug.spotless.extra.integration.DiffMessageFormatter; import com.diffplug.spotless.maven.incremental.UpToDateChecker; @@ -78,7 +78,7 @@ protected void process(Iterable files, Formatter formatter, UpToDateChecke } buildContext.removeMessages(file); try { - PaddedCell.DirtyState dirtyState = PaddedCell.calculateDirtyState(formatter, file); + DirtyState dirtyState = DirtyState.of(formatter, file).calculateDirtyState(); if (!dirtyState.isClean() && !dirtyState.didNotConverge()) { problemFiles.add(file); if (buildContext.isIncremental()) { From 856ab9fc3fc014b946d0b37cbd6835c9c39d68e5 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sun, 1 Jan 2023 22:38:11 -0800 Subject: [PATCH 03/13] Removed `rootDir` property `Formatter` and its builder because it was only used to annnotate errors, which will now done by the lint phase. --- .../integration/DiffMessageFormatter.java | 16 +++++---- .../java/com/diffplug/spotless/Formatter.java | 33 +++---------------- .../diffplug/spotless/generic/FenceStep.java | 2 -- .../gradle/spotless/SpotlessTask.java | 1 - .../spotless/maven/AbstractSpotlessMojo.java | 2 +- .../spotless/maven/FormatterFactory.java | 1 - .../spotless/maven/SpotlessCheckMojo.java | 4 +-- .../maven/incremental/NoopCheckerTest.java | 6 +--- .../incremental/PluginFingerprintTest.java | 4 --- .../com/diffplug/spotless/StepHarness.java | 4 --- .../spotless/StepHarnessWithFile.java | 1 - .../com/diffplug/spotless/FormatterTest.java | 26 +-------------- .../com/diffplug/spotless/PaddedCellTest.java | 1 - 13 files changed, 19 insertions(+), 82 deletions(-) diff --git a/lib-extra/src/main/java/com/diffplug/spotless/extra/integration/DiffMessageFormatter.java b/lib-extra/src/main/java/com/diffplug/spotless/extra/integration/DiffMessageFormatter.java index 9294055a4f..a0d056bc0a 100644 --- a/lib-extra/src/main/java/com/diffplug/spotless/extra/integration/DiffMessageFormatter.java +++ b/lib-extra/src/main/java/com/diffplug/spotless/extra/integration/DiffMessageFormatter.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. @@ -58,15 +58,17 @@ interface CleanProvider { } private static class CleanProviderFormatter implements CleanProvider { + private final Path rootDir; private final Formatter formatter; - CleanProviderFormatter(Formatter formatter) { + CleanProviderFormatter(Path rootDir, Formatter formatter) { + this.rootDir = Objects.requireNonNull(rootDir); this.formatter = Objects.requireNonNull(formatter); } @Override public Path getRootDir() { - return formatter.getRootDir(); + return rootDir; } @Override @@ -123,8 +125,8 @@ public Builder runToFix(String runToFix) { return this; } - public Builder formatter(Formatter formatter) { - this.formatter = new CleanProviderFormatter(formatter); + public Builder formatter(Path rootDir, Formatter formatter) { + this.formatter = new CleanProviderFormatter(rootDir, formatter); return this; } @@ -244,8 +246,8 @@ private String diff(File file) throws IOException { * look like if formatted using the given formatter. Does not end with any newline * sequence (\n, \r, \r\n). The key of the map entry is the 0-based line where the first difference occurred. */ - public static Map.Entry diff(Formatter formatter, File file) throws IOException { - return diff(new CleanProviderFormatter(formatter), file); + public static Map.Entry diff(Path rootDir, Formatter formatter, File file) throws IOException { + return diff(new CleanProviderFormatter(rootDir, formatter), file); } private static Map.Entry diff(CleanProvider formatter, File file) throws IOException { diff --git a/lib/src/main/java/com/diffplug/spotless/Formatter.java b/lib/src/main/java/com/diffplug/spotless/Formatter.java index 4dea2e3d3d..bcc131f0a8 100644 --- a/lib/src/main/java/com/diffplug/spotless/Formatter.java +++ b/lib/src/main/java/com/diffplug/spotless/Formatter.java @@ -24,8 +24,6 @@ import java.io.ObjectStreamException; import java.io.Serializable; import java.nio.charset.Charset; -import java.nio.file.Path; -import java.nio.file.Paths; import java.util.ArrayList; import java.util.List; import java.util.Objects; @@ -38,15 +36,12 @@ public final class Formatter implements Serializable, AutoCloseable { private String name; private LineEnding.Policy lineEndingsPolicy; private Charset encoding; - private Path rootDir; private List steps; private FormatExceptionPolicy exceptionPolicy; - private Formatter(String name, LineEnding.Policy lineEndingsPolicy, Charset encoding, Path rootDirectory, List steps, FormatExceptionPolicy exceptionPolicy) { - this.name = name; + private Formatter(LineEnding.Policy lineEndingsPolicy, Charset encoding, List steps, FormatExceptionPolicy exceptionPolicy) { this.lineEndingsPolicy = Objects.requireNonNull(lineEndingsPolicy, "lineEndingsPolicy"); this.encoding = Objects.requireNonNull(encoding, "encoding"); - this.rootDir = Objects.requireNonNull(rootDirectory, "rootDir"); this.steps = requireElementsNonNull(new ArrayList<>(steps)); this.exceptionPolicy = Objects.requireNonNull(exceptionPolicy, "exceptionPolicy"); } @@ -56,7 +51,6 @@ private void writeObject(ObjectOutputStream out) throws IOException { out.writeObject(name); out.writeObject(lineEndingsPolicy); out.writeObject(encoding.name()); - out.writeObject(rootDir.toString()); out.writeObject(steps); out.writeObject(exceptionPolicy); } @@ -67,7 +61,6 @@ private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundE name = (String) in.readObject(); 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(); } @@ -90,10 +83,6 @@ public Charset getEncoding() { return encoding; } - public Path getRootDir() { - return rootDir; - } - public List getSteps() { return steps; } @@ -112,7 +101,6 @@ public static class Builder { // required parameters private LineEnding.Policy lineEndingsPolicy; private Charset encoding; - private Path rootDir; private List steps; private FormatExceptionPolicy exceptionPolicy; @@ -133,11 +121,6 @@ public Builder encoding(Charset encoding) { return this; } - public Builder rootDir(Path rootDir) { - this.rootDir = rootDir; - return this; - } - public Builder steps(List steps) { this.steps = steps; return this; @@ -149,7 +132,7 @@ public Builder exceptionPolicy(FormatExceptionPolicy exceptionPolicy) { } public Formatter build() { - return new Formatter(name, lineEndingsPolicy, encoding, rootDir, steps, + return new Formatter(lineEndingsPolicy, encoding, steps, exceptionPolicy == null ? FormatExceptionPolicy.failOnlyOnError() : exceptionPolicy); } } @@ -188,13 +171,9 @@ public String compute(String unix, File file) { unix = LineEnding.toUnix(formatted); } } catch (Throwable e) { - if (file == NO_FILE_SENTINEL) { - exceptionPolicy.handleError(e, step, ""); - } else { - // Path may be forged from a different FileSystem than Filesystem.default - String relativePath = rootDir.relativize(rootDir.getFileSystem().getPath(file.getPath())).toString(); - exceptionPolicy.handleError(e, step, relativePath); - } + // TODO: this is not accurate, but it won't matter when add support for linting + String relativePath = file.toString(); + exceptionPolicy.handleError(e, step, relativePath); } } return unix; @@ -207,7 +186,6 @@ public int hashCode() { result = prime * result + name.hashCode(); result = prime * result + encoding.hashCode(); result = prime * result + lineEndingsPolicy.hashCode(); - result = prime * result + rootDir.hashCode(); result = prime * result + steps.hashCode(); result = prime * result + exceptionPolicy.hashCode(); return result; @@ -228,7 +206,6 @@ public boolean equals(Object obj) { return name.equals(other.name) && encoding.equals(other.encoding) && lineEndingsPolicy.equals(other.lineEndingsPolicy) && - rootDir.equals(other.rootDir) && steps.equals(other.steps) && exceptionPolicy.equals(other.exceptionPolicy); } diff --git a/lib/src/main/java/com/diffplug/spotless/generic/FenceStep.java b/lib/src/main/java/com/diffplug/spotless/generic/FenceStep.java index 1db94d0885..9ce34675f8 100644 --- a/lib/src/main/java/com/diffplug/spotless/generic/FenceStep.java +++ b/lib/src/main/java/com/diffplug/spotless/generic/FenceStep.java @@ -18,7 +18,6 @@ import java.io.File; import java.io.Serializable; import java.nio.charset.StandardCharsets; -import java.nio.file.Path; import java.util.ArrayList; import java.util.List; import java.util.Objects; @@ -177,7 +176,6 @@ protected Formatter buildFormatter() { .encoding(StandardCharsets.UTF_8) // can be any UTF, doesn't matter .lineEndingsPolicy(LineEnding.UNIX.createPolicy()) // just internal, won't conflict with user .steps(steps) - .rootDir(Path.of("")) // TODO: error messages will be suboptimal for now, but it will get fixed when we ship linting .build(); } 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 f930685baf..9a0962ec09 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 @@ -178,7 +178,6 @@ Formatter buildFormatter() { .name(formatName()) .lineEndingsPolicy(getLineEndingsPolicy().get()) .encoding(Charset.forName(encoding)) - .rootDir(getProjectDir().get().getAsFile().toPath()) .steps(steps) .exceptionPolicy(exceptionPolicy) .build(); diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java index 9f4d0e01bc..68b8571182 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java @@ -121,7 +121,7 @@ public abstract class AbstractSpotlessMojo extends AbstractMojo { private List repositories; @Parameter(defaultValue = "${project.basedir}", required = true, readonly = true) - private File baseDir; + protected File baseDir; @Parameter(defaultValue = "${project.build.directory}", required = true, readonly = true) private File buildDir; diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterFactory.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterFactory.java index 6393c65858..01d765f95b 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterFactory.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterFactory.java @@ -107,7 +107,6 @@ public final Formatter newFormatter(Supplier> filesToFormat, Form .lineEndingsPolicy(formatterLineEndingPolicy) .exceptionPolicy(new FormatExceptionPolicyStrict()) .steps(formatterSteps) - .rootDir(config.getFileLocator().getBaseDir().toPath()) .build(); } diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java index 22d91de1cf..07a8e0eda4 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java @@ -82,7 +82,7 @@ protected void process(Iterable files, Formatter formatter, UpToDateChecke if (!dirtyState.isClean() && !dirtyState.didNotConverge()) { problemFiles.add(file); if (buildContext.isIncremental()) { - Map.Entry diffEntry = DiffMessageFormatter.diff(formatter, file); + Map.Entry diffEntry = DiffMessageFormatter.diff(baseDir.toPath(), formatter, file); buildContext.addMessage(file, diffEntry.getKey() + 1, 0, INCREMENTAL_MESSAGE_PREFIX + diffEntry.getValue(), m2eIncrementalBuildMessageSeverity.getSeverity(), null); } counter.cleaned(); @@ -106,7 +106,7 @@ protected void process(Iterable files, Formatter formatter, UpToDateChecke if (!problemFiles.isEmpty()) { throw new MojoExecutionException(DiffMessageFormatter.builder() .runToFix("Run 'mvn spotless:apply' to fix these violations.") - .formatter(formatter) + .formatter(baseDir.toPath(), formatter) .problemFiles(problemFiles) .getMessage()); } diff --git a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/NoopCheckerTest.java b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/NoopCheckerTest.java index 404b5e2191..4025d5b271 100644 --- a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/NoopCheckerTest.java +++ b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/NoopCheckerTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2021-2022 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. @@ -28,7 +28,6 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.Paths; import org.apache.maven.model.Build; import org.apache.maven.model.Plugin; @@ -37,7 +36,6 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import com.diffplug.spotless.FormatExceptionPolicyStrict; import com.diffplug.spotless.Formatter; import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.LineEnding; @@ -116,11 +114,9 @@ private MavenProject buildMavenProject() throws IOException { private static Formatter dummyFormatter() { return Formatter.builder() - .rootDir(Paths.get("")) .lineEndingsPolicy(LineEnding.UNIX.createPolicy()) .encoding(UTF_8) .steps(singletonList(mock(FormatterStep.class, withSettings().serializable()))) - .exceptionPolicy(new FormatExceptionPolicyStrict()) .build(); } } diff --git a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/PluginFingerprintTest.java b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/PluginFingerprintTest.java index 4257e5595c..a0d55da86e 100644 --- a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/PluginFingerprintTest.java +++ b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/PluginFingerprintTest.java @@ -21,7 +21,6 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import java.io.ByteArrayInputStream; -import java.nio.file.Paths; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -35,7 +34,6 @@ import org.codehaus.plexus.util.xml.XmlStreamReader; import org.junit.jupiter.api.Test; -import com.diffplug.spotless.FormatExceptionPolicyStrict; import com.diffplug.spotless.Formatter; import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.LineEnding; @@ -173,11 +171,9 @@ private static Formatter formatter(FormatterStep... steps) { private static Formatter formatter(LineEnding lineEnding, FormatterStep... steps) { return Formatter.builder() - .rootDir(Paths.get("")) .lineEndingsPolicy(lineEnding.createPolicy()) .encoding(UTF_8) .steps(Arrays.asList(steps)) - .exceptionPolicy(new FormatExceptionPolicyStrict()) .build(); } } diff --git a/testlib/src/main/java/com/diffplug/spotless/StepHarness.java b/testlib/src/main/java/com/diffplug/spotless/StepHarness.java index 6bec145f66..f4f95ca502 100644 --- a/testlib/src/main/java/com/diffplug/spotless/StepHarness.java +++ b/testlib/src/main/java/com/diffplug/spotless/StepHarness.java @@ -19,7 +19,6 @@ import java.io.File; import java.nio.charset.StandardCharsets; -import java.nio.file.Paths; import java.util.Arrays; import org.assertj.core.api.AbstractStringAssert; @@ -42,8 +41,6 @@ public static StepHarness forSteps(FormatterStep... steps) { .steps(Arrays.asList(steps)) .lineEndingsPolicy(LineEnding.UNIX.createPolicy()) .encoding(StandardCharsets.UTF_8) - .rootDir(Paths.get("")) - .exceptionPolicy(new FormatExceptionPolicyStrict()) .build()); } @@ -57,7 +54,6 @@ public static StepHarness forStepNoRoundtrip(FormatterStep step) { .steps(Arrays.asList(step)) .lineEndingsPolicy(LineEnding.UNIX.createPolicy()) .encoding(StandardCharsets.UTF_8) - .rootDir(Paths.get("")) .exceptionPolicy(new FormatExceptionPolicyStrict()) .build(), RoundTrip.DONT_ROUNDTRIP); } diff --git a/testlib/src/main/java/com/diffplug/spotless/StepHarnessWithFile.java b/testlib/src/main/java/com/diffplug/spotless/StepHarnessWithFile.java index 871da6f385..38bc8ce074 100644 --- a/testlib/src/main/java/com/diffplug/spotless/StepHarnessWithFile.java +++ b/testlib/src/main/java/com/diffplug/spotless/StepHarnessWithFile.java @@ -41,7 +41,6 @@ public static StepHarnessWithFile forStep(ResourceHarness harness, FormatterStep .encoding(StandardCharsets.UTF_8) .lineEndingsPolicy(LineEnding.UNIX.createPolicy()) .steps(Collections.singletonList(step)) - .rootDir(harness.rootFolder().toPath()) .exceptionPolicy(new FormatExceptionPolicyStrict()) .build()); } diff --git a/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java b/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java index 314507bd8d..3d3eb03c59 100644 --- a/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/FormatterTest.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. @@ -47,9 +47,7 @@ void equality() { new SerializableEqualityTester() { 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 { @@ -61,25 +59,8 @@ protected void setupTest(API api) throws Exception { encoding = StandardCharsets.UTF_16; api.areDifferentThan(); - rootDir = rootDir.getParent(); - api.areDifferentThan(); - steps.add(EndWithNewlineStep.create()); api.areDifferentThan(); - - { - FormatExceptionPolicyStrict standard = new FormatExceptionPolicyStrict(); - standard.excludePath("path"); - exceptionPolicy = standard; - api.areDifferentThan(); - } - - { - FormatExceptionPolicyStrict standard = new FormatExceptionPolicyStrict(); - standard.excludeStep("step"); - exceptionPolicy = standard; - api.areDifferentThan(); - } } @Override @@ -87,9 +68,7 @@ protected Formatter create() { return Formatter.builder() .lineEndingsPolicy(lineEndingsPolicy) .encoding(encoding) - .rootDir(rootDir) .steps(steps) - .exceptionPolicy(exceptionPolicy) .build(); } }.testEquals(); @@ -112,7 +91,6 @@ public void testExceptionWithEmptyPath() throws Exception { Formatter formatter = Formatter.builder() .lineEndingsPolicy(lineEndingsPolicy) .encoding(encoding) - .rootDir(rootDir) .steps(steps) .exceptionPolicy(exceptionPolicy) .build(); @@ -137,7 +115,6 @@ public void testExceptionWithSentinelNoFileOnDisk() throws Exception { Formatter formatter = Formatter.builder() .lineEndingsPolicy(lineEndingsPolicy) .encoding(encoding) - .rootDir(rootDir) .steps(steps) .exceptionPolicy(exceptionPolicy) .build(); @@ -177,7 +154,6 @@ public void testExceptionWithRootDirIsNotFileSystem() throws Exception { Formatter formatter = Formatter.builder() .lineEndingsPolicy(lineEndingsPolicy) .encoding(encoding) - .rootDir(rootDir) .steps(steps) .exceptionPolicy(exceptionPolicy) .build(); diff --git a/testlib/src/test/java/com/diffplug/spotless/PaddedCellTest.java b/testlib/src/test/java/com/diffplug/spotless/PaddedCellTest.java index 28fc6a0710..2820aadeef 100644 --- a/testlib/src/test/java/com/diffplug/spotless/PaddedCellTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/PaddedCellTest.java @@ -51,7 +51,6 @@ private void testCase(SerializedFunction step, String input, Pad try (Formatter formatter = Formatter.builder() .lineEndingsPolicy(LineEnding.UNIX.createPolicy()) .encoding(StandardCharsets.UTF_8) - .rootDir(rootFolder.toPath()) .steps(formatterSteps).build()) { File file = new File(rootFolder, "input"); From b7f559a9fe7bc7d33cc6176b3df9a379eaed2bf6 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 29 May 2024 14:56:22 -0700 Subject: [PATCH 04/13] Mark the tests which are broken as part of the lint refactor. --- ...GrEclipseFormatterStepSpecialCaseTest.java | 6 +++- .../spotless/tag/ForLintRefactor.java | 30 +++++++++++++++++++ .../com/diffplug/spotless/FormatterTest.java | 24 --------------- .../gherkin/GherkinUtilsStepTest.java | 7 ++++- .../spotless/json/JsonSimpleStepTest.java | 8 ++++- .../spotless/json/gson/GsonStepTest.java | 6 +++- 6 files changed, 53 insertions(+), 28 deletions(-) create mode 100644 testlib/src/main/java/com/diffplug/spotless/tag/ForLintRefactor.java diff --git a/lib-extra/src/test/java/com/diffplug/spotless/extra/groovy/GrEclipseFormatterStepSpecialCaseTest.java b/lib-extra/src/test/java/com/diffplug/spotless/extra/groovy/GrEclipseFormatterStepSpecialCaseTest.java index 0702881ef9..8064633a4e 100644 --- a/lib-extra/src/test/java/com/diffplug/spotless/extra/groovy/GrEclipseFormatterStepSpecialCaseTest.java +++ b/lib-extra/src/test/java/com/diffplug/spotless/extra/groovy/GrEclipseFormatterStepSpecialCaseTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2023 DiffPlug + * 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. @@ -16,10 +16,12 @@ package com.diffplug.spotless.extra.groovy; import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import com.diffplug.spotless.StepHarness; import com.diffplug.spotless.TestProvisioner; +import com.diffplug.spotless.tag.ForLintRefactor; public class GrEclipseFormatterStepSpecialCaseTest { /** @@ -29,6 +31,8 @@ public class GrEclipseFormatterStepSpecialCaseTest { * works: ${parm == null ? "" : "parm"} */ @Test + @Disabled + @ForLintRefactor public void issue_1657() { Assertions.assertThrows(RuntimeException.class, () -> { StepHarness.forStep(GrEclipseFormatterStep.createBuilder(TestProvisioner.mavenCentral()).build()) diff --git a/testlib/src/main/java/com/diffplug/spotless/tag/ForLintRefactor.java b/testlib/src/main/java/com/diffplug/spotless/tag/ForLintRefactor.java new file mode 100644 index 0000000000..a6ac9b090b --- /dev/null +++ b/testlib/src/main/java/com/diffplug/spotless/tag/ForLintRefactor.java @@ -0,0 +1,30 @@ +/* + * 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. + * 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.tag; + +import static java.lang.annotation.ElementType.METHOD; +import static java.lang.annotation.ElementType.TYPE; +import static java.lang.annotation.RetentionPolicy.RUNTIME; + +import java.lang.annotation.Retention; +import java.lang.annotation.Target; + +import org.junit.jupiter.api.Tag; + +@Target({TYPE, METHOD}) +@Retention(RUNTIME) +@Tag("black") +public @interface ForLintRefactor {} diff --git a/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java b/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java index 3d3eb03c59..8114d75cab 100644 --- a/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java @@ -74,30 +74,6 @@ protected Formatter create() { }.testEquals(); } - // new File("") as filePath is known to fail - @Test - public void testExceptionWithEmptyPath() throws Exception { - LineEnding.Policy lineEndingsPolicy = LineEnding.UNIX.createPolicy(); - Charset encoding = StandardCharsets.UTF_8; - FormatExceptionPolicy exceptionPolicy = FormatExceptionPolicy.failOnlyOnError(); - - Path rootDir = Paths.get(StandardSystemProperty.USER_DIR.value()); - - FormatterStep step = Mockito.mock(FormatterStep.class); - Mockito.when(step.getName()).thenReturn("someFailingStep"); - Mockito.when(step.format(Mockito.anyString(), Mockito.any(File.class))).thenThrow(new IllegalArgumentException("someReason")); - List steps = Collections.singletonList(step); - - Formatter formatter = Formatter.builder() - .lineEndingsPolicy(lineEndingsPolicy) - .encoding(encoding) - .steps(steps) - .exceptionPolicy(exceptionPolicy) - .build(); - - Assertions.assertThrows(IllegalArgumentException.class, () -> formatter.compute("someFileContent", new File(""))); - } - // If there is no File actually holding the content, one may rely on Formatter.NO_FILE_ON_DISK @Test public void testExceptionWithSentinelNoFileOnDisk() throws Exception { diff --git a/testlib/src/test/java/com/diffplug/spotless/gherkin/GherkinUtilsStepTest.java b/testlib/src/test/java/com/diffplug/spotless/gherkin/GherkinUtilsStepTest.java index 6bc478027b..5ffaa029ae 100644 --- a/testlib/src/test/java/com/diffplug/spotless/gherkin/GherkinUtilsStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/gherkin/GherkinUtilsStepTest.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. @@ -24,6 +24,7 @@ import com.diffplug.spotless.SerializableEqualityTester; import com.diffplug.spotless.StepHarness; import com.diffplug.spotless.TestProvisioner; +import com.diffplug.spotless.tag.ForLintRefactor; public class GherkinUtilsStepTest { @@ -59,6 +60,8 @@ public void handlesRuleWithTag() throws Exception { } @Test + @Disabled + @ForLintRefactor public void handlesInvalidGherkin() { assertThatThrownBy(() -> doWithResource(stepHarness, "invalidGherkin")) .isInstanceOf(IllegalArgumentException.class) @@ -66,6 +69,8 @@ public void handlesInvalidGherkin() { } @Test + @Disabled + @ForLintRefactor public void handlesNotGherkin() { assertThatThrownBy(() -> doWithResource(stepHarness, "notGherkin")) .isInstanceOf(IllegalArgumentException.class) diff --git a/testlib/src/test/java/com/diffplug/spotless/json/JsonSimpleStepTest.java b/testlib/src/test/java/com/diffplug/spotless/json/JsonSimpleStepTest.java index 19edc1a8e8..5d7c08f129 100644 --- a/testlib/src/test/java/com/diffplug/spotless/json/JsonSimpleStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/json/JsonSimpleStepTest.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. @@ -17,12 +17,14 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.SerializableEqualityTester; import com.diffplug.spotless.StepHarness; import com.diffplug.spotless.TestProvisioner; +import com.diffplug.spotless.tag.ForLintRefactor; class JsonSimpleStepTest { @@ -72,12 +74,16 @@ void handlesObjectWithNull() { } @Test + @Disabled + @ForLintRefactor void handlesInvalidJson() { stepHarness.testResourceExceptionMsg("json/invalidJsonBefore.json") .contains("Expected a ',' or '}' at 9 [character 0 line 3]"); } @Test + @Disabled + @ForLintRefactor void handlesNotJson() { stepHarness.testResourceExceptionMsg("json/notJsonBefore.json") .contains("Unable to determine JSON type, expected a '{' or '[' but found '#'"); diff --git a/testlib/src/test/java/com/diffplug/spotless/json/gson/GsonStepTest.java b/testlib/src/test/java/com/diffplug/spotless/json/gson/GsonStepTest.java index 3f4d2a84e0..29c65f8493 100644 --- a/testlib/src/test/java/com/diffplug/spotless/json/gson/GsonStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/json/gson/GsonStepTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2022-2023 DiffPlug + * Copyright 2022-2024 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,6 +18,7 @@ import java.io.File; import org.assertj.core.api.Assertions; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import com.diffplug.spotless.FormatterStep; @@ -25,6 +26,7 @@ import com.diffplug.spotless.StepHarness; import com.diffplug.spotless.TestProvisioner; import com.diffplug.spotless.json.JsonFormatterStepCommonTests; +import com.diffplug.spotless.tag.ForLintRefactor; public class GsonStepTest extends JsonFormatterStepCommonTests { @@ -41,6 +43,8 @@ void handlesObjectWithNull() { } @Test + @Disabled + @ForLintRefactor void handlesInvalidJson() { getStepHarness().testResourceExceptionMsg("json/invalidJsonBefore.json").isEqualTo("End of input at line 3 column 1 path $.a"); } From 7bd9c5954a3786f58194c007f000b898c4244422 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 29 May 2024 14:58:43 -0700 Subject: [PATCH 05/13] Remove the name property from Formatter, and route it in a different way. --- .../java/com/diffplug/spotless/Formatter.java | 18 +--------- .../gradle/spotless/SpotlessTask.java | 1 - .../spotless/maven/AbstractSpotlessMojo.java | 13 ++++--- .../spotless/maven/FormatterFactory.java | 2 -- .../spotless/maven/FormattersHolder.java | 36 +++++++++---------- .../spotless/maven/SpotlessApplyMojo.java | 6 ++-- .../spotless/maven/SpotlessCheckMojo.java | 6 ++-- .../spotless/StepHarnessWithFile.java | 1 - 8 files changed, 29 insertions(+), 54 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/Formatter.java b/lib/src/main/java/com/diffplug/spotless/Formatter.java index bcc131f0a8..40f363a786 100644 --- a/lib/src/main/java/com/diffplug/spotless/Formatter.java +++ b/lib/src/main/java/com/diffplug/spotless/Formatter.java @@ -33,7 +33,6 @@ public final class Formatter implements Serializable, AutoCloseable { private static final long serialVersionUID = 1L; // The name is used for logging purpose. It does not convey any applicative purpose - private String name; private LineEnding.Policy lineEndingsPolicy; private Charset encoding; private List steps; @@ -48,7 +47,6 @@ private Formatter(LineEnding.Policy lineEndingsPolicy, Charset encoding, List) in.readObject(); @@ -71,10 +68,6 @@ private void readObjectNoData() throws ObjectStreamException { throw new UnsupportedOperationException(); } - public String getName() { - return name; - } - public LineEnding.Policy getLineEndingsPolicy() { return lineEndingsPolicy; } @@ -96,8 +89,6 @@ public static Formatter.Builder builder() { } public static class Builder { - // optional parameters - private String name = "unnamed"; // required parameters private LineEnding.Policy lineEndingsPolicy; private Charset encoding; @@ -106,11 +97,6 @@ public static class Builder { private Builder() {} - public Builder name(String name) { - this.name = name; - return this; - } - public Builder lineEndingsPolicy(LineEnding.Policy lineEndingsPolicy) { this.lineEndingsPolicy = lineEndingsPolicy; return this; @@ -183,7 +169,6 @@ public String compute(String unix, File file) { public int hashCode() { final int prime = 31; int result = 1; - result = prime * result + name.hashCode(); result = prime * result + encoding.hashCode(); result = prime * result + lineEndingsPolicy.hashCode(); result = prime * result + steps.hashCode(); @@ -203,8 +188,7 @@ public boolean equals(Object obj) { return false; } Formatter other = (Formatter) obj; - return name.equals(other.name) && - encoding.equals(other.encoding) && + return encoding.equals(other.encoding) && lineEndingsPolicy.equals(other.lineEndingsPolicy) && steps.equals(other.steps) && exceptionPolicy.equals(other.exceptionPolicy); 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 9a0962ec09..de8f77904b 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 @@ -175,7 +175,6 @@ String formatName() { Formatter buildFormatter() { return Formatter.builder() - .name(formatName()) .lineEndingsPolicy(getLineEndingsPolicy().get()) .encoding(Charset.forName(encoding)) .steps(steps) diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java index 68b8571182..f3d940d30c 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java @@ -27,7 +27,6 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.Map.Entry; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -208,7 +207,7 @@ public abstract class AbstractSpotlessMojo extends AbstractMojo { @Parameter(defaultValue = "false") protected boolean m2eEnableForIncrementalBuild; - protected abstract void process(Iterable files, Formatter formatter, UpToDateChecker upToDateChecker) throws MojoExecutionException; + protected abstract void process(String name, Iterable files, Formatter formatter, UpToDateChecker upToDateChecker) throws MojoExecutionException; private static final int MINIMUM_JRE = 11; @@ -237,11 +236,11 @@ public final void execute() throws MojoExecutionException { } try (FormattersHolder formattersHolder = FormattersHolder.create(formatterFactoryToFiles, config); - UpToDateChecker upToDateChecker = createUpToDateChecker(formattersHolder.getFormatters())) { - for (Entry>> entry : formattersHolder.getFormattersWithFiles().entrySet()) { - Formatter formatter = entry.getKey(); - Iterable files = entry.getValue().get(); - process(files, formatter, upToDateChecker); + UpToDateChecker upToDateChecker = createUpToDateChecker(formattersHolder.openFormatters.values())) { + for (FormatterFactory factory : formattersHolder.openFormatters.keySet()) { + Formatter formatter = formattersHolder.openFormatters.get(factory); + Iterable files = formattersHolder.factoryToFiles.get(factory).get(); + process(formattersHolder.nameFor(factory), files, formatter, upToDateChecker); } } catch (PluginException e) { throw e.asMojoExecutionException(); diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterFactory.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterFactory.java index 01d765f95b..57e20d9b7c 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterFactory.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterFactory.java @@ -100,9 +100,7 @@ public final Formatter newFormatter(Supplier> filesToFormat, Form formatterSteps = List.of(toggle.createFence().preserveWithin(formatterStepsBeforeToggle)); } - String formatterName = this.getClass().getSimpleName(); return Formatter.builder() - .name(formatterName) .encoding(formatterEncoding) .lineEndingsPolicy(formatterLineEndingPolicy) .exceptionPolicy(new FormatExceptionPolicyStrict()) diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormattersHolder.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormattersHolder.java index 873321f9df..9b279c8845 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormattersHolder.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormattersHolder.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. @@ -16,62 +16,58 @@ package com.diffplug.spotless.maven; import java.io.File; +import java.util.Collection; import java.util.LinkedHashMap; import java.util.Map; import java.util.Map.Entry; -import java.util.Set; import java.util.function.Supplier; import com.diffplug.spotless.Formatter; class FormattersHolder implements AutoCloseable { + final Map openFormatters; + final Map>> factoryToFiles; - private final Map>> formatterToFiles; + FormattersHolder(Map openFormatters, Map>> factoryToFiles) { + this.openFormatters = openFormatters; + this.factoryToFiles = factoryToFiles; + } - FormattersHolder(Map>> formatterToFiles) { - this.formatterToFiles = formatterToFiles; + public String nameFor(FormatterFactory factory) { + return factory.getClass().getSimpleName(); } static FormattersHolder create(Map>> formatterFactoryToFiles, FormatterConfig config) { - Map>> formatterToFiles = new LinkedHashMap<>(); + Map openFormatters = new LinkedHashMap<>(); try { for (Entry>> entry : formatterFactoryToFiles.entrySet()) { FormatterFactory formatterFactory = entry.getKey(); Supplier> files = entry.getValue(); - Formatter formatter = formatterFactory.newFormatter(files, config); - formatterToFiles.put(formatter, files); + openFormatters.put(formatterFactory, formatter); } } catch (RuntimeException openError) { try { - close(formatterToFiles.keySet()); + close(openFormatters.values()); } catch (Exception closeError) { openError.addSuppressed(closeError); } throw openError; } - return new FormattersHolder(formatterToFiles); - } - - Iterable getFormatters() { - return formatterToFiles.keySet(); - } - - Map>> getFormattersWithFiles() { - return formatterToFiles; + return new FormattersHolder(openFormatters, formatterFactoryToFiles); } @Override public void close() { try { - close(formatterToFiles.keySet()); + close(openFormatters.values()); } catch (Exception e) { throw new RuntimeException("Unable to close formatters", e); } } - private static void close(Set formatters) throws Exception { + private static void close(Collection formatters) throws Exception { Exception error = null; for (Formatter formatter : formatters) { try { diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessApplyMojo.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessApplyMojo.java index 317c61f80c..0d67ed8c9f 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessApplyMojo.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessApplyMojo.java @@ -42,7 +42,7 @@ public class SpotlessApplyMojo extends AbstractSpotlessMojo { private boolean spotlessIdeHookUseStdOut; @Override - protected void process(Iterable files, Formatter formatter, UpToDateChecker upToDateChecker) throws MojoExecutionException { + protected void process(String name, Iterable files, Formatter formatter, UpToDateChecker upToDateChecker) throws MojoExecutionException { if (isIdeHook()) { IdeHook.performHook(files, formatter, spotlessIdeHook, spotlessIdeHookUseStdIn, spotlessIdeHookUseStdOut); return; @@ -79,9 +79,9 @@ protected void process(Iterable files, Formatter formatter, UpToDateChecke // We print the number of considered files which is useful when ratchetFrom is setup if (counter.getTotal() > 0) { getLog().info(String.format("Spotless.%s is keeping %s files clean - %s were changed to be clean, %s were already clean, %s were skipped because caching determined they were already clean", - formatter.getName(), counter.getTotal(), counter.getCleaned(), counter.getCheckedButAlreadyClean(), counter.getSkippedAsCleanCache())); + name, counter.getTotal(), counter.getCleaned(), counter.getCheckedButAlreadyClean(), counter.getSkippedAsCleanCache())); } else { - getLog().debug(String.format("Spotless.%s has no target files. Examine your ``: https://github.com/diffplug/spotless/tree/main/plugin-maven#quickstart", formatter.getName())); + getLog().debug(String.format("Spotless.%s has no target files. Examine your ``: https://github.com/diffplug/spotless/tree/main/plugin-maven#quickstart", name)); } } diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java index 07a8e0eda4..183f20ef9e 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java @@ -64,7 +64,7 @@ public int getSeverity() { private MessageSeverity m2eIncrementalBuildMessageSeverity; @Override - protected void process(Iterable files, Formatter formatter, UpToDateChecker upToDateChecker) throws MojoExecutionException { + protected void process(String name, Iterable files, Formatter formatter, UpToDateChecker upToDateChecker) throws MojoExecutionException { ImpactedFilesTracker counter = new ImpactedFilesTracker(); List problemFiles = new ArrayList<>(); @@ -98,9 +98,9 @@ protected void process(Iterable files, Formatter formatter, UpToDateChecke // We print the number of considered files which is useful when ratchetFrom is setup if (counter.getTotal() > 0) { getLog().info(String.format("Spotless.%s is keeping %s files clean - %s needs changes to be clean, %s were already clean, %s were skipped because caching determined they were already clean", - formatter.getName(), counter.getTotal(), counter.getCleaned(), counter.getCheckedButAlreadyClean(), counter.getSkippedAsCleanCache())); + name, counter.getTotal(), counter.getCleaned(), counter.getCheckedButAlreadyClean(), counter.getSkippedAsCleanCache())); } else { - getLog().debug(String.format("Spotless.%s has no target files. Examine your ``: https://github.com/diffplug/spotless/tree/main/plugin-maven#quickstart", formatter.getName())); + getLog().debug(String.format("Spotless.%s has no target files. Examine your ``: https://github.com/diffplug/spotless/tree/main/plugin-maven#quickstart", name)); } if (!problemFiles.isEmpty()) { diff --git a/testlib/src/main/java/com/diffplug/spotless/StepHarnessWithFile.java b/testlib/src/main/java/com/diffplug/spotless/StepHarnessWithFile.java index 38bc8ce074..ba2322f4cc 100644 --- a/testlib/src/main/java/com/diffplug/spotless/StepHarnessWithFile.java +++ b/testlib/src/main/java/com/diffplug/spotless/StepHarnessWithFile.java @@ -37,7 +37,6 @@ private StepHarnessWithFile(ResourceHarness harness, Formatter formatter, RoundT /** Creates a harness for testing steps which do depend on the file. */ public static StepHarnessWithFile forStep(ResourceHarness harness, FormatterStep step) { return forFormatter(harness, Formatter.builder() - .name(step.getName()) .encoding(StandardCharsets.UTF_8) .lineEndingsPolicy(LineEnding.UNIX.createPolicy()) .steps(Collections.singletonList(step)) From c6cfb6ddc88a6404edba0d44f8e742f31f6b922f Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sun, 1 Jan 2023 22:51:03 -0800 Subject: [PATCH 06/13] Remove the `FormatExceptionPolicy` from the `Formatter` because it will become part of linting. --- .../java/com/diffplug/spotless/Formatter.java | 32 ++++++------------- .../gradle/spotless/SpotlessTask.java | 1 - .../spotless/maven/FormatterFactory.java | 2 -- .../com/diffplug/spotless/StepHarness.java | 1 - .../spotless/StepHarnessWithFile.java | 1 - .../com/diffplug/spotless/FormatterTest.java | 8 ----- 6 files changed, 9 insertions(+), 36 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/Formatter.java b/lib/src/main/java/com/diffplug/spotless/Formatter.java index 40f363a786..1e2e44fe3a 100644 --- a/lib/src/main/java/com/diffplug/spotless/Formatter.java +++ b/lib/src/main/java/com/diffplug/spotless/Formatter.java @@ -36,13 +36,11 @@ public final class Formatter implements Serializable, AutoCloseable { private LineEnding.Policy lineEndingsPolicy; private Charset encoding; private List steps; - private FormatExceptionPolicy exceptionPolicy; - private Formatter(LineEnding.Policy lineEndingsPolicy, Charset encoding, List steps, FormatExceptionPolicy exceptionPolicy) { + private Formatter(LineEnding.Policy lineEndingsPolicy, Charset encoding, List steps) { this.lineEndingsPolicy = Objects.requireNonNull(lineEndingsPolicy, "lineEndingsPolicy"); this.encoding = Objects.requireNonNull(encoding, "encoding"); this.steps = requireElementsNonNull(new ArrayList<>(steps)); - this.exceptionPolicy = Objects.requireNonNull(exceptionPolicy, "exceptionPolicy"); } // override serialize output @@ -50,7 +48,6 @@ private void writeObject(ObjectOutputStream out) throws IOException { out.writeObject(lineEndingsPolicy); out.writeObject(encoding.name()); out.writeObject(steps); - out.writeObject(exceptionPolicy); } // override serialize input @@ -59,7 +56,6 @@ private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundE lineEndingsPolicy = (LineEnding.Policy) in.readObject(); encoding = Charset.forName((String) in.readObject()); steps = (List) in.readObject(); - exceptionPolicy = (FormatExceptionPolicy) in.readObject(); } // override serialize input @@ -80,10 +76,6 @@ public List getSteps() { return steps; } - public FormatExceptionPolicy getExceptionPolicy() { - return exceptionPolicy; - } - public static Formatter.Builder builder() { return new Formatter.Builder(); } @@ -93,7 +85,6 @@ public static class Builder { private LineEnding.Policy lineEndingsPolicy; private Charset encoding; private List steps; - private FormatExceptionPolicy exceptionPolicy; private Builder() {} @@ -112,14 +103,8 @@ 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, steps, - exceptionPolicy == null ? FormatExceptionPolicy.failOnlyOnError() : exceptionPolicy); + return new Formatter(lineEndingsPolicy, encoding, steps); } } @@ -157,9 +142,12 @@ public String compute(String unix, File file) { unix = LineEnding.toUnix(formatted); } } catch (Throwable e) { - // TODO: this is not accurate, but it won't matter when add support for linting - String relativePath = file.toString(); - exceptionPolicy.handleError(e, step, relativePath); + // TODO: this is bad, but it won't matter when add support for linting + if (e instanceof RuntimeException) { + throw (RuntimeException) e; + } else { + throw new RuntimeException(e); + } } } return unix; @@ -172,7 +160,6 @@ public int hashCode() { result = prime * result + encoding.hashCode(); result = prime * result + lineEndingsPolicy.hashCode(); result = prime * result + steps.hashCode(); - result = prime * result + exceptionPolicy.hashCode(); return result; } @@ -190,8 +177,7 @@ public boolean equals(Object obj) { Formatter other = (Formatter) obj; return encoding.equals(other.encoding) && lineEndingsPolicy.equals(other.lineEndingsPolicy) && - steps.equals(other.steps) && - exceptionPolicy.equals(other.exceptionPolicy); + steps.equals(other.steps); } @SuppressWarnings("rawtypes") 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 de8f77904b..19d81eb947 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 @@ -178,7 +178,6 @@ Formatter buildFormatter() { .lineEndingsPolicy(getLineEndingsPolicy().get()) .encoding(Charset.forName(encoding)) .steps(steps) - .exceptionPolicy(exceptionPolicy) .build(); } } diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterFactory.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterFactory.java index 57e20d9b7c..8c2236bf35 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterFactory.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterFactory.java @@ -32,7 +32,6 @@ import org.apache.maven.project.MavenProject; import com.diffplug.common.collect.Sets; -import com.diffplug.spotless.FormatExceptionPolicyStrict; import com.diffplug.spotless.Formatter; import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.LineEnding; @@ -103,7 +102,6 @@ public final Formatter newFormatter(Supplier> filesToFormat, Form return Formatter.builder() .encoding(formatterEncoding) .lineEndingsPolicy(formatterLineEndingPolicy) - .exceptionPolicy(new FormatExceptionPolicyStrict()) .steps(formatterSteps) .build(); } diff --git a/testlib/src/main/java/com/diffplug/spotless/StepHarness.java b/testlib/src/main/java/com/diffplug/spotless/StepHarness.java index f4f95ca502..ae59a8d176 100644 --- a/testlib/src/main/java/com/diffplug/spotless/StepHarness.java +++ b/testlib/src/main/java/com/diffplug/spotless/StepHarness.java @@ -54,7 +54,6 @@ public static StepHarness forStepNoRoundtrip(FormatterStep step) { .steps(Arrays.asList(step)) .lineEndingsPolicy(LineEnding.UNIX.createPolicy()) .encoding(StandardCharsets.UTF_8) - .exceptionPolicy(new FormatExceptionPolicyStrict()) .build(), RoundTrip.DONT_ROUNDTRIP); } diff --git a/testlib/src/main/java/com/diffplug/spotless/StepHarnessWithFile.java b/testlib/src/main/java/com/diffplug/spotless/StepHarnessWithFile.java index ba2322f4cc..bf537cf030 100644 --- a/testlib/src/main/java/com/diffplug/spotless/StepHarnessWithFile.java +++ b/testlib/src/main/java/com/diffplug/spotless/StepHarnessWithFile.java @@ -40,7 +40,6 @@ public static StepHarnessWithFile forStep(ResourceHarness harness, FormatterStep .encoding(StandardCharsets.UTF_8) .lineEndingsPolicy(LineEnding.UNIX.createPolicy()) .steps(Collections.singletonList(step)) - .exceptionPolicy(new FormatExceptionPolicyStrict()) .build()); } diff --git a/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java b/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java index 8114d75cab..139244e7da 100644 --- a/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java @@ -21,7 +21,6 @@ import java.nio.file.FileSystem; import java.nio.file.FileSystems; import java.nio.file.Path; -import java.nio.file.Paths; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -30,7 +29,6 @@ import org.junit.jupiter.api.Test; import org.mockito.Mockito; -import com.diffplug.common.base.StandardSystemProperty; import com.diffplug.spotless.generic.EndWithNewlineStep; class FormatterTest { @@ -79,9 +77,6 @@ protected Formatter create() { public void testExceptionWithSentinelNoFileOnDisk() throws Exception { LineEnding.Policy lineEndingsPolicy = LineEnding.UNIX.createPolicy(); Charset encoding = StandardCharsets.UTF_8; - FormatExceptionPolicy exceptionPolicy = FormatExceptionPolicy.failOnlyOnError(); - - Path rootDir = Paths.get(StandardSystemProperty.USER_DIR.value()); FormatterStep step = Mockito.mock(FormatterStep.class); Mockito.when(step.getName()).thenReturn("someFailingStep"); @@ -92,7 +87,6 @@ public void testExceptionWithSentinelNoFileOnDisk() throws Exception { .lineEndingsPolicy(lineEndingsPolicy) .encoding(encoding) .steps(steps) - .exceptionPolicy(exceptionPolicy) .build(); formatter.compute("someFileContent", Formatter.NO_FILE_SENTINEL); @@ -103,7 +97,6 @@ public void testExceptionWithSentinelNoFileOnDisk() throws Exception { public void testExceptionWithRootDirIsNotFileSystem() throws Exception { LineEnding.Policy lineEndingsPolicy = LineEnding.UNIX.createPolicy(); Charset encoding = StandardCharsets.UTF_8; - FormatExceptionPolicy exceptionPolicy = FormatExceptionPolicy.failOnlyOnError(); Path rootDir = Mockito.mock(Path.class); FileSystem customFileSystem = Mockito.mock(FileSystem.class); @@ -131,7 +124,6 @@ public void testExceptionWithRootDirIsNotFileSystem() throws Exception { .lineEndingsPolicy(lineEndingsPolicy) .encoding(encoding) .steps(steps) - .exceptionPolicy(exceptionPolicy) .build(); formatter.compute("someFileContent", new File("/some/folder/some.file")); From baf0f11a95f1bcea40d2b07c9843574cbfeef7cf Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 29 May 2024 15:37:44 -0700 Subject: [PATCH 07/13] And now the tests that do and don't work have shuffled once again. --- ...GrEclipseFormatterStepSpecialCaseTest.java | 4 -- .../gradle/spotless/BiomeIntegrationTest.java | 5 ++ .../spotless/ErrorShouldRethrowTest.java | 6 +- .../gradle/spotless/KotlinExtensionTest.java | 7 +- .../spotless/maven/biome/BiomeMavenTest.java | 4 ++ .../com/diffplug/spotless/FormatterTest.java | 64 ------------------- .../gherkin/GherkinUtilsStepTest.java | 5 -- .../spotless/json/JsonSimpleStepTest.java | 6 -- .../spotless/json/gson/GsonStepTest.java | 4 -- 9 files changed, 20 insertions(+), 85 deletions(-) diff --git a/lib-extra/src/test/java/com/diffplug/spotless/extra/groovy/GrEclipseFormatterStepSpecialCaseTest.java b/lib-extra/src/test/java/com/diffplug/spotless/extra/groovy/GrEclipseFormatterStepSpecialCaseTest.java index 8064633a4e..e5159a4498 100644 --- a/lib-extra/src/test/java/com/diffplug/spotless/extra/groovy/GrEclipseFormatterStepSpecialCaseTest.java +++ b/lib-extra/src/test/java/com/diffplug/spotless/extra/groovy/GrEclipseFormatterStepSpecialCaseTest.java @@ -16,12 +16,10 @@ package com.diffplug.spotless.extra.groovy; import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import com.diffplug.spotless.StepHarness; import com.diffplug.spotless.TestProvisioner; -import com.diffplug.spotless.tag.ForLintRefactor; public class GrEclipseFormatterStepSpecialCaseTest { /** @@ -31,8 +29,6 @@ public class GrEclipseFormatterStepSpecialCaseTest { * works: ${parm == null ? "" : "parm"} */ @Test - @Disabled - @ForLintRefactor public void issue_1657() { Assertions.assertThrows(RuntimeException.class, () -> { StepHarness.forStep(GrEclipseFormatterStep.createBuilder(TestProvisioner.mavenCentral()).build()) diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/BiomeIntegrationTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/BiomeIntegrationTest.java index ba39af1705..aff28664c5 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/BiomeIntegrationTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/BiomeIntegrationTest.java @@ -21,9 +21,12 @@ import java.io.IOException; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.owasp.encoder.Encode; +import com.diffplug.spotless.tag.ForLintRefactor; + class BiomeIntegrationTest extends GradleIntegrationHarness { /** * Tests that biome can be used as a generic formatting step. @@ -320,6 +323,8 @@ void failureWhenExeNotFound() throws Exception { * @throws Exception When a test failure occurs. */ @Test + @Disabled + @ForLintRefactor void failureWhenNotParseable() throws Exception { setFile("build.gradle").toLines( "plugins {", diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ErrorShouldRethrowTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ErrorShouldRethrowTest.java index d1ced01609..d8e9cbd2fa 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ErrorShouldRethrowTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ErrorShouldRethrowTest.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. @@ -23,13 +23,17 @@ import org.assertj.core.api.Assertions; import org.gradle.testkit.runner.BuildResult; import org.gradle.testkit.runner.TaskOutcome; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import com.diffplug.common.base.CharMatcher; import com.diffplug.common.base.Splitter; import com.diffplug.spotless.LineEnding; +import com.diffplug.spotless.tag.ForLintRefactor; /** Tests the desired behavior from https://github.com/diffplug/spotless/issues/46. */ +@Disabled +@ForLintRefactor class ErrorShouldRethrowTest extends GradleIntegrationHarness { private void writeBuild(String... toInsert) throws IOException { List lines = new ArrayList<>(); diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/KotlinExtensionTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/KotlinExtensionTest.java index 1e96d34574..aa82c371f3 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/KotlinExtensionTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/KotlinExtensionTest.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. @@ -20,8 +20,11 @@ import java.io.File; import java.io.IOException; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; +import com.diffplug.spotless.tag.ForLintRefactor; + class KotlinExtensionTest extends GradleIntegrationHarness { private static final String HEADER = "// License Header"; private static final String HEADER_WITH_YEAR = "// License Header $YEAR"; @@ -147,6 +150,8 @@ void testSetEditorConfigCanOverrideEditorConfigFile() throws IOException { } @Test + @Disabled + @ForLintRefactor void withCustomRuleSetApply() throws IOException { setFile("build.gradle.kts").toLines( "plugins {", diff --git a/plugin-maven/src/test/java/com/diffplug/spotless/maven/biome/BiomeMavenTest.java b/plugin-maven/src/test/java/com/diffplug/spotless/maven/biome/BiomeMavenTest.java index ff763c3cf8..24ce1d09b7 100644 --- a/plugin-maven/src/test/java/com/diffplug/spotless/maven/biome/BiomeMavenTest.java +++ b/plugin-maven/src/test/java/com/diffplug/spotless/maven/biome/BiomeMavenTest.java @@ -20,9 +20,11 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.owasp.encoder.Encode.forXml; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import com.diffplug.spotless.maven.MavenIntegrationHarness; +import com.diffplug.spotless.tag.ForLintRefactor; class BiomeMavenTest extends MavenIntegrationHarness { /** @@ -190,6 +192,8 @@ void failureWhenExeNotFound() throws Exception { * @throws Exception When a test failure occurs. */ @Test + @Disabled + @ForLintRefactor void failureWhenNotParseable() throws Exception { writePomWithBiomeSteps("**/*.js", "1.2.0json"); setFile("biome_test.js").toResource("biome/js/fileBefore.js"); diff --git a/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java b/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java index 139244e7da..cbd219b33d 100644 --- a/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java @@ -15,19 +15,13 @@ */ package com.diffplug.spotless; -import java.io.File; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; -import java.nio.file.FileSystem; -import java.nio.file.FileSystems; -import java.nio.file.Path; import java.util.ArrayList; -import java.util.Collections; import java.util.List; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; -import org.mockito.Mockito; import com.diffplug.spotless.generic.EndWithNewlineStep; @@ -71,62 +65,4 @@ protected Formatter create() { } }.testEquals(); } - - // If there is no File actually holding the content, one may rely on Formatter.NO_FILE_ON_DISK - @Test - public void testExceptionWithSentinelNoFileOnDisk() throws Exception { - LineEnding.Policy lineEndingsPolicy = LineEnding.UNIX.createPolicy(); - Charset encoding = StandardCharsets.UTF_8; - - FormatterStep step = Mockito.mock(FormatterStep.class); - Mockito.when(step.getName()).thenReturn("someFailingStep"); - Mockito.when(step.format(Mockito.anyString(), Mockito.any(File.class))).thenThrow(new IllegalArgumentException("someReason")); - List steps = Collections.singletonList(step); - - Formatter formatter = Formatter.builder() - .lineEndingsPolicy(lineEndingsPolicy) - .encoding(encoding) - .steps(steps) - .build(); - - formatter.compute("someFileContent", Formatter.NO_FILE_SENTINEL); - } - - // rootDir may be a path not from the default FileSystem - @Test - public void testExceptionWithRootDirIsNotFileSystem() throws Exception { - LineEnding.Policy lineEndingsPolicy = LineEnding.UNIX.createPolicy(); - Charset encoding = StandardCharsets.UTF_8; - - Path rootDir = Mockito.mock(Path.class); - FileSystem customFileSystem = Mockito.mock(FileSystem.class); - Mockito.when(rootDir.getFileSystem()).thenReturn(customFileSystem); - - Path pathFromFile = Mockito.mock(Path.class); - Mockito.when(customFileSystem.getPath(Mockito.anyString())).thenReturn(pathFromFile); - - Path relativized = Mockito.mock(Path.class); - Mockito.when(rootDir.relativize(Mockito.any(Path.class))).then(invok -> { - Path filePath = invok.getArgument(0); - if (filePath.getFileSystem() == FileSystems.getDefault()) { - throw new IllegalArgumentException("Can not relativize through different FileSystems"); - } - - return relativized; - }); - - FormatterStep step = Mockito.mock(FormatterStep.class); - Mockito.when(step.getName()).thenReturn("someFailingStep"); - Mockito.when(step.format(Mockito.anyString(), Mockito.any(File.class))).thenThrow(new IllegalArgumentException("someReason")); - List steps = Collections.singletonList(step); - - Formatter formatter = Formatter.builder() - .lineEndingsPolicy(lineEndingsPolicy) - .encoding(encoding) - .steps(steps) - .build(); - - formatter.compute("someFileContent", new File("/some/folder/some.file")); - } - } diff --git a/testlib/src/test/java/com/diffplug/spotless/gherkin/GherkinUtilsStepTest.java b/testlib/src/test/java/com/diffplug/spotless/gherkin/GherkinUtilsStepTest.java index 5ffaa029ae..02099a5fe8 100644 --- a/testlib/src/test/java/com/diffplug/spotless/gherkin/GherkinUtilsStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/gherkin/GherkinUtilsStepTest.java @@ -24,7 +24,6 @@ import com.diffplug.spotless.SerializableEqualityTester; import com.diffplug.spotless.StepHarness; import com.diffplug.spotless.TestProvisioner; -import com.diffplug.spotless.tag.ForLintRefactor; public class GherkinUtilsStepTest { @@ -60,8 +59,6 @@ public void handlesRuleWithTag() throws Exception { } @Test - @Disabled - @ForLintRefactor public void handlesInvalidGherkin() { assertThatThrownBy(() -> doWithResource(stepHarness, "invalidGherkin")) .isInstanceOf(IllegalArgumentException.class) @@ -69,8 +66,6 @@ public void handlesInvalidGherkin() { } @Test - @Disabled - @ForLintRefactor public void handlesNotGherkin() { assertThatThrownBy(() -> doWithResource(stepHarness, "notGherkin")) .isInstanceOf(IllegalArgumentException.class) diff --git a/testlib/src/test/java/com/diffplug/spotless/json/JsonSimpleStepTest.java b/testlib/src/test/java/com/diffplug/spotless/json/JsonSimpleStepTest.java index 5d7c08f129..e81c9d7284 100644 --- a/testlib/src/test/java/com/diffplug/spotless/json/JsonSimpleStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/json/JsonSimpleStepTest.java @@ -17,14 +17,12 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.SerializableEqualityTester; import com.diffplug.spotless.StepHarness; import com.diffplug.spotless.TestProvisioner; -import com.diffplug.spotless.tag.ForLintRefactor; class JsonSimpleStepTest { @@ -74,16 +72,12 @@ void handlesObjectWithNull() { } @Test - @Disabled - @ForLintRefactor void handlesInvalidJson() { stepHarness.testResourceExceptionMsg("json/invalidJsonBefore.json") .contains("Expected a ',' or '}' at 9 [character 0 line 3]"); } @Test - @Disabled - @ForLintRefactor void handlesNotJson() { stepHarness.testResourceExceptionMsg("json/notJsonBefore.json") .contains("Unable to determine JSON type, expected a '{' or '[' but found '#'"); diff --git a/testlib/src/test/java/com/diffplug/spotless/json/gson/GsonStepTest.java b/testlib/src/test/java/com/diffplug/spotless/json/gson/GsonStepTest.java index 29c65f8493..9513c88a5c 100644 --- a/testlib/src/test/java/com/diffplug/spotless/json/gson/GsonStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/json/gson/GsonStepTest.java @@ -18,7 +18,6 @@ import java.io.File; import org.assertj.core.api.Assertions; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import com.diffplug.spotless.FormatterStep; @@ -26,7 +25,6 @@ import com.diffplug.spotless.StepHarness; import com.diffplug.spotless.TestProvisioner; import com.diffplug.spotless.json.JsonFormatterStepCommonTests; -import com.diffplug.spotless.tag.ForLintRefactor; public class GsonStepTest extends JsonFormatterStepCommonTests { @@ -43,8 +41,6 @@ void handlesObjectWithNull() { } @Test - @Disabled - @ForLintRefactor void handlesInvalidJson() { getStepHarness().testResourceExceptionMsg("json/invalidJsonBefore.json").isEqualTo("End of input at line 3 column 1 path $.a"); } From 9347888f4d8eb7d6f6aacdd6d3abfff18f837023 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 29 May 2024 15:56:10 -0700 Subject: [PATCH 08/13] Reduce the size of the diff. --- .../com/diffplug/spotless/gherkin/GherkinUtilsStepTest.java | 2 +- .../java/com/diffplug/spotless/json/JsonSimpleStepTest.java | 2 +- .../test/java/com/diffplug/spotless/json/gson/GsonStepTest.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/testlib/src/test/java/com/diffplug/spotless/gherkin/GherkinUtilsStepTest.java b/testlib/src/test/java/com/diffplug/spotless/gherkin/GherkinUtilsStepTest.java index 02099a5fe8..6bc478027b 100644 --- a/testlib/src/test/java/com/diffplug/spotless/gherkin/GherkinUtilsStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/gherkin/GherkinUtilsStepTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2021-2024 DiffPlug + * Copyright 2021-2023 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/testlib/src/test/java/com/diffplug/spotless/json/JsonSimpleStepTest.java b/testlib/src/test/java/com/diffplug/spotless/json/JsonSimpleStepTest.java index e81c9d7284..19edc1a8e8 100644 --- a/testlib/src/test/java/com/diffplug/spotless/json/JsonSimpleStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/json/JsonSimpleStepTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2021-2024 DiffPlug + * Copyright 2021-2023 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/testlib/src/test/java/com/diffplug/spotless/json/gson/GsonStepTest.java b/testlib/src/test/java/com/diffplug/spotless/json/gson/GsonStepTest.java index 9513c88a5c..3f4d2a84e0 100644 --- a/testlib/src/test/java/com/diffplug/spotless/json/gson/GsonStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/json/gson/GsonStepTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2022-2024 DiffPlug + * Copyright 2022-2023 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. From 5f159a038a8ffedff88240840ca8b0abbc353a51 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 4 Jun 2024 01:20:22 -0700 Subject: [PATCH 09/13] No reason for DirtyState.Calculation to be public API. --- lib/src/main/java/com/diffplug/spotless/DirtyState.java | 8 ++++---- .../main/java/com/diffplug/gradle/spotless/IdeHook.java | 2 +- .../com/diffplug/gradle/spotless/SpotlessTaskImpl.java | 2 +- .../main/java/com/diffplug/spotless/maven/IdeHook.java | 2 +- .../com/diffplug/spotless/maven/SpotlessApplyMojo.java | 2 +- .../com/diffplug/spotless/maven/SpotlessCheckMojo.java | 2 +- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/DirtyState.java b/lib/src/main/java/com/diffplug/spotless/DirtyState.java index 3d133927c6..e8667dd12d 100644 --- a/lib/src/main/java/com/diffplug/spotless/DirtyState.java +++ b/lib/src/main/java/com/diffplug/spotless/DirtyState.java @@ -69,15 +69,15 @@ public static DirtyState clean() { static final DirtyState didNotConverge = new DirtyState(null); static final DirtyState isClean = new DirtyState(null); - public static Calculation of(Formatter formatter, File file) throws IOException { + public static DirtyState of(Formatter formatter, File file) throws IOException { return of(formatter, file, Files.readAllBytes(file.toPath())); } - public static Calculation of(Formatter formatter, File file, byte[] rawBytes) { - return new Calculation(formatter, file, rawBytes); + public static DirtyState of(Formatter formatter, File file, byte[] rawBytes) { + return new Calculation(formatter, file, rawBytes).calculateDirtyState(); } - public static class Calculation { + private static class Calculation { private final Formatter formatter; private final File file; private final byte[] rawBytes; diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/IdeHook.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/IdeHook.java index 4c198dd42c..cc767ce111 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/IdeHook.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/IdeHook.java @@ -55,7 +55,7 @@ static void performHook(SpotlessTaskImpl spotlessTask) { } else { bytes = Files.readAllBytes(file.toPath()); } - DirtyState dirty = DirtyState.of(formatter, file, bytes).calculateDirtyState(); + DirtyState dirty = DirtyState.of(formatter, file, bytes); if (dirty.isClean()) { dumpIsClean(); } else if (dirty.didNotConverge()) { diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java index cc694942c1..b7586f742b 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java @@ -102,7 +102,7 @@ void processInputFile(@Nullable GitRatchet ratchet, Formatter formatter, File in dirtyState = DirtyState.clean(); } else { try { - dirtyState = DirtyState.of(formatter, input).calculateDirtyState(); + dirtyState = DirtyState.of(formatter, input); } catch (IOException e) { throw new IOException("Issue processing file: " + input, e); } catch (RuntimeException e) { diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/IdeHook.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/IdeHook.java index 5289af4756..6b7750c060 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/IdeHook.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/IdeHook.java @@ -49,7 +49,7 @@ static void performHook(Iterable projectFiles, Formatter formatter, String } else { bytes = Files.readAllBytes(file.toPath()); } - DirtyState dirty = DirtyState.of(formatter, file, bytes).calculateDirtyState(); + DirtyState dirty = DirtyState.of(formatter, file, bytes); if (dirty.isClean()) { dumpIsClean(); } else if (dirty.didNotConverge()) { diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessApplyMojo.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessApplyMojo.java index 0d67ed8c9f..820d89352c 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessApplyMojo.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessApplyMojo.java @@ -60,7 +60,7 @@ protected void process(String name, Iterable files, Formatter formatter, U } try { - DirtyState dirtyState = DirtyState.of(formatter, file).calculateDirtyState(); + DirtyState dirtyState = DirtyState.of(formatter, file); if (!dirtyState.isClean() && !dirtyState.didNotConverge()) { getLog().info(String.format("Writing clean file: %s", file)); dirtyState.writeCanonicalTo(file); diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java index 183f20ef9e..116a24dcf4 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java @@ -78,7 +78,7 @@ protected void process(String name, Iterable files, Formatter formatter, U } buildContext.removeMessages(file); try { - DirtyState dirtyState = DirtyState.of(formatter, file).calculateDirtyState(); + DirtyState dirtyState = DirtyState.of(formatter, file); if (!dirtyState.isClean() && !dirtyState.didNotConverge()) { problemFiles.add(file); if (buildContext.isIncremental()) { From baea8c943a16dadcaa9252d267b53a8473e97b76 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 4 Jun 2024 01:21:40 -0700 Subject: [PATCH 10/13] In fact there wasn't any reason for the DirtyState.Calculation class to exist at all. --- .../com/diffplug/spotless/DirtyState.java | 97 ++++++++----------- 1 file changed, 38 insertions(+), 59 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/DirtyState.java b/lib/src/main/java/com/diffplug/spotless/DirtyState.java index e8667dd12d..39d5da47d6 100644 --- a/lib/src/main/java/com/diffplug/spotless/DirtyState.java +++ b/lib/src/main/java/com/diffplug/spotless/DirtyState.java @@ -74,68 +74,47 @@ public static DirtyState of(Formatter formatter, File file) throws IOException { } public static DirtyState of(Formatter formatter, File file, byte[] rawBytes) { - return new Calculation(formatter, file, rawBytes).calculateDirtyState(); - } + String raw = new String(rawBytes, formatter.getEncoding()); + // check that all characters were encodable + String encodingError = EncodingErrorMsg.msg(raw, rawBytes, formatter.getEncoding()); + if (encodingError != null) { + throw new IllegalArgumentException(encodingError); + } + + String rawUnix = LineEnding.toUnix(raw); + + // enforce the format + String formattedUnix = formatter.compute(rawUnix, file); + // convert the line endings if necessary + String formatted = formatter.computeLineEndings(formattedUnix, file); + + // if F(input) == input, then the formatter is well-behaving and the input is clean + byte[] formattedBytes = formatted.getBytes(formatter.getEncoding()); + if (Arrays.equals(rawBytes, formattedBytes)) { + return isClean; + } + + // F(input) != input, so we'll do a padded check + String doubleFormattedUnix = formatter.compute(formattedUnix, file); + if (doubleFormattedUnix.equals(formattedUnix)) { + // most dirty files are idempotent-dirty, so this is a quick-short circuit for that common case + return new DirtyState(formattedBytes); + } - private static class Calculation { - private final Formatter formatter; - private final File file; - private final byte[] rawBytes; - private final String raw; - - private Calculation(Formatter formatter, File file, byte[] rawBytes) { - this.formatter = formatter; - this.file = file; - this.rawBytes = rawBytes; - this.raw = new String(rawBytes, formatter.getEncoding()); - // check that all characters were encodable - String encodingError = EncodingErrorMsg.msg(raw, rawBytes, formatter.getEncoding()); - if (encodingError != null) { - throw new IllegalArgumentException(encodingError); - } + PaddedCell cell = PaddedCell.check(formatter, file, rawUnix); + if (!cell.isResolvable()) { + return didNotConverge; } - /** - * Calculates whether the given file is dirty according to a PaddedCell invocation of the given formatter. - * DirtyState includes the clean state of the file, as well as a warning if we were not able to apply the formatter - * due to diverging idempotence. - */ - public DirtyState calculateDirtyState() { - String rawUnix = LineEnding.toUnix(raw); - - // enforce the format - String formattedUnix = formatter.compute(rawUnix, file); - // convert the line endings if necessary - String formatted = formatter.computeLineEndings(formattedUnix, file); - - // if F(input) == input, then the formatter is well-behaving and the input is clean - byte[] formattedBytes = formatted.getBytes(formatter.getEncoding()); - if (Arrays.equals(rawBytes, formattedBytes)) { - return isClean; - } - - // F(input) != input, so we'll do a padded check - String doubleFormattedUnix = formatter.compute(formattedUnix, file); - if (doubleFormattedUnix.equals(formattedUnix)) { - // most dirty files are idempotent-dirty, so this is a quick-short circuit for that common case - return new DirtyState(formattedBytes); - } - - PaddedCell cell = PaddedCell.check(formatter, file, rawUnix); - if (!cell.isResolvable()) { - return didNotConverge; - } - - // get the canonical bytes - String canonicalUnix = cell.canonical(); - String canonical = formatter.computeLineEndings(canonicalUnix, file); - byte[] canonicalBytes = canonical.getBytes(formatter.getEncoding()); - if (!Arrays.equals(rawBytes, canonicalBytes)) { - // and write them to disk if needed - return new DirtyState(canonicalBytes); - } else { - return isClean; - } + // get the canonical bytes + String canonicalUnix = cell.canonical(); + String canonical = formatter.computeLineEndings(canonicalUnix, file); + byte[] canonicalBytes = canonical.getBytes(formatter.getEncoding()); + if (!Arrays.equals(rawBytes, canonicalBytes)) { + // and write them to disk if needed + return new DirtyState(canonicalBytes); + } else { + return isClean; } } } From 4f5bf5195e24541b354392bb76454fdefe50b094 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 4 Jun 2024 01:32:45 -0700 Subject: [PATCH 11/13] Update contributor docs a bit. --- CONTRIBUTING.md | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 27e6221e76..c4945b70f3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -10,21 +10,20 @@ In order to use and combine `FormatterStep`, you first create a `Formatter`, whi - an encoding - a list of `FormatterStep` -- a line endings policy (`LineEnding.GIT_ATTRIBUTES` is almost always the best choice) +- a line endings policy (`LineEnding.GIT_ATTRIBUTES_FAST_ALLSAME` is almost always the best choice) -Once you have an instance of `Formatter`, you can call `boolean isClean(File)`, or `void applyTo(File)` to either check or apply formatting to a file. Spotless will then: +Once you have an instance of `Formatter`, you can call `DirtyState.of(Formatter, File)`. Under the hood, Spotless will: - parse the raw bytes into a String according to the encoding - normalize its line endings to `\n` - pass the unix string to each `FormatterStep` one after the other +- check for idempotence problems, and repeatedly apply the steps until the [result is stable](PADDEDCELL.md). - apply line endings according to the policy You can also use lower-level methods like `String compute(String unix, File file)` if you'd like to do lower-level processing. All `FormatterStep` implement `Serializable`, `equals`, and `hashCode`, so build systems that support up-to-date checks can easily and correctly determine if any actions need to be taken. -Spotless also provides `PaddedCell`, which makes it easy to diagnose and correct idempotence problems. - ## Project layout For the folders below in monospace text, they are published on MavenCentral at the coordinate `com.diffplug.spotless:spotless-${FOLDER_NAME}`. The other folders are dev infrastructure. @@ -39,15 +38,16 @@ For the folders below in monospace text, they are published on MavenCentral at t ## How to add a new FormatterStep -The easiest way to create a FormatterStep is `FormatterStep createNeverUpToDate(String name, FormatterFunc function)`, which you can use like this: +The easiest way to create a FormatterStep is to just create `class FooStep implements FormatterStep`. It has one abstract method which is the formatting function, and you're ready to tinker. To work with the build plugins, this class will need to -```java -FormatterStep identityStep = FormatterStep.createNeverUpToDate("identity", unixStr -> unixStr) -``` +- implement equality and hashcode +- support lossless roundtrip serialization + +You can use `StepHarness` (if you don't care about the `File` argument) or `StepHarnessWithFile` to test. The harness will roundtrip serialize your step, check that it's equal to itself, and then perform all tests on the roundtripped step. -This creates a step which will fail up-to-date checks (it is equal only to itself), and will use the function you passed in to do the formatting pass. +## Implementing equality in terms of serialization -To create a step which can handle up-to-date checks properly, use the method ` FormatterStep create(String name, State state, Function stateToFormatter)`. Here's an example: +Spotless has infrastructure which uses the serialized form of your step to implement equality for you. Here is an example: ```java public final class ReplaceStep { @@ -62,10 +62,10 @@ public final class ReplaceStep { private static final class State implements Serializable { private static final long serialVersionUID = 1L; - private final CharSequence target; - private final CharSequence replacement; + private final String target; + private final String replacement; - State(CharSequence target, CharSequence replacement) { + State(String target, String replacement) { this.target = target; this.replacement = replacement; } @@ -82,8 +82,6 @@ The `FormatterStep` created above implements `equals` and `hashCode` based on th Oftentimes, a rule's state will be expensive to compute. `EclipseFormatterStep`, for example, depends on a formatting file. Ideally, we would like to only pay the cost of the I/O needed to load that file if we have to - we'd like to create the FormatterStep now but load its state lazily at the last possible moment. For this purpose, each of the `FormatterStep.create` methods has a lazy counterpart. Here are their signatures: ```java -FormatterStep createNeverUpToDate (String name, FormatterFunc function ) -FormatterStep createNeverUpToDateLazy(String name, Supplier functionSupplier) FormatterStep create (String name, State state , Function stateToFormatter) FormatterStep createLazy(String name, Supplier stateSupplier, Function stateToFormatter) ``` @@ -101,7 +99,7 @@ Here's a checklist for creating a new step for Spotless: ### 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: +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` can be used to take absolute paths out of the equality check. To make this work, roundtrip compatible steps can 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 From 33444a7b771a1de9ea7c4cfc961c9a72f742e7e1 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 4 Jun 2024 01:34:11 -0700 Subject: [PATCH 12/13] Update changelog. --- CHANGES.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 1de166cd2b..8b3b11609f 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -34,8 +34,8 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( * **BREAKING** Remove `JarState.getMavenCoordinate(String prefix)`. ([#1945](https://github.com/diffplug/spotless/pull/1945)) * **BREAKING** Replace `PipeStepPair` with `FenceStep`. ([#1954](https://github.com/diffplug/spotless/pull/1954)) * **BREAKING** Fully removed `Rome`, use `Biome` instead. ([#2119](https://github.com/diffplug/spotless/pull/2119)) -* **BREAKING** Moved `PaddedCell.DirtyState` to its own top-level class with new methods. -* **BREAKING** Removed `isClean`, `applyTo`, and `applyToAndReturnResultIfDirty` from `Formatter` because users should instead use `PaddedCell.check()`. +* **BREAKING** Moved `PaddedCell.DirtyState` to its own top-level class with new methods. ([#2148](https://github.com/diffplug/spotless/pull/2148)) + * **BREAKING** Removed `isClean`, `applyTo`, and `applyToAndReturnResultIfDirty` from `Formatter` because users should instead use `DirtyState`. ## [2.45.0] - 2024-01-23 ### Added From 5822660d42e6da2f440d85156fcc3f8cc7565475 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 15 Oct 2024 11:33:16 -0700 Subject: [PATCH 13/13] spotlessApply --- lib/src/main/java/com/diffplug/spotless/Formatter.java | 3 +-- .../com/diffplug/gradle/spotless/BiomeIntegrationTest.java | 2 -- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/Formatter.java b/lib/src/main/java/com/diffplug/spotless/Formatter.java index ea9a38410b..4bc017a834 100644 --- a/lib/src/main/java/com/diffplug/spotless/Formatter.java +++ b/lib/src/main/java/com/diffplug/spotless/Formatter.java @@ -87,8 +87,7 @@ public static class Builder { private Charset encoding; private List steps; - private Builder() { - } + private Builder() {} public Builder lineEndingsPolicy(LineEnding.Policy lineEndingsPolicy) { this.lineEndingsPolicy = lineEndingsPolicy; diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/BiomeIntegrationTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/BiomeIntegrationTest.java index e3ae71392c..abf8d34fb0 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/BiomeIntegrationTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/BiomeIntegrationTest.java @@ -19,8 +19,6 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; -import java.io.IOException; - import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.owasp.encoder.Encode;