Reject (robot) comments with ranges referring to line 0

Line numbers are 1-based except when they are used to indicate a file
comment. To highlight that behavior, comments are added to the REST
documentation as well as to the code. Those comments additionally
indicate whether the values are inclusive or exclusive.

Change-Id: I4b4409ac25acd2124b3e74cb2ee075206ebae0eb

Documentation/rest-api-changes.txt

@@ -5386,10 +5386,10 @@ The `CommentRange` entity describes the range of an inline comment.
 [options="header",cols="1,^1,5"]
 |===========================
 |Field Name    ||Description
-|`start_line`        ||The start line number of the range.
+|`start_line`        ||The start line number of the range. (1-based, inclusive)
-|`start_character`   ||The character position in the start line.
+|`start_character`   ||The character position in the start line. (0-based, inclusive)
-|`end_line`          ||The end line number of the range.
+|`end_line`          ||The end line number of the range. (1-based, exclusive)
-|`end_character`     ||The character position in the end line.
+|`end_character`     ||The character position in the end line. (0-based, exclusive)
 |===========================
 
 [[commit-info]]

gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/Comment.java

@@ -29,7 +29,7 @@ public abstract class Comment {
   public String path;
   public Side side;
   public Integer parent;
-  public Integer line;
+  public Integer line; // value 0 or null indicates a file comment, normal lines start at 1
   public Range range;
   public String inReplyTo;
   public Timestamp updated;
@@ -37,15 +37,15 @@ public abstract class Comment {
   public Boolean unresolved;
 
   public static class Range {
-    public int startLine;
+    public int startLine; // 1-based, inclusive
-    public int startCharacter;
+    public int startCharacter; // 0-based, inclusive
-    public int endLine;
+    public int endLine; // 1-based, exclusive
-    public int endCharacter;
+    public int endCharacter; // 0-based, exclusive
 
     public boolean isValid() {
-      return startLine >= 0
+      return startLine > 0
           && startCharacter >= 0
-          && endLine >= 0
+          && endLine > 0
           && endCharacter >= 0
           && startLine <= endLine
           && (startLine != endLine || startCharacter <= endCharacter);

gerrit-extension-api/src/test/java/com/google/gerrit/extensions/client/RangeTest.java

@@ -62,6 +62,30 @@ public class RangeTest {
     assertThat(range).isInvalid();
   }
 
+  @Test
+  public void zeroStartLineResultsInInvalidRange() {
+    Comment.Range range = createRange(0, 2, 19, 10);
+    assertThat(range).isInvalid();
+  }
+
+  @Test
+  public void zeroEndLineResultsInInvalidRange() {
+    Comment.Range range = createRange(13, 2, 0, 10);
+    assertThat(range).isInvalid();
+  }
+
+  @Test
+  public void zeroStartCharacterResultsInValidRange() {
+    Comment.Range range = createRange(13, 0, 19, 10);
+    assertThat(range).isValid();
+  }
+
+  @Test
+  public void zeroEndCharacterResultsInValidRange() {
+    Comment.Range range = createRange(13, 31, 19, 0);
+    assertThat(range).isValid();
+  }
+
   @Test
   public void startLineGreaterThanEndLineResultsInInvalidRange() {
     Comment.Range range = createRange(20, 2, 19, 10);

gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Comment.java

@@ -131,10 +131,10 @@ public class Comment {
   }
 
   public static class Range {
-    public int startLine;
+    public int startLine; // 1-based, inclusive
-    public int startChar;
+    public int startChar; // 0-based, inclusive
-    public int endLine;
+    public int endLine; // 1-based, exclusive
-    public int endChar;
+    public int endChar; // 0-based, exclusive
 
     public Range(Range r) {
       this(r.startLine, r.startChar, r.endLine, r.endChar);