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

False positive consider-using-with on a with statement #4676

Closed
adam-azarchs opened this issue Jul 6, 2021 · 5 comments · Fixed by #4679
Closed

False positive consider-using-with on a with statement #4676

adam-azarchs opened this issue Jul 6, 2021 · 5 comments · Fixed by #4679
Assignees
Labels
False Positive 🦟 A message is emitted but nothing is wrong with the code
Milestone

Comments

@adam-azarchs
Copy link

Steps to reproduce

Given a file a.py:

# pylint: disable=missing-module-docstring,missing-function-docstring
# pylint: enable=consider-using-with


import contextlib


def read_files(file1, file2):
    with open(file1) as input_file1, (
        open(file2) if file2 else contextlib.nullcontext()
    ) as input_file2:
        if input_file2:
            return input_file1.read(), input_file2.read()
        return input_file1.read()

Current behavior

t.py:10:8: R1732: Consider using 'with' for resource-allocating operations (consider-using-with)

-->

Expected behavior

No warning. I'm already using a with statement.

pylint --version output

Result of pylint --version output:

pylint 2.9.1
astroid 2.6.2
Python 3.8.10 (default, Jun  4 2021, 15:09:15)
[GCC 7.5.0]
@Pierre-Sassoulas
Copy link
Member

Thanks for reporting the issue. It's probably due to the contextlib.nullcontext(), and related to #4665

@Pierre-Sassoulas Pierre-Sassoulas added the False Positive 🦟 A message is emitted but nothing is wrong with the code label Jul 6, 2021
@Pierre-Sassoulas
Copy link
Member

@DudeNr33 maybe I'm wrong and it's only the double open in with that is causing a problem ? What do you think ?

@DudeNr33
Copy link
Collaborator

DudeNr33 commented Jul 6, 2021

Either that, or the ternary if-else on file2.
I will look at it.
So many corner cases for this check I haven't thought of. 😀

@adam-azarchs
Copy link
Author

Simpler reproducer confirms the issue is the ternary:

# pylint: disable=missing-module-docstring,missing-function-docstring
# pylint: enable=consider-using-with


def read_files(file1, file2, which):
    with (open(file1) if which else open(file2)) as input_file:
        return input_file.read()

gives two errors:

t.py:6:10: R1732: Consider using 'with' for resource-allocating operations (consider-using-with)
t.py:6:36: R1732: Consider using 'with' for resource-allocating operations (consider-using-with)

This isn't that weird a corner case, as it's a bit awkward to use with in situations like that otherwise.

@Pierre-Sassoulas
Copy link
Member

It will be fixed in 2.9.4 thanks to @DudeNr33 !

@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.9.4 milestone Jul 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
False Positive 🦟 A message is emitted but nothing is wrong with the code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants