Skip to content

Commit

Permalink
Support overlapping --lines ranges in google-java-format.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
java-team-github-bot authored and google-java-format Team committed Apr 6, 2024
1 parent 33bf757 commit c4b467d
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

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 java.util.Optional;

/**
Expand Down Expand Up @@ -178,7 +180,7 @@ static Builder builder() {
static class Builder {

private final ImmutableList.Builder<String> files = ImmutableList.builder();
private final ImmutableRangeSet.Builder<Integer> lines = ImmutableRangeSet.builder();
private final RangeSet<Integer> lines = TreeRangeSet.create();
private final ImmutableList.Builder<Integer> offsets = ImmutableList.builder();
private final ImmutableList.Builder<Integer> lengths = ImmutableList.builder();
private boolean inPlace = false;
Expand All @@ -204,7 +206,7 @@ Builder inPlace(boolean inPlace) {
return this;
}

ImmutableRangeSet.Builder<Integer> linesBuilder() {
RangeSet<Integer> linesBuilder() {
return lines;
}

Expand Down Expand Up @@ -282,7 +284,7 @@ CommandLineOptions build() {
return new CommandLineOptions(
files.build(),
inPlace,
lines.build(),
ImmutableRangeSet.copyOf(lines),
offsets.build(),
lengths.build(),
aosp,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@

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 java.io.IOException;
import java.io.UncheckedIOException;
import java.nio.file.Files;
Expand Down Expand Up @@ -157,7 +157,7 @@ private static String getValue(String flag, Iterator<String> it, String value) {
* number. Line numbers are {@code 1}-based, but are converted to the {@code 0}-based numbering
* used internally by google-java-format.
*/
private static void parseRangeSet(ImmutableRangeSet.Builder<Integer> result, String ranges) {
private static void parseRangeSet(RangeSet<Integer> result, String ranges) {
for (String range : COMMA_SPLITTER.split(ranges)) {
result.add(parseRange(range));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ final class UsageException extends Exception {
" --set-exit-if-changed",
" Return exit code 1 if there are any formatting changes.",
" --lines, -lines, --line, -line",
" Line range(s) to format, like 5:10 (1-based; default is all).",
" Line range(s) to format, e.g. the first 5 lines are 1:5 (1-based; default is all).",
" --offset, -offset",
" Character offset to format (0-based; default is all).",
" --length, -length",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

import static com.google.common.truth.Truth.assertThat;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.junit.Assert.fail;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.Range;
Expand Down Expand Up @@ -151,15 +150,13 @@ public void setExitIfChanged() {
.isTrue();
}

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

@Test
Expand Down

0 comments on commit c4b467d

Please sign in to comment.