Skip to content

Commit

Permalink
Fix false-positive with contextmanager missing cleanup (#9654) (#9657)
Browse files Browse the repository at this point in the history
  • Loading branch information
github-actions[bot] committed May 20, 2024
1 parent 9a9db8f commit 98c5af9
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@ def cm():
print("cm exit")


def genfunc_with_cm(): # [contextmanager-generator-missing-cleanup]
with cm() as context:
def genfunc_with_cm():
with cm() as context: # [contextmanager-generator-missing-cleanup]
yield context * 2
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,24 @@ because the ways to use a contextmanager are many.
A contextmanager can be used as a decorator (which immediately has ``__enter__``/``__exit__`` applied)
and the use of ``as ...`` or discard of the return value also implies whether the context needs cleanup or not.
So for this message, warning the invoker of the contextmanager is important.

The check can create false positives if ``yield`` is used inside an ``if-else`` block without custom cleanup. Use ``pylint: disable`` for these.

.. code-block:: python
from contextlib import contextmanager
@contextmanager
def good_cm_no_cleanup():
contextvar = "acquired context"
print("cm enter")
if some_condition:
yield contextvar
else:
yield contextvar
def good_cm_no_cleanup_genfunc():
# pylint: disable-next=contextmanager-generator-missing-cleanup
with good_cm_no_cleanup() as context:
yield context * 2
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,15 @@ def good_cm_finally():
def good_cm_finally_genfunc():
with good_cm_finally() as context:
yield context * 2


@contextlib.contextmanager
def good_cm_no_cleanup():
contextvar = "acquired context"
print("cm enter")
yield contextvar


def good_cm_no_cleanup_genfunc():
with good_cm_no_cleanup() as context:
yield context * 2
4 changes: 4 additions & 0 deletions doc/whatsnew/fragments/9625.false_positive
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Exclude context manager without cleanup from
``contextmanager-generator-missing-cleanup`` checks.

Closes #9625
16 changes: 15 additions & 1 deletion pylint/checkers/base/function_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ def _check_contextmanager_generator_missing_cleanup(
if self._node_fails_contextmanager_cleanup(inferred_node, yield_nodes):
self.add_message(
"contextmanager-generator-missing-cleanup",
node=node,
node=with_node,
args=(node.name,),
)

Expand All @@ -85,6 +85,7 @@ def _node_fails_contextmanager_cleanup(
Current checks for a contextmanager:
- only if the context manager yields a non-constant value
- only if the context manager lacks a finally, or does not catch GeneratorExit
- only if some statement follows the yield, some manually cleanup happens
:param node: Node to check
:type node: nodes.FunctionDef
Expand Down Expand Up @@ -114,6 +115,19 @@ def check_handles_generator_exceptions(try_node: nodes.Try) -> bool:
for yield_node in yield_nodes
):
return False

# Check if yield expression is last statement
yield_nodes = list(node.nodes_of_class(nodes.Yield))
if len(yield_nodes) == 1:
n = yield_nodes[0].parent
while n is not node:
if n.next_sibling() is not None:
break
n = n.parent
else:
# No next statement found
return False

# if function body has multiple Try, filter down to the ones that have a yield node
try_with_yield_nodes = [
try_node
Expand Down
28 changes: 20 additions & 8 deletions tests/functional/c/contextmanager_generator_missing_cleanup.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ def cm():
print("cm exit")


def genfunc_with_cm(): # [contextmanager-generator-missing-cleanup]
with cm() as context:
def genfunc_with_cm():
with cm() as context: # [contextmanager-generator-missing-cleanup]
yield context * 2


Expand All @@ -27,13 +27,13 @@ def name_cm():
print("cm exit")


def genfunc_with_name_cm(): # [contextmanager-generator-missing-cleanup]
with name_cm() as context:
def genfunc_with_name_cm():
with name_cm() as context: # [contextmanager-generator-missing-cleanup]
yield context * 2


def genfunc_with_cm_after(): # [contextmanager-generator-missing-cleanup]
with after_cm() as context:
def genfunc_with_cm_after():
with after_cm() as context: # [contextmanager-generator-missing-cleanup]
yield context * 2


Expand All @@ -56,8 +56,8 @@ def cm_with_improper_handling():
print("cm exit")


def genfunc_with_cm_improper(): # [contextmanager-generator-missing-cleanup]
with cm_with_improper_handling() as context:
def genfunc_with_cm_improper():
with cm_with_improper_handling() as context: # [contextmanager-generator-missing-cleanup]
yield context * 2


Expand Down Expand Up @@ -175,3 +175,15 @@ def genfunc_with_cm_bare_handler():
def genfunc_with_cm_base_exception_handler():
with cm_base_exception_handler() as context:
yield context * 2


@contextlib.contextmanager
def good_cm_no_cleanup():
contextvar = "acquired context"
print("cm enter")
yield contextvar


def good_cm_no_cleanup_genfunc():
with good_cm_no_cleanup() as context:
yield context * 2
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
contextmanager-generator-missing-cleanup:17:0:17:19:genfunc_with_cm:The context used in function 'genfunc_with_cm' will not be exited.:UNDEFINED
contextmanager-generator-missing-cleanup:30:0:30:24:genfunc_with_name_cm:The context used in function 'genfunc_with_name_cm' will not be exited.:UNDEFINED
contextmanager-generator-missing-cleanup:35:0:35:25:genfunc_with_cm_after:The context used in function 'genfunc_with_cm_after' will not be exited.:UNDEFINED
contextmanager-generator-missing-cleanup:59:0:59:28:genfunc_with_cm_improper:The context used in function 'genfunc_with_cm_improper' will not be exited.:UNDEFINED
contextmanager-generator-missing-cleanup:18:4:19:25:genfunc_with_cm:The context used in function 'genfunc_with_cm' will not be exited.:UNDEFINED
contextmanager-generator-missing-cleanup:31:4:32:25:genfunc_with_name_cm:The context used in function 'genfunc_with_name_cm' will not be exited.:UNDEFINED
contextmanager-generator-missing-cleanup:36:4:37:25:genfunc_with_cm_after:The context used in function 'genfunc_with_cm_after' will not be exited.:UNDEFINED
contextmanager-generator-missing-cleanup:60:4:61:25:genfunc_with_cm_improper:The context used in function 'genfunc_with_cm_improper' will not be exited.:UNDEFINED

0 comments on commit 98c5af9

Please sign in to comment.