Skip to content

Refactoring to start using getattr_static #832

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 4, 2020
Merged

Refactoring to start using getattr_static #832

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2ca6b7c
Refactor to start using getattr_static for python3
rybarczykj Jul 27, 2020
d410c8c
Add test coverage for has_attr_safe
rybarczykj Jul 29, 2020
234b8ba
re-add AttrCleaner for one spot and add testing
rybarczykj Aug 3, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 24 additions & 27 deletions bpython/autocomplete.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,9 +340,6 @@ def attr_matches(self, text, namespace):
"""Taken from rlcompleter.py and bent to my will.
"""

# Gna, Py 2.6's rlcompleter searches for __call__ inside the
# instance instead of the type, so we monkeypatch to prevent
# side-effects (__getattr__/__getattribute__)
m = self.attr_matches_re.match(text)
if not m:
return []
Expand All @@ -356,19 +353,17 @@ def attr_matches(self, text, namespace):
obj = safe_eval(expr, namespace)
except EvaluationError:
return []
with inspection.AttrCleaner(obj):
matches = self.attr_lookup(obj, expr, attr)
matches = self.attr_lookup(obj, expr, attr)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment about about monkeypatching to prevent side-effects (__getattr__/__getattribute__) is out of date now that we no longer use AttrCleaner here. Let's remove the comment. This is fine because 1) we don't support 2.6 anymore, and 2) we don't call callable anywhere in here anyway.

Also, if we're going to remove AttrCleaner then attr_lookup() and this method should be combined; the whole point of separating them was so that attr_lookup() could be run in the AttrCleaner context manager.

return matches

def attr_lookup(self, obj, expr, attr):
"""Second half of original attr_matches method factored out so it can
be wrapped in a safe try/finally block in case anything bad happens to
restore the original __getattribute__ method."""
"""Second half of attr_matches."""
words = self.list_attributes(obj)
if hasattr(obj, "__class__"):
if inspection.hasattr_safe(obj, "__class__"):
words.append("__class__")
words = words + rlcompleter.get_class_members(obj.__class__)
if not isinstance(obj.__class__, abc.ABCMeta):
klass = inspection.getattr_safe(obj, "__class__")
words = words + rlcompleter.get_class_members(klass)
if not isinstance(klass, abc.ABCMeta):
try:
words.remove("__abstractmethods__")
except ValueError:
Expand All @@ -388,21 +383,25 @@ def attr_lookup(self, obj, expr, attr):
if py3:

def list_attributes(self, obj):
return dir(obj)
# TODO: re-implement dir using getattr_static to avoid using
# AttrCleaner here?
with inspection.AttrCleaner(obj):
return dir(obj)

else:

def list_attributes(self, obj):
if isinstance(obj, InstanceType):
try:
with inspection.AttrCleaner(obj):
if isinstance(obj, InstanceType):
try:
return dir(obj)
except Exception:
# This is a case where we can not prevent user code from
# running. We return a default list attributes on error
# instead. (#536)
return ["__doc__", "__module__"]
else:
return dir(obj)
except Exception:
# This is a case where we can not prevent user code from
# running. We return a default list attributes on error
# instead. (#536)
return ["__doc__", "__module__"]
else:
return dir(obj)


class DictKeyCompletion(BaseCompletionType):
Expand Down Expand Up @@ -537,10 +536,9 @@ def matches(self, cursor_offset, line, **kwargs):
obj = evaluate_current_expression(cursor_offset, line, locals_)
except EvaluationError:
return set()
with inspection.AttrCleaner(obj):
# strips leading dot
matches = [m[1:] for m in self.attr_lookup(obj, "", attr.word)]

# strips leading dot
matches = [m[1:] for m in self.attr_lookup(obj, "", attr.word)]
return set(m for m in matches if few_enough_underscores(attr.word, m))


Expand Down Expand Up @@ -679,7 +677,6 @@ def get_completer_bpython(cursor_offset, line, **kwargs):

def _callable_postfix(value, word):
"""rlcompleter's _callable_postfix done right."""
with inspection.AttrCleaner(value):
if inspection.is_callable(value):
word += "("
if inspection.is_callable(value):
word += "("
return word
47 changes: 44 additions & 3 deletions bpython/inspection.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
from six.moves import range

from pygments.token import Token
from types import MemberDescriptorType

from ._py3compat import PythonLexer, py3
from .lazyre import LazyReCompile
Expand Down Expand Up @@ -214,7 +215,7 @@ def getpydocspec(f, func):
if s is None:
return None

if not hasattr(f, "__name__") or s.groups()[0] != f.__name__:
if not hasattr_safe(f, "__name__") or s.groups()[0] != f.__name__:
return None

args = list()
Expand Down Expand Up @@ -276,8 +277,7 @@ def getfuncprops(func, f):
argspec = ArgSpec(*argspec)
fprops = FuncProps(func, argspec, is_bound_method)
except (TypeError, KeyError, ValueError):
with AttrCleaner(f):
argspec = getpydocspec(f, func)
argspec = getpydocspec(f, func)
if argspec is None:
return None
if inspect.ismethoddescriptor(f):
Expand Down Expand Up @@ -392,6 +392,47 @@ def get_encoding_file(fname):
return "ascii"


if not py3:

def getattr_safe(obj, name):
"""side effect free getattr"""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While we're in this code, how about we rename get_attr_safe to getattr_safe and has_attr_safe to hasattr_safe; getattr and hasattr are Python builtins, and our semantics are trying to match those but be safer.

if not is_new_style(obj):
return getattr(obj, name)

with AttrCleaner(obj):
to_look_through = (
obj.__mro__
if inspect.isclass(obj)
else (obj,) + type(obj).__mro__
)
for cls in to_look_through:
if hasattr(cls, "__dict__") and name in cls.__dict__:
result = cls.__dict__[name]
if isinstance(result, MemberDescriptorType):
result = getattr(obj, name)
return result
raise AttributeError(name)


else:

def getattr_safe(obj, name):
"""side effect free getattr (calls getattr_static)."""
result = inspect.getattr_static(obj, name)
# Slots are a MemberDescriptorType
if isinstance(result, MemberDescriptorType):
result = getattr(obj, name)
return result


def hasattr_safe(obj, name):
try:
getattr_safe(obj, name)
return True
except AttributeError:
return False


if py3:

def get_source_unicode(obj):
Expand Down
46 changes: 3 additions & 43 deletions bpython/simpleeval.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@

from . import line as line_properties
from ._py3compat import py3
from .inspection import is_new_style, AttrCleaner
from .inspection import getattr_safe

_string_type_nodes = (ast.Str, ast.Bytes) if py3 else (ast.Str,)
_numeric_types = (int, float, complex) + (() if py3 else (long,))
Expand Down Expand Up @@ -163,14 +163,15 @@ def _convert(node):
if isinstance(node, ast.Attribute):
obj = _convert(node.value)
attr = node.attr
return safe_get_attribute(obj, attr)
return getattr_safe(obj, attr)

raise ValueError("malformed string")

return _convert(node_or_string)


def safe_getitem(obj, index):
""" Safely tries to access obj[index] """
if type(obj) in (list, tuple, dict, bytes) + string_types:
try:
return obj[index]
Expand Down Expand Up @@ -251,44 +252,3 @@ def evaluate_current_attribute(cursor_offset, line, namespace=None):
"can't lookup attribute %s on %r" % (attr.word, obj)
)


def safe_get_attribute(obj, attr):
"""Gets attributes without triggering descriptors on new-style classes"""
if is_new_style(obj):
with AttrCleaner(obj):
result = safe_get_attribute_new_style(obj, attr)
if isinstance(result, member_descriptor):
# will either be the same slot descriptor or the value
return getattr(obj, attr)
return result
return getattr(obj, attr)


class _ClassWithSlots(object):
__slots__ = ["a"]


member_descriptor = type(_ClassWithSlots.a)


def safe_get_attribute_new_style(obj, attr):
"""Returns approximately the attribute returned by getattr(obj, attr)

The object returned ought to be callable if getattr(obj, attr) was.
Fake callable objects may be returned instead, in order to avoid executing
arbitrary code in descriptors.

If the object is an instance of a class that uses __slots__, will return
the member_descriptor object instead of the value.
"""
if not is_new_style(obj):
raise ValueError("%r is not a new-style class or object" % obj)
to_look_through = (
obj.__mro__ if inspect.isclass(obj) else (obj,) + type(obj).__mro__
)

for cls in to_look_through:
if hasattr(cls, "__dict__") and attr in cls.__dict__:
return cls.__dict__[attr]

raise AttributeError()
13 changes: 12 additions & 1 deletion bpython/test/test_autocomplete.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,9 @@ def asserts_when_called(self):
class Slots(object):
__slots__ = ["a", "b"]

class OverriddenGetattribute(Foo):
def __getattribute__(self,name):
raise AssertionError("custom get attribute invoked")

class TestAttrCompletion(unittest.TestCase):
@classmethod
Expand Down Expand Up @@ -298,12 +301,20 @@ def test_descriptor_attributes_not_run(self):
com.matches(2, "a.", locals_={"a": Properties()}),
set(["a.b", "a.a", "a.method", "a.asserts_when_called"]),
)

def test_custom_get_attribute_not_invoked(self):
com = autocomplete.AttrCompletion()
self.assertSetEqual(
com.matches(2, "a.", locals_={"a": OverriddenGetattribute()}),
set(["a.b", "a.a", "a.method"]),
)


def test_slots_not_crash(self):
com = autocomplete.AttrCompletion()
self.assertSetEqual(
com.matches(2, "A.", locals_={"A": Slots}),
set(["A.b", "A.a", "A.mro"]),
set(["A.b", "A.a"]),
)


Expand Down
119 changes: 119 additions & 0 deletions bpython/test/test_inspection.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,5 +155,124 @@ def test_get_source_file(self):
self.assertEqual(encoding, "utf-8")


class A(object):
a = "a"


class B(A):
b = "b"


class Property(object):
@property
def prop(self):
raise AssertionError("Property __get__ executed")


class Slots(object):
__slots__ = ["s1", "s2", "s3"]

if not py3:

@property
def s3(self):
raise AssertionError("Property __get__ executed")


class SlotsSubclass(Slots):
@property
def s4(self):
raise AssertionError("Property __get__ executed")


class OverriddenGetattr(object):
def __getattr__(self, attr):
raise AssertionError("custom __getattr__ executed")

a = 1


class OverriddenGetattribute(object):
def __getattribute__(self, attr):
raise AssertionError("custom __getattribute__ executed")

a = 1


class OverriddenMRO(object):
def __mro__(self):
raise AssertionError("custom mro executed")

a = 1

member_descriptor = type(Slots.s1)

class TestSafeGetAttribute(unittest.TestCase):
def test_lookup_on_object(self):
a = A()
a.x = 1
self.assertEqual(inspection.getattr_safe(a, "x"), 1)
self.assertEqual(inspection.getattr_safe(a, "a"), "a")
b = B()
b.y = 2
self.assertEqual(inspection.getattr_safe(b, "y"), 2)
self.assertEqual(inspection.getattr_safe(b, "a"), "a")
self.assertEqual(inspection.getattr_safe(b, "b"), "b")

self.assertEqual(inspection.hasattr_safe(b, "y"), True)
self.assertEqual(inspection.hasattr_safe(b, "b"), True)


def test_avoid_running_properties(self):
p = Property()
self.assertEqual(inspection.getattr_safe(p, "prop"), Property.prop)
self.assertEqual(inspection.hasattr_safe(p, "prop"), True)

def test_lookup_with_slots(self):
s = Slots()
s.s1 = "s1"
self.assertEqual(inspection.getattr_safe(s, "s1"), "s1")
with self.assertRaises(AttributeError):
inspection.getattr_safe(s, "s2")

self.assertEqual(inspection.hasattr_safe(s, "s1"), True)
self.assertEqual(inspection.hasattr_safe(s, "s2"), False)

def test_lookup_on_slots_classes(self):
sga = inspection.getattr_safe
s = SlotsSubclass()
self.assertIsInstance(sga(Slots, "s1"), member_descriptor)
self.assertIsInstance(sga(SlotsSubclass, "s1"), member_descriptor)
self.assertIsInstance(sga(SlotsSubclass, "s4"), property)
self.assertIsInstance(sga(s, "s4"), property)

self.assertEqual(inspection.hasattr_safe(s, "s1"), False)
self.assertEqual(inspection.hasattr_safe(s, "s4"), True)

@unittest.skipIf(py3, "Py 3 doesn't allow slots and prop in same class")
def test_lookup_with_property_and_slots(self):
sga = inspection.getattr_safe
s = SlotsSubclass()
self.assertIsInstance(sga(Slots, "s3"), property)
self.assertEqual(inspection.getattr_safe(s, "s3"), Slots.__dict__["s3"])
self.assertIsInstance(sga(SlotsSubclass, "s3"), property)

def test_lookup_on_overridden_methods(self):
sga = inspection.getattr_safe
self.assertEqual(sga(OverriddenGetattr(), "a"), 1)
self.assertEqual(sga(OverriddenGetattribute(), "a"), 1)
self.assertEqual(sga(OverriddenMRO(), "a"), 1)
with self.assertRaises(AttributeError):
sga(OverriddenGetattr(), "b")
with self.assertRaises(AttributeError):
sga(OverriddenGetattribute(), "b")
with self.assertRaises(AttributeError):
sga(OverriddenMRO(), "b")

self.assertEqual(inspection.hasattr_safe(OverriddenGetattr(), "b"), False)
self.assertEqual(inspection.hasattr_safe(OverriddenGetattribute(), "b"), False)
self.assertEqual(inspection.hasattr_safe(OverriddenMRO(), "b"), False)


if __name__ == "__main__":
unittest.main()
Loading