Skip to content

Commit

Permalink
Raise warinings when extend repeated field with none iterable in OSS.…
Browse files Browse the repository at this point in the history
… OSS will raise errors soon

PiperOrigin-RevId: 542037298
  • Loading branch information
anandolee authored and copybara-github committed Jun 20, 2023
1 parent c95ebb7 commit 96ca7d9
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 25 deletions.
3 changes: 1 addition & 2 deletions protobuf_deps.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,6 @@ def protobuf_deps():
_github_archive(
name = "upb",
repo = "https://github.com/protocolbuffers/upb",
commit = "56a770818cf47f8ac9e2ac1585a8d2b764214479",
sha256 = "",
commit = "0ea9f73be35e35db242ccc65aa9c87487b792324",
patches = ["@com_google_protobuf//build_defs:upb.patch"],
)
6 changes: 3 additions & 3 deletions python/google/protobuf/internal/containers.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,17 +159,17 @@ 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(b/286557203): 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:
# silently ignore falsy inputs :-/.
# TODO(ptucker): Deprecate this behavior. b/18413862
warnings.warn('Value is not iterable. Please remove the wrong '
'usage. This will be changed to raise TypeError soon.')
return
raise

new_values = [self._type_checker.CheckValue(elem) for elem in elem_seq_iter]
if new_values:
self._values.extend(new_values)
Expand Down
27 changes: 8 additions & 19 deletions python/google/protobuf/internal/message_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1005,47 +1005,36 @@ def testExtendShouldNotSwallowExceptions(self, message_module):
with self.assertRaises(NameError) as _:
m.repeated_nested_enum.extend(a for i in range(10)) # pylint: disable=undefined-variable

FALSY_VALUES = [None, False, 0, 0.0, b'', u'', bytearray(), [], {}, set()]
FALSY_VALUES = [None, False, 0, 0.0]
EMPTY_VALUES = [b'', u'', bytearray(), [], {}, set()]

def testExtendInt32WithNothing(self, message_module):
"""Test no-ops extending repeated int32 fields."""
m = message_module.TestAllTypes()
self.assertSequenceEqual([], m.repeated_int32)

# TODO(ptucker): Deprecate this behavior. b/18413862
for falsy_value in MessageTest.FALSY_VALUES:
m.repeated_int32.extend(falsy_value)
for empty_value in MessageTest.EMPTY_VALUES:
m.repeated_int32.extend(empty_value)
self.assertSequenceEqual([], m.repeated_int32)

m.repeated_int32.extend([])
self.assertSequenceEqual([], m.repeated_int32)

def testExtendFloatWithNothing(self, message_module):
"""Test no-ops extending repeated float fields."""
m = message_module.TestAllTypes()
self.assertSequenceEqual([], m.repeated_float)

# TODO(ptucker): Deprecate this behavior. b/18413862
for falsy_value in MessageTest.FALSY_VALUES:
m.repeated_float.extend(falsy_value)
for empty_value in MessageTest.EMPTY_VALUES:
m.repeated_float.extend(empty_value)
self.assertSequenceEqual([], m.repeated_float)

m.repeated_float.extend([])
self.assertSequenceEqual([], m.repeated_float)

def testExtendStringWithNothing(self, message_module):
"""Test no-ops extending repeated string fields."""
m = message_module.TestAllTypes()
self.assertSequenceEqual([], m.repeated_string)

# TODO(ptucker): Deprecate this behavior. b/18413862
for falsy_value in MessageTest.FALSY_VALUES:
m.repeated_string.extend(falsy_value)
for empty_value in MessageTest.EMPTY_VALUES:
m.repeated_string.extend(empty_value)
self.assertSequenceEqual([], m.repeated_string)

m.repeated_string.extend([])
self.assertSequenceEqual([], m.repeated_string)

def testExtendInt32WithPythonList(self, message_module):
"""Test extending repeated int32 fields with python lists."""
m = message_module.TestAllTypes()
Expand Down
8 changes: 7 additions & 1 deletion python/google/protobuf/pyext/repeated_scalar_container.cc
Original file line number Diff line number Diff line change
Expand Up @@ -466,11 +466,17 @@ static int AssSubscript(PyObject* pself, PyObject* slice, PyObject* value) {
PyObject* Extend(RepeatedScalarContainer* self, PyObject* value) {
cmessage::AssureWritable(self->parent);

// TODO(ptucker): Deprecate this behavior. b/18413862
// TODO(b/286557203): 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;
}

Expand Down

0 comments on commit 96ca7d9

Please sign in to comment.