diff --git a/conformance/BUILD b/conformance/BUILD index d72237841..eb129470f 100644 --- a/conformance/BUILD +++ b/conformance/BUILD @@ -248,7 +248,7 @@ _TESTS_TO_SKIP_LEGACY_DASHBOARD = [ ] # Generates a bunch of `cc_test` whose names follow the pattern -# `conformance_(...)_{arena|refcount}_{optimized|unoptimized}_{recursive|iterative}`. +# `conformance_(...)_{pratt|antlr}_{optimized|unoptimized}_{recursive|iterative}`. gen_conformance_tests( name = "conformance_parse_only", data = _ALL_TESTS, @@ -317,7 +317,7 @@ gen_conformance_tests( ) # Generates a bunch of `cc_test` whose names follow the pattern -# `conformance_dashboard_..._{arena|refcount}_{optimized|unoptimized}_{recursive|iterative}`. +# `conformance_dashboard_..._{pratt|antlr}_{optimized|unoptimized}_{recursive|iterative}`. gen_conformance_tests( name = "conformance_dashboard_parse_only", dashboard = True, diff --git a/conformance/run.bzl b/conformance/run.bzl index 8faeb6c16..76b33efb3 100644 --- a/conformance/run.bzl +++ b/conformance/run.bzl @@ -47,16 +47,17 @@ def _expand_tests_to_skip(tests_to_skip): result.append(test_to_skip[0:slash] + part) return result -def _conformance_test_name(name, optimize, recursive): +def _conformance_test_name(name, pratt, optimize, recursive): return "_".join( [ name, + "pratt" if pratt else "antlr", "optimized" if optimize else "unoptimized", "recursive" if recursive else "iterative", ], ) -def _conformance_test_args(modern, optimize, recursive, select_opt, skip_check, dashboard, enable_variadic_logical_operators): +def _conformance_test_args(modern, optimize, recursive, select_opt, skip_check, dashboard, enable_variadic_logical_operators, pratt): args = [] if modern: args.append("--modern") @@ -74,12 +75,16 @@ def _conformance_test_args(modern, optimize, recursive, select_opt, skip_check, args.append("--dashboard") if enable_variadic_logical_operators: args.append("--enable_variadic_logical_operators") + if pratt: + args.append("--enable_pratt_parser") + else: + args.append("--noenable_pratt_parser") return args -def _conformance_test(name, data, modern, optimize, recursive, select_opt, skip_check, skip_tests, tags, dashboard, enable_variadic_logical_operators): +def _conformance_test(name, data, modern, optimize, recursive, select_opt, skip_check, skip_tests, tags, dashboard, enable_variadic_logical_operators, pratt): cc_test( - name = _conformance_test_name(name, optimize, recursive), - args = _conformance_test_args(modern, optimize, recursive, select_opt, skip_check, dashboard, enable_variadic_logical_operators) + ["$(rlocationpath {})".format(test) for test in data], + name = _conformance_test_name(name, pratt, optimize, recursive), + args = _conformance_test_args(modern, optimize, recursive, select_opt, skip_check, dashboard, enable_variadic_logical_operators, pratt) + ["$(rlocationpath {})".format(test) for test in data], env = select( { "@platforms//os:windows": {"CEL_SKIP_TESTS": ",".join(skip_tests + _TESTS_TO_SKIP_WINDOWS)}, @@ -108,23 +113,25 @@ def gen_conformance_tests(name, data, modern = False, checked = False, select_op """ skip_check = not checked tests = [] - for optimize in (True, False): - for recursive in (True, False): - test_name = _conformance_test_name(name, optimize, recursive) - tests.append(test_name) - _conformance_test( - name, - data, - modern = modern, - optimize = optimize, - recursive = recursive, - select_opt = select_opt, - skip_check = skip_check, - skip_tests = _expand_tests_to_skip(skip_tests), - tags = tags, - dashboard = dashboard, - enable_variadic_logical_operators = enable_variadic_logical_operators, - ) + for pratt in (True, False): + for optimize in (True, False): + for recursive in (True, False): + test_name = _conformance_test_name(name, pratt, optimize, recursive) + tests.append(test_name) + _conformance_test( + name, + data, + modern = modern, + optimize = optimize, + recursive = recursive, + select_opt = select_opt, + skip_check = skip_check, + skip_tests = _expand_tests_to_skip(skip_tests), + tags = tags, + dashboard = dashboard, + enable_variadic_logical_operators = enable_variadic_logical_operators, + pratt = pratt, + ) native.test_suite( name = name, tests = tests, diff --git a/conformance/run.cc b/conformance/run.cc index 1be16ba60..6b7bae071 100644 --- a/conformance/run.cc +++ b/conformance/run.cc @@ -69,6 +69,8 @@ ABSL_FLAG(bool, select_optimization, false, "Enable select optimization."); ABSL_FLAG(bool, enable_variadic_logical_operators, false, "Enable parsing logical AND & OR operators as a single flat variadic " "call."); +ABSL_FLAG(bool, enable_pratt_parser, true, + "Enable manual (Pratt) parser instead of ANTLR parser."); namespace { @@ -266,6 +268,7 @@ NewConformanceServiceFromFlags() { .select_optimization = absl::GetFlag(FLAGS_select_optimization), .enable_variadic_logical_operators = absl::GetFlag(FLAGS_enable_variadic_logical_operators), + .enable_pratt_parser = absl::GetFlag(FLAGS_enable_pratt_parser), }); ABSL_CHECK_OK(status_or_service); return std::shared_ptr( diff --git a/conformance/service.cc b/conformance/service.cc index 4d3815d37..8a456a9f4 100644 --- a/conformance/service.cc +++ b/conformance/service.cc @@ -130,7 +130,8 @@ cel::expr::Expr ExtractExpr( absl::Status LegacyParse(const conformance::v1alpha1::ParseRequest& request, conformance::v1alpha1::ParseResponse& response, bool enable_optional_syntax, - bool enable_variadic_logical_operators) { + bool enable_variadic_logical_operators, + bool enable_pratt_parser) { if (request.cel_source().empty()) { return absl::InvalidArgumentError("no source code"); } @@ -138,6 +139,7 @@ absl::Status LegacyParse(const conformance::v1alpha1::ParseRequest& request, options.enable_optional_syntax = enable_optional_syntax; options.enable_quoted_identifiers = true; options.enable_variadic_logical_operators = enable_variadic_logical_operators; + options.enable_pratt_parser = enable_pratt_parser; cel::MacroRegistry macros; CEL_RETURN_IF_ERROR(cel::RegisterStandardMacros(macros, options)); CEL_RETURN_IF_ERROR( @@ -240,7 +242,7 @@ class LegacyConformanceServiceImpl : public ConformanceServiceInterface { public: static absl::StatusOr> Create( bool optimize, bool recursive, bool select_optimization, - bool enable_variadic_logical_operators) { + bool enable_variadic_logical_operators, bool enable_pratt_parser) { static auto* constant_arena = new Arena(); google::protobuf::LinkMessageReflection< @@ -319,14 +321,15 @@ class LegacyConformanceServiceImpl : public ConformanceServiceInterface { builder->GetRegistry(), options)); return absl::WrapUnique(new LegacyConformanceServiceImpl( - std::move(builder), enable_variadic_logical_operators)); + std::move(builder), enable_variadic_logical_operators, + enable_pratt_parser)); } void Parse(const conformance::v1alpha1::ParseRequest& request, conformance::v1alpha1::ParseResponse& response) override { auto status = LegacyParse(request, response, /*enable_optional_syntax=*/false, - enable_variadic_logical_operators_); + enable_variadic_logical_operators_, enable_pratt_parser_); if (!status.ok()) { auto* issue = response.add_issues(); issue->set_code(ToGrpcCode(status.code())); @@ -425,19 +428,22 @@ class LegacyConformanceServiceImpl : public ConformanceServiceInterface { private: LegacyConformanceServiceImpl(std::unique_ptr builder, - bool enable_variadic_logical_operators) + bool enable_variadic_logical_operators, + bool enable_pratt_parser) : builder_(std::move(builder)), - enable_variadic_logical_operators_(enable_variadic_logical_operators) {} + enable_variadic_logical_operators_(enable_variadic_logical_operators), + enable_pratt_parser_(enable_pratt_parser) {} std::unique_ptr builder_; bool enable_variadic_logical_operators_; + bool enable_pratt_parser_; }; class ModernConformanceServiceImpl : public ConformanceServiceInterface { public: static absl::StatusOr> Create( bool optimize, bool recursive, bool select_optimization, - bool enable_variadic_logical_operators) { + bool enable_variadic_logical_operators, bool enable_pratt_parser) { google::protobuf::LinkMessageReflection< cel::expr::conformance::proto3::TestAllTypes>(); google::protobuf::LinkMessageReflection< @@ -479,9 +485,9 @@ class ModernConformanceServiceImpl : public ConformanceServiceInterface { options.max_recursion_depth = 48; } - return absl::WrapUnique( - new ModernConformanceServiceImpl(options, optimize, select_optimization, - enable_variadic_logical_operators)); + return absl::WrapUnique(new ModernConformanceServiceImpl( + options, optimize, select_optimization, + enable_variadic_logical_operators, enable_pratt_parser)); } absl::StatusOr> Setup( @@ -538,7 +544,7 @@ class ModernConformanceServiceImpl : public ConformanceServiceInterface { conformance::v1alpha1::ParseResponse& response) override { auto status = LegacyParse(request, response, /*enable_optional_syntax=*/true, - enable_variadic_logical_operators_); + enable_variadic_logical_operators_, enable_pratt_parser_); if (!status.ok()) { auto* issue = response.add_issues(); issue->set_code(ToGrpcCode(status.code())); @@ -630,11 +636,13 @@ class ModernConformanceServiceImpl : public ConformanceServiceInterface { ModernConformanceServiceImpl(const RuntimeOptions& options, bool enable_optimizations, bool enable_select_optimization, - bool enable_variadic_logical_operators) + bool enable_variadic_logical_operators, + bool enable_pratt_parser) : options_(options), enable_optimizations_(enable_optimizations), enable_select_optimization_(enable_select_optimization), - enable_variadic_logical_operators_(enable_variadic_logical_operators) {} + enable_variadic_logical_operators_(enable_variadic_logical_operators), + enable_pratt_parser_(enable_pratt_parser) {} static absl::StatusOr> Plan( const cel::Runtime& runtime, @@ -666,6 +674,7 @@ class ModernConformanceServiceImpl : public ConformanceServiceInterface { bool enable_optimizations_; bool enable_select_optimization_; bool enable_variadic_logical_operators_; + bool enable_pratt_parser_; }; } // namespace @@ -679,11 +688,11 @@ NewConformanceService(const ConformanceServiceOptions& options) { if (options.modern) { return google::api::expr::runtime::ModernConformanceServiceImpl::Create( options.optimize, options.recursive, options.select_optimization, - options.enable_variadic_logical_operators); + options.enable_variadic_logical_operators, options.enable_pratt_parser); } else { return google::api::expr::runtime::LegacyConformanceServiceImpl::Create( options.optimize, options.recursive, options.select_optimization, - options.enable_variadic_logical_operators); + options.enable_variadic_logical_operators, options.enable_pratt_parser); } } diff --git a/conformance/service.h b/conformance/service.h index 8eb97296e..4cf2aec73 100644 --- a/conformance/service.h +++ b/conformance/service.h @@ -47,6 +47,7 @@ struct ConformanceServiceOptions { bool recursive; bool select_optimization; bool enable_variadic_logical_operators = false; + bool enable_pratt_parser = true; }; absl::StatusOr> diff --git a/parser/BUILD b/parser/BUILD index 6650d9fe9..04a2c0516 100644 --- a/parser/BUILD +++ b/parser/BUILD @@ -42,6 +42,7 @@ cc_library( ":source_factory", "//common:ast", "//common:constant", + "//common:expr", "//common:expr_factory", "//common:operators", "//common:source", @@ -50,8 +51,8 @@ cc_library( "//internal:lexis", "//internal:status_macros", "//internal:strings", - "//internal:utf8", "//parser/internal:cel_cc_parser", + "//parser/internal:pratt_parser", "@antlr4-cpp-runtime", "@com_google_absl//absl/base:core_headers", "@com_google_absl//absl/cleanup", @@ -60,6 +61,7 @@ cc_library( "@com_google_absl//absl/container:flat_hash_set", "@com_google_absl//absl/functional:overload", "@com_google_absl//absl/log:absl_check", + "@com_google_absl//absl/log:check", "@com_google_absl//absl/memory", "@com_google_absl//absl/status", "@com_google_absl//absl/status:statusor", diff --git a/parser/internal/BUILD b/parser/internal/BUILD index bf95d66b7..8638fcc92 100644 --- a/parser/internal/BUILD +++ b/parser/internal/BUILD @@ -117,6 +117,7 @@ cc_library( "//parser:macro_registry", "//parser:options", "//parser:parser_interface", + "@com_google_absl//absl/algorithm:container", "@com_google_absl//absl/base:nullability", "@com_google_absl//absl/cleanup", "@com_google_absl//absl/container:flat_hash_map", diff --git a/parser/internal/ast_factory_test.cc b/parser/internal/ast_factory_test.cc index 3e80a4a17..585c1a269 100644 --- a/parser/internal/ast_factory_test.cc +++ b/parser/internal/ast_factory_test.cc @@ -405,8 +405,8 @@ class TestMacroExprExpanderSupport public: int64_t NextId() override { return 42; } int64_t CopyId(int64_t id) override { return id; } - cel::Expr ReportError(std::string_view) override { return cel::Expr(); } - cel::Expr ReportErrorAt(const cel::Expr&, std::string_view) override { + cel::Expr ReportError(absl::string_view) override { return cel::Expr(); } + cel::Expr ReportErrorAt(const cel::Expr&, absl::string_view) override { return cel::Expr(); } }; diff --git a/parser/internal/pratt_parser.cc b/parser/internal/pratt_parser.cc index e83c21fce..0ccc28c15 100644 --- a/parser/internal/pratt_parser.cc +++ b/parser/internal/pratt_parser.cc @@ -14,6 +14,7 @@ #include "parser/internal/pratt_parser.h" +#include #include #include #include @@ -21,6 +22,7 @@ #include #include +#include "absl/algorithm/container.h" #include "absl/base/nullability.h" #include "absl/cleanup/cleanup.h" #include "absl/container/flat_hash_map.h" @@ -158,21 +160,34 @@ template class PrattParserWorker; absl::StatusOr> PrattParserImpl::ParseImpl( const cel::Source& source, std::vector* absl_nullable parse_issues) const { - if (source.content().size() > options_.expression_size_codepoint_limit) { + return PrattParseImpl(source, macro_registry_, options_, parse_issues); +} + +absl::StatusOr> PrattParseImpl( + const cel::Source& source, const cel::MacroRegistry& registry, + const ParserOptions& options, std::vector* parse_issues) { + if (source.content().size() > options.expression_size_codepoint_limit) { return absl::InvalidArgumentError(absl::StrFormat( - "expression size exceeds codepoint limit: %zu > %d", - source.content().size(), options_.expression_size_codepoint_limit)); + "expression size exceeds codepoint limit. input size: %zu, limit: %d", + source.content().size(), options.expression_size_codepoint_limit)); } std::vector issues; - AstFactory factory(¯o_registry_); - PrattParserWorker worker(source, options_, &issues, factory); + AstFactory factory(®istry); + PrattParserWorker worker(source, options, &issues, factory); Expr expr = worker.Parse(); if (worker.is_recursion_limit_exceeded()) { return absl::CancelledError( absl::StrFormat("Expression recursion limit exceeded. limit: %d", - options_.max_recursion_depth)); + options.max_recursion_depth)); } if (worker.has_errors()) { + absl::c_stable_sort( + issues, [](const cel::ParseIssue& lhs, const cel::ParseIssue& rhs) { + if (lhs.location().line != rhs.location().line) { + return lhs.location().line < rhs.location().line; + } + return lhs.location().column < rhs.location().column; + }); std::string err_msg = FormatIssues(source, issues); if (parse_issues != nullptr) { parse_issues->swap(issues); diff --git a/parser/internal/pratt_parser.h b/parser/internal/pratt_parser.h index 21e9878f3..469f48aec 100644 --- a/parser/internal/pratt_parser.h +++ b/parser/internal/pratt_parser.h @@ -67,6 +67,11 @@ class PrattParserImpl final : public cel::Parser { absl::flat_hash_set library_ids_; }; +absl::StatusOr> PrattParseImpl( + const cel::Source& source, const cel::MacroRegistry& registry, + const ParserOptions& options, + std::vector* parse_issues = nullptr); + class PrattParserBuilderImpl final : public cel::ParserBuilder { public: explicit PrattParserBuilderImpl(const cel::ParserOptions& options) diff --git a/parser/internal/pratt_parser_test.cc b/parser/internal/pratt_parser_test.cc index 2956ec792..7b2570514 100644 --- a/parser/internal/pratt_parser_test.cc +++ b/parser/internal/pratt_parser_test.cc @@ -1810,10 +1810,10 @@ TEST(PrattParserErrorRecoveryTest, ErrorRecoveryLimitOne) { EXPECT_THAT(result, StatusIs(absl::StatusCode::kInvalidArgument)); ASSERT_OK_AND_ASSIGN(auto source, cel::NewSource("......")); EXPECT_EQ(FormatIssues(*source, issues), + "ERROR: :-1:0: Error recovery limit (1) exceeded\n" "ERROR: :1:2: expected identifier\n" " | ......\n" - " | .^\n" - "ERROR: :-1:0: Error recovery limit (1) exceeded"); + " | .^"); } } // namespace diff --git a/parser/options.h b/parser/options.h index 22154e1f9..eca41810a 100644 --- a/parser/options.h +++ b/parser/options.h @@ -66,6 +66,17 @@ struct ParserOptions final { // Enables parsing logical AND & OR operators as a single flat variadic call // instead of a balanced/nested binary AST structure. bool enable_variadic_logical_operators = false; + + // Enable the manual (Pratt) parser instead of the ANTLR parser. + // + // CAUTION: Pratt Parser is work in progress. Don't use unless you are aware + // of the risks. There is currently no support whatsoever available for + // clients of this option. + // + // This option is temporary and should not be used by new code outside of the + // early testing of the Pratt parser. + // TODO(b/527638023): Remove this option once the ANTLR parser is removed. + bool enable_pratt_parser = false; }; } // namespace cel diff --git a/parser/parser.cc b/parser/parser.cc index 24b4ca079..6a66ffc9a 100644 --- a/parser/parser.cc +++ b/parser/parser.cc @@ -25,8 +25,9 @@ #include #include #include +#include #include -#include +#include #include #include @@ -39,6 +40,7 @@ #include "absl/container/flat_hash_set.h" #include "absl/functional/overload.h" #include "absl/log/absl_check.h" +#include "absl/log/check.h" #include "absl/memory/memory.h" #include "absl/status/status.h" #include "absl/status/statusor.h" @@ -57,13 +59,14 @@ #include "common/ast/expr_proto.h" #include "common/ast/source_info_proto.h" #include "common/constant.h" +#include "common/expr.h" #include "common/expr_factory.h" #include "common/operators.h" #include "common/source.h" #include "internal/lexis.h" #include "internal/status_macros.h" #include "internal/strings.h" -#include "internal/utf8.h" +#include "parser/internal/pratt_parser.h" #pragma push_macro("IN") #undef IN #include "parser/internal/CelBaseVisitor.h" @@ -1661,6 +1664,7 @@ absl::StatusOr ParseImpl( const cel::Source& source, const cel::MacroRegistry& registry, const ParserOptions& options, std::vector* parse_issues = nullptr) { + ABSL_DCHECK(!options.enable_pratt_parser); try { CodePointStream input(source.content(), source.description()); if (input.size() > options.expression_size_codepoint_limit) { @@ -1844,6 +1848,10 @@ class ParserBuilderImpl : public cel::ParserBuilder { library_ids.insert("optional"); } CEL_RETURN_IF_ERROR(macro_registry.RegisterMacros(individual_macros)); + if (options_.enable_pratt_parser) { + return std::make_unique( + options_, std::move(macro_registry), std::move(library_ids)); + } return std::make_unique(options_, std::move(macro_registry), std::move(library_ids)); } @@ -1903,9 +1911,19 @@ absl::StatusOr EnrichedParse( absl::StatusOr EnrichedParse( const cel::Source& source, const cel::MacroRegistry& registry, const ParserOptions& options) { + ParsedExpr parsed_expr; + if (options.enable_pratt_parser) { + CEL_ASSIGN_OR_RETURN( + std::unique_ptr ast, + cel::parser_internal::PrattParseImpl(source, registry, options)); + CEL_RETURN_IF_ERROR(cel::ast_internal::ExprToProto( + ast->root_expr(), parsed_expr.mutable_expr())); + CEL_RETURN_IF_ERROR(cel::ast_internal::SourceInfoToProto( + ast->source_info(), parsed_expr.mutable_source_info())); + return VerboseParsedExpr(std::move(parsed_expr), EnrichedSourceInfo()); + } CEL_ASSIGN_OR_RETURN(ParseResult parse_result, ParseImpl(source, registry, options)); - ParsedExpr parsed_expr; CEL_RETURN_IF_ERROR(cel::ast_internal::ExprToProto( parse_result.expr, parsed_expr.mutable_expr())); diff --git a/parser/parser_test.cc b/parser/parser_test.cc index 357a83a79..9ea6e7832 100644 --- a/parser/parser_test.cc +++ b/parser/parser_test.cc @@ -16,7 +16,7 @@ #include #include -#include +#include #include #include @@ -56,8 +56,16 @@ using ::testing::Not; struct TestInfo { TestInfo(const std::string& I, const std::string& P, const std::string& E = "", const std::string& L = "", - const std::string& R = "", const std::string& M = "") - : I(I), P(P), E(E), L(L), R(R), M(M) {} + const std::string& R = "", const std::string& M = "", + const std::string& P_PRATT = "", const std::string& E_PRATT = "") + : I(I), + P(P), + E(E), + L(L), + R(R), + M(M), + P_PRATT(P_PRATT), + E_PRATT(E_PRATT) {} // I contains the input expression to be parsed. std::string I; @@ -77,6 +85,12 @@ struct TestInfo { // M contains the expected macro call output of hte expression tree. std::string M; + + // P_PRATT contains alternative adorned AST string when using pratt parser. + std::string P_PRATT; + + // E_PRATT contains alternative error output when using pratt parser. + std::string E_PRATT; }; std::vector test_cases = { @@ -130,6 +144,12 @@ std::vector test_cases = { "{\n" " foo^#3:Expr.Ident#:5^#4:int64#^#2:Expr.CreateStruct.Entry#,\n" " bar^#6:Expr.Ident#:\"xyz\"^#7:string#^#5:Expr.CreateStruct.Entry#\n" + "}^#1:Expr.CreateStruct#", + "", "", "", "", + // PRATT PARSER AST + "{\n" + " foo^#2:Expr.Ident#:5^#4:int64#^#3:Expr.CreateStruct.Entry#,\n" + " bar^#5:Expr.Ident#:\"xyz\"^#7:string#^#6:Expr.CreateStruct.Entry#\n" "}^#1:Expr.CreateStruct#"}, {"a > 5 && a < 10", "_&&_(\n" @@ -160,6 +180,11 @@ std::vector test_cases = { "NUM_FLOAT, " "NUM_INT, " "NUM_UINT, STRING, BYTES, IDENTIFIER}\n | {\n" + " | .^", + "", "", "", "", + // PRATT PARSER ERROR MESSAGE + "ERROR: :1:2: expected '}'\n" + " | {\n" " | .^"}, // test cases from Go @@ -353,6 +378,12 @@ std::vector test_cases = { "{\n" " a^#3:Expr.Ident#:b^#4:Expr.Ident#^#2:Expr.CreateStruct.Entry#,\n" " c^#6:Expr.Ident#:d^#7:Expr.Ident#^#5:Expr.CreateStruct.Entry#\n" + "}^#1:Expr.CreateStruct#", + "", "", "", "", + // PRATT PARSER AST + "{\n" + " a^#2:Expr.Ident#:b^#4:Expr.Ident#^#3:Expr.CreateStruct.Entry#,\n" + " c^#5:Expr.Ident#:d^#7:Expr.Ident#^#6:Expr.CreateStruct.Entry#\n" "}^#1:Expr.CreateStruct#"}, {"[]", "[]^#1:Expr.CreateList#"}, {"[a]", @@ -417,14 +448,27 @@ std::vector test_cases = { " | ....^\n" "ERROR: :1:7: Syntax error: extraneous input 'b' expecting \n" " | *@a | b\n" - " | ......^"}, + " | ......^", + "", "", "", "", + // PRATT PARSER ERROR MESSAGE + "ERROR: :1:1: unexpected token\n" + " | *@a | b\n" + " | ^\n" + "ERROR: :1:2: unexpected character\n" + " | *@a | b\n" + " | .^"}, {"a | b", "", "ERROR: :1:3: Syntax error: token recognition error at: '| '\n" " | a | b\n" " | ..^\n" "ERROR: :1:5: Syntax error: extraneous input 'b' expecting \n" " | a | b\n" - " | ....^"}, + " | ....^", + "", "", "", "", + // PRATT PARSER ERROR MESSAGE + "ERROR: :1:3: unexpected single '|', expected '||'\n" + " | a | b\n" + " | ..^"}, {"?", "", "ERROR: :1:1: Syntax error: mismatched input '?' expecting " "{'[', '{', '(', '.', '-', '!', 'true', 'false', 'null', NUM_FLOAT, " @@ -432,13 +476,23 @@ std::vector test_cases = { "ERROR: :1:2: Syntax error: mismatched input '' expecting " "{'[', '{', '(', '.', '-', '!', 'true', 'false', 'null', NUM_FLOAT, " "NUM_INT, NUM_UINT, STRING, BYTES, IDENTIFIER}\n | ?\n | .^\n" - "ERROR: :4294967295:0: <> parsetree"}, + "ERROR: :4294967295:0: <> parsetree", + "", "", "", "", + // PRATT PARSER ERROR MESSAGE + "ERROR: :1:1: unexpected token\n" + " | ?\n" + " | ^"}, {"t{>C}", "", "ERROR: :1:3: Syntax error: extraneous input '>' expecting {'}', " "',', '\\u003F', IDENTIFIER, ESC_IDENTIFIER}\n | t{>C}\n | ..^\nERROR: " ":1:5: " "Syntax error: " - "mismatched input '}' expecting ':'\n | t{>C}\n | ....^"}, + "mismatched input '}' expecting ':'\n | t{>C}\n | ....^", + "", "", "", "", + // PRATT PARSER ERROR MESSAGE + "ERROR: :1:3: expected struct field name\n" + " | t{>C}\n" + " | ..^"}, // Macro tests {"has(m.f)", "m^#2:Expr.Ident#.f~test-only~^#4:Expr.Select#", "", @@ -584,6 +638,12 @@ std::vector test_cases = { "{\n" " 1^#3:int64#:2u^#4:uint64#^#2:Expr.CreateStruct.Entry#,\n" " 2^#6:int64#:3u^#7:uint64#^#5:Expr.CreateStruct.Entry#\n" + "}^#1:Expr.CreateStruct#", + "", "", "", "", + // PRATT PARSER AST + "{\n" + " 1^#2:int64#:2u^#4:uint64#^#3:Expr.CreateStruct.Entry#,\n" + " 2^#5:int64#:3u^#7:uint64#^#6:Expr.CreateStruct.Entry#\n" "}^#1:Expr.CreateStruct#"}, {"TestAllTypes{single_int32: 1, single_int64: 2}", "TestAllTypes{\n" @@ -593,6 +653,11 @@ std::vector test_cases = { {"TestAllTypes(){single_int32: 1, single_int64: 2}", "", "ERROR: :1:15: Syntax error: mismatched input '{' expecting \n" " | TestAllTypes(){single_int32: 1, single_int64: 2}\n" + " | ..............^", + "", "", "", "", + // PRATT PARSER ERROR MESSAGE + "ERROR: :1:15: unexpected token after expression\n" + " | TestAllTypes(){single_int32: 1, single_int64: 2}\n" " | ..............^"}, {"size(x) == x.size()", "_==_(\n" @@ -610,12 +675,22 @@ std::vector test_cases = { "'{', '(', '.', '-', '!', 'true', 'false', 'null', NUM_FLOAT, NUM_INT, " "NUM_UINT, STRING, BYTES, IDENTIFIER}\n" " | 1 + $\n" - " | .....^"}, + " | .....^", + "", "", "", "", + // PRATT PARSER ERROR MESSAGE + "ERROR: :1:5: unexpected character\n" + " | 1 + $\n" + " | ....^"}, {"1 + 2\n" "3 +", "", "ERROR: :2:1: Syntax error: mismatched input '3' expecting \n" " | 3 +\n" + " | ^", + "", "", "", "", + // PRATT PARSER ERROR MESSAGE + "ERROR: :2:1: unexpected token after expression\n" + " | 3 +\n" " | ^"}, {"\"\\\"\"", "\"\\\"\"^#1:string#"}, {"[1,3,4][0]", @@ -634,24 +709,50 @@ std::vector test_cases = { {"[].all(.x, x)", "", "ERROR: :1:9: all() variable name must be a simple identifier\n" " | [].all(.x, x)\n" - " | ........^"}, + " | ........^", + "", "", "", "", + // PRATT PARSER ERROR MESSAGE + "ERROR: :1:8: all() variable name must be a simple identifier\n" + " | [].all(.x, x)\n" + " | .......^"}, {"[].exists(.x, x)", "", "ERROR: :1:12: exists() variable name must be a simple identifier\n" " | [].exists(.x, x)\n" - " | ...........^"}, + " | ...........^", + "", "", "", "", + // PRATT PARSER ERROR MESSAGE + "ERROR: :1:11: exists() variable name must be a simple identifier\n" + " | [].exists(.x, x)\n" + " | ..........^"}, {"[].exists_one(.x, x)", "", "ERROR: :1:16: exists_one() variable name must be a simple " "identifier\n" " | [].exists_one(.x, x)\n" - " | ...............^"}, + " | ...............^", + "", "", "", "", + // PRATT PARSER ERROR MESSAGE + "ERROR: :1:15: exists_one() variable name must be a simple " + "identifier\n" + " | [].exists_one(.x, x)\n" + " | ..............^"}, {"[].map(.x, x, x)", "", "ERROR: :1:9: map() variable name must be a simple identifier\n" " | [].map(.x, x, x)\n" - " | ........^"}, + " | ........^", + "", "", "", "", + // PRATT PARSER ERROR MESSAGE + "ERROR: :1:8: map() variable name must be a simple identifier\n" + " | [].map(.x, x, x)\n" + " | .......^"}, {"[].filter(.x, x)", "", "ERROR: :1:12: filter() variable name must be a simple identifier\n" " | [].filter(.x, x)\n" - " | ...........^"}, + " | ...........^", + "", "", "", "", + // PRATT PARSER ERROR MESSAGE + "ERROR: :1:11: filter() variable name must be a simple identifier\n" + " | [].filter(.x, x)\n" + " | ..........^"}, {"x[\"a\"].single_int32 == 23", "_==_(\n" " _[_](\n" @@ -707,6 +808,15 @@ std::vector test_cases = { {"---a", "-_(\n" " a^#2:Expr.Ident#\n" + ")^#1:Expr.Call#", + "", "", "", "", + // PRATT PARSER AST + "-_(\n" + " -_(\n" + " -_(\n" + " a^#4:Expr.Ident#\n" + " )^#3:Expr.Call#\n" + " )^#2:Expr.Call#\n" ")^#1:Expr.Call#"}, {"1 + +", "", "ERROR: :1:5: Syntax error: mismatched input '+' expecting {'[', " @@ -721,7 +831,12 @@ std::vector test_cases = { "'{', '(', '.', '-', '!', 'true', 'false', 'null', NUM_FLOAT, NUM_INT, " "NUM_UINT, STRING, BYTES, IDENTIFIER}\n" " | 1 + +\n" - " | .....^"}, + " | .....^", + "", "", "", "", + // PRATT PARSER ERROR MESSAGE + "ERROR: :1:5: unexpected token\n" + " | 1 + +\n" + " | ....^"}, {"\"abc\" + \"def\"", "_+_(\n" " \"abc\"^#1:string#,\n" @@ -731,6 +846,11 @@ std::vector test_cases = { "ERROR: :1:10: Syntax error: no viable alternative at input " "'.\"a\"'\n" " | {\"a\": 1}.\"a\"\n" + " | .........^", + "", "", "", "", + // PRATT PARSER ERROR MESSAGE + "ERROR: :1:10: expected identifier after '.'\n" + " | {\"a\": 1}.\"a\"\n" " | .........^"}, {"\"\\xC3\\XBF\"", "\"ÿ\"^#1:string#"}, {"\"\\303\\277\"", "\"ÿ\"^#1:string#"}, @@ -750,7 +870,13 @@ std::vector test_cases = { "'{', '(', '.', '-', '!', 'true', 'false', 'null', NUM_FLOAT, NUM_INT, " "NUM_UINT, STRING, BYTES, IDENTIFIER}\n" " | \"\\xFh\"\n" - " | ......^"}, + " | ......^", + "", "", "", "", + // PRATT PARSER ERROR MESSAGE + "ERROR: :1:1: Invalid string literal: Illegal escape sequence: Hex " + "escape must be followed by 2 hex digits but saw: \\xFh\n" + " | \"\\xFh\"\n" + " | ^"}, {"\"\\a\\b\\f\\n\\r\\t\\v\\'\\\"\\\\\\? Illegal escape \\>\"", "", "ERROR: :1:1: Syntax error: token recognition error at: " "'\"\\a\\b\\f\\n\\r\\t\\v\\'\\\"\\\\\\? Illegal escape \\>'\n" @@ -764,7 +890,13 @@ std::vector test_cases = { " '{', '(', '.', '-', '!', 'true', 'false', 'null', NUM_FLOAT, NUM_INT, " "NUM_UINT, STRING, BYTES, IDENTIFIER}\n" " | \"\\a\\b\\f\\n\\r\\t\\v\\'\\\"\\\\\\? Illegal escape \\>\"\n" - " | ..........................................^"}, + " | ..........................................^", + "", "", "", "", + // PRATT PARSER ERROR MESSAGE + "ERROR: :1:1: Invalid string literal: Illegal escape sequence: " + "\\>\n" + " | \"\\a\\b\\f\\n\\r\\t\\v\\'\\\"\\\\\\? Illegal escape \\>\"\n" + " | ^"}, {"'😁' in ['😁', '😑', '😦']", "@in(\n" " \"😁\"^#1:string#,\n" @@ -814,7 +946,15 @@ std::vector test_cases = { " | .........＾\n" "ERROR: :2:11: Syntax error: no viable alternative at input '.'\n" " | && in.😁\n" - " | .........．^"}, + " | .........．^", + "", "", "", "", + // PRATT PARSER ERROR MESSAGE + "ERROR: :2:7: unexpected token\n" + " | && in.😁\n" + " | ......^\n" + "ERROR: :2:10: unexpected character\n" + " | && in.😁\n" + " | .........＾"}, {"as", "", "ERROR: :1:1: reserved identifier: as\n" " | as\n" @@ -862,7 +1002,12 @@ std::vector test_cases = { "'{', '(', '.', '-', '!', 'true', 'false', 'null', NUM_FLOAT, NUM_INT, " "NUM_UINT, STRING, BYTES, IDENTIFIER}\n" " | in\n" - " | ..^"}, + " | ..^", + "", "", "", "", + // PRATT PARSER ERROR MESSAGE + "ERROR: :1:1: unexpected token\n" + " | in\n" + " | ^"}, {"let", "", "ERROR: :1:1: reserved identifier: let\n" " | let\n" @@ -907,6 +1052,17 @@ std::vector test_cases = { " | ...................^\n" "ERROR: :1:26: reserved identifier: var\n" " | [1, 2, 3].map(var, var * var)\n" + " | .........................^", + "", "", "", "", + // PRATT PARSER ERROR MESSAGE + "ERROR: :1:15: reserved identifier: var\n" + " | [1, 2, 3].map(var, var * var)\n" + " | ..............^\n" + "ERROR: :1:20: reserved identifier: var\n" + " | [1, 2, 3].map(var, var * var)\n" + " | ...................^\n" + "ERROR: :1:26: reserved identifier: var\n" + " | [1, 2, 3].map(var, var * var)\n" " | .........................^"}, {"[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[" "[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[" @@ -929,14 +1085,17 @@ std::vector test_cases = { "", "", }, - { - "[\n\t\r[\n\t\r[\n\t\r]\n\t\r]\n\t\r", - "", // parse output not validated as it is too large. - "ERROR: :6:3: Syntax error: mismatched input '' expecting " - "{']', ','}\n" - " | \r\n" - " | ..^", - }, + {"[\n\t\r[\n\t\r[\n\t\r]\n\t\r]\n\t\r", "", + // parse output not validated as it is too large. + "ERROR: :6:3: Syntax error: mismatched input '' expecting " + "{']', ','}\n" + " | \r\n" + " | ..^", + "", "", "", "", + // PRATT PARSER ERROR MESSAGE + "ERROR: :6:3: expected ']'\n" + " | \r\n" + " | ..^"}, // Identifier quoting syntax tests. {"a.`b`", "a^#1:Expr.Ident#.b^#2:Expr.Select#"}, @@ -977,21 +1136,36 @@ std::vector test_cases = { " | ..^\n" "ERROR: :1:7: Syntax error: token recognition error at: '`'\n" " | a.`b c`\n" - " | ......^"}, + " | ......^", + "", "", "", "", + // PRATT PARSER ERROR MESSAGE + "ERROR: :1:3: unexpected quoted identifier\n" + " | a.`b c`\n" + " | ..^"}, {"a.`@foo`", "", "ERROR: :1:3: Syntax error: token recognition error at: '`@'\n" " | a.`@foo`\n" " | ..^\n" "ERROR: :1:8: Syntax error: token recognition error at: '`'\n" " | a.`@foo`\n" - " | .......^"}, + " | .......^", + "", "", "", "", + // PRATT PARSER ERROR MESSAGE + "ERROR: :1:3: unexpected quoted identifier\n" + " | a.`@foo`\n" + " | ..^"}, {"a.`$foo`", "", "ERROR: :1:3: Syntax error: token recognition error at: '`$'\n" " | a.`$foo`\n" " | ..^\n" "ERROR: :1:8: Syntax error: token recognition error at: '`'\n" " | a.`$foo`\n" - " | .......^"}, + " | .......^", + "", "", "", "", + // PRATT PARSER ERROR MESSAGE + "ERROR: :1:3: unexpected quoted identifier\n" + " | a.`$foo`\n" + " | ..^"}, {"`a.b`", "", "ERROR: :1:1: Syntax error: mismatched input '`a.b`' expecting " "{'[', '{', " @@ -999,6 +1173,11 @@ std::vector test_cases = { "NUM_UINT, STRING, " "BYTES, IDENTIFIER}\n" " | `a.b`\n" + " | ^", + "", "", "", "", + // PRATT PARSER ERROR MESSAGE + "ERROR: :1:1: unexpected quoted identifier\n" + " | `a.b`\n" " | ^"}, {"`a.b`()", "", "ERROR: :1:1: Syntax error: extraneous input '`a.b`' expecting " @@ -1010,11 +1189,21 @@ std::vector test_cases = { "'{', '(', '.', '-', '!', 'true', 'false', 'null', NUM_FLOAT, NUM" "_INT, NUM_UINT, STRING, BYTES, IDENTIFIER}\n" " | `a.b`()\n" - " | ......^"}, + " | ......^", + "", "", "", "", + // PRATT PARSER ERROR MESSAGE + "ERROR: :1:1: unexpected quoted identifier\n" + " | `a.b`()\n" + " | ^"}, {"foo.`a.b`()", "", "ERROR: :1:10: Syntax error: mismatched input '(' expecting \n" " | foo.`a.b`()\n" - " | .........^"}, + " | .........^", + "", "", "", "", + // PRATT PARSER ERROR MESSAGE + "ERROR: :1:5: unexpected quoted identifier\n" + " | foo.`a.b`()\n" + " | ....^"}, // Macro calls tests {"x.filter(y, y.filter(z, z > 0))", @@ -1290,7 +1479,12 @@ std::vector test_cases = { {"{?'key': value}", "{\n " "?\"key\"^#3:string#:value^#4:Expr.Ident#^#2:Expr.CreateStruct.Entry#\n}^#" - "1:Expr.CreateStruct#"}, + "1:Expr.CreateStruct#", + "", "", "", "", + // PRATT PARSER AST + "{\n" + " ?\"key\"^#2:string#:value^#4:Expr.Ident#^#3:Expr.CreateStruct.Entry#\n" + "}^#1:Expr.CreateStruct#"}, {"[?a, ?b]", "[\n ?a^#2:Expr.Ident#,\n ?b^#3:Expr.Ident#\n]^#1:Expr.CreateList#"}, {"[?a[?b]]", @@ -1489,46 +1683,59 @@ std::string ConvertMacroCallsToString( return result.substr(0, result.size() - 3); } -class ExpressionTest : public testing::TestWithParam {}; +class ExpressionTest + : public testing::TestWithParam> { + protected: + ExpressionTest() { options_.enable_pratt_parser = std::get<1>(GetParam()); } + + ParserOptions options_; +}; TEST_P(ExpressionTest, Parse) { - const TestInfo& test_info = GetParam(); - ParserOptions options; + const TestInfo& test_info = std::get<0>(GetParam()); if (!test_info.M.empty()) { - options.add_macro_calls = true; + options_.add_macro_calls = true; } - options.enable_optional_syntax = true; - options.enable_quoted_identifiers = true; + options_.enable_optional_syntax = true; + options_.enable_quoted_identifiers = true; std::vector macros = Macro::AllMacros(); macros.push_back(cel::OptMapMacro()); macros.push_back(cel::OptFlatMapMacro()); - auto result = EnrichedParse(test_info.I, macros, "", options); + auto result = EnrichedParse(test_info.I, macros, "", options_); if (test_info.E.empty()) { ASSERT_THAT(result, IsOk()); } else { EXPECT_THAT(result, Not(IsOk())); - EXPECT_EQ(test_info.E, result.status().message()); + if (options_.enable_pratt_parser && !test_info.E_PRATT.empty()) { + EXPECT_EQ(test_info.E_PRATT, result.status().message()); + } else { + EXPECT_EQ(test_info.E, result.status().message()); + } } if (!test_info.P.empty()) { KindAndIdAdorner kind_and_id_adorner; ExprPrinter w(kind_and_id_adorner); std::string adorned_string = w.PrintProto(result->parsed_expr().expr()); - EXPECT_EQ(test_info.P, adorned_string) - << result->parsed_expr().ShortDebugString(); + if (options_.enable_pratt_parser && !test_info.P_PRATT.empty()) { + EXPECT_EQ(test_info.P_PRATT, adorned_string) + << result->parsed_expr().ShortDebugString(); + } else { + EXPECT_EQ(test_info.P, adorned_string) + << result->parsed_expr().ShortDebugString(); + } } - if (!test_info.L.empty()) { + if (!options_.enable_pratt_parser && !test_info.L.empty()) { LocationAdorner location_adorner(result->parsed_expr().source_info()); ExprPrinter w(location_adorner); std::string adorned_string = w.PrintProto(result->parsed_expr().expr()); EXPECT_EQ(test_info.L, adorned_string) << result->parsed_expr().ShortDebugString(); - ; } - if (!test_info.R.empty()) { + if (!options_.enable_pratt_parser && !test_info.R.empty()) { EXPECT_EQ(test_info.R, ConvertEnrichedSourceInfoToString( result->enriched_source_info())); } @@ -1537,7 +1744,6 @@ TEST_P(ExpressionTest, Parse) { EXPECT_EQ(test_info.M, ConvertMacroCallsToString( result.value().parsed_expr().source_info())) << result->parsed_expr().ShortDebugString(); - ; } } @@ -1564,7 +1770,14 @@ TEST(ExpressionTest, CompositeExpressionOffsets) { EXPECT_EQ(msg_offsets.at(1), std::make_pair(0, 8)); } -TEST(ExpressionTest, TsanOom) { +class ExpressionImplTest : public testing::TestWithParam { + protected: + ExpressionImplTest() { options_.enable_pratt_parser = GetParam(); } + + ParserOptions options_; +}; + +TEST_P(ExpressionImplTest, TsanOom) { Parse( "[[a([[???[a[[??[a([[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[" "[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[" @@ -1580,7 +1793,8 @@ TEST(ExpressionTest, TsanOom) { "[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[" "[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[" "[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[???[" - "a([[????") + "a([[????", + "", options_) .IgnoreError(); } @@ -1595,18 +1809,16 @@ TEST_P(ExpressionTest, ErrorRecoveryLimits) { "'..'\n | ......\n | .^"); } -TEST(ExpressionTest, ExpressionSizeLimit) { - ParserOptions options; - options.expression_size_codepoint_limit = 10; - auto result = Parse("...............", "", options); +TEST_P(ExpressionImplTest, ExpressionSizeLimit) { + options_.expression_size_codepoint_limit = 10; + auto result = Parse("...............", "", options_); EXPECT_THAT(result, Not(IsOk())); EXPECT_EQ( result.status().message(), "expression size exceeds codepoint limit. input size: 15, limit: 10"); } -TEST(ExpressionTest, RecursionDepthLongArgList) { - ParserOptions options; +TEST_P(ExpressionImplTest, RecursionDepthLongArgList) { // The particular number here is an implementation detail: the underlying // visitor will recurse up to 8 times before branching to the create list or // const steps. The call graph looks something like: @@ -1615,9 +1827,9 @@ TEST(ExpressionTest, RecursionDepthLongArgList) { // ->visitCreateList->visit[arg]->visitExpr... // The expected max depth for create list with an arbitrary number of elements // is 15. - options.max_recursion_depth = 16; + options_.max_recursion_depth = 16; - EXPECT_THAT(Parse("[1, 2, 3, 4, 5, 6, 7, 8, 9, 10]", "", options), IsOk()); + EXPECT_THAT(Parse("[1, 2, 3, 4, 5, 6, 7, 8, 9, 10]", "", options_), IsOk()); } TEST(ExpressionTest, RecursionDepthExceeded) { @@ -1634,10 +1846,9 @@ TEST(ExpressionTest, RecursionDepthExceeded) { HasSubstr("Exceeded max recursion depth of 6 when parsing.")); } -TEST(ExpressionTest, DisableQuotedIdentifiers) { - ParserOptions options; - options.enable_quoted_identifiers = false; - auto result = Parse("foo.`bar`", "", options); +TEST_P(ExpressionImplTest, DisableQuotedIdentifiers) { + options_.enable_quoted_identifiers = false; + auto result = Parse("foo.`bar`", "", options_); EXPECT_THAT(result, Not(IsOk())); EXPECT_THAT(result.status().message(), @@ -1646,11 +1857,10 @@ TEST(ExpressionTest, DisableQuotedIdentifiers) { " | ....^")); } -TEST(ExpressionTest, DisableStandardMacros) { - ParserOptions options; - options.disable_standard_macros = true; +TEST_P(ExpressionImplTest, DisableStandardMacros) { + options_.disable_standard_macros = true; - auto result = Parse("has(foo.bar)", "", options); + auto result = Parse("has(foo.bar)", "", options_); ASSERT_THAT(result, IsOk()); KindAndIdAdorner kind_and_id_adorner; @@ -1671,8 +1881,15 @@ TEST(ExpressionTest, RecursionDepthIgnoresParentheses) { EXPECT_THAT(result, IsOk()); } -TEST(NewParserBuilderTest, Defaults) { - auto builder = cel::NewParserBuilder(); +class NewParserBuilderTest : public testing::TestWithParam { + protected: + NewParserBuilderTest() { options_.enable_pratt_parser = GetParam(); } + + ParserOptions options_; +}; + +TEST_P(NewParserBuilderTest, Defaults) { + auto builder = cel::NewParserBuilder(options_); ASSERT_OK_AND_ASSIGN(auto parser, std::move(*builder).Build()); ASSERT_OK_AND_ASSIGN(auto source, @@ -1682,8 +1899,8 @@ TEST(NewParserBuilderTest, Defaults) { EXPECT_FALSE(ast->IsChecked()); } -TEST(NewParserBuilderTest, CustomMacros) { - auto builder = cel::NewParserBuilder(); +TEST_P(NewParserBuilderTest, CustomMacros) { + auto builder = cel::NewParserBuilder(options_); builder->GetOptions().disable_standard_macros = true; ASSERT_THAT(builder->AddMacro(cel::HasMacro()), IsOk()); ASSERT_OK_AND_ASSIGN(auto parser, std::move(*builder).Build()); @@ -1705,8 +1922,8 @@ TEST(NewParserBuilderTest, CustomMacros) { ")^#9:Expr.Call#"); } -TEST(NewParserBuilderTest, StandardMacrosNotAddedWithStdlib) { - auto builder = cel::NewParserBuilder(); +TEST_P(NewParserBuilderTest, StandardMacrosNotAddedWithStdlib) { + auto builder = cel::NewParserBuilder(options_); builder->GetOptions().disable_standard_macros = false; // Add a fake stdlib to check that we don't try to add the standard macros // again. Emulates what happens when we add support for subsetting stdlib by @@ -1735,15 +1952,15 @@ TEST(NewParserBuilderTest, StandardMacrosNotAddedWithStdlib) { ")^#9:Expr.Call#"); } -TEST(NewParserBuilderTest, ForwardsOptions) { - auto builder = cel::NewParserBuilder(); +TEST_P(NewParserBuilderTest, ForwardsOptions) { + auto builder = cel::NewParserBuilder(options_); builder->GetOptions().enable_optional_syntax = true; ASSERT_OK_AND_ASSIGN(auto parser, std::move(*builder).Build()); ASSERT_OK_AND_ASSIGN(auto source, cel::NewSource("a.?b")); ASSERT_OK_AND_ASSIGN(auto ast, parser->Parse(*source)); EXPECT_FALSE(ast->IsChecked()); - builder = cel::NewParserBuilder(); + builder = cel::NewParserBuilder(options_); builder->GetOptions().enable_optional_syntax = false; ASSERT_OK_AND_ASSIGN(parser, std::move(*builder).Build()); ASSERT_OK_AND_ASSIGN(source, cel::NewSource("a.?b")); @@ -1751,8 +1968,8 @@ TEST(NewParserBuilderTest, ForwardsOptions) { StatusIs(absl::StatusCode::kInvalidArgument)); } -TEST(NewParserBuilderTest, ToBuilderCopiesConfig) { - auto builder = cel::NewParserBuilder(); +TEST_P(NewParserBuilderTest, ToBuilderCopiesConfig) { + auto builder = cel::NewParserBuilder(options_); builder->GetOptions().enable_optional_syntax = true; builder->GetOptions().disable_standard_macros = true; ASSERT_THAT(builder->AddLibrary({"custom_lib", @@ -1773,8 +1990,8 @@ TEST(NewParserBuilderTest, ToBuilderCopiesConfig) { EXPECT_FALSE(ast->IsChecked()); } -TEST(NewParserBuilderTest, ToBuilderHandlesStdlibAndOptionalByLibrary) { - auto builder = cel::NewParserBuilder(); +TEST_P(NewParserBuilderTest, ToBuilderHandlesStdlibAndOptionalByLibrary) { + auto builder = cel::NewParserBuilder(options_); builder->GetOptions().disable_standard_macros = true; builder->GetOptions().enable_optional_syntax = false; @@ -1809,8 +2026,8 @@ TEST(NewParserBuilderTest, ToBuilderHandlesStdlibAndOptionalByLibrary) { ")^#1:Expr.Call#"); } -TEST(NewParserBuilderTest, ToBuilderPreservesStdlibAndOptionalFromOptions) { - auto builder = cel::NewParserBuilder(); +TEST_P(NewParserBuilderTest, ToBuilderPreservesStdlibAndOptionalFromOptions) { + auto builder = cel::NewParserBuilder(options_); builder->GetOptions().disable_standard_macros = false; builder->GetOptions().enable_optional_syntax = true; @@ -1832,11 +2049,19 @@ struct VariadicLogicalOperatorsTestCase { }; class VariadicLogicalOperatorsTest - : public testing::TestWithParam {}; + : public testing::TestWithParam< + std::tuple> { + protected: + VariadicLogicalOperatorsTest() { + options_.enable_pratt_parser = std::get<1>(GetParam()); + } + + ParserOptions options_; +}; TEST_P(VariadicLogicalOperatorsTest, Parse) { - const auto& test_case = GetParam(); - auto builder = cel::NewParserBuilder(); + const auto& test_case = std::get<0>(GetParam()); + auto builder = cel::NewParserBuilder(options_); builder->GetOptions().enable_variadic_logical_operators = true; ASSERT_OK_AND_ASSIGN(auto parser, std::move(*builder).Build()); ASSERT_OK_AND_ASSIGN(auto source, cel::NewSource(test_case.input)); @@ -1848,39 +2073,56 @@ TEST_P(VariadicLogicalOperatorsTest, Parse) { EXPECT_EQ(adorned_string, test_case.expected_adorned_string); } +std::string VariadicLogicalOperatorsTestName( + const testing::TestParamInfo< + std::tuple>& test_info) { + bool enable_pratt = std::get<1>(test_info.param); + return absl::StrCat(test_info.index, "_", enable_pratt ? "Pratt" : "Legacy"); +} + INSTANTIATE_TEST_SUITE_P( VariadicLogicalOperators, VariadicLogicalOperatorsTest, - testing::Values( - VariadicLogicalOperatorsTestCase{ - .input = "a && b && c && d", - .expected_adorned_string = "_&&_(\n" - " a^#1:Expr.Ident#,\n" - " b^#2:Expr.Ident#,\n" - " c^#4:Expr.Ident#,\n" - " d^#6:Expr.Ident#\n" - ")^#3:Expr.Call#"}, - VariadicLogicalOperatorsTestCase{ - .input = "a || b || c || d", - .expected_adorned_string = "_||_(\n" - " a^#1:Expr.Ident#,\n" - " b^#2:Expr.Ident#,\n" - " c^#4:Expr.Ident#,\n" - " d^#6:Expr.Ident#\n" - ")^#3:Expr.Call#"}, - VariadicLogicalOperatorsTestCase{ - .input = "a && b && (c || d || e)", - .expected_adorned_string = "_&&_(\n" - " a^#1:Expr.Ident#,\n" - " b^#2:Expr.Ident#,\n" - " _||_(\n" - " c^#4:Expr.Ident#,\n" - " d^#5:Expr.Ident#,\n" - " e^#7:Expr.Ident#\n" - " )^#6:Expr.Call#\n" - ")^#3:Expr.Call#"})); - -TEST(ParserTest, ParseFailurePopulatesIssues) { - auto builder = cel::NewParserBuilder(); + testing::Combine( + testing::Values( + VariadicLogicalOperatorsTestCase{ + .input = "a && b && c && d", + .expected_adorned_string = "_&&_(\n" + " a^#1:Expr.Ident#,\n" + " b^#2:Expr.Ident#,\n" + " c^#4:Expr.Ident#,\n" + " d^#6:Expr.Ident#\n" + ")^#3:Expr.Call#"}, + VariadicLogicalOperatorsTestCase{ + .input = "a || b || c || d", + .expected_adorned_string = "_||_(\n" + " a^#1:Expr.Ident#,\n" + " b^#2:Expr.Ident#,\n" + " c^#4:Expr.Ident#,\n" + " d^#6:Expr.Ident#\n" + ")^#3:Expr.Call#"}, + VariadicLogicalOperatorsTestCase{ + .input = "a && b && (c || d || e)", + .expected_adorned_string = "_&&_(\n" + " a^#1:Expr.Ident#,\n" + " b^#2:Expr.Ident#,\n" + " _||_(\n" + " c^#4:Expr.Ident#,\n" + " d^#5:Expr.Ident#,\n" + " e^#7:Expr.Ident#\n" + " )^#6:Expr.Call#\n" + ")^#3:Expr.Call#"}), + testing::Bool()), + VariadicLogicalOperatorsTestName); + +class ParserTest : public testing::TestWithParam { + protected: + ParserTest() { options_.enable_pratt_parser = GetParam(); } + + ParserOptions options_; +}; + +TEST_P(ParserTest, ParseFailurePopulatesIssues) { + auto builder = cel::NewParserBuilder(options_); ASSERT_OK_AND_ASSIGN(auto parser, std::move(*builder).Build()); ASSERT_OK_AND_ASSIGN(auto source, cel::NewSource("a +", "test.cel")); @@ -1898,15 +2140,33 @@ TEST(ParserTest, ParseFailurePopulatesIssues) { EXPECT_EQ(issues[0].location().column, 3); } -std::string TestName(const testing::TestParamInfo& test_info) { - std::string name = absl::StrCat(test_info.index, "-", test_info.param.I); +std::string ExpressionTestName( + const testing::TestParamInfo>& test_info) { + const TestInfo& info = std::get<0>(test_info.param); + bool enable_pratt = std::get<1>(test_info.param); + std::string name = absl::StrCat(test_info.index, "_", + enable_pratt ? "Pratt_" : "Legacy_", info.I); absl::c_replace_if(name, [](char c) { return !absl::ascii_isalnum(c); }, '_'); return name; - return name; } INSTANTIATE_TEST_SUITE_P(CelParserTest, ExpressionTest, - testing::ValuesIn(test_cases), TestName); + testing::Combine(testing::ValuesIn(test_cases), + testing::Bool()), + ExpressionTestName); + +std::string ParserImplTestName(const testing::TestParamInfo& info) { + return info.param ? "Pratt" : "Legacy"; +} + +INSTANTIATE_TEST_SUITE_P(CelParserTest, ExpressionImplTest, testing::Bool(), + ParserImplTestName); + +INSTANTIATE_TEST_SUITE_P(CelParserTest, NewParserBuilderTest, testing::Bool(), + ParserImplTestName); + +INSTANTIATE_TEST_SUITE_P(CelParserTest, ParserTest, testing::Bool(), + ParserImplTestName); } // namespace } // namespace google::api::expr::parser