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..c4b25eeb7c 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_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_classes_of_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,9 +209,13 @@ 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_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.Instance): + return [node.name] + [x.name for x in node.ancestors()] + return [] diff --git a/tests/checkers/unittest_refactoring.py b/tests/checkers/unittest_refactoring.py index accc4b0670..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,48 +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_of_node(with_bool) == [ - "ClassWithBool", - "list", - "object", - ] - assert ImplicitBooleanessChecker().base_classes_of_node(without_bool) == [ - "ClassWithoutBool", - "dict", - "object", - ] - assert ImplicitBooleanessChecker().base_classes_of_node(child_with_bool) == [ - "ChildClassWithBool", - "ClassWithBool", - "list", - "object", - ] - assert ImplicitBooleanessChecker().base_classes_of_node(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): 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