diff --git a/CHANGES.md b/CHANGES.md index a3aaa298a2..6f19b9751d 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -12,6 +12,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( ## [Unreleased] ### Added * Support for google-java-format 1.8 (including test infrastructure for Java 11). ([#562](https://github.com/diffplug/spotless/issues/562)) +* Improved PaddedCell such that it is as performant as non-padded cell - no reason not to have it always enabled. Deprecated all of `PaddedCellBulk`. ([#561](https://github.com/diffplug/spotless/pull/561)) ## [1.28.1] - 2020-04-02 ### Fixed diff --git a/PADDEDCELL.md b/PADDEDCELL.md index 54bb70b321..41bc13dc7e 100644 --- a/PADDEDCELL.md +++ b/PADDEDCELL.md @@ -1,19 +1,4 @@ -# You have a misbehaving rule that needs a `paddedCell()` - -`spotlessCheck` has detected that one of your rules is misbehaving. This will cause `spotlessCheck` to fail even after you have called `spotlessApply`. To bandaid over this problem, add `paddedCell()` to your `build.gradle`, as such: - -```gradle -spotless { - java { - ... - paddedCell() - } -} -``` - -This is not a bug in Spotless itself, but in one of the third-party formatters, such as the [eclipse formatter](https://bugs.eclipse.org/bugs/show_bug.cgi?id=310642), [google-java-format](https://github.com/google/google-java-format/issues), or some custom rule. - -`paddedCell()` will ensure that your code continues to be formatted, although it will be a little slower. Now when you run `spotlessCheck`, it will generate helpful bug report files in the `build/spotless-diagnose-` folder which will contain the states that your rules are fighting over. These files are very helpful to the developers of the code formatter you are using. +# PaddedCell ## How spotless works @@ -46,26 +31,7 @@ The rule we wrote above is obviously a bad idea. But complex code formatters ca Formally, a correct formatter `F` must satisfy `F(F(input)) == F(input)` for all values of input. Any formatter which doesn't meet this rule is misbehaving. -## How does `paddedCell()` work? - -Spotless now has a special `paddedCell()` mode. If you add it to your format as such: - -```gradle -spotless { - format 'cpp', { - ... - paddedCell() - } -} -``` - -then it will run in the following way: - -- When you call `spotlessApply`, it will automatically check for a ping-pong condition. -- If there is a ping-pong condition, it will resolve the ambiguity arbitrarily, but consistently -- It will also warn that `filename such-and-such cycles between 2 steps`. - -## How is the ambiguity resolved? +## How spotless fixes this automatically This is easiest to show in an example: 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 398ccca1ad..d2d86ff7db 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 @@ -48,7 +48,6 @@ public static class Builder { private Builder() {} private String runToFix; - private boolean isPaddedCell; private Formatter formatter; private List problemFiles; @@ -58,8 +57,9 @@ public Builder runToFix(String runToFix) { return this; } + @Deprecated public Builder isPaddedCell(boolean isPaddedCell) { - this.isPaddedCell = isPaddedCell; + System.err.println("PaddedCell is now always on, and cannot be turned off."); return this; } @@ -171,11 +171,7 @@ private static String diff(Builder builder, File file) throws IOException { String raw = new String(Files.readAllBytes(file.toPath()), builder.formatter.getEncoding()); String rawUnix = LineEnding.toUnix(raw); String formattedUnix; - if (builder.isPaddedCell) { - formattedUnix = PaddedCell.check(builder.formatter, file, rawUnix).canonical(); - } else { - formattedUnix = builder.formatter.compute(rawUnix, file); - } + formattedUnix = PaddedCell.check(builder.formatter, file, rawUnix).canonical(); if (rawUnix.equals(formattedUnix)) { // the formatting is fine, so it's a line-ending issue diff --git a/lib/src/main/java/com/diffplug/spotless/PaddedCell.java b/lib/src/main/java/com/diffplug/spotless/PaddedCell.java index d8fd282051..0888e1646a 100644 --- a/lib/src/main/java/com/diffplug/spotless/PaddedCell.java +++ b/lib/src/main/java/com/diffplug/spotless/PaddedCell.java @@ -18,8 +18,12 @@ 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.nio.file.StandardOpenOption; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.Comparator; import java.util.List; @@ -171,4 +175,93 @@ 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()); + String raw = new String(rawBytes, formatter.getEncoding()); + 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 is clean, and there's nothing else to say + * - {@link #isConverged()} means that we were able 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()} to get the canonical form of the given file. + */ + public static class DirtyState { + private final byte[] canonicalBytes; + + private DirtyState(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 `!isClean()` and `!didNotConverge()`"); + } + return canonicalBytes; + } + + public void writeCanonicalTo(File file) throws IOException { + Files.write(file.toPath(), canonicalBytes(), StandardOpenOption.TRUNCATE_EXISTING); + } + + public void writeCanonicalTo(OutputStream out) throws IOException { + out.write(canonicalBytes()); + } + } + + private static final DirtyState didNotConverge = new DirtyState(null); + private static final DirtyState isClean = new DirtyState(null); } diff --git a/lib/src/main/java/com/diffplug/spotless/PaddedCellBulk.java b/lib/src/main/java/com/diffplug/spotless/PaddedCellBulk.java index 1d37ea72dc..e249ba1ae1 100644 --- a/lib/src/main/java/com/diffplug/spotless/PaddedCellBulk.java +++ b/lib/src/main/java/com/diffplug/spotless/PaddedCellBulk.java @@ -22,10 +22,8 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.nio.file.SimpleFileVisitor; -import java.nio.file.StandardOpenOption; import java.nio.file.attribute.BasicFileAttributes; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Locale; @@ -35,27 +33,8 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; -/** - * Incorporates the PaddedCell machinery into broader apply / check usage. - * - * Here's the general workflow: - * - * ### Identify that paddedCell is needed - * - * Initially, paddedCell is off. That's the default, and there's no need for users to know about it. - * - * If they encounter a scenario where `spotlessCheck` fails after calling `spotlessApply`, then they would - * justifiably be frustrated. Luckily, every time `spotlessCheck` fails, it passes the failed files to - * {@link #anyMisbehave(Formatter, List)}, which checks to see if any of the rules are causing a cycle - * or some other kind of mischief. If they are, it can give the user a special error message instructing - * them to turn on paddedCell. - * - * ### spotlessCheck with paddedCell on - * - * Spotless check behaves as normal, finding a list of problem files, but then passes that list - * to {@link #check(File, File, Formatter, List)}. If there were no problem files, then `paddedCell` - * is no longer necessary, so users might as well turn it off, so we give that info as a warning. - */ +/** COMPLETELY DEPRECATED, use {@link PaddedCell#canonicalIfDirty(Formatter, File)} instead. */ +@Deprecated public final class PaddedCellBulk { private static final Logger logger = Logger.getLogger(PaddedCellBulk.class.getName()); @@ -72,11 +51,13 @@ public final class PaddedCellBulk { * tell the user about a misbehaving rule and alert her to how to enable * paddedCell mode, with minimal effort. */ + @Deprecated public static boolean anyMisbehave(Formatter formatter, List problemFiles) { return anyMisbehave(formatter, problemFiles, 500); } /** Same as {@link #anyMisbehave(Formatter, List)} but with a customizable timeout. */ + @Deprecated public static boolean anyMisbehave(Formatter formatter, List problemFiles, long timeoutMs) { Objects.requireNonNull(formatter, "formatter"); Objects.requireNonNull(problemFiles, "problemFiles"); @@ -105,6 +86,7 @@ public static boolean anyMisbehave(Formatter formatter, List problemFiles, * @return A list of files which are failing because of paddedCell problems, but could be fixed. (specifically, the files for which spotlessApply would be effective) */ @SuppressFBWarnings("NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE") + @Deprecated public static List check(File rootDir, File diagnoseDir, Formatter formatter, List problemFiles) throws IOException { Objects.requireNonNull(rootDir, "rootDir"); Objects.requireNonNull(diagnoseDir, "diagnoseDir"); @@ -191,47 +173,20 @@ public String getName() { } /** Performs the typical spotlessApply, but with PaddedCell handling of misbehaving FormatterSteps. */ + @Deprecated public static void apply(Formatter formatter, File file) throws IOException { applyAnyChanged(formatter, file); } /** Performs the typical spotlessApply, but with PaddedCell handling of misbehaving FormatterSteps. */ + @Deprecated public static boolean applyAnyChanged(Formatter formatter, File file) throws IOException { - Objects.requireNonNull(formatter, "formatter"); - Objects.requireNonNull(file, "file"); - - byte[] rawBytes = Files.readAllBytes(file.toPath()); - String raw = new String(rawBytes, formatter.getEncoding()); - 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)) { + PaddedCell.DirtyState dirtyState = PaddedCell.calculateDirtyState(formatter, file); + if (dirtyState.isClean() || dirtyState.didNotConverge()) { return false; - } - - // F(input) != input, so we'll do a padded check - PaddedCell cell = PaddedCell.check(formatter, file, rawUnix); - if (!cell.isResolvable()) { - // nothing we can do, but check will warn and dump out the divergence path - return false; - } - - // 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 - Files.write(file.toPath(), canonicalBytes, StandardOpenOption.TRUNCATE_EXISTING); - return true; } else { - return false; + dirtyState.writeCanonicalTo(file); + return true; } } diff --git a/plugin-gradle/CHANGES.md b/plugin-gradle/CHANGES.md index 1c97e892de..dc1562e180 100644 --- a/plugin-gradle/CHANGES.md +++ b/plugin-gradle/CHANGES.md @@ -3,6 +3,8 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `3.27.0`). ## [Unreleased] +### Changed +* PaddedCell is now always enabled. It is strictly better than non-padded cell, and there is no performance penalty. [See here](https://github.com/diffplug/spotless/pull/560#issuecomment-621752798) for detailed explanation. ([#561](https://github.com/diffplug/spotless/pull/561)) ## [3.28.1] - 2020-04-02 ### Added diff --git a/plugin-gradle/README.md b/plugin-gradle/README.md index 9517fc4623..ebe9e958d8 100644 --- a/plugin-gradle/README.md +++ b/plugin-gradle/README.md @@ -168,8 +168,6 @@ Configuration for Groovy is similar to [Java](#java). Most java steps, like `li The groovy formatter's default behavior is to format all `.groovy` and `.java` files found in the Groovy source directories. If you would like to exclude the `.java` files, set the parameter `excludeJava`, or you can set the `target` parameter as described in the [Custom rules](#custom) section. -Due to cyclic ambiguities of groovy formatter results, e.g. for nested closures, the use of [paddedCell()](../PADDEDCELL.md) and/or [Custom rules](#custom) is recommended to bandaid over this third-party formatter problem. - ```gradle apply plugin: 'groovy' ... @@ -182,7 +180,6 @@ spotless { groovy { licenseHeaderFile 'spotless.license.java' excludeJava() // excludes all Java sources within the Groovy source dirs from formatting - paddedCell() // Avoid cyclic ambiguities // the Groovy Eclipse formatter extends the Java Eclipse formatter, // so it formats Java files by default (unless `excludeJava` is used). greclipse().configFile('greclipse.properties') diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java index b771c78e58..c193fbb8ca 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java @@ -67,16 +67,16 @@ private String formatName() { throw new IllegalStateException("This format is not contained by any SpotlessExtension."); } - boolean paddedCell = false; - /** Enables paddedCell mode. @see Padded cell */ + @Deprecated public void paddedCell() { paddedCell(true); } /** Enables paddedCell mode. @see Padded cell */ + @Deprecated public void paddedCell(boolean paddedCell) { - this.paddedCell = paddedCell; + root.project.getLogger().warn("PaddedCell is now always on, and cannot be turned off."); } LineEnding lineEndings; @@ -593,7 +593,6 @@ public EclipseWtpConfig eclipseWtp(EclipseWtpFormatterStep type, String version) /** Sets up a format task according to the values in this extension. */ protected void setupTask(SpotlessTask task) { - task.setPaddedCell(paddedCell); task.setEncoding(getEncoding().name()); task.setExceptionPolicy(exceptionPolicy); if (targetExclude == null) { diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/PaddedCellGradle.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/PaddedCellGradle.java deleted file mode 100644 index 6c69487054..0000000000 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/PaddedCellGradle.java +++ /dev/null @@ -1,103 +0,0 @@ -/* - * Copyright 2016 DiffPlug - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.diffplug.gradle.spotless; - -import java.io.File; -import java.io.IOException; -import java.nio.file.Path; -import java.util.List; - -import org.gradle.api.GradleException; - -import com.diffplug.common.base.StringPrinter; -import com.diffplug.spotless.Formatter; -import com.diffplug.spotless.PaddedCellBulk; - -import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; - -/** - * Incorporates the PaddedCell machinery into SpotlessTask.apply() and SpotlessTask.check(). - * - * Here's the general workflow: - * - * ### Identify that paddedCell is needed - * - * Initially, paddedCell is off. That's the default, and there's no need for users to know about it. - * - * If they encounter a scenario where `spotlessCheck` fails after calling `spotlessApply`, then they would - * justifiably be frustrated. Luckily, every time `spotlessCheck` fails, it passes the failed files to - * {@link #anyMisbehave(Formatter, List)}, which checks to see if any of the rules are causing a cycle - * or some other kind of mischief. If they are, it throws a special error message, - * {@link #youShouldTurnOnPaddedCell(SpotlessTask)} which tells them to turn on paddedCell. - * - * ### spotlessCheck with paddedCell on - * - * Spotless check behaves as normal, finding a list of problem files, but then passes that list - * to {@link #check(SpotlessTask, Formatter, List)}. If there were no problem files, then `paddedCell` - * is no longer necessary, so users might as well turn it off, so we give that info as a warning. - */ -// TODO: Cleanup this javadoc - it's a copy of the javadoc of PaddedCellBulk, so some info -// is out-of-date (like the link to #anyMisbehave(Formatter, List)) -class PaddedCellGradle { - /** URL to a page which describes the padded cell thing. */ - private static final String URL = "https://github.com/diffplug/spotless/blob/master/PADDEDCELL.md"; - - static GradleException youShouldTurnOnPaddedCell(SpotlessTask task) { - Path rootPath = task.getProject().getRootDir().toPath(); - return new GradleException(StringPrinter.buildStringFromLines( - "You have a misbehaving rule which can't make up its mind.", - "This means that spotlessCheck will fail even after spotlessApply has run.", - "", - "This is a bug in a formatting rule, not Spotless itself, but Spotless can", - "work around this bug and generate helpful bug reports for the broken rule", - "if you add 'paddedCell()' to your build.gradle as such: ", - "", - " spotless {", - " format 'someFormat', {", - " ...", - " paddedCell()", - " }", - " }", - "", - "The next time you run spotlessCheck, it will put helpful bug reports into", - "'" + rootPath.relativize(diagnoseDir(task).toPath()) + "', and spotlessApply", - "and spotlessCheck will be self-consistent from here on out.", - "", - "For details see " + URL)); - } - - private static File diagnoseDir(SpotlessTask task) { - return new File(task.getProject().getBuildDir(), "spotless-diagnose-" + task.formatName()); - } - - @SuppressFBWarnings("NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE") - static void check(SpotlessTask task, Formatter formatter, List problemFiles) throws IOException { - if (problemFiles.isEmpty()) { - // if the first pass was successful, then paddedCell() mode is unnecessary - task.getLogger().info(StringPrinter.buildStringFromLines( - task.getName() + " is in paddedCell() mode, but it doesn't need to be.", - "If you remove that option, spotless will run ~2x faster.", - "For details see " + URL)); - } - - File diagnoseDir = diagnoseDir(task); - File rootDir = task.getProject().getRootDir(); - List stillFailing = PaddedCellBulk.check(rootDir, diagnoseDir, formatter, problemFiles); - if (!stillFailing.isEmpty()) { - throw task.formatViolationsFor(formatter, problemFiles); - } - } -} diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessDiagnoseTask.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessDiagnoseTask.java new file mode 100644 index 0000000000..f20957e650 --- /dev/null +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessDiagnoseTask.java @@ -0,0 +1,75 @@ +/* + * Copyright 2016 DiffPlug + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.diffplug.gradle.spotless; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Locale; + +import org.gradle.api.DefaultTask; +import org.gradle.api.tasks.Internal; +import org.gradle.api.tasks.TaskAction; + +import com.diffplug.spotless.Formatter; +import com.diffplug.spotless.PaddedCell; + +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + +public class SpotlessDiagnoseTask extends DefaultTask { + SpotlessTask source; + + @Internal + public SpotlessTask getSource() { + return source; + } + + @TaskAction + @SuppressFBWarnings("NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE") + public void performAction() throws IOException { + Path srcRoot = getProject().getProjectDir().toPath(); + Path diagnoseRoot = getProject().getBuildDir().toPath().resolve("spotless-diagnose-" + source.formatName()); + getProject().delete(diagnoseRoot.toFile()); + try (Formatter formatter = source.buildFormatter()) { + for (File file : source.target) { + getLogger().debug("Running padded cell check on " + file); + PaddedCell padded = PaddedCell.check(formatter, file); + if (!padded.misbehaved()) { + getLogger().debug(" well-behaved."); + } else { + // the file is misbehaved, so we'll write all its steps to DIAGNOSE_DIR + Path relative = srcRoot.relativize(file.toPath()); + Path diagnoseFile = diagnoseRoot.resolve(relative); + for (int i = 0; i < padded.steps().size(); ++i) { + Path path = Paths.get(diagnoseFile + "." + padded.type().name().toLowerCase(Locale.ROOT) + i); + Files.createDirectories(path.getParent()); + String version = padded.steps().get(i); + Files.write(path, version.getBytes(formatter.getEncoding())); + } + // dump the type of the misbehavior to console + getLogger().lifecycle(" " + relative + " " + padded.userMessage()); + } + } + } + if (Files.exists(diagnoseRoot)) { + getLogger().lifecycle("Some formatters are misbehaving, you can see details at " + diagnoseRoot); + } else { + getLogger().lifecycle("All formatters are well behaved for all files."); + } + } +} diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtension.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtension.java index 4503317d97..7ebb62bab4 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtension.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtension.java @@ -38,12 +38,13 @@ public class SpotlessExtension { final Project project; - final Task rootCheckTask, rootApplyTask; + final Task rootCheckTask, rootApplyTask, rootDiagnoseTask; final RegisterDependenciesTask registerDependenciesTask; static final String EXTENSION = "spotless"; static final String CHECK = "Check"; static final String APPLY = "Apply"; + static final String DIAGNOSE = "Diagnose"; private static final String TASK_GROUP = "Verification"; private static final String CHECK_DESCRIPTION = "Checks that sourcecode satisfies formatting steps."; @@ -59,6 +60,8 @@ public SpotlessExtension(Project project) { rootApplyTask = project.task(EXTENSION + APPLY); rootApplyTask.setGroup(TASK_GROUP); rootApplyTask.setDescription(APPLY_DESCRIPTION); + rootDiagnoseTask = project.task(EXTENSION + DIAGNOSE); + rootDiagnoseTask.setGroup(TASK_GROUP); // no description on purpose RegisterDependenciesTask registerDependenciesTask = (RegisterDependenciesTask) project.getRootProject().getTasks().findByName(RegisterDependenciesTask.TASK_NAME); if (registerDependenciesTask == null) { @@ -293,5 +296,11 @@ public Object doCall(TaskExecutionGraph graph) { // the root tasks depend on the control tasks rootCheckTask.dependsOn(checkTask); rootApplyTask.dependsOn(applyTask); + + // create the diagnose task + SpotlessDiagnoseTask diagnoseTask = project.getTasks().create(taskName + DIAGNOSE, SpotlessDiagnoseTask.class); + diagnoseTask.source = spotlessTask; + rootDiagnoseTask.dependsOn(diagnoseTask); + diagnoseTask.mustRunAfter(clean); } } 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 1cca30ec5e..0a40d8b3ac 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 @@ -18,8 +18,6 @@ import java.io.File; import java.io.Serializable; import java.nio.charset.Charset; -import java.nio.file.Files; -import java.nio.file.StandardOpenOption; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -47,7 +45,6 @@ import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.LineEnding; import com.diffplug.spotless.PaddedCell; -import com.diffplug.spotless.PaddedCellBulk; import com.diffplug.spotless.extra.integration.DiffMessageFormatter; public class SpotlessTask extends DefaultTask { @@ -74,16 +71,15 @@ public void setLineEndingsPolicy(LineEnding.Policy lineEndingsPolicy) { this.lineEndingsPolicy = Objects.requireNonNull(lineEndingsPolicy); } - // set by FormatExtension - protected boolean paddedCell = false; - - @Input + @Deprecated + @Internal public boolean isPaddedCell() { - return paddedCell; + return true; } + @Deprecated public void setPaddedCell(boolean paddedCell) { - this.paddedCell = paddedCell; + getLogger().warn("PaddedCell is now always on, and cannot be turned off."); } protected String filePatterns = ""; @@ -228,14 +224,7 @@ public void performAction(IncrementalTaskInputs inputs) throws Exception { } // create the formatter - try (Formatter formatter = Formatter.builder() - .lineEndingsPolicy(lineEndingsPolicy) - .encoding(Charset.forName(encoding)) - .rootDir(getProject().getRootDir().toPath()) - .steps(steps) - .exceptionPolicy(exceptionPolicy) - .build()) { - + try (Formatter formatter = buildFormatter()) { if (apply) { List changedFiles = applyAnyChanged(formatter, outOfDate); if (!changedFiles.isEmpty()) { @@ -254,6 +243,16 @@ public void performAction(IncrementalTaskInputs inputs) throws Exception { } } + Formatter buildFormatter() { + return Formatter.builder() + .lineEndingsPolicy(lineEndingsPolicy) + .encoding(Charset.forName(encoding)) + .rootDir(getProject().getRootDir().toPath()) + .steps(steps) + .exceptionPolicy(exceptionPolicy) + .build(); + } + static class LastApply implements Serializable { private static final long serialVersionUID = 6245070824310295090L; @@ -263,42 +262,16 @@ static class LastApply implements Serializable { private List applyAnyChanged(Formatter formatter, List outOfDate) throws Exception { List changed = new ArrayList<>(outOfDate.size()); - if (isPaddedCell()) { - for (File file : outOfDate) { - getLogger().debug("Applying format to " + file); - if (PaddedCellBulk.applyAnyChanged(formatter, file)) { - changed.add(file); - } - } - } else { - boolean anyMisbehave = false; - for (File file : outOfDate) { - getLogger().debug("Applying format to " + file); - String unixResultIfDirty = formatter.applyToAndReturnResultIfDirty(file); - if (unixResultIfDirty != null) { - changed.add(file); - } - // because apply will count as up-to-date, it's important - // that every call to apply will get a PaddedCell check - if (!anyMisbehave && unixResultIfDirty != null) { - String onceMore = formatter.compute(unixResultIfDirty, file); - // f(f(input) == f(input) for an idempotent function - if (!onceMore.equals(unixResultIfDirty)) { - // it's not idempotent. but, if it converges, then it's likely a glitch that won't reoccur, - // so there's no need to make a bunch of noise for the user - PaddedCell result = PaddedCell.check(formatter, file, onceMore); - if (result.type() == PaddedCell.Type.CONVERGE) { - String finalResult = formatter.computeLineEndings(result.canonical(), file); - Files.write(file.toPath(), finalResult.getBytes(formatter.getEncoding()), StandardOpenOption.TRUNCATE_EXISTING); - } else { - // it didn't converge, so the user is going to need padded cell mode - anyMisbehave = true; - } - } - } - } - if (anyMisbehave) { - throw PaddedCellGradle.youShouldTurnOnPaddedCell(this); + for (File file : outOfDate) { + getLogger().debug("Applying format to " + file); + PaddedCell.DirtyState dirtyState = PaddedCell.calculateDirtyState(formatter, file); + if (dirtyState.isClean()) { + // do nothing + } else if (dirtyState.didNotConverge()) { + getLogger().warn("Skipping '" + file + "' because it does not converge. Run `spotlessDiagnose` to understand why"); + } else { + dirtyState.writeCanonicalTo(file); + changed.add(file); } } return changed; @@ -308,21 +281,17 @@ private void check(Formatter formatter, List outOfDate) throws Exception { List problemFiles = new ArrayList<>(); for (File file : outOfDate) { getLogger().debug("Checking format on " + file); - if (!formatter.isClean(file)) { + PaddedCell.DirtyState dirtyState = PaddedCell.calculateDirtyState(formatter, file); + if (dirtyState.isClean()) { + // do nothing + } else if (dirtyState.didNotConverge()) { + getLogger().warn("Skipping '" + file + "' because it does not converge. Run `spotlessDiagnose` to understand why"); + } else { problemFiles.add(file); } } - if (paddedCell) { - PaddedCellGradle.check(this, formatter, problemFiles); - } else { - if (!problemFiles.isEmpty()) { - // if we're not in paddedCell mode, we'll check if maybe we should be - if (PaddedCellBulk.anyMisbehave(formatter, problemFiles)) { - throw PaddedCellGradle.youShouldTurnOnPaddedCell(this); - } else { - throw formatViolationsFor(formatter, problemFiles); - } - } + if (!problemFiles.isEmpty()) { + throw formatViolationsFor(formatter, problemFiles); } } @@ -330,7 +299,6 @@ private void check(Formatter formatter, List outOfDate) throws Exception { GradleException formatViolationsFor(Formatter formatter, List problemFiles) { return new GradleException(DiffMessageFormatter.builder() .runToFix("Run 'gradlew spotlessApply' to fix these violations.") - .isPaddedCell(paddedCell) .formatter(formatter) .problemFiles(problemFiles) .getMessage()); diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PaddedCellTaskTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PaddedCellTaskTest.java index 1d05b4419c..1bd60f56f8 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PaddedCellTaskTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PaddedCellTaskTest.java @@ -23,7 +23,6 @@ import java.util.Collections; import java.util.Locale; -import org.assertj.core.api.Assertions; import org.gradle.api.Project; import org.junit.Assert; import org.junit.Test; @@ -44,12 +43,14 @@ private static String slashify(String input) { } private class Bundle { + String name; Project project = TestProvisioner.gradleProject(rootFolder()); File file; SpotlessTask check; SpotlessTask apply; Bundle(String name, FormatterFunc function) throws IOException { + this.name = name; file = setFile("src/test." + name).toContent("CCC"); FormatterStep step = FormatterStep.createNeverUpToDate(name, function); check = createCheckTask(name, step); @@ -76,12 +77,6 @@ private SpotlessTask createApplyTask(String name, FormatterStep step) { return task; } - private Bundle paddedCell() { - check.setPaddedCell(true); - apply.setPaddedCell(true); - return this; - } - private String checkFailureMsg() { try { execute(check); @@ -90,6 +85,12 @@ private String checkFailureMsg() { return e.getMessage(); } } + + private void diagnose() throws IOException { + SpotlessDiagnoseTask diagnose = project.getTasks().create("spotless" + SpotlessPlugin.capitalize(name) + "Diagnose", SpotlessDiagnoseTask.class); + diagnose.source = check; + diagnose.performAction(); + } } private Bundle wellbehaved() throws IOException { @@ -108,20 +109,12 @@ private Bundle diverge() throws IOException { return new Bundle("diverge", x -> x + " "); } - @Test - public void failsWithoutPaddedCell() throws IOException { - Assertions.assertThat(wellbehaved().checkFailureMsg()).startsWith("The following files had format violations"); - Assertions.assertThat(cycle().checkFailureMsg()).startsWith("You have a misbehaving rule"); - Assertions.assertThat(converge().checkFailureMsg()).startsWith("You have a misbehaving rule"); - Assertions.assertThat(diverge().checkFailureMsg()).startsWith("You have a misbehaving rule"); - } - @Test public void paddedCellApply() throws Exception { - Bundle wellbehaved = wellbehaved().paddedCell(); - Bundle cycle = cycle().paddedCell(); - Bundle converge = converge().paddedCell(); - Bundle diverge = diverge().paddedCell(); + Bundle wellbehaved = wellbehaved(); + Bundle cycle = cycle(); + Bundle converge = converge(); + Bundle diverge = diverge(); execute(wellbehaved.apply); execute(cycle.apply); @@ -140,11 +133,11 @@ public void paddedCellApply() throws Exception { } @Test - public void paddedCellCheckFailureFiles() throws Exception { - wellbehaved().paddedCell().checkFailureMsg(); - cycle().paddedCell().checkFailureMsg(); - converge().paddedCell().checkFailureMsg(); - execute(diverge().paddedCell().check); + public void diagnose() throws Exception { + wellbehaved().diagnose(); + cycle().diagnose(); + converge().diagnose(); + diverge().diagnose(); assertFolderContents("build", "spotless-diagnose-converge", @@ -179,7 +172,7 @@ private void assertFolderContents(String subfolderName, String... files) throws @Test public void paddedCellCheckCycleFailureMsg() throws IOException { - assertFailureMessage(cycle().paddedCell(), + assertFailureMessage(cycle(), "The following files had format violations:", slashify(" src/test.cycle"), " @@ -1 +1 @@", @@ -190,7 +183,7 @@ public void paddedCellCheckCycleFailureMsg() throws IOException { @Test public void paddedCellCheckConvergeFailureMsg() throws IOException { - assertFailureMessage(converge().paddedCell(), + assertFailureMessage(converge(), "The following files had format violations:", slashify(" src/test.converge"), " @@ -1 +0,0 @@",