Formatter is now AutoCloseable, so that facilitates FormatterFunc.Closeable works. (#284)

* Formatter is now AutoCloseable, which facilitates `FormatterFunc.Closeable`.

* Makes sure that we don't accidentally create NeverUpToDate steps with FormatterFunc.Closeable in the future.

* Updated changelog.

Author: nedtwigg

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)))
 

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();
+			}
+		}
+	}
 }

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<Integer> {
 		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);
 		}

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

@@ -113,52 +113,53 @@ public static List<File> 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<File> 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<File> 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(). */

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<File> 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<File> 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<File> 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<File> 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);
 		}
 	}
 

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<File> files = collectFiles(formatterFactory);
-		Formatter formatter = formatterFactory.newFormatter(files, getFormatterConfig());
-		process(files, formatter);
+		try (Formatter formatter = formatterFactory.newFormatter(files, getFormatterConfig())) {
+			process(files, formatter);
+		}
 	}
 
 	private List<File> collectFiles(FormatterFactory formatterFactory) throws MojoExecutionException {

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() {

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<FormatterStep> 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());
+			}
 		}
 	}