Skip to content

Commit

Permalink
@beartype 0.16.3 inheritance regression.
Browse files Browse the repository at this point in the history
This commit resolves a critical regression introduced by @beartype's
prior stable release (i.e., @beartype 0.16.3), resolving issue #294
kindly submitted by Ontarian AI superstar MaximilienLC (Maximilien Le
Cleï). Notably, the `@beartype` decorator silently failed to type-check
subclass methods overriding superclass methods under @beartype 0.16.3.
Specifically, this commit:

* Resolves that critical regression.
* Defines a new regression test preventing this from happening again.
  Oh, please. Hear our plea, pytest: "Let this never happen again."

(*Critical critique beyond yonder boutique!*)
  • Loading branch information
leycec committed Oct 21, 2023
1 parent 84613f2 commit 7525fa3
Show file tree
Hide file tree
Showing 5 changed files with 222 additions and 100 deletions.
122 changes: 31 additions & 91 deletions beartype/_decor/_decortype.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@
BeartypeableT,
TypeStack,
)
from beartype._util.cls.utilclsset import set_type_attr
from beartype._util.module.utilmodget import get_object_module_name_or_none
from beartype._util.py.utilpyversion import IS_PYTHON_AT_LEAST_3_10
from collections import defaultdict
from functools import wraps

Expand Down Expand Up @@ -98,7 +98,7 @@ class variable or method annotated by this hint *or* :data:`None`).
# "object" superclass. Unfortunately, *ALL* of these methods are C-based and
# thus do *NOT* directly support monkey-patching: e.g.,
# >>> class AhMahGoddess(object): pass
# >>> AhMahGoddess.__init__.__beartyped_cls = True
# >>> AhMahGoddess.__init__.__beartyped_cls = AhMahGoddess
# AttributeError: 'wrapper_descriptor' object has no attribute
# '__beartyped_cls'
#
Expand All @@ -115,14 +115,17 @@ class variable or method annotated by this hint *or* :data:`None`).
cls_sizeof_old = cls.__sizeof__

# True only if this decorator has already decorated this class, as indicated
# by the @beartype-specific instance variable "__beartyped_cls"
# monkey-patched into a pure-Python __sizeof__() dunder method wrapper by a
# prior call to this decorator passed this class.
is_cls_beartyped = getattr(cls_sizeof_old, '__beartyped_cls', False)

# If this decorator has already decorated this class, silently reduce to a
# noop by returning this class as is.
if is_cls_beartyped:
# by the @beartype-specific class variable "__beartyped_cls" monkey-patched
# into a pure-Python __sizeof__() dunder method wrapper by a prior call to
# this decorator passed this class.
is_cls_beartyped = getattr(cls_sizeof_old, '__beartyped_cls', None)

# If the value of this variable is that of this class, a prior call to this
# decorator has already decorated this class. In this case, silently reduce
# to a noop by returning this class as is.
#
# See where this variable is set below for further details.
if is_cls_beartyped is cls:
# print(f'Ignoring repeat decoration of {repr(cls)}...')
return cls # type: ignore[return-value]
# Else, this decorator has yet to decorate this class.
Expand Down Expand Up @@ -245,7 +248,7 @@ class variable or method annotated by this hint *or* :data:`None`).
)

# Replace this undecorated attribute with this decorated attribute.
_set_type_attr(cls, attr_name, attr_value_beartyped)
set_type_attr(cls, attr_name, attr_value_beartyped)
# Else, this attribute is *NOT* beartypeable. In this case, silently
# ignore this attribute.

Expand All @@ -257,15 +260,25 @@ def cls_sizeof_new(self) -> int:
return cls_sizeof_old(self) # type: ignore[call-arg]

# Monkey-patch a @beartype-specific instance variable into this wrapper,
# recording that this decorator has now decorated this class. For safety,
# leverage our high-level _set_type_attr() setter rather than attempting to
# set this low-level attribute directly. Although doing so efficiently
# succeeds for standard mutable classes, doing so fails catastrophically
# for edge-case immutable classes (e.g., "enum.Enum" subclasses).
cls_sizeof_new.__beartyped_cls = True # type: ignore[attr-defined]
# recording that this decorator has now decorated this class.
#
# Note that we intentionally set this variable to this class rather than an
# arbitrary value (e.g., "False", "None"). Why? Because subclasses of this
# class will inherit this wrapper. If we simply set this variable to an
# arbitrary value, we would be unable to decide above between the following
# two cases:
# * Whether this wrapper was inherited from its superclass, in which case
# this class has yet to be decorated by @beartype.
# * Whether this wrapper was *NOT* inherited from its superclass, in which
# case this class has already been decorated by @beartype.
cls_sizeof_new.__beartyped_cls = cls # type: ignore[attr-defined]

# Replace the original C-based __sizeof__() dunder method with this wrapper.
_set_type_attr(cls, '__sizeof__', cls_sizeof_new)
# We intentionally call our set_type_attr() setter rather than attempting to
# set this attribute directly. The latter approach efficiently succeeds for
# standard pure-Python mutable classes but catastrophically fails for
# non-standard C-based immutable classes (e.g., "enum.Enum" subclasses).
set_type_attr(cls, '__sizeof__', cls_sizeof_new)

# Return this class as is.
return cls # type: ignore[return-value]
Expand All @@ -278,76 +291,3 @@ def cls_sizeof_new(self) -> int:
the :func:`beartype.beartype` decorator to the set of the unqualified basenames
of all classes in that module decorated by that decorator).
'''

# ....................{ PRIVATE ~ setters }....................
def _set_type_attr(cls: type, attr_name: str, attr_value: object) -> None:
'''
Dynamically set the class attribute with the passed name to the passed value
on the dictionary of the passed class.
Parameters
----------
cls : type
Class to set this attribute on.
attr_name : str
Name of the class attribute to be set.
attr_value : object
Value to set this class attribute to.
Caveats
-------
**This function is unavoidably slow.** Class attributes are *only* settable
by calling the tragically slow :func:`setattr` builtin. Attempting to
directly set an attribute on the class dictionary raises an exception. Why?
Because class dictionaries are actually low-level :class:`mappingproxy`
objects that intentionally override the ``__setattr__()`` dunder method to
unconditionally raise an exception. Why? Because that constraint enables the
:meth:`type.__setattr__` dunder method to enforce critical efficiency
constraints on class attributes -- including that class attribute keys are
*not* only strings but also valid Python identifiers:
.. code-block:: pycon
>>> class OhGodHelpUs(object): pass
>>> OhGodHelpUs.__dict__['even_god_cannot_help'] = 2
TypeError: 'mappingproxy' object does not support item
assignment
See also this `relevant StackOverflow answer by Python luminary
Raymond Hettinger <answer_>`__.
.. _answer:
https://stackoverflow.com/a/32720603/2809027
'''

# Attempt to set the class attribute with this name to this value.
try:
setattr(cls, attr_name, attr_value)
# If doing so raises a builtin "TypeError"...
except TypeError as exception:
# Message raised with this "TypeError".
exception_message = str(exception)

# If this message satisfies a well-known pattern unique to the current
# Python version, then this exception signifies this attribute to be
# inherited from an immutable builtin type (e.g., "str") subclassed by
# this user-defined subclass. In this case, silently skip past this
# uncheckable attribute to the next.
if (
# The active Python interpreter targets Python >= 3.10, match a
# message of the form "cannot set '{attr_name}' attribute of
# immutable type '{cls_name}'".
IS_PYTHON_AT_LEAST_3_10 and (
exception_message.startswith("cannot set '") and
"' attribute of immutable type " in exception_message
# Else, the active Python interpreter targets Python <= 3.9. In this
# case, match a message of the form "can't set attributes of
# built-in/extension type '{cls_name}'".
) or exception_message.startswith(
"can't set attributes of built-in/extension type '")
):
return
# Else, this message does *NOT* satisfy that pattern.

# Preserve this exception by re-raising this exception.
raise
15 changes: 8 additions & 7 deletions beartype/_util/cls/utilclsget.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
# See "LICENSE" for further details.

'''
Project-wide **class getters** (i.e., low-level callables querying for various
Project-wide **class getters** (i.e., low-level callables obtaining various
properties of arbitrary classes).
This private submodule is *not* intended for importation by downstream callers.
Expand Down Expand Up @@ -34,7 +34,7 @@ class is dynamically defined in-memory by a prior call to the :func:`exec`
Class to be inspected.
Returns
----------
-------
Optional[str]
Either:
Expand Down Expand Up @@ -70,6 +70,7 @@ class is dynamically defined in-memory by a prior call to the :func:`exec`
if type_module:
# Return the filename defining this module if any *OR* "None".
return get_module_filename_or_none(type_module)
# Else, *NO* modules defines this type.

# If all else fails, this type was probably declared in-memory rather than
# on-disk. In this case, fallback to merely returning "None".
Expand All @@ -89,14 +90,14 @@ def get_type_locals(
attribute directly declared by that class) for the passed class.
Caveats
----------
-------
**This getter returns an immutable rather than mutable mapping.** Callers
requiring the latter are encouraged to manually coerce the immutable mapping
returned by this getter into a mutable mapping (e.g., by passing the former
to the :class:`dict` constructor as is).
Design
----------
------
This getter currently reduces to a trivial one-liner returning
``cls.__dict__`` and has thus been defined mostly just for orthogonality
with the comparable
Expand Down Expand Up @@ -149,13 +150,13 @@ def get_type_locals(
:exc:`_BeartypeUtilTypeException`.
Returns
----------
-------
LexicalScope
Local scope for this class.
Raises
----------
:exc:`exception_cls`
------
exception_cls
If the next non-ignored frame following the last ignored frame is *not*
the parent callable or module directly declaring the passed callable.
'''
Expand Down
88 changes: 88 additions & 0 deletions beartype/_util/cls/utilclsset.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
#!/usr/bin/env python3
# --------------------( LICENSE )--------------------
# Copyright (c) 2014-2023 Beartype authors.
# See "LICENSE" for further details.

'''
Project-wide **class setters** (i.e., low-level callables modifying various
properties of arbitrary classes).
This private submodule is *not* intended for importation by downstream callers.
'''

# ....................{ IMPORTS }....................
from beartype._util.py.utilpyversion import IS_PYTHON_AT_LEAST_3_10

# ....................{ SETTERS }....................
#FIXME: Unit test us up.
def set_type_attr(cls: type, attr_name: str, attr_value: object) -> None:
'''
Dynamically set the **class variable** (i.e., attribute of the passed class)
with the passed name to the passed value.
Parameters
----------
cls : type
Class to set this attribute on.
attr_name : str
Name of the class attribute to be set.
attr_value : object
Value to set this class attribute to.
Caveats
-------
**This function is unavoidably slow.** Class attributes are *only* settable
by calling the tragically slow :func:`setattr` builtin. Attempting to
directly set an attribute on the class dictionary raises an exception. Why?
Because class dictionaries are actually low-level :class:`mappingproxy`
objects that intentionally override the ``__setattr__()`` dunder method to
unconditionally raise an exception. Why? Because that constraint enables the
:meth:`type.__setattr__` dunder method to enforce critical efficiency
constraints on class attributes -- including that class attribute keys are
*not* only strings but also valid Python identifiers:
.. code-block:: pycon
>>> class OhGodHelpUs(object): pass
>>> OhGodHelpUs.__dict__['even_god_cannot_help'] = 2
TypeError: 'mappingproxy' object does not support item
assignment
See also this `relevant StackOverflow answer by Python luminary
Raymond Hettinger <answer_>`__.
.. _answer:
https://stackoverflow.com/a/32720603/2809027
'''

# Attempt to set the class attribute with this name to this value.
try:
setattr(cls, attr_name, attr_value)
# If doing so raises a builtin "TypeError"...
except TypeError as exception:
# Message raised with this "TypeError".
exception_message = str(exception)

# If this message satisfies a well-known pattern unique to the current
# Python version, then this exception signifies this attribute to be
# inherited from an immutable builtin type (e.g., "str") subclassed by
# this user-defined subclass. In this case, silently skip past this
# uncheckable attribute to the next.
if (
# The active Python interpreter targets Python >= 3.10, match a
# message of the form "cannot set '{attr_name}' attribute of
# immutable type '{cls_name}'".
IS_PYTHON_AT_LEAST_3_10 and (
exception_message.startswith("cannot set '") and
"' attribute of immutable type " in exception_message
# Else, the active Python interpreter targets Python <= 3.9. In this
# case, match a message of the form "can't set attributes of
# built-in/extension type '{cls_name}'".
) or exception_message.startswith(
"can't set attributes of built-in/extension type '")
):
return
# Else, this message does *NOT* satisfy that pattern.

# Preserve this exception by re-raising this exception.
raise
2 changes: 1 addition & 1 deletion beartype/meta.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ def _convert_version_str_to_tuple(version_str: str): # -> _Tuple[int, ...]:
# For further details, see http://semver.org.
#!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!

VERSION = '0.17.0'
VERSION = '0.16.4'
'''
Human-readable package version as a ``.``-delimited string.
'''
Expand Down

0 comments on commit 7525fa3

Please sign in to comment.