From 75b2fec98e43f86b8426176259acd78396caa8af Mon Sep 17 00:00:00 2001 From: Dan Date: Mon, 10 Nov 2025 18:52:28 -0500 Subject: [PATCH 1/4] fix-21342 --- .../fixtures/flake8_use_pathlib/full_name.py | 9 +- .../src/rules/flake8_use_pathlib/helpers.rs | 28 ++++- ...ake8_use_pathlib__tests__full_name.py.snap | 47 ++++++++ ..._pathlib__tests__preview_full_name.py.snap | 101 ++++++++++++++++++ 4 files changed, 183 insertions(+), 2 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_use_pathlib/full_name.py b/crates/ruff_linter/resources/test/fixtures/flake8_use_pathlib/full_name.py index 0ed75fb834..bbf27b162f 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_use_pathlib/full_name.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_use_pathlib/full_name.py @@ -136,4 +136,11 @@ os.chmod("pth1_file", 0o700, None, True, 1, *[1], **{"x": 1}, foo=1) os.rename("pth1_file", "pth1_file1", None, None, 1, *[1], **{"x": 1}, foo=1) os.replace("pth1_file1", "pth1_file", None, None, 1, *[1], **{"x": 1}, foo=1) -os.path.samefile("pth1_file", "pth1_link", 1, *[1], **{"x": 1}, foo=1) \ No newline at end of file +os.path.samefile("pth1_file", "pth1_link", 1, *[1], **{"x": 1}, foo=1) + +# https://github.com/astral-sh/ruff/issues/21342 +# if `dir_fd=None` is explicitly set, should still trigger the diagnostic +os.readlink(p, dir_fd=None) +os.unlink(p, dir_fd=None) +os.remove(p, dir_fd=None) +os.rmdir(p, dir_fd=None) \ No newline at end of file diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs index 3ba017ecb7..682d2c6bd4 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs @@ -59,7 +59,33 @@ pub(crate) fn check_os_pathlib_single_arg_calls( violation: impl Violation, applicability: Option, ) { - if call.arguments.len() != 1 { + // Check that we have exactly 1 positional argument OR the argument is passed as a keyword + // This allows: func(arg), func(name=arg), and func(arg, dir_fd=None) + let has_positional_arg = call.arguments.args.len() == 1; + let has_keyword_arg = call.arguments.find_keyword(fn_argument).is_some(); + + if !has_positional_arg && !has_keyword_arg { + return; + } + + // If we have both positional and keyword for the main argument, that's invalid + if has_positional_arg && has_keyword_arg { + return; + } + + // If `dir_fd` is set to a non-default value, skip (pathlib doesn't support it) + if is_keyword_only_argument_non_default(&call.arguments, "dir_fd") { + return; + } + + // If there are keyword arguments other than `dir_fd` or the main argument, skip + // We need to allow the main argument to be passed as a keyword, and `dir_fd=None` + let allowed_keywords = if has_keyword_arg { + &[fn_argument, "dir_fd"][..] + } else { + &["dir_fd"][..] + }; + if has_unknown_keywords_or_starred_expr(&call.arguments, allowed_keywords) { return; } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__full_name.py.snap b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__full_name.py.snap index f8bdfa55ad..363da32682 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__full_name.py.snap +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__full_name.py.snap @@ -567,5 +567,52 @@ PTH121 `os.path.samefile()` should be replaced by `Path.samefile()` 138 | 139 | os.path.samefile("pth1_file", "pth1_link", 1, *[1], **{"x": 1}, foo=1) | ^^^^^^^^^^^^^^^^ +140 | +141 | # https://github.com/astral-sh/ruff/issues/21342 | help: Replace with `Path(...).samefile()` + +PTH115 `os.readlink()` should be replaced by `Path.readlink()` + --> full_name.py:143:1 + | +141 | # https://github.com/astral-sh/ruff/issues/21342 +142 | # if `dir_fd=None` is explicitly set, should still trigger the diagnostic +143 | os.readlink(p, dir_fd=None) + | ^^^^^^^^^^^ +144 | os.unlink(p, dir_fd=None) +145 | os.remove(p, dir_fd=None) + | +help: Replace with `Path(...).readlink()` + +PTH108 `os.unlink()` should be replaced by `Path.unlink()` + --> full_name.py:144:1 + | +142 | # if `dir_fd=None` is explicitly set, should still trigger the diagnostic +143 | os.readlink(p, dir_fd=None) +144 | os.unlink(p, dir_fd=None) + | ^^^^^^^^^ +145 | os.remove(p, dir_fd=None) +146 | os.rmdir(p, dir_fd=None) + | +help: Replace with `Path(...).unlink()` + +PTH107 `os.remove()` should be replaced by `Path.unlink()` + --> full_name.py:145:1 + | +143 | os.readlink(p, dir_fd=None) +144 | os.unlink(p, dir_fd=None) +145 | os.remove(p, dir_fd=None) + | ^^^^^^^^^ +146 | os.rmdir(p, dir_fd=None) + | +help: Replace with `Path(...).unlink()` + +PTH106 `os.rmdir()` should be replaced by `Path.rmdir()` + --> full_name.py:146:1 + | +144 | os.unlink(p, dir_fd=None) +145 | os.remove(p, dir_fd=None) +146 | os.rmdir(p, dir_fd=None) + | ^^^^^^^^ + | +help: Replace with `Path(...).rmdir()` diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_full_name.py.snap b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_full_name.py.snap index 7292a3b5d2..5ed3ecbb65 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_full_name.py.snap +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_full_name.py.snap @@ -1037,5 +1037,106 @@ PTH121 `os.path.samefile()` should be replaced by `Path.samefile()` 138 | 139 | os.path.samefile("pth1_file", "pth1_link", 1, *[1], **{"x": 1}, foo=1) | ^^^^^^^^^^^^^^^^ +140 | +141 | # https://github.com/astral-sh/ruff/issues/21342 | help: Replace with `Path(...).samefile()` + +PTH115 [*] `os.readlink()` should be replaced by `Path.readlink()` + --> full_name.py:143:1 + | +141 | # https://github.com/astral-sh/ruff/issues/21342 +142 | # if `dir_fd=None` is explicitly set, should still trigger the diagnostic +143 | os.readlink(p, dir_fd=None) + | ^^^^^^^^^^^ +144 | os.unlink(p, dir_fd=None) +145 | os.remove(p, dir_fd=None) + | +help: Replace with `Path(...).readlink()` +1 | import os +2 | import os.path +3 + import pathlib +4 | +5 | p = "/foo" +6 | q = "bar" +-------------------------------------------------------------------------------- +141 | +142 | # https://github.com/astral-sh/ruff/issues/21342 +143 | # if `dir_fd=None` is explicitly set, should still trigger the diagnostic + - os.readlink(p, dir_fd=None) +144 + pathlib.Path(p).readlink() +145 | os.unlink(p, dir_fd=None) +146 | os.remove(p, dir_fd=None) +147 | os.rmdir(p, dir_fd=None) + +PTH108 [*] `os.unlink()` should be replaced by `Path.unlink()` + --> full_name.py:144:1 + | +142 | # if `dir_fd=None` is explicitly set, should still trigger the diagnostic +143 | os.readlink(p, dir_fd=None) +144 | os.unlink(p, dir_fd=None) + | ^^^^^^^^^ +145 | os.remove(p, dir_fd=None) +146 | os.rmdir(p, dir_fd=None) + | +help: Replace with `Path(...).unlink()` +1 | import os +2 | import os.path +3 + import pathlib +4 | +5 | p = "/foo" +6 | q = "bar" +-------------------------------------------------------------------------------- +142 | # https://github.com/astral-sh/ruff/issues/21342 +143 | # if `dir_fd=None` is explicitly set, should still trigger the diagnostic +144 | os.readlink(p, dir_fd=None) + - os.unlink(p, dir_fd=None) +145 + pathlib.Path(p).unlink() +146 | os.remove(p, dir_fd=None) +147 | os.rmdir(p, dir_fd=None) + +PTH107 [*] `os.remove()` should be replaced by `Path.unlink()` + --> full_name.py:145:1 + | +143 | os.readlink(p, dir_fd=None) +144 | os.unlink(p, dir_fd=None) +145 | os.remove(p, dir_fd=None) + | ^^^^^^^^^ +146 | os.rmdir(p, dir_fd=None) + | +help: Replace with `Path(...).unlink()` +1 | import os +2 | import os.path +3 + import pathlib +4 | +5 | p = "/foo" +6 | q = "bar" +-------------------------------------------------------------------------------- +143 | # if `dir_fd=None` is explicitly set, should still trigger the diagnostic +144 | os.readlink(p, dir_fd=None) +145 | os.unlink(p, dir_fd=None) + - os.remove(p, dir_fd=None) +146 + pathlib.Path(p).unlink() +147 | os.rmdir(p, dir_fd=None) + +PTH106 [*] `os.rmdir()` should be replaced by `Path.rmdir()` + --> full_name.py:146:1 + | +144 | os.unlink(p, dir_fd=None) +145 | os.remove(p, dir_fd=None) +146 | os.rmdir(p, dir_fd=None) + | ^^^^^^^^ + | +help: Replace with `Path(...).rmdir()` +1 | import os +2 | import os.path +3 + import pathlib +4 | +5 | p = "/foo" +6 | q = "bar" +-------------------------------------------------------------------------------- +144 | os.readlink(p, dir_fd=None) +145 | os.unlink(p, dir_fd=None) +146 | os.remove(p, dir_fd=None) + - os.rmdir(p, dir_fd=None) +147 + pathlib.Path(p).rmdir() From e066e9ba246c9046f0749325f56d83084a236811 Mon Sep 17 00:00:00 2001 From: Dan Date: Mon, 10 Nov 2025 19:19:09 -0500 Subject: [PATCH 2/4] cargo fmt --- crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs index 682d2c6bd4..8541c3fb30 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs @@ -63,11 +63,11 @@ pub(crate) fn check_os_pathlib_single_arg_calls( // This allows: func(arg), func(name=arg), and func(arg, dir_fd=None) let has_positional_arg = call.arguments.args.len() == 1; let has_keyword_arg = call.arguments.find_keyword(fn_argument).is_some(); - + if !has_positional_arg && !has_keyword_arg { return; } - + // If we have both positional and keyword for the main argument, that's invalid if has_positional_arg && has_keyword_arg { return; From 74266724b4be47fdc64dc96ae0904b0699c21ea3 Mon Sep 17 00:00:00 2001 From: Dan Date: Tue, 11 Nov 2025 16:41:25 -0500 Subject: [PATCH 3/4] Remove redundant `dir_fd` check from helper --- .../src/rules/flake8_use_pathlib/helpers.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs index 8541c3fb30..8b472c9625 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs @@ -47,9 +47,9 @@ pub(crate) fn is_pure_path_subclass_with_preview(checker: &Checker, segments: &[ false } -/// We check functions that take only 1 argument, this does not apply to functions -/// with `dir_fd` argument, because `dir_fd` is not supported by pathlib, -/// so check if it's set to non-default values +/// We check functions that take only 1 argument. +/// The caller is responsible for checking that `dir_fd` is not set to a non-default value +/// (since `dir_fd` is not supported by pathlib), but this helper allows `dir_fd=None` to pass through. pub(crate) fn check_os_pathlib_single_arg_calls( checker: &Checker, call: &ExprCall, @@ -73,13 +73,9 @@ pub(crate) fn check_os_pathlib_single_arg_calls( return; } - // If `dir_fd` is set to a non-default value, skip (pathlib doesn't support it) - if is_keyword_only_argument_non_default(&call.arguments, "dir_fd") { - return; - } - // If there are keyword arguments other than `dir_fd` or the main argument, skip // We need to allow the main argument to be passed as a keyword, and `dir_fd=None` + // Note: `dir_fd` non-default value checking is done by the caller before invoking this helper let allowed_keywords = if has_keyword_arg { &[fn_argument, "dir_fd"][..] } else { From df296fbac1795054cb3b14c4f728c1360298ed92 Mon Sep 17 00:00:00 2001 From: Dan Date: Wed, 19 Nov 2025 19:02:46 -0500 Subject: [PATCH 4/4] Remove unnecessary keyword argument checks --- .../src/rules/flake8_use_pathlib/helpers.rs | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs index 8b472c9625..490af45e2f 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs @@ -73,18 +73,6 @@ pub(crate) fn check_os_pathlib_single_arg_calls( return; } - // If there are keyword arguments other than `dir_fd` or the main argument, skip - // We need to allow the main argument to be passed as a keyword, and `dir_fd=None` - // Note: `dir_fd` non-default value checking is done by the caller before invoking this helper - let allowed_keywords = if has_keyword_arg { - &[fn_argument, "dir_fd"][..] - } else { - &["dir_fd"][..] - }; - if has_unknown_keywords_or_starred_expr(&call.arguments, allowed_keywords) { - return; - } - let Some(arg) = call.arguments.find_argument_value(fn_argument, 0) else { return; };