From 5c2476ff84bb0308e1169f7aa84029a3c4979021 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Sat, 8 Jan 2022 19:45:35 +0100 Subject: [PATCH 1/4] Fix crash on properties & inherited methods in ``implicit-booleaness`` --- ChangeLog | 5 +++ doc/whatsnew/2.13.rst | 5 +++ .../implicit_booleaness_checker.py | 28 +++++++----- .../use_implicit_booleaness_not_comparison.py | 43 +++++++++++++++++++ ...use_implicit_booleaness_not_comparison.txt | 2 + 5 files changed, 73 insertions(+), 10 deletions(-) diff --git a/ChangeLog b/ChangeLog index 2bf37a089a..41d21030a3 100644 --- a/ChangeLog +++ b/ChangeLog @@ -37,6 +37,11 @@ Release date: TBA Closes #5483 +* Fixed crash on properties and inherited class methods when comparing them for + equality against an empty dict. + + Closes #5646 + * Fixed a false positive for ``assigning-non-slot`` when the slotted class defined ``__setattr__``. diff --git a/doc/whatsnew/2.13.rst b/doc/whatsnew/2.13.rst index c1b5922378..5c8abc00e1 100644 --- a/doc/whatsnew/2.13.rst +++ b/doc/whatsnew/2.13.rst @@ -40,6 +40,11 @@ Other Changes * When run in parallel mode ``pylint`` now pickles the data passed to subprocesses with the ``dill`` package. The ``dill`` package has therefore been added as a dependency. +* Fixed crash on properties and inherited class methods when comparing them for + equality against an empty dict. + + Closes #5646 + * By default, pylint does no longer take files starting with ``.#`` into account. Those are considered `emacs file locks`_. This behavior can be reverted by redefining the ``ignore-patterns`` option. diff --git a/pylint/checkers/refactoring/implicit_booleaness_checker.py b/pylint/checkers/refactoring/implicit_booleaness_checker.py index 6167e3e88f..a8bee57c3a 100644 --- a/pylint/checkers/refactoring/implicit_booleaness_checker.py +++ b/pylint/checkers/refactoring/implicit_booleaness_checker.py @@ -1,9 +1,9 @@ # Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html # For details: https://github.com/PyCQA/pylint/blob/main/LICENSE -from typing import List +from typing import List, Union import astroid -from astroid import nodes +from astroid import bases, nodes from pylint import checkers, interfaces from pylint.checkers import utils @@ -107,7 +107,7 @@ def visit_call(self, node: nodes.Call) -> None: except astroid.InferenceError: # Probably undefined-variable, abort check return - mother_classes = self.base_classes_of_node(instance) + mother_classes = self.base_classes_and_return_names(instance) affected_by_pep8 = any( t in mother_classes for t in ("str", "tuple", "list", "set") ) @@ -165,7 +165,7 @@ def _check_use_implicit_booleaness_not_comparison( target_instance = utils.safe_infer(target_node) if target_instance is None: continue - mother_classes = self.base_classes_of_node(target_instance) + mother_classes = self.base_classes_and_return_names(target_instance) is_base_comprehension_type = any( t in mother_classes for t in ("tuple", "list", "dict", "set") ) @@ -209,9 +209,17 @@ def _check_use_implicit_booleaness_not_comparison( ) @staticmethod - def base_classes_of_node(instance: nodes.ClassDef) -> List[str]: - """Return all the classes names that a ClassDef inherit from including 'object'.""" - try: - return [instance.name] + [x.name for x in instance.ancestors()] - except TypeError: - return [instance.name] + def base_classes_and_return_names( + node: Union[bases.UnboundMethod, bases.Instance] + ) -> List[str]: + """Return all names inherited by a class instance or those returned by a function. + + The inherited names include 'object'. + """ + if isinstance(node, bases.UnboundMethod): + return [i.name for i in node.infer_call_result(node)] + if isinstance(node, bases.Instance): + return [node.name] + [x.name for x in node.ancestors()] + + # For example for astroid.nodes.Uninferable + return [] diff --git a/tests/functional/u/use/use_implicit_booleaness_not_comparison.py b/tests/functional/u/use/use_implicit_booleaness_not_comparison.py index ce64fd1ed3..681ee0607a 100644 --- a/tests/functional/u/use/use_implicit_booleaness_not_comparison.py +++ b/tests/functional/u/use/use_implicit_booleaness_not_comparison.py @@ -194,3 +194,46 @@ def test_function(): long_test = {} if long_test == { }: # [use-implicit-booleaness-not-comparison] pass + + +# Check for properties and uninferable class methods +# See https://github.com/PyCQA/pylint/issues/5646 +from xyz import AnotherClassWithProperty + + +class ParentWithProperty: + + @classmethod + @property + def parent_function(cls): + return {} + + +class MyClassWithProxy(ParentWithProperty): + + attribute = True + + @property + @classmethod + def my_property(cls): + return {} + + @property + @classmethod + def my_difficult_property(cls): + if cls.attribute: + return {} + return MyClassWithProxy() + + + +def test_func(): + """Some assertions against empty dicts.""" + my_class = MyClassWithProxy() + assert my_class.parent_function == {} # [use-implicit-booleaness-not-comparison] + assert my_class.my_property == {} # [use-implicit-booleaness-not-comparison] + + # If the return value is not always implicit boolean, don't raise + assert my_class.my_difficult_property == {} + # Uninferable does not raise + assert AnotherClassWithProperty().my_property == {} diff --git a/tests/functional/u/use/use_implicit_booleaness_not_comparison.txt b/tests/functional/u/use/use_implicit_booleaness_not_comparison.txt index ecd9189d65..d316d5acd3 100644 --- a/tests/functional/u/use/use_implicit_booleaness_not_comparison.txt +++ b/tests/functional/u/use/use_implicit_booleaness_not_comparison.txt @@ -28,3 +28,5 @@ use-implicit-booleaness-not-comparison:160:3:160:20::'numpy_array >= ()' can be use-implicit-booleaness-not-comparison:185:3:185:13::'data == {}' can be simplified to 'not data' as an empty sequence is falsey:UNDEFINED use-implicit-booleaness-not-comparison:187:3:187:13::'data != {}' can be simplified to 'data' as an empty sequence is falsey:UNDEFINED use-implicit-booleaness-not-comparison:195:3:195:26::'long_test == {}' can be simplified to 'not long_test' as an empty sequence is falsey:UNDEFINED +use-implicit-booleaness-not-comparison:233:11:233:41:test_func:'my_class.parent_function == {}' can be simplified to 'not my_class.parent_function' as an empty sequence is falsey:UNDEFINED +use-implicit-booleaness-not-comparison:234:11:234:37:test_func:'my_class.my_property == {}' can be simplified to 'not my_class.my_property' as an empty sequence is falsey:UNDEFINED From 4088c24b07780dec1f5a8b712693e4ec3eb5b708 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Sat, 8 Jan 2022 19:55:14 +0100 Subject: [PATCH 2/4] Change names in test --- tests/checkers/unittest_refactoring.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/checkers/unittest_refactoring.py b/tests/checkers/unittest_refactoring.py index accc4b0670..d50d929126 100644 --- a/tests/checkers/unittest_refactoring.py +++ b/tests/checkers/unittest_refactoring.py @@ -46,23 +46,27 @@ class ChildClassWithoutBool(ClassWithoutBool): """ ) with_bool, without_bool, child_with_bool, child_without_bool = module.body - assert ImplicitBooleanessChecker().base_classes_of_node(with_bool) == [ + assert ImplicitBooleanessChecker().base_classes_and_return_names(with_bool) == [ "ClassWithBool", "list", "object", ] - assert ImplicitBooleanessChecker().base_classes_of_node(without_bool) == [ + assert ImplicitBooleanessChecker().base_classes_and_return_names(without_bool) == [ "ClassWithoutBool", "dict", "object", ] - assert ImplicitBooleanessChecker().base_classes_of_node(child_with_bool) == [ + assert ImplicitBooleanessChecker().base_classes_and_return_names( + child_with_bool + ) == [ "ChildClassWithBool", "ClassWithBool", "list", "object", ] - assert ImplicitBooleanessChecker().base_classes_of_node(child_without_bool) == [ + assert ImplicitBooleanessChecker().base_classes_and_return_names( + child_without_bool + ) == [ "ChildClassWithoutBool", "ClassWithoutBool", "dict", From e6d6c5ea0eee217c7fd17162e1bbaa6426cdecbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Thu, 13 Jan 2022 17:57:52 +0100 Subject: [PATCH 3/4] Rename and remove test --- .../implicit_booleaness_checker.py | 6 +-- tests/checkers/unittest_refactoring.py | 48 ------------------- 2 files changed, 3 insertions(+), 51 deletions(-) diff --git a/pylint/checkers/refactoring/implicit_booleaness_checker.py b/pylint/checkers/refactoring/implicit_booleaness_checker.py index a8bee57c3a..06b52e47af 100644 --- a/pylint/checkers/refactoring/implicit_booleaness_checker.py +++ b/pylint/checkers/refactoring/implicit_booleaness_checker.py @@ -107,7 +107,7 @@ def visit_call(self, node: nodes.Call) -> None: except astroid.InferenceError: # Probably undefined-variable, abort check return - mother_classes = self.base_classes_and_return_names(instance) + mother_classes = self.base_names_of_base_node(instance) affected_by_pep8 = any( t in mother_classes for t in ("str", "tuple", "list", "set") ) @@ -165,7 +165,7 @@ def _check_use_implicit_booleaness_not_comparison( target_instance = utils.safe_infer(target_node) if target_instance is None: continue - mother_classes = self.base_classes_and_return_names(target_instance) + mother_classes = self.base_names_of_base_node(target_instance) is_base_comprehension_type = any( t in mother_classes for t in ("tuple", "list", "dict", "set") ) @@ -209,7 +209,7 @@ def _check_use_implicit_booleaness_not_comparison( ) @staticmethod - def base_classes_and_return_names( + def base_names_of_base_node( node: Union[bases.UnboundMethod, bases.Instance] ) -> List[str]: """Return all names inherited by a class instance or those returned by a function. diff --git a/tests/checkers/unittest_refactoring.py b/tests/checkers/unittest_refactoring.py index d50d929126..a2694200b7 100644 --- a/tests/checkers/unittest_refactoring.py +++ b/tests/checkers/unittest_refactoring.py @@ -5,10 +5,8 @@ import signal from contextlib import contextmanager -import astroid import pytest -from pylint.checkers.refactoring import ImplicitBooleanessChecker from pylint.lint import Run from pylint.reporters.text import TextReporter @@ -28,52 +26,6 @@ def _handle(_signum, _frame): signal.signal(signal.SIGALRM, signal.SIG_DFL) -def test_class_tree_detection() -> None: - module = astroid.parse( - """ -class ClassWithBool(list): - def __bool__(self): - return True - -class ClassWithoutBool(dict): - pass - -class ChildClassWithBool(ClassWithBool): - pass - -class ChildClassWithoutBool(ClassWithoutBool): - pass -""" - ) - with_bool, without_bool, child_with_bool, child_without_bool = module.body - assert ImplicitBooleanessChecker().base_classes_and_return_names(with_bool) == [ - "ClassWithBool", - "list", - "object", - ] - assert ImplicitBooleanessChecker().base_classes_and_return_names(without_bool) == [ - "ClassWithoutBool", - "dict", - "object", - ] - assert ImplicitBooleanessChecker().base_classes_and_return_names( - child_with_bool - ) == [ - "ChildClassWithBool", - "ClassWithBool", - "list", - "object", - ] - assert ImplicitBooleanessChecker().base_classes_and_return_names( - child_without_bool - ) == [ - "ChildClassWithoutBool", - "ClassWithoutBool", - "dict", - "object", - ] - - @pytest.mark.skipif(not hasattr(signal, "setitimer"), reason="Assumes POSIX signals") def test_process_tokens() -> None: with timeout(8.0): From e4fba86c6516bc163c4c51d529d895e253f12fc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Thu, 13 Jan 2022 18:38:30 +0100 Subject: [PATCH 4/4] Fix coverage --- .../refactoring/implicit_booleaness_checker.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/pylint/checkers/refactoring/implicit_booleaness_checker.py b/pylint/checkers/refactoring/implicit_booleaness_checker.py index 06b52e47af..c4b25eeb7c 100644 --- a/pylint/checkers/refactoring/implicit_booleaness_checker.py +++ b/pylint/checkers/refactoring/implicit_booleaness_checker.py @@ -107,7 +107,7 @@ def visit_call(self, node: nodes.Call) -> None: except astroid.InferenceError: # Probably undefined-variable, abort check return - mother_classes = self.base_names_of_base_node(instance) + mother_classes = self.base_names_of_instance(instance) affected_by_pep8 = any( t in mother_classes for t in ("str", "tuple", "list", "set") ) @@ -165,7 +165,7 @@ def _check_use_implicit_booleaness_not_comparison( target_instance = utils.safe_infer(target_node) if target_instance is None: continue - mother_classes = self.base_names_of_base_node(target_instance) + mother_classes = self.base_names_of_instance(target_instance) is_base_comprehension_type = any( t in mother_classes for t in ("tuple", "list", "dict", "set") ) @@ -209,17 +209,13 @@ def _check_use_implicit_booleaness_not_comparison( ) @staticmethod - def base_names_of_base_node( - node: Union[bases.UnboundMethod, bases.Instance] + def base_names_of_instance( + node: Union[bases.Uninferable, bases.Instance] ) -> List[str]: """Return all names inherited by a class instance or those returned by a function. The inherited names include 'object'. """ - if isinstance(node, bases.UnboundMethod): - return [i.name for i in node.infer_call_result(node)] if isinstance(node, bases.Instance): return [node.name] + [x.name for x in node.ancestors()] - - # For example for astroid.nodes.Uninferable return []