diff --git a/CHANGES.md b/CHANGES.md index 9f8b7a56c8..88cebb33ed 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -9,6 +9,7 @@ You might be looking for: * Updated JSR305 annotation from 3.0.0 to 3.0.2 ([#274](https://github.com/diffplug/spotless/pull/274)) * Migrated from FindBugs annotations 3.0.0 to SpotBugs annotations 3.1.6 ([#274](https://github.com/diffplug/spotless/pull/274)) +* `Formatter` now implements `AutoCloseable`. This means that users of `Formatter` are expected to use the try-with-resources pattern. The reason for this change is so that `FormatterFunc.Closeable` actually works. ([#284](https://github.com/diffplug/spotless/pull/284)) ### Version 1.14.0 - July 24th 2018 (javadoc [lib](https://diffplug.github.io/spotless/javadoc/spotless-lib/1.14.0/) [lib-extra](https://diffplug.github.io/spotless/javadoc/spotless-lib-extra/1.14.0/), artifact [lib]([jcenter](https://bintray.com/diffplug/opensource/spotless-lib), [lib-extra]([jcenter](https://bintray.com/diffplug/opensource/spotless-lib-extra))) diff --git a/lib/src/main/java/com/diffplug/spotless/Formatter.java b/lib/src/main/java/com/diffplug/spotless/Formatter.java index 8cfef38f02..038e77fa64 100644 --- a/lib/src/main/java/com/diffplug/spotless/Formatter.java +++ b/lib/src/main/java/com/diffplug/spotless/Formatter.java @@ -36,7 +36,7 @@ import javax.annotation.Nullable; /** Formatter which performs the full formatting. */ -public final class Formatter implements Serializable { +public final class Formatter implements Serializable, AutoCloseable { private static final long serialVersionUID = 1L; private LineEnding.Policy lineEndingsPolicy; @@ -274,4 +274,14 @@ public boolean equals(Object obj) { steps.equals(other.steps) && exceptionPolicy.equals(other.exceptionPolicy); } + + @SuppressWarnings("rawtypes") + @Override + public void close() { + for (FormatterStep step : steps) { + if (step instanceof FormatterStepImpl.Standard) { + ((FormatterStepImpl.Standard) step).cleanupFormatterFunc(); + } + } + } } diff --git a/lib/src/main/java/com/diffplug/spotless/FormatterStepImpl.java b/lib/src/main/java/com/diffplug/spotless/FormatterStepImpl.java index 35a124df75..51eb11a141 100644 --- a/lib/src/main/java/com/diffplug/spotless/FormatterStepImpl.java +++ b/lib/src/main/java/com/diffplug/spotless/FormatterStepImpl.java @@ -77,6 +77,12 @@ protected String format(State state, String rawUnix, File file) throws Exception } return formatter.apply(rawUnix); } + + void cleanupFormatterFunc() { + if (formatter instanceof FormatterFunc.Closeable) { + ((FormatterFunc.Closeable) formatter).close(); + } + } } /** Formatter which is equal to itself, but not to any other Formatter. */ @@ -97,6 +103,9 @@ static class NeverUpToDate extends FormatterStepImpl { protected String format(Integer state, String rawUnix, File file) throws Exception { if (formatter == null) { formatter = formatterSupplier.get(); + if (formatter instanceof FormatterFunc.Closeable) { + throw new AssertionError("NeverUpToDate does not support FormatterFunc.Closeable. See https://github.com/diffplug/spotless/pull/284"); + } } return formatter.apply(rawUnix); } diff --git a/lib/src/main/java/com/diffplug/spotless/PaddedCellBulk.java b/lib/src/main/java/com/diffplug/spotless/PaddedCellBulk.java index eb8dfd2e67..6037146058 100644 --- a/lib/src/main/java/com/diffplug/spotless/PaddedCellBulk.java +++ b/lib/src/main/java/com/diffplug/spotless/PaddedCellBulk.java @@ -113,52 +113,53 @@ public static List check(File rootDir, File diagnoseDir, Formatter formatt // "fake" Formatter which can use the already-computed result of a PaddedCell as FakeStep paddedCellStep = new FakeStep(); - Formatter paddedFormatter = Formatter.builder() + try (Formatter paddedFormatter = Formatter.builder() .lineEndingsPolicy(formatter.getLineEndingsPolicy()) .encoding(formatter.getEncoding()) .rootDir(formatter.getRootDir()) .steps(Collections.singletonList(paddedCellStep)) .exceptionPolicy(formatter.getExceptionPolicy()) - .build(); + .build()) { - // empty out the diagnose folder - Path rootPath = rootDir.toPath(); - Path diagnosePath = diagnoseDir.toPath(); - cleanDir(diagnosePath); + // empty out the diagnose folder + Path rootPath = rootDir.toPath(); + Path diagnosePath = diagnoseDir.toPath(); + cleanDir(diagnosePath); - List stillFailing = new ArrayList<>(); - for (File problemFile : problemFiles) { - logger.fine("Running padded cell check on " + problemFile); - PaddedCell padded = PaddedCell.check(formatter, problemFile); - if (!padded.misbehaved()) { - logger.fine(" well-behaved."); - } else { - // the file is misbehaved, so we'll write all its steps to DIAGNOSE_DIR - Path relative = rootPath.relativize(problemFile.toPath()); - Path diagnoseFile = diagnosePath.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 - logger.finer(" " + relative + " " + padded.userMessage()); - - if (!padded.isResolvable()) { - // if it's not resolvable, then there's - // no point killing the build over it + List stillFailing = new ArrayList<>(); + for (File problemFile : problemFiles) { + logger.fine("Running padded cell check on " + problemFile); + PaddedCell padded = PaddedCell.check(formatter, problemFile); + if (!padded.misbehaved()) { + logger.fine(" well-behaved."); } else { - // if the input is resolvable, we'll use that to try again at - // determining if it's clean - paddedCellStep.set(problemFile, padded.canonical()); - if (!paddedFormatter.isClean(problemFile)) { - stillFailing.add(problemFile); + // the file is misbehaved, so we'll write all its steps to DIAGNOSE_DIR + Path relative = rootPath.relativize(problemFile.toPath()); + Path diagnoseFile = diagnosePath.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 + logger.finer(" " + relative + " " + padded.userMessage()); + + if (!padded.isResolvable()) { + // if it's not resolvable, then there's + // no point killing the build over it + } else { + // if the input is resolvable, we'll use that to try again at + // determining if it's clean + paddedCellStep.set(problemFile, padded.canonical()); + if (!paddedFormatter.isClean(problemFile)) { + stillFailing.add(problemFile); + } } } } + return stillFailing; } - return stillFailing; } /** Helper for check(). */ 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 801e45f17d..7201af4a6f 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 @@ -161,48 +161,49 @@ public void performAction(IncrementalTaskInputs inputs) throws Exception { } // create the formatter - Formatter formatter = Formatter.builder() + try (Formatter formatter = Formatter.builder() .lineEndingsPolicy(lineEndingsPolicy) .encoding(Charset.forName(encoding)) .rootDir(getProject().getRootDir().toPath()) .steps(steps) .exceptionPolicy(exceptionPolicy) - .build(); - // find the outOfDate files - List outOfDate = new ArrayList<>(); - inputs.outOfDate(inputDetails -> { - File file = inputDetails.getFile(); - if (file.isFile() && !file.equals(getCacheFile())) { - outOfDate.add(file); - } - }); - // load the files that were changed by the last run - // because it's possible the user changed them back to their - // unformatted form, so we need to treat them as dirty - // (see bug #144) - if (getCacheFile().exists()) { - LastApply lastApply = SerializableMisc.fromFile(LastApply.class, getCacheFile()); - for (File file : lastApply.changedFiles) { - if (!outOfDate.contains(file) && file.exists() && Iterables.contains(target, file)) { + .build()) { + // find the outOfDate files + List outOfDate = new ArrayList<>(); + inputs.outOfDate(inputDetails -> { + File file = inputDetails.getFile(); + if (file.isFile() && !file.equals(getCacheFile())) { outOfDate.add(file); } + }); + // load the files that were changed by the last run + // because it's possible the user changed them back to their + // unformatted form, so we need to treat them as dirty + // (see bug #144) + if (getCacheFile().exists()) { + LastApply lastApply = SerializableMisc.fromFile(LastApply.class, getCacheFile()); + for (File file : lastApply.changedFiles) { + if (!outOfDate.contains(file) && file.exists() && Iterables.contains(target, file)) { + outOfDate.add(file); + } + } } - } - if (apply) { - List changedFiles = applyAnyChanged(formatter, outOfDate); - if (!changedFiles.isEmpty()) { - // If any file changed, we need to mark the task as dirty - // next time to avoid bug #144. - LastApply lastApply = new LastApply(); - lastApply.timestamp = System.currentTimeMillis(); - lastApply.changedFiles = changedFiles; + if (apply) { + List changedFiles = applyAnyChanged(formatter, outOfDate); + if (!changedFiles.isEmpty()) { + // If any file changed, we need to mark the task as dirty + // next time to avoid bug #144. + LastApply lastApply = new LastApply(); + lastApply.timestamp = System.currentTimeMillis(); + lastApply.changedFiles = changedFiles; - SerializableMisc.toFile(lastApply, getCacheFile()); + SerializableMisc.toFile(lastApply, getCacheFile()); + } + } + if (check) { + check(formatter, outOfDate); } - } - if (check) { - check(formatter, outOfDate); } } 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 8dccb5d63f..c30f3b1831 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 @@ -99,8 +99,9 @@ public final void execute() throws MojoExecutionException { private void execute(FormatterFactory formatterFactory) throws MojoExecutionException { List files = collectFiles(formatterFactory); - Formatter formatter = formatterFactory.newFormatter(files, getFormatterConfig()); - process(files, formatter); + try (Formatter formatter = formatterFactory.newFormatter(files, getFormatterConfig())) { + process(files, formatter); + } } private List collectFiles(FormatterFactory formatterFactory) throws MojoExecutionException { diff --git a/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java b/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java index 02089058fa..0b6e65436c 100644 --- a/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java @@ -28,6 +28,7 @@ import com.diffplug.spotless.generic.EndWithNewlineStep; public class FormatterTest { + // Formatter normally needs to be closed, but no resources will be leaked in this special case @Test public void equality() { new SerializableEqualityTester() { diff --git a/testlib/src/test/java/com/diffplug/spotless/PaddedCellTest.java b/testlib/src/test/java/com/diffplug/spotless/PaddedCellTest.java index d801415daf..cfdf8258b9 100644 --- a/testlib/src/test/java/com/diffplug/spotless/PaddedCellTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/PaddedCellTest.java @@ -49,29 +49,30 @@ private void wellBehaved(FormatterFunc step, String input, PaddedCell.Type expec private void testCase(FormatterFunc step, String input, PaddedCell.Type expectedOutputType, String expectedSteps, String canonical, boolean misbehaved) throws IOException { List formatterSteps = new ArrayList<>(); formatterSteps.add(FormatterStep.createNeverUpToDate("step", step)); - Formatter formatter = Formatter.builder() + try (Formatter formatter = Formatter.builder() .lineEndingsPolicy(LineEnding.UNIX.createPolicy()) .encoding(StandardCharsets.UTF_8) .rootDir(folder.getRoot().toPath()) - .steps(formatterSteps).build(); + .steps(formatterSteps).build()) { - File file = folder.newFile(); - Files.write(file.toPath(), input.getBytes(StandardCharsets.UTF_8)); + File file = folder.newFile(); + Files.write(file.toPath(), input.getBytes(StandardCharsets.UTF_8)); - PaddedCell result = PaddedCell.check(formatter, file); - Assert.assertEquals(misbehaved, result.misbehaved()); - Assert.assertEquals(expectedOutputType, result.type()); + PaddedCell result = PaddedCell.check(formatter, file); + Assert.assertEquals(misbehaved, result.misbehaved()); + Assert.assertEquals(expectedOutputType, result.type()); - String actual = String.join(",", result.steps()); - Assert.assertEquals(expectedSteps, actual); + String actual = String.join(",", result.steps()); + Assert.assertEquals(expectedSteps, actual); - if (canonical == null) { - try { - result.canonical(); - Assert.fail("Expected exception"); - } catch (IllegalArgumentException expected) {} - } else { - Assert.assertEquals(canonical, result.canonical()); + if (canonical == null) { + try { + result.canonical(); + Assert.fail("Expected exception"); + } catch (IllegalArgumentException expected) {} + } else { + Assert.assertEquals(canonical, result.canonical()); + } } }