Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Aug 20, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion lib/src/main/java/com/diffplug/spotless/Formatter.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand All @@ -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);
}
Expand Down
69 changes: 35 additions & 34 deletions lib/src/main/java/com/diffplug/spotless/PaddedCellBulk.java
Original file line number Diff line number Diff line change
Expand Up @@ -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(). */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
33 changes: 17 additions & 16 deletions testlib/src/test/java/com/diffplug/spotless/PaddedCellTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}
}

Expand Down