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

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

@@ -102,7 +102,6 @@ private static boolean in(Object lval, Object rval, Location location, Environme
     }
   }
 
-  /** Helper method. Reused from the LValue class. */
   public static Object evaluate(
       Operator operator,
       Object lval,

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

@@ -1547,23 +1547,6 @@ public Object invoke(SkylarkDict<?, ?> self, Object key, Object defaultValue) {
     }
   };
 
-  // unary minus
-  @SkylarkSignature(
-    name = "-",
-    returnType = Integer.class,
-    documented = false,
-    doc = "Unary minus operator.",
-    parameters = {
-      @Param(name = "num", type = Integer.class, doc = "The number to negate.")
-    }
-  )
-  private static final BuiltinFunction minus =
-      new BuiltinFunction("-") {
-        public Integer invoke(Integer num) throws ConversionException {
-          return -num;
-        }
-      };
-
   @SkylarkSignature(
     name = "tuple",
     returnType = Tuple.class,
@@ -2221,7 +2204,7 @@ public static final class BoolModule {}
   static final List<BaseFunction> defaultGlobalFunctions =
       ImmutableList.<BaseFunction>of(
           all, any, bool, dict, dir, fail, getattr, hasattr, hash, enumerate, int_, len, list, max,
-          min, minus, print, range, repr, reversed, sorted, str, tuple, zip);
+          min, print, range, repr, reversed, sorted, str, tuple, zip);
 
   static {
     SkylarkSignatureProcessor.configureSkylarkFunctions(MethodLibrary.class);

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

@@ -11,11 +11,10 @@
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 // See the License for the specific language governing permissions and
 // limitations under the License.
+
 package com.google.devtools.build.lib.syntax;
 
-/**
- * Infix operators supported by the build language.
- */
+/** Infix binary operators. */
 public enum Operator {
 
   AND("and"),
@@ -39,13 +38,12 @@ public enum Operator {
 
   private final String name;
 
-  private Operator(String name) {
+  Operator(String name) {
     this.name = name;
   }
 
   @Override
   public String toString() {
     return name;
   }
-
 }

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

@@ -697,10 +697,9 @@ private Expression parsePrimary() {
       case MINUS:
         {
           nextToken();
-          List<Argument.Passed> args = new ArrayList<>();
           Expression expr = parsePrimaryWithSuffix();
-          args.add(setLocation(new Argument.Positional(expr), start, expr));
-          return makeFuncallExpression(null, new Identifier("-"), args, start, token.right);
+          UnaryOperatorExpression minus = new UnaryOperatorExpression(UnaryOperator.MINUS, expr);
+          return setLocation(minus, start, expr);
         }
       default:
         {
@@ -1061,7 +1060,8 @@ private Expression parseNotExpression(int prec) {
     int start = token.left;
     expect(TokenKind.NOT);
     Expression expression = parseNonTupleExpression(prec + 1);
-    NotExpression notExpression = new NotExpression(expression);
+    UnaryOperatorExpression notExpression =
+        new UnaryOperatorExpression(UnaryOperator.NOT, expression);
     return setLocation(notExpression, start, token.right);
   }
 

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

@@ -139,8 +139,8 @@ public void visit(DictionaryEntryLiteral node) {
     visit(node.getValue());
   }
 
-  public void visit(NotExpression node) {
-    visit(node.getExpression());
+  public void visit(UnaryOperatorExpression node) {
+    visit(node.getOperand());
   }
 
   public void visit(DotExpression node) {

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

@@ -0,0 +1,34 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.syntax;
+
+/** Unary operators. */
+public enum UnaryOperator {
+
+  // Include trailing whitespace in name for non-symbolic operators (for pretty printing).
+  NOT("not "),
+  MINUS("-");
+
+  private final String name;
+
+  UnaryOperator(String name) {
+    this.name = name;
+  }
+
+  @Override
+  public String toString() {
+    return name;
+  }
+}

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

@@ -0,0 +1,83 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.syntax;
+
+import com.google.devtools.build.lib.events.Location;
+
+/** Syntax node for a unary operator expression. */
+public final class UnaryOperatorExpression extends Expression {
+
+  private final UnaryOperator operator;
+
+  private final Expression operand;
+
+  public UnaryOperatorExpression(UnaryOperator operator, Expression operand) {
+    this.operator = operator;
+    this.operand = operand;
+  }
+
+  public UnaryOperator getOperator() {
+    return operator;
+  }
+
+  public Expression getOperand() {
+    return operand;
+  }
+
+  @Override
+  public String toString() {
+    // All current and planned unary operators happen to be prefix operators.
+    // Non-symbolic operators have trailing whitespace built into their name.
+    return operator.toString() + operand;
+  }
+
+  private static Object evaluate(
+      UnaryOperator operator,
+      Object value,
+      Environment env,
+      Location loc)
+      throws EvalException, InterruptedException {
+    switch (operator) {
+      case NOT:
+        return !EvalUtils.toBoolean(value);
+
+      case MINUS:
+        if (!(value instanceof Integer)) {
+          throw new EvalException(
+              loc,
+              String.format("unsupported operand type for -: '%s'",
+                  EvalUtils.getDataTypeName(value)));
+        }
+        return -((Integer) value);
+
+      default:
+        throw new AssertionError("Unsupported unary operator: " + operator);
+    }
+  }
+
+  @Override
+  Object doEval(Environment env) throws EvalException, InterruptedException {
+    return evaluate(operator, operand.eval(env), env, getLocation());
+  }
+
+  @Override
+  public void accept(SyntaxTreeVisitor visitor) {
+    visitor.visit(this);
+  }
+
+  @Override
+  void validate(ValidationEnvironment env) throws EvalException {
+    operand.validate(env);
+  }
+}

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

@@ -122,7 +122,6 @@ public void testGetVariableNames() throws Exception {
                 "False",
                 "None",
                 "True",
-                "-",
                 "all",
                 "any",
                 "bool",
@@ -155,7 +154,6 @@ public void testGetVariableNames() throws Exception {
                 "False",
                 "None",
                 "True",
-                "-",
                 "all",
                 "any",
                 "bool",

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

@@ -58,7 +58,9 @@ public void testExprs() throws Exception {
         .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