Turn unary minus into a proper AST node

This unifies unary minus with NotExpression, and moves the minus logic from a fake builtin function into the interpreter proper.

RELNOTES: None
PiperOrigin-RevId: 160266266

3 files changed, 36 insertions, 50 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/syntax/EnvironmentTest.java b/src/test/java/com/google/devtools/build/lib/syntax/EnvironmentTest.java
index 8001f4e469..00319c62cc 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/EnvironmentTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/EnvironmentTest.java
@@ -122,7 +122,6 @@ public class EnvironmentTest extends EvaluationTestCase {
                 "False",
                 "None",
                 "True",
-                "-",
                 "all",
                 "any",
                 "bool",
@@ -155,7 +154,6 @@ public class EnvironmentTest extends EvaluationTestCase {
                 "False",
                 "None",
                 "True",
-                "-",
                 "all",
                 "any",
                 "bool",
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
index ff5940a95e..606c4ceee8 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
@@ -58,7 +58,9 @@ public class EvaluationTest extends EvaluationTestCase {
         .testStatement("123 + 456", 579)
         .testStatement("456 - 123", 333)
         .testStatement("8 % 3", 2)
-        .testIfErrorContains("unsupported operand type(s) for %: 'int' and 'string'", "3 % 'foo'");
+        .testIfErrorContains("unsupported operand type(s) for %: 'int' and 'string'", "3 % 'foo'")
+        .testStatement("-5", -5)
+        .testIfErrorContains("unsupported operand type for -: 'string'", "-'foo'");
   }
 
   @Test
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 c7bec75162..04eb664d69 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
@@ -58,6 +58,17 @@ public class ParserTest extends EvaluationTestCase {
                           node.getLocation().getEndOffset());
   }
 
+  private void assertLocation(int start, int end, Location location)
+      throws Exception {
+    int actualStart = location.getStartOffset();
+    int actualEnd = location.getEndOffset();
+
+    if (actualStart != start || actualEnd != end) {
+      fail("Expected location = [" + start + ", " + end + "), found ["
+          + actualStart + ", " + actualEnd + ")");
+    }
+  }
+
   // helper func for testListLiterals:
   private static int getIntElem(DictionaryEntryLiteral entry, boolean key) {
     return ((IntegerLiteral) (key ? entry.getKey() : entry.getValue())).getValue();
@@ -150,17 +161,15 @@ public class ParserTest extends EvaluationTestCase {
 
   @Test
   public void testUnaryMinusExpr() throws Exception {
-    FuncallExpression e = (FuncallExpression) parseExpression("-5");
-    FuncallExpression e2 = (FuncallExpression) parseExpression("- 5");
-
-    assertThat(e.getFunction().getName()).isEqualTo("-");
-    assertThat(e2.getFunction().getName()).isEqualTo("-");
-
-    assertThat(e.getArguments()).hasSize(1);
-    assertThat(e.getNumPositionalArguments()).isEqualTo(1);
+    UnaryOperatorExpression e = (UnaryOperatorExpression) parseExpression("-5");
+    UnaryOperatorExpression e2 = (UnaryOperatorExpression) parseExpression("- 5");
 
-    IntegerLiteral arg0 = (IntegerLiteral) e.getArguments().get(0).getValue();
-    assertThat((int) arg0.getValue()).isEqualTo(5);
+    IntegerLiteral i = (IntegerLiteral) e.getOperand();
+    assertThat(i.getValue()).isEqualTo(5);
+    IntegerLiteral i2 = (IntegerLiteral) e2.getOperand();
+    assertThat(i2.getValue()).isEqualTo(5);
+    assertLocation(0, 2, e.getLocation());
+    assertLocation(0, 3, e2.getLocation());
   }
 
   @Test
@@ -268,6 +277,14 @@ public class ParserTest extends EvaluationTestCase {
   }
 
   @Test
+  public void testIndex() throws Exception {
+    IndexExpression e = (IndexExpression) parseExpression("a[i]");
+    assertThat(e.getObject().toString()).isEqualTo("a");
+    assertThat(e.getKey().toString()).isEqualTo("i");
+    assertLocation(0, 4, e.getLocation());
+  }
+
+  @Test
   public void testSubstring() throws Exception {
     SliceExpression s = (SliceExpression) parseExpression("'FOO.CC'[:].lower()[1:]");
     assertThat(((IntegerLiteral) s.getStart()).value).isEqualTo(1);
@@ -294,37 +311,18 @@ public class ParserTest extends EvaluationTestCase {
     evalSlice("'0123'[1::-1]", 1, Runtime.NONE, -1);
     evalSlice("'0123'[:3:-1]", Runtime.NONE, 3, -1);
     evalSlice("'0123'[1:3:-1]", 1, 3, -1);
+
+    Expression slice = parseExpression("'0123'[1:3:-1]");
+    assertLocation(0, 14, slice.getLocation());
   }
 
   private void evalSlice(String statement, Object... expectedArgs) {
     SliceExpression e = (SliceExpression) parseExpression(statement);
 
     // There is no way to evaluate the expression here, so we rely on string comparison.
-    assertThat(e.getStart().toString()).isEqualTo(printSliceArg(expectedArgs[0]));
-    assertThat(e.getEnd().toString()).isEqualTo(printSliceArg(expectedArgs[1]));
-    assertThat(e.getStep().toString()).isEqualTo(printSliceArg(expectedArgs[2]));
-  }
-
-  private String printSliceArg(Object arg) {
-    // The parser sees negative integer constants as FuncallExpressions instead of negative
-    // IntegerLiterals.
-    // Consequently, the string representation of -1 is "-(1)", not "-1".
-    if (arg instanceof Integer) {
-      int value = (int) arg;
-      return value < 0 ? String.format("-(%d)", -value) : String.valueOf(value);
-    }
-    return arg.toString();
-  }
-
-  private void assertLocation(int start, int end, Location location)
-      throws Exception {
-    int actualStart = location.getStartOffset();
-    int actualEnd = location.getEndOffset();
-
-    if (actualStart != start || actualEnd != end) {
-      fail("Expected location = [" + start + ", " + end + "), found ["
-          + actualStart + ", " + actualEnd + ")");
-    }
+    assertThat(e.getStart().toString()).isEqualTo(expectedArgs[0].toString());
+    assertThat(e.getEnd().toString()).isEqualTo(expectedArgs[1].toString());
+    assertThat(e.getStep().toString()).isEqualTo(expectedArgs[2].toString());
   }
 
   @Test
@@ -484,18 +482,6 @@ public class ParserTest extends EvaluationTestCase {
   }
 
   @Test
-  public void testSpecialFuncallLocation() throws Exception {
-    List<Statement> statements = parseFile("-x\n");
-    assertLocation(0, 3, statements.get(0).getLocation());
-
-    statements = parseFile("arr[15]\n");
-    assertLocation(0, 8, statements.get(0).getLocation());
-
-    statements = parseFile("str[1:12]\n");
-    assertLocation(0, 10, statements.get(0).getLocation());
-  }
-
-  @Test
   public void testListPositions() throws Exception {
     String expr = "[0,f(1),2]";
     ListLiteral list = (ListLiteral) parseExpression(expr);