Skip to content

Commit 245b35c

Browse files
nedtwiggnedtwigg
authored
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.
1 parent 946f543 commit 245b35c

File tree

8 files changed

+109
-84
lines changed

8 files changed

+109
-84
lines changed

‎CHANGES.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ You might be looking for:
99

1010
* Updated JSR305 annotation from 3.0.0 to 3.0.2 ([#274](https://github.com/diffplug/spotless/pull/274))
1111
* Migrated from FindBugs annotations 3.0.0 to SpotBugs annotations 3.1.6 ([#274](https://github.com/diffplug/spotless/pull/274))
12+
* `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))
1213

1314
### 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)))
1415

‎lib/src/main/java/com/diffplug/spotless/Formatter.java

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
import javax.annotation.Nullable;
3737

3838
/** Formatter which performs the full formatting. */
39-
public final class Formatter implements Serializable {
39+
public final class Formatter implements Serializable, AutoCloseable {
4040
private static final long serialVersionUID = 1L;
4141

4242
private LineEnding.Policy lineEndingsPolicy;
@@ -274,4 +274,14 @@ public boolean equals(Object obj) {
274274
steps.equals(other.steps) &&
275275
exceptionPolicy.equals(other.exceptionPolicy);
276276
}
277+
278+
@SuppressWarnings("rawtypes")
279+
@Override
280+
public void close() {
281+
for (FormatterStep step : steps) {
282+
if (step instanceof FormatterStepImpl.Standard) {
283+
((FormatterStepImpl.Standard) step).cleanupFormatterFunc();
284+
}
285+
}
286+
}
277287
}

‎lib/src/main/java/com/diffplug/spotless/FormatterStepImpl.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,12 @@ protected String format(State state, String rawUnix, File file) throws Exception
7777
}
7878
return formatter.apply(rawUnix);
7979
}
80+
81+
void cleanupFormatterFunc() {
82+
if (formatter instanceof FormatterFunc.Closeable) {
83+
((FormatterFunc.Closeable) formatter).close();
84+
}
85+
}
8086
}
8187

8288
/** Formatter which is equal to itself, but not to any other Formatter. */
@@ -97,6 +103,9 @@ static class NeverUpToDate extends FormatterStepImpl<Integer> {
97103
protected String format(Integer state, String rawUnix, File file) throws Exception {
98104
if (formatter == null) {
99105
formatter = formatterSupplier.get();
106+
if (formatter instanceof FormatterFunc.Closeable) {
107+
throw new AssertionError("NeverUpToDate does not support FormatterFunc.Closeable. See https://github.com/diffplug/spotless/pull/284");
108+
}
100109
}
101110
return formatter.apply(rawUnix);
102111
}

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

Lines changed: 35 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -113,52 +113,53 @@ public static List<File> check(File rootDir, File diagnoseDir, Formatter formatt
113113

114114
// "fake" Formatter which can use the already-computed result of a PaddedCell as
115115
FakeStep paddedCellStep = new FakeStep();
116-
Formatter paddedFormatter = Formatter.builder()
116+
try (Formatter paddedFormatter = Formatter.builder()
117117
.lineEndingsPolicy(formatter.getLineEndingsPolicy())
118118
.encoding(formatter.getEncoding())
119119
.rootDir(formatter.getRootDir())
120120
.steps(Collections.singletonList(paddedCellStep))
121121
.exceptionPolicy(formatter.getExceptionPolicy())
122-
.build();
122+
.build()) {
123123

124-
// empty out the diagnose folder
125-
Path rootPath = rootDir.toPath();
126-
Path diagnosePath = diagnoseDir.toPath();
127-
cleanDir(diagnosePath);
124+
// empty out the diagnose folder
125+
Path rootPath = rootDir.toPath();
126+
Path diagnosePath = diagnoseDir.toPath();
127+
cleanDir(diagnosePath);
128128

129-
List<File> stillFailing = new ArrayList<>();
130-
for (File problemFile : problemFiles) {
131-
logger.fine("Running padded cell check on " + problemFile);
132-
PaddedCell padded = PaddedCell.check(formatter, problemFile);
133-
if (!padded.misbehaved()) {
134-
logger.fine(" well-behaved.");
135-
} else {
136-
// the file is misbehaved, so we'll write all its steps to DIAGNOSE_DIR
137-
Path relative = rootPath.relativize(problemFile.toPath());
138-
Path diagnoseFile = diagnosePath.resolve(relative);
139-
for (int i = 0; i < padded.steps().size(); ++i) {
140-
Path path = Paths.get(diagnoseFile + "." + padded.type().name().toLowerCase(Locale.ROOT) + i);
141-
Files.createDirectories(path.getParent());
142-
String version = padded.steps().get(i);
143-
Files.write(path, version.getBytes(formatter.getEncoding()));
144-
}
145-
// dump the type of the misbehavior to console
146-
logger.finer(" " + relative + " " + padded.userMessage());
147-
148-
if (!padded.isResolvable()) {
149-
// if it's not resolvable, then there's
150-
// no point killing the build over it
129+
List<File> stillFailing = new ArrayList<>();
130+
for (File problemFile : problemFiles) {
131+
logger.fine("Running padded cell check on " + problemFile);
132+
PaddedCell padded = PaddedCell.check(formatter, problemFile);
133+
if (!padded.misbehaved()) {
134+
logger.fine(" well-behaved.");
151135
} else {
152-
// if the input is resolvable, we'll use that to try again at
153-
// determining if it's clean
154-
paddedCellStep.set(problemFile, padded.canonical());
155-
if (!paddedFormatter.isClean(problemFile)) {
156-
stillFailing.add(problemFile);
136+
// the file is misbehaved, so we'll write all its steps to DIAGNOSE_DIR
137+
Path relative = rootPath.relativize(problemFile.toPath());
138+
Path diagnoseFile = diagnosePath.resolve(relative);
139+
for (int i = 0; i < padded.steps().size(); ++i) {
140+
Path path = Paths.get(diagnoseFile + "." + padded.type().name().toLowerCase(Locale.ROOT) + i);
141+
Files.createDirectories(path.getParent());
142+
String version = padded.steps().get(i);
143+
Files.write(path, version.getBytes(formatter.getEncoding()));
144+
}
145+
// dump the type of the misbehavior to console
146+
logger.finer(" " + relative + " " + padded.userMessage());
147+
148+
if (!padded.isResolvable()) {
149+
// if it's not resolvable, then there's
150+
// no point killing the build over it
151+
} else {
152+
// if the input is resolvable, we'll use that to try again at
153+
// determining if it's clean
154+
paddedCellStep.set(problemFile, padded.canonical());
155+
if (!paddedFormatter.isClean(problemFile)) {
156+
stillFailing.add(problemFile);
157+
}
157158
}
158159
}
159160
}
161+
return stillFailing;
160162
}
161-
return stillFailing;
162163
}
163164

164165
/** Helper for check(). */

‎plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java

Lines changed: 32 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -161,48 +161,49 @@ public void performAction(IncrementalTaskInputs inputs) throws Exception {
161161
}
162162

163163
// create the formatter
164-
Formatter formatter = Formatter.builder()
164+
try (Formatter formatter = Formatter.builder()
165165
.lineEndingsPolicy(lineEndingsPolicy)
166166
.encoding(Charset.forName(encoding))
167167
.rootDir(getProject().getRootDir().toPath())
168168
.steps(steps)
169169
.exceptionPolicy(exceptionPolicy)
170-
.build();
171-
// find the outOfDate files
172-
List<File> outOfDate = new ArrayList<>();
173-
inputs.outOfDate(inputDetails -> {
174-
File file = inputDetails.getFile();
175-
if (file.isFile() && !file.equals(getCacheFile())) {
176-
outOfDate.add(file);
177-
}
178-
});
179-
// load the files that were changed by the last run
180-
// because it's possible the user changed them back to their
181-
// unformatted form, so we need to treat them as dirty
182-
// (see bug #144)
183-
if (getCacheFile().exists()) {
184-
LastApply lastApply = SerializableMisc.fromFile(LastApply.class, getCacheFile());
185-
for (File file : lastApply.changedFiles) {
186-
if (!outOfDate.contains(file) && file.exists() && Iterables.contains(target, file)) {
170+
.build()) {
171+
// find the outOfDate files
172+
List<File> outOfDate = new ArrayList<>();
173+
inputs.outOfDate(inputDetails -> {
174+
File file = inputDetails.getFile();
175+
if (file.isFile() && !file.equals(getCacheFile())) {
187176
outOfDate.add(file);
188177
}
178+
});
179+
// load the files that were changed by the last run
180+
// because it's possible the user changed them back to their
181+
// unformatted form, so we need to treat them as dirty
182+
// (see bug #144)
183+
if (getCacheFile().exists()) {
184+
LastApply lastApply = SerializableMisc.fromFile(LastApply.class, getCacheFile());
185+
for (File file : lastApply.changedFiles) {
186+
if (!outOfDate.contains(file) && file.exists() && Iterables.contains(target, file)) {
187+
outOfDate.add(file);
188+
}
189+
}
189190
}
190-
}
191191

192-
if (apply) {
193-
List<File> changedFiles = applyAnyChanged(formatter, outOfDate);
194-
if (!changedFiles.isEmpty()) {
195-
// If any file changed, we need to mark the task as dirty
196-
// next time to avoid bug #144.
197-
LastApply lastApply = new LastApply();
198-
lastApply.timestamp = System.currentTimeMillis();
199-
lastApply.changedFiles = changedFiles;
192+
if (apply) {
193+
List<File> changedFiles = applyAnyChanged(formatter, outOfDate);
194+
if (!changedFiles.isEmpty()) {
195+
// If any file changed, we need to mark the task as dirty
196+
// next time to avoid bug #144.
197+
LastApply lastApply = new LastApply();
198+
lastApply.timestamp = System.currentTimeMillis();
199+
lastApply.changedFiles = changedFiles;
200200

201-
SerializableMisc.toFile(lastApply, getCacheFile());
201+
SerializableMisc.toFile(lastApply, getCacheFile());
202+
}
203+
}
204+
if (check) {
205+
check(formatter, outOfDate);
202206
}
203-
}
204-
if (check) {
205-
check(formatter, outOfDate);
206207
}
207208
}
208209

‎plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,9 @@ public final void execute() throws MojoExecutionException {
9999

100100
private void execute(FormatterFactory formatterFactory) throws MojoExecutionException {
101101
List<File> files = collectFiles(formatterFactory);
102-
Formatter formatter = formatterFactory.newFormatter(files, getFormatterConfig());
103-
process(files, formatter);
102+
try (Formatter formatter = formatterFactory.newFormatter(files, getFormatterConfig())) {
103+
process(files, formatter);
104+
}
104105
}
105106

106107
private List<File> collectFiles(FormatterFactory formatterFactory) throws MojoExecutionException {

‎testlib/src/test/java/com/diffplug/spotless/FormatterTest.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import com.diffplug.spotless.generic.EndWithNewlineStep;
2929

3030
public class FormatterTest {
31+
// Formatter normally needs to be closed, but no resources will be leaked in this special case
3132
@Test
3233
public void equality() {
3334
new SerializableEqualityTester() {

‎testlib/src/test/java/com/diffplug/spotless/PaddedCellTest.java

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -49,29 +49,30 @@ private void wellBehaved(FormatterFunc step, String input, PaddedCell.Type expec
4949
private void testCase(FormatterFunc step, String input, PaddedCell.Type expectedOutputType, String expectedSteps, String canonical, boolean misbehaved) throws IOException {
5050
List<FormatterStep> formatterSteps = new ArrayList<>();
5151
formatterSteps.add(FormatterStep.createNeverUpToDate("step", step));
52-
Formatter formatter = Formatter.builder()
52+
try (Formatter formatter = Formatter.builder()
5353
.lineEndingsPolicy(LineEnding.UNIX.createPolicy())
5454
.encoding(StandardCharsets.UTF_8)
5555
.rootDir(folder.getRoot().toPath())
56-
.steps(formatterSteps).build();
56+
.steps(formatterSteps).build()) {
5757

58-
File file = folder.newFile();
59-
Files.write(file.toPath(), input.getBytes(StandardCharsets.UTF_8));
58+
File file = folder.newFile();
59+
Files.write(file.toPath(), input.getBytes(StandardCharsets.UTF_8));
6060

61-
PaddedCell result = PaddedCell.check(formatter, file);
62-
Assert.assertEquals(misbehaved, result.misbehaved());
63-
Assert.assertEquals(expectedOutputType, result.type());
61+
PaddedCell result = PaddedCell.check(formatter, file);
62+
Assert.assertEquals(misbehaved, result.misbehaved());
63+
Assert.assertEquals(expectedOutputType, result.type());
6464

65-
String actual = String.join(",", result.steps());
66-
Assert.assertEquals(expectedSteps, actual);
65+
String actual = String.join(",", result.steps());
66+
Assert.assertEquals(expectedSteps, actual);
6767

68-
if (canonical == null) {
69-
try {
70-
result.canonical();
71-
Assert.fail("Expected exception");
72-
} catch (IllegalArgumentException expected) {}
73-
} else {
74-
Assert.assertEquals(canonical, result.canonical());
68+
if (canonical == null) {
69+
try {
70+
result.canonical();
71+
Assert.fail("Expected exception");
72+
} catch (IllegalArgumentException expected) {}
73+
} else {
74+
Assert.assertEquals(canonical, result.canonical());
75+
}
7576
}
7677
}
7778

0 commit comments

Comments
 (0)