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
False-positive consider-using-with
when returning a file handler
#4748
Comments
@DudeNr33 Would you mind taking a look at this one? |
Will do, but I don't really know if this should be considered a false positive, at least with the example you gave. The intention of this message is to inform the user that he is doing something which shifts the responsibility of freeing the allocated resources to somebody else. |
The reason I opened the issue is that I almost introduced a bug because of it. I didn't realize at the time that even after a return in a context manager the file is closed. It is pretty similar to def func():
with open("test.py") as fp:
return fp.read() which does work as expected. You are right that it shifts the responsibility to the caller, but I think emitting the |
I understand your concern. Trying to satisfy the linter should ideally not lead to erroneous code, even though this can not always be avoided - I've seen that more than once that fixing linter messages introduced new bugs. The question is: should we rather I had a quick look at the file changed by the commit where you mentioned this issue. from contextlib import contextmanager
@contextmanager
def load_data(...):
try:
# other code
with open(filepath, "rb") as file_content:
yield file_content
except:
# exception handling
yield None You can then use In my opinion that's a suitable solution and something that the (Edited the code: there must only be one |
Oh, or option d) Actually do suppress |
I would like that! |
You can assign this issue to me if you like. :) |
Bug description
(Configuration)
Using the following configuration:
Command used
(Expected behavior)
The example above might not be consider good code style, but it's enough to reproduce the issue. If the code is replaced by
Python will through an exception
(Version affected)
(OS / Environment)
No response
(Additional dependencies)
No response
The text was updated successfully, but these errors were encountered: