From f60f478f45143b74158380528cb757509d627592 Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Wed, 18 Jan 2023 14:01:54 -0800 Subject: [PATCH] Enable new JSON field name conflict handling. This will apply uniformly in both proto2 and proto3, taking into account `json_name` options. See https://github.com/protocolbuffers/protobuf/pull/10750 for more details. PiperOrigin-RevId: 502972769 --- .../com/google/protobuf/test_bad_identifiers.proto | 3 +++ python/google/protobuf/internal/descriptor_test.py | 2 ++ src/google/protobuf/descriptor.cc | 12 ++---------- src/google/protobuf/descriptor_unittest.cc | 4 +++- 4 files changed, 10 insertions(+), 11 deletions(-) diff --git a/java/core/src/test/proto/com/google/protobuf/test_bad_identifiers.proto b/java/core/src/test/proto/com/google/protobuf/test_bad_identifiers.proto index 58665e0486..bf8e71ad3a 100644 --- a/java/core/src/test/proto/com/google/protobuf/test_bad_identifiers.proto +++ b/java/core/src/test/proto/com/google/protobuf/test_bad_identifiers.proto @@ -177,6 +177,9 @@ service TestConflictingMethodNames { } message TestConflictingFieldNames { + // TODO(b/261750190) Remove these tests once this behavior is removed. + option deprecated_legacy_json_field_conflicts = true; + enum TestEnum { UNKNOWN = 0; FOO = 1; diff --git a/python/google/protobuf/internal/descriptor_test.py b/python/google/protobuf/internal/descriptor_test.py index 614a84580d..b070bca87d 100755 --- a/python/google/protobuf/internal/descriptor_test.py +++ b/python/google/protobuf/internal/descriptor_test.py @@ -1186,6 +1186,7 @@ class MakeDescriptorTest(unittest.TestCase): def testCamelcaseName(self): descriptor_proto = descriptor_pb2.DescriptorProto() + descriptor_proto.options.deprecated_legacy_json_field_conflicts = True descriptor_proto.name = 'Bar' names = ['foo_foo', 'FooBar', 'fooBaz', 'fooFoo', 'foobar'] camelcase_names = ['fooFoo', 'fooBar', 'fooBaz', 'fooFoo', 'foobar'] @@ -1200,6 +1201,7 @@ class MakeDescriptorTest(unittest.TestCase): def testJsonName(self): descriptor_proto = descriptor_pb2.DescriptorProto() + descriptor_proto.options.deprecated_legacy_json_field_conflicts = True descriptor_proto.name = 'TestJsonName' names = ['field_name', 'fieldName', 'FieldName', '_field_name', 'FIELD_NAME', 'json_name'] diff --git a/src/google/protobuf/descriptor.cc b/src/google/protobuf/descriptor.cc index f914711b42..9c3c9b9e1a 100644 --- a/src/google/protobuf/descriptor.cc +++ b/src/google/protobuf/descriptor.cc @@ -5617,16 +5617,8 @@ void DescriptorBuilder::CheckFieldJsonNameUniqueness( this_type, field.name(), details.orig_name, existing_type, match.field->name(), name_suffix); - bool involves_default = !details.is_custom || !match.is_custom; - if (syntax == FileDescriptor::SYNTAX_PROTO2 && involves_default) { - // TODO(b/261750676) Upgrade this to an error once downstream proto2 files - // have been fixed. - AddWarning(message_name, field, DescriptorPool::ErrorCollector::NAME, - error_message); - } else { - AddError(message_name, field, DescriptorPool::ErrorCollector::NAME, - error_message); - } + AddError(message_name, field, DescriptorPool::ErrorCollector::NAME, + error_message); } } diff --git a/src/google/protobuf/descriptor_unittest.cc b/src/google/protobuf/descriptor_unittest.cc index c1fa463932..a82d8f812e 100644 --- a/src/google/protobuf/descriptor_unittest.cc +++ b/src/google/protobuf/descriptor_unittest.cc @@ -1277,6 +1277,8 @@ class StylizedFieldNamesTest : public testing::Test { AddExtensionRange(AddMessage(&file, "ExtendableMessage"), 1, 1000); DescriptorProto* message = AddMessage(&file, "TestMessage"); + message->mutable_options()->set_deprecated_legacy_json_field_conflicts( + true); AddField(message, "foo_foo", 1, FieldDescriptorProto::LABEL_OPTIONAL, FieldDescriptorProto::TYPE_INT32); AddField(message, "FooBar", 2, FieldDescriptorProto::LABEL_OPTIONAL, @@ -7012,7 +7014,7 @@ TEST_F(ValidationErrorTest, ValidateJsonNameConflictProto3) { } TEST_F(ValidationErrorTest, ValidateJsonNameConflictProto2) { - BuildFileWithWarnings( + BuildFileWithErrors( "name: 'foo.proto' " "syntax: 'proto2' " "message_type {"