Skip to content

Add --dry-run option to simplify CI system integrations #154

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
final class CommandLineOptions {

private final ImmutableList<String> files;
private final boolean dryRun;
private final boolean inPlace;
private final ImmutableRangeSet<Integer> lines;
private final ImmutableList<Integer> offsets;
Expand All @@ -41,6 +42,7 @@ final class CommandLineOptions {

CommandLineOptions(
ImmutableList<String> files,
boolean dryRun,
boolean inPlace,
ImmutableRangeSet<Integer> lines,
ImmutableList<Integer> offsets,
Expand All @@ -54,6 +56,7 @@ final class CommandLineOptions {
boolean sortImports,
boolean removeUnusedImports) {
this.files = files;
this.dryRun = dryRun;
this.inPlace = inPlace;
this.lines = lines;
this.offsets = offsets;
Expand All @@ -73,6 +76,11 @@ ImmutableList<String> files() {
return files;
}

/** Dry run. */
boolean dryRun() {
return dryRun;
}

/** Format files in place. */
boolean inPlace() {
return inPlace;
Expand Down Expand Up @@ -151,6 +159,7 @@ static class Builder {
private final ImmutableRangeSet.Builder<Integer> lines = ImmutableRangeSet.builder();
private final ImmutableList.Builder<Integer> offsets = ImmutableList.builder();
private final ImmutableList.Builder<Integer> lengths = ImmutableList.builder();
private Boolean dryRun = false;
private Boolean inPlace = false;
private Boolean aosp = false;
private Boolean version = false;
Expand All @@ -165,6 +174,11 @@ ImmutableList.Builder<String> filesBuilder() {
return files;
}

Builder dryRun(boolean dryRun) {
this.dryRun = dryRun;
return this;
}

Builder inPlace(boolean inPlace) {
this.inPlace = inPlace;
return this;
Expand Down Expand Up @@ -227,6 +241,7 @@ Builder removeUnusedImports(boolean removeUnusedImports) {
CommandLineOptions build() {
return new CommandLineOptions(
this.files.build(),
this.dryRun,
this.inPlace,
this.lines.build(),
this.offsets.build(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ static CommandLineOptions parse(Iterable<String> options) {
}
// NOTE: update usage information in UsageException when new flags are added
switch (flag) {
case "-n":
case "--dry-run":
optionsBuilder.dryRun(true);
break;
case "-i":
case "-r":
case "-replace":
Expand Down
18 changes: 17 additions & 1 deletion core/src/main/java/com/google/googlejavaformat/java/Main.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
import java.util.Arrays;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Set;
import java.util.TreeSet;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
Expand Down Expand Up @@ -131,6 +133,7 @@ private int formatFiles(CommandLineOptions parameters, JavaFormatterOptions opti
}

boolean allOk = true;
Set<Path> needFormatting = new TreeSet<>();
for (Map.Entry<Path, Future<String>> result : results.entrySet()) {
String formatted;
try {
Expand All @@ -151,7 +154,13 @@ private int formatFiles(CommandLineOptions parameters, JavaFormatterOptions opti
allOk = false;
continue;
}
if (parameters.inPlace()) {
if (parameters.dryRun()) {
if (formatted.equals(inputs.get(result.getKey()))) {
continue;
} else {
needFormatting.add(result.getKey());
}
} else if (parameters.inPlace()) {
if (formatted.equals(inputs.get(result.getKey()))) {
continue; // preserve original file
}
Expand All @@ -166,6 +175,13 @@ private int formatFiles(CommandLineOptions parameters, JavaFormatterOptions opti
outWriter.write(formatted);
}
}

if (!needFormatting.isEmpty()) {
errWriter.println("Need Formatting:");
errWriter.println(needFormatting);
return 1;
}

return allOk ? 0 : 1;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ public final class UsageException extends Exception {
"Usage: google-java-format [options] file(s)",
"",
"Options:",
" -n, --dry-run",
" Don't change anything, only check; supposed to be used form the CI.",
Copy link
Author

Choose a reason for hiding this comment

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

supposed to be used from the CI

" -i, -r, -replace, --replace",
" Send formatted output back to files, not stdout.",
" -",
Expand Down