Skip to content

Commit 5bd7d02

Browse files
Deprecate Descriptor Label. As an alternative, add helper methods for checking whether a field is required or repeated.
This is for C++, Java, and Python. PiperOrigin-RevId: 740152256
1 parent daa9a9d commit 5bd7d02

File tree

9 files changed

+317
-283
lines changed

9 files changed

+317
-283
lines changed

java/core/src/main/java/com/google/protobuf/Descriptors.java

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1351,7 +1351,12 @@ public boolean isRequired() {
13511351
== DescriptorProtos.FeatureSet.FieldPresence.LEGACY_REQUIRED;
13521352
}
13531353

1354-
/** Is this field declared optional? */
1354+
/**
1355+
* Is this field declared optional? *
1356+
*
1357+
* <p>This method is deprecated. Use !isRequired() && !isRepeated() instead.
1358+
*/
1359+
@Deprecated
13551360
public boolean isOptional() {
13561361
return proto.getLabel() == FieldDescriptorProto.Label.LABEL_OPTIONAL
13571362
&& getFeatures().getFieldPresence()
@@ -1450,7 +1455,8 @@ public OneofDescriptor getRealContainingOneof() {
14501455
boolean hasOptionalKeyword() {
14511456
return isProto3Optional
14521457
|| (file.getEdition() == Edition.EDITION_PROTO2
1453-
&& isOptional()
1458+
&& !isRequired()
1459+
&& !isRepeated()
14541460
&& getContainingOneof() == null);
14551461
}
14561462

@@ -1937,9 +1943,9 @@ void validateFeatures() throws DescriptorValidationException {
19371943
if (containingType != null
19381944
&& containingType.toProto().getOptions().getMessageSetWireFormat()) {
19391945
if (isExtension()) {
1940-
if (!isOptional() || getType() != Type.MESSAGE) {
1946+
if (isRequired() || isRepeated() || getType() != Type.MESSAGE) {
19411947
throw new DescriptorValidationException(
1942-
this, "Extensions of MessageSets must be optional messages.");
1948+
this, "Extensions of MessageSets may not be required or repeated messages.");
19431949
}
19441950
}
19451951
}

python/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,5 +160,6 @@ py_extension(
160160
"//upb/hash",
161161
"//upb/util:def_to_proto",
162162
"//upb/util:required_fields",
163+
"@abseil-cpp//absl/base:core_headers",
163164
],
164165
)

python/descriptor.c

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1056,9 +1056,23 @@ static PyObject* PyUpb_FieldDescriptor_GetCppType(PyUpb_DescriptorBase* self,
10561056

10571057
static PyObject* PyUpb_FieldDescriptor_GetLabel(PyUpb_DescriptorBase* self,
10581058
void* closure) {
1059+
PyErr_WarnEx(
1060+
PyExc_DeprecationWarning,
1061+
"label() is deprecated. Use is_required() or is_repeated() instead.", 2);
1062+
10591063
return PyLong_FromLong(upb_FieldDef_Label(self->def));
10601064
}
10611065

1066+
static PyObject* PyUpb_FieldDescriptor_IsRequired(PyUpb_DescriptorBase* self,
1067+
void* closure) {
1068+
return PyBool_FromLong(upb_FieldDef_IsRequired(self->def));
1069+
}
1070+
1071+
static PyObject* PyUpb_FieldDescriptor_IsRepeated(PyUpb_DescriptorBase* self,
1072+
void* closure) {
1073+
return PyBool_FromLong(upb_FieldDef_IsRepeated(self->def));
1074+
}
1075+
10621076
static PyObject* PyUpb_FieldDescriptor_GetIsExtension(
10631077
PyUpb_DescriptorBase* self, void* closure) {
10641078
return PyBool_FromLong(upb_FieldDef_IsExtension(self->def));
@@ -1165,6 +1179,10 @@ static PyGetSetDef PyUpb_FieldDescriptor_Getters[] = {
11651179
{"type", (getter)PyUpb_FieldDescriptor_GetType, NULL, "Type"},
11661180
{"cpp_type", (getter)PyUpb_FieldDescriptor_GetCppType, NULL, "C++ Type"},
11671181
{"label", (getter)PyUpb_FieldDescriptor_GetLabel, NULL, "Label"},
1182+
{"is_required", (getter)PyUpb_FieldDescriptor_IsRequired, NULL,
1183+
"Is Required"},
1184+
{"is_repeated", (getter)PyUpb_FieldDescriptor_IsRepeated, NULL,
1185+
"Is Repeated"},
11681186
{"number", (getter)PyUpb_FieldDescriptor_GetNumber, NULL, "Number"},
11691187
{"index", (getter)PyUpb_FieldDescriptor_GetIndex, NULL, "Index"},
11701188
{"default_value", (getter)PyUpb_FieldDescriptor_GetDefaultValue, NULL,

python/google/protobuf/descriptor.py

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -721,13 +721,33 @@ def type(self, val):
721721

722722
@property
723723
def label(self):
724+
warnings.warn(
725+
'Call to deprecated label property. Use is_required() or is_repeated()'
726+
' methods instead.',
727+
category=DeprecationWarning,
728+
stacklevel=2,
729+
)
730+
724731
if (
725732
self._GetFeatures().field_presence
726733
== _FEATURESET_FIELD_PRESENCE_LEGACY_REQUIRED
727734
):
728735
return FieldDescriptor.LABEL_REQUIRED
729736
return self._label
730737

738+
@property
739+
def is_required(self):
740+
"""Returns if the field is required."""
741+
return (
742+
self._GetFeatures().field_presence
743+
== _FEATURESET_FIELD_PRESENCE_LEGACY_REQUIRED
744+
)
745+
746+
@property
747+
def is_repeated(self):
748+
"""Returns if the field is repeated."""
749+
return self._label == FieldDescriptor.LABEL_REPEATED
750+
731751
@property
732752
def camelcase_name(self):
733753
"""Camelcase name of this field.
@@ -746,7 +766,7 @@ def has_presence(self):
746766
Raises:
747767
RuntimeError: singular field that is not linked with message nor file.
748768
"""
749-
if self.label == FieldDescriptor.LABEL_REPEATED:
769+
if self.is_repeated:
750770
return False
751771
if (
752772
self.cpp_type == FieldDescriptor.CPPTYPE_MESSAGE
@@ -763,7 +783,7 @@ def has_presence(self):
763783
@property
764784
def is_packed(self):
765785
"""Returns if the field is packed."""
766-
if self.label != FieldDescriptor.LABEL_REPEATED:
786+
if not self.is_repeated:
767787
return False
768788
field_type = self.type
769789
if (field_type == FieldDescriptor.TYPE_STRING or

python/google/protobuf/internal/descriptor_test.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -588,12 +588,16 @@ def testDefault(self):
588588
message_descriptor = unittest_pb2.TestAllTypes.DESCRIPTOR
589589
field = message_descriptor.fields_by_name['repeated_int32']
590590
self.assertEqual(field.default_value, [])
591+
self.assertTrue(field.is_repeated)
591592
field = message_descriptor.fields_by_name['repeated_nested_message']
592593
self.assertEqual(field.default_value, [])
594+
self.assertTrue(field.is_repeated)
593595
field = message_descriptor.fields_by_name['optionalgroup']
594596
self.assertEqual(field.default_value, None)
597+
self.assertFalse(field.is_required)
595598
field = message_descriptor.fields_by_name['optional_nested_message']
596599
self.assertEqual(field.default_value, None)
600+
self.assertFalse(field.is_required)
597601

598602

599603
class NewDescriptorTest(DescriptorTest):
@@ -650,6 +654,7 @@ def CheckFieldDescriptor(self, field_descriptor):
650654
self.assertIn(field_descriptor, {field_descriptor: None})
651655
self.assertEqual(None, field_descriptor.extension_scope)
652656
self.assertEqual(None, field_descriptor.enum_type)
657+
self.assertFalse(field_descriptor.is_required)
653658
self.assertTrue(field_descriptor.has_presence)
654659
if api_implementation.Type() == 'cpp':
655660
# For test coverage only
@@ -1288,10 +1293,7 @@ def testFeaturesStripped(self):
12881293

12891294
def testLegacyRequiredTransform(self):
12901295
desc = unittest_legacy_features_pb2.TestEditionsMessage.DESCRIPTOR
1291-
self.assertEqual(
1292-
desc.fields_by_name['required_field'].label,
1293-
descriptor.FieldDescriptor.LABEL_REQUIRED,
1294-
)
1296+
self.assertTrue(desc.fields_by_name['required_field'].is_required)
12951297

12961298
def testLegacyGroupTransform(self):
12971299
desc = unittest_legacy_features_pb2.TestEditionsMessage.DESCRIPTOR

python/google/protobuf/internal/python_message.py

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,6 @@ def _IsMessageMapField(field):
269269
return value_type.cpp_type == _FieldDescriptor.CPPTYPE_MESSAGE
270270

271271
def _AttachFieldHelpers(cls, field_descriptor):
272-
is_repeated = field_descriptor.label == _FieldDescriptor.LABEL_REPEATED
273272
field_descriptor._default_constructor = _DefaultValueConstructorForField(
274273
field_descriptor
275274
)
@@ -282,7 +281,9 @@ def AddFieldByTag(wiretype, is_packed):
282281
type_checkers.FIELD_TYPE_TO_WIRE_TYPE[field_descriptor.type], False
283282
)
284283

285-
if is_repeated and wire_format.IsTypePackable(field_descriptor.type):
284+
if field_descriptor.is_repeated and wire_format.IsTypePackable(
285+
field_descriptor.type
286+
):
286287
# To support wire compatibility of adding packed = true, add a decoder for
287288
# packed values regardless of the field's options.
288289
AddFieldByTag(wire_format.WIRETYPE_LENGTH_DELIMITED, True)
@@ -291,7 +292,7 @@ def AddFieldByTag(wiretype, is_packed):
291292
def _MaybeAddEncoder(cls, field_descriptor):
292293
if hasattr(field_descriptor, '_encoder'):
293294
return
294-
is_repeated = (field_descriptor.label == _FieldDescriptor.LABEL_REPEATED)
295+
is_repeated = field_descriptor.is_repeated
295296
is_map_entry = _IsMapField(field_descriptor)
296297
is_packed = field_descriptor.is_packed
297298

@@ -316,7 +317,7 @@ def _MaybeAddDecoder(cls, field_descriptor):
316317
if hasattr(field_descriptor, '_decoders'):
317318
return
318319

319-
is_repeated = field_descriptor.label == _FieldDescriptor.LABEL_REPEATED
320+
is_repeated = field_descriptor.is_repeated
320321
is_map_entry = _IsMapField(field_descriptor)
321322
helper_decoders = {}
322323

@@ -387,7 +388,7 @@ def _AddEnumValues(descriptor, cls):
387388

388389

389390
def _GetInitializeDefaultForMap(field):
390-
if field.label != _FieldDescriptor.LABEL_REPEATED:
391+
if not field.is_repeated:
391392
raise ValueError('map_entry set on non-repeated field %s' % (
392393
field.name))
393394
fields_by_name = field.message_type.fields_by_name
@@ -425,7 +426,7 @@ def _DefaultValueConstructorForField(field):
425426
if _IsMapField(field):
426427
return _GetInitializeDefaultForMap(field)
427428

428-
if field.label == _FieldDescriptor.LABEL_REPEATED:
429+
if field.is_repeated:
429430
if field.has_default_value and field.default_value != []:
430431
raise ValueError('Repeated field default value not empty list: %s' % (
431432
field.default_value))
@@ -517,7 +518,7 @@ def init(self, **kwargs):
517518
if field_value is None:
518519
# field=None is the same as no field at all.
519520
continue
520-
if field.label == _FieldDescriptor.LABEL_REPEATED:
521+
if field.is_repeated:
521522
field_copy = field._default_constructor(self)
522523
if field.cpp_type == _FieldDescriptor.CPPTYPE_MESSAGE: # Composite
523524
if _IsMapField(field):
@@ -636,7 +637,7 @@ def _AddPropertiesForField(field, cls):
636637
constant_name = field.name.upper() + '_FIELD_NUMBER'
637638
setattr(cls, constant_name, field.number)
638639

639-
if field.label == _FieldDescriptor.LABEL_REPEATED:
640+
if field.is_repeated:
640641
_AddPropertiesForRepeatedField(field, cls)
641642
elif field.cpp_type == _FieldDescriptor.CPPTYPE_MESSAGE:
642643
_AddPropertiesForNonRepeatedCompositeField(field, cls)
@@ -838,7 +839,7 @@ def _IsPresent(item):
838839
"""Given a (FieldDescriptor, value) tuple from _fields, return true if the
839840
value should be included in the list returned by ListFields()."""
840841

841-
if item[0].label == _FieldDescriptor.LABEL_REPEATED:
842+
if item[0].is_repeated:
842843
return bool(item[1])
843844
elif item[0].cpp_type == _FieldDescriptor.CPPTYPE_MESSAGE:
844845
return item[1]._is_present_in_parent
@@ -862,7 +863,7 @@ def _AddHasFieldMethod(message_descriptor, cls):
862863

863864
hassable_fields = {}
864865
for field in message_descriptor.fields:
865-
if field.label == _FieldDescriptor.LABEL_REPEATED:
866+
if field.is_repeated:
866867
continue
867868
# For proto3, only submessages and fields inside a oneof have presence.
868869
if not field.has_presence:
@@ -950,7 +951,7 @@ def _AddHasExtensionMethod(cls):
950951
"""Helper for _AddMessageMethods()."""
951952
def HasExtension(self, field_descriptor):
952953
extension_dict._VerifyExtensionHandle(self, field_descriptor)
953-
if field_descriptor.label == _FieldDescriptor.LABEL_REPEATED:
954+
if field_descriptor.is_repeated:
954955
raise KeyError('"%s" is repeated.' % field_descriptor.full_name)
955956

956957
if field_descriptor.cpp_type == _FieldDescriptor.CPPTYPE_MESSAGE:
@@ -1284,7 +1285,7 @@ def IsInitialized(self, errors=None):
12841285

12851286
for field, value in list(self._fields.items()): # dict can change size!
12861287
if field.cpp_type == _FieldDescriptor.CPPTYPE_MESSAGE:
1287-
if field.label == _FieldDescriptor.LABEL_REPEATED:
1288+
if field.is_repeated:
12881289
if (field.message_type._is_map_entry):
12891290
continue
12901291
for element in value:
@@ -1332,7 +1333,7 @@ def FindInitializationErrors(self):
13321333
else:
13331334
# ScalarMaps can't have any initialization errors.
13341335
pass
1335-
elif field.label == _FieldDescriptor.LABEL_REPEATED:
1336+
elif field.is_repeated:
13361337
for i in range(len(value)):
13371338
element = value[i]
13381339
prefix = '%s[%d].' % (name, i)
@@ -1357,7 +1358,6 @@ def _FullyQualifiedClassName(klass):
13571358

13581359

13591360
def _AddMergeFromMethod(cls):
1360-
LABEL_REPEATED = _FieldDescriptor.LABEL_REPEATED
13611361
CPPTYPE_MESSAGE = _FieldDescriptor.CPPTYPE_MESSAGE
13621362

13631363
def MergeFrom(self, msg):
@@ -1373,7 +1373,7 @@ def MergeFrom(self, msg):
13731373
fields = self._fields
13741374

13751375
for field, value in msg._fields.items():
1376-
if field.label == LABEL_REPEATED:
1376+
if field.is_repeated:
13771377
field_value = fields.get(field)
13781378
if field_value is None:
13791379
# Construct a new object to represent this field.
@@ -1442,7 +1442,7 @@ def _DiscardUnknownFields(self):
14421442
if _IsMessageMapField(field):
14431443
for key in value:
14441444
value[key].DiscardUnknownFields()
1445-
elif field.label == _FieldDescriptor.LABEL_REPEATED:
1445+
elif field.is_repeated:
14461446
for sub_message in value:
14471447
sub_message.DiscardUnknownFields()
14481448
else:

0 commit comments

Comments
 (0)