Skip to content

Commit

Permalink
warn on enum comparison to bool or float
Browse files Browse the repository at this point in the history
Summary:
Seen some bugs in user code where people inadvertently compare enum to `bool`. This can cause accidental equality with enums that have value `0` and `1`.

Only doing in debug mode. Comparison with `cFollyDebug` cpp constant should mean entire `if` block is removed in `opt`.

Second change, allow "kwarg-style" call to `EnumMeta`. E.g., `Color(value=2)`. This allows thrift-py3 enums to use this metaclass.

Reviewed By: yoney

Differential Revision: D62667187

fbshipit-source-id: a25630a3a21ae472eda9fbd6671dccd02af9a134
  • Loading branch information
ahilger authored and facebook-github-bot committed Sep 17, 2024
1 parent 65b5766 commit 9cea03b
Showing 1 changed file with 15 additions and 1 deletion.
16 changes: 15 additions & 1 deletion third-party/thrift/src/thrift/lib/python/types.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

from collections import defaultdict
from collections.abc import Iterable, Mapping, Sequence, Set as pySet
import warnings
from types import MappingProxyType

from folly.iobuf cimport cIOBuf, IOBuf, from_unique_ptr
Expand All @@ -34,6 +35,7 @@ import functools
import itertools
import threading

from folly cimport cFollyIsDebug
from thrift.python.exceptions cimport GeneratedError
from thrift.python.serializer cimport cserialize, cdeserialize

Expand Down Expand Up @@ -2240,7 +2242,7 @@ class EnumMeta(type):
def __delattr__(cls, name):
raise AttributeError(f"{cls.__name__}: cannot delete Enum member.")

def __call__(cls, value, /):
def __call__(cls, value):
if isinstance(value, cls):
return value
try:
Expand Down Expand Up @@ -2293,11 +2295,23 @@ class Enum(metaclass=EnumMeta):
def __eq__(self, other):
if isinstance(other, Enum):
return self is other
if cFollyIsDebug and isinstance(other, (bool, float)):
warnings.warn(
f"Did you really mean to compare {type(self)} and {type(other)}?",
RuntimeWarning,
stacklevel=1
)
return self._fbthrift_value_ == other

def __ne__(self, other):
if isinstance(other, Enum):
return self is not other
if cFollyIsDebug and isinstance(other, (bool, float)):
warnings.warn(
f"Did you really mean to compare {type(self)} and {type(other)}?",
RuntimeWarning,
stacklevel=1
)
return self._fbthrift_value_ != other

def __hash__(self):
Expand Down

0 comments on commit 9cea03b

Please sign in to comment.