Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix false-positive with contextmanager missing cleanup #9654

Merged
merged 4 commits into from
May 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading