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 24d9daee25..1ad7610fa7 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, @@ -59,7 +59,17 @@ pub(crate) fn check_os_pathlib_single_arg_calls( violation: impl Violation, applicability: Applicability, ) { - 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; } 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()