Linting - breaking changes to internal API to prepare (#2148)

Author: nedtwigg

CHANGES.md

@@ -73,6 +73,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. ([#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

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 `<State extends Serializable> FormatterStep create(String name, State state, Function<State, FormatterFunc> 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<FormatterFunc> functionSupplier)
 FormatterStep create    (String name, State state                  , Function<State, FormatterFunc> stateToFormatter)
 FormatterStep createLazy(String name, Supplier<State> stateSupplier, Function<State, FormatterFunc> 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 

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<Integer, String> diff(Formatter formatter, File file) throws IOException {
-		return diff(new CleanProviderFormatter(formatter), file);
+	public static Map.Entry<Integer, String> diff(Path rootDir, Formatter formatter, File file) throws IOException {
+		return diff(new CleanProviderFormatter(rootDir, formatter), file);
 	}
 
 	private static Map.Entry<Integer, String> diff(CleanProvider formatter, File file) throws IOException {

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.

lib/src/main/java/com/diffplug/spotless/DirtyState.java

@@ -0,0 +1,120 @@
+/*
+ * 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 DirtyState of(Formatter formatter, File file) throws IOException {
+		return of(formatter, file, Files.readAllBytes(file.toPath()));
+	}
+
+	public static DirtyState of(Formatter formatter, File file, byte[] rawBytes) {
+		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;
+		}
+	}
+}