Automated rollback of commit 8c2416311f7b2c5410e620197baacf9d82b86ebd.
PiperOrigin-RevId: 580960903
diff --git a/src/google/protobuf/message.h b/src/google/protobuf/message.h
index 27a5772..8630cdc 100644
--- a/src/google/protobuf/message.h
+++ b/src/google/protobuf/message.h
@@ -135,7 +135,6 @@
class MapValueRef;
class MapIterator;
class MapReflectionTester;
-class TextFormat;
namespace internal {
struct FuzzPeer;
@@ -1011,7 +1010,6 @@
return schema_.IsSplit(field);
}
- friend class google::protobuf::TextFormat;
friend class FastReflectionBase;
friend class FastReflectionMessageMutator;
friend bool internal::IsDescendant(Message& root, const Message& message);
diff --git a/src/google/protobuf/text_format.cc b/src/google/protobuf/text_format.cc
index e29b07b..9abd847 100644
--- a/src/google/protobuf/text_format.cc
+++ b/src/google/protobuf/text_format.cc
@@ -295,14 +295,6 @@
}
} // namespace
-const void* TextFormat::Parser::UnsetFieldsMetadata::GetUnsetFieldAddress(
- const Message& message, const Reflection& reflection,
- const FieldDescriptor& fd) {
- // reflection->GetRaw() is a simple cast for any non-repeated type, so for
- // simplicity we just pass in char as the template argument.
- return &reflection.GetRaw<char>(message, &fd);
-}
-
// ===========================================================================
// Internal class for parsing an ASCII representation of a Protocol Message.
// This class makes use of the Protocol Message compiler's tokenizer found
@@ -339,7 +331,7 @@
bool allow_unknown_extension, bool allow_unknown_enum,
bool allow_field_number, bool allow_relaxed_whitespace,
bool allow_partial, int recursion_limit,
- UnsetFieldsMetadata* no_op_fields)
+ bool error_on_no_op_fields)
: error_collector_(error_collector),
finder_(finder),
parse_info_tree_(parse_info_tree),
@@ -357,7 +349,7 @@
recursion_limit_(recursion_limit),
had_silent_marker_(false),
had_errors_(false),
- no_op_fields_(no_op_fields) {
+ error_on_no_op_fields_(error_on_no_op_fields) {
// For backwards-compatibility with proto1, we need to allow the 'f' suffix
// for floats.
tokenizer_.set_allow_f_after_float(true);
@@ -842,21 +834,19 @@
// When checking for no-op operations, We verify that both the existing value in
// the message and the new value are the default. If the existing field value is
// not the default, setting it to the default should not be treated as a no-op.
-// The pointer of this is kept in no_op_fields_ for bookkeeping.
-#define SET_FIELD(CPPTYPE, CPPTYPELCASE, VALUE) \
- if (field->is_repeated()) { \
- reflection->Add##CPPTYPE(message, field, VALUE); \
- } else { \
- if (no_op_fields_ && !field->has_presence() && \
- field->default_value_##CPPTYPELCASE() == \
- reflection->Get##CPPTYPE(*message, field) && \
- field->default_value_##CPPTYPELCASE() == VALUE) { \
- no_op_fields_->addresses_.insert( \
- UnsetFieldsMetadata::GetUnsetFieldAddress(*message, *reflection, \
- *field)); \
- } else { \
- reflection->Set##CPPTYPE(message, field, std::move(VALUE)); \
- } \
+#define SET_FIELD(CPPTYPE, CPPTYPELCASE, VALUE) \
+ if (field->is_repeated()) { \
+ reflection->Add##CPPTYPE(message, field, VALUE); \
+ } else { \
+ if (error_on_no_op_fields_ && !field->has_presence() && \
+ field->default_value_##CPPTYPELCASE() == \
+ reflection->Get##CPPTYPE(*message, field) && \
+ field->default_value_##CPPTYPELCASE() == VALUE) { \
+ ReportError("Input field " + field->full_name() + \
+ " did not change resulting proto."); \
+ } else { \
+ reflection->Set##CPPTYPE(message, field, std::move(VALUE)); \
+ } \
}
switch (field->cpp_type()) {
@@ -1439,7 +1429,7 @@
int recursion_limit_;
bool had_silent_marker_;
bool had_errors_;
- UnsetFieldsMetadata* no_op_fields_{};
+ bool error_on_no_op_fields_;
};
@@ -1737,7 +1727,7 @@
allow_case_insensitive_field_, allow_unknown_field_,
allow_unknown_extension_, allow_unknown_enum_,
allow_field_number_, allow_relaxed_whitespace_,
- allow_partial_, recursion_limit_, no_op_fields_);
+ allow_partial_, recursion_limit_, error_on_no_op_fields_);
return MergeUsingImpl(input, output, &parser);
}
@@ -1762,7 +1752,7 @@
allow_case_insensitive_field_, allow_unknown_field_,
allow_unknown_extension_, allow_unknown_enum_,
allow_field_number_, allow_relaxed_whitespace_,
- allow_partial_, recursion_limit_, no_op_fields_);
+ allow_partial_, recursion_limit_, error_on_no_op_fields_);
return MergeUsingImpl(input, output, &parser);
}
@@ -1798,7 +1788,7 @@
allow_case_insensitive_field_, allow_unknown_field_,
allow_unknown_extension_, allow_unknown_enum_,
allow_field_number_, allow_relaxed_whitespace_,
- allow_partial_, recursion_limit_, no_op_fields_);
+ allow_partial_, recursion_limit_, error_on_no_op_fields_);
return parser.ParseField(field, output);
}
diff --git a/src/google/protobuf/text_format.h b/src/google/protobuf/text_format.h
index e16ed98..f851c91 100644
--- a/src/google/protobuf/text_format.h
+++ b/src/google/protobuf/text_format.h
@@ -21,7 +21,6 @@
#include <vector>
#include "absl/container/flat_hash_map.h"
-#include "absl/container/flat_hash_set.h"
#include "absl/strings/cord.h"
#include "absl/strings/string_view.h"
#include "google/protobuf/descriptor.h"
@@ -82,9 +81,6 @@
// Converts a protobuf message to a string with redaction enabled.
PROTOBUF_EXPORT std::string StringifyMessage(const Message& message,
Option option);
-
-class UnsetFieldsMetadataTextFormatTestUtil;
-class UnsetFieldsMetadataMessageDifferencerTestUtil;
} // namespace internal
// This class implements protocol buffer text format, colloquially known as text
@@ -723,40 +719,11 @@
// the maximum allowed nesting of proto messages.
void SetRecursionLimit(int limit) { recursion_limit_ = limit; }
- // Uniquely addresses fields in a message that was explicitly unset in
- // textproto. Example:
- // "some_int_field: 0"
- // where some_int_field is non-optional.
- //
- // This class should only be used to pass data between the text_format
- // parser and the MessageDifferencer.
- class UnsetFieldsMetadata {
- public:
- UnsetFieldsMetadata() = default;
-
- private:
- // Return a pointer to the unset field in the given message.
- static const void* GetUnsetFieldAddress(const Message& message,
- const Reflection& reflection,
- const FieldDescriptor& fd);
-
- // List of addresses of explicitly unset proto fields.
- absl::flat_hash_set<const void*> addresses_;
-
- friend class ::google::protobuf::internal::
- UnsetFieldsMetadataMessageDifferencerTestUtil;
- friend class ::google::protobuf::internal::UnsetFieldsMetadataTextFormatTestUtil;
- friend class ::google::protobuf::util::MessageDifferencer;
- friend class ::google::protobuf::TextFormat::Parser;
- };
-
- // If called, the parser will report the parsed fields that had no
+ // If called, the parser will report an error if a parsed field had no
// effect on the resulting proto (for example, fields with no presence that
- // were set to their default value). These can be passed to the Partially()
- // matcher as an indicator to explicitly check these fields are missing
- // in the actual.
- void OutputNoOpFields(UnsetFieldsMetadata* no_op_fields) {
- no_op_fields_ = no_op_fields;
+ // were set to their default value).
+ void ErrorOnNoOpFields(bool return_error) {
+ error_on_no_op_fields_ = return_error;
}
private:
@@ -781,7 +748,7 @@
bool allow_relaxed_whitespace_;
bool allow_singular_overwrites_;
int recursion_limit_;
- UnsetFieldsMetadata* no_op_fields_ = nullptr;
+ bool error_on_no_op_fields_ = false;
};
diff --git a/src/google/protobuf/text_format_unittest.cc b/src/google/protobuf/text_format_unittest.cc
index 13fbf33..9f60c9f 100644
--- a/src/google/protobuf/text_format_unittest.cc
+++ b/src/google/protobuf/text_format_unittest.cc
@@ -15,11 +15,9 @@
#include <stdlib.h>
#include <atomic>
-#include <cstdint>
#include <limits>
#include <memory>
#include <string>
-#include <vector>
#include "google/protobuf/testing/file.h"
#include "google/protobuf/testing/file.h"
@@ -36,7 +34,6 @@
#include "absl/strings/str_format.h"
#include "absl/strings/str_replace.h"
#include "absl/strings/substitute.h"
-#include "google/protobuf/descriptor.h"
#include "google/protobuf/io/tokenizer.h"
#include "google/protobuf/io/zero_copy_stream_impl.h"
#include "google/protobuf/map_unittest.pb.h"
@@ -55,33 +52,10 @@
namespace google {
namespace protobuf {
-namespace internal {
-class UnsetFieldsMetadataTextFormatTestUtil {
- public:
- static std::vector<const void*> GetRawAddresses(
- const TextFormat::Parser::UnsetFieldsMetadata& metadata) {
- return std::vector<const void*>(metadata.addresses_.begin(),
- metadata.addresses_.end());
- }
- static const void* GetAddress(const Message& message,
- const Reflection& reflection,
- const FieldDescriptor& fd) {
- return TextFormat::Parser::UnsetFieldsMetadata::GetUnsetFieldAddress(
- message, reflection, fd);
- }
-
- static int32_t NumFields(
- const TextFormat::Parser::UnsetFieldsMetadata& metadata) {
- return metadata.addresses_.size();
- }
-};
-} // namespace internal
-
// Can't use an anonymous namespace here due to brokenness of Tru64 compiler.
namespace text_format_unittest {
using ::google::protobuf::internal::kDebugStringSilentMarker;
-using ::google::protobuf::internal::UnsetFieldsMetadataTextFormatTestUtil;
using ::testing::AllOf;
using ::testing::HasSubstr;
@@ -1023,207 +997,86 @@
EXPECT_EQ(-2147483648, proto.repeated_nested_enum(3));
}
-TEST_F(TextFormatTest, PopulatesNoOpFields) {
+TEST_F(TextFormatTest, ErrorOnNoOpFieldsProto3) {
proto3_unittest::TestAllTypes proto;
TextFormat::Parser parser;
- TextFormat::Parser::UnsetFieldsMetadata no_op_fields;
- parser.OutputNoOpFields(&no_op_fields);
+ parser.ErrorOnNoOpFields(true);
{
- no_op_fields = {};
const absl::string_view singular_int_parse_string = "optional_int32: 0";
EXPECT_TRUE(TextFormat::ParseFromString(singular_int_parse_string, &proto));
- EXPECT_TRUE(parser.ParseFromString(singular_int_parse_string, &proto));
- EXPECT_EQ(UnsetFieldsMetadataTextFormatTestUtil::NumFields(no_op_fields),
- 1);
+ EXPECT_FALSE(parser.ParseFromString(singular_int_parse_string, &proto));
}
{
- no_op_fields = {};
const absl::string_view singular_bool_parse_string = "optional_bool: false";
EXPECT_TRUE(
TextFormat::ParseFromString(singular_bool_parse_string, &proto));
- EXPECT_TRUE(parser.ParseFromString(singular_bool_parse_string, &proto));
- EXPECT_EQ(UnsetFieldsMetadataTextFormatTestUtil::NumFields(no_op_fields),
- 1);
+ EXPECT_FALSE(parser.ParseFromString(singular_bool_parse_string, &proto));
}
{
- no_op_fields = {};
const absl::string_view singular_string_parse_string =
"optional_string: ''";
EXPECT_TRUE(
TextFormat::ParseFromString(singular_string_parse_string, &proto));
- EXPECT_TRUE(parser.ParseFromString(singular_string_parse_string, &proto));
- EXPECT_EQ(UnsetFieldsMetadataTextFormatTestUtil::NumFields(no_op_fields),
- 1);
+ EXPECT_FALSE(parser.ParseFromString(singular_string_parse_string, &proto));
}
{
- no_op_fields = {};
const absl::string_view nested_message_parse_string =
"optional_nested_message { bb: 0 } ";
EXPECT_TRUE(
TextFormat::ParseFromString(nested_message_parse_string, &proto));
- EXPECT_TRUE(parser.ParseFromString(nested_message_parse_string, &proto));
- EXPECT_EQ(UnsetFieldsMetadataTextFormatTestUtil::NumFields(no_op_fields),
- 1);
+ EXPECT_FALSE(parser.ParseFromString(nested_message_parse_string, &proto));
}
{
- no_op_fields = {};
- const absl::string_view nested_message_parse_string =
- "optional_nested_message { bb: 1 } ";
- EXPECT_TRUE(
- TextFormat::ParseFromString(nested_message_parse_string, &proto));
- EXPECT_TRUE(parser.ParseFromString(nested_message_parse_string, &proto));
- EXPECT_EQ(UnsetFieldsMetadataTextFormatTestUtil::NumFields(no_op_fields),
- 0);
- }
- {
- no_op_fields = {};
const absl::string_view foreign_message_parse_string =
"optional_foreign_message { c: 0 } ";
EXPECT_TRUE(
TextFormat::ParseFromString(foreign_message_parse_string, &proto));
- EXPECT_TRUE(parser.ParseFromString(foreign_message_parse_string, &proto));
- EXPECT_EQ(UnsetFieldsMetadataTextFormatTestUtil::NumFields(no_op_fields),
- 1);
+ EXPECT_FALSE(parser.ParseFromString(foreign_message_parse_string, &proto));
}
{
- no_op_fields = {};
const absl::string_view nested_enum_parse_string =
"optional_nested_enum: ZERO ";
EXPECT_TRUE(TextFormat::ParseFromString(nested_enum_parse_string, &proto));
- EXPECT_TRUE(parser.ParseFromString(nested_enum_parse_string, &proto));
- EXPECT_EQ(UnsetFieldsMetadataTextFormatTestUtil::NumFields(no_op_fields),
- 1);
+ EXPECT_FALSE(parser.ParseFromString(nested_enum_parse_string, &proto));
}
{
- no_op_fields = {};
const absl::string_view foreign_enum_parse_string =
"optional_foreign_enum: FOREIGN_ZERO ";
EXPECT_TRUE(TextFormat::ParseFromString(foreign_enum_parse_string, &proto));
- EXPECT_TRUE(parser.ParseFromString(foreign_enum_parse_string, &proto));
- EXPECT_EQ(UnsetFieldsMetadataTextFormatTestUtil::NumFields(no_op_fields),
- 1);
+ EXPECT_FALSE(parser.ParseFromString(foreign_enum_parse_string, &proto));
}
{
- no_op_fields = {};
const absl::string_view string_piece_parse_string =
"optional_string_piece: '' ";
EXPECT_TRUE(TextFormat::ParseFromString(string_piece_parse_string, &proto));
- EXPECT_TRUE(parser.ParseFromString(string_piece_parse_string, &proto));
- EXPECT_EQ(UnsetFieldsMetadataTextFormatTestUtil::NumFields(no_op_fields),
- 1);
+ EXPECT_FALSE(parser.ParseFromString(string_piece_parse_string, &proto));
}
{
- no_op_fields = {};
const absl::string_view cord_parse_string = "optional_cord: '' ";
EXPECT_TRUE(TextFormat::ParseFromString(cord_parse_string, &proto));
- EXPECT_TRUE(parser.ParseFromString(cord_parse_string, &proto));
- EXPECT_EQ(UnsetFieldsMetadataTextFormatTestUtil::NumFields(no_op_fields),
- 1);
+ EXPECT_FALSE(parser.ParseFromString(cord_parse_string, &proto));
}
{
- no_op_fields = {};
// Sanity check that repeated fields work the same.
const absl::string_view repeated_int32_parse_string = "repeated_int32: 0 ";
EXPECT_TRUE(
TextFormat::ParseFromString(repeated_int32_parse_string, &proto));
EXPECT_TRUE(parser.ParseFromString(repeated_int32_parse_string, &proto));
- EXPECT_EQ(UnsetFieldsMetadataTextFormatTestUtil::NumFields(no_op_fields),
- 0);
}
{
- no_op_fields = {};
const absl::string_view repeated_bool_parse_string =
"repeated_bool: false ";
EXPECT_TRUE(
TextFormat::ParseFromString(repeated_bool_parse_string, &proto));
EXPECT_TRUE(parser.ParseFromString(repeated_bool_parse_string, &proto));
- EXPECT_EQ(UnsetFieldsMetadataTextFormatTestUtil::NumFields(no_op_fields),
- 0);
}
{
- no_op_fields = {};
const absl::string_view repeated_string_parse_string =
"repeated_string: '' ";
EXPECT_TRUE(
TextFormat::ParseFromString(repeated_string_parse_string, &proto));
EXPECT_TRUE(parser.ParseFromString(repeated_string_parse_string, &proto));
- EXPECT_EQ(UnsetFieldsMetadataTextFormatTestUtil::NumFields(no_op_fields),
- 0);
- }
-}
-
-TEST_F(TextFormatTest, FieldsPopulatedCorrectly) {
- proto3_unittest::TestAllTypes proto;
- TextFormat::Parser parser;
- TextFormat::Parser::UnsetFieldsMetadata no_op_fields;
- parser.OutputNoOpFields(&no_op_fields);
- {
- no_op_fields = {};
- const absl::string_view parse_string = R"pb(
- optional_int32: 0
- optional_uint32: 10
- optional_nested_message { bb: 0 }
- )pb";
- EXPECT_TRUE(parser.ParseFromString(parse_string, &proto));
- ASSERT_EQ(UnsetFieldsMetadataTextFormatTestUtil::NumFields(no_op_fields),
- 2);
- std::vector<const void*> ptrs =
- UnsetFieldsMetadataTextFormatTestUtil::GetRawAddresses(no_op_fields);
- EXPECT_EQ(*static_cast<const int32_t*>(ptrs[0]), 0);
- EXPECT_EQ(*static_cast<const int32_t*>(ptrs[1]), 0);
- proto.set_optional_int32(20);
- proto.mutable_optional_nested_message()->set_bb(30);
- const std::vector<int32_t> new_values{
- *static_cast<const int32_t*>(ptrs[0]),
- *static_cast<const int32_t*>(ptrs[1])};
- EXPECT_THAT(new_values, testing::UnorderedElementsAre(20, 30));
- }
- {
- no_op_fields = {};
- const absl::string_view parse_string = R"pb(
- optional_bool: false
- optional_uint32: 10
- optional_nested_message { bb: 20 }
- )pb";
- EXPECT_TRUE(parser.ParseFromString(parse_string, &proto));
- ASSERT_EQ(UnsetFieldsMetadataTextFormatTestUtil::NumFields(no_op_fields),
- 1);
- std::vector<const void*> ptrs =
- UnsetFieldsMetadataTextFormatTestUtil::GetRawAddresses(no_op_fields);
- EXPECT_EQ(*static_cast<const bool*>(ptrs[0]), false);
- proto.set_optional_bool(true);
- EXPECT_EQ(*static_cast<const bool*>(ptrs[0]), true);
- }
- {
- // The address returned by the field is a string_view, which is a separate
- // alocation. Check address directly.
- no_op_fields = {};
- const absl::string_view parse_string = "optional_string: \"\"";
- EXPECT_TRUE(parser.ParseFromString(parse_string, &proto));
- ASSERT_EQ(UnsetFieldsMetadataTextFormatTestUtil::NumFields(no_op_fields),
- 1);
- const Reflection* reflection = proto.GetReflection();
- const FieldDescriptor* field = proto.GetDescriptor()->FindFieldByNumber(14);
- EXPECT_EQ(
- UnsetFieldsMetadataTextFormatTestUtil::GetRawAddresses(no_op_fields)[0],
- UnsetFieldsMetadataTextFormatTestUtil::GetAddress(proto, *reflection,
- *field));
- }
- {
- // The address returned by the field is a string_view, which is a separate
- // alocation. Check address directly.
- no_op_fields = {};
- const absl::string_view parse_string = "optional_bytes: \"\"";
- EXPECT_TRUE(parser.ParseFromString(parse_string, &proto));
- ASSERT_EQ(UnsetFieldsMetadataTextFormatTestUtil::NumFields(no_op_fields),
- 1);
- const Reflection* reflection = proto.GetReflection();
- const FieldDescriptor* field = proto.GetDescriptor()->FindFieldByNumber(15);
- EXPECT_EQ(
- UnsetFieldsMetadataTextFormatTestUtil::GetRawAddresses(no_op_fields)[0],
- UnsetFieldsMetadataTextFormatTestUtil::GetAddress(proto, *reflection,
- *field));
}
}
diff --git a/src/google/protobuf/util/message_differencer.cc b/src/google/protobuf/util/message_differencer.cc
index df0311c..853d5da 100644
--- a/src/google/protobuf/util/message_differencer.cc
+++ b/src/google/protobuf/util/message_differencer.cc
@@ -696,7 +696,7 @@
// we are merely checking for a difference in field values,
// rather than the addition or deletion of fields).
std::vector<const FieldDescriptor*> fields_union =
- CombineFields(message1, message1_fields, FULL, message2_fields, FULL);
+ CombineFields(message1_fields, FULL, message2_fields, FULL);
return CompareWithFieldsInternal(message1, message2, unpacked_any,
fields_union, fields_union,
parent_fields);
@@ -719,8 +719,8 @@
// but only the intersection for message2. This way, any fields
// only present in message2 will be ignored, but any fields only
// present in message1 will be marked as a difference.
- std::vector<const FieldDescriptor*> fields_intersection = CombineFields(
- message1, message1_fields, PARTIAL, message2_fields, PARTIAL);
+ std::vector<const FieldDescriptor*> fields_intersection =
+ CombineFields(message1_fields, PARTIAL, message2_fields, PARTIAL);
return CompareWithFieldsInternal(message1, message2, unpacked_any,
message1_fields, fields_intersection,
parent_fields);
@@ -728,48 +728,9 @@
}
}
-namespace {
-bool ValidMissingField(const FieldDescriptor& f) {
- switch (f.cpp_type()) {
- case FieldDescriptor::CPPTYPE_INT32:
- case FieldDescriptor::CPPTYPE_UINT32:
- case FieldDescriptor::CPPTYPE_INT64:
- case FieldDescriptor::CPPTYPE_UINT64:
- case FieldDescriptor::CPPTYPE_FLOAT:
- case FieldDescriptor::CPPTYPE_DOUBLE:
- case FieldDescriptor::CPPTYPE_STRING:
- case FieldDescriptor::CPPTYPE_BOOL:
- case FieldDescriptor::CPPTYPE_ENUM:
- return true;
- default:
- return false;
- }
-}
-} // namespace
-
-bool MessageDifferencer::ShouldCompareNoPresence(
- const Message& message1, const Reflection& reflection1,
- const FieldDescriptor* field2) const {
- const bool compare_no_presence_by_field = force_compare_no_presence_ &&
- !field2->has_presence() &&
- !field2->is_repeated();
- if (compare_no_presence_by_field) {
- return true;
- }
- const bool compare_no_presence_by_address =
- !field2->is_repeated() && !field2->has_presence() &&
- ValidMissingField(*field2) &&
- require_no_presence_fields_.addresses_.contains(
- TextFormat::Parser::UnsetFieldsMetadata::GetUnsetFieldAddress(
- message1, reflection1, *field2));
- return compare_no_presence_by_address;
-}
-
std::vector<const FieldDescriptor*> MessageDifferencer::CombineFields(
- const Message& message1, const std::vector<const FieldDescriptor*>& fields1,
- Scope fields1_scope, const std::vector<const FieldDescriptor*>& fields2,
- Scope fields2_scope) {
- const Reflection* reflection1 = message1.GetReflection();
+ const std::vector<const FieldDescriptor*>& fields1, Scope fields1_scope,
+ const std::vector<const FieldDescriptor*>& fields2, Scope fields2_scope) {
size_t index1 = 0;
size_t index2 = 0;
@@ -787,8 +748,8 @@
} else if (FieldBefore(field2, field1)) {
if (fields2_scope == FULL) {
tmp_message_fields_.push_back(field2);
- } else if (fields2_scope == PARTIAL &&
- ShouldCompareNoPresence(message1, *reflection1, field2)) {
+ } else if (fields2_scope == PARTIAL && force_compare_no_presence_ &&
+ !field2->has_presence() && !field2->is_repeated()) {
// In order to make MessageDifferencer play nicely with no-presence
// fields in unit tests, we want to check if the expected proto
// (message1) has some fields which are set to their default value but
diff --git a/src/google/protobuf/util/message_differencer.h b/src/google/protobuf/util/message_differencer.h
index 6d8e9a9..5c058f9 100644
--- a/src/google/protobuf/util/message_differencer.h
+++ b/src/google/protobuf/util/message_differencer.h
@@ -32,7 +32,6 @@
#include "absl/log/absl_check.h"
#include "google/protobuf/descriptor.h" // FieldDescriptor
#include "google/protobuf/message.h" // Message
-#include "google/protobuf/text_format.h"
#include "google/protobuf/unknown_field_set.h"
#include "google/protobuf/util/field_comparator.h"
@@ -582,13 +581,6 @@
// in the comparison.
void set_force_compare_no_presence(bool value);
- // If set, the fields in message1 that equal the fields passed here will be
- // treated as required for comparison, even if they are absent.
- void set_require_no_presence_fields(
- const google::protobuf::TextFormat::Parser::UnsetFieldsMetadata& fields) {
- require_no_presence_fields_ = fields;
- }
-
// DEPRECATED. Pass a DefaultFieldComparator instance instead.
// Sets the type of comparison (as defined in the FloatComparison enumeration
// above) that is used by this differencer when comparing float (and double)
@@ -776,7 +768,6 @@
// list. Fields only present in one of the lists will only appear in the
// combined list if the corresponding fields_scope option is set to FULL.
std::vector<const FieldDescriptor*> CombineFields(
- const Message& message1,
const std::vector<const FieldDescriptor*>& fields1, Scope fields1_scope,
const std::vector<const FieldDescriptor*>& fields2, Scope fields2_scope);
@@ -928,17 +919,11 @@
const FieldDescriptor* field,
const RepeatedFieldComparison& new_comparison);
- // Whether we should still compare the field despite its absence in message1.
- bool ShouldCompareNoPresence(const Message& message1,
- const Reflection& reflection1,
- const FieldDescriptor* field2) const;
-
Reporter* reporter_;
DefaultFieldComparator default_field_comparator_;
MessageFieldComparison message_field_comparison_;
Scope scope_;
absl::flat_hash_set<const FieldDescriptor*> force_compare_no_presence_fields_;
- google::protobuf::TextFormat::Parser::UnsetFieldsMetadata require_no_presence_fields_;
absl::flat_hash_set<std::string> force_compare_failure_triggering_fields_;
RepeatedFieldComparison repeated_field_comparison_;
diff --git a/src/google/protobuf/util/message_differencer_unittest.cc b/src/google/protobuf/util/message_differencer_unittest.cc
index 191281a..e673d02 100644
--- a/src/google/protobuf/util/message_differencer_unittest.cc
+++ b/src/google/protobuf/util/message_differencer_unittest.cc
@@ -29,10 +29,8 @@
#include "absl/strings/str_split.h"
#include "absl/strings/string_view.h"
#include "google/protobuf/any_test.pb.h"
-#include "google/protobuf/descriptor.h"
#include "google/protobuf/map_test_util.h"
#include "google/protobuf/map_unittest.pb.h"
-#include "google/protobuf/message.h"
#include "google/protobuf/test_util.h"
#include "google/protobuf/text_format.h"
#include "google/protobuf/unittest.pb.h"
@@ -46,31 +44,12 @@
namespace google {
namespace protobuf {
-namespace internal {
-class UnsetFieldsMetadataMessageDifferencerTestUtil {
- public:
- static void AddExplicitUnsetField(
- const Message& message, const Reflection& reflection,
- const FieldDescriptor& fd,
- TextFormat::Parser::UnsetFieldsMetadata* metadata) {
- metadata->addresses_.insert(
- TextFormat::Parser::UnsetFieldsMetadata::GetUnsetFieldAddress(
- message, reflection, fd));
- }
-};
-} // namespace internal
-
namespace {
-using ::google::protobuf::internal::UnsetFieldsMetadataMessageDifferencerTestUtil;
-
proto3_unittest::TestNoPresenceField MakeTestNoPresenceField() {
proto3_unittest::TestNoPresenceField msg1, msg2;
msg1.set_no_presence_bool(true);
- msg1.set_no_presence_bool2(true);
- msg1.set_no_presence_bool3(true);
- msg1.set_no_presence_string("yolo");
msg2 = msg1;
*msg1.mutable_no_presence_nested() = msg2;
*msg1.add_no_presence_repeated_nested() = msg2;
@@ -400,212 +379,6 @@
}
TEST(MessageDifferencerTest,
- PartialEqualityTestBooleanPresenceFieldMissingWithAddress) {
- util::MessageDifferencer address_differencer;
- address_differencer.set_scope(util::MessageDifferencer::PARTIAL);
-
- // This differencer is not setting force_compare_no_presence.
- util::MessageDifferencer default_differencer;
- default_differencer.set_scope(util::MessageDifferencer::PARTIAL);
- default_differencer.set_force_compare_no_presence(false);
-
- // When clearing a singular no presence field, it will be included in the
- // comparison.
- proto3_unittest::TestNoPresenceField msg1 = MakeTestNoPresenceField();
- proto3_unittest::TestNoPresenceField msg2 = MakeTestNoPresenceField();
-
- const Reflection* reflection1 = msg1.GetReflection();
- const FieldDescriptor* fd1 = msg1.GetDescriptor()->FindFieldByNumber(1);
- TextFormat::Parser::UnsetFieldsMetadata metadata;
- UnsetFieldsMetadataMessageDifferencerTestUtil::AddExplicitUnsetField(
- msg1, *reflection1, *fd1, &metadata);
- address_differencer.set_require_no_presence_fields(metadata);
-
- EXPECT_TRUE(address_differencer.Compare(msg1, msg2));
- EXPECT_TRUE(default_differencer.Compare(msg1, msg2));
-
- msg1.clear_no_presence_bool();
-
- EXPECT_FALSE(address_differencer.Compare(msg1, msg2));
- EXPECT_TRUE(default_differencer.Compare(msg1, msg2));
-
- msg2.clear_no_presence_bool();
-
- EXPECT_TRUE(address_differencer.Compare(msg1, msg2));
- EXPECT_TRUE(default_differencer.Compare(msg1, msg2));
-
- msg1.set_no_presence_bool(true);
-
- EXPECT_FALSE(default_differencer.Compare(msg1, msg2));
- EXPECT_FALSE(address_differencer.Compare(msg1, msg2));
-}
-
-TEST(MessageDifferencerTest,
- PartialEqualityTestStringPresenceFieldMissingWithAddress) {
- util::MessageDifferencer address_differencer;
- address_differencer.set_scope(util::MessageDifferencer::PARTIAL);
-
- // This differencer is not setting force_compare_no_presence.
- util::MessageDifferencer default_differencer;
- default_differencer.set_scope(util::MessageDifferencer::PARTIAL);
- default_differencer.set_force_compare_no_presence(false);
-
- // When clearing a singular no presence field, it will be included in the
- // comparison.
- proto3_unittest::TestNoPresenceField msg1 = MakeTestNoPresenceField();
- proto3_unittest::TestNoPresenceField msg2 = MakeTestNoPresenceField();
-
- const Reflection* reflection1 = msg1.GetReflection();
- const FieldDescriptor* fd1 = msg1.GetDescriptor()->FindFieldByNumber(4);
- TextFormat::Parser::UnsetFieldsMetadata metadata;
- UnsetFieldsMetadataMessageDifferencerTestUtil::AddExplicitUnsetField(
- msg1, *reflection1, *fd1, &metadata);
- address_differencer.set_require_no_presence_fields(metadata);
-
- EXPECT_TRUE(address_differencer.Compare(msg1, msg2));
- EXPECT_TRUE(default_differencer.Compare(msg1, msg2));
-
- msg1.clear_no_presence_string();
-
- EXPECT_FALSE(address_differencer.Compare(msg1, msg2));
- EXPECT_TRUE(default_differencer.Compare(msg1, msg2));
-
- msg2.clear_no_presence_string();
-
- EXPECT_TRUE(address_differencer.Compare(msg1, msg2));
- EXPECT_TRUE(default_differencer.Compare(msg1, msg2));
-
- msg1.set_no_presence_string("yolo");
-
- EXPECT_FALSE(default_differencer.Compare(msg1, msg2));
- EXPECT_FALSE(address_differencer.Compare(msg1, msg2));
-}
-
-// Ensure multiple booleans are addressed distinctly. This is trivially the case
-// now, but tests against possible optimizations in the future to use bitfields.
-TEST(MessageDifferencerTest,
- PartialEqualityTestTwoBoolsPresenceFieldMissingWithAddress) {
- util::MessageDifferencer address_differencer;
- address_differencer.set_scope(util::MessageDifferencer::PARTIAL);
-
- // This differencer is not setting force_compare_no_presence.
- util::MessageDifferencer default_differencer;
- default_differencer.set_scope(util::MessageDifferencer::PARTIAL);
- default_differencer.set_force_compare_no_presence(false);
-
- // When clearing a singular no presence field, it will be included in the
- // comparison.
- proto3_unittest::TestNoPresenceField msg1 = MakeTestNoPresenceField();
- proto3_unittest::TestNoPresenceField msg2 = MakeTestNoPresenceField();
-
- const Reflection* reflection1 = msg1.GetReflection();
- const FieldDescriptor* fd1 = msg1.GetDescriptor()->FindFieldByNumber(5);
- TextFormat::Parser::UnsetFieldsMetadata metadata;
- UnsetFieldsMetadataMessageDifferencerTestUtil::AddExplicitUnsetField(
- msg1, *reflection1, *fd1, &metadata);
- address_differencer.set_require_no_presence_fields(metadata);
-
- EXPECT_TRUE(address_differencer.Compare(msg1, msg2));
- EXPECT_TRUE(default_differencer.Compare(msg1, msg2));
-
- // Trigger on bool2.
- msg1.clear_no_presence_bool2();
-
- EXPECT_FALSE(address_differencer.Compare(msg1, msg2));
- EXPECT_TRUE(default_differencer.Compare(msg1, msg2));
-
- // Triggering on bool2 still ignores bool3.
- msg1.set_no_presence_bool2(true);
- msg1.clear_no_presence_bool3();
-
- EXPECT_TRUE(address_differencer.Compare(msg1, msg2));
- EXPECT_TRUE(default_differencer.Compare(msg1, msg2));
-}
-
-TEST(MessageDifferencerTest,
- PartialEqualityTestBooleanNestedMessagePresenceFieldMissingWithAddress) {
- util::MessageDifferencer address_differencer;
- address_differencer.set_scope(util::MessageDifferencer::PARTIAL);
-
- // This differencer is not setting force_compare_no_presence.
- util::MessageDifferencer default_differencer;
- default_differencer.set_scope(util::MessageDifferencer::PARTIAL);
- default_differencer.set_force_compare_no_presence(false);
-
- // When clearing a singular no presence field, it will be included in the
- // comparison.
- proto3_unittest::TestNoPresenceField msg1 = MakeTestNoPresenceField();
- proto3_unittest::TestNoPresenceField msg2 = MakeTestNoPresenceField();
-
- const Reflection* reflection1 = msg1.no_presence_nested().GetReflection();
- const FieldDescriptor* fd1 = msg1.GetDescriptor()->FindFieldByNumber(1);
- TextFormat::Parser::UnsetFieldsMetadata metadata;
- UnsetFieldsMetadataMessageDifferencerTestUtil::AddExplicitUnsetField(
- msg1.no_presence_nested(), *reflection1, *fd1, &metadata);
- address_differencer.set_require_no_presence_fields(metadata);
-
- EXPECT_TRUE(address_differencer.Compare(msg1, msg2));
- EXPECT_TRUE(default_differencer.Compare(msg1, msg2));
-
- msg1.mutable_no_presence_nested()->clear_no_presence_bool();
-
- EXPECT_FALSE(address_differencer.Compare(msg1, msg2));
- EXPECT_TRUE(default_differencer.Compare(msg1, msg2));
-
- msg2.mutable_no_presence_nested()->clear_no_presence_bool();
-
- EXPECT_TRUE(address_differencer.Compare(msg1, msg2));
- EXPECT_TRUE(default_differencer.Compare(msg1, msg2));
-
- msg1.mutable_no_presence_nested()->set_no_presence_bool(true);
-
- EXPECT_FALSE(default_differencer.Compare(msg1, msg2));
- EXPECT_FALSE(address_differencer.Compare(msg1, msg2));
-}
-
-TEST(MessageDifferencerTest,
- PartialEqualityTestBooleanRepeatedMessagePresenceFieldMissingWithAddress) {
- util::MessageDifferencer address_differencer;
- address_differencer.set_scope(util::MessageDifferencer::PARTIAL);
-
- // This differencer is not setting force_compare_no_presence.
- util::MessageDifferencer default_differencer;
- default_differencer.set_scope(util::MessageDifferencer::PARTIAL);
- default_differencer.set_force_compare_no_presence(false);
-
- // When clearing a singular no presence field, it will be included in the
- // comparison.
- proto3_unittest::TestNoPresenceField msg1 = MakeTestNoPresenceField();
- proto3_unittest::TestNoPresenceField msg2 = MakeTestNoPresenceField();
-
- const Reflection* reflection1 =
- msg1.no_presence_repeated_nested(0).GetReflection();
- const FieldDescriptor* fd1 = msg1.GetDescriptor()->FindFieldByNumber(1);
- TextFormat::Parser::UnsetFieldsMetadata metadata;
- UnsetFieldsMetadataMessageDifferencerTestUtil::AddExplicitUnsetField(
- msg1.no_presence_repeated_nested(0), *reflection1, *fd1, &metadata);
- address_differencer.set_require_no_presence_fields(metadata);
-
- EXPECT_TRUE(address_differencer.Compare(msg1, msg2));
- EXPECT_TRUE(default_differencer.Compare(msg1, msg2));
-
- msg1.mutable_no_presence_repeated_nested(0)->clear_no_presence_bool();
-
- EXPECT_FALSE(address_differencer.Compare(msg1, msg2));
- EXPECT_TRUE(default_differencer.Compare(msg1, msg2));
-
- msg2.mutable_no_presence_repeated_nested(0)->clear_no_presence_bool();
-
- EXPECT_TRUE(address_differencer.Compare(msg1, msg2));
- EXPECT_TRUE(default_differencer.Compare(msg1, msg2));
-
- msg1.mutable_no_presence_repeated_nested(0)->set_no_presence_bool(true);
-
- EXPECT_FALSE(default_differencer.Compare(msg1, msg2));
- EXPECT_FALSE(address_differencer.Compare(msg1, msg2));
-}
-
-TEST(MessageDifferencerTest,
PartialEqualityTestExtraFieldNoPresenceForceCompareReporterAware) {
std::string output;
// Before we can check the output string, we must make sure the
diff --git a/src/google/protobuf/util/message_differencer_unittest_proto3.proto b/src/google/protobuf/util/message_differencer_unittest_proto3.proto
index b139554..55540ce 100644
--- a/src/google/protobuf/util/message_differencer_unittest_proto3.proto
+++ b/src/google/protobuf/util/message_differencer_unittest_proto3.proto
@@ -18,7 +18,4 @@
bool no_presence_bool = 1;
TestNoPresenceField no_presence_nested = 2;
repeated TestNoPresenceField no_presence_repeated_nested = 3;
- string no_presence_string = 4;
- bool no_presence_bool2 = 5;
- bool no_presence_bool3 = 6;
}
