Skylark parser: make the end position of location ranges inclusive.

Previously, the end line and column of a location were the position past the actual end of a location. This makes sense for the end offset, because one can use `input.substring(startOffset, endOffset)` to get the string belonging to an ASTNode.

However the line and column (as opposed to the offset) aren't used for that. Therefore I made the change that the end line and column now point to the last character in the location. This is also they way every compiler I know does it.

RELNOTES: none
PiperOrigin-RevId: 170723732

src/main/java/com/google/devtools/build/lib/events/Location.java

diff --git a/src/main/java/com/google/devtools/build/lib/events/Location.java b/src/main/java/com/google/devtools/build/lib/events/Location.java
index 8420254..24d2c2d 100644
--- a/src/main/java/com/google/devtools/build/lib/events/Location.java
+++ b/src/main/java/com/google/devtools/build/lib/events/Location.java
@@ -17,7 +17,6 @@
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
-
 import java.io.Serializable;
 import java.util.Objects;
 
@@ -117,11 +116,11 @@
   }
 
   /**
-   * Returns the end offset relative to the beginning of the file the object
-   * resides in.
+   * Returns the end offset relative to the beginning of the file the object resides in.
    *
-   * <p>The end offset is one position past the actual end position, making this method
-   * behave in a compatible fashion with {@link String#substring(int, int)}.
+   * <p>The end offset is one position past the last character in range, making this method behave
+   * in a compatible fashion with {@link String#substring(int, int)}. (By contrast, {@link
+   * #getEndLineAndColumn} returns the actual end position.)
    *
    * <p>To compute the length of this location, use {@code getEndOffset() - getStartOffset()}.
    */
@@ -161,8 +160,14 @@
   }
 
   /**
-   * Returns a (line, column) pair corresponding to the position denoted by
-   * getEndOffset.  Returns null if this information is not available.
+   * Returns a (line, column) pair corresponding to the end position or null if this information is
+   * unavailable.
+   *
+   * <p>The returned line and column are the position of the last character in the location range.
+   * (By contrast, {@link #getEndOffset} returns the position <strong>past</strong> the actual end
+   * position.) In particular, this means that the location spans {@code
+   * getEndLineAndColumn().getColumn() - getStartLineAndColumn().getColumn() + 1} columns but
+   * contains {@code getEndOffset() - getStartOffset()} characters.
    */
   public LineAndColumn getEndLineAndColumn() {
     return null;

src/main/java/com/google/devtools/build/lib/syntax/Lexer.java

diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Lexer.java b/src/main/java/com/google/devtools/build/lib/syntax/Lexer.java
index b1b63c8..10ff03e 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Lexer.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Lexer.java
@@ -184,7 +184,12 @@
 
     @Override
     public LineAndColumn getEndLineAndColumn() {
-      return lineNumberTable.getLineAndColumn(getEndOffset());
+      // The end offset is the location *past* the actual end position --> subtract 1:
+      int endOffset = getEndOffset() - 1;
+      if (endOffset < 0) {
+        endOffset = 0;
+      }
+      return lineNumberTable.getLineAndColumn(endOffset);
     }
 
 

src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java

diff --git a/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java b/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
index 20923da..20ea5c0 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
@@ -20,6 +20,7 @@
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.events.Location.LineAndColumn;
 import com.google.devtools.build.lib.syntax.DictionaryLiteral.DictionaryEntryLiteral;
 import com.google.devtools.build.lib.syntax.Parser.ParsingLevel;
 import com.google.devtools.build.lib.syntax.SkylarkImports.SkylarkImportSyntaxException;
@@ -356,7 +357,7 @@
 
     assertLocation(5, 29, arg1val.getLocation());
     assertThat(expr.substring(5, 28)).isEqualTo("[x for foo foo foo foo]");
-    assertThat(arg1val.getLocation().getEndLineAndColumn().getColumn()).isEqualTo(30);
+    assertThat(arg1val.getLocation().getEndLineAndColumn().getColumn()).isEqualTo(29);
 
     IntegerLiteral arg2 = (IntegerLiteral) e.getArguments().get(2).getValue();
     assertThat((int) arg2.getValue()).isEqualTo(3);
@@ -480,6 +481,14 @@
   }
 
   @Test
+  public void testEndLineAndColumnIsInclusive() {
+    AssignmentStatement stmt =
+        (AssignmentStatement) parseStatement(ParsingLevel.LOCAL_LEVEL, "a = b");
+    assertThat(stmt.getLValue().getLocation().getEndLineAndColumn())
+        .isEqualTo(new LineAndColumn(1, 1));
+  }
+
+  @Test
   public void testFuncallLocation() {
     List<Statement> statements = parseFile("a(b);c = d\n");
     Statement statement = statements.get(0);