From a5c90107f3f3570d34a336530dce63c50111ee08 Mon Sep 17 00:00:00 2001 From: google-java-format Team Date: Thu, 24 Sep 2026 23:06:13 +0300 Subject: [PATCH] Merge --lines ranges that overlap instead of rejecting them "--lines=1:5 --lines=3:8" failed with "Overlapping ranges not permitted but found [0..5) overlapping [2..8)" and printed the usage text. The ranges were collected in an ImmutableRangeSet.Builder, which refuses overlapping and even adjacent ranges, so a caller that computes ranges from a diff had to merge them itself, and the same line given twice was an error. This ports google/google-java-format#1093: the ranges are collected in a TreeRangeSet, which merges them, and the usage text says that 1:5 means the first five lines. The mergedLines test comes from upstream; repeatedLines replaces the test that expected the rejection. --- .../javaformat/java/CommandLineOptions.java | 9 +++++--- .../java/CommandLineOptionsParser.java | 3 ++- .../javaformat/java/UsageException.java | 2 +- .../java/CommandLineOptionsParserTest.java | 22 +++++++++++-------- 4 files changed, 22 insertions(+), 14 deletions(-) diff --git a/open-java-format/src/main/java/com/palantir/javaformat/java/CommandLineOptions.java b/open-java-format/src/main/java/com/palantir/javaformat/java/CommandLineOptions.java index c4602e778..f36722b94 100644 --- a/open-java-format/src/main/java/com/palantir/javaformat/java/CommandLineOptions.java +++ b/open-java-format/src/main/java/com/palantir/javaformat/java/CommandLineOptions.java @@ -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; /** @@ -183,7 +185,8 @@ static Builder builder() { static final class Builder { private final ImmutableList.Builder files = ImmutableList.builder(); - private final ImmutableRangeSet.Builder lines = ImmutableRangeSet.builder(); + // A TreeRangeSet merges ranges that touch or overlap, which ImmutableRangeSet.Builder rejects + private final RangeSet lines = TreeRangeSet.create(); private final ImmutableRangeSet.Builder characterRanges = ImmutableRangeSet.builder(); private final ImmutableList.Builder offsets = ImmutableList.builder(); private final ImmutableList.Builder lengths = ImmutableList.builder(); @@ -212,7 +215,7 @@ Builder inPlace(boolean inPlace) { return this; } - ImmutableRangeSet.Builder linesBuilder() { + RangeSet linesBuilder() { return lines; } @@ -294,7 +297,7 @@ CommandLineOptions build() { return new CommandLineOptions( files.build(), inPlace, - lines.build(), + ImmutableRangeSet.copyOf(lines), characterRanges.build(), offsets.build(), lengths.build(), diff --git a/open-java-format/src/main/java/com/palantir/javaformat/java/CommandLineOptionsParser.java b/open-java-format/src/main/java/com/palantir/javaformat/java/CommandLineOptionsParser.java index 638ed34b1..6e4dac004 100644 --- a/open-java-format/src/main/java/com/palantir/javaformat/java/CommandLineOptionsParser.java +++ b/open-java-format/src/main/java/com/palantir/javaformat/java/CommandLineOptionsParser.java @@ -18,6 +18,7 @@ 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; @@ -212,7 +213,7 @@ private static Range parseCharacterRange(String range) { * --lines flags or separated by commas. A single line can be set by a single 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 result, String ranges) { + private static void parseRangeSet(RangeSet result, String ranges) { for (String range : COMMA_SPLITTER.split(ranges)) { result.add(parseRange(range)); } diff --git a/open-java-format/src/main/java/com/palantir/javaformat/java/UsageException.java b/open-java-format/src/main/java/com/palantir/javaformat/java/UsageException.java index f20b27e0c..eca7a781e 100644 --- a/open-java-format/src/main/java/com/palantir/javaformat/java/UsageException.java +++ b/open-java-format/src/main/java/com/palantir/javaformat/java/UsageException.java @@ -52,7 +52,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).", " --character-ranges, -character-ranges, --character-range, -character-range", " character range(s) to format, like 5:10 (0-based; default is all).", " --offset, -offset", diff --git a/open-java-format/src/test/java/com/palantir/javaformat/java/CommandLineOptionsParserTest.java b/open-java-format/src/test/java/com/palantir/javaformat/java/CommandLineOptionsParserTest.java index ced15a57c..84953bbeb 100644 --- a/open-java-format/src/test/java/com/palantir/javaformat/java/CommandLineOptionsParserTest.java +++ b/open-java-format/src/test/java/com/palantir/javaformat/java/CommandLineOptionsParserTest.java @@ -17,7 +17,6 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static org.assertj.core.api.Assertions.fail; import com.google.common.collect.Range; import java.io.IOException; @@ -150,15 +149,20 @@ 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("fail"); - } catch (IllegalArgumentException e) { - assertThat(e.getMessage()).contains("overlap"); - } + public void mergedLines() { + assertThat(CommandLineOptionsParser.parse(Arrays.asList("-lines=1:5", "-lines=2:8")) + .lines() + .asRanges()) + .containsExactly(Range.closedOpen(0, 8)); + } + + @Test + public void repeatedLines() { + assertThat(CommandLineOptionsParser.parse(Arrays.asList("-lines=1:1", "-lines=1:1")) + .lines() + .asRanges()) + .containsExactly(Range.closedOpen(0, 1)); } @Test