Skip to content

Fix crash on properties & inherited methods in implicit-booleaness #5652

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.

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 4 commits into from
Jan 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -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__``.

Expand Down
5 changes: 5 additions & 0 deletions doc/whatsnew/2.13.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
24 changes: 14 additions & 10 deletions pylint/checkers/refactoring/implicit_booleaness_checker.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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")
)
Expand Down Expand Up @@ -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")
)
Expand Down Expand Up @@ -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 []
44 changes: 0 additions & 44 deletions tests/checkers/unittest_refactoring.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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):
Expand Down
43 changes: 43 additions & 0 deletions tests/functional/u/use/use_implicit_booleaness_not_comparison.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 == {}
Original file line number Diff line number Diff line change
Expand Up @@ -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