From be36103c84cebb31d238594ef7c3e935a3701488 Mon Sep 17 00:00:00 2001 From: Jonathan Tatum Date: Wed, 7 May 2025 16:57:23 -0700 Subject: [PATCH] Add utility for formatting type names for display in error messages. Remove recursion in cel::Type::DebugString (only report kind of type parameters). PiperOrigin-RevId: 756060510 --- checker/internal/BUILD | 24 +++ checker/internal/format_type_name.cc | 180 +++++++++++++++++++++ checker/internal/format_type_name.h | 30 ++++ checker/internal/format_type_name_test.cc | 116 +++++++++++++ checker/internal/type_checker_impl.cc | 22 +-- checker/internal/type_checker_impl_test.cc | 14 +- checker/optional_test.cc | 8 +- common/BUILD | 1 - common/type.h | 2 + common/types/list_type.cc | 3 +- common/types/map_type.cc | 5 +- common/types/opaque_type.cc | 10 +- common/types/type_type.cc | 4 +- 13 files changed, 391 insertions(+), 28 deletions(-) create mode 100644 checker/internal/format_type_name.cc create mode 100644 checker/internal/format_type_name.h create mode 100644 checker/internal/format_type_name_test.cc diff --git a/checker/internal/BUILD b/checker/internal/BUILD index cf3bf3386..21ff8ca16 100644 --- a/checker/internal/BUILD +++ b/checker/internal/BUILD @@ -116,6 +116,7 @@ cc_library( "type_checker_impl.h", ], deps = [ + ":format_type_name", ":namespace_generator", ":type_check_env", ":type_inference_context", @@ -246,3 +247,26 @@ cc_test( "@com_google_protobuf//:protobuf", ], ) + +cc_library( + name = "format_type_name", + srcs = ["format_type_name.cc"], + hdrs = ["format_type_name.h"], + deps = [ + "//common:type", + "//common:type_kind", + "@com_google_absl//absl/strings", + ], +) + +cc_test( + name = "format_type_name_test", + srcs = ["format_type_name_test.cc"], + deps = [ + ":format_type_name", + "//common:type", + "//internal:testing", + "@com_google_cel_spec//proto/cel/expr/conformance/proto2:test_all_types_cc_proto", + "@com_google_protobuf//:protobuf", + ], +) diff --git a/checker/internal/format_type_name.cc b/checker/internal/format_type_name.cc new file mode 100644 index 000000000..7cd17251f --- /dev/null +++ b/checker/internal/format_type_name.cc @@ -0,0 +1,180 @@ +// Copyright 2025 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +#include "checker/internal/format_type_name.h" + +#include +#include + +#include "absl/strings/str_cat.h" +#include "common/type.h" +#include "common/type_kind.h" + +namespace cel::checker_internal { + +namespace { +struct FormatImplRecord { + Type type; + int offset; +}; + +// Parameterized types can be arbitrarily nested, so we use a vector as +// a stack to avoid overflow. Practically, we don't expect nesting +// to ever be very deep, but fuzzers and pathological inputs can easily +// trigger stack overflow with a recursive implementation. +void FormatImpl(const Type& cur, int offset, + std::vector& stack, std::string* out) { + switch (cur.kind()) { + case TypeKind::kDyn: + absl::StrAppend(out, "dyn"); + return; + case TypeKind::kAny: + absl::StrAppend(out, "any"); + return; + case TypeKind::kBool: + absl::StrAppend(out, "bool"); + return; + case TypeKind::kBoolWrapper: + absl::StrAppend(out, "wrapper(bool)"); + return; + case TypeKind::kBytes: + absl::StrAppend(out, "bytes"); + return; + case TypeKind::kBytesWrapper: + absl::StrAppend(out, "wrapper(bytes)"); + return; + case TypeKind::kDouble: + absl::StrAppend(out, "double"); + return; + case TypeKind::kDoubleWrapper: + absl::StrAppend(out, "wrapper(double)"); + return; + case TypeKind::kDuration: + absl::StrAppend(out, "google.protobuf.Duration"); + return; + case TypeKind::kEnum: + absl::StrAppend(out, "int"); + return; + case TypeKind::kInt: + absl::StrAppend(out, "int"); + return; + case TypeKind::kIntWrapper: + absl::StrAppend(out, "wrapper(int)"); + return; + case TypeKind::kList: + if (offset == 0) { + absl::StrAppend(out, "list("); + stack.push_back({cur, 1}); + stack.push_back({cur.AsList()->GetElement(), 0}); + } else { + absl::StrAppend(out, ")"); + } + return; + case TypeKind::kMap: + if (offset == 0) { + absl::StrAppend(out, "map("); + stack.push_back({cur, 1}); + stack.push_back({cur.AsMap()->GetKey(), 0}); + return; + } + if (offset == 1) { + absl::StrAppend(out, ", "); + stack.push_back({cur, 2}); + stack.push_back({cur.AsMap()->GetValue(), 0}); + return; + } + absl::StrAppend(out, ")"); + return; + case TypeKind::kNull: + absl::StrAppend(out, "null_type"); + return; + case TypeKind::kOpaque: { + OpaqueType opaque = *cur.AsOpaque(); + if (offset == 0) { + absl::StrAppend(out, cur.AsOpaque()->name()); + if (!opaque.GetParameters().empty()) { + absl::StrAppend(out, "("); + stack.push_back({cur, 1}); + stack.push_back({cur.AsOpaque()->GetParameters()[0], 0}); + } + return; + } + if (offset >= opaque.GetParameters().size()) { + absl::StrAppend(out, ")"); + return; + } + absl::StrAppend(out, ", "); + stack.push_back({cur, offset + 1}); + stack.push_back({cur.AsOpaque()->GetParameters()[offset], 0}); + return; + } + case TypeKind::kString: + absl::StrAppend(out, "string"); + return; + case TypeKind::kStringWrapper: + absl::StrAppend(out, "wrapper(string)"); + return; + case TypeKind::kStruct: + absl::StrAppend(out, cur.AsStruct()->name()); + return; + case TypeKind::kTimestamp: + absl::StrAppend(out, "google.protobuf.Timestamp"); + return; + case TypeKind::kType: { + TypeType type_type = *cur.AsType(); + if (offset == 0) { + absl::StrAppend(out, type_type.name()); + if (!type_type.GetParameters().empty()) { + absl::StrAppend(out, "("); + stack.push_back({cur, 1}); + stack.push_back({cur.AsType()->GetParameters()[0], 0}); + } + return; + } + absl::StrAppend(out, ")"); + return; + } + case TypeKind::kTypeParam: + absl::StrAppend(out, cur.AsTypeParam()->name()); + return; + case TypeKind::kUint: + absl::StrAppend(out, "uint"); + return; + case TypeKind::kUintWrapper: + absl::StrAppend(out, "wrapper(uint)"); + return; + case TypeKind::kUnknown: + absl::StrAppend(out, "*unknown*"); + return; + case TypeKind::kError: + case TypeKind::kFunction: + default: + absl::StrAppend(out, "*error*"); + return; + } +} +} // namespace + +std::string FormatTypeName(const Type& type) { + std::vector stack; + std::string out; + stack.push_back({type, 0}); + while (!stack.empty()) { + auto [type, offset] = stack.back(); + stack.pop_back(); + FormatImpl(type, offset, stack, &out); + } + return out; +} + +} // namespace cel::checker_internal diff --git a/checker/internal/format_type_name.h b/checker/internal/format_type_name.h new file mode 100644 index 000000000..c31e1c4d0 --- /dev/null +++ b/checker/internal/format_type_name.h @@ -0,0 +1,30 @@ +// Copyright 2025 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef THIRD_PARTY_CEL_CPP_CHECKER_INTERNAL_FORMAT_TYPE_NAME_H_ +#define THIRD_PARTY_CEL_CPP_CHECKER_INTERNAL_FORMAT_TYPE_NAME_H_ + +#include + +#include "common/type.h" + +namespace cel::checker_internal { + +// Format the type name for presentation in error messages. Matches the +// formatting used in github.com/cel-spec. +std::string FormatTypeName(const Type& type); + +} // namespace cel::checker_internal + +#endif // THIRD_PARTY_CEL_CPP_CHECKER_INTERNAL_FORMAT_TYPE_NAME_H_ diff --git a/checker/internal/format_type_name_test.cc b/checker/internal/format_type_name_test.cc new file mode 100644 index 000000000..23bc2bda9 --- /dev/null +++ b/checker/internal/format_type_name_test.cc @@ -0,0 +1,116 @@ +// Copyright 2025 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "checker/internal/format_type_name.h" + +#include "common/type.h" +#include "internal/testing.h" +#include "cel/expr/conformance/proto2/test_all_types.pb.h" +#include "google/protobuf/arena.h" + +namespace cel::checker_internal { +namespace { + +using ::cel::expr::conformance::proto2::GlobalEnum_descriptor; +using ::cel::expr::conformance::proto2::TestAllTypes; +using ::testing::MatchesRegex; + +TEST(FormatTypeNameTest, PrimitiveTypes) { + EXPECT_EQ(FormatTypeName(IntType()), "int"); + EXPECT_EQ(FormatTypeName(UintType()), "uint"); + EXPECT_EQ(FormatTypeName(DoubleType()), "double"); + EXPECT_EQ(FormatTypeName(StringType()), "string"); + EXPECT_EQ(FormatTypeName(BytesType()), "bytes"); + EXPECT_EQ(FormatTypeName(BoolType()), "bool"); + EXPECT_EQ(FormatTypeName(NullType()), "null_type"); + EXPECT_EQ(FormatTypeName(DynType()), "dyn"); +} + +TEST(FormatTypeNameTest, SpecialTypes) { + EXPECT_EQ(FormatTypeName(ErrorType()), "*error*"); + EXPECT_EQ(FormatTypeName(UnknownType()), "*unknown*"); + EXPECT_EQ(FormatTypeName(FunctionType()), "*error*"); +} + +TEST(FormatTypeNameTest, WellKnownTypes) { + EXPECT_EQ(FormatTypeName(AnyType()), "any"); + EXPECT_EQ(FormatTypeName(DurationType()), "google.protobuf.Duration"); + EXPECT_EQ(FormatTypeName(TimestampType()), "google.protobuf.Timestamp"); +} + +TEST(FormatTypeNameTest, Wrappers) { + EXPECT_EQ(FormatTypeName(IntWrapperType()), "wrapper(int)"); + EXPECT_EQ(FormatTypeName(UintWrapperType()), "wrapper(uint)"); + EXPECT_EQ(FormatTypeName(DoubleWrapperType()), "wrapper(double)"); + EXPECT_EQ(FormatTypeName(StringWrapperType()), "wrapper(string)"); + EXPECT_EQ(FormatTypeName(BytesWrapperType()), "wrapper(bytes)"); + EXPECT_EQ(FormatTypeName(BoolWrapperType()), "wrapper(bool)"); +} + +TEST(FormatTypeNameTest, ProtobufTypes) { + EXPECT_EQ(FormatTypeName(MessageType(TestAllTypes::descriptor())), + "cel.expr.conformance.proto2.TestAllTypes"); + EXPECT_EQ(FormatTypeName(EnumType(GlobalEnum_descriptor())), "int"); +} + +TEST(FormatTypeNameTest, Type) { + google::protobuf::Arena arena; + EXPECT_EQ(FormatTypeName(TypeType()), "type"); + EXPECT_EQ(FormatTypeName(TypeType(&arena, IntType())), "type(int)"); + EXPECT_EQ(FormatTypeName(TypeType(&arena, TypeType(&arena, IntType()))), + "type(type(int))"); + EXPECT_EQ(FormatTypeName(TypeType(&arena, TypeParamType("T"))), "type(T)"); +} + +TEST(FormatTypeNameTest, List) { + google::protobuf::Arena arena; + EXPECT_EQ(FormatTypeName(ListType()), "list(dyn)"); + EXPECT_EQ(FormatTypeName(ListType(&arena, IntType())), "list(int)"); + EXPECT_EQ(FormatTypeName(ListType(&arena, ListType(&arena, IntType()))), + "list(list(int))"); +} + +TEST(FormatTypeNameTest, Map) { + google::protobuf::Arena arena; + EXPECT_EQ(FormatTypeName(MapType()), "map(dyn, dyn)"); + EXPECT_EQ(FormatTypeName(MapType(&arena, IntType(), IntType())), + "map(int, int)"); + EXPECT_EQ(FormatTypeName(MapType(&arena, IntType(), + MapType(&arena, IntType(), IntType()))), + "map(int, map(int, int))"); +} + +TEST(FormatTypeNameTest, Opaque) { + google::protobuf::Arena arena; + EXPECT_EQ(FormatTypeName(OpaqueType(&arena, "opaque", {})), "opaque"); + Type two_tuple_type = OpaqueType(&arena, "tuple", {IntType(), IntType()}); + Type three_tuple_type = OpaqueType( + &arena, "tuple", {two_tuple_type, two_tuple_type, two_tuple_type}); + EXPECT_EQ(FormatTypeName(three_tuple_type), + "tuple(tuple(int, int), tuple(int, int), tuple(int, int))"); +} + +TEST(FormatTypeNameTest, ArbitraryNesting) { + google::protobuf::Arena arena; + Type type = IntType(); + for (int i = 0; i < 1000; ++i) { + type = OpaqueType(&arena, "ptype", {type}); + } + + EXPECT_THAT(FormatTypeName(type), + MatchesRegex(R"(^(ptype\(){1000}int(\)){1000})")); +} + +} // namespace +} // namespace cel::checker_internal diff --git a/checker/internal/type_checker_impl.cc b/checker/internal/type_checker_impl.cc index ad8cb4fe4..f4e282cd8 100644 --- a/checker/internal/type_checker_impl.cc +++ b/checker/internal/type_checker_impl.cc @@ -33,6 +33,7 @@ #include "absl/types/optional.h" #include "absl/types/span.h" #include "checker/checker_options.h" +#include "checker/internal/format_type_name.h" #include "checker/internal/namespace_generator.h" #include "checker/internal/type_check_env.h" #include "checker/internal/type_inference_context.h" @@ -396,9 +397,9 @@ class ResolveVisitor : public AstVisitorBase { ReportIssue(TypeCheckIssue::CreateError( ComputeSourceLocation(*ast_, expr_id), absl::StrCat("expected type '", - inference_context_->FinalizeType(expected).DebugString(), + FormatTypeName(inference_context_->FinalizeType(expected)), "' but found '", - inference_context_->FinalizeType(actual).DebugString(), + FormatTypeName(inference_context_->FinalizeType(actual)), "'"))); } @@ -428,9 +429,9 @@ class ResolveVisitor : public AstVisitorBase { ComputeSourceLocation(*ast_, field.id()), absl::StrCat( "expected type of field '", field_info->name(), "' is '", - inference_context_->FinalizeType(field_type).DebugString(), + FormatTypeName(inference_context_->FinalizeType(field_type)), "' but provided type is '", - inference_context_->FinalizeType(value_type).DebugString(), + FormatTypeName(inference_context_->FinalizeType(value_type)), "'"))); continue; } @@ -625,7 +626,7 @@ void ResolveVisitor::PostVisitMap(const Expr& expr, const MapExpr& map) { Severity::kWarning, ComputeSourceLocation(*ast_, key->id()), absl::StrCat( "unsupported map key type: ", - inference_context_->FinalizeType(key_type).DebugString()))); + FormatTypeName(inference_context_->FinalizeType(key_type))))); } if (!assignability_context.IsAssignable(key_type, overall_key_type)) { @@ -882,7 +883,7 @@ void ResolveVisitor::PostVisitComprehensionSubexpression( ComputeSourceLocation(*ast_, comprehension.iter_range().id()), absl::StrCat( "expression of type '", - inference_context_->FinalizeType(range_type).DebugString(), + FormatTypeName(inference_context_->FinalizeType(range_type)), "' cannot be the range of a comprehension (must be " "list, map, or dynamic)"))); break; @@ -955,7 +956,7 @@ void ResolveVisitor::ResolveFunctionOverloads(const Expr& expr, "' applied to '(", absl::StrJoin(arg_types, ", ", [](std::string* out, const Type& type) { - out->append(type.DebugString()); + out->append(FormatTypeName(type)); }), ")'"))); types_[&expr] = ErrorType(); @@ -1116,9 +1117,10 @@ absl::optional ResolveVisitor::CheckFieldType(int64_t id, ReportIssue(TypeCheckIssue::CreateError( ComputeSourceLocation(*ast_, id), - absl::StrCat("expression of type '", - inference_context_->FinalizeType(operand_type).DebugString(), - "' cannot be the operand of a select operation"))); + absl::StrCat( + "expression of type '", + FormatTypeName(inference_context_->FinalizeType(operand_type)), + "' cannot be the operand of a select operation"))); return absl::nullopt; } diff --git a/checker/internal/type_checker_impl_test.cc b/checker/internal/type_checker_impl_test.cc index 5a7a667cb..90c97265e 100644 --- a/checker/internal/type_checker_impl_test.cc +++ b/checker/internal/type_checker_impl_test.cc @@ -1326,7 +1326,7 @@ TEST(TypeCheckerImplTest, ExpectedTypeDoesntMatch) { result.GetIssues(), Contains(IsIssueWithSubstring( Severity::kError, - "expected type 'map' but found 'map'"))); + "expected type 'map(string, string)' but found 'map(string, int)'"))); } TEST(TypeCheckerImplTest, BadSourcePosition) { @@ -1563,7 +1563,7 @@ INSTANTIATE_TEST_SUITE_P( .expr = "Int32Value{value: 10}.value", .expected_result_type = AstType(), .error_substring = - "expression of type 'google.protobuf.Int64Value' cannot be the " + "expression of type 'wrapper(int)' cannot be the " "operand of a select operation"}, CheckedExprTestCase{ .expr = "Int64Value{value: 10}", @@ -1820,8 +1820,8 @@ INSTANTIATE_TEST_SUITE_P( .expr = "TestAllTypes{single_struct: {1: 2}}", .expected_result_type = AstType(), .error_substring = "expected type of field 'single_struct' is " - "'map' but " - "provided type is 'map'"}, + "'map(string, dyn)' but " + "provided type is 'map(int, int)'"}, CheckedExprTestCase{ .expr = "TestAllTypes{list_value: [1, 2, 3]}", .expected_result_type = AstType(ast_internal::MessageType( @@ -1836,7 +1836,7 @@ INSTANTIATE_TEST_SUITE_P( .expr = "TestAllTypes{list_value: 1}", .expected_result_type = AstType(), .error_substring = - "expected type of field 'list_value' is 'list' but " + "expected type of field 'list_value' is 'list(dyn)' but " "provided type is 'int'"}, CheckedExprTestCase{ .expr = "TestAllTypes{single_int64_wrapper: 1}", @@ -1882,12 +1882,12 @@ INSTANTIATE_TEST_SUITE_P( .expr = "TestAllTypes{repeated_int64: ['string']}", .expected_result_type = AstType(), .error_substring = - "expected type of field 'repeated_int64' is 'list'"}, + "expected type of field 'repeated_int64' is 'list(int)'"}, CheckedExprTestCase{ .expr = "TestAllTypes{map_string_int64: ['string']}", .expected_result_type = AstType(), .error_substring = "expected type of field 'map_string_int64' is " - "'map'"}, + "'map(string, int)'"}, CheckedExprTestCase{ .expr = "TestAllTypes{map_string_int64: {'string': 1}}", .expected_result_type = AstType(ast_internal::MessageType( diff --git a/checker/optional_test.cc b/checker/optional_test.cc index 714e1cf50..85f621591 100644 --- a/checker/optional_test.cc +++ b/checker/optional_test.cc @@ -219,7 +219,7 @@ INSTANTIATE_TEST_SUITE_P( std::unique_ptr( new AstType(ast_internal::PrimitiveType::kString)))))}, TestCase{"{'k': 'v', ?'k2': 'v'}", _, - "expected type 'optional_type' but found 'string'"}, + "expected type 'optional_type(string)' but found 'string'"}, TestCase{"[?optional.of('v')]", Eq(AstType(ast_internal::ListType(std::unique_ptr( new AstType(ast_internal::PrimitiveType::kString)))))}, @@ -227,7 +227,7 @@ INSTANTIATE_TEST_SUITE_P( Eq(AstType(ast_internal::ListType(std::unique_ptr( new AstType(ast_internal::PrimitiveType::kString)))))}, TestCase{"['v1', ?'v2']", _, - "expected type 'optional_type' but found 'string'"}, + "expected type 'optional_type(string)' but found 'string'"}, TestCase{"[optional.of(dyn('1')), optional.of('2')][0]", IsOptionalType(AstType(ast_internal::DynamicType()))}, TestCase{"[optional.of('1'), optional.of(dyn('2'))][0]", @@ -239,7 +239,7 @@ INSTANTIATE_TEST_SUITE_P( TestCase{"[optional.of('1'), optional.of(2)][0]", Eq(AstType(ast_internal::DynamicType()))}, TestCase{"['v1', ?'v2']", _, - "expected type 'optional_type' but found 'string'"}, + "expected type 'optional_type(string)' but found 'string'"}, TestCase{"cel.expr.conformance.proto3.TestAllTypes{?single_int64: " "optional.of(1)}", Eq(AstType(ast_internal::MessageType( @@ -324,7 +324,7 @@ INSTANTIATE_TEST_SUITE_P( ::testing::Values( TestCase{ "cel.expr.conformance.proto3.TestAllTypes{?single_int64: null}", _, - "expected type of field 'single_int64' is 'optional_type' but " + "expected type of field 'single_int64' is 'optional_type(int)' but " "provided type is 'null_type'"}, TestCase{"cel.expr.conformance.proto3.TestAllTypes{}.?single_int64 " "== null", diff --git a/common/BUILD b/common/BUILD index 5463c7009..008e3ceaf 100644 --- a/common/BUILD +++ b/common/BUILD @@ -490,7 +490,6 @@ cc_library( ], deps = [ ":memory", - ":native_type", ":type_kind", "//internal:string_pool", "@com_google_absl//absl/algorithm:container", diff --git a/common/type.h b/common/type.h index dbb2ce3c8..e19562d1d 100644 --- a/common/type.h +++ b/common/type.h @@ -147,6 +147,8 @@ class Type final { absl::string_view name() const ABSL_ATTRIBUTE_LIFETIME_BOUND; + // Returns a debug string for the type. Not suitable for user-facing error + // messages. std::string DebugString() const; Parameters GetParameters() const ABSL_ATTRIBUTE_LIFETIME_BOUND; diff --git a/common/types/list_type.cc b/common/types/list_type.cc index 056a39bb8..2e32d2e34 100644 --- a/common/types/list_type.cc +++ b/common/types/list_type.cc @@ -19,6 +19,7 @@ #include "absl/log/absl_check.h" #include "absl/strings/str_cat.h" #include "common/type.h" +#include "common/type_kind.h" #include "google/protobuf/arena.h" #include "google/protobuf/descriptor.h" @@ -50,7 +51,7 @@ ListType::ListType(google::protobuf::Arena* ABSL_NONNULL arena, const Type& elem : common_internal::ListTypeData::Create(arena, element)) {} std::string ListType::DebugString() const { - return absl::StrCat("list<", element().DebugString(), ">"); + return absl::StrCat("list<", TypeKindToString(GetElement().kind()), ">"); } TypeParameters ListType::GetParameters() const { diff --git a/common/types/map_type.cc b/common/types/map_type.cc index 026440282..d4a446563 100644 --- a/common/types/map_type.cc +++ b/common/types/map_type.cc @@ -19,6 +19,7 @@ #include "absl/log/absl_check.h" #include "absl/strings/str_cat.h" #include "common/type.h" +#include "common/type_kind.h" #include "google/protobuf/arena.h" #include "google/protobuf/descriptor.h" @@ -60,8 +61,8 @@ MapType::MapType(google::protobuf::Arena* ABSL_NONNULL arena, const Type& key, : common_internal::MapTypeData::Create(arena, key, value)) {} std::string MapType::DebugString() const { - return absl::StrCat("map<", key().DebugString(), ", ", value().DebugString(), - ">"); + return absl::StrCat("map<", TypeKindToString(key().kind()), ", ", + TypeKindToString(value().kind()), ">"); } TypeParameters MapType::GetParameters() const { diff --git a/common/types/opaque_type.cc b/common/types/opaque_type.cc index d6ee4d4fd..54719de38 100644 --- a/common/types/opaque_type.cc +++ b/common/types/opaque_type.cc @@ -26,6 +26,7 @@ #include "absl/types/span.h" #include "absl/utility/utility.h" #include "common/type.h" +#include "common/type_kind.h" #include "google/protobuf/arena.h" namespace cel { @@ -37,8 +38,13 @@ std::string OpaqueDebugString(absl::string_view name, if (parameters.empty()) { return std::string(name); } - return absl::StrCat( - name, "<", absl::StrJoin(parameters, ", ", absl::StreamFormatter()), ">"); + return absl::StrCat(name, "<", + absl::StrJoin(parameters, ", ", + [](std::string* out, const Type& type) { + absl::StrAppend( + out, TypeKindToString(type.kind())); + }), + ">"); } } // namespace diff --git a/common/types/type_type.cc b/common/types/type_type.cc index c83a1d59e..cb8774e98 100644 --- a/common/types/type_type.cc +++ b/common/types/type_type.cc @@ -19,6 +19,7 @@ #include "absl/base/nullability.h" #include "absl/strings/str_cat.h" #include "absl/types/span.h" +#include "common/type_kind.h" #include "google/protobuf/arena.h" namespace cel { @@ -47,7 +48,8 @@ struct TypeTypeData final { std::string TypeType::DebugString() const { std::string s(name()); if (!GetParameters().empty()) { - absl::StrAppend(&s, "(", GetParameters().front().DebugString(), ")"); + absl::StrAppend(&s, "(", TypeKindToString(GetParameters().front().kind()), + ")"); } return s; }