-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[flake8-use-pathlib] Add fix for PTH123
#20169
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
Conversation
|
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| PTH123 | 1684 | 0 | 0 | 1684 | 0 |
|
I'm not sure which tests should be included in the tests files, but i tested in this file: The first line with # ruff: noqa: SIM115, PLW1514, FBT003
import builtins
from pathlib import Path
_file = "file.txt"
_x = ("r+", -1)
r_plus = "r+"
builtins.open(file=_file) # noqa: PTH123
builtins.open(file=_file)
Path(_file).open()
open(mode="wb", file=_file) # noqa: PTH123
open(mode="wb", file=_file)
Path(_file).open(mode="wb")
open(mode="r+", buffering=-1, file=_file, encoding="utf-8") # noqa: PTH123
open(mode="r+", buffering=-1, file=_file, encoding="utf-8")
Path(_file).open(mode="r+", buffering=-1, encoding="utf-8")
open(_file, "r+", -1, None, None, None) # noqa: PTH123
open(_file, "r+", -1, None, None, None)
Path(_file).open("r+", -1, None, None, None)
open(_file, "r+", -1, None, None, None, closefd=True, opener=None) # noqa: PTH123
open(_file, "r+", -1, None, None, None, closefd=True, opener=None)
Path(_file).open("r+", -1, None, None, None)
open(_file, mode="r+", buffering=-1, encoding=None, errors=None, newline=None) # noqa: PTH123
open(_file, mode="r+", buffering=-1, encoding=None, errors=None, newline=None)
Path(_file).open(mode="r+", buffering=-1, encoding=None, errors=None, newline=None)
open(buffering=- 1, file=_file, encoding= "utf-8") # noqa: PTH123
open(buffering=- 1, file=_file, encoding= "utf-8")
Path(_file).open(buffering=-1, encoding="utf-8")
open(_file, "r+", - 1, None, None, None, True, None) # noqa: PTH123
open(_file, "r+", - 1, None, None, None, True, None)
Path(_file).open("r+", -1, None, None, None)
open(_file, "r+ ", - 1) # noqa: PTH123
open(_file, "r+ ", - 1)
Path(_file).open("r+ ", -1)
open(_file, f" {r_plus} ", - 1) # noqa: PTH123
open(_file, f" {r_plus} ", - 1)
Path(_file).open(f" {r_plus} ", -1)
# Only diagnostic
open()
open(_file, *_x)
open(_file, "r+", unknown=True)
open(_file, "r+", closefd=False)
open(_file, "r+", None, None, None, None, None, None, None) |
ntBre
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I think you could add those tests to a new file called PTH123.py. It looks like there's some overlap with the test you added in import_from.py, so we could either add the additional file or just add more cases there?
This looks good to me overall, just a few minor suggestions.
crates/ruff_linter/src/rules/flake8_use_pathlib/rules/builtin_open.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/flake8_use_pathlib/rules/builtin_open.rs
Outdated
Show resolved
Hide resolved
...pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_from.py.snap
Outdated
Show resolved
Hide resolved
|
i don't think that's the way to fix it either |
ntBre
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Just two more minor things, but I think this looks good otherwise.
crates/ruff_linter/src/rules/flake8_use_pathlib/rules/builtin_open.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/flake8_use_pathlib/rules/builtin_open.rs
Outdated
Show resolved
Hide resolved
ntBre
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
crates/ruff_linter/src/rules/flake8_use_pathlib/rules/builtin_open.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/flake8_use_pathlib/rules/builtin_open.rs
Outdated
Show resolved
Hide resolved
|
I guess I forgot about that, thanks for the change |
Summary
Part of #2331
Test Plan
cargo nextest run flake8_use_pathlib