Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

* Changed ParseTextProto, ParseTextProtoOrDie to only work for actual derived proto types. #3

Merged
merged 1 commit into from
Jan 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .clang-format
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,20 @@ BreakBeforeBinaryOperators: NonAssignment
BreakBeforeConceptDeclarations: Always
BreakBeforeTernaryOperators: true
BreakInheritanceList: BeforeComma
DerivePointerAlignment: false
EmptyLineBeforeAccessModifier: Always
FixNamespaceComments: true
IndentRequiresClause: false
InsertBraces: true
InsertNewlineAtEOF: true
# InsertTrailingCommas: true <- Would change structs to never bin pack
## InsertTrailingCommas: true <- Would change structs to never bin pack
IntegerLiteralSeparator:
Binary: 0
Decimal: 3
KeepEmptyLinesAtTheStartOfBlocks: false
MaxEmptyLinesToKeep: 1
PackConstructorInitializers: NextLine
PenaltyReturnTypeOnItsOwnLine: 1000
PointerAlignment: Left
QualifierAlignment: Left
ReferenceAlignment: Left
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@
/compile_commands.json
# Ignore the directory in which `clangd` stores its local index.
/.cache/
/.vscode/
13 changes: 13 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v2.3.0
hooks:
- id: check-merge-conflict
- id: check-yaml
- id: end-of-file-fixer
- id: trailing-whitespace

- repo: https://github.com/pre-commit/mirrors-clang-format
rev: v16.0.4
hooks:
- id: clang-format
27 changes: 27 additions & 0 deletions BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,3 +1,30 @@
# Copyright 2024 M.Boerger
#
# 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
#
# http://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.

load("@hedron_compile_commands//:refresh_compile_commands.bzl", "refresh_compile_commands")

package(default_visibility = ["//visibility:private"])

licenses(["notice"])

COM_GOOGLE_PROTOBUF="bazel-out/../../../external/com_google_protobuf/src"

refresh_compile_commands(
name = "refresh_all",
targets = {
"//...": " ".join([
"--cxxopt=-I" + COM_GOOGLE_PROTOBUF,
])
},
)
17 changes: 17 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# 0.4

* Changed `ParseTextProto`, `ParseTextProtoOrDie` to only work for actual derived proto types.
* Added `ParseProto` which returns `absl::StatusOr<MessageType>`.
* Updated external dependencies.

# 0.3

* Added missing dependency.

# 0.2

* Small fixes, mostly to support a few older abseil/re2 versions.

# 0.1

* Initial release
4 changes: 4 additions & 0 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ load(":workspace.bzl", "workspace_load_modules")

workspace_load_modules()

load("@bazel_skylib//:workspace.bzl", "bazel_skylib_workspace")

bazel_skylib_workspace()

load("@com_google_protobuf//:protobuf_deps.bzl", "protobuf_deps")

protobuf_deps()
1 change: 1 addition & 0 deletions mbo/proto/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ cc_library(
"@com_google_absl//absl/log:absl_check",
"@com_google_absl//absl/log:absl_log",
"@com_google_absl//absl/status",
"@com_google_absl//absl/status:statusor",
"@com_google_absl//absl/strings:str_format",
"@com_google_absl//absl/strings",
"@com_google_protobuf//:protobuf",
Expand Down
72 changes: 37 additions & 35 deletions mbo/proto/parse_text_proto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,53 +25,50 @@
#include "absl/strings/str_format.h"
#include "absl/strings/string_view.h"

namespace mbo::proto {
namespace internal {

namespace mbo::proto::internal {
namespace {

// Collects errors from proto parsing.
class SilentErrorCollector : public google::protobuf::io::ErrorCollector {
public:
struct ErrorInfo {
int line;
int column;
int line = 0;
int column = 0;
std::string message;
absl::LogSeverity severity = absl::LogSeverity::kError;
};

void AddError(int line, int column, const std::string& message) override;
void AddWarning(int line, int column, const std::string& message) override;
// NOLINTNEXTLINE(bugprone-easily-swappable-parameters)
void AddError(int line, int column, const std::string& message) override {
errors_.push_back({
.line = line,
.column = column,
.message = message,
.severity = absl::LogSeverity::kError,
});
}

std::string GetErrors() const;
const std::vector<ErrorInfo>& errors() const { return errors_; }
// NOLINTNEXTLINE(bugprone-easily-swappable-parameters)
void AddWarning(int line, int column, const std::string& message) override {
errors_.push_back({
.line = line,
.column = column,
.message = message,
.severity = absl::LogSeverity::kWarning,
});
}

std::string GetErrorStr() const;
const std::vector<ErrorInfo>& GetErrors() const { return errors_; }

private:
std::vector<ErrorInfo> errors_;
};

void SilentErrorCollector::AddError(int line, int column,
const std::string& message) {
ErrorInfo error;
error.line = line;
error.column = column;
error.message = message;
error.severity = absl::LogSeverity::kError;
errors_.push_back(error);
}

void SilentErrorCollector::AddWarning(int line, int column,
const std::string& message) {
ErrorInfo error;
error.line = line;
error.column = column;
error.message = message;
error.severity = absl::LogSeverity::kWarning;
errors_.push_back(error);
}

std::string SilentErrorCollector::GetErrors() const {
std::string SilentErrorCollector::GetErrorStr() const {
std::string result;
for (const auto& error : errors()) {
for (const auto& error : GetErrors()) {
absl::StrAppendFormat(&result, "Line %d, Col %d: %s\n", error.line,
error.column, error.message);
}
Expand All @@ -80,18 +77,23 @@ std::string SilentErrorCollector::GetErrors() const {

} // namespace

absl::Status ParseTextInternal(std::string_view text_proto, ::google::protobuf::Message* message,
absl::Status ParseTextInternal(std::string_view text, ::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)) {
if (parser.ParseFromString(std::string(text), message)) {
return absl::OkStatus();
}
return absl::InvalidArgumentError(
absl::StrFormat("%s\nFile: '%s', Line: %d", error_collector.GetErrors(),
absl::StrFormat("%s\nFile: '%s', Line: %d", error_collector.GetErrorStr(),
loc.file_name(), loc.line()));
}

} // namespace internal
} // namespace mbo::proto
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;
}

} // namespace mbo::proto::internal
82 changes: 56 additions & 26 deletions mbo/proto/parse_text_proto.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,55 +22,85 @@

#include "absl/log/absl_check.h"
#include "absl/status/status.h"
#include "absl/strings/str_format.h"
#include "absl/status/statusor.h"
#include "absl/strings/string_view.h"
#include "google/protobuf/message.h"

namespace mbo::proto {
namespace internal {

absl::Status ParseTextInternal(std::string_view text_proto, ::google::protobuf::Message* message,
std::source_location loc);
void ParseTextOrDieInternal(
std::string_view text,
::google::protobuf::Message* message,
std::string_view func,
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 <typename T>
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);
ABSL_CHECK_OK(result) << "ParseTextOrDie<" << T::GetDescriptor()->name() << ">"
<< " @" << loc.file_name() << ":" << loc.line() << ": " << result;
return message;
}
absl::Status ParseTextInternal(std::string_view text, ::google::protobuf::Message *message, std::source_location loc);

class ParseTextProtoHelper final {
public:
ParseTextProtoHelper(std::string_view text_proto, std::source_location loc)
: text_proto_(text_proto), loc_(loc), parsed_(false) {}
~ParseTextProtoHelper() noexcept { ABSL_CHECK(parsed_) << "ParseTextProtoOrDie<T> result unused"; }

~ParseTextProtoHelper() {
ABSL_CHECK(parsed_) << "ParseTextProtoOrDie<T> result unused";
}
ParseTextProtoHelper(std::string_view text_proto, std::source_location loc) noexcept
: text_proto_(text_proto), loc_(loc) {}

// This is a purely temporary object... no copy or move may be used.
ParseTextProtoHelper(const ParseTextProtoHelper&) noexcept = delete;
ParseTextProtoHelper& operator=(const ParseTextProtoHelper&) noexcept = delete;
ParseTextProtoHelper(ParseTextProtoHelper&&) noexcept = delete;
ParseTextProtoHelper& operator=(ParseTextProtoHelper&&) noexcept = delete;

template <typename T>
template<typename T>
requires(std::derived_from<T, ::google::protobuf::Message> && !std::same_as<T, ::google::protobuf::Message>)
operator T() { // NOLINT clangtidy(google-explicit-constructor)
parsed_ = true;
return ParseTextOrDie<T>(text_proto_, loc_);
T message;
ParseTextOrDieInternal(text_proto_, &message, "ParseTextProtoOrDie", loc_);
return message;
}

private:
const std::string text_proto_;
const std::source_location loc_;
bool parsed_;
bool parsed_{false};
};

inline ParseTextProtoHelper ParseTextProtoOrDie(
} // namespace internal

// Parses the text in 'text_proto' into the proto message type requested as return type.
// The function dies if parsing fails. Example:
//
// ```
// MyProtoType message = ParseTextProtoOrDie(R"pb(field: 42)pb");
// ```
inline internal::ParseTextProtoHelper ParseTextProtoOrDie(
std::string_view text_proto,
std::source_location loc = std::source_location::current()) {
return ParseTextProtoHelper(text_proto, loc);
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.
template<typename T>
requires(std::derived_from<T, ::google::protobuf::Message> && !std::same_as<T, ::google::protobuf::Message>)
absl::StatusOr<T> ParseText(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;
}
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.
template<typename T>
requires(std::derived_from<T, ::google::protobuf::Message> && !std::same_as<T, ::google::protobuf::Message>)
T ParseTextOrDie(std::string_view text_proto, std::source_location loc = std::source_location::current()) {
T message;
internal::ParseTextOrDieInternal(text_proto, &message, "ParseTextOrDie", loc);
return message;
}

} // namespace mbo::proto
Expand Down
19 changes: 16 additions & 3 deletions mbo/proto/parse_text_proto_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,12 @@ using ::mbo::proto::tests::SimpleMessage;

class ParseTextProtoTest : public ::testing::Test {};

TEST_F(ParseTextProtoTest, ParseOk) {
TEST_F(ParseTextProtoTest, ParseTextOrDiePass) {
EXPECT_THAT(ParseTextOrDie<SimpleMessage>(""), EqualsProto(""));
EXPECT_THAT(ParseTextOrDie<SimpleMessage>("one: 42"), EqualsProto("one: 42"));
EXPECT_THAT((SimpleMessage)ParseTextProtoOrDie("one: 25"), EqualsProto("one: 25"));
}

TEST_F(ParseTextProtoTest, ParseError) {
TEST_F(ParseTextProtoTest, ParseTextOrDieFail) {
EXPECT_DEATH(
ParseTextOrDie<SimpleMessage>("!!!"),
// Using [0-9] in lieu of \\d to be compatible in open source.
Expand All @@ -43,5 +42,19 @@ TEST_F(ParseTextProtoTest, ParseError) {
"INVALID_ARGUMENT: Line 0, Col 0: Expected identifier, got: !");
}

TEST_F(ParseTextProtoTest, ParseTextProtoOrDiePass) {
EXPECT_THAT((SimpleMessage)ParseTextProtoOrDie("one: 25"), EqualsProto("one: 25"));
}

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<SimpleMessage>.*"
"INVALID_ARGUMENT: Line 0, Col 0: Expected identifier, got: !");
}

} // namespace
} // namespace mbo::proto
Loading