diff --git a/.bazelrc b/.bazelrc index af611b2..b4fef7a 100644 --- a/.bazelrc +++ b/.bazelrc @@ -1,5 +1,5 @@ # Enabling bzlmod results in protobuf version issues... -build --noenable_bzlmod +common --noenable_bzlmod # To debug bazel options, uncomment next line. # build --announce_rc diff --git a/CHANGELOG.md b/CHANGELOG.md index f048b42..f64d427 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,12 @@ +# 0.5 + +* Added support for com_google_protobuf >= Protocol Buffers v28. +* Moved `ParseTextProtoHelper` into namespace `mbo::proto::proto_internal`. +* Changed `ParseTextProtoHelper` to only accept derived proto messages on operator access. +* Changed `ParseTextProtoHelper` to die on parsing failure. +* Added `ParseText() -> absl::StatusOr`. +* Improved error message detail for all parsing functions. + # 0.4 * Changed `ParseTextProto`, `ParseTextProtoOrDie` to only work for actual derived proto types. diff --git a/README.md b/README.md index 8432244..b5f971e 100644 --- a/README.md +++ b/README.md @@ -28,16 +28,22 @@ The project is formatted with specific `clang-format` settings which require cla * namespace: `mbo::proto` * `ParseTextProtoOrDie`(`text_proto` [, `std::source_location`]) - * `text_proto` is a text proto best identified as a raw-string with marker 'pb'. + * `text_proto` is a text proto best identified as a raw-string with marker `pb` or `proto`. * If `text_proto` cannot be parsed into the receiving proto type, then the function will fail. * Prefer this function over template function `ParseTextOrDie`. * `ParseTextOrDie`<`Proto`>(`text_proto` [, `std::source_location`]) - * `text_proto` is a text proto best identified as a raw-string with marker 'pb'. + * `text_proto` is a text proto best identified as a raw-string with marker `pb` or `proto`. * `Proto` is the type to produce. * If `text_proto` cannot be parsed as a `Proto`, then the function will fail. * Use this only if the `Proto` type cannot be inferred by `ParserTextProtoOrDie`. +* `ParseTest`<`Proto`>(`text_proto` [, `std::source_location`]) -> `absl::StatusOr`<`Proto`> + * `text_proto` is a text proto best identified as a raw-string with marker `pb` or `proto`. + * `Proto` is the type to produce. + * If `text_proto` cannot be parse as a `Proto`, then the function returns a non-`absl::OkStatus`. + * Use this function in cases where errors are expected. + ## Usage BUILD.bazel: @@ -179,6 +185,8 @@ cp ../cpp-proto-builder/proto_builder/oss/tests/simple_message.proto proto/mbo/p patch - #include -+#include - --#include "google/protobuf/io/tokenizer.h" -+#include "absl/log/absl_check.h" -+#include "absl/log/absl_log.h" -+#include "absl/strings/substitute.h" - #include "gmock/gmock.h" -+#include "google/protobuf/io/tokenizer.h" - #include "gtest/gtest.h" --#include "absl/strings/substitute.h" - #include "re2/re2.h" - --namespace testing { --namespace oss { -- -+namespace mbo::testing::proto { - namespace internal { - --using absl::string_view; --using RegExpStringPiece = ::re2::StringPiece; -+using RegExpStringPiece = std::string_view; - - // Utilities. - -@@ -71,10 +71,10 @@ - } - - template --std::string JoinStringPieces(const Container& strings, string_view separator) { -+std::string JoinStringPieces(const Container& strings, std::string_view separator) { - std::stringstream stream; -- absl::string_view sep = ""; -- for (const absl::string_view str : strings) { -+ std::string_view sep = ""; -+ for (const std::string_view str : strings) { - stream << sep << str; - sep = separator; - } -@@ -86,7 +86,7 @@ - const ::google::protobuf::Descriptor* proto_descriptor, - const std::vector& ignore_fields) { - std::vector ignore_descriptors; -- std::vector remaining_descriptors; -+ std::vector remaining_descriptors; - - const ::google::protobuf::DescriptorPool* pool = proto_descriptor->file()->pool(); - for (const std::string& name : ignore_fields) { -@@ -97,7 +97,7 @@ - } - } - -- QCHECK(remaining_descriptors.empty()) -+ ABSL_QCHECK(remaining_descriptors.empty()) - << "Could not find fields for proto " << proto_descriptor->full_name() - << " with fully qualified names: " - << JoinStringPieces(remaining_descriptors, ","); -@@ -197,7 +197,7 @@ - while (!input.empty()) { - // Consume a dot, except on the first iteration. - if (input.size() < relative_field_path.size() && !Consume(&input, ".")) { -- LOG(FATAL) << "Cannot parse field path '" << relative_field_path -+ ABSL_LOG(FATAL) << "Cannot parse field path '" << relative_field_path - << "' at offset " << relative_field_path.size() - input.size() - << ": expected '.'"; - } -@@ -208,41 +208,41 @@ - RE2::Consume(&input, field_regex, &name)) { - if (field_path.empty()) { - field.field = root_descriptor.FindFieldByName(std::string(name)); -- CHECK(field.field) << "No such field '" << name << "' in message '" -+ ABSL_CHECK(field.field) << "No such field '" << name << "' in message '" - << root_descriptor.full_name() << "'"; - } else { - const ::google::protobuf::util::MessageDifferencer::SpecificField& parent = - field_path.back(); - field.field = parent.field->message_type()->FindFieldByName(std::string(name)); -- CHECK(field.field) << "No such field '" << name << "' in '" -+ ABSL_CHECK(field.field) << "No such field '" << name << "' in '" - << parent.field->full_name() << "'"; - } - } else if (RE2::Consume(&input, extension_regex, &name)) { - field.field = - ::google::protobuf::DescriptorPool::generated_pool()->FindExtensionByName(name); -- CHECK(field.field) << "No such extension '" << name << "'"; -+ ABSL_CHECK(field.field) << "No such extension '" << name << "'"; - if (field_path.empty()) { -- CHECK(root_descriptor.IsExtensionNumber(field.field->number())) -+ ABSL_CHECK(root_descriptor.IsExtensionNumber(field.field->number())) - << "Extension '" << name << "' does not extend message '" - << root_descriptor.full_name() << "'"; - } else { - const ::google::protobuf::util::MessageDifferencer::SpecificField& parent = - field_path.back(); -- CHECK(parent.field->message_type()->IsExtensionNumber( -+ ABSL_CHECK(parent.field->message_type()->IsExtensionNumber( - field.field->number())) - << "Extension '" << name << "' does not extend '" - << parent.field->full_name() << "'"; - } - } else { -- LOG(FATAL) << "Cannot parse field path '" << relative_field_path -+ ABSL_LOG(FATAL) << "Cannot parse field path '" << relative_field_path - << "' at offset " << relative_field_path.size() - input.size() - << ": expected field or extension"; - } - field_path.push_back(field); - } - -- CHECK(!field_path.empty()); -- CHECK(field_path.back().index == -1) -+ ABSL_CHECK(!field_path.empty()); -+ ABSL_CHECK(field_path.back().index == -1) - << "Terminally ignoring fields by index is currently not supported ('" - << relative_field_path << "')"; - return field_path; -@@ -382,5 +382,4 @@ - } - - } // namespace internal --} // namespace oss --} // namespace testing -+} // namespace mbo::testing::proto -diff -du proto/mbo/testing/proto/proto_test_util.h proto-matchers/mbo/testing/proto/proto_test_util.h ---- proto/mbo/testing/proto/proto_test_util.h 2023-07-15 17:57:58 -+++ proto/mbo/testing/proto/proto_test_util.h 2023-07-15 17:41:47 -@@ -14,21 +14,17 @@ - - // READ: https://google.github.io/cpp-proto-builder - --#ifndef PROTO_BUILDER_OSS_TESTING_PROTO_TEST_UTIL_H_ --#define PROTO_BUILDER_OSS_TESTING_PROTO_TEST_UTIL_H_ -+#ifndef MBO_TESTING_PROTO_PROTO_TEST_UTIL_H_ -+#define MBO_TESTING_PROTO_PROTO_TEST_UTIL_H_ - -+#include "absl/log/absl_check.h" -+#include "gmock/gmock.h" - #include "google/protobuf/message.h" - #include "google/protobuf/text_format.h" - #include "google/protobuf/util/message_differencer.h" --#include "gmock/gmock.h" - #include "gtest/gtest.h" - --// Last include to override Macros --#include "proto_builder/oss/logging.h" -- --namespace testing { --namespace oss { -- -+namespace mbo::testing::proto { - namespace internal { - - // Utilities. -@@ -205,7 +201,7 @@ - - // Sets the margin of error for approximate floating point comparison. - void SetMargin(double margin) { -- CHECK_GE(margin, 0.0) << "Using a negative margin for Approximately"; -+ ABSL_CHECK_GE(margin, 0.0) << "Using a negative margin for Approximately"; - comp_->has_custom_margin = true; - comp_->float_margin = margin; - } -@@ -213,7 +209,7 @@ - // Sets the relative fraction of error for approximate floating point - // comparison. - void SetFraction(double fraction) { -- CHECK(0.0 <= fraction && fraction < 1.0) -+ ABSL_CHECK(0.0 <= fraction && fraction < 1.0) - << "Fraction for Approximately must be >= 0.0 and < 1.0"; - comp_->has_custom_fraction = true; - comp_->float_fraction = fraction; -@@ -317,7 +313,7 @@ - : ProtoMatcherBase(must_be_initialized, comp), - expected_(CloneProto2(expected)) { - if (must_be_initialized) { -- CHECK(expected.IsInitialized()) -+ ABSL_CHECK(expected.IsInitialized()) - << "The protocol buffer given to *InitializedProto() " - << "must itself be initialized, but the following required fields " - << "are missing: " << expected.InitializationErrorString() << "."; -@@ -586,7 +582,7 @@ - - // Sets the margin of error for approximate floating point comparison. - void SetMargin(double margin) { -- CHECK_GE(margin, 0.0) << "Using a negative margin for Approximately"; -+ ABSL_CHECK_GE(margin, 0.0) << "Using a negative margin for Approximately"; - comp_->has_custom_margin = true; - comp_->float_margin = margin; - } -@@ -594,7 +590,7 @@ - // Sets the relative fraction of error for approximate floating point - // comparison. - void SetFraction(double fraction) { -- CHECK(0.0 <= fraction && fraction <= 1.0) -+ ABSL_CHECK(0.0 <= fraction && fraction <= 1.0) - << "Fraction for Relatively must be >= 0.0 and < 1.0"; - comp_->has_custom_fraction = true; - comp_->float_fraction = fraction; -@@ -820,7 +816,6 @@ - ::testing::SafeMatcherCast(inner_matcher))); - } - --} // namespace oss --} // namespace testing -+} // namespace mbo::testing::proto - --#endif // PROTO_BUILDER_OSS_TESTING_PROTO_TEST_UTIL_H_ -+#endif // MBO_TESTING_PROTO_PROTO_TEST_UTIL_H_ diff --git a/mbo/proto/parse_text_proto.cc b/mbo/proto/parse_text_proto.cc index af864cf..f519642 100644 --- a/mbo/proto/parse_text_proto.cc +++ b/mbo/proto/parse_text_proto.cc @@ -22,11 +22,11 @@ #include "google/protobuf/io/tokenizer.h" #include "google/protobuf/text_format.h" #include "absl/base/log_severity.h" +#include "absl/log/absl_log.h" #include "absl/strings/str_format.h" #include "absl/strings/string_view.h" - -namespace mbo::proto::internal { +namespace mbo::proto::proto_internal { namespace { // Collects errors from proto parsing. @@ -39,36 +39,38 @@ class SilentErrorCollector : public google::protobuf::io::ErrorCollector { absl::LogSeverity severity = absl::LogSeverity::kError; }; - // NOLINTNEXTLINE(bugprone-easily-swappable-parameters) +#if GOOGLE_PROTOBUF_VERSION >= 5028000 + void RecordError(int line, int column, std::string_view message) override { +#else void AddError(int line, int column, const std::string& message) override { - errors_.push_back({ - .line = line, - .column = column, - .message = message, - .severity = absl::LogSeverity::kError, - }); +#endif + errors_.push_back({.line = line, + .column = column, + .message = std::string(message), + .severity = absl::LogSeverity::kError}); } - // NOLINTNEXTLINE(bugprone-easily-swappable-parameters) +#if GOOGLE_PROTOBUF_VERSION >= 5028000 + void RecordWarning(int line, int column, std::string_view message) override { +#else void AddWarning(int line, int column, const std::string& message) override { - errors_.push_back({ - .line = line, - .column = column, - .message = message, - .severity = absl::LogSeverity::kWarning, - }); +#endif + errors_.push_back({.line = line, + .column = column, + .message = std::string(message), + .severity = absl::LogSeverity::kWarning}); } - std::string GetErrorStr() const; - const std::vector& GetErrors() const { return errors_; } + std::string GetErrors() const; + const std::vector& errors() const { return errors_; } private: std::vector errors_; }; -std::string SilentErrorCollector::GetErrorStr() const { +std::string SilentErrorCollector::GetErrors() const { std::string result; - for (const auto& error : GetErrors()) { + for (const auto& error : errors()) { absl::StrAppendFormat(&result, "Line %d, Col %d: %s\n", error.line, error.column, error.message); } @@ -77,23 +79,28 @@ std::string SilentErrorCollector::GetErrorStr() const { } // namespace -absl::Status ParseTextInternal(std::string_view text, ::google::protobuf::Message* message, - std::source_location loc) { +absl::Status ParseTextInternal(std::string_view text_proto, ::google::protobuf::Message* message, + std::string_view func, std::source_location loc) { google::protobuf::TextFormat::Parser parser; SilentErrorCollector error_collector; parser.RecordErrorsTo(&error_collector); - if (parser.ParseFromString(std::string(text), message)) { + if (parser.ParseFromString(std::string(text_proto), message)) { return absl::OkStatus(); } return absl::InvalidArgumentError( - absl::StrFormat("%s\nFile: '%s', Line: %d", error_collector.GetErrorStr(), - loc.file_name(), loc.line())); + absl::StrFormat("%s<%s>\nFile: '%s', Line: %d: %s\nError: %s", + func, message->GetDescriptor()->name(), loc.file_name(), loc.line(), loc.function_name(), error_collector.GetErrors())); } -void ParseTextOrDieInternal(std::string_view text, ::google::protobuf::Message* message, std::string_view func, std::source_location loc) { - absl::Status result = internal::ParseTextInternal(text, message, loc); - ABSL_CHECK_OK(result) << func << "<" << message->GetDescriptor()->name() << ">" - << " @" << loc.file_name() << ":" << loc.line() << ": " << result; +void ParseTextOrDieInternal( + std::string_view text, + ::google::protobuf::Message* message, + std::string_view func, + std::source_location loc) { + const absl::Status status = ParseTextInternal(text, message, func, loc); + if (!status.ok()) { + ABSL_LOG(FATAL) << "Check failed: " << status; + } } -} // namespace mbo::proto::internal +} // namespace mbo::proto::proto_internal diff --git a/mbo/proto/parse_text_proto.diff b/mbo/proto/parse_text_proto.diff deleted file mode 100644 index d7026bb..0000000 --- a/mbo/proto/parse_text_proto.diff +++ /dev/null @@ -1,208 +0,0 @@ -diff -du ../cpp-proto-builder/proto_builder/oss/parse_text_proto.cc proto/mbo/proto/parse_text_proto.cc ---- ../cpp-proto-builder/proto_builder/oss/parse_text_proto.cc 2023-07-15 17:24:10 -+++ proto/mbo/proto/parse_text_proto.cc 2023-07-15 20:58:22 -@@ -1,4 +1,5 @@ - // Copyright 2021 The CPP Proto Builder Authors -+// Copyright 2023 M.Boerger - // - // Licensed under the Apache License, Version 2.0 (the "License"); - // you may not use this file except in compliance with the License. -@@ -12,10 +13,9 @@ - // See the License for the specific language governing permissions and - // limitations under the License. - --// READ: https://google.github.io/cpp-proto-builder -+#include "mbo/proto/parse_text_proto.h" - --#include "proto_builder/oss/parse_text_proto.h" -- -+#include - #include - #include - -@@ -25,7 +25,7 @@ - #include "absl/strings/str_format.h" - #include "absl/strings/string_view.h" - --namespace proto_builder::oss { -+namespace mbo::proto { - namespace internal { - namespace { - -@@ -80,20 +80,18 @@ - - } // namespace - --absl::Status ParseTextInternal(absl::string_view text_proto, Message* message, -- SourceLocation loc) { -+absl::Status ParseTextInternal(std::string_view text_proto, ::google::protobuf::Message* message, -+ std::source_location loc) { - google::protobuf::TextFormat::Parser parser; - SilentErrorCollector error_collector; - parser.RecordErrorsTo(&error_collector); - if (parser.ParseFromString(std::string(text_proto), message)) { - return absl::OkStatus(); - } -- // TODO When SourceLocation becomes open source, pass loc to error -- // status. Until then, append loc to error message. - return absl::InvalidArgumentError( - absl::StrFormat("%s\nFile: '%s', Line: %d", error_collector.GetErrors(), - loc.file_name(), loc.line())); - } - - } // namespace internal --} // namespace proto_builder::oss -+} // namespace mbo::proto -diff -du ../cpp-proto-builder/proto_builder/oss/parse_text_proto.h proto/mbo/proto/parse_text_proto.h ---- ../cpp-proto-builder/proto_builder/oss/parse_text_proto.h 2023-07-15 17:24:10 -+++ proto/mbo/proto/parse_text_proto.h 2023-07-15 20:56:57 -@@ -1,4 +1,5 @@ - // Copyright 2021 The CPP Proto Builder Authors -+// Copyright 2023 M.Boerger - // - // Licensed under the Apache License, Version 2.0 (the "License"); - // you may not use this file except in compliance with the License. -@@ -12,54 +13,46 @@ - // See the License for the specific language governing permissions and - // limitations under the License. - --// READ: https://google.github.io/cpp-proto-builder -+#ifndef MBO_PROTO_PARSE_TEXT_PROTO_H_ -+#define MBO_PROTO_PARSE_TEXT_PROTO_H_ - --#ifndef PROTO_BUILDER_OSS_PARSE_TEXT_PROTO_H_ --#define PROTO_BUILDER_OSS_PARSE_TEXT_PROTO_H_ -- -+#include - #include -+#include - --#include "proto_builder/oss/qcheck.h" --#include "proto_builder/oss/source_location.h" --#include "google/protobuf/message.h" -+#include "absl/log/absl_check.h" - #include "absl/status/status.h" - #include "absl/strings/str_format.h" - #include "absl/strings/string_view.h" -+#include "google/protobuf/message.h" - --namespace proto_builder::oss { -- --using ::google::protobuf::Message; -- -+namespace mbo::proto { - namespace internal { - --absl::Status ParseTextInternal(absl::string_view text_proto, Message* message, -- SourceLocation loc); -+absl::Status ParseTextInternal(std::string_view text_proto, ::google::protobuf::Message* message, -+ std::source_location loc); - - } // namespace internal - - // Parses the text in 'text_proto' into a prot message of type 'T'. - // The function dies if parsing fails. - template --T ParseTextOrDie(absl::string_view text_proto, -- SourceLocation loc = SourceLocation::current()) { -+T ParseTextOrDie(std::string_view text_proto, -+ std::source_location loc = std::source_location::current()) { - T message; - absl::Status result = internal::ParseTextInternal(text_proto, &message, loc); -- QCheck( -- result.ok(), -- absl::StrFormat("ParseTextOrDie<%s>", T::GetDescriptor()->name()).c_str(), -- QCheckOptions::kDefault, loc) -- << result; -+ ABSL_CHECK_OK(result) << "ParseTextOrDie<" << T::GetDescriptor()->name() << ">" -+ << " @" << loc.file_name() << ":" << loc.line() << ": " << result; - return message; - } - - class ParseTextProtoHelper final { - public: -- ParseTextProtoHelper(absl::string_view text_proto, SourceLocation loc) -+ ParseTextProtoHelper(std::string_view text_proto, std::source_location loc) - : text_proto_(text_proto), loc_(loc), parsed_(false) {} - - ~ParseTextProtoHelper() { -- QCheck(parsed_, -- absl::StrFormat("ParseTextProtoOrDie result unused").c_str()); -+ ABSL_CHECK(parsed_) << "ParseTextProtoOrDie result unused"; - } - - template -@@ -70,16 +63,16 @@ - - private: - const std::string text_proto_; -- const SourceLocation loc_; -+ const std::source_location loc_; - bool parsed_; - }; - - inline ParseTextProtoHelper ParseTextProtoOrDie( -- absl::string_view text_proto, -- SourceLocation loc = SourceLocation::current()) { -+ std::string_view text_proto, -+ std::source_location loc = std::source_location::current()) { - return ParseTextProtoHelper(text_proto, loc); - } - --} // namespace proto_builder::oss -+} // namespace mbo::proto - --#endif // PROTO_BUILDER_OSS_PARSE_TEXT_PROTO_H_ -+#endif // MBO_PROTO_PARSE_TEXT_PROTO_H_ -diff -du ../cpp-proto-builder/proto_builder/oss/parse_text_proto_test.cc proto/mbo/proto/parse_text_proto_test.cc ---- ../cpp-proto-builder/proto_builder/oss/parse_text_proto_test.cc 2023-07-15 17:24:10 -+++ proto/mbo/proto/parse_text_proto_test.cc 2023-07-15 21:04:22 -@@ -1,4 +1,5 @@ - // Copyright 2021 The CPP Proto Builder Authors -+// Copyright 2023 M.Boerger - // - // Licensed under the Apache License, Version 2.0 (the "License"); - // you may not use this file except in compliance with the License. -@@ -12,18 +13,17 @@ - // See the License for the specific language governing permissions and - // limitations under the License. - --// READ: https://google.github.io/cpp-proto-builder -+#include "mbo/proto/parse_text_proto.h" - --#include "proto_builder/oss/parse_text_proto.h" -- --#include "proto_builder/oss/tests/simple_message.pb.h" - #include "gmock/gmock.h" --#include "proto_builder/oss/testing/cpp_pb_gunit.h" -+#include "mbo/proto/tests/simple_message.pb.h" -+#include "mbo/testing/proto/proto_matchers.h" - --namespace proto_builder::oss { -+namespace mbo::proto { - namespace { - --using ::testing::oss::EqualsProto; -+using ::mbo::testing::proto::EqualsProto; -+using ::mbo::proto::tests::SimpleMessage; - - class ParseTextProtoTest : public ::testing::Test {}; - -@@ -35,12 +35,13 @@ - } - - TEST_F(ParseTextProtoTest, ParseError) { -- EXPECT_DEATH(ParseTextOrDie("!!!"), -+ EXPECT_DEATH(ParseTextOrDie("!!!"), - // Using [0-9] in lieu of \\d to be compatible in open source. -- "File: '.*/parse_text_proto_test.cc', Line: [0-9]+\n" -- "Check failed: ParseTextOrDie\n" -+ ".*Check failed:.*\n*" -+ "File: '.*/parse_text_proto.*', Line: [0-9]+.*" -+ "ParseTextOrDie.*" - "INVALID_ARGUMENT: Line 0, Col 0: Expected identifier, got: !"); - } - - } // namespace --} // namespace proto_builder::oss -+} // namespace mbo::proto diff --git a/mbo/proto/parse_text_proto.h b/mbo/proto/parse_text_proto.h index e482654..f11be1e 100644 --- a/mbo/proto/parse_text_proto.h +++ b/mbo/proto/parse_text_proto.h @@ -27,7 +27,7 @@ #include "google/protobuf/message.h" namespace mbo::proto { -namespace internal { +namespace proto_internal { void ParseTextOrDieInternal( std::string_view text, @@ -35,7 +35,7 @@ void ParseTextOrDieInternal( std::string_view func, std::source_location loc); -absl::Status ParseTextInternal(std::string_view text, ::google::protobuf::Message *message, std::source_location loc); +absl::Status ParseTextInternal(std::string_view text, ::google::protobuf::Message *message, std::string_view func, std::source_location loc); class ParseTextProtoHelper final { public: @@ -65,7 +65,7 @@ class ParseTextProtoHelper final { bool parsed_{false}; }; -} // namespace internal +} // namespace proto_internal // Parses the text in 'text_proto' into the proto message type requested as return type. // The function dies if parsing fails. Example: @@ -73,33 +73,35 @@ class ParseTextProtoHelper final { // ``` // MyProtoType message = ParseTextProtoOrDie(R"pb(field: 42)pb"); // ``` -inline internal::ParseTextProtoHelper ParseTextProtoOrDie( +inline proto_internal::ParseTextProtoHelper ParseTextProtoOrDie( std::string_view text_proto, std::source_location loc = std::source_location::current()) { return {text_proto, loc}; } -// Parses the text in 'text_proto' into a proto message of type 'T' and return it wrapped as StatusOr. -// If parsing fails, then an error status will be returned. +// Parses the text in 'text_proto' into a proto message of type 'T'. +// The function dies if parsing fails. +// Use this function only if the return type cannot be determined automatically. template requires(std::derived_from && !std::same_as) -absl::StatusOr ParseText(std::string_view text_proto, std::source_location loc = std::source_location::current()) { +inline T ParseTextOrDie(std::string_view text_proto, std::source_location loc = std::source_location::current()) { T message; - absl::Status result = internal::ParseTextInternal(text_proto, &message, loc); - if (!result.ok()) { - return result; - } + proto_internal::ParseTextOrDieInternal(text_proto, &message, "ParseTextOrDie", loc); return message; } -// Parses the text in 'text_proto' into a proto message of type 'T'. -// The function dies if parsing fails. -// Use this function only if the return type cannot be determined automatically. +// Parses the text in 'text_proto' into a proto message of type 'T' and return it wrapped as StatusOr. +// If parsing fails, then an error status will be returned. template requires(std::derived_from && !std::same_as) -T ParseTextOrDie(std::string_view text_proto, std::source_location loc = std::source_location::current()) { +inline absl::StatusOr ParseText(std::string_view text_proto, std::source_location loc = std::source_location::current()) { T message; - internal::ParseTextOrDieInternal(text_proto, &message, "ParseTextOrDie", loc); + absl::Status result = proto_internal::ParseTextInternal(text_proto, &message, "ParseText", loc); + if (!result.ok()) { + // We are missing the Google internal update mechanisms, so we may loose details here. + // But Google also did not bless the Abseil open-source versions with anything we could drop here... + return absl::Status(result.code(), absl::StrCat(result.message())); + } return message; } diff --git a/mbo/proto/parse_text_proto_test.cc b/mbo/proto/parse_text_proto_test.cc index 42a9919..077e312 100644 --- a/mbo/proto/parse_text_proto_test.cc +++ b/mbo/proto/parse_text_proto_test.cc @@ -24,6 +24,7 @@ namespace { using ::mbo::proto::EqualsProto; using ::mbo::proto::tests::SimpleMessage; +using ::testing::ContainsRegex; class ParseTextProtoTest : public ::testing::Test {}; @@ -36,10 +37,11 @@ TEST_F(ParseTextProtoTest, ParseTextOrDieFail) { EXPECT_DEATH( ParseTextOrDie("!!!"), // Using [0-9] in lieu of \\d to be compatible in open source. - ".*Check failed:.*\n*" - "File: '.*/parse_text_proto.*', Line: [0-9]+.*" - "ParseTextOrDie.*" - "INVALID_ARGUMENT: Line 0, Col 0: Expected identifier, got: !"); + ".*Check failed: " // Prefix + "INVALID_ARGUMENT: " // Status + "ParseTextOrDie.*\n.*" // Called function + "File: '.*/parse_text_proto.*', Line: [0-9]+.*\n.*" // Source + "Line 0, Col 0: Expected identifier, got: !.*"); // Error } TEST_F(ParseTextProtoTest, ParseTextProtoOrDiePass) { @@ -50,10 +52,28 @@ TEST_F(ParseTextProtoTest, ParseTextProtoOrDieFail) { EXPECT_DEATH( SimpleMessage msg = ParseTextProtoOrDie("!!!"), // Using [0-9] in lieu of \\d to be compatible in open source. - ".*Check failed:.*\n*" - "File: '.*/parse_text_proto.*', Line: [0-9]+.*" - "ParseTextProtoOrDie.*" - "INVALID_ARGUMENT: Line 0, Col 0: Expected identifier, got: !"); + ".*Check failed: " // Prefix + "INVALID_ARGUMENT: " // StatusCode + "ParseTextProtoOrDie.*\n.*" // Called function + "File: '.*/parse_text_proto.*', Line: [0-9]+.*\n.*" // Source + "Line 0, Col 0: Expected identifier, got: !.*"); // Error +} + +TEST_F(ParseTextProtoTest, ParseText) { + EXPECT_THAT(*ParseText("one: 25"), EqualsProto("one: 25")); + const absl::StatusOr status = ParseText("!!!"); + static constexpr std::string_view expected = // + // Using [0-9] in lieu of \\d to be compatible in open source. + "ParseText.*\n.*" // Called function + "File: '.*/parse_text_proto.*', Line: [0-9]+.*\n.*" // Source + "Line 0, Col 0: Expected identifier, got: !.*"; // Error + ASSERT_THAT(status.status().ok(), false); + EXPECT_THAT(status.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT(status.status().message(), ContainsRegex(expected)); + EXPECT_DEATH(auto msg = *ParseText("!!!"), absl::StrCat( + "RAW: Attempting to fetch value instead of handling error ", // Prefix + "INVALID_ARGUMENT: ", // StatusCode + expected)); } } // namespace