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

Suppress consider-using-with on return statements #4851

Merged
merged 1 commit into from
Aug 15, 2021
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions ChangeLog
Expand Up @@ -16,6 +16,10 @@ Release date: TBA

Closes #4498

* ``consider-using-with`` is no longer triggered if a context manager is returned from a function.

Closes #4748

* pylint does not crash with a traceback anymore when a file is problematic. It
creates a template text file for opening an issue on the bug tracker instead.
The linting can go on for other non problematic files instead of being impossible.
Expand Down
2 changes: 2 additions & 0 deletions doc/whatsnew/2.10.rst
Expand Up @@ -46,6 +46,8 @@ Other Changes

* Pyreverse - add output in PlantUML format

* ``consider-using-with`` is no longer triggered if a context manager is returned from a function.

* pylint does not crash with a traceback anymore when a file is problematic. It
creates a template text file for opening an issue on the bug tracker instead.
The linting can go on for other non problematic files instead of being impossible.
Expand Down
16 changes: 14 additions & 2 deletions pylint/checkers/refactoring/refactoring_checker.py
Expand Up @@ -113,6 +113,16 @@ def _is_inside_context_manager(node: astroid.Call) -> bool:
)


def _is_a_return_statement(node: astroid.Call) -> bool:
frame = node.frame()
parent = node.parent
while parent is not frame:
if isinstance(parent, astroid.Return):
return True
parent = parent.parent
return False


def _is_part_of_with_items(node: astroid.Call) -> bool:
"""
Checks if one of the node's parents is an ``astroid.With`` node and that the node itself is located
Expand Down Expand Up @@ -1443,8 +1453,10 @@ def _append_context_managers_to_stack(self, node: astroid.Assign) -> None:
stack[varname] = value

def _check_consider_using_with(self, node: astroid.Call):
if _is_inside_context_manager(node):
# if we are inside a context manager itself, we assume that it will handle the resource management itself.
if _is_inside_context_manager(node) or _is_a_return_statement(node):
# If we are inside a context manager itself, we assume that it will handle the resource management itself.
# If the node is a child of a return, we assume that the caller knows he is getting a context manager
# he should use properly (i.e. in a ``with``).
return
if (
node
Expand Down
4 changes: 4 additions & 0 deletions tests/functional/c/consider/consider_using_with_open.py
Expand Up @@ -64,3 +64,7 @@ def test_single_line_with(file1):
def test_multiline_with_items(file1, file2, which):
with (open(file1, encoding="utf-8") if which
else open(file2, encoding="utf-8")) as input_file: return input_file.read()


def test_suppress_on_return():
return open("foo", encoding="utf8") # must not trigger