Skip to content

Commit c753e2c

Browse files
awturnercpovirk
authored andcommitted
google-java-format: construct the Replacement range in the constructor, in order to guarantee it is closedOpen.
Remove the Replacement.create(Range,String) overload, since the bounds were not checked for correctness (open/closed, lower bound >= 0). ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=262950211
1 parent 6f86d4f commit c753e2c

File tree

3 files changed

+23
-36
lines changed

3 files changed

+23
-36
lines changed

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

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@
1414

1515
package com.google.googlejavaformat.java;
1616

17+
import static com.google.common.base.Preconditions.checkArgument;
18+
import static com.google.common.base.Preconditions.checkNotNull;
19+
1720
import com.google.common.collect.Range;
1821
import java.util.Objects;
1922

@@ -23,28 +26,20 @@
2326
* <p>google-java-format doesn't depend on AutoValue, to allow AutoValue to depend on
2427
* google-java-format.
2528
*/
26-
public class Replacement {
29+
public final class Replacement {
2730

2831
public static Replacement create(int startPosition, int endPosition, String replaceWith) {
32+
checkArgument(startPosition >= 0, "startPosition must be non-negative");
33+
checkArgument(startPosition <= endPosition, "startPosition cannot be after endPosition");
2934
return new Replacement(Range.closedOpen(startPosition, endPosition), replaceWith);
3035
}
3136

32-
public static Replacement create(Range<Integer> range, String replaceWith) {
33-
return new Replacement(range, replaceWith);
34-
}
35-
3637
private final Range<Integer> replaceRange;
3738
private final String replacementString;
3839

39-
Replacement(Range<Integer> replaceRange, String replacementString) {
40-
if (replaceRange == null) {
41-
throw new NullPointerException("Null replaceRange");
42-
}
43-
this.replaceRange = replaceRange;
44-
if (replacementString == null) {
45-
throw new NullPointerException("Null replacementString");
46-
}
47-
this.replacementString = replacementString;
40+
private Replacement(Range<Integer> replaceRange, String replacementString) {
41+
this.replaceRange = checkNotNull(replaceRange, "Null replaceRange");
42+
this.replacementString = checkNotNull(replacementString, "Null replacementString");
4843
}
4944

5045
/** The range of characters in the original source to replace. */

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

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,12 @@
1414

1515
package com.google.googlejavaformat.java;
1616

17+
import static com.google.common.collect.ImmutableList.toImmutableList;
18+
1719
import com.google.common.base.CharMatcher;
1820
import com.google.common.base.Preconditions;
1921
import com.google.common.collect.DiscreteDomain;
22+
import com.google.common.collect.ImmutableList;
2023
import com.google.common.collect.Range;
2124
import com.google.common.collect.RangeSet;
2225
import com.google.common.collect.TreeRangeSet;
@@ -87,7 +90,7 @@ private static List<Range<Integer>> offsetRanges(List<Range<Integer>> ranges, in
8790
}
8891

8992
/** Runs the Google Java formatter on the given source, with only the given ranges specified. */
90-
public List<Replacement> format(
93+
public ImmutableList<Replacement> format(
9194
SnippetKind kind,
9295
String source,
9396
List<Range<Integer>> ranges,
@@ -114,14 +117,9 @@ public List<Replacement> format(
114117
wrapper.offset,
115118
replacement.length() - (wrapper.contents.length() - wrapper.offset - source.length()));
116119

117-
List<Replacement> replacements = toReplacements(source, replacement);
118-
List<Replacement> filtered = new ArrayList<>();
119-
for (Replacement r : replacements) {
120-
if (rangeSet.encloses(r.getReplaceRange())) {
121-
filtered.add(r);
122-
}
123-
}
124-
return filtered;
120+
return toReplacements(source, replacement).stream()
121+
.filter(r -> rangeSet.encloses(r.getReplaceRange()))
122+
.collect(toImmutableList());
125123
}
126124

127125
/**
@@ -142,7 +140,7 @@ private static List<Replacement> toReplacements(String source, String replacemen
142140
int i = NOT_WHITESPACE.indexIn(source);
143141
int j = NOT_WHITESPACE.indexIn(replacement);
144142
if (i != 0 || j != 0) {
145-
replacements.add(Replacement.create(Range.closedOpen(0, i), replacement.substring(0, j)));
143+
replacements.add(Replacement.create(0, i, replacement.substring(0, j)));
146144
}
147145
while (i != -1 && j != -1) {
148146
int i2 = NOT_WHITESPACE.indexIn(source, i + 1);
@@ -152,8 +150,7 @@ private static List<Replacement> toReplacements(String source, String replacemen
152150
}
153151
if ((i2 - i) != (j2 - j)
154152
|| !source.substring(i + 1, i2).equals(replacement.substring(j + 1, j2))) {
155-
replacements.add(
156-
Replacement.create(Range.closedOpen(i + 1, i2), replacement.substring(j + 1, j2)));
153+
replacements.add(Replacement.create(i + 1, i2, replacement.substring(j + 1, j2)));
157154
}
158155
i = i2;
159156
j = j2;

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

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,7 @@ public void expression() throws FormatterException {
3939
4,
4040
false);
4141
assertThat(replacements)
42-
.containsExactly(
43-
Replacement.create(Range.closedOpen(1, 2), " "),
44-
Replacement.create(Range.closedOpen(3, 3), " "));
42+
.containsExactly(Replacement.create(1, 2, " "), Replacement.create(3, 3, " "));
4543
}
4644

4745
@Test
@@ -56,9 +54,7 @@ public void statement() throws FormatterException {
5654
4,
5755
false);
5856
assertThat(replacements)
59-
.containsExactly(
60-
Replacement.create(Range.closedOpen(5, 6), " "),
61-
Replacement.create(Range.closedOpen(7, 7), " "));
57+
.containsExactly(Replacement.create(5, 6, " "), Replacement.create(7, 7, " "));
6258
}
6359

6460
@Test
@@ -72,7 +68,7 @@ public void classMember() throws FormatterException {
7268
ImmutableList.of(Range.closedOpen(0, input.length())),
7369
4,
7470
false);
75-
assertThat(replacements).containsExactly(Replacement.create(Range.closedOpen(10, 11), ""));
71+
assertThat(replacements).containsExactly(Replacement.create(10, 11, ""));
7672
}
7773

7874
@Test
@@ -86,7 +82,7 @@ public void compilation() throws FormatterException {
8682
ImmutableList.of(Range.closedOpen(input.indexOf("class"), input.length())),
8783
4,
8884
false);
89-
assertThat(replacements).containsExactly(Replacement.create(Range.closedOpen(22, 23), ""));
85+
assertThat(replacements).containsExactly(Replacement.create(22, 23, ""));
9086
}
9187

9288
@Test
@@ -101,7 +97,6 @@ public void compilationWithComments() throws FormatterException {
10197
4,
10298
true);
10399
assertThat(replacements)
104-
.containsExactly(
105-
Replacement.create(Range.closedOpen(0, 24), "/** a b */\nclass Test {}\n"));
100+
.containsExactly(Replacement.create(0, 24, "/** a b */\nclass Test {}\n"));
106101
}
107102
}

0 commit comments

Comments
 (0)