From ce812416c52078d60e2db9a9b269c705b894eb04 Mon Sep 17 00:00:00 2001 From: DudeNr33 <3929834+DudeNr33@users.noreply.github.com> Date: Sat, 3 Jul 2021 18:59:15 +0200 Subject: [PATCH 1/7] Fix false-positive ``consider-using-with`` when using ``contextlib.ExitStack`` --- ChangeLog | 4 ++++ .../refactoring/refactoring_checker.py | 21 +++++++++++++++++-- .../c/consider/consider_using_with.py | 8 +++++++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index f9a000d245..437216e82c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -17,6 +17,10 @@ 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 + What's New in Pylint 2.9.3? =========================== diff --git a/pylint/checkers/refactoring/refactoring_checker.py b/pylint/checkers/refactoring/refactoring_checker.py index 73bd68296e..19688b777b 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,21 @@ 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",) + ) + parent = node.parent + if not isinstance(parent, astroid.Call): + return False + func = utils.safe_infer(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 +1314,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 From b62a53842a6a6e9a0ac0180f2bc73735fabe25d4 Mon Sep 17 00:00:00 2001 From: DudeNr33 <3929834+DudeNr33@users.noreply.github.com> Date: Sat, 3 Jul 2021 19:04:19 +0200 Subject: [PATCH 2/7] Add ``whatsnew`` entry for #4654 --- doc/whatsnew/2.9.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/doc/whatsnew/2.9.rst b/doc/whatsnew/2.9.rst index 18bd4507ea..34ed581008 100644 --- a/doc/whatsnew/2.9.rst +++ b/doc/whatsnew/2.9.rst @@ -62,6 +62,9 @@ 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. From 1b67a062a40043c4b72d75c4cd32f7ccf59c0845 Mon Sep 17 00:00:00 2001 From: dudenr33 <3929834+DudeNr33@users.noreply.github.com> Date: Sat, 3 Jul 2021 20:53:19 +0200 Subject: [PATCH 3/7] Python 3.6 needs special treatment - ``ExitStack`` did not inherit from ``_BaseExitStack`` until Python 3.7 --- pylint/checkers/refactoring/refactoring_checker.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pylint/checkers/refactoring/refactoring_checker.py b/pylint/checkers/refactoring/refactoring_checker.py index 19688b777b..32da1d7f4f 100644 --- a/pylint/checkers/refactoring/refactoring_checker.py +++ b/pylint/checkers/refactoring/refactoring_checker.py @@ -118,7 +118,10 @@ 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._BaseExitStack.enter_context", + "contextlib.ExitStack.enter_context", # necessary for Python 3.6 compatibility + ) ) parent = node.parent if not isinstance(parent, astroid.Call): From 78764655736523d288262155251201c90af7bc65 Mon Sep 17 00:00:00 2001 From: dudenr33 <3929834+DudeNr33@users.noreply.github.com> Date: Sat, 3 Jul 2021 20:58:21 +0200 Subject: [PATCH 4/7] Fix shall be included in pylint 2.10, not 2.9.4 --- ChangeLog | 8 ++++---- doc/whatsnew/2.10.rst | 2 ++ doc/whatsnew/2.9.rst | 2 -- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/ChangeLog b/ChangeLog index 437216e82c..548f00204a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -9,6 +9,10 @@ Release date: TBA .. Put new features here and also in 'doc/whatsnew/2.10.rst' +* Fix false-positive ``consider-using-with`` (R1732) if ``contextlib.ExitStack`` takes care of calling the ``__exit__`` method + + Closes #4654 + What's New in Pylint 2.9.4? =========================== @@ -17,10 +21,6 @@ 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 - What's New in Pylint 2.9.3? =========================== diff --git a/doc/whatsnew/2.10.rst b/doc/whatsnew/2.10.rst index 6e119650ca..ff41200e5f 100644 --- a/doc/whatsnew/2.10.rst +++ b/doc/whatsnew/2.10.rst @@ -16,3 +16,5 @@ New checkers Other Changes ============= + +* Fix false-positive ``consider-using-with`` (R1732) if ``contextlib.ExitStack`` takes care of calling the ``__exit__`` method diff --git a/doc/whatsnew/2.9.rst b/doc/whatsnew/2.9.rst index 34ed581008..7343bd9b80 100644 --- a/doc/whatsnew/2.9.rst +++ b/doc/whatsnew/2.9.rst @@ -63,8 +63,6 @@ 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. From 5355ad271ff3bde9a11922e5410af7a5d9deaf38 Mon Sep 17 00:00:00 2001 From: DudeNr33 <3929834+DudeNr33@users.noreply.github.com> Date: Sun, 4 Jul 2021 19:38:03 +0200 Subject: [PATCH 5/7] Revert "Fix shall be included in pylint 2.10, not 2.9.4" This reverts commit 78764655736523d288262155251201c90af7bc65. --- ChangeLog | 8 ++++---- doc/whatsnew/2.10.rst | 2 -- doc/whatsnew/2.9.rst | 2 ++ 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/ChangeLog b/ChangeLog index 548f00204a..437216e82c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -9,10 +9,6 @@ Release date: TBA .. Put new features here and also in 'doc/whatsnew/2.10.rst' -* Fix false-positive ``consider-using-with`` (R1732) if ``contextlib.ExitStack`` takes care of calling the ``__exit__`` method - - Closes #4654 - What's New in Pylint 2.9.4? =========================== @@ -21,6 +17,10 @@ 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 + What's New in Pylint 2.9.3? =========================== diff --git a/doc/whatsnew/2.10.rst b/doc/whatsnew/2.10.rst index ff41200e5f..6e119650ca 100644 --- a/doc/whatsnew/2.10.rst +++ b/doc/whatsnew/2.10.rst @@ -16,5 +16,3 @@ New checkers Other Changes ============= - -* Fix false-positive ``consider-using-with`` (R1732) if ``contextlib.ExitStack`` takes care of calling the ``__exit__`` method diff --git a/doc/whatsnew/2.9.rst b/doc/whatsnew/2.9.rst index 7343bd9b80..34ed581008 100644 --- a/doc/whatsnew/2.9.rst +++ b/doc/whatsnew/2.9.rst @@ -63,6 +63,8 @@ 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. From 3442efc47baf17e95495a5f8a7d59f245530de24 Mon Sep 17 00:00:00 2001 From: DudeNr33 <3929834+DudeNr33@users.noreply.github.com> Date: Sun, 4 Jul 2021 19:40:10 +0200 Subject: [PATCH 6/7] Remove empty line --- doc/whatsnew/2.9.rst | 1 - 1 file changed, 1 deletion(-) diff --git a/doc/whatsnew/2.9.rst b/doc/whatsnew/2.9.rst index 34ed581008..a9a3336a86 100644 --- a/doc/whatsnew/2.9.rst +++ b/doc/whatsnew/2.9.rst @@ -62,7 +62,6 @@ 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 From b22ea4c0923faded92ca77f8eca0a4369c83c548 Mon Sep 17 00:00:00 2001 From: DudeNr33 <3929834+DudeNr33@users.noreply.github.com> Date: Sun, 4 Jul 2021 19:44:02 +0200 Subject: [PATCH 7/7] Use node.parent directly instead of saving it to a variable --- pylint/checkers/refactoring/refactoring_checker.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pylint/checkers/refactoring/refactoring_checker.py b/pylint/checkers/refactoring/refactoring_checker.py index 32da1d7f4f..0ce6aa360b 100644 --- a/pylint/checkers/refactoring/refactoring_checker.py +++ b/pylint/checkers/refactoring/refactoring_checker.py @@ -123,10 +123,9 @@ def _will_be_released_automatically(node: astroid.Call) -> bool: "contextlib.ExitStack.enter_context", # necessary for Python 3.6 compatibility ) ) - parent = node.parent - if not isinstance(parent, astroid.Call): + if not isinstance(node.parent, astroid.Call): return False - func = utils.safe_infer(parent.func) + func = utils.safe_infer(node.parent.func) if not func: return False return func.qname() in callables_taking_care_of_exit