Merge pull request #71 from openjavaformat/overlapping-lines-ranges

Merge --lines ranges that overlap instead of rejecting them

Author: abashev

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<String> files = ImmutableList.builder();
-        private final ImmutableRangeSet.Builder<Integer> lines = ImmutableRangeSet.builder();
+        // A TreeRangeSet merges ranges that touch or overlap, which ImmutableRangeSet.Builder rejects
+        private final RangeSet<Integer> lines = TreeRangeSet.create();
         private final ImmutableRangeSet.Builder<Integer> characterRanges = ImmutableRangeSet.builder();
         private final ImmutableList.Builder<Integer> offsets = ImmutableList.builder();
         private final ImmutableList.Builder<Integer> lengths = ImmutableList.builder();
@@ -212,7 +215,7 @@ Builder inPlace(boolean inPlace) {
             return this;
         }
 
-        ImmutableRangeSet.Builder<Integer> linesBuilder() {
+        RangeSet<Integer> 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(),

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<Integer> 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<Integer> result, String ranges) {
+    private static void parseRangeSet(RangeSet<Integer> result, String ranges) {
         for (String range : COMMA_SPLITTER.split(ranges)) {
             result.add(parseRange(range));
         }

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",

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