Skip to content

Commit c4b467d

Browse files
java-team-github-botgoogle-java-format Team
authored and
google-java-format Team
committed
Support overlapping --lines ranges in google-java-format.
The strict behavior comes from the underlying ImmutableRangeSet.Builder class, which does not allow overlapping ranges to be added. Let's use TreeRangeSet instead. I have also updated the documentation for the --lines flag to make it clear that the line numbers are a closed-closed interval. Tested with: ``` blaze run :google_java_format_binary -- --lines=1:5 --lines=3:8 - < Foo.java ``` Before this CL the command fails with: ``` '--lines=1:5' '--lines=3:8' - Overlapping ranges not permitted but found [0..5) overlapping [2..8) ``` After this CL the command succeeds. PiperOrigin-RevId: 622410610
1 parent 33bf757 commit c4b467d

File tree

4 files changed

+14
-15
lines changed

4 files changed

+14
-15
lines changed

core/src/main/java/com/google/googlejavaformat/java/CommandLineOptions.java

+5-3
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
import com.google.common.collect.ImmutableList;
1818
import com.google.common.collect.ImmutableRangeSet;
19+
import com.google.common.collect.RangeSet;
20+
import com.google.common.collect.TreeRangeSet;
1921
import java.util.Optional;
2022

2123
/**
@@ -178,7 +180,7 @@ static Builder builder() {
178180
static class Builder {
179181

180182
private final ImmutableList.Builder<String> files = ImmutableList.builder();
181-
private final ImmutableRangeSet.Builder<Integer> lines = ImmutableRangeSet.builder();
183+
private final RangeSet<Integer> lines = TreeRangeSet.create();
182184
private final ImmutableList.Builder<Integer> offsets = ImmutableList.builder();
183185
private final ImmutableList.Builder<Integer> lengths = ImmutableList.builder();
184186
private boolean inPlace = false;
@@ -204,7 +206,7 @@ Builder inPlace(boolean inPlace) {
204206
return this;
205207
}
206208

207-
ImmutableRangeSet.Builder<Integer> linesBuilder() {
209+
RangeSet<Integer> linesBuilder() {
208210
return lines;
209211
}
210212

@@ -282,7 +284,7 @@ CommandLineOptions build() {
282284
return new CommandLineOptions(
283285
files.build(),
284286
inPlace,
285-
lines.build(),
287+
ImmutableRangeSet.copyOf(lines),
286288
offsets.build(),
287289
lengths.build(),
288290
aosp,

core/src/main/java/com/google/googlejavaformat/java/CommandLineOptionsParser.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@
1818

1919
import com.google.common.base.CharMatcher;
2020
import com.google.common.base.Splitter;
21-
import com.google.common.collect.ImmutableRangeSet;
2221
import com.google.common.collect.Range;
22+
import com.google.common.collect.RangeSet;
2323
import java.io.IOException;
2424
import java.io.UncheckedIOException;
2525
import java.nio.file.Files;
@@ -157,7 +157,7 @@ private static String getValue(String flag, Iterator<String> it, String value) {
157157
* number. Line numbers are {@code 1}-based, but are converted to the {@code 0}-based numbering
158158
* used internally by google-java-format.
159159
*/
160-
private static void parseRangeSet(ImmutableRangeSet.Builder<Integer> result, String ranges) {
160+
private static void parseRangeSet(RangeSet<Integer> result, String ranges) {
161161
for (String range : COMMA_SPLITTER.split(ranges)) {
162162
result.add(parseRange(range));
163163
}

core/src/main/java/com/google/googlejavaformat/java/UsageException.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ final class UsageException extends Exception {
5656
" --set-exit-if-changed",
5757
" Return exit code 1 if there are any formatting changes.",
5858
" --lines, -lines, --line, -line",
59-
" Line range(s) to format, like 5:10 (1-based; default is all).",
59+
" Line range(s) to format, e.g. the first 5 lines are 1:5 (1-based; default is all).",
6060
" --offset, -offset",
6161
" Character offset to format (0-based; default is all).",
6262
" --length, -length",

core/src/test/java/com/google/googlejavaformat/java/CommandLineOptionsParserTest.java

+6-9
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
import static com.google.common.truth.Truth.assertThat;
1818
import static java.nio.charset.StandardCharsets.UTF_8;
19-
import static org.junit.Assert.fail;
2019

2120
import com.google.common.collect.ImmutableList;
2221
import com.google.common.collect.Range;
@@ -151,15 +150,13 @@ public void setExitIfChanged() {
151150
.isTrue();
152151
}
153152

154-
// TODO(cushon): consider handling this in the parser and reporting a more detailed error
155153
@Test
156-
public void illegalLines() {
157-
try {
158-
CommandLineOptionsParser.parse(Arrays.asList("-lines=1:1", "-lines=1:1"));
159-
fail();
160-
} catch (IllegalArgumentException e) {
161-
assertThat(e).hasMessageThat().contains("overlap");
162-
}
154+
public void mergedLines() {
155+
assertThat(
156+
CommandLineOptionsParser.parse(Arrays.asList("-lines=1:5", "-lines=2:8"))
157+
.lines()
158+
.asRanges())
159+
.containsExactly(Range.closedOpen(0, 8));
163160
}
164161

165162
@Test

0 commit comments

Comments
 (0)