From 4ef589a252f97c21461a2b3f7b010aa5f843fe3a Mon Sep 17 00:00:00 2001 From: Dmitri Plotnikov Date: Mon, 8 Jun 2026 14:01:24 -0700 Subject: [PATCH] Update YAML configuration and constructors to use type and overload signatures PiperOrigin-RevId: 928757149 --- checker/BUILD | 2 +- checker/type_checker_subset_factory.cc | 10 ++--- common/BUILD | 38 ++++++++++++++++++- common/decl.cc | 5 +-- common/internal/BUILD | 36 ------------------ common/{internal => }/signature.cc | 6 +-- common/{internal => }/signature.h | 10 ++--- common/{internal => }/signature_test.cc | 33 ++++++++-------- env/BUILD | 4 +- env/env.cc | 8 ++-- env/env_yaml.cc | 46 ++++++++++++----------- env/env_yaml_test.cc | 50 +++++++++++++++++++++++++ 12 files changed, 148 insertions(+), 100 deletions(-) rename common/{internal => }/signature.cc (99%) rename common/{internal => }/signature.h (93%) rename common/{internal => }/signature_test.cc (96%) diff --git a/checker/BUILD b/checker/BUILD index efca3ff73..7f3ccfef7 100644 --- a/checker/BUILD +++ b/checker/BUILD @@ -231,7 +231,7 @@ cc_library( deps = [ ":type_checker_builder", "//common:decl", - "//common/internal:signature", + "//common:signature", "@com_google_absl//absl/container:flat_hash_set", "@com_google_absl//absl/strings:string_view", "@com_google_absl//absl/types:span", diff --git a/checker/type_checker_subset_factory.cc b/checker/type_checker_subset_factory.cc index e5335e220..1b146c5a5 100644 --- a/checker/type_checker_subset_factory.cc +++ b/checker/type_checker_subset_factory.cc @@ -22,7 +22,7 @@ #include "absl/types/span.h" #include "checker/type_checker_builder.h" #include "common/decl.h" -#include "common/internal/signature.h" +#include "common/signature.h" namespace cel { @@ -33,8 +33,8 @@ TypeCheckerSubset::FunctionPredicate IncludeOverloadsByIdPredicate( if (overload_ids.contains(overload.id())) { return true; } - auto signature = common_internal::MakeOverloadSignature( - function, overload.args(), overload.member()); + auto signature = + MakeOverloadSignature(function, overload.args(), overload.member()); return signature.ok() && overload_ids.contains(*signature); }; } @@ -52,8 +52,8 @@ TypeCheckerSubset::FunctionPredicate ExcludeOverloadsByIdPredicate( if (overload_ids.contains(overload.id())) { return false; } - auto signature = common_internal::MakeOverloadSignature( - function, overload.args(), overload.member()); + auto signature = + MakeOverloadSignature(function, overload.args(), overload.member()); return !signature.ok() || !overload_ids.contains(*signature); }; } diff --git a/common/BUILD b/common/BUILD index 0bd3632dd..0426c0827 100644 --- a/common/BUILD +++ b/common/BUILD @@ -79,6 +79,42 @@ cc_test( ], ) +cc_library( + name = "signature", + srcs = ["signature.cc"], + hdrs = ["signature.h"], + deps = [ + ":ast", + ":type", + ":type_spec_resolver", + "//internal:status_macros", + "@com_google_absl//absl/status", + "@com_google_absl//absl/status:statusor", + "@com_google_absl//absl/strings", + "@com_google_absl//absl/types:optional", + "@com_google_protobuf//:protobuf", + ], +) + +cc_test( + name = "signature_test", + srcs = ["signature_test.cc"], + deps = [ + ":ast", + ":signature", + ":type", + ":type_kind", + ":type_spec_resolver", + "//internal:testing", + "//internal:testing_descriptor_pool", + "@com_google_absl//absl/base:no_destructor", + "@com_google_absl//absl/status", + "@com_google_absl//absl/status:status_matchers", + "@com_google_absl//absl/status:statusor", + "@com_google_protobuf//:protobuf", + ], +) + cc_library( name = "expr", srcs = ["expr.cc"], @@ -145,9 +181,9 @@ cc_library( hdrs = ["decl.h"], deps = [ ":constant", + ":signature", ":type", ":type_kind", - "//common/internal:signature", "//internal:status_macros", "@com_google_absl//absl/algorithm:container", "@com_google_absl//absl/base:core_headers", diff --git a/common/decl.cc b/common/decl.cc index d2d50964a..858e6fb49 100644 --- a/common/decl.cc +++ b/common/decl.cc @@ -26,7 +26,7 @@ #include "absl/status/statusor.h" #include "absl/strings/str_cat.h" #include "absl/strings/string_view.h" -#include "common/internal/signature.h" +#include "common/signature.h" #include "common/type.h" #include "common/type_kind.h" @@ -117,8 +117,7 @@ void AddOverloadInternal(std::string_view function_name, } absl::StatusOr signature = - common_internal::MakeOverloadSignature(function_name, overload.args(), - overload.member()); + MakeOverloadSignature(function_name, overload.args(), overload.member()); if (!signature.ok()) { status = signature.status(); return; diff --git a/common/internal/BUILD b/common/internal/BUILD index b07faf229..3be350754 100644 --- a/common/internal/BUILD +++ b/common/internal/BUILD @@ -135,39 +135,3 @@ cc_library( "@com_google_protobuf//src/google/protobuf/io", ], ) - -cc_library( - name = "signature", - srcs = ["signature.cc"], - hdrs = ["signature.h"], - deps = [ - "//common:ast", - "//common:type", - "//common:type_spec_resolver", - "//internal:status_macros", - "@com_google_absl//absl/status", - "@com_google_absl//absl/status:statusor", - "@com_google_absl//absl/strings", - "@com_google_absl//absl/types:optional", - "@com_google_protobuf//:protobuf", - ], -) - -cc_test( - name = "signature_test", - srcs = ["signature_test.cc"], - deps = [ - ":signature", - "//common:ast", - "//common:type", - "//common:type_kind", - "//common:type_spec_resolver", - "//internal:testing", - "//internal:testing_descriptor_pool", - "@com_google_absl//absl/base:no_destructor", - "@com_google_absl//absl/status", - "@com_google_absl//absl/status:status_matchers", - "@com_google_absl//absl/status:statusor", - "@com_google_protobuf//:protobuf", - ], -) diff --git a/common/internal/signature.cc b/common/signature.cc similarity index 99% rename from common/internal/signature.cc rename to common/signature.cc index fe315bb04..e497e780d 100644 --- a/common/internal/signature.cc +++ b/common/signature.cc @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "common/internal/signature.h" +#include "common/signature.h" #include #include @@ -34,7 +34,7 @@ #include "google/protobuf/arena.h" #include "google/protobuf/descriptor.h" -namespace cel::common_internal { +namespace cel { // Signature generator helper functions. namespace { @@ -637,4 +637,4 @@ absl::StatusOr ParseType(std::string_view signature, google::protobuf::Are return cel::ConvertTypeSpecToType(type_spec, arena, pool); } -} // namespace cel::common_internal +} // namespace cel diff --git a/common/internal/signature.h b/common/signature.h similarity index 93% rename from common/internal/signature.h rename to common/signature.h index 8a44fbd5c..777f03439 100644 --- a/common/internal/signature.h +++ b/common/signature.h @@ -12,8 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -#ifndef THIRD_PARTY_CEL_CPP_COMMON_INTERNAL_SIGNATURE_H_ -#define THIRD_PARTY_CEL_CPP_COMMON_INTERNAL_SIGNATURE_H_ +#ifndef THIRD_PARTY_CEL_CPP_COMMON_SIGNATURE_H_ +#define THIRD_PARTY_CEL_CPP_COMMON_SIGNATURE_H_ #include #include @@ -25,7 +25,7 @@ #include "google/protobuf/arena.h" #include "google/protobuf/descriptor.h" -namespace cel::common_internal { +namespace cel { // Generates a signature for a `cel::Type`, which is a string representation of // the type. @@ -96,6 +96,6 @@ struct ParsedFunctionOverload { absl::StatusOr ParseFunctionSignature( std::string_view signature); -} // namespace cel::common_internal +} // namespace cel -#endif // THIRD_PARTY_CEL_CPP_COMMON_INTERNAL_SIGNATURE_H_ +#endif // THIRD_PARTY_CEL_CPP_COMMON_SIGNATURE_H_ diff --git a/common/internal/signature_test.cc b/common/signature_test.cc similarity index 96% rename from common/internal/signature_test.cc rename to common/signature_test.cc index 17b628d88..ea51eb566 100644 --- a/common/internal/signature_test.cc +++ b/common/signature_test.cc @@ -1,4 +1,4 @@ -#include "common/internal/signature.h" +#include "common/signature.h" // Copyright 2026 Google LLC // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -30,7 +30,7 @@ #include "internal/testing_descriptor_pool.h" #include "google/protobuf/arena.h" -namespace cel::common_internal { +namespace cel { namespace { using ::absl_testing::IsOkAndHolds; @@ -77,8 +77,7 @@ using TypeSignatureTest = testing::TestWithParam; TEST_P(TypeSignatureTest, TypeSignature) { const auto& param = GetParam(); - absl::StatusOr signature = - common_internal::MakeTypeSpecSignature(param.type); + absl::StatusOr signature = MakeTypeSpecSignature(param.type); if (!param.expected_error.empty()) { EXPECT_THAT(signature, StatusIs(absl::StatusCode::kInvalidArgument, HasSubstr(param.expected_error))); @@ -257,25 +256,24 @@ INSTANTIATE_TEST_SUITE_P(TypeSignatureTest, TypeSignatureTest, ValuesIn(GetTypeSignatureTestCases())); TEST(TypeSignatureTest, UnsupportedTypes) { - EXPECT_THAT(common_internal::MakeTypeSignature(UnknownType{}), + EXPECT_THAT(MakeTypeSignature(UnknownType{}), StatusIs(absl::StatusCode::kInvalidArgument, HasSubstr("Unsupported Type kind: *unknown*"))); - EXPECT_THAT(common_internal::MakeTypeSignature(ErrorType{}), + EXPECT_THAT(MakeTypeSignature(ErrorType{}), StatusIs(absl::StatusCode::kInvalidArgument, HasSubstr("Unsupported type in signature: *error*"))); - EXPECT_THAT(common_internal::MakeTypeSpecSignature( - TypeSpec(static_cast(999))), + EXPECT_THAT(MakeTypeSpecSignature(TypeSpec(static_cast(999))), StatusIs(absl::StatusCode::kInvalidArgument, HasSubstr("Unsupported primitive type"))); - EXPECT_THAT(common_internal::MakeTypeSpecSignature( - TypeSpec(static_cast(999))), - StatusIs(absl::StatusCode::kInvalidArgument, - HasSubstr("Unsupported well-known type"))); + EXPECT_THAT( + MakeTypeSpecSignature(TypeSpec(static_cast(999))), + StatusIs(absl::StatusCode::kInvalidArgument, + HasSubstr("Unsupported well-known type"))); - EXPECT_THAT(common_internal::MakeTypeSpecSignature(TypeSpec( + EXPECT_THAT(MakeTypeSpecSignature(TypeSpec( PrimitiveTypeWrapper(static_cast(999)))), StatusIs(absl::StatusCode::kInvalidArgument, HasSubstr("Unsupported wrapper type"))); @@ -308,8 +306,7 @@ TEST_P(OverloadSignatureTest, OverloadSignature) { const auto& param = GetParam(); absl::StatusOr signature = - common_internal::MakeOverloadSignature(param.function_name, param.args, - param.is_member); + MakeOverloadSignature(param.function_name, param.args, param.is_member); if (!param.expected_error.empty()) { EXPECT_THAT(signature, StatusIs(absl::StatusCode::kInvalidArgument, HasSubstr(param.expected_error))); @@ -433,8 +430,8 @@ std::vector GetOverloadSignatureTestCases() { } TEST(OverloadSignatureTest, MemberFunctionNoReceiverError) { - auto signature = common_internal::MakeOverloadSignature( - "hello", std::vector{}, true); + auto signature = + MakeOverloadSignature("hello", std::vector{}, true); EXPECT_THAT(signature, StatusIs(absl::StatusCode::kInvalidArgument, HasSubstr("Member function with no receiver"))); @@ -784,4 +781,4 @@ TEST(OverloadSignatureTest, ArgumentTypeVector) { } } // namespace -} // namespace cel::common_internal +} // namespace cel diff --git a/env/BUILD b/env/BUILD index 1816238a5..0c17d6305 100644 --- a/env/BUILD +++ b/env/BUILD @@ -55,8 +55,8 @@ cc_library( "//common:constant", "//common:container", "//common:decl", + "//common:signature", "//common:type", - "//common/internal:signature", "//compiler", "//compiler:compiler_factory", "//compiler:standard_library", @@ -124,7 +124,7 @@ cc_library( ":config", "//common:ast", "//common:constant", - "//common/internal:signature", + "//common:signature", "//internal:status_macros", "//internal:strings", "@com_google_absl//absl/algorithm:container", diff --git a/env/env.cc b/env/env.cc index 4fa4e7398..85c5139da 100644 --- a/env/env.cc +++ b/env/env.cc @@ -26,7 +26,7 @@ #include "common/constant.h" #include "common/container.h" #include "common/decl.h" -#include "common/internal/signature.h" +#include "common/signature.h" #include "common/type.h" #include "compiler/compiler.h" #include "compiler/compiler_factory.h" @@ -71,8 +71,7 @@ bool ShouldIncludeFunction(const Config::StandardLibraryConfig& config, return false; } absl::StatusOr signature = - common_internal::MakeOverloadSignature(function, overload.args(), - overload.member()); + MakeOverloadSignature(function, overload.args(), overload.member()); if (signature.ok() && config.excluded_functions.contains(std::make_pair( std::string(function), *std::move(signature)))) { return false; @@ -89,8 +88,7 @@ bool ShouldIncludeFunction(const Config::StandardLibraryConfig& config, // Ok to call MakeOverloadSignature() again, because in practice either // included or excluded functions may be specified, but not both. absl::StatusOr signature = - common_internal::MakeOverloadSignature(function, overload.args(), - overload.member()); + MakeOverloadSignature(function, overload.args(), overload.member()); if (signature.ok() && config.included_functions.contains(std::make_pair( std::string(function), *std::move(signature)))) { return true; diff --git a/env/env_yaml.cc b/env/env_yaml.cc index e7b8a7885..281cf3ff1 100644 --- a/env/env_yaml.cc +++ b/env/env_yaml.cc @@ -38,7 +38,7 @@ #include "absl/time/time.h" #include "common/ast.h" #include "common/constant.h" -#include "common/internal/signature.h" +#include "common/signature.h" #include "env/config.h" #include "env/type_info.h" #include "internal/status_macros.h" @@ -434,8 +434,7 @@ absl::StatusOr ParseTypeInfo(const YAML::Node& node, if (!type.IsScalar()) { return YamlError(yaml, type, "Node 'type' is not a string"); } - CEL_ASSIGN_OR_RETURN(auto type_spec, - common_internal::ParseTypeSpec(GetString(yaml, type))); + CEL_ASSIGN_OR_RETURN(auto type_spec, ParseTypeSpec(GetString(yaml, type))); CEL_ASSIGN_OR_RETURN(auto type_config, TypeSpecToTypeInfo(type_spec)); return type_config; } @@ -714,9 +713,8 @@ absl::StatusOr ParseFunctionOverloadConfig( } std::string signature = GetString(yaml, signature_node); - CEL_ASSIGN_OR_RETURN( - common_internal::ParsedFunctionOverload parsed_signature, - common_internal::ParseFunctionSignature(signature)); + CEL_ASSIGN_OR_RETURN(ParsedFunctionOverload parsed_signature, + ParseFunctionSignature(signature)); if (parsed_signature.function_name != function_name) { return YamlError(yaml, signature_node, absl::StrCat("Function overload name \"", @@ -725,6 +723,9 @@ absl::StatusOr ParseFunctionOverloadConfig( function_name, "\"")); } overload_config.is_member_function = parsed_signature.is_member; + if (overload_config.overload_id.empty()) { + overload_config.overload_id = signature; + } if (!parsed_signature.signature_type.has_function()) { return absl::InternalError(absl::StrCat( "Function overload signature has no function type: ", signature)); @@ -764,8 +765,8 @@ absl::StatusOr ParseFunctionOverloadConfig( const YAML::Node return_type = overload["return"]; if (return_type.IsDefined()) { if (return_type.IsScalar()) { - CEL_ASSIGN_OR_RETURN(auto type_spec, common_internal::ParseTypeSpec( - GetString(yaml, return_type))); + CEL_ASSIGN_OR_RETURN(auto type_spec, + ParseTypeSpec(GetString(yaml, return_type))); CEL_ASSIGN_OR_RETURN(overload_config.return_type, TypeSpecToTypeInfo(type_spec)); } else if (return_type.IsMap()) { @@ -990,8 +991,7 @@ void EmitTypeInfo(const Config::TypeInfo& type_info, YAML::Emitter& out, if (options.use_type_signatures) { absl::StatusOr type_spec = TypeInfoToTypeSpec(type_info); if (type_spec.ok()) { - absl::StatusOr signature = - common_internal::MakeTypeSpecSignature(*type_spec); + absl::StatusOr signature = MakeTypeSpecSignature(*type_spec); if (signature.ok()) { out << YAML::Key << "type"; out << YAML::Value << YAML::DoubleQuoted << *signature; @@ -1101,11 +1101,8 @@ void EmitFunctionOverloadConfig( const Config::FunctionOverloadConfig& overload_config, YAML::Emitter& out, const EnvConfigToYamlOptions& options) { out << YAML::BeginMap; - if (!overload_config.overload_id.empty()) { - out << YAML::Key << "id"; - out << YAML::Value << YAML::DoubleQuoted << overload_config.overload_id; - } bool signature_generated = false; + std::string signature_str; if (options.use_type_signatures) { bool param_type_spec_generated = true; std::vector params; @@ -1119,16 +1116,24 @@ void EmitFunctionOverloadConfig( params.push_back(std::move(*type_spec)); } if (param_type_spec_generated) { - absl::StatusOr signature = - common_internal::MakeOverloadSignature( - function_name, params, overload_config.is_member_function); + absl::StatusOr signature = MakeOverloadSignature( + function_name, params, overload_config.is_member_function); if (signature.ok()) { - out << YAML::Key << "signature"; - out << YAML::Value << YAML::DoubleQuoted << *signature; + signature_str = std::move(*signature); signature_generated = true; } } } + if (!overload_config.overload_id.empty()) { + if (!signature_generated || overload_config.overload_id != signature_str) { + out << YAML::Key << "id"; + out << YAML::Value << YAML::DoubleQuoted << overload_config.overload_id; + } + } + if (signature_generated) { + out << YAML::Key << "signature"; + out << YAML::Value << YAML::DoubleQuoted << signature_str; + } if (!signature_generated) { if (overload_config.is_member_function) { out << YAML::Key << "target" << YAML::Value; @@ -1168,8 +1173,7 @@ void EmitFunctionOverloadConfig( absl::StatusOr type_spec = TypeInfoToTypeSpec(overload_config.return_type); if (type_spec.ok()) { - absl::StatusOr signature = - common_internal::MakeTypeSpecSignature(*type_spec); + absl::StatusOr signature = MakeTypeSpecSignature(*type_spec); if (signature.ok()) { out << YAML::Key << "return"; out << YAML::Value << YAML::DoubleQuoted << *signature; diff --git a/env/env_yaml_test.cc b/env/env_yaml_test.cc index 38f08e371..c5bd1b787 100644 --- a/env/env_yaml_test.cc +++ b/env/env_yaml_test.cc @@ -545,12 +545,15 @@ std::vector GetParseFunctionTestCases() { .overload_configs = { Config::FunctionOverloadConfig{ + .overload_id = + "google.protobuf.StringValue.isEmpty()", .examples = {"''.isEmpty() // true"}, .is_member_function = true, .parameters = {{.name = "string_wrapper"}}, .return_type = {.name = "bool"}, }, Config::FunctionOverloadConfig{ + .overload_id = "list<~T>.isEmpty()", .examples = {"[].isEmpty() // true", "[1].isEmpty() // false"}, .is_member_function = true, @@ -635,6 +638,7 @@ std::vector GetParseFunctionTestCases() { .overload_configs = { Config::FunctionOverloadConfig{ + .overload_id = "contains(list<~T>, ~T)", .examples = {"contains([1, 2, 3], 2) // true"}, .is_member_function = false, .parameters = @@ -1740,6 +1744,45 @@ std::vector GetExportTestCases() { - type_name: "int" )yaml", }, + ExportTestCase{ + .config = []() -> absl::StatusOr { + Config config; + CEL_RETURN_IF_ERROR(config.AddFunctionConfig( + {.name = "foo", + .overload_configs = { + {.overload_id = "timestamp.foo(A<~B>)", + .is_member_function = true, + .parameters = {{.name = "timestamp"}, + {.name = "A", + .params = {{.name = "B", + .is_type_param = true}}}}, + .return_type = {.name = "int"}}, + }})); + return config; + }(), + .expected_yaml = R"yaml( + functions: + - name: "foo" + overloads: + - signature: "timestamp.foo(A<~B>)" + return: "int" + )yaml", + .expected_alt_yaml = R"yaml( + functions: + - name: "foo" + overloads: + - id: "timestamp.foo(A<~B>)" + target: + type_name: "timestamp" + args: + - type_name: "A" + params: + - type_name: "B" + is_type_param: true + return: + type_name: "int" + )yaml", + }, }; }; @@ -1888,6 +1931,13 @@ std::vector GetSignatureRoundTripTestCases() { signature: "foo(timestamp,A<~B>)" return: "list" )yaml", + R"yaml( + functions: + - name: "foo" + overloads: + - signature: "timestamp.foo(A<~B>)" + return: "int" + )yaml", }; }