Skip to content

[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

Merged
merged 17 commits into from
Jul 7, 2025

Conversation

chirizxc
Copy link
Contributor

@chirizxc chirizxc commented Jun 24, 2025

Summary

Part of #2331 | #18763

Test Plan

update snapshots

@chirizxc chirizxc marked this pull request as draft June 24, 2025 19:40
@chirizxc
Copy link
Contributor Author

chirizxc commented Jun 24, 2025

idk why cargo nextest run pth205 doesn't go through properly after these names
изображение

@MichaReiser
Copy link
Member

You'll need to update the snapshots (they assert the expected output) using cargo insta review (https://insta.rs/docs/cli/)

@chirizxc
Copy link
Contributor Author

You'll need to update the snapshots (they assert the expected output) usi

i did, but the problem is that they don't update correctly

@MichaReiser
Copy link
Member

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

cargo nextest run -p ruff_linter --no-fail-fast
cargo insta review

and the snapshots were updated correctly

@chirizxc
Copy link
Contributor Author

cargo insta review

изображение

@chirizxc
Copy link
Contributor Author

i applied the changes from past snapshots, but for some reason they didn't overwrite now, very strange

@MichaReiser
Copy link
Member

Did you add them with git add <path to snapshot> or git add .?

@chirizxc
Copy link
Contributor Author

Did you add them with git add <path to snapshot> or git add .?

yes

@MichaReiser
Copy link
Member

Can you run git status and git diff and share the output

@chirizxc
Copy link
Contributor Author

chirizxc commented Jun 25, 2025

❯ 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.
       ```

@chirizxc
Copy link
Contributor Author

chirizxc commented Jun 25, 2025

❯ 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     

@MichaReiser
Copy link
Member

The output suggests that you still have uncommited local changes. You have to stage them using git add ., then commit and push them.

Copy link
Contributor

github-actions bot commented Jun 25, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+0 -0 violations, +28 -0 fixes in 3 projects; 52 projects unchanged)

apache/airflow (+0 -0 violations, +12 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview --select ALL

+ airflow-core/src/airflow/dag_processing/manager.py:931:38: PTH204 [*] `os.path.getmtime` should be replaced by `Path.stat().st_mtime`
- airflow-core/src/airflow/dag_processing/manager.py:931:38: PTH204 `os.path.getmtime` should be replaced by `Path.stat().st_mtime`
+ airflow-core/src/airflow/models/dagbag.py:318:64: PTH204 [*] `os.path.getmtime` should be replaced by `Path.stat().st_mtime`
- airflow-core/src/airflow/models/dagbag.py:318:64: PTH204 `os.path.getmtime` should be replaced by `Path.stat().st_mtime`
+ devel-common/src/sphinx_exts/docs_build/fetch_inventories.py:92:71: PTH204 [*] `os.path.getmtime` should be replaced by `Path.stat().st_mtime`
- devel-common/src/sphinx_exts/docs_build/fetch_inventories.py:92:71: PTH204 `os.path.getmtime` should be replaced by `Path.stat().st_mtime`
+ providers/standard/src/airflow/providers/standard/sensors/filesystem.py:132:60: PTH204 [*] `os.path.getmtime` should be replaced by `Path.stat().st_mtime`
- providers/standard/src/airflow/providers/standard/sensors/filesystem.py:132:60: PTH204 `os.path.getmtime` should be replaced by `Path.stat().st_mtime`
+ providers/standard/src/airflow/providers/standard/triggers/file.py:124:30: PTH204 [*] `os.path.getmtime` should be replaced by `Path.stat().st_mtime`
- providers/standard/src/airflow/providers/standard/triggers/file.py:124:30: PTH204 `os.path.getmtime` should be replaced by `Path.stat().st_mtime`
+ providers/standard/src/airflow/providers/standard/triggers/file.py:77:34: PTH204 [*] `os.path.getmtime` should be replaced by `Path.stat().st_mtime`
- providers/standard/src/airflow/providers/standard/triggers/file.py:77:34: PTH204 `os.path.getmtime` should be replaced by `Path.stat().st_mtime`

bokeh/bokeh (+0 -0 violations, +4 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview --select ALL

+ src/bokeh/sphinxext/bokeh_gallery.py:125:26: PTH204 [*] `os.path.getmtime` should be replaced by `Path.stat().st_mtime`
- src/bokeh/sphinxext/bokeh_gallery.py:125:26: PTH204 `os.path.getmtime` should be replaced by `Path.stat().st_mtime`
+ src/bokeh/sphinxext/bokeh_gallery.py:146:41: PTH204 [*] `os.path.getmtime` should be replaced by `Path.stat().st_mtime`
- src/bokeh/sphinxext/bokeh_gallery.py:146:41: PTH204 `os.path.getmtime` should be replaced by `Path.stat().st_mtime`

zulip/zulip (+0 -0 violations, +12 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview --select ALL

+ scripts/lib/zulip_tools.py:359:12: PTH205 [*] `os.path.getctime` should be replaced by `Path.stat().st_ctime`
- scripts/lib/zulip_tools.py:359:12: PTH205 `os.path.getctime` should be replaced by `Path.stat().st_ctime`
+ zerver/data_import/mattermost.py:362:24: PTH204 [*] `os.path.getmtime` should be replaced by `Path.stat().st_mtime`
- zerver/data_import/mattermost.py:362:24: PTH204 `os.path.getmtime` should be replaced by `Path.stat().st_mtime`
+ zerver/lib/compatibility.py:30:49: PTH204 [*] `os.path.getmtime` should be replaced by `Path.stat().st_mtime`
- zerver/lib/compatibility.py:30:49: PTH204 `os.path.getmtime` should be replaced by `Path.stat().st_mtime`
+ zerver/lib/test_fixtures.py:329:33: PTH204 [*] `os.path.getmtime` should be replaced by `Path.stat().st_mtime`
- zerver/lib/test_fixtures.py:329:33: PTH204 `os.path.getmtime` should be replaced by `Path.stat().st_mtime`
+ zerver/lib/test_fixtures.py:357:33: PTH204 [*] `os.path.getmtime` should be replaced by `Path.stat().st_mtime`
- zerver/lib/test_fixtures.py:357:33: PTH204 `os.path.getmtime` should be replaced by `Path.stat().st_mtime`
+ zerver/lib/upload/local.py:134:43: PTH204 [*] `os.path.getmtime` should be replaced by `Path.stat().st_mtime`
- zerver/lib/upload/local.py:134:43: PTH204 `os.path.getmtime` should be replaced by `Path.stat().st_mtime`

Changes by rule (2 rules affected)

code total + violation - violation + fix - fix
PTH204 26 0 0 26 0
PTH205 2 0 0 2 0

@chirizxc
Copy link
Contributor Author

tests on github runners also passed, i don't really understand why, maybe it's because i did something wrong in adding the tests crates/ruff_linter/src/rules/flake8_use_pathlib/mod.rs

@chirizxc
Copy link
Contributor Author

oh, sorry, i found my stupid mistake :(

@chirizxc chirizxc marked this pull request as ready for review June 25, 2025 18:59
@MichaReiser MichaReiser added fixes Related to suggested fixes for violations labels Jun 26, 2025
@MichaReiser MichaReiser requested a review from ntBre June 26, 2025 08:09
Copy link
Contributor

@ntBre ntBre left a 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.

@chirizxc
Copy link
Contributor Author

looks like windows runner down

Copy link
Contributor

@ntBre ntBre left a 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.

@ntBre ntBre added the preview Related to preview mode features label Jul 4, 2025
Copy link
Contributor

@ntBre ntBre left a 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.

@chirizxc
Copy link
Contributor Author

chirizxc commented Jul 4, 2025

done! waiting CI

Copy link

codspeed-hq bot commented Jul 4, 2025

CodSpeed Instrumentation Performance Report

Merging #18922 will not alter performance

Comparing chirizxc:feat/autofixes (de70bda) with main (44f2f77)

Summary

✅ 40 untouched benchmarks

@ntBre
Copy link
Contributor

ntBre commented Jul 4, 2025

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
Copy link
Contributor

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.

Copy link
Contributor

@ntBre ntBre left a 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.

@ntBre ntBre closed this Jul 4, 2025
@ntBre ntBre reopened this Jul 4, 2025
@ntBre
Copy link
Contributor

ntBre commented Jul 4, 2025

Hmm, rerunning didn't seem to help. Do you want to try merging main into your branch? Just in case it's a real change from the past couple of days.

if checker
.semantic()
.resolve_qualified_name(&call.func)
.is_some_and(|qualified_name| qualified_name.segments() != ["os", "path", fn_name])
Copy link
Contributor

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.

Suggested change
.is_some_and(|qualified_name| qualified_name.segments() != ["os", "path", fn_name])
.is_none_or(|qualified_name| qualified_name.segments() != ["os", "path", fn_name])

@chirizxc
Copy link
Contributor Author

chirizxc commented Jul 5, 2025

oh, it seems to working, thank you!

@MichaReiser MichaReiser requested a review from ntBre July 7, 2025 16:39
Copy link
Contributor

@ntBre ntBre left a 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

@ntBre ntBre merged commit e23780c into astral-sh:main Jul 7, 2025
35 checks passed
UnboundVariable pushed a commit to UnboundVariable/ruff that referenced this pull request Jul 7, 2025
…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)
  ...
@chirizxc chirizxc deleted the feat/autofixes branch July 8, 2025 05:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixes Related to suggested fixes for violations preview Related to preview mode features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants