Skip to content

Commit

Permalink
Fix false positive for unused-argument where nested function uses p…
Browse files Browse the repository at this point in the history
…arent argument as a `nonlocal` (#5906)

Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
  • Loading branch information
jacobtylerwalls and Pierre-Sassoulas committed Mar 26, 2022
1 parent 54d03df commit 903ce58
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 7 deletions.
7 changes: 6 additions & 1 deletion ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ Release date: TBA
Put new features here and also in 'doc/whatsnew/2.14.rst'



..
Insert your changelog randomly, it will reduce merge conflicts
(Ie. not necessarily at the end)
Expand All @@ -30,6 +29,12 @@ Release date: TBA

Closes #2793

* Fixed false positive for ``unused-argument`` when a ``nonlocal`` name is used
in a nested function that is returned without being called by its parent.

Closes #5187


What's New in Pylint 2.13.0?
============================
Release date: 2022-03-24
Expand Down
7 changes: 6 additions & 1 deletion doc/whatsnew/2.13.rst
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,12 @@ Other Changes

Closes #5950

* Fixed false positive for ``unused-argument`` when a ``nonlocal`` name is used
in a nested function that is returned without being called by its parent.

Closes #5187

* Avoid emitting ``raising-bad-type`` when there is inference ambiguity on
the variable being raised.

Closes #2793
Closes #2793
19 changes: 15 additions & 4 deletions pylint/checkers/variables.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
Any,
DefaultDict,
Dict,
Iterable,
Iterator,
List,
NamedTuple,
Optional,
Expand Down Expand Up @@ -343,7 +345,9 @@ def _import_name_is_global(stmt, global_names):
return False


def _flattened_scope_names(iterator):
def _flattened_scope_names(
iterator: Iterator[Union[nodes.Global, nodes.Nonlocal]]
) -> Set[str]:
values = (set(stmt.names) for stmt in iterator)
return set(itertools.chain.from_iterable(values))

Expand Down Expand Up @@ -2265,7 +2269,9 @@ def _loopvar_name(self, node: astroid.Name) -> None:
if not elements:
self.add_message("undefined-loop-variable", args=node.name, node=node)

def _check_is_unused(self, name, node, stmt, global_names, nonlocal_names):
def _check_is_unused(
self, name, node, stmt, global_names, nonlocal_names: Iterable[str]
):
# Ignore some special names specified by user configuration.
if self._is_name_ignored(stmt, name):
return
Expand All @@ -2287,7 +2293,7 @@ def _check_is_unused(self, name, node, stmt, global_names, nonlocal_names):
argnames = node.argnames()
# Care about functions with unknown argument (builtins)
if name in argnames:
self._check_unused_arguments(name, node, stmt, argnames)
self._check_unused_arguments(name, node, stmt, argnames, nonlocal_names)
else:
if stmt.parent and isinstance(
stmt.parent, (nodes.Assign, nodes.AnnAssign, nodes.Tuple)
Expand Down Expand Up @@ -2354,7 +2360,9 @@ def _is_name_ignored(self, stmt, name):
regex = authorized_rgx
return regex and regex.match(name)

def _check_unused_arguments(self, name, node, stmt, argnames):
def _check_unused_arguments(
self, name, node, stmt, argnames, nonlocal_names: Iterable[str]
):
is_method = node.is_method()
klass = node.parent.frame(future=True)
if is_method and isinstance(klass, nodes.ClassDef):
Expand Down Expand Up @@ -2395,6 +2403,9 @@ def _check_unused_arguments(self, name, node, stmt, argnames):
if utils.is_protocol_class(klass):
return

if name in nonlocal_names:
return

self.add_message("unused-argument", args=name, node=stmt, confidence=confidence)

def _check_late_binding_closure(self, node: nodes.Name) -> None:
Expand Down
9 changes: 9 additions & 0 deletions tests/functional/u/unused/unused_argument_py3.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,12 @@ def func(first, *, second): # [unused-argument, unused-argument]
def only_raises(first, second=42): # [unused-argument]
if first == 24:
raise ValueError


def increment_factory(initial):

def increment():
nonlocal initial
initial += 1

return increment
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
unused-wildcard-import:3:0:3:34::Unused import(s) func and only_raises from wildcard import of unused_argument_py3:UNDEFINED
unused-wildcard-import:3:0:3:34::Unused import(s) func, only_raises and increment_factory from wildcard import of unused_argument_py3:UNDEFINED
wildcard-import:3:0:3:34::Wildcard import unused_argument_py3:UNDEFINED
unused-wildcard-import:4:0:4:38::Unused import(s) VAR from wildcard import of unused_global_variable1:UNDEFINED
wildcard-import:4:0:4:38::Wildcard import unused_global_variable1:UNDEFINED
Expand Down

0 comments on commit 903ce58

Please sign in to comment.