[`flake8-use-pathlib`] Add autofix for `PTH202` by chirizxc · Pull Request #18763 · astral-sh/ruff

chirizxc · 2025-06-18T17:47:00Z

Summary

/closes #2331

Test Plan

update snapshots

chirizxc · 2025-06-18T17:51:32Z

this not the final version yet, and i still have some questions

chirizxc · 2025-06-18T17:56:55Z

should we remove [ import os, import import os.path, from os.path import getsize ] if it is not needed after the fix?

import os

x = os.path.getsize(filename="filename")
y = os.path.getsize(filename=b"filename")
z = os.path.getsize(filename=__file__)

=>

import os # unused
from pathlib import Path # we also have to import it if it wasn't there before

x = Path("filename").stat().st_size
y = Path(b"filename").stat().st_size
z = Path(__file__).stat().st_size

ntBre · 2025-06-18T18:00:54Z

I don't think you need to remove the imports, you can defer to unused-import (F401) for that, but you do need to add the Path import if it's required for the fix.

Let me know if you have more questions or when it's ready for review!

chirizxc · 2025-06-18T18:01:58Z

as well as, for example

os.path.getsize(Path("filename"))

=>

Path(Path("filename")).stat().st_size

chirizxc · 2025-06-18T18:03:24Z

The call to Path with parentheses here seems redundant, but I don't think we can consider it unnecessary since it might have additional methods like .parents[3] or .resolve()

github-actions · 2025-06-18T18:05:46Z

`ruff-ecosystem` results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+0 -0 violations, +20 -0 fixes in 2 projects; 53 projects unchanged)

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

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

+ airflow-core/tests/unit/utils/test_log_handlers.py:375:29: PTH202 [*] `os.path.getsize` should be replaced by `Path.stat().st_size`
- airflow-core/tests/unit/utils/test_log_handlers.py:375:29: PTH202 `os.path.getsize` should be replaced by `Path.stat().st_size`
+ airflow-core/tests/unit/utils/test_log_handlers.py:376:30: PTH202 [*] `os.path.getsize` should be replaced by `Path.stat().st_size`
- airflow-core/tests/unit/utils/test_log_handlers.py:376:30: PTH202 `os.path.getsize` should be replaced by `Path.stat().st_size`
+ dev/breeze/src/airflow_breeze/utils/parallel.py:308:24: PTH202 [*] `os.path.getsize` should be replaced by `Path.stat().st_size`
- dev/breeze/src/airflow_breeze/utils/parallel.py:308:24: PTH202 `os.path.getsize` should be replaced by `Path.stat().st_size`
+ providers/amazon/src/airflow/providers/amazon/aws/transfers/dynamodb_to_s3.py:244:16: PTH202 [*] `os.path.getsize` should be replaced by `Path.stat().st_size`
- providers/amazon/src/airflow/providers/amazon/aws/transfers/dynamodb_to_s3.py:244:16: PTH202 `os.path.getsize` should be replaced by `Path.stat().st_size`

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/setup/generate_secrets.py:65:47: PTH202 [*] `os.path.getsize` should be replaced by `Path.stat().st_size`
- scripts/setup/generate_secrets.py:65:47: PTH202 `os.path.getsize` should be replaced by `Path.stat().st_size`
+ tools/lib/test_server.py:64:45: PTH202 [*] `os.path.getsize` should be replaced by `Path.stat().st_size`
- tools/lib/test_server.py:64:45: PTH202 `os.path.getsize` should be replaced by `Path.stat().st_size`
+ zerver/data_import/mattermost.py:361:21: PTH202 [*] `os.path.getsize` should be replaced by `Path.stat().st_size`
- zerver/data_import/mattermost.py:361:21: PTH202 `os.path.getsize` should be replaced by `Path.stat().st_size`
+ zerver/data_import/slack.py:1550:70: PTH202 [*] `os.path.getsize` should be replaced by `Path.stat().st_size`
- zerver/data_import/slack.py:1550:70: PTH202 `os.path.getsize` should be replaced by `Path.stat().st_size`
+ zerver/lib/export.py:2955:41: PTH202 [*] `os.path.getsize` should be replaced by `Path.stat().st_size`
- zerver/lib/export.py:2955:41: PTH202 `os.path.getsize` should be replaced by `Path.stat().st_size`
+ zerver/lib/upload/local.py:110:45: PTH202 [*] `os.path.getsize` should be replaced by `Path.stat().st_size`
- zerver/lib/upload/local.py:110:45: PTH202 `os.path.getsize` should be replaced by `Path.stat().st_size`

Changes by rule (1 rules affected)

code	total	+ violation	- violation	+ fix	- fix
PTH202	20	0	0	20	0

chirizxc · 2025-06-18T19:38:19Z

as i understand: PTH203, PTH204, PTH205 will be exactly the same in implementation

chirizxc · 2025-06-18T19:43:10Z

is it worth splitting pr if the changes are the same as this pr? the question is more to this one

MichaReiser · 2025-06-19T14:39:12Z

is it worth splitting pr if the changes are the same as this pr? the question is more to #18752 (comment)

Let's start with one fix. Then the second PR can generalize it and apply it to the other rules.

ntBre

Thanks! I have some questions and suggestions here.

crates/ruff_linter/src/checkers/ast/analyze/expression.rs

crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getsize.rs

# Conflicts: # crates/ruff_linter/src/checkers/ast/analyze/expression.rs

ntBre

Thanks, this looks good. I often forget this, but based on our versioning policy, adding a safe fix actually needs to be done in preview. Could you add a preview method like this and gate the fix behind that?

ruff/crates/ruff_linter/src/preview.rs

Lines 43 to 46 in 833be2e

    
           // https://github.com/astral-sh/ruff/pull/16719 
        
           pub(crate) const fn is_fix_manual_dict_comprehension_enabled(settings: &LinterSettings) -> bool { 
        
               settings.preview.is_enabled() 
        
           }

ntBre · 2025-06-24T12:51:09Z

...8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__PTH202_PTH202.py.snap

+10 10 | os.path.getsize("filename")
+11 11 | os.path.getsize(b"filename")
+12    |-os.path.getsize(Path("filename"))
+   12 |+Path(Path("filename")).stat().st_size


Have you looked into how hard it would be to detect this? I think it would just be a match on the argument to see if it's a call to Path. I don't think this is a deal-breaker for this PR, but it is a bit unfortunate, as you pointed out.

I think if you look just for an Expr::Call, it should avoid your concern about additional attributes like Path(...).resolve() because those would be Expr::Attributes.

chirizxc · 2025-06-24T14:49:58Z

Thanks, this looks good. I often forget this, but based on our versioning policy, adding a safe fix actually needs to be done in preview. Could you add a preview method like this and gate the fix behind that?

ruff/crates/ruff_linter/src/preview.rs

Lines 43 to 46 in 833be2e

// https://github.com/astral-sh/ruff/pull/16719

pub(crate) const fn is_fix_manual_dict_comprehension_enabled(settings: &LinterSettings) -> bool {

settings.preview.is_enabled()

}

should the changes disappear after that in the snapshots?

chirizxc · 2025-06-24T14:51:49Z

Thanks, this looks good. I often forget this, but based on our versioning policy, adding a safe fix actually needs to be done in preview. Could you add a preview method like this and gate the fix behind that?

ruff/crates/ruff_linter/src/preview.rs

Lines 43 to 46 in 833be2e

// https://github.com/astral-sh/ruff/pull/16719

pub(crate) const fn is_fix_manual_dict_comprehension_enabled(settings: &LinterSettings) -> bool {

settings.preview.is_enabled()

}

should the changes disappear after that in the spanshots?

i added is_fix_os_path_path_getsize_enabled, but the changes of fix to the snapshots are gone

ntBre · 2025-06-24T15:36:30Z

Yes, you'll need to add a version of the PTH202 test that enables preview mode to see the preview snapshot changes.

chirizxc · 2025-06-24T16:18:13Z

done

ntBre

Thanks!

crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getsize.rs

…getsize.rs

* main: [ty] Fix false positives when subscripting an object inferred as having an `Intersection` type (#18920) [`flake8-use-pathlib`] Add autofix for `PTH202` (#18763) [ty] Add relative import completion tests [ty] Clarify what "cursor" means [ty] Add a cursor test builder [ty] Enforce sort order of completions (#18917) [formatter] Fix missing blank lines before decorated classes in .pyi files (#18888) Apply fix availability and applicability when adding to `DiagnosticGuard` and remove `NoqaCode::rule` (#18834) py-fuzzer: allow relative executable paths (#18915) [ty] Change `environment.root` to accept multiple paths (#18913) [ty] Rename `src.root` setting to `environment.root` (#18760) Use file path for detecting package root (#18914) Consider virtual path for various server actions (#18910) [ty] Introduce `UnionType::try_from_elements` and `UnionType::try_map` (#18911) [ty] Support narrowing on `isinstance()`/`issubclass()` if the second argument is a dynamic, intersection, union or typevar type (#18900) [ty] Add decorator check for implicit attribute assignments (#18587) [`ruff`] Trigger `RUF037` for empty string and byte strings (#18862) [ty] Avoid duplicate diagnostic in unpacking (#18897) [`pyupgrade`] Extend version detection to include `sys.version_info.major` (`UP036`) (#18633) [`ruff`] Frozen Dataclass default should be valid (`RUF009`) (#18735)

…#18922)  ## Summary Part of #2331 | [#18763](#18763 (comment))  ## Test Plan update snapshots

[flake8-use-pathlib] add autofix for PTH202

87448bc

ntBre self-requested a review June 18, 2025 17:50

ntBre added the fixes Related to suggested fixes for violations label Jun 18, 2025

fix: clippy

f835f7d

chirizxc added 3 commits June 18, 2025 21:23

fix: add pathlib import if it needed

cf5aca8

fix: clippy

ba0d9ed

fix: make FixAvailability Sometimes

5b06806

chirizxc added 4 commits June 20, 2025 13:59

doc: add Fix Safety

5835c09

fix: fmt

d660a4d

fix: doc

aa711cd

chore: more test

78f06ab

ntBre requested changes Jun 20, 2025

View reviewed changes

chirizxc added 3 commits June 20, 2025 17:28

chore: apply review suggestions

5fc55ae

Merge remote-tracking branch 'origin/main' into feat/pth202-autofix

56bda9b

# Conflicts: # crates/ruff_linter/src/checkers/ast/analyze/expression.rs

fix

453e932

ntBre approved these changes Jun 24, 2025

View reviewed changes

feat: avoid redundant Path()` wrapping in os.path.getsize fix

0878808

chore: add preview

7d2bf85

fix: clippy

1d6587f

chirizxc added 4 commits June 24, 2025 19:45

fix

aa94484

fix: fmt

695fc84

fix: clippy

ab5b2b4

fix: clippy (x2)

e4c7aab

ntBre approved these changes Jun 24, 2025

View reviewed changes

crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getsize.rs Show resolved Hide resolved

Update crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_…

aa82460

…getsize.rs

ntBre added the preview Related to preview mode features label Jun 24, 2025

ntBre changed the title ~~[flake8-use-pathlib] add autofix for PTH202~~ [flake8-use-pathlib] Add autofix for PTH202 Jun 24, 2025

ntBre enabled auto-merge (squash) June 24, 2025 17:55

ntBre merged commit 3220242 into astral-sh:main Jun 24, 2025
33 checks passed

chirizxc deleted the feat/pth202-autofix branch June 24, 2025 18:18

chirizxc mentioned this pull request Jun 24, 2025

[flake8-use-pathlib] Add autofixes for PTH203, PTH204, PTH205 #18922

Merged

ntBre mentioned this pull request Jun 26, 2025

autofix for flake8-use-pathlib (PTH) rules #2331

Open

BrewTestBot mentioned this pull request Jun 26, 2025

ruff 0.12.1 Homebrew/homebrew-core#228300

Merged

	// https://github.com/astral-sh/ruff/pull/16719
	pub(crate) const fn is_fix_manual_dict_comprehension_enabled(settings: &LinterSettings) -> bool {
	settings.preview.is_enabled()
	}

Conversation

chirizxc commented Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Plan

Uh oh!

chirizxc commented Jun 18, 2025

Uh oh!

chirizxc commented Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ntBre commented Jun 18, 2025

Uh oh!

chirizxc commented Jun 18, 2025

Uh oh!

chirizxc commented Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ruff-ecosystem results

Linter (stable)

Linter (preview)

Uh oh!

chirizxc commented Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chirizxc commented Jun 18, 2025

Uh oh!

MichaReiser commented Jun 19, 2025

Uh oh!

ntBre left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ntBre left a comment

Choose a reason for hiding this comment

Uh oh!

ntBre Jun 24, 2025

Choose a reason for hiding this comment

Uh oh!

chirizxc commented Jun 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chirizxc commented Jun 24, 2025

Uh oh!

ntBre commented Jun 24, 2025

Uh oh!

chirizxc commented Jun 24, 2025

Uh oh!

ntBre left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

chirizxc commented Jun 18, 2025 •

edited

Loading

chirizxc commented Jun 18, 2025 •

edited

Loading

chirizxc commented Jun 18, 2025 •

edited

Loading

github-actions bot commented Jun 18, 2025 •

edited

Loading

`ruff-ecosystem` results

chirizxc commented Jun 18, 2025 •

edited

Loading

chirizxc commented Jun 24, 2025 •

edited

Loading