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

Make padded cell mandatory #561

Merged
merged 19 commits into from
May 3, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
a86e56c
Remove `paddedCell` option throughout Spotless, and deprecate its get…
nedtwigg Apr 30, 2020
eeb85ba
Update documentation for the always-paddedcell future.
nedtwigg Apr 30, 2020
ed81aad
Fix compile-errors in the "remove paddedcell" adventure.
nedtwigg Apr 30, 2020
f8ab7ee
Update spotbugs to latest.
nedtwigg Apr 30, 2020
d4af11f
Fix tests now that PaddedCell isn't present.
nedtwigg Apr 30, 2020
f47d6bb
Refactor PaddedCellBulk's important API into PaddedCell.canonicalIfDirty
nedtwigg Apr 30, 2020
345c519
Improve performance for the common case where PaddedCell is not needed.
nedtwigg Apr 30, 2020
18719fd
Deprecated all of PaddedCellBulk, deleted PaddedCellGradle.
nedtwigg Apr 30, 2020
8f54aa1
Renamed PaddedCell.canonicalIfDirty to PaddedCell.calculateDirtyState…
nedtwigg May 1, 2020
1684d65
Added the spotlessDiagnose task, which does what used to be baked-in …
nedtwigg May 1, 2020
7b81436
Fix spotbugs warning.
nedtwigg May 1, 2020
df30fdb
Small change to the PaddedCell.DirtyState API.
nedtwigg May 1, 2020
d8db03f
Minor changelog improvement.
nedtwigg May 1, 2020
9bfdc90
Update the lib changelog.
nedtwigg May 1, 2020
b9ac203
Improve the padded cell documentation.
nedtwigg May 2, 2020
8c1a006
Protect `PaddedCell.DirtyState` byte[] from user corruption
nedtwigg May 2, 2020
58def19
Merge branch 'master' into feat/paddedcell-mandatory
nedtwigg May 3, 2020
5fbf5bb
Oops! Meant to get the plain OutputStream, not the CORBA one.
nedtwigg May 3, 2020
36a8f97
Revert the spotbugs upgrade.
nedtwigg May 3, 2020
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
21 changes: 1 addition & 20 deletions PADDEDCELL.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,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:

Expand Down
20 changes: 17 additions & 3 deletions lib/src/main/java/com/diffplug/spotless/PaddedCell.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.StandardOpenOption;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
Expand All @@ -28,6 +29,8 @@
import java.util.Objects;
import java.util.function.Function;

import org.omg.CORBA.portable.OutputStream;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, did you mean to import java.io.OutputStream rather than org.omg.CORBA.portable.OutputStream here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lol, thanks, good catch!!! The dangers of not following yagni ;-)


/**
* Models the result of applying a {@link Formatter} on a given {@link File}
* while characterizing various failure modes (slow convergence, cycles, and divergence).
Expand Down Expand Up @@ -227,7 +230,7 @@ public static DirtyState calculateDirtyState(Formatter formatter, File file) thr
* - {@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 #canonicalBytes()} to get the canonical form of the given file.
* then you can call {@link #writeCanonicalTo()} to get the canonical form of the given file.
*/
public static class DirtyState {
private final byte[] canonicalBytes;
Expand All @@ -244,8 +247,19 @@ public boolean didNotConverge() {
return this == didNotConverge;
}

public byte[] canonicalBytes() {
return Objects.requireNonNull(canonicalBytes, "First make sure that `!isClean()` and `!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());
}
}

Expand Down
3 changes: 1 addition & 2 deletions lib/src/main/java/com/diffplug/spotless/PaddedCellBulk.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
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.Collections;
Expand Down Expand Up @@ -186,7 +185,7 @@ public static boolean applyAnyChanged(Formatter formatter, File file) throws IOE
if (dirtyState.isClean() || dirtyState.didNotConverge()) {
return false;
} else {
Files.write(file.toPath(), dirtyState.canonicalBytes(), StandardOpenOption.TRUNCATE_EXISTING);
dirtyState.writeCanonicalTo(file);
return true;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -272,7 +270,7 @@ private List<File> applyAnyChanged(Formatter formatter, List<File> outOfDate) th
} else if (dirtyState.didNotConverge()) {
getLogger().warn("Skipping '" + file + "' because it does not converge. Run `spotlessDiagnose` to understand why");
} else {
Files.write(file.toPath(), dirtyState.canonicalBytes(), StandardOpenOption.TRUNCATE_EXISTING);
dirtyState.writeCanonicalTo(file);
changed.add(file);
}
}
Expand Down