-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[flake8-use-pathlib
] Add autofixes for PTH203
, PTH204
, PTH205
#18922
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
You'll need to update the snapshots (they assert the expected output) using |
i did, but the problem is that they don't update correctly |
Can you say more about why they don't update correctly? I did
and the snapshots were updated correctly |
i applied the changes from past snapshots, but for some reason they didn't overwrite now, very strange |
Did you add them with |
yes |
Can you run |
❯ git status
On branch feat/autofixes
Your branch is up to date with 'origin/feat/autofixes'.
Changes to be committed:
(use "git restore --staged <file>..." to unstage)
modified: crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__PTH203_PTH203.py.snap
new file: crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview__PTH203_PTH203.py.snap
new file: crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview__PTH204_PTH204.py.snap
new file: crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview__PTH205_PTH205.py.snap
Changes not staged for commit:
(use "git add <file>..." to update what will be committed)
(use "git restore <file>..." to discard changes in working directory)
modified: crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__PTH204_PTH204.py.snap
modified: crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__PTH205_PTH205.py.
``` |
❯ git diff
diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__PTH204_PTH204.py.snap b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__PTH204_PTH204.py.snap
index b137cbd77..6d3b40def 100644
--- a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__PTH204_PTH204.py.snap
+++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__PTH204_PTH204.py.snap
@@ -1,51 +1,357 @@
---
source: crates/ruff_linter/src/rules/flake8_use_pathlib/mod.rs
-snapshot_kind: text
---
-PTH204.py:6:1: PTH204 `os.path.getmtime` should be replaced by `Path.stat().st_mtime`
- |
-6 | os.path.getmtime("filename")
- | ^^^^^^^^^^^^^^^^ PTH204
-7 | os.path.getmtime(b"filename")
-8 | os.path.getmtime(Path("filename"))
- |
+PTH204.py:11:1: PTH202 `os.path.getsize` should be replaced by `Path.stat().st_size`
+ |
+11 | os.path.getmtime("filename")
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PTH202
+12 | os.path.getmtime(b"filename")
+13 | os.path.getmtime(Path("filename"))
+ |
:...skipping...
diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__PTH204_PTH204.py.snap b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__PTH204_PTH204.py.snap
index b137cbd77..6d3b40def 100644
--- a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__PTH204_PTH204.py.snap
+++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__PTH204_PTH204.py.snap
@@ -1,51 +1,357 @@
---
source: crates/ruff_linter/src/rules/flake8_use_pathlib/mod.rs
-snapshot_kind: text
---
-PTH204.py:6:1: PTH204 `os.path.getmtime` should be replaced by `Path.stat().st_mtime`
- |
-6 | os.path.getmtime("filename")
- | ^^^^^^^^^^^^^^^^ PTH204
-7 | os.path.getmtime(b"filename")
-8 | os.path.getmtime(Path("filename"))
- |
+PTH204.py:11:1: PTH202 `os.path.getsize` should be replaced by `Path.stat().st_size`
+ |
+11 | os.path.getmtime("filename")
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PTH202
+12 | os.path.getmtime(b"filename")
+13 | os.path.getmtime(Path("filename"))
+ |
+ = help: Replace with `Path(...).stat().st_size`
+
+PTH204.py:12:1: PTH202 `os.path.getsize` should be replaced by `Path.stat().st_size`
+ |
+11 | os.path.getmtime("filename")
ESCOD
diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__PTH204_PTH204.py.snap b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__PTH204_PTH204.py.snap
index b137cbd77..6d3b40def 100644
--- a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__PTH204_PTH204.py.snap
+++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__PTH204_PTH204.py.snap
index b137cbd77..6d3b40def 100644
--- a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__PTH204_PTH204.py.snap
+++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__PTH204_PTH204.py.snap
@@ -1,51 +1,357 @@
---
source: crates/ruff_linter/src/rules/flake8_use_pathlib/mod.rs
-snapshot_kind: text
---
-PTH204.py:6:1: PTH204 `os.path.getmtime` should be replaced by `Path.stat().st_mtime`
- |
-6 | os.path.getmtime("filename")
- | ^^^^^^^^^^^^^^^^ PTH204
-7 | os.path.getmtime(b"filename")
-8 | os.path.getmtime(Path("filename"))
- |
+PTH204.py:11:1: PTH202 `os.path.getsize` should be replaced by `Path.stat().st_size`
+ |
+11 | os.path.getmtime("filename")
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PTH202
+12 | os.path.getmtime(b"filename")
+13 | os.path.getmtime(Path("filename"))
+ |
+ = help: Replace with `Path(...).stat().st_size`
+
+PTH204.py:12:1: PTH202 `os.path.getsize` should be replaced by `Path.stat().st_size`
+ |
+11 | os.path.getmtime("filename")
+12 | os.path.getmtime(b"filename")
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PTH202 |
The output suggests that you still have uncommited local changes. You have to stage them using |
|
code | total | + violation | - violation | + fix | - fix |
---|---|---|---|---|---|
PTH204 | 26 | 0 | 0 | 26 | 0 |
PTH205 | 2 | 0 | 0 | 2 | 0 |
tests on github runners also passed, i don't really understand why, maybe it's because i did something wrong in adding the tests |
oh, sorry, i found my stupid mistake :( |
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 had some suggestions to factor out common code and maybe strip down new test cases a bit.
crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getatime.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getctime.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getmtime.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/resources/test/fixtures/flake8_use_pathlib/PTH203.py
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getctime.rs
Show resolved
Hide resolved
looks like windows runner down |
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 a couple more suggestions, mostly around test organization.
crates/ruff_linter/resources/test/fixtures/flake8_use_pathlib/PTH203.py
Outdated
Show resolved
Hide resolved
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 a couple more nits and a larger, but easy to fix, concern about the diagnostic range that I didn't notice before.
...8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__PTH203_PTH203.py.snap
Outdated
Show resolved
Hide resolved
crates/ruff_linter/resources/test/fixtures/flake8_use_pathlib/PTH204.py
Outdated
Show resolved
Hide resolved
done! waiting CI |
CodSpeed Instrumentation Performance ReportMerging #18922 will not alter performanceComparing Summary
|
Uh, I think something very weird just happened with CI. Both the ecosystem report and benchmarks are showing very unreasonable results. I'll try rerunning it. I definitely don't think your changes caused this. |
@@ -4,7 +4,7 @@ source: crates/ruff_linter/src/rules/flake8_use_pathlib/mod.rs | |||
PTH202.py:10:1: PTH202 `os.path.getsize` should be replaced by `Path.stat().st_size` | |||
| | |||
10 | os.path.getsize("filename") | |||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ PTH202 | |||
| ^^^^^^^^^^^^^^^ PTH202 |
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.
Ahh, I see. I missed this on #18763. The shorter range is how PTH202 used to be too, so this is perfect.
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! We'll get CI to pass somehow and then this is good to go. I may try closing and reopening the PR to fully re-trigger it if re-running through the actions interface doesn't work.
Hmm, rerunning didn't seem to help. Do you want to try merging |
if checker | ||
.semantic() | ||
.resolve_qualified_name(&call.func) | ||
.is_some_and(|qualified_name| qualified_name.segments() != ["os", "path", fn_name]) |
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.
Well this is a bit embarrassing, but I just realized that my suggestion did break your code, and CI was correct. This should be is_none_or
to preserve the original behavior.
.is_some_and(|qualified_name| qualified_name.segments() != ["os", "path", fn_name]) | |
.is_none_or(|qualified_name| qualified_name.segments() != ["os", "path", fn_name]) |
oh, it seems to working, thank you! |
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.
Great, thanks again! And sorry for the bad suggestion
…c_tokens * 'main' of https://github.com/astral-sh/ruff: (27 commits) [ty] First cut at semantic token provider (astral-sh#19108) [`flake8-simplify`] Make example error out-of-the-box (`SIM116`) (astral-sh#19111) [`flake8-use-pathlib`] Make example error out-of-the-box (`PTH210`) (astral-sh#19189) [`flake8-use-pathlib`] Add autofixes for `PTH203`, `PTH204`, `PTH205` (astral-sh#18922) [`flake8-type-checking`] Fix syntax error introduced by fix (`TC008`) (astral-sh#19150) [`flake8-pyi`] Make example error out-of-the-box (`PYI007`, `PYI008`) (astral-sh#19103) Update Rust crate indicatif to 0.18.0 (astral-sh#19165) [ty] Add separate CI job for memory usage stats (astral-sh#19134) [ty] Add documentation for server traits (astral-sh#19137) Rename to `SessionSnapshot`, move unwind assertion closer (astral-sh#19177) [`flake8-type-checking`] Make example error out-of-the-box (`TC001`) (astral-sh#19151) [ty] Bare `ClassVar` annotations (astral-sh#15768) [ty] Re-enable multithreaded pydantic benchmark (astral-sh#19176) [ty] Implement equivalence for protocols with method members (astral-sh#18659) [ty] Use RHS inferred type for bare `Final` symbols (astral-sh#19142) [ty] Support declaration-only attributes (astral-sh#19048) [ty] Sync vendored typeshed stubs (astral-sh#19174) Update dependency pyodide to ^0.28.0 (astral-sh#19164) Update NPM Development dependencies (astral-sh#19170) Update taiki-e/install-action action to v2.56.7 (astral-sh#19169) ...
Summary
Part of #2331 | #18763
Test Plan
update snapshots