From db2713a7b81e20cb135d58f3a48a634bc7df2f05 Mon Sep 17 00:00:00 2001 From: Liam Miller-Cushon Date: Tue, 8 Apr 2025 16:48:38 -0700 Subject: [PATCH] Migrate to CommandLineOptions to a record and AutoBuilder This allows the CheckReturnValue analysis to tell that it's a builder and ignore the results of builder methods. PiperOrigin-RevId: 745341492 --- .../java/CommandLineOptions.java | 322 +++++------------- .../java/CommandLineOptionsParser.java | 8 +- 2 files changed, 83 insertions(+), 247 deletions(-) diff --git a/core/src/main/java/com/google/googlejavaformat/java/CommandLineOptions.java b/core/src/main/java/com/google/googlejavaformat/java/CommandLineOptions.java index d5480a790..e794a1815 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/CommandLineOptions.java +++ b/core/src/main/java/com/google/googlejavaformat/java/CommandLineOptions.java @@ -14,291 +14,121 @@ package com.google.googlejavaformat.java; +import com.google.auto.value.AutoBuilder; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableRangeSet; -import com.google.common.collect.RangeSet; -import com.google.common.collect.TreeRangeSet; +import com.google.errorprone.annotations.CanIgnoreReturnValue; import java.util.Optional; /** * Command line options for google-java-format. * - *

google-java-format doesn't depend on AutoValue, to allow AutoValue to depend on - * google-java-format. + * @param files The files to format. + * @param inPlace Format files in place. + * @param lines Line ranges to format. + * @param offsets Character offsets for partial formatting, paired with {@code lengths}. + * @param lengths Partial formatting region lengths, paired with {@code offsets}. + * @param aosp Use AOSP style instead of Google Style (4-space indentation). + * @param version Print the version. + * @param help Print usage information. + * @param stdin Format input from stdin. + * @param fixImportsOnly Fix imports, but do no formatting. + * @param sortImports Sort imports. + * @param removeUnusedImports Remove unused imports. + * @param dryRun Print the paths of the files whose contents would change if the formatter were run + * normally. + * @param setExitIfChanged Return exit code 1 if there are any formatting changes. + * @param assumeFilename Return the name to use for diagnostics when formatting standard input. */ -final class CommandLineOptions { - - private final ImmutableList files; - private final boolean inPlace; - private final ImmutableRangeSet lines; - private final ImmutableList offsets; - private final ImmutableList lengths; - private final boolean aosp; - private final boolean version; - private final boolean help; - private final boolean stdin; - private final boolean fixImportsOnly; - private final boolean sortImports; - private final boolean removeUnusedImports; - private final boolean dryRun; - private final boolean setExitIfChanged; - private final Optional assumeFilename; - private final boolean reflowLongStrings; - private final boolean formatJavadoc; - - CommandLineOptions( - ImmutableList files, - boolean inPlace, - ImmutableRangeSet lines, - ImmutableList offsets, - ImmutableList lengths, - boolean aosp, - boolean version, - boolean help, - boolean stdin, - boolean fixImportsOnly, - boolean sortImports, - boolean removeUnusedImports, - boolean dryRun, - boolean setExitIfChanged, - Optional assumeFilename, - boolean reflowLongStrings, - boolean formatJavadoc) { - this.files = files; - this.inPlace = inPlace; - this.lines = lines; - this.offsets = offsets; - this.lengths = lengths; - this.aosp = aosp; - this.version = version; - this.help = help; - this.stdin = stdin; - this.fixImportsOnly = fixImportsOnly; - this.sortImports = sortImports; - this.removeUnusedImports = removeUnusedImports; - this.dryRun = dryRun; - this.setExitIfChanged = setExitIfChanged; - this.assumeFilename = assumeFilename; - this.reflowLongStrings = reflowLongStrings; - this.formatJavadoc = formatJavadoc; - } - - /** The files to format. */ - ImmutableList files() { - return files; - } - - /** Format files in place. */ - boolean inPlace() { - return inPlace; - } - - /** Line ranges to format. */ - ImmutableRangeSet lines() { - return lines; - } - - /** Character offsets for partial formatting, paired with {@code lengths}. */ - ImmutableList offsets() { - return offsets; - } - - /** Partial formatting region lengths, paired with {@code offsets}. */ - ImmutableList lengths() { - return lengths; - } - - /** Use AOSP style instead of Google Style (4-space indentation). */ - boolean aosp() { - return aosp; - } - - /** Print the version. */ - boolean version() { - return version; - } - - /** Print usage information. */ - boolean help() { - return help; - } - - /** Format input from stdin. */ - boolean stdin() { - return stdin; - } - - /** Fix imports, but do no formatting. */ - boolean fixImportsOnly() { - return fixImportsOnly; - } - - /** Sort imports. */ - boolean sortImports() { - return sortImports; - } - - /** Remove unused imports. */ - boolean removeUnusedImports() { - return removeUnusedImports; - } - - /** - * Print the paths of the files whose contents would change if the formatter were run normally. - */ - boolean dryRun() { - return dryRun; - } - - /** Return exit code 1 if there are any formatting changes. */ - boolean setExitIfChanged() { - return setExitIfChanged; - } - - /** Return the name to use for diagnostics when formatting standard input. */ - Optional assumeFilename() { - return assumeFilename; - } - - boolean reflowLongStrings() { - return reflowLongStrings; - } +record CommandLineOptions( + ImmutableList files, + boolean inPlace, + ImmutableRangeSet lines, + ImmutableList offsets, + ImmutableList lengths, + boolean aosp, + boolean version, + boolean help, + boolean stdin, + boolean fixImportsOnly, + boolean sortImports, + boolean removeUnusedImports, + boolean dryRun, + boolean setExitIfChanged, + Optional assumeFilename, + boolean reflowLongStrings, + boolean formatJavadoc) { /** Returns true if partial formatting was selected. */ boolean isSelection() { return !lines().isEmpty() || !offsets().isEmpty() || !lengths().isEmpty(); } - boolean formatJavadoc() { - return formatJavadoc; - } - static Builder builder() { - return new Builder(); + return new AutoBuilder_CommandLineOptions_Builder() + .sortImports(true) + .removeUnusedImports(true) + .reflowLongStrings(true) + .formatJavadoc(true) + .aosp(false) + .version(false) + .help(false) + .stdin(false) + .fixImportsOnly(false) + .dryRun(false) + .setExitIfChanged(false) + .inPlace(false); } - static class Builder { + @AutoBuilder + interface Builder { - private final ImmutableList.Builder files = ImmutableList.builder(); - private final RangeSet lines = TreeRangeSet.create(); - private final ImmutableList.Builder offsets = ImmutableList.builder(); - private final ImmutableList.Builder lengths = ImmutableList.builder(); - private boolean inPlace = false; - private boolean aosp = false; - private boolean version = false; - private boolean help = false; - private boolean stdin = false; - private boolean fixImportsOnly = false; - private boolean sortImports = true; - private boolean removeUnusedImports = true; - private boolean dryRun = false; - private boolean setExitIfChanged = false; - private Optional assumeFilename = Optional.empty(); - private boolean reflowLongStrings = true; - private boolean formatJavadoc = true; + ImmutableList.Builder filesBuilder(); - ImmutableList.Builder filesBuilder() { - return files; - } + Builder inPlace(boolean inPlace); - Builder inPlace(boolean inPlace) { - this.inPlace = inPlace; - return this; - } + Builder lines(ImmutableRangeSet lines); - RangeSet linesBuilder() { - return lines; - } + ImmutableList.Builder offsetsBuilder(); - Builder addOffset(Integer offset) { - offsets.add(offset); + @CanIgnoreReturnValue + default Builder addOffset(Integer offset) { + offsetsBuilder().add(offset); return this; } - Builder addLength(Integer length) { - lengths.add(length); - return this; - } + ImmutableList.Builder lengthsBuilder(); - Builder aosp(boolean aosp) { - this.aosp = aosp; + @CanIgnoreReturnValue + default Builder addLength(Integer length) { + lengthsBuilder().add(length); return this; } - Builder version(boolean version) { - this.version = version; - return this; - } + Builder aosp(boolean aosp); - Builder help(boolean help) { - this.help = help; - return this; - } + Builder version(boolean version); - Builder stdin(boolean stdin) { - this.stdin = stdin; - return this; - } + Builder help(boolean help); - Builder fixImportsOnly(boolean fixImportsOnly) { - this.fixImportsOnly = fixImportsOnly; - return this; - } + Builder stdin(boolean stdin); - Builder sortImports(boolean sortImports) { - this.sortImports = sortImports; - return this; - } + Builder fixImportsOnly(boolean fixImportsOnly); - Builder removeUnusedImports(boolean removeUnusedImports) { - this.removeUnusedImports = removeUnusedImports; - return this; - } + Builder sortImports(boolean sortImports); - Builder dryRun(boolean dryRun) { - this.dryRun = dryRun; - return this; - } + Builder removeUnusedImports(boolean removeUnusedImports); - Builder setExitIfChanged(boolean setExitIfChanged) { - this.setExitIfChanged = setExitIfChanged; - return this; - } + Builder dryRun(boolean dryRun); - Builder assumeFilename(String assumeFilename) { - this.assumeFilename = Optional.of(assumeFilename); - return this; - } + Builder setExitIfChanged(boolean setExitIfChanged); - Builder reflowLongStrings(boolean reflowLongStrings) { - this.reflowLongStrings = reflowLongStrings; - return this; - } + Builder assumeFilename(String assumeFilename); - Builder formatJavadoc(boolean formatJavadoc) { - this.formatJavadoc = formatJavadoc; - return this; - } + Builder reflowLongStrings(boolean reflowLongStrings); - CommandLineOptions build() { - return new CommandLineOptions( - files.build(), - inPlace, - ImmutableRangeSet.copyOf(lines), - offsets.build(), - lengths.build(), - aosp, - version, - help, - stdin, - fixImportsOnly, - sortImports, - removeUnusedImports, - dryRun, - setExitIfChanged, - assumeFilename, - reflowLongStrings, - formatJavadoc); - } + Builder formatJavadoc(boolean formatJavadoc); + + CommandLineOptions build(); } } diff --git a/core/src/main/java/com/google/googlejavaformat/java/CommandLineOptionsParser.java b/core/src/main/java/com/google/googlejavaformat/java/CommandLineOptionsParser.java index 52a5e05d4..098a263f4 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/CommandLineOptionsParser.java +++ b/core/src/main/java/com/google/googlejavaformat/java/CommandLineOptionsParser.java @@ -18,8 +18,10 @@ import com.google.common.base.CharMatcher; import com.google.common.base.Splitter; +import com.google.common.collect.ImmutableRangeSet; import com.google.common.collect.Range; import com.google.common.collect.RangeSet; +import com.google.common.collect.TreeRangeSet; import java.io.IOException; import java.io.UncheckedIOException; import java.nio.file.Files; @@ -43,6 +45,9 @@ static CommandLineOptions parse(Iterable options) { List expandedOptions = new ArrayList<>(); expandParamsFiles(options, expandedOptions); Iterator it = expandedOptions.iterator(); + // Accumulate the ranges in a mutable builder to merge overlapping ranges, + // which ImmutableRangeSet doesn't support. + RangeSet linesBuilder = TreeRangeSet.create(); while (it.hasNext()) { String option = it.next(); if (!option.startsWith("-")) { @@ -71,7 +76,7 @@ static CommandLineOptions parse(Iterable options) { case "-lines": case "--line": case "-line": - parseRangeSet(optionsBuilder.linesBuilder(), getValue(flag, it, value)); + parseRangeSet(linesBuilder, getValue(flag, it, value)); break; case "--offset": case "-offset": @@ -129,6 +134,7 @@ static CommandLineOptions parse(Iterable options) { throw new IllegalArgumentException("unexpected flag: " + flag); } } + optionsBuilder.lines(ImmutableRangeSet.copyOf(linesBuilder)); return optionsBuilder.build(); }