Skip to content

Commit

Permalink
Return False on type error for MapKeysView and MapItemsView __contains__
Browse files Browse the repository at this point in the history
Summary: Mutable container __contains__ operations should return False on  type errors similar to immutable thrift-python.

Reviewed By: ahilger

Differential Revision: D65353969

fbshipit-source-id: e1710760cfd3bf60ab6dfe75626494d09e66fc24
  • Loading branch information
yoney authored and facebook-github-bot committed Nov 2, 2024
1 parent ceb1866 commit 18aaaeb
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 20 deletions.
14 changes: 11 additions & 3 deletions third-party/thrift/src/thrift/lib/python/mutable_containers.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -701,7 +701,11 @@ cdef class MapKeysView:
if key is None:
return False

internal_key = self._key_typeinfo.to_internal_data(key)
try:
internal_key = self._key_typeinfo.to_internal_data(key)
except (TypeError, OverflowError):
return False

return internal_key in self._dict_keys

def __iter__(self):
Expand All @@ -721,8 +725,12 @@ cdef class MapItemsView:
if item is None:
return False

internal_item = (self._key_typeinfo.to_internal_data(item[0]),
self._val_typeinfo.to_internal_data(item[1]))
try:
internal_item = (self._key_typeinfo.to_internal_data(item[0]),
self._val_typeinfo.to_internal_data(item[1]))
except (TypeError, OverflowError):
return False

return internal_item in self._dict_items

def __iter__(self):
Expand Down
22 changes: 5 additions & 17 deletions third-party/thrift/src/thrift/lib/python/test/mutable_map_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -265,11 +265,8 @@ def test_keys(self) -> None:
self.assertNotIn("x", mutable_map.keys())
self.assertNotIn("y", mutable_map.keys())

# `MapKeysView.__contains__()` raises `TypeError` on key type mismatch.
with self.assertRaisesRegex(
TypeError, "Expected type <class 'str'>, got: <class 'int'>"
):
_ = 1 in mutable_map.keys()
# `MapKeysView.__contains__()` returns `False` on key type mismatch.
self.assertNotIn(1, mutable_map.keys())

def test_keys_view(self) -> None:
mutable_map = _create_MutableMap_str_i32({})
Expand Down Expand Up @@ -322,19 +319,10 @@ def test_items(self) -> None:
self.assertNotIn(("A", 66), mutable_map.items())
self.assertNotIn(("B", 65), mutable_map.items())

# `MapItemsView.__contains__()` raises `TypeError` on key or value type
# `MapItemsView.__contains__()` returns `False` on key or value type
# mismatch.
with self.assertRaisesRegex(
TypeError, "Expected type <class 'str'>, got: <class 'int'>"
):
# pyre-ignore[6]: Intentional for test
_ = (1, 97) in mutable_map.items()

with self.assertRaisesRegex(
TypeError, "not a <class 'int'>, is actually of type <class 'str'>"
):
# pyre-ignore[6]: Intentional for test
_ = ("a", "Not an integer") in mutable_map.items()
self.assertNotIn((1, 97), mutable_map.items())
self.assertNotIn(("a", "Not an integer"), mutable_map.items())

def test_items_view(self) -> None:
mutable_map = _create_MutableMap_str_i32({})
Expand Down

0 comments on commit 18aaaeb

Please sign in to comment.