diff --git a/ChangeLog b/ChangeLog index 6d69e4e308..829f79977e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -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`` diff --git a/doc/whatsnew/2.9.rst b/doc/whatsnew/2.9.rst index 18bd4507ea..a9a3336a86 100644 --- a/doc/whatsnew/2.9.rst +++ b/doc/whatsnew/2.9.rst @@ -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. diff --git a/pylint/checkers/refactoring/refactoring_checker.py b/pylint/checkers/refactoring/refactoring_checker.py index 73bd68296e..0ce6aa360b 100644 --- a/pylint/checkers/refactoring/refactoring_checker.py +++ b/pylint/checkers/refactoring/refactoring_checker.py @@ -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) @@ -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 @@ -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): diff --git a/tests/functional/c/consider/consider_using_with.py b/tests/functional/c/consider/consider_using_with.py index 84b394ce4c..7ba58fdb0f 100644 --- a/tests/functional/c/consider/consider_using_with.py +++ b/tests/functional/c/consider/consider_using_with.py @@ -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