Skip to content

Commit

Permalink
google-java-format: construct the Replacement range in the constructo…
Browse files Browse the repository at this point in the history
…r, 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
  • Loading branch information
awturner authored and cpovirk committed Aug 19, 2019
1 parent 6f86d4f commit c753e2c
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 36 deletions.
Expand Up @@ -14,6 +14,9 @@

package com.google.googlejavaformat.java;

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;

import com.google.common.collect.Range;
import java.util.Objects;

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

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

public static Replacement create(Range<Integer> range, String replaceWith) {
return new Replacement(range, replaceWith);
}

private final Range<Integer> replaceRange;
private final String replacementString;

Replacement(Range<Integer> replaceRange, String replacementString) {
if (replaceRange == null) {
throw new NullPointerException("Null replaceRange");
}
this.replaceRange = replaceRange;
if (replacementString == null) {
throw new NullPointerException("Null replacementString");
}
this.replacementString = replacementString;
private Replacement(Range<Integer> replaceRange, String replacementString) {
this.replaceRange = checkNotNull(replaceRange, "Null replaceRange");
this.replacementString = checkNotNull(replacementString, "Null replacementString");
}

/** The range of characters in the original source to replace. */
Expand Down
Expand Up @@ -14,9 +14,12 @@

package com.google.googlejavaformat.java;

import static com.google.common.collect.ImmutableList.toImmutableList;

import com.google.common.base.CharMatcher;
import com.google.common.base.Preconditions;
import com.google.common.collect.DiscreteDomain;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Range;
import com.google.common.collect.RangeSet;
import com.google.common.collect.TreeRangeSet;
Expand Down Expand Up @@ -87,7 +90,7 @@ private static List<Range<Integer>> offsetRanges(List<Range<Integer>> ranges, in
}

/** Runs the Google Java formatter on the given source, with only the given ranges specified. */
public List<Replacement> format(
public ImmutableList<Replacement> format(
SnippetKind kind,
String source,
List<Range<Integer>> ranges,
Expand All @@ -114,14 +117,9 @@ public List<Replacement> format(
wrapper.offset,
replacement.length() - (wrapper.contents.length() - wrapper.offset - source.length()));

List<Replacement> replacements = toReplacements(source, replacement);
List<Replacement> filtered = new ArrayList<>();
for (Replacement r : replacements) {
if (rangeSet.encloses(r.getReplaceRange())) {
filtered.add(r);
}
}
return filtered;
return toReplacements(source, replacement).stream()
.filter(r -> rangeSet.encloses(r.getReplaceRange()))
.collect(toImmutableList());
}

/**
Expand All @@ -142,7 +140,7 @@ private static List<Replacement> toReplacements(String source, String replacemen
int i = NOT_WHITESPACE.indexIn(source);
int j = NOT_WHITESPACE.indexIn(replacement);
if (i != 0 || j != 0) {
replacements.add(Replacement.create(Range.closedOpen(0, i), replacement.substring(0, j)));
replacements.add(Replacement.create(0, i, replacement.substring(0, j)));
}
while (i != -1 && j != -1) {
int i2 = NOT_WHITESPACE.indexIn(source, i + 1);
Expand All @@ -152,8 +150,7 @@ private static List<Replacement> toReplacements(String source, String replacemen
}
if ((i2 - i) != (j2 - j)
|| !source.substring(i + 1, i2).equals(replacement.substring(j + 1, j2))) {
replacements.add(
Replacement.create(Range.closedOpen(i + 1, i2), replacement.substring(j + 1, j2)));
replacements.add(Replacement.create(i + 1, i2, replacement.substring(j + 1, j2)));
}
i = i2;
j = j2;
Expand Down
Expand Up @@ -39,9 +39,7 @@ public void expression() throws FormatterException {
4,
false);
assertThat(replacements)
.containsExactly(
Replacement.create(Range.closedOpen(1, 2), " "),
Replacement.create(Range.closedOpen(3, 3), " "));
.containsExactly(Replacement.create(1, 2, " "), Replacement.create(3, 3, " "));
}

@Test
Expand All @@ -56,9 +54,7 @@ public void statement() throws FormatterException {
4,
false);
assertThat(replacements)
.containsExactly(
Replacement.create(Range.closedOpen(5, 6), " "),
Replacement.create(Range.closedOpen(7, 7), " "));
.containsExactly(Replacement.create(5, 6, " "), Replacement.create(7, 7, " "));
}

@Test
Expand All @@ -72,7 +68,7 @@ public void classMember() throws FormatterException {
ImmutableList.of(Range.closedOpen(0, input.length())),
4,
false);
assertThat(replacements).containsExactly(Replacement.create(Range.closedOpen(10, 11), ""));
assertThat(replacements).containsExactly(Replacement.create(10, 11, ""));
}

@Test
Expand All @@ -86,7 +82,7 @@ public void compilation() throws FormatterException {
ImmutableList.of(Range.closedOpen(input.indexOf("class"), input.length())),
4,
false);
assertThat(replacements).containsExactly(Replacement.create(Range.closedOpen(22, 23), ""));
assertThat(replacements).containsExactly(Replacement.create(22, 23, ""));
}

@Test
Expand All @@ -101,7 +97,6 @@ public void compilationWithComments() throws FormatterException {
4,
true);
assertThat(replacements)
.containsExactly(
Replacement.create(Range.closedOpen(0, 24), "/** a b */\nclass Test {}\n"));
.containsExactly(Replacement.create(0, 24, "/** a b */\nclass Test {}\n"));
}
}

0 comments on commit c753e2c

Please sign in to comment.