Skip to content

Commit

Permalink
Merge pull request #4709 from yushao2/fix-unused-private-member-4673
Browse files Browse the repository at this point in the history
[unused-private-member] add logic for checking nested functions
  • Loading branch information
yushao2 committed Aug 1, 2021
2 parents 8ceb26d + 1d09bc8 commit 2ad8247
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 3 deletions.
6 changes: 5 additions & 1 deletion ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ Release date: TBA

* pyreverse: Show class has-a relationships inferred from the type-hint

Closes #4744
Closes #4744

* Added ``ignored-parents`` option to the design checker to ignore specific
classes from the ``too-many-ancestors`` check (R0901).
Expand Down Expand Up @@ -56,6 +56,10 @@ Closes #4744

Closes #3692

* Fix false-positive of ``unused-private-member`` when using nested functions in a class

Closes #4673


What's New in Pylint 2.9.6?
===========================
Expand Down
19 changes: 17 additions & 2 deletions pylint/checkers/classes.py
Original file line number Diff line number Diff line change
Expand Up @@ -909,6 +909,13 @@ def _check_unused_private_functions(self, node: astroid.ClassDef) -> None:
function_def = cast(astroid.FunctionDef, function_def)
if not is_attr_private(function_def.name):
continue
parent_scope = function_def.parent.scope()
if isinstance(parent_scope, astroid.FunctionDef):
# Handle nested functions
if function_def.name in (
n.name for n in parent_scope.nodes_of_class(astroid.Name)
):
continue
for attribute in node.nodes_of_class(astroid.Attribute):
attribute = cast(astroid.Attribute, attribute)
if (
Expand All @@ -931,11 +938,19 @@ def _check_unused_private_functions(self, node: astroid.ClassDef) -> None:
):
break
else:
function_repr = f"{function_def.name}({function_def.args.as_string()})"
name_stack = []
curr = parent_scope
# Generate proper names for nested functions
while curr != node:
name_stack.append(curr.name)
curr = curr.parent.scope()

outer_level_names = f"{'.'.join(reversed(name_stack))}"
function_repr = f"{outer_level_names}.{function_def.name}({function_def.args.as_string()})"
self.add_message(
"unused-private-member",
node=function_def,
args=(node.name, function_repr),
args=(node.name, function_repr.lstrip(".")),
)

def _check_unused_private_variables(self, node: astroid.ClassDef) -> None:
Expand Down
41 changes: 41 additions & 0 deletions tests/functional/u/unused/unused_private_member.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,3 +160,44 @@ def __new__(cls, func, *args):
def exec(self):
print(self.__secret_bool)
return self.func(*self.__args)


# Test cases for false-positive reported in #4673
# https://github.com/PyCQA/pylint/issues/4673
class FalsePositive4673:
""" The testing class """

def __init__(self, in_thing):
self.thing = False
self.do_thing(in_thing)

def do_thing(self, in_thing):
""" Checks the false-positive condition, sets a property. """
def __false_positive(in_thing):
print(in_thing)

def __true_positive(in_thing): # [unused-private-member]
print(in_thing)

__false_positive(in_thing)
self.thing = True

def undo_thing(self):
""" Unsets a property. """
self.thing = False

def complicated_example(self, flag):
def __inner_1():
pass

def __inner_2():
pass

def __inner_3(fn):
return fn

def __inner_4(): # [unused-private-member]
pass

fn_to_return = __inner_1 if flag else __inner_3(__inner_2)
return fn_to_return
2 changes: 2 additions & 0 deletions tests/functional/u/unused/unused_private_member.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,5 @@ unused-private-member:34:4:HasUnusedInClass.__test:Unused private member `HasUnu
unused-private-member:55:4:HasUnusedInClass.__test_recursive:Unused private member `HasUnusedInClass.__test_recursive(self)`:HIGH
unused-private-member:132:8:FalsePositive4657.__init__:Unused private member `FalsePositive4657.__attr_c`:HIGH
undefined-variable:137:15:FalsePositive4657.attr_c:Undefined variable 'cls':HIGH
unused-private-member:179:8:FalsePositive4673.do_thing.__true_positive:Unused private member `FalsePositive4673.do_thing.__true_positive(in_thing)`:HIGH
unused-private-member:199:8:FalsePositive4673.complicated_example.__inner_4:Unused private member `FalsePositive4673.complicated_example.__inner_4()`:HIGH

0 comments on commit 2ad8247

Please sign in to comment.