diff --git a/parser/internal/BUILD b/parser/internal/BUILD index 535a1bb79..e5593a9e2 100644 --- a/parser/internal/BUILD +++ b/parser/internal/BUILD @@ -26,6 +26,7 @@ cc_library( deps = [ "@com_google_absl//absl/functional:function_ref", "@com_google_absl//absl/status:statusor", + "@com_google_absl//absl/types:span", ], ) @@ -38,10 +39,15 @@ cc_library( "//common:expr", "//common:expr_factory", "//internal:status_macros", + "//parser:macro", + "//parser:macro_expr_factory", + "//parser:macro_registry", + "@com_google_absl//absl/base:nullability", "@com_google_absl//absl/functional:function_ref", "@com_google_absl//absl/status", "@com_google_absl//absl/status:statusor", "@com_google_absl//absl/strings:string_view", + "@com_google_absl//absl/types:span", ], ) @@ -86,6 +92,7 @@ cc_library( "//parser:options", "//parser:parser_interface", "@com_google_absl//absl/base:nullability", + "@com_google_absl//absl/cleanup", "@com_google_absl//absl/container:flat_hash_map", "@com_google_absl//absl/status:statusor", "@com_google_absl//absl/strings", @@ -127,12 +134,17 @@ cc_test( srcs = ["ast_factory_test.cc"], deps = [ ":ast_factory", + ":ast_factory_interface", "//common:constant", "//common:expr", "//internal:testing", + "//parser:macro", + "//parser:macro_expr_factory", + "//parser:macro_registry", "@com_google_absl//absl/status", "@com_google_absl//absl/status:status_matchers", "@com_google_absl//absl/strings:string_view", + "@com_google_absl//absl/types:span", ], ) @@ -159,6 +171,8 @@ cc_test( "//common:source", "//internal:status_macros", "//internal:testing", + "//parser:macro", + "//parser:macro_expr_factory", "//parser:options", "//parser:parser_interface", "//testutil:expr_printer", @@ -169,6 +183,7 @@ cc_test( "@com_google_absl//absl/strings", "@com_google_absl//absl/strings:str_format", "@com_google_absl//absl/strings:string_view", + "@com_google_absl//absl/types:span", ], ) diff --git a/parser/internal/ast_factory.cc b/parser/internal/ast_factory.cc index b339f06b4..0c9a7d661 100644 --- a/parser/internal/ast_factory.cc +++ b/parser/internal/ast_factory.cc @@ -14,6 +14,7 @@ #include "parser/internal/ast_factory.h" +#include #include #include #include @@ -25,6 +26,8 @@ #include "absl/strings/string_view.h" #include "common/expr.h" #include "internal/status_macros.h" +#include "parser/internal/ast_factory_interface.h" +#include "parser/macro.h" namespace cel::parser_internal { @@ -266,4 +269,19 @@ MapNodeBuilder AstFactoryInterface::NewMapBuilder( return MapNodeBuilder(id); } +std::optional> +AstFactoryInterface::NewMacroExprExpander(std::string_view name, + size_t arg_count, + bool receiver_style) { + if (macro_registry_ == nullptr) { + return std::nullopt; + } + std::optional macro = + macro_registry_->FindMacro(name, arg_count, receiver_style); + if (!macro) { + return std::nullopt; + } + return std::optional>(std::in_place, *macro); +} + } // namespace cel::parser_internal diff --git a/parser/internal/ast_factory.h b/parser/internal/ast_factory.h index 2736b02ec..6240ce527 100644 --- a/parser/internal/ast_factory.h +++ b/parser/internal/ast_factory.h @@ -15,16 +15,24 @@ #ifndef THIRD_PARTY_CEL_CPP_PARSER_INTERNAL_AST_FACTORY_H_ #define THIRD_PARTY_CEL_CPP_PARSER_INTERNAL_AST_FACTORY_H_ +#include #include +#include #include #include +#include +#include "absl/base/nullability.h" #include "absl/functional/function_ref.h" #include "absl/status/statusor.h" #include "absl/strings/string_view.h" +#include "absl/types/span.h" #include "common/expr.h" #include "common/expr_factory.h" #include "parser/internal/ast_factory_interface.h" +#include "parser/macro.h" +#include "parser/macro_expr_factory.h" +#include "parser/macro_registry.h" namespace cel::parser_internal { @@ -71,10 +79,35 @@ class StructNodeBuilder { cel::Expr expr_; }; +template <> +class AstFactoryInterface; + +template <> +class MacroExprExpanderSupport : public cel::MacroExprFactory {}; + +template <> +class MacroExprExpander { + public: + explicit MacroExprExpander(cel::Macro macro) : macro_(std::move(macro)) {} + + std::optional Expand( + std::optional> target, + absl::Span args, + MacroExprExpanderSupport& support) { + return macro_.Expand(support, target, args); + } + + private: + cel::Macro macro_; +}; + template <> class AstFactoryInterface : public cel::ExprFactory { public: - AstFactoryInterface() = default; + explicit AstFactoryInterface( + const cel::MacroRegistry* absl_nullable macro_registry = nullptr) + : macro_registry_(macro_registry) {} + AstFactoryInterface(const AstFactoryInterface&) = delete; AstFactoryInterface(AstFactoryInterface&&) = delete; AstFactoryInterface& operator=(const AstFactoryInterface&) = delete; @@ -126,6 +159,12 @@ class AstFactoryInterface : public cel::ExprFactory { StructNodeBuilder NewStructBuilder(int64_t id, std::string name); MapNodeBuilder NewMapBuilder(int64_t id); + + std::optional> NewMacroExprExpander( + std::string_view name, size_t arg_count, bool receiver_style); + + private: + const cel::MacroRegistry* absl_nullable macro_registry_ = nullptr; }; using AstFactory = AstFactoryInterface; diff --git a/parser/internal/ast_factory_interface.h b/parser/internal/ast_factory_interface.h index c656da22d..dbfa02a0f 100644 --- a/parser/internal/ast_factory_interface.h +++ b/parser/internal/ast_factory_interface.h @@ -15,7 +15,9 @@ #ifndef THIRD_PARTY_CEL_CPP_PARSER_INTERNAL_AST_FACTORY_INTERFACE_H_ #define THIRD_PARTY_CEL_CPP_PARSER_INTERNAL_AST_FACTORY_INTERFACE_H_ +#include #include +#include #include #include #include @@ -23,6 +25,7 @@ #include "absl/functional/function_ref.h" #include "absl/status/statusor.h" +#include "absl/types/span.h" namespace cel::parser_internal { @@ -49,6 +52,17 @@ class StructNodeBuilder { ExprNode Build(); }; +template +class MacroExprExpanderSupport {}; + +template +class MacroExprExpander { + public: + std::optional Expand( + std::optional> target, + absl::Span args, MacroExprExpanderSupport& support); +}; + // Interface for decoupling parser logic from the underlying AST node // data structures. // @@ -104,6 +118,11 @@ class AstFactoryInterface { ListNodeBuilder NewListBuilder(int64_t id); MapNodeBuilder NewMapBuilder(int64_t id); StructNodeBuilder NewStructBuilder(int64_t id, std::string name); + + // Returns a macro expander for the given macro name, or null if there + // is no registered macro with that name and argument count. + std::optional> NewMacroExprExpander( + std::string_view name, size_t arg_count, bool receiver_style); }; } // namespace cel::parser_internal diff --git a/parser/internal/ast_factory_test.cc b/parser/internal/ast_factory_test.cc index 3ef08825e..3e80a4a17 100644 --- a/parser/internal/ast_factory_test.cc +++ b/parser/internal/ast_factory_test.cc @@ -22,13 +22,20 @@ #include "absl/status/status.h" #include "absl/status/status_matchers.h" #include "absl/strings/string_view.h" +#include "absl/types/span.h" #include "common/constant.h" #include "common/expr.h" #include "internal/testing.h" +#include "parser/internal/ast_factory_interface.h" +#include "parser/macro.h" +#include "parser/macro_expr_factory.h" +#include "parser/macro_registry.h" namespace cel::parser_internal { namespace { +using ::absl_testing::IsOk; + using ::absl_testing::StatusIs; TEST(AstFactoryInterfaceTest, AstFactoryUnspecified) { @@ -393,5 +400,50 @@ TEST(AstFactoryInterfaceTest, CopyAndReplaceMaxRecursionDepth) { StatusIs(absl::StatusCode::kInvalidArgument)); } +class TestMacroExprExpanderSupport + : public MacroExprExpanderSupport { + 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 { + return cel::Expr(); + } +}; + +TEST(AstFactoryInterfaceTest, MacroExprExpander) { + MacroRegistry macro_registry; + AstFactory factory(¯o_registry); + ASSERT_OK_AND_ASSIGN( + auto foo_macro, + Macro::Global("foo", 1, + [](MacroExprFactory& macro_factory, + absl::Span args) -> std::optional { + return macro_factory.NewCall("my_macro", std::move(args)); + })); + + ASSERT_THAT(macro_registry.RegisterMacro(foo_macro), IsOk()); + + auto expander1 = factory.NewMacroExprExpander("foo", 1, false); + ASSERT_TRUE(expander1.has_value()); + + std::vector expand_args; + expand_args.push_back(factory.NewIdent(1, "x")); + + TestMacroExprExpanderSupport support; + auto result = + expander1->Expand(std::nullopt, absl::MakeSpan(expand_args), support); + ASSERT_TRUE(result.has_value()); + + std::vector expected_args; + expected_args.push_back(factory.NewIdent(1, "x")); + Expr expected = factory.NewCall(42, "my_macro", std::move(expected_args)); + + EXPECT_EQ(*result, expected); + + auto expander2 = factory.NewMacroExprExpander("bar", 1, false); + EXPECT_FALSE(expander2.has_value()); +} + } // namespace } // namespace cel::parser_internal diff --git a/parser/internal/pratt_parser.cc b/parser/internal/pratt_parser.cc index f86974a36..c321fa7b8 100644 --- a/parser/internal/pratt_parser.cc +++ b/parser/internal/pratt_parser.cc @@ -65,104 +65,91 @@ std::string FormatIssues(const cel::Source& source, }); } -class PrattParserBuilderImpl final : public cel::ParserBuilder { - public: - explicit PrattParserBuilderImpl(const cel::ParserOptions& options) - : options_(options) {} - - cel::ParserOptions& GetOptions() override { return options_; } - - absl::Status AddMacro(const cel::Macro& macro) override { - for (const cel::Macro& existing_macro : macros_) { - if (existing_macro.key() == macro.key()) { - return absl::AlreadyExistsError( - absl::StrCat("macro already exists: ", macro.key())); - } - } - macros_.push_back(macro); - return absl::OkStatus(); - } +} // namespace - absl::Status AddLibrary(cel::ParserLibrary library) override { - if (!library.id.empty()) { - auto [it, inserted] = library_ids_.insert(library.id); - if (!inserted) { - return absl::AlreadyExistsError( - absl::StrCat("parser library already exists: ", library.id)); - } +absl::Status PrattParserBuilderImpl::AddMacro(const cel::Macro& macro) { + for (const cel::Macro& existing_macro : macros_) { + if (existing_macro.key() == macro.key()) { + return absl::AlreadyExistsError( + absl::StrCat("macro already exists: ", macro.key())); } - libraries_.push_back(std::move(library)); - return absl::OkStatus(); } + macros_.push_back(macro); + return absl::OkStatus(); +} - absl::Status AddLibrarySubset(cel::ParserLibrarySubset subset) override { - if (subset.library_id.empty()) { - return absl::InvalidArgumentError("subset must have a library id"); - } - std::string library_id = subset.library_id; - auto [it, inserted] = - library_subsets_.insert({library_id, std::move(subset)}); +absl::Status PrattParserBuilderImpl::AddLibrary(cel::ParserLibrary library) { + if (!library.id.empty()) { + auto [it, inserted] = library_ids_.insert(library.id); if (!inserted) { return absl::AlreadyExistsError( - absl::StrCat("parser library subset already exists: ", library_id)); + absl::StrCat("parser library already exists: ", library.id)); } - return absl::OkStatus(); } + libraries_.push_back(std::move(library)); + return absl::OkStatus(); +} + +absl::Status PrattParserBuilderImpl::AddLibrarySubset( + cel::ParserLibrarySubset subset) { + if (subset.library_id.empty()) { + return absl::InvalidArgumentError("subset must have a library id"); + } + std::string library_id = subset.library_id; + auto [it, inserted] = + library_subsets_.insert({library_id, std::move(subset)}); + if (!inserted) { + return absl::AlreadyExistsError( + absl::StrCat("parser library subset already exists: ", library_id)); + } + return absl::OkStatus(); +} - absl::StatusOr> Build() override { - using std::swap; - std::vector individual_macros; - swap(individual_macros, macros_); - absl::Cleanup cleanup([&] { swap(macros_, individual_macros); }); - - cel::MacroRegistry macro_registry; - - for (const cel::ParserLibrary& library : libraries_) { - CEL_RETURN_IF_ERROR(library.configure(*this)); - if (!library.id.empty()) { - auto it = library_subsets_.find(library.id); - if (it != library_subsets_.end()) { - const cel::ParserLibrarySubset& subset = it->second; - for (const cel::Macro& macro : macros_) { - if (subset.should_include_macro(macro)) { - CEL_RETURN_IF_ERROR(macro_registry.RegisterMacro(macro)); - } +absl::StatusOr> PrattParserBuilderImpl::Build() { + using std::swap; + std::vector individual_macros; + swap(individual_macros, macros_); + absl::Cleanup cleanup([&] { swap(macros_, individual_macros); }); + + cel::MacroRegistry macro_registry; + + for (const cel::ParserLibrary& library : libraries_) { + CEL_RETURN_IF_ERROR(library.configure(*this)); + if (!library.id.empty()) { + auto it = library_subsets_.find(library.id); + if (it != library_subsets_.end()) { + const cel::ParserLibrarySubset& subset = it->second; + for (const cel::Macro& macro : macros_) { + if (subset.should_include_macro(macro)) { + CEL_RETURN_IF_ERROR(macro_registry.RegisterMacro(macro)); } - macros_.clear(); - continue; } + macros_.clear(); + continue; } - - CEL_RETURN_IF_ERROR(macro_registry.RegisterMacros(macros_)); - macros_.clear(); } - absl::flat_hash_set library_ids(library_ids_); + CEL_RETURN_IF_ERROR(macro_registry.RegisterMacros(macros_)); + macros_.clear(); + } - if (!options_.disable_standard_macros && !library_ids_.contains("stdlib")) { - CEL_RETURN_IF_ERROR(macro_registry.RegisterMacros(Macro::AllMacros())); - library_ids.insert("stdlib"); - } + absl::flat_hash_set library_ids(library_ids_); - if (options_.enable_optional_syntax && !library_ids_.contains("optional")) { - CEL_RETURN_IF_ERROR(macro_registry.RegisterMacro(cel::OptMapMacro())); - CEL_RETURN_IF_ERROR(macro_registry.RegisterMacro(cel::OptFlatMapMacro())); - library_ids.insert("optional"); - } - - CEL_RETURN_IF_ERROR(macro_registry.RegisterMacros(individual_macros)); - return std::make_unique( - options_, std::move(macro_registry), std::move(library_ids)); + if (!options_.disable_standard_macros && !library_ids_.contains("stdlib")) { + CEL_RETURN_IF_ERROR(macro_registry.RegisterMacros(Macro::AllMacros())); + library_ids.insert("stdlib"); } - cel::ParserOptions options_; - std::vector macros_; - absl::flat_hash_set library_ids_; - std::vector libraries_; - absl::flat_hash_map library_subsets_; -}; + if (options_.enable_optional_syntax && !library_ids_.contains("optional")) { + CEL_RETURN_IF_ERROR(macro_registry.RegisterMacro(cel::OptMapMacro())); + CEL_RETURN_IF_ERROR(macro_registry.RegisterMacro(cel::OptFlatMapMacro())); + library_ids.insert("optional"); + } -} // namespace + CEL_RETURN_IF_ERROR(macro_registry.RegisterMacros(individual_macros)); + return std::make_unique(options_, std::move(macro_registry), + std::move(library_ids)); +} template class PrattParserWorker; @@ -175,7 +162,8 @@ absl::StatusOr> PrattParserImpl::ParseImpl( source.content().size(), options_.expression_size_codepoint_limit)); } std::vector issues; - PrattParserWorker worker(source, options_, &issues); + AstFactory factory(¯o_registry_); + PrattParserWorker worker(source, options_, &issues, factory); Expr expr = worker.Parse(); if (worker.is_recursion_limit_exceeded()) { return absl::CancelledError( @@ -195,10 +183,11 @@ absl::StatusOr> PrattParserImpl::ParseImpl( for (const auto& [id, pos] : worker.GetNodePositions()) { source_info.mutable_positions().insert({id, pos}); } - source_info.mutable_line_offsets().reserve(worker.GetLineOffsets().size()); - for (int32_t offset : worker.GetLineOffsets()) { + source_info.mutable_line_offsets().reserve(source.line_offsets().size()); + for (int32_t offset : source.line_offsets()) { source_info.mutable_line_offsets().push_back(offset); } + source_info.mutable_macro_calls() = worker.ReleaseMacroCalls(); return std::make_unique(std::move(expr), std::move(source_info)); } @@ -209,9 +198,4 @@ std::unique_ptr PrattParserImpl::ToBuilder() const { return ins; } -std::unique_ptr NewPrattParserBuilder( - const cel::ParserOptions& options) { - return std::make_unique(options); -} - } // namespace cel::parser_internal diff --git a/parser/internal/pratt_parser.h b/parser/internal/pratt_parser.h index c39c68521..21e9878f3 100644 --- a/parser/internal/pratt_parser.h +++ b/parser/internal/pratt_parser.h @@ -21,10 +21,13 @@ #include #include "absl/base/nullability.h" +#include "absl/container/flat_hash_map.h" #include "absl/container/flat_hash_set.h" +#include "absl/status/status.h" #include "absl/status/statusor.h" #include "common/ast.h" #include "common/source.h" +#include "parser/macro.h" #include "parser/macro_registry.h" #include "parser/options.h" #include "parser/parser_interface.h" @@ -64,8 +67,35 @@ class PrattParserImpl final : public cel::Parser { absl::flat_hash_set library_ids_; }; -std::unique_ptr NewPrattParserBuilder( - const cel::ParserOptions& options = cel::ParserOptions()); +class PrattParserBuilderImpl final : public cel::ParserBuilder { + public: + explicit PrattParserBuilderImpl(const cel::ParserOptions& options) + : options_(options) {} + + cel::ParserOptions& GetOptions() override { return options_; } + + absl::Status AddMacro(const cel::Macro& macro) override; + + absl::Status AddLibrary(cel::ParserLibrary library) override; + + absl::Status AddLibrarySubset(cel::ParserLibrarySubset subset) override; + + absl::StatusOr> Build() override; + + private: + friend class PrattParserImpl; + + cel::ParserOptions options_; + std::vector macros_; + absl::flat_hash_set library_ids_; + std::vector libraries_; + absl::flat_hash_map library_subsets_; +}; + +inline std::unique_ptr NewPrattParserBuilder( + const cel::ParserOptions& options = cel::ParserOptions()) { + return std::make_unique(options); +} } // namespace cel::parser_internal diff --git a/parser/internal/pratt_parser_test.cc b/parser/internal/pratt_parser_test.cc index b1dd81fbb..708936a4e 100644 --- a/parser/internal/pratt_parser_test.cc +++ b/parser/internal/pratt_parser_test.cc @@ -15,8 +15,11 @@ #include "parser/internal/pratt_parser.h" #include +#include #include +#include #include +#include #include #include "absl/algorithm/container.h" @@ -29,6 +32,7 @@ #include "absl/strings/str_join.h" #include "absl/strings/str_split.h" #include "absl/strings/string_view.h" +#include "absl/types/span.h" #include "common/ast.h" #include "common/constant.h" #include "common/expr.h" @@ -37,6 +41,8 @@ #include "internal/testing.h" #include "parser/internal/lexer.h" #include "parser/internal/pratt_parser_worker.h" +#include "parser/macro.h" +#include "parser/macro_expr_factory.h" #include "parser/options.h" #include "parser/parser_interface.h" #include "testutil/expr_printer.h" @@ -47,11 +53,19 @@ namespace cel::parser_internal { namespace { +using ::absl_testing::IsOk; using ::absl_testing::IsOkAndHolds; using ::absl_testing::StatusIs; using ::testing::Eq; using ::testing::NotNull; +template +std::string TestName(const testing::TestParamInfo& test_info) { + std::string name = absl::StrCat(test_info.index, "-", test_info.param.source); + absl::c_replace_if(name, [](char c) { return !absl::ascii_isalnum(c); }, '_'); + return name; +} + absl::StatusOr> Parse( std::string_view expression, const cel::ParserOptions& options = cel::ParserOptions(), @@ -76,18 +90,6 @@ struct TestCase { class PrattParserTest : public testing::TestWithParam {}; -std::string Unindent(std::string_view multiline) { - std::vector unindented_lines; - int indent = -1; - for (std::string_view line : absl::StrSplit(multiline, '\n')) { - std::size_t pos = line.find_first_not_of(" \t"); - if (pos == std::string_view::npos) continue; - if (indent == -1) indent = pos; - unindented_lines.push_back(std::string(line.substr(indent))); - } - return absl::StrJoin(unindented_lines, "\n"); -} - std::string_view ConstantKind(const cel::Constant& c) { switch (c.kind_case()) { case ConstantKindCase::kBool: @@ -153,6 +155,31 @@ class KindAndIdAdorner : public cel::test::ExpressionAdorner { } }; +std::string Unindent(std::string_view multiline) { + std::vector unindented_lines; + int indent = -1; + for (std::string_view line : absl::StrSplit(multiline, '\n')) { + std::size_t pos = line.find_first_not_of(" \t"); + if (pos == std::string_view::npos) continue; + if (indent == -1) indent = pos; + unindented_lines.push_back(std::string(line.substr(indent))); + } + return absl::StrJoin(unindented_lines, "\n"); +} + +MATCHER_P(AstIs, expected_ast, "") { + KindAndIdAdorner kind_and_id_adorner; + test::ExprPrinter printer(kind_and_id_adorner); + std::string actual = Unindent(printer.Print(arg)); + std::string expected = Unindent(expected_ast); + if (actual == expected) { + return true; + } + *result_listener << "\n Actual: " << actual + << "\n Expected: " << expected; + return false; +} + TEST_P(PrattParserTest, Parse) { const TestCase& test_case = GetParam(); cel::ParserOptions options; @@ -161,10 +188,7 @@ TEST_P(PrattParserTest, Parse) { test_case.enable_variadic_logical_operators; ASSERT_OK_AND_ASSIGN(std::unique_ptr ast, Parse(test_case.source, options)); - const Expr& root = ast->root_expr(); - KindAndIdAdorner kind_and_id_adorner; - test::ExprPrinter printer(kind_and_id_adorner); - EXPECT_EQ(Unindent(printer.Print(root)), Unindent(test_case.expected_ast)); + EXPECT_THAT(ast->root_expr(), AstIs(test_case.expected_ast)); } std::vector GetParserTestCases() { @@ -1012,14 +1036,9 @@ std::vector GetParserTestCases() { }; } -std::string TestName(const testing::TestParamInfo& test_info) { - std::string name = absl::StrCat(test_info.index, "-", test_info.param.source); - absl::c_replace_if(name, [](char c) { return !absl::ascii_isalnum(c); }, '_'); - return name; -} - INSTANTIATE_TEST_SUITE_P(PrattParserTest, PrattParserTest, - testing::ValuesIn(GetParserTestCases()), TestName); + testing::ValuesIn(GetParserTestCases()), + TestName); struct ErrorTestCase { std::string_view source; @@ -1375,7 +1394,8 @@ std::vector GetErrorTestCases() { } INSTANTIATE_TEST_SUITE_P(PrattParserErrorTest, PrattParserErrorTest, - testing::ValuesIn(GetErrorTestCases())); + testing::ValuesIn(GetErrorTestCases()), + TestName); TEST(PrattParserTest, SourceInfoPositionsPopulated) { ASSERT_OK_AND_ASSIGN(std::unique_ptr ast, Parse("a + b")); @@ -1430,5 +1450,292 @@ TEST(ParserWorkerTest, GetTokenTextBoundsChecking) { ""); } +struct MacroTestCase { + std::string_view source; + std::string_view expected_ast; +}; + +class PrattParserMacroTest : public testing::TestWithParam {}; + +TEST_P(PrattParserMacroTest, MacroExprExpander) { + const MacroTestCase& test_case = GetParam(); + auto builder = NewPrattParserBuilder(); + ASSERT_OK_AND_ASSIGN( + auto global_macro, + Macro::Global("foo", 1, + [](MacroExprFactory& macro_factory, + absl::Span args) -> std::optional { + return macro_factory.NewCall("my_macro", std::move(args)); + })); + + ASSERT_OK_AND_ASSIGN( + auto receiver_macro, + Macro::Receiver("bar", 2, + [](MacroExprFactory& macro_factory, Expr& target, + absl::Span args) -> std::optional { + return macro_factory.NewMemberCall( + "my_bar", std::move(target), std::move(args)); + })); + + ASSERT_THAT(builder->AddMacro(global_macro), IsOk()); + ASSERT_THAT(builder->AddMacro(receiver_macro), IsOk()); + ASSERT_OK_AND_ASSIGN(auto parser, builder->Build()); + + ASSERT_OK_AND_ASSIGN(auto source, cel::NewSource(test_case.source)); + ASSERT_OK_AND_ASSIGN(auto ast, parser->Parse(*source)); + + EXPECT_THAT(ast->root_expr(), AstIs(test_case.expected_ast)); +} + +std::vector GetMacroTestCases() { + return { + MacroTestCase{ + .source = "foo(x)", + .expected_ast = R"( + my_macro( + x^#2:Expr.Ident# + )^#3:Expr.Call# + )", + }, + MacroTestCase{ + .source = "x.bar(y, z)", + .expected_ast = R"( + x^#1:Expr.Ident#.my_bar( + y^#3:Expr.Ident#, + z^#4:Expr.Ident# + )^#5:Expr.Call# + )", + }, + // Number of args doesn't match the macro definition + MacroTestCase{ + .source = "foo(x, y)", + .expected_ast = R"( + foo( + x^#2:Expr.Ident#, + y^#3:Expr.Ident# + )^#1:Expr.Call# + )", + }, + // Number of args doesn't match the macro definition + MacroTestCase{ + .source = "x.bar(y)", + .expected_ast = R"( + x^#1:Expr.Ident#.bar( + y^#3:Expr.Ident# + )^#2:Expr.Call# + )", + }, + // No target provided for receiver macro + MacroTestCase{ + .source = "bar(x, y)", + .expected_ast = R"( + bar( + x^#2:Expr.Ident#, + y^#3:Expr.Ident# + )^#1:Expr.Call# + )", + }, + // Target provided for global macro + MacroTestCase{ + .source = "x.foo(y)", + .expected_ast = R"( + x^#1:Expr.Ident#.foo( + y^#3:Expr.Ident# + )^#2:Expr.Call# + )", + }, + // Global macro not registered + MacroTestCase{ + .source = "baz(x)", + .expected_ast = R"( + baz( + x^#2:Expr.Ident# + )^#1:Expr.Call# + )", + }, + // Receiver macro not registered + MacroTestCase{ + .source = "x.baz(x)", + .expected_ast = R"( + x^#1:Expr.Ident#.baz( + x^#3:Expr.Ident# + )^#2:Expr.Call# + )", + }, + // has() macro + MacroTestCase{ + .source = "has(message.field)", + .expected_ast = R"( + message^#2:Expr.Ident#.field~test-only~^#4:Expr.Select# + )", + }, + }; +} + +INSTANTIATE_TEST_SUITE_P(PrattParserMacroTest, PrattParserMacroTest, + testing::ValuesIn(GetMacroTestCases()), + TestName); + +TEST(PrattParserMacroErrorTest, ReportError) { + auto builder = NewPrattParserBuilder(); + ASSERT_OK_AND_ASSIGN( + auto error_macro, + Macro::Global("bad_macro", 1, + [](MacroExprFactory& macro_factory, + absl::Span args) -> std::optional { + return macro_factory.ReportError("custom macro error"); + })); + + ASSERT_THAT(builder->AddMacro(error_macro), IsOk()); + ASSERT_OK_AND_ASSIGN(auto parser, builder->Build()); + + ASSERT_OK_AND_ASSIGN(auto source, cel::NewSource("42 + bad_macro(x)")); + std::vector issues; + auto ast = parser->Parse(*source, &issues); + EXPECT_THAT(ast, StatusIs(absl::StatusCode::kInvalidArgument)); + EXPECT_EQ(FormatIssues(*source, issues), + "ERROR: :1:6: custom macro error\n" + " | 42 + bad_macro(x)\n" + " | .....^"); +} + +TEST(PrattParserMacroErrorTest, ReportErrorAt) { + auto builder = NewPrattParserBuilder(); + ASSERT_OK_AND_ASSIGN( + auto error_at_macro, + Macro::Global("bad_macro_at", 1, + [](MacroExprFactory& macro_factory, + absl::Span args) -> std::optional { + return macro_factory.ReportErrorAt(args[0], + "custom error at arg"); + })); + + ASSERT_THAT(builder->AddMacro(error_at_macro), IsOk()); + ASSERT_OK_AND_ASSIGN(auto parser, builder->Build()); + + ASSERT_OK_AND_ASSIGN(auto source, cel::NewSource("bad_macro_at(x)")); + std::vector issues; + auto ast = parser->Parse(*source, &issues); + EXPECT_THAT(ast, StatusIs(absl::StatusCode::kInvalidArgument)); + EXPECT_EQ(FormatIssues(*source, issues), + "ERROR: :1:14: custom error at arg\n" + " | bad_macro_at(x)\n" + " | .............^"); +} + +TEST(PrattParserMacroCallsTest, MacroCallsDisabledByDefault) { + cel::ParserOptions options; + options.add_macro_calls = false; + ASSERT_OK_AND_ASSIGN(auto ast, Parse("has(a.b)", options)); + EXPECT_TRUE(ast->source_info().macro_calls().empty()); +} + +TEST(PrattParserMacroCallsTest, GlobalMacroCallRecorded) { + cel::ParserOptions options; + options.add_macro_calls = true; + ASSERT_OK_AND_ASSIGN(auto ast, Parse("has(a.b)", options)); + + const auto& macro_calls = ast->source_info().macro_calls(); + EXPECT_FALSE(macro_calls.empty()); + EXPECT_TRUE(macro_calls.contains(ast->root_expr().id())); + + const auto& macro_call = macro_calls.at(ast->root_expr().id()); + EXPECT_THAT(macro_call, AstIs(R"( + has( + a^#2:Expr.Ident#.b^#3:Expr.Select# + )^#0:Expr.Call# + )")); +} + +TEST(PrattParserMacroCallsTest, ReceiverMacroCallRecorded) { + cel::ParserOptions options; + options.add_macro_calls = true; + ASSERT_OK_AND_ASSIGN(auto ast, Parse("[1, 2].exists(x, x > 0)", options)); + + const auto& macro_calls = ast->source_info().macro_calls(); + EXPECT_FALSE(macro_calls.empty()); + EXPECT_TRUE(macro_calls.contains(ast->root_expr().id())); + + const auto& exists_macro_call = macro_calls.at(ast->root_expr().id()); + EXPECT_THAT(exists_macro_call, AstIs(R"( + [ + 1^#2:int64#, + 2^#3:int64# + ]^#1:Expr.CreateList#.exists( + x^#5:Expr.Ident#, + _>_( + x^#6:Expr.Ident#, + 0^#8:int64# + )^#7:Expr.Call# + )^#0:Expr.Call# + )")); +} + +TEST(PrattParserMacroCallsTest, NestedMacroCallsUseCopyAndReplaceReplacer) { + cel::ParserOptions options; + options.add_macro_calls = true; + ASSERT_OK_AND_ASSIGN(auto ast, Parse("[1, 2].all(x, has(x.b))", options)); + + const auto& root_expr = ast->root_expr(); + EXPECT_THAT(root_expr, AstIs(R"( + __comprehension__( + // Variable + x, + // Target + [ + 1^#2:int64#, + 2^#3:int64# + ]^#1:Expr.CreateList#, + // Accumulator + @result, + // Init + true^#10:bool#, + // LoopCondition + @not_strictly_false( + @result^#11:Expr.Ident# + )^#12:Expr.Call#, + // LoopStep + _&&_( + @result^#13:Expr.Ident#, + x^#7:Expr.Ident#.b~test-only~^#9:Expr.Select# + )^#14:Expr.Call#, + // Result + @result^#15:Expr.Ident#)^#16:Expr.Comprehension# + )")); + + const auto& macro_calls = ast->source_info().macro_calls(); + // There should be 2 recorded macro calls: one for 'all', one for 'has'. + EXPECT_EQ(macro_calls.size(), 2); + + int64_t all_macro_id = root_expr.id(); + EXPECT_TRUE(macro_calls.contains(all_macro_id)); + + const auto& all_macro_call = macro_calls.at(all_macro_id); + // The second argument of 'all' is the inner 'has(x.b)' call. + // Because 'has(x.b)' was already expanded and recorded in macro_calls + // it is represented as an UnspecifiedExpr holding the inner macro's ID. + EXPECT_THAT(all_macro_call, AstIs(R"( + [ + 1^#2:int64#, + 2^#3:int64# + ]^#1:Expr.CreateList#.all( + x^#5:Expr.Ident#, + ^#9:unspecified_expr# + )^#0:Expr.Call# + )")); + + const auto& has_call = all_macro_call.call_expr().args()[1]; + int64_t has_macro_id = has_call.id(); + EXPECT_EQ(has_macro_id, 9); // ^#9:unspecified_expr# + EXPECT_TRUE(macro_calls.contains(has_macro_id)); + + const auto& has_macro_call = macro_calls.at(has_macro_id); + EXPECT_THAT(has_macro_call, AstIs(R"( + has( + x^#7:Expr.Ident#.b^#8:Expr.Select# + )^#0:Expr.Call# + )")); +} + } // namespace } // namespace cel::parser_internal diff --git a/parser/internal/pratt_parser_worker.cc b/parser/internal/pratt_parser_worker.cc index ff60e7962..4e40adc1e 100644 --- a/parser/internal/pratt_parser_worker.cc +++ b/parser/internal/pratt_parser_worker.cc @@ -16,11 +16,11 @@ #include #include -#include #include #include "absl/base/nullability.h" #include "absl/strings/str_cat.h" +#include "absl/strings/string_view.h" #include "common/source.h" #include "parser/internal/lexer.h" #include "parser/options.h" @@ -70,7 +70,7 @@ Token ParserWorker::NextToken() { return current_token_; } -bool ParserWorker::Expect(TokenType type, std::string_view msg) { +bool ParserWorker::Expect(TokenType type, absl::string_view msg) { if (peek_token_.type == type) { NextToken(); return true; @@ -139,7 +139,7 @@ void ParserWorker::EraseId(int64_t id) { } } -void ParserWorker::ReportError(int32_t position, std::string_view msg) { +void ParserWorker::ReportError(int32_t position, absl::string_view msg) { cel::SourceLocation loc; if (auto found = source_.GetLocation(position); found.has_value()) { loc = *found; @@ -148,7 +148,7 @@ void ParserWorker::ReportError(int32_t position, std::string_view msg) { } void ParserWorker::ReportError(const SourceLocation& loc, - std::string_view msg) { + absl::string_view msg) { error_count_++; if (parse_issues_ != nullptr) { parse_issues_->push_back(cel::ParseIssue(loc, std::string(msg))); diff --git a/parser/internal/pratt_parser_worker.h b/parser/internal/pratt_parser_worker.h index a510f32a3..45221714f 100644 --- a/parser/internal/pratt_parser_worker.h +++ b/parser/internal/pratt_parser_worker.h @@ -17,6 +17,7 @@ #include #include +#include #include #include #include @@ -24,6 +25,7 @@ #include #include "absl/base/nullability.h" +#include "absl/cleanup/cleanup.h" #include "absl/container/flat_hash_map.h" #include "absl/status/statusor.h" #include "absl/strings/ascii.h" @@ -114,16 +116,60 @@ class ParserWorker { template class PrattParserWorker : public ParserWorker { public: + using ParserWorker::NextId; + explicit PrattParserWorker( const cel::Source& source, const cel::ParserOptions& options, - std::vector* absl_nullable parse_issues) - : ParserWorker(source, options, parse_issues) { + std::vector* absl_nullable parse_issues, + AstFactoryInterface& ast_factory) + : ParserWorker(source, options, parse_issues), ast_factory_(ast_factory) { this->InitTokenStream(); } ExprNode Parse(); + absl::flat_hash_map ReleaseMacroCalls() { + return std::move(macro_calls_); + } + private: + class MacroExpanderSupport : public MacroExprExpanderSupport { + public: + MacroExpanderSupport(PrattParserWorker& worker, int32_t macro_position) + : worker_(worker), macro_position_(macro_position) {} + + int64_t NextId() { + return macro_position_ >= 0 ? worker_.NextId(macro_position_) + : worker_.NextId(); + } + + int64_t CopyId(int64_t id) { return worker_.CopyId(id); } + + ExprNode ReportError(absl::string_view message) { + if (macro_position_ >= 0) { + worker_.ReportError(macro_position_, message); + } else { + worker_.ReportError(worker_.current_token_.start, message); + } + return ExprNode(); + } + + ExprNode ReportErrorAt(const ExprNode& expr, absl::string_view message) { + int32_t pos = 0; + auto it = + worker_.GetNodePositions().find(worker_.ast_factory_.GetId(expr)); + if (it != worker_.GetNodePositions().end()) { + pos = it->second; + } + worker_.ReportError(pos, message); + return ExprNode(); + } + + private: + PrattParserWorker& worker_; + int32_t macro_position_; + }; + using CelOperator = ::google::api::expr::common::CelOperator; ExprNode ParseExpr(); @@ -151,7 +197,18 @@ class PrattParserWorker : public ParserWorker { ExprNode BalanceLogical(absl::string_view op, std::vector terms, std::vector ops, bool enable_variadic); - AstFactoryInterface ast_factory_; + std::optional TryExpandMacro(int64_t expr_id, + absl::string_view function, + ExprNode* target, + std::vector& args); + + // Save the original structure of the expression before macro expansion. + void RecordMacroCall(int64_t macro_id, absl::string_view function, + std::optional target, + std::vector arguments); + + AstFactoryInterface& ast_factory_; + absl::flat_hash_map macro_calls_; }; template @@ -172,28 +229,26 @@ ExprNode PrattParserWorker::ParseExpr() { if (recursion_limit_exceeded_) { return ExprNode(); } - if (recursion_depth_ >= options_.max_recursion_depth) { + if (recursion_depth_ > options_.max_recursion_depth) { recursion_limit_exceeded_ = true; return ExprNode(); } recursion_depth_++; + auto recursion_cleanup = absl::MakeCleanup([this]() { recursion_depth_--; }); ExprNode lhs = ParseConditionalOr(); if (peek_token_.type == TokenType::kQuestion) { NextToken(); int64_t op_id = NextId(); ExprNode true_expr = ParseConditionalOr(); if (!Expect(TokenType::kColon, "expected ':' in conditional expression")) { - recursion_depth_--; return lhs; } ExprNode false_expr = ParseExpr(); - recursion_depth_--; return ast_factory_.NewCall( op_id, CelOperator::CONDITIONAL, std::vector{std::move(lhs), std::move(true_expr), std::move(false_expr)}); } - recursion_depth_--; return lhs; } @@ -417,8 +472,14 @@ ExprNode PrattParserWorker::ParseMember() { Token lparen = NextToken(); int64_t call_id = NextId(lparen); std::vector args = ParseArguments(TokenType::kRightParen); - lhs = ast_factory_.NewMemberCall(call_id, id_text, std::move(lhs), - std::move(args)); + if (std::optional expanded = + TryExpandMacro(call_id, id_text, &lhs, args); + expanded.has_value()) { + lhs = std::move(*expanded); + } else { + lhs = ast_factory_.NewMemberCall(call_id, id_text, std::move(lhs), + std::move(args)); + } } else { lhs = ast_factory_.NewSelect(NextId(dot_tok), std::move(lhs), id_text); } @@ -509,7 +570,12 @@ ExprNode PrattParserWorker::ParsePrimary() { if (peek_token_.type == TokenType::kLeftParen) { NextToken(); std::vector args = ParseArguments(TokenType::kRightParen); - expr = ast_factory_.NewCall(id, name, std::move(args)); + if (auto expanded = TryExpandMacro(id, name, nullptr, args); + expanded.has_value()) { + expr = std::move(*expanded); + } else { + expr = ast_factory_.NewCall(id, name, std::move(args)); + } } else { expr = ast_factory_.NewIdent(id, std::move(name)); } @@ -855,6 +921,97 @@ ExprNode PrattParserWorker::BalanceLogical( return BalancedTree(op, terms, ops, 0, ops.size() - 1); } +template +std::optional PrattParserWorker::TryExpandMacro( + int64_t expr_id, absl::string_view function, ExprNode* target, + std::vector& args) { + bool is_receiver = target != nullptr; + auto expander = + ast_factory_.NewMacroExprExpander(function, args.size(), is_receiver); + if (!expander) { + return std::nullopt; + } + + std::vector macro_args; + ExprNode macro_target; + bool add_macro_calls = options_.add_macro_calls; + // We must build the copies of the macro arguments before calling Expand, + // because Expand is allowed to mutate the arguments in-place upon success. + // However, we only record the macro call if Expand actually succeeds. + if (add_macro_calls) { + auto build_macro_call_arg = [&](const ExprNode& expr) -> ExprNode { + absl::StatusOr copy_or = ast_factory_.CopyAndReplace( + expr, + [this](const ExprNode& e) -> std::optional { + if (auto it = macro_calls_.find(ast_factory_.GetId(e)); + it != macro_calls_.end()) { + return ast_factory_.NewUnspecified(ast_factory_.GetId(e)); + } + return std::nullopt; + }, + options_.max_recursion_depth - recursion_depth_); + if (!copy_or.ok()) { + int32_t macro_position = 0; + if (auto it = positions_.find(expr_id); it != positions_.end()) { + macro_position = it->second; + } + ReportError(macro_position, copy_or.status().message()); + return ast_factory_.NewUnspecified(0); + } + return *std::move(copy_or); + }; + macro_args.reserve(args.size()); + for (const auto& arg : args) { + macro_args.push_back(build_macro_call_arg(arg)); + } + if (target != nullptr) { + macro_target = build_macro_call_arg(*target); + } + } + + absl::optional> target_ref; + if (target != nullptr) { + target_ref = *target; + } + + int32_t macro_position = 0; + if (auto it = positions_.find(expr_id); it != positions_.end()) { + macro_position = it->second; + } + MacroExpanderSupport support(*this, macro_position); + std::optional expanded_expr = + expander->Expand(target_ref, absl::MakeSpan(args), support); + + if (expanded_expr) { + if (add_macro_calls) { + RecordMacroCall(ast_factory_.GetId(*expanded_expr), function, + target != nullptr + ? std::make_optional(std::move(macro_target)) + : std::nullopt, + std::move(macro_args)); + } + EraseId(expr_id); + return expanded_expr; + } + + return std::nullopt; +} + +template +void PrattParserWorker::RecordMacroCall( + int64_t macro_id, absl::string_view function, + std::optional target, std::vector arguments) { + ExprNode call_expr; + if (target.has_value()) { + call_expr = ast_factory_.NewMemberCall( + 0, std::string(function), std::move(*target), std::move(arguments)); + } else { + call_expr = + ast_factory_.NewCall(0, std::string(function), std::move(arguments)); + } + macro_calls_.insert({macro_id, std::move(call_expr)}); +} + } // namespace cel::parser_internal #endif // THIRD_PARTY_CEL_CPP_PARSER_INTERNAL_PRATT_PARSER_WORKER_H_ diff --git a/parser/macro_expr_factory.h b/parser/macro_expr_factory.h index c66aa4fe0..75f8d1d74 100644 --- a/parser/macro_expr_factory.h +++ b/parser/macro_expr_factory.h @@ -33,6 +33,11 @@ namespace cel { class ParserMacroExprFactory; class TestMacroExprFactory; +namespace parser_internal { +template +class MacroExprExpanderSupport; +} // namespace parser_internal + // `MacroExprFactory` is a specialization of `ExprFactory` for `MacroExpander` // which disallows explicitly specifying IDs. class MacroExprFactory : protected ExprFactory { @@ -318,6 +323,7 @@ class MacroExprFactory : protected ExprFactory { private: friend class ParserMacroExprFactory; friend class TestMacroExprFactory; + friend class parser_internal::MacroExprExpanderSupport; explicit MacroExprFactory() = default; };