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

SIM115 false positive on functions that return file handler objects #13862

Closed
gboeing opened this issue Oct 21, 2024 · 3 comments · Fixed by #14066
Closed

SIM115 false positive on functions that return file handler objects #13862

gboeing opened this issue Oct 21, 2024 · 3 comments · Fixed by #14066
Assignees
Labels
bug Something isn't working help wanted Contributions especially welcome rule Implementing or modifying a lint rule

Comments

@gboeing
Copy link

gboeing commented Oct 21, 2024

ruff 0.7 gives a false positive for SIM115 on something like:

def my_function(filepath, encoding):
    return bz2.open(filepath, mode="rt", encoding=encoding)

which can be resolved like:

def my_function(filepath, encoding):
    with bz2.open(filepath, mode="rt", encoding=encoding) as f:
        return f

But this prevents the calling function from being able to work with f because the context manager has closed it. It seems that SIM115 should not trigger when returning a file handler object from a function?

@zanieb
Copy link
Member

zanieb commented Oct 21, 2024

Yeah strong agree that this should not be flagged.

Related #13679 (comment)

@zanieb zanieb added bug Something isn't working rule Implementing or modifying a lint rule labels Oct 21, 2024
@MichaReiser MichaReiser added the help wanted Contributions especially welcome label Oct 21, 2024
@AlexWaygood
Copy link
Member

I agree that this could be improved. FWIW, though, this is really a pre-existing issue that could occur prior to Ruff 0.7. For example, Ruff 0.1 emits SIM115 on this code that uses the builtin open function:

def my_function(filepath, encoding):
    return open(filepath, mode="rt", encoding=encoding)

The only difference in Ruff 0.7 is that the rule is now applied consistently to more stdlib functions that open files, rather than just the builtin open function and pathlib.Path.open.

@AlexWaygood AlexWaygood changed the title SIM115 false positive in 0.7 SIM115 false positive on functions that return file handler objects Oct 21, 2024
@AlexWaygood
Copy link
Member

def my_function(filepath, encoding):
    return bz2.open(filepath, mode="rt", encoding=encoding)

Ideally, of course, rather than writing a wrapper function that returns an open file handle, you'd write a wrapper context manager that yields an open file handle, because then you can be confident that the file will always be closed after you're done with it:

from contextlib import contextmanager

@contextmanager
def my_function(filepath, encoding):
    with bz2.open(filepath, mode="rt", encoding=encoding) as f:
        yield f

Enforcing that kind of practice is probably outside the scope of this rule, however.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Contributions especially welcome rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants