Add more info into Comment.Range comments
Add more info into Comment.Range comments and add more corner cases to
FixReplacementInterpreterTest
Change-Id: I8c87a2d6bf92cbad2db6147dcc689294688c60a0
diff --git a/java/com/google/gerrit/reviewdb/client/Comment.java b/java/com/google/gerrit/reviewdb/client/Comment.java
index 94e7583..eb2aa52 100644
--- a/java/com/google/gerrit/reviewdb/client/Comment.java
+++ b/java/com/google/gerrit/reviewdb/client/Comment.java
@@ -124,6 +124,24 @@
}
}
+ /**
+ * The Range class defines continuous range of character.
+ *
+ * <p>The pair (startLine, startChar) defines the first character in the range. The pair (endLine,
+ * endChar) defines the first character AFTER the range (i.e. it doesn't belong the range).
+ * (endLine, endChar) must be a valid character inside text, except EOF case.</p>
+ * <p>Special cases:</p>
+ * <ul>
+ * <li>Zero length range: (startLine, startChar) = (endLine, endChar). Range defines insert
+ * position right before the (startLine, startChar) character (for {@link FixReplacement)
+ * <li>EOF case - range includes the last character in the file:
+ * <ul>
+ * <li>if a file ends with EOL mark, then (endLine, endChar) = (num_of_lines + 1, 0)
+ * <li>if a file doesn't end with EOL mark, then
+ * (endLine, endChar) = (num_of_lines, num_of_chars_in_last_line)
+ * </ul>
+ * </ul>
+ */
public static class Range implements Comparable<Range> {
private static final Comparator<Range> RANGE_COMPARATOR =
Comparator.<Range>comparingInt(range -> range.startLine)
@@ -131,10 +149,10 @@
.thenComparingInt(range -> range.endLine)
.thenComparingInt(range -> range.endChar);
- public int startLine; // 1-based, inclusive
- public int startChar; // 0-based, inclusive
- public int endLine; // 1-based, exclusive
- public int endChar; // 0-based, exclusive
+ public int startLine; // 1-based
+ public int startChar; // 0-based
+ public int endLine; // 1-based
+ public int endChar; // 0-based
public Range(Range r) {
this(r.startLine, r.startChar, r.endLine, r.endChar);
diff --git a/javatests/com/google/gerrit/server/fixes/FixReplacementInterpreterTest.java b/javatests/com/google/gerrit/server/fixes/FixReplacementInterpreterTest.java
index 2174927..fb49657 100644
--- a/javatests/com/google/gerrit/server/fixes/FixReplacementInterpreterTest.java
+++ b/javatests/com/google/gerrit/server/fixes/FixReplacementInterpreterTest.java
@@ -148,6 +148,22 @@
.isEqualTo("First line\nA modification\nThird line\n");
}
+ @Test()
+ public void startAfterEndOfLineMarkThrowsAnException() throws Exception {
+ FixReplacement fixReplacement =
+ new FixReplacement(filePath1, new Range(1, 11, 2, 6), "A modification");
+ mockFileContent(filePath1, "First line\nSecond line\nThird line\n");
+ assertThrows(ResourceConflictException.class, () -> toTreeModifications(fixReplacement));
+ }
+
+ @Test()
+ public void endAfterEndOfLineMarkThrowsAnException() throws Exception {
+ FixReplacement fixReplacement =
+ new FixReplacement(filePath1, new Range(2, 0, 2, 12), "A modification");
+ mockFileContent(filePath1, "First line\nSecond line\nThird line\n");
+ assertThrows(ResourceConflictException.class, () -> toTreeModifications(fixReplacement));
+ }
+
@Test
public void replacementsMayTouch() throws Exception {
FixReplacement fixReplacement1 =
@@ -180,6 +196,34 @@
}
@Test
+ public void replacementsCanChangeLastLine() throws Exception {
+ FixReplacement fixReplacement =
+ new FixReplacement(filePath1, new Range(3, 0, 4, 0), "New content\n");
+ mockFileContent(filePath1, "First line\nSecond line\nThird line\n");
+
+ List<TreeModification> treeModifications = toTreeModifications(fixReplacement);
+ assertThatList(treeModifications)
+ .onlyElement()
+ .asChangeFileContentModification()
+ .newContent()
+ .isEqualTo("First line\nSecond line\nNew content\n");
+ }
+
+ @Test
+ public void replacementsCanChangeLastLineWithoutEOLMark() throws Exception {
+ FixReplacement fixReplacement =
+ new FixReplacement(filePath1, new Range(3, 0, 3, 10), "New content\n");
+ mockFileContent(filePath1, "First line\nSecond line\nThird line");
+
+ List<TreeModification> treeModifications = toTreeModifications(fixReplacement);
+ assertThatList(treeModifications)
+ .onlyElement()
+ .asChangeFileContentModification()
+ .newContent()
+ .isEqualTo("First line\nSecond line\nNew content\n");
+ }
+
+ @Test
public void replacementsCanModifySeveralFilesInAnyOrder() throws Exception {
FixReplacement fixReplacement1 =
new FixReplacement(filePath1, new Range(1, 1, 3, 2), "Modified content");
