Skip to content

Commit

Permalink
Fix false-positive consider-using-with when using ``contextlib.Ex…
Browse files Browse the repository at this point in the history
…itStack`` (#4665)

* Fix false-positive ``consider-using-with`` when using ``contextlib.ExitStack``

* Add ``whatsnew`` entry for #4654

* Python 3.6 needs special treatment - ``ExitStack`` did not inherit from ``_BaseExitStack`` until Python 3.7
  • Loading branch information
DudeNr33 committed Jul 4, 2021
1 parent c026826 commit c2d03c6
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 2 deletions.
5 changes: 5 additions & 0 deletions ChangeLog
Expand Up @@ -17,6 +17,11 @@ Release date: TBA
..
Put bug fixes that should not wait for a new minor version here


* Fix false-positive ``consider-using-with`` (R1732) if ``contextlib.ExitStack`` takes care of calling the ``__exit__`` method

Closes #4654

* Fix a false positive for ``unused-private-member`` when mutating a private attribute
with ``cls``

Expand Down
2 changes: 2 additions & 0 deletions doc/whatsnew/2.9.rst
Expand Up @@ -62,6 +62,8 @@ New checkers
Other Changes
=============

* Fix false-positive ``consider-using-with`` (R1732) if ``contextlib.ExitStack`` takes care of calling the ``__exit__`` method

* Add type annotations to pyreverse dot files

* Pylint's tags are now the standard form ``vX.Y.Z`` and not ``pylint-X.Y.Z`` anymore.
Expand Down
23 changes: 21 additions & 2 deletions pylint/checkers/refactoring/refactoring_checker.py
Expand Up @@ -103,7 +103,7 @@ def get_curline_index_start():
return False


def _is_inside_context_manager(node):
def _is_inside_context_manager(node: astroid.Call) -> bool:
frame = node.frame()
if not isinstance(
frame, (astroid.FunctionDef, astroid.BoundMethod, astroid.UnboundMethod)
Expand All @@ -114,6 +114,23 @@ def _is_inside_context_manager(node):
)


def _will_be_released_automatically(node: astroid.Call) -> bool:
"""Checks if a call that could be used in a ``with`` statement is used in an alternative
construct which would ensure that its __exit__ method is called."""
callables_taking_care_of_exit = frozenset(
(
"contextlib._BaseExitStack.enter_context",
"contextlib.ExitStack.enter_context", # necessary for Python 3.6 compatibility
)
)
if not isinstance(node.parent, astroid.Call):
return False
func = utils.safe_infer(node.parent.func)
if not func:
return False
return func.qname() in callables_taking_care_of_exit


class RefactoringChecker(checkers.BaseTokenChecker):
"""Looks for code which can be refactored
Expand Down Expand Up @@ -1299,7 +1316,9 @@ def _check_consider_using_with(self, node: astroid.Call):
and not isinstance(node.parent, astroid.With)
)
)
if could_be_used_in_with and not _is_inside_context_manager(node):
if could_be_used_in_with and not (
_is_inside_context_manager(node) or _will_be_released_automatically(node)
):
self.add_message("consider-using-with", node=node)

def _check_consider_using_join(self, aug_assign):
Expand Down
8 changes: 8 additions & 0 deletions tests/functional/c/consider/consider_using_with.py
Expand Up @@ -154,3 +154,11 @@ def test_popen():
_ = subprocess.Popen("sh") # [consider-using-with]
with subprocess.Popen("sh"):
pass


def test_suppress_in_exit_stack():
"""Regression test for issue #4654 (false positive)"""
with contextlib.ExitStack() as stack:
_ = stack.enter_context(
open("/sys/firmware/devicetree/base/hwid,location", "r")
) # must not trigger

0 comments on commit c2d03c6

Please sign in to comment.