Remove dev.cel.parser.Operator, migrate parser package to dev.cel.common.Operator

PiperOrigin-RevId: 826106837

Author: l46kok

common/src/main/java/dev/cel/common/Operator.java

@@ -15,14 +15,14 @@
 package dev.cel.common;
 
 import com.google.common.collect.ImmutableMap;
-import dev.cel.common.ast.CelExpr;
 import java.util.Objects;
 import java.util.Optional;
 
 /**
  * Package-private enumeration of Common Expression Language operators.
  *
- * <p>Equivalent to https://pkg.go.dev/github.com/google/cel-go/common/operators.
+ * <p>Equivalent to <a
+ * href="https://pkg.go.dev/github.com/google/cel-go/common/operators">operators<a>.
  */
 public enum Operator {
   CONDITIONAL("_?_:_"),
@@ -96,7 +96,7 @@ String getSymbol() {
           .buildOrThrow();
 
   /** Lookup an operator by its unmangled name, as used with the source text of an expression. */
-  static Optional<Operator> find(String text) {
+  public static Optional<Operator> find(String text) {
     return Optional.ofNullable(OPERATORS.get(text));
   }
 
@@ -193,26 +193,23 @@ static Optional<Operator> findReverseBinaryOperator(String op) {
     return Optional.ofNullable(REVERSE_OPERATORS.get(op));
   }
 
-  static int lookupPrecedence(String op) {
+  /**
+   * Returns the precedence of the operator.
+   *
+   * @return Integer value describing precedence. Higher value means higher precedence. Returns 0 if
+   *     the operator is not found.
+   */
+  public static int lookupPrecedence(String op) {
     return PRECEDENCES.getOrDefault(op, 0);
   }
 
-  static Optional<String> lookupUnaryOperator(String op) {
+  /** Looks up the associated unary operator based on its function name (Ex: !_ -> !) */
+  public static Optional<String> lookupUnaryOperator(String op) {
     return Optional.ofNullable(UNARY_OPERATORS.get(op));
   }
 
-  static Optional<String> lookupBinaryOperator(String op) {
+  /** Looks up the associated binary operator based on its function name (Ex: _||_ -> ||) */
+  public static Optional<String> lookupBinaryOperator(String op) {
     return Optional.ofNullable(BINARY_OPERATORS.get(op));
   }
-
-  static boolean isOperatorLowerPrecedence(String op, CelExpr expr) {
-    if (!expr.exprKind().getKind().equals(CelExpr.ExprKind.Kind.CALL)) {
-      return false;
-    }
-    return lookupPrecedence(op) < lookupPrecedence(expr.call().function());
-  }
-
-  static boolean isOperatorLeftRecursive(String op) {
-    return !op.equals(LOGICAL_AND.getFunction()) && !op.equals(LOGICAL_OR.getFunction());
-  }
 }

common/src/test/java/dev/cel/common/BUILD.bazel

@@ -17,6 +17,7 @@ java_library(
         "//common:cel_source",
         "//common:compiler_common",
         "//common:container",
+        "//common:operator",
         "//common:options",
         "//common:proto_ast",
         "//common:proto_json_adapter",

parser/src/test/java/dev/cel/parser/OperatorTest.java renamed to common/src/test/java/dev/cel/common/OperatorTest.java

@@ -12,18 +12,14 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package dev.cel.parser;
+package dev.cel.common;
 
 import static com.google.common.truth.Truth.assertThat;
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
 
 import com.google.testing.junit.testparameterinjector.TestParameter;
 import com.google.testing.junit.testparameterinjector.TestParameterInjector;
 import com.google.testing.junit.testparameterinjector.TestParameters;
-import dev.cel.common.ast.CelExpr;
-import dev.cel.common.ast.CelExpr.CelCall;
 import java.util.Optional;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -150,70 +146,4 @@ public void lookupBinaryOperator_nonEmpty(String operator, String value) {
   public void lookupBinaryOperator_empty(String operator) {
     assertEquals(Operator.lookupBinaryOperator(operator), Optional.empty());
   }
-
-  @Test
-  @TestParameters({
-    "{operator1: '_[_]', operator2: '_&&_'}",
-    "{operator1: '_&&_', operator2: '_||_'}",
-    "{operator1: '_||_', operator2: '_?_:_'}",
-    "{operator1: '!_', operator2: '_*_'}",
-    "{operator1: '_==_', operator2: '_&&_'}",
-    "{operator1: '_!=_', operator2: '_?_:_'}",
-  })
-  public void operatorLowerPrecedence(String operator1, String operator2) {
-    CelExpr expr =
-        CelExpr.newBuilder().setCall(CelCall.newBuilder().setFunction(operator2).build()).build();
-
-    assertTrue(Operator.isOperatorLowerPrecedence(operator1, expr));
-  }
-
-  @Test
-  @TestParameters({
-    "{operator1: '_?_:_', operator2: '_&&_'}",
-    "{operator1: '_&&_', operator2: '_[_]'}",
-    "{operator1: '_||_', operator2: '!_'}",
-    "{operator1: '!_', operator2: '-_'}",
-    "{operator1: '_==_', operator2: '_!=_'}",
-    "{operator1: '_!=_', operator2: '_-_'}",
-  })
-  public void operatorNotLowerPrecedence(String operator1, String operator2) {
-    CelExpr expr =
-        CelExpr.newBuilder().setCall(CelCall.newBuilder().setFunction(operator2).build()).build();
-
-    assertFalse(Operator.isOperatorLowerPrecedence(operator1, expr));
-  }
-
-  @Test
-  @TestParameters({
-    "{operator: '_[_]'}",
-    "{operator: '!_'}",
-    "{operator: '_==_'}",
-    "{operator: '_?_:_'}",
-    "{operator: '_!=_'}",
-    "{operator: '_<_'}",
-    "{operator: '_<=_'}",
-    "{operator: '_>_'}",
-    "{operator: '_>=_'}",
-    "{operator: '_+_'}",
-    "{operator: '_-_'}",
-    "{operator: '_*_'}",
-    "{operator: '_/_'}",
-    "{operator: '_%_'}",
-    "{operator: '-_'}",
-    "{operator: 'has'}",
-    "{operator: '_[?_]'}",
-    "{operator: '@not_strictly_false'}",
-  })
-  public void operatorLeftRecursive(String operator) {
-    assertTrue(Operator.isOperatorLeftRecursive(operator));
-  }
-
-  @Test
-  @TestParameters({
-    "{operator: '_&&_'}",
-    "{operator: '_||_'}",
-  })
-  public void operatorNotLeftRecursive(String operator) {
-    assertFalse(Operator.isOperatorLeftRecursive(operator));
-  }
 }

parser/BUILD.bazel

@@ -31,11 +31,6 @@ java_library(
     exports = ["//parser/src/main/java/dev/cel/parser:macro"],
 )
 
-java_library(
-    name = "operator",
-    exports = ["//parser/src/main/java/dev/cel/parser:operator"],
-)
-
 java_library(
     name = "cel_g4_visitors",
     visibility = ["//:internal"],

parser/src/main/java/dev/cel/parser/BUILD.bazel

@@ -56,11 +56,11 @@ java_library(
     ],
     deps = [
         ":macro",
-        ":operator",
         ":parser_builder",
         "//common:cel_ast",
         "//common:cel_source",
         "//common:compiler_common",
+        "//common:operator",
         "//common:options",
         "//common:source_location",
         "//common/annotations",
@@ -94,9 +94,9 @@ java_library(
     tags = [
     ],
     deps = [
-        ":operator",
         "//:auto_value",
         "//common:compiler_common",
+        "//common:operator",
         "//common:source_location",
         "//common/ast",
         "//common/ast:expr_factory",
@@ -105,17 +105,6 @@ java_library(
     ],
 )
 
-java_library(
-    name = "operator",
-    srcs = ["Operator.java"],
-    tags = [
-    ],
-    deps = [
-        "//common/ast",
-        "@maven//:com_google_guava_guava",
-    ],
-)
-
 java_library(
     name = "unparser",
     srcs = UNPARSER_SOURCES,
@@ -134,9 +123,9 @@ java_library(
     tags = [
     ],
     deps = [
-        ":operator",
         "//common:cel_ast",
         "//common:cel_source",
+        "//common:operator",
         "//common/ast",
         "//common/ast:cel_expr_visitor",
         "//common/values:cel_byte_string",

parser/src/main/java/dev/cel/parser/CelStandardMacro.java

@@ -20,6 +20,7 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import dev.cel.common.CelIssue;
+import dev.cel.common.Operator;
 import dev.cel.common.ast.CelExpr;
 import java.util.Optional;
 

parser/src/main/java/dev/cel/parser/CelUnparserVisitor.java

@@ -13,10 +13,15 @@
 // limitations under the License.
 package dev.cel.parser;
 
+import static dev.cel.common.Operator.LOGICAL_AND;
+import static dev.cel.common.Operator.LOGICAL_OR;
+
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableSet;
 import com.google.re2j.Pattern;
 import dev.cel.common.CelAbstractSyntaxTree;
 import dev.cel.common.CelSource;
+import dev.cel.common.Operator;
 import dev.cel.common.ast.CelConstant;
 import dev.cel.common.ast.CelExpr;
 import dev.cel.common.ast.CelExpr.CelCall;
@@ -273,7 +278,7 @@ private void visitBinary(CelCall expr, String op) {
     // add parens if the current operator is lower precedence than the rhs expr
     // operator, or the same precedence and the operator is left recursive.
     boolean rhsParen = isComplexOperatorWithRespectTo(rhs, fun);
-    if (!rhsParen && Operator.isOperatorLeftRecursive(fun)) {
+    if (!rhsParen && isOperatorLeftRecursive(fun)) {
       rhsParen = isOperatorSamePrecedence(fun, rhs);
     }
 
@@ -366,7 +371,7 @@ private boolean isComplexOperatorWithRespectTo(CelExpr expr, String op) {
       return false;
     }
     // Otherwise, return whether the given op has lower precedence than expr
-    return Operator.isOperatorLowerPrecedence(op, expr);
+    return isOperatorLowerPrecedence(op, expr);
   }
 
   // bytesToOctets converts byte sequences to a string using a three digit octal encoded value
@@ -378,4 +383,17 @@ private String bytesToOctets(CelByteString bytes) {
     }
     return sb.toString();
   }
+
+  @VisibleForTesting
+  static boolean isOperatorLeftRecursive(String op) {
+    return !op.equals(LOGICAL_AND.getFunction()) && !op.equals(LOGICAL_OR.getFunction());
+  }
+
+  @VisibleForTesting
+  static boolean isOperatorLowerPrecedence(String op, CelExpr expr) {
+    if (!expr.exprKind().getKind().equals(CelExpr.ExprKind.Kind.CALL)) {
+      return false;
+    }
+    return Operator.lookupPrecedence(op) < Operator.lookupPrecedence(expr.call().function());
+  }
 }