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()