-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[flake8-use-pathlib] Add autofix for PTH109
#19245
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 |
|---|---|---|---|---|---|
| PTH109 | 50 | 0 | 0 | 50 | 0 |
|
@ntBre I'm not really sure about the behavior of cwd = os.getcwd(
# 1
)when we have comments inside brackets, because I've seen this somewhere, even though the function takes no arguments it's still worth marking the autofix as unsafe |
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, this looks good. We just need to validate the number of arguments before offering a fix.
when we have comments inside brackets, because I've seen this somewhere, even though the function takes no arguments it's still worth marking the autofix as unsafe
I think you made the right call making the fix unsafe, even if this is probably uncommon 👍
...e_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_full_name.py.snap
Outdated
Show resolved
Hide resolved
|
What about |
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!
What about os.getcwdb()? Is it worth adding an example in the documentation?
I think it's okay to leave out the example since it's so similar to the existing one. The ## What it does section mentions getcwdb at least.
Summary
Part of #2331
Test Plan
cargo nextest run flake8_use_pathlib