Skip to content

Commit

Permalink
Fixes "exception-escape" false positive with generators (#3128) (#3182)
Browse files Browse the repository at this point in the history
As reported on #3128, there is a bug when the handler bound object is being used inside a generator causing a false positive. This was due to the way we were identifying if the object was "inside" its handler or not. In the previous implementation we would visit the node parents until we reached the object scope (in the generator case, the scope of the node is the generator, not the handler), while this change continues to go up the parent stack until it reaches its parent handler, or, if it's not inside its handler, it reaches the module's parent (None).

Object is considered inside its handler if it reaches it by going up in the parent chain. It's considered outside its handler if it reaches None.

Signed-off-by: Gabriel R Sezefredo <g@briel.dev>
  • Loading branch information
gabrieldrs authored and PCManticore committed Oct 16, 2019
1 parent 2f43a6c commit 89d29d7
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 9 deletions.
4 changes: 4 additions & 0 deletions CONTRIBUTORS.txt
Original file line number Diff line number Diff line change
Expand Up @@ -334,4 +334,8 @@ contributors:

* Rémi Cardona: contributor

* Gabriel R. Sezefredo: contributor
- Fixed "exception-escape" false positive with generators

* laike9m: contributor

5 changes: 5 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,14 @@ Release date: TBA

Close #617

* Fix exception-escape false positive with generators

Close #3128

* ``inspect.getargvalues`` is no longer marked as deprecated.



What's New in Pylint 2.4.3?
===========================

Expand Down
13 changes: 4 additions & 9 deletions pylint/checkers/python3.py
Original file line number Diff line number Diff line change
Expand Up @@ -1265,16 +1265,11 @@ def visit_attribute(self, node):
def visit_excepthandler(self, node):
"""Visit an except handler block and check for exception unpacking."""

def _is_used_in_except_block(node):
scope = node.scope()
def _is_used_in_except_block(node, block):
current = node
while (
current
and current != scope
and not isinstance(current, astroid.ExceptHandler)
):
while current and current is not block:
current = current.parent
return isinstance(current, astroid.ExceptHandler) and current.type != node
return current is not None

if isinstance(node.name, (astroid.Tuple, astroid.List)):
self.add_message("unpacking-in-except", node=node)
Expand All @@ -1292,7 +1287,7 @@ def _is_used_in_except_block(node):
for scope_name in scope_names
if scope_name.name == node.name.name
and scope_name.lineno > node.lineno
and not _is_used_in_except_block(scope_name)
and not _is_used_in_except_block(scope_name, node)
]
reassignments_for_same_name = {
assign_name.lineno
Expand Down
6 changes: 6 additions & 0 deletions tests/unittest_checker_python3.py
Original file line number Diff line number Diff line change
Expand Up @@ -857,6 +857,11 @@ def test_exception_escape(self):
2/0
except (ValueError, TypeError): #@
exc = 2
exc #@
try:
1/0
except (ValueError, TypeError) as exc:
foo(bar for bar in exc.bar)
"""
)
message = testutils.Message("exception-escape", node=module.body[1].value)
Expand All @@ -865,6 +870,7 @@ def test_exception_escape(self):
with self.assertNoMessages():
self.checker.visit_excepthandler(module.body[2].handlers[0])
self.checker.visit_excepthandler(module.body[4].handlers[0])
self.checker.visit_excepthandler(module.body[6].handlers[0])

def test_bad_sys_attribute(self):
node = astroid.extract_node(
Expand Down

0 comments on commit 89d29d7

Please sign in to comment.