From 54c6d9c455abc6316a27f7ad443a86546af5f706 Mon Sep 17 00:00:00 2001 From: Jack DeVries Date: Tue, 27 Jul 2021 22:57:01 -0400 Subject: [PATCH 1/3] bpo-44752: rlcompleter does not call `@property` methods * rlcompleter was calling these methods to identify whether to add parenthesis to the completion, based on if the attribute is callable. * for property objects, completion with parenthesis are never desirable. * property methods with print statements behaved very strangely, which was especially unfriendly to language newcomers. could suddenly produce output unexpectedly. --- Lib/rlcompleter.py | 10 ++++++++++ Lib/test/test_rlcompleter.py | 2 +- .../Library/2021-07-27-22-11-29.bpo-44752._bvbrZ.rst | 2 ++ 3 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 Misc/NEWS.d/next/Library/2021-07-27-22-11-29.bpo-44752._bvbrZ.rst diff --git a/Lib/rlcompleter.py b/Lib/rlcompleter.py index c06388e8d9c2dd..34b259916e8e72 100644 --- a/Lib/rlcompleter.py +++ b/Lib/rlcompleter.py @@ -176,6 +176,16 @@ def attr_matches(self, text): if (word[:n] == attr and not (noprefix and word[:n+1] == noprefix)): match = "%s.%s" % (expr, word) + if isinstance(getattr(type(thisobject), word, None), + property): + # bpo-44752: thisobject.word is a method decorated by + # `@property`. What follows applies a postfix if + # thisobject.word is callable, but know we know that + # this is not callable (because it is a property). + # Also, getattr(thisobject, word) will evaluate the + # property method, which is not desirable. + matches.append(match) + continue try: val = getattr(thisobject, word) except Exception: diff --git a/Lib/test/test_rlcompleter.py b/Lib/test/test_rlcompleter.py index ee3019d8782d17..a393d5276be62d 100644 --- a/Lib/test/test_rlcompleter.py +++ b/Lib/test/test_rlcompleter.py @@ -91,7 +91,7 @@ def bar(self): f = Foo() completer = rlcompleter.Completer(dict(f=f)) self.assertEqual(completer.complete('f.b', 0), 'f.bar') - self.assertEqual(f.calls, 1) + self.assertLessEqual(f.calls, 1) def test_uncreated_attr(self): # Attributes like properties and slots should be completed even when diff --git a/Misc/NEWS.d/next/Library/2021-07-27-22-11-29.bpo-44752._bvbrZ.rst b/Misc/NEWS.d/next/Library/2021-07-27-22-11-29.bpo-44752._bvbrZ.rst new file mode 100644 index 00000000000000..0d8a2cd6a5e0db --- /dev/null +++ b/Misc/NEWS.d/next/Library/2021-07-27-22-11-29.bpo-44752._bvbrZ.rst @@ -0,0 +1,2 @@ +:mod:`rcompleter` does not call :func:`getattr` on :class:`property` objects +to avoid the side-effect of evaluating the corresponding method. From 15d54a397bfa7aba8becdc4781bab00ae7dc6226 Mon Sep 17 00:00:00 2001 From: Jack DeVries Date: Wed, 28 Jul 2021 18:41:13 -0400 Subject: [PATCH 2/3] separate test cases for getattr_called and property_method_not... --- Lib/test/test_rlcompleter.py | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_rlcompleter.py b/Lib/test/test_rlcompleter.py index a393d5276be62d..d2214c1039b0de 100644 --- a/Lib/test/test_rlcompleter.py +++ b/Lib/test/test_rlcompleter.py @@ -80,18 +80,41 @@ def test_attr_matches(self): ['egg.{}('.format(x) for x in dir(str) if x.startswith('s')]) - def test_excessive_getattr(self): + def test_getattr_called(self): # Ensure getattr() is invoked no more than once per attribute + # note the special case for @properties methods below; that is why + # we use __dir__ and __getattr__ in class Foo. class Foo: calls = 0 + def __getattr__(self, name): + if name == 'bar': + self.calls += 1 + return None + return super().__getattr__(name) + + def __dir__(self): + return list(super().__dir__()) + ['bar'] + + f = Foo() + completer = rlcompleter.Completer(dict(f=f)) + self.assertEqual(completer.complete('f.b', 0), 'f.bar') + self.assertEqual(f.calls, 1) + + def test_property_method_not_called(self): + class Foo: + _bar = 0 + property_called = False + @property def bar(self): - self.calls += 1 - return None + self.property_called = True + return self._bar + f = Foo() completer = rlcompleter.Completer(dict(f=f)) self.assertEqual(completer.complete('f.b', 0), 'f.bar') - self.assertLessEqual(f.calls, 1) + self.assertFalse(f.property_called) + def test_uncreated_attr(self): # Attributes like properties and slots should be completed even when From 4b3996b755752b7103b680017828a3f4feeb8ce0 Mon Sep 17 00:00:00 2001 From: Jack DeVries Date: Wed, 28 Jul 2021 18:49:57 -0400 Subject: [PATCH 3/3] fix: simplify test_excessive_getattr --- Lib/test/test_rlcompleter.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/Lib/test/test_rlcompleter.py b/Lib/test/test_rlcompleter.py index d2214c1039b0de..1f7a6ed3f639e0 100644 --- a/Lib/test/test_rlcompleter.py +++ b/Lib/test/test_rlcompleter.py @@ -80,20 +80,21 @@ def test_attr_matches(self): ['egg.{}('.format(x) for x in dir(str) if x.startswith('s')]) - def test_getattr_called(self): - # Ensure getattr() is invoked no more than once per attribute - # note the special case for @properties methods below; that is why - # we use __dir__ and __getattr__ in class Foo. + def test_excessive_getattr(self): + """Ensure getattr() is invoked no more than once per attribute""" + + # note the special case for @property methods below; that is why + # we use __dir__ and __getattr__ in class Foo to create a "magic" + # class attribute 'bar'. This forces `getattr` to call __getattr__ + # (which is doesn't necessarily do). class Foo: calls = 0 - def __getattr__(self, name): + bar = '' + def __getattribute__(self, name): if name == 'bar': self.calls += 1 return None - return super().__getattr__(name) - - def __dir__(self): - return list(super().__dir__()) + ['bar'] + return super().__getattribute__(name) f = Foo() completer = rlcompleter.Completer(dict(f=f))