Skip to content

Commit

Permalink
BREAKING CHANGE in v26: Reject extend repeated field with none iterab…
Browse files Browse the repository at this point in the history
…le (Raise TypeError)

For example m.repeated_int32.extend(None) will be rejected

PiperOrigin-RevId: 595840357
  • Loading branch information
anandolee authored and copybara-github committed Jan 5, 2024
1 parent c51f111 commit 1658213
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 25 deletions.
12 changes: 1 addition & 11 deletions python/google/protobuf/internal/containers.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,17 +136,7 @@ def insert(self, key: int, value: _T) -> None:

def extend(self, elem_seq: Iterable[_T]) -> None:
"""Extends by appending the given iterable. Similar to list.extend()."""
# TODO: Change OSS to raise error too
if elem_seq is None:
return
try:
elem_seq_iter = iter(elem_seq)
except TypeError:
if not elem_seq:
warnings.warn('Value is not iterable. Please remove the wrong '
'usage. This will be changed to raise TypeError soon.')
return
raise
elem_seq_iter = iter(elem_seq)
new_values = [self._type_checker.CheckValue(elem) for elem in elem_seq_iter]
if new_values:
self._values.extend(new_values)
Expand Down
18 changes: 18 additions & 0 deletions python/google/protobuf/internal/message_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1012,6 +1012,12 @@ def testExtendInt32WithNothing(self, message_module):
m = message_module.TestAllTypes()
self.assertSequenceEqual([], m.repeated_int32)

for falsy_value in MessageTest.FALSY_VALUES:
with self.assertRaises(TypeError) as context:
m.repeated_int32.extend(falsy_value)
self.assertIn('iterable', str(context.exception))
self.assertSequenceEqual([], m.repeated_int32)

for empty_value in MessageTest.EMPTY_VALUES:
m.repeated_int32.extend(empty_value)
self.assertSequenceEqual([], m.repeated_int32)
Expand All @@ -1021,6 +1027,12 @@ def testExtendFloatWithNothing(self, message_module):
m = message_module.TestAllTypes()
self.assertSequenceEqual([], m.repeated_float)

for falsy_value in MessageTest.FALSY_VALUES:
with self.assertRaises(TypeError) as context:
m.repeated_float.extend(falsy_value)
self.assertIn('iterable', str(context.exception))
self.assertSequenceEqual([], m.repeated_float)

for empty_value in MessageTest.EMPTY_VALUES:
m.repeated_float.extend(empty_value)
self.assertSequenceEqual([], m.repeated_float)
Expand All @@ -1030,6 +1042,12 @@ def testExtendStringWithNothing(self, message_module):
m = message_module.TestAllTypes()
self.assertSequenceEqual([], m.repeated_string)

for falsy_value in MessageTest.FALSY_VALUES:
with self.assertRaises(TypeError) as context:
m.repeated_string.extend(falsy_value)
self.assertIn('iterable', str(context.exception))
self.assertSequenceEqual([], m.repeated_string)

for empty_value in MessageTest.EMPTY_VALUES:
m.repeated_string.extend(empty_value)
self.assertSequenceEqual([], m.repeated_string)
Expand Down
14 changes: 0 additions & 14 deletions python/google/protobuf/pyext/repeated_scalar_container.cc
Original file line number Diff line number Diff line change
Expand Up @@ -443,20 +443,6 @@ static int AssSubscript(PyObject* pself, PyObject* slice, PyObject* value) {
PyObject* Extend(RepeatedScalarContainer* self, PyObject* value) {
cmessage::AssureWritable(self->parent);

// TODO: Remove this in OSS
if (value == Py_None) {
PyErr_Warn(nullptr,
"Value is not iterable. Please remove the wrong usage."
" This will be changed to raise TypeError soon.");
Py_RETURN_NONE;
}
if ((Py_TYPE(value)->tp_as_sequence == nullptr) && PyObject_Not(value)) {
PyErr_Warn(nullptr,
"Value is not iterable. Please remove the wrong usage."
" This will be changed to raise TypeError soon.");
Py_RETURN_NONE;
}

ScopedPyObjectPtr iter(PyObject_GetIter(value));
if (iter == nullptr) {
PyErr_SetString(PyExc_TypeError, "Value must be iterable");
Expand Down

0 comments on commit 1658213

Please sign in to comment.