Add option to enable/disable repeated unary op folding in parser.

PiperOrigin-RevId: 968569494

Author: jnthntatum

parser/internal/BUILD

@@ -117,6 +117,7 @@ cc_test(
     deps = [
         ":antlr_parser",
         "//common:ast",
+        "//common:navigable_ast",
         "//common:source",
         "//internal:status_macros",
         "//internal:testing",

parser/internal/antlr_parser.cc

@@ -615,7 +615,8 @@ class ParserVisitor final : public CelBaseVisitor,
                 bool add_macro_calls = false,
                 bool enable_optional_syntax = false,
                 bool enable_quoted_identifiers = false,
-                bool enable_variadic_logical_operators = false)
+                bool enable_variadic_logical_operators = false,
+                bool fold_unary_operators = false)
       : source_(source),
         factory_(source_, max_expression_node_count),
         macro_registry_(macro_registry),
@@ -624,7 +625,8 @@ class ParserVisitor final : public CelBaseVisitor,
         add_macro_calls_(add_macro_calls),
         enable_optional_syntax_(enable_optional_syntax),
         enable_quoted_identifiers_(enable_quoted_identifiers),
-        enable_variadic_logical_operators_(enable_variadic_logical_operators) {}
+        enable_variadic_logical_operators_(enable_variadic_logical_operators),
+        fold_unary_operators_(fold_unary_operators) {}
 
   ~ParserVisitor() override = default;
 
@@ -700,6 +702,9 @@ class ParserVisitor final : public CelBaseVisitor,
                                    const Expr& e);
 
   std::string NormalizeIdentifier(CelParser::EscapeIdentContext* ctx);
+  std::any VisitUnaryOps(const std::vector<antlr4::Token*>& ops,
+                         CelParser::MemberContext* member,
+                         absl::string_view op_name);
   // Attempt to unnest parse context.
   //
   // Walk the parse tree to the first complex term to reduce recursive depth in
@@ -716,6 +721,7 @@ class ParserVisitor final : public CelBaseVisitor,
   const bool enable_optional_syntax_;
   const bool enable_quoted_identifiers_;
   const bool enable_variadic_logical_operators_;
+  const bool fold_unary_operators_;
 };
 
 template <typename T, typename = std::enable_if_t<
@@ -989,24 +995,37 @@ std::any ParserVisitor::visitUnary(CelParser::UnaryContext* ctx) {
       factory_.NextId(SourceRangeFromParserRuleContext(ctx)), "<<error>>"));
 }
 
-std::any ParserVisitor::visitLogicalNot(CelParser::LogicalNotContext* ctx) {
-  if (ctx->ops.size() % 2 == 0) {
-    return visit(ctx->member());
+std::any ParserVisitor::VisitUnaryOps(const std::vector<antlr4::Token*>& ops,
+                                      CelParser::MemberContext* member,
+                                      absl::string_view op_name) {
+  if (fold_unary_operators_) {
+    if (ops.size() % 2 == 0) {
+      return visit(member);
+    }
+    int64_t op_id = factory_.NextId(SourceRangeFromToken(ops[0]));
+    auto target = ExprFromAny(visit(member));
+    return ExprToAny(GlobalCallOrMacro(op_id, op_name, std::move(target)));
   }
-  int64_t op_id = factory_.NextId(SourceRangeFromToken(ctx->ops[0]));
-  auto target = ExprFromAny(visit(ctx->member()));
-  return ExprToAny(
-      GlobalCallOrMacro(op_id, CelOperator::LOGICAL_NOT, std::move(target)));
+
+  std::vector<int64_t> op_ids;
+  op_ids.reserve(ops.size());
+  for (const auto* op : ops) {
+    op_ids.push_back(factory_.NextId(SourceRangeFromToken(op)));
+  }
+
+  auto target = ExprFromAny(visit(member));
+  for (int i = static_cast<int>(op_ids.size()) - 1; i >= 0; --i) {
+    target = GlobalCallOrMacro(op_ids[i], op_name, std::move(target));
+  }
+  return ExprToAny(std::move(target));
+}
+
+std::any ParserVisitor::visitLogicalNot(CelParser::LogicalNotContext* ctx) {
+  return VisitUnaryOps(ctx->ops, ctx->member(), CelOperator::LOGICAL_NOT);
 }
 
 std::any ParserVisitor::visitNegate(CelParser::NegateContext* ctx) {
-  if (ctx->ops.size() % 2 == 0) {
-    return visit(ctx->member());
-  }
-  int64_t op_id = factory_.NextId(SourceRangeFromToken(ctx->ops[0]));
-  auto target = ExprFromAny(visit(ctx->member()));
-  return ExprToAny(
-      GlobalCallOrMacro(op_id, CelOperator::NEGATE, std::move(target)));
+  return VisitUnaryOps(ctx->ops, ctx->member(), CelOperator::NEGATE);
 }
 
 std::string ParserVisitor::NormalizeIdentifier(
@@ -1684,7 +1703,8 @@ absl::StatusOr<std::unique_ptr<cel::Ast>> AntlrParseImpl(
         source, options.max_recursion_depth, options.expression_node_limit,
         registry, options.add_macro_calls, options.enable_optional_syntax,
         options.enable_quoted_identifiers,
-        options.enable_variadic_logical_operators);
+        options.enable_variadic_logical_operators,
+        options.fold_unary_operators);
 
     lexer.removeErrorListeners();
     parser.removeErrorListeners();

parser/internal/antlr_parser_test.cc

@@ -23,6 +23,7 @@
 #include "absl/status/statusor.h"
 #include "absl/strings/string_view.h"
 #include "common/ast.h"
+#include "common/navigable_ast.h"
 #include "common/source.h"
 #include "internal/status_macros.h"
 #include "internal/testing.h"
@@ -72,5 +73,20 @@ TEST(AntlrParserTest, RecursionDepthExceeded) {
               HasSubstr("Exceeded max recursion depth of 6 when parsing."));
 }
 
+TEST(AntlrParserTest, UnaryOperatorsUnfoldedOption) {
+  ParserOptions options;
+  options.fold_unary_operators = false;
+
+  ASSERT_OK_AND_ASSIGN(auto ast, Parse("---a", "", options));
+  auto nav_ast = cel::NavigableAst::Build(ast->root_expr());
+  EXPECT_EQ(nav_ast.Root().height(), 4);
+
+  for (const auto& node : nav_ast.Root().DescendantsPostorder()) {
+    if (node.node_kind() == cel::NodeKind::kCall) {
+      EXPECT_EQ(node.expr()->call_expr().function(), "-_");
+    }
+  }
+}
+
 }  // namespace
 }  // namespace cel::parser_internal

parser/internal/pratt_parser_test.cc

@@ -434,9 +434,7 @@ std::vector<TestCase> GetParserTestCases() {
       TestCase{
           .source = "- -1",
           .expected_ast = R"(
-              -_(
-                -1^#2:int64#
-              )^#1:Expr.Call#
+              1^#3:int64#
             )",
       },
       TestCase{
@@ -454,11 +452,7 @@ std::vector<TestCase> GetParserTestCases() {
           .source = "---a",
           .expected_ast = R"(
               -_(
-                -_(
-                  -_(
-                    a^#4:Expr.Ident#
-                  )^#3:Expr.Call#
-                )^#2:Expr.Call#
+                a^#4:Expr.Ident#
               )^#1:Expr.Call#
             )",
       },

parser/internal/pratt_parser_worker.h

@@ -544,7 +544,15 @@ ExprNode PrattParserWorker<ExprNode>::ParseUnaryOpsChain(Token first_op) {
 
   ExprNode operand;
   if (!ops.empty() && ops.back().type == TokenType::kMinus) {
-    if (peek_token_.type == TokenType::kInt) {
+    if (options_.fold_unary_operators && ops.size() > 1 &&
+        ops[ops.size() - 2].type == TokenType::kMinus) {
+      // Match the ANTLR parser behavior where `-(-)+` prefers to match as
+      // repeated negate operators instead of a negation of an int literal.
+      // ---9223372036854775808 will fail to parse.
+      ops.pop_back();
+      ops.pop_back();
+      operand = ParseSelectorChain();
+    } else if (peek_token_.type == TokenType::kInt) {
       int64_t op_id = ops.back().id;
       ops.pop_back();
       operand = ParseNegativeIntLiteral(op_id);
@@ -561,6 +569,13 @@ ExprNode PrattParserWorker<ExprNode>::ParseUnaryOpsChain(Token first_op) {
 
   for (int i = static_cast<int>(ops.size()) - 1; i >= 0; --i) {
     std::vector<ExprNode> args;
+    if (options_.fold_unary_operators && i > 0) {
+      if (ops[i - 1].type == ops[i].type) {
+        i--;
+        continue;
+      }
+    }
+
     args.push_back(std::move(operand));
     absl::string_view op_name = (ops[i].type == TokenType::kExclamation)
                                     ? CelOperator::LOGICAL_NOT

parser/options.h

@@ -85,6 +85,22 @@ struct ParserOptions final {
   // early testing of the Pratt parser.
   // TODO(b/527638023): Remove this option once the ANTLR parser is removed.
   bool enable_pratt_parser = false;
+
+  // Folds repeated unary operators (!, -).
+  //
+  // If the operator appears repeatedly, the parser will ignore every contiguous
+  // pair.
+  //
+  // This makes it possible to parse some semantically invalid expressions as
+  // valid ones, though they are not particularly harmful.
+  //
+  // Examples that parse to the same AST:
+  //
+  // `---1` : `-(1)`
+  // `!!!!!true` : !`true`
+  // `--0u` : `0u`
+  // `!!"hello"` : `"hello"`).
+  bool fold_unary_operators = true;
 };
 
 }  // namespace cel

parser/parser_test.cc

@@ -820,11 +820,7 @@ std::vector<TestInfo> test_cases = {
      "", "", "", "",
      // PRATT PARSER AST
      "-_(\n"
-     "  -_(\n"
-     "    -_(\n"
-     "      a^#4:Expr.Ident#\n"
-     "    )^#3:Expr.Call#\n"
-     "  )^#2:Expr.Call#\n"
+     "  a^#4:Expr.Ident#\n"
      ")^#1:Expr.Call#"},
     {"1 + +", "",
      "ERROR: <input>:1:5: Syntax error: mismatched input '+' expecting {'[', "