[flake8-use-pathlib] Expand PTH201 to check all PurePath subclasses#19440
[flake8-use-pathlib] Expand PTH201 to check all PurePath subclasses#19440ntBre merged 5 commits intoastral-sh:mainfrom
flake8-use-pathlib] Expand PTH201 to check all PurePath subclasses#19440Conversation
|
ntBre
changed the title
flake8_use_pathlib] Expand PTH201 to check all PurePath subclassesflake8-use-pathlib] Expand PTH201 to check all PurePath subclasses
ntBre
left a comment
ntBre
left a comment
There was a problem hiding this comment.
Thanks! I had one nit about the tests that will make this much easier to review, and then a suggestion about code reuse. I think factoring this out into a helper function would be fine for now, and then we could create a follow-up issue to check the other PTH rules.
| let is_pathlib = matches!( | ||
| segments, | ||
| [ | ||
| "pathlib", | ||
| "Path" | "PurePath" | "PosixPath" | "PurePosixPath" | "WindowsPath" | "PureWindowsPath" | ||
| ] | ||
| ); | ||
| let is_packagepath = matches!(segments, ["importlib", "metadata", "PackagePath"]); |
There was a problem hiding this comment.
It seems we have a couple of different existing checks that are similar to this:
ruff/crates/ruff_python_semantic/src/analyze/typing.rs
Lines 1091 to 1093 in e867830
which will check Bindings and includes all of these except PackagePath, and
ruff/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs
Lines 14 to 21 in e867830
which only checks for pathlib.Path. I don't think we can switch all of the current calls to one or the other, but we might at least want to factor this code into a helper function (is_pure_path_subclass maybe) so that we can use it in the other PTH rules, if appropriate.
cc @chirizxc who has been working on PTH autofixes
| _ = PosixPath(".") | ||
| _ = PurePosixPath(".") | ||
| _ = WindowsPath(".") | ||
| _ = PureWindowsPath(".") | ||
| _ = PackagePath(".") |
There was a problem hiding this comment.
Can we move these and the new imports to the bottom of the file? Adding them here has created quite a large diff for a file that should mostly be the same.
|
It looks like the overall snapshot diff is still rather large because of the additional line from importing |
|
I would just move the new imports to the bottom too, with the new code. It's obviously not good Python style in general, but it works well for our tests :) Or we could split out a separate test file, that works too. |
|
It's a bit annoying, but I'm wondering if we need to preview-gate this. There aren't any new ecosystem hits, but it's still a pretty big expansion to the rule. I guess we should put it in preview, just to be safe. |
ntBre
left a comment
ntBre
left a comment
There was a problem hiding this comment.
Looks good, just one nit about the test location.
…_flake8_use_pathlib`
* main: (39 commits) [ty] Initial test suite for `TypedDict` (#19686) [ty] Improve debuggability of protocol types (#19662) [ty] Simplify lifetime requirements for `PySlice` trait (#19687) [ty] Improve `isinstance()` truthiness analysis for generic types (#19668) [`refurb`] Make example error out-of-the-box (`FURB164`) (#19673) Fix link: unused_import.rs (#19648) [ty] Remove `Specialization::display` (full) (#19682) [ty] Remove `KnownModule::is_enum` (#19681) [ty] Support `__setitem__` and improve `__getitem__` related diagnostics (#19578) [ty] Sync vendored typeshed stubs (#19676) [`flake8-use-pathlib`] Expand `PTH201` to check all `PurePath` subclasses (#19440) [`refurb`] Make example error out-of-the-box (`FURB180`) (#19672) [`pyupgrade`] Prevent infinite loop with `I002` (`UP010`, `UP035`) (#19413) [ty] Improve the `Display` for generic `type[]` types (#19667) [ty] Refactor `TypeInferenceBuilder::infer_subscript_expression_types` (#19658) Fix tests on 32-bit architectures (#19652) [ty] Move `pandas-stubs` to bad.txt (#19659) [ty] Remove special casing for string-literal-in-tuple `__contains__` (#19642) Update pre-commit's `ruff` id (#19654) Update salsa (#19449) ...
2 participants
Summary
Fixes #19437