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 eb888f40f65ced..5d2fca01873dff 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 @@ -33,6 +33,8 @@ fp.read() open(p).close() os.getcwdb(p) +os.path.join(p, *q) +os.sep.join(p, *q) # https://github.com/astral-sh/ruff/issues/7620 def opener(path, flags): diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/replaceable_by_pathlib.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/replaceable_by_pathlib.rs index c4e9d97e3a125e..18d14b091f90c6 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/replaceable_by_pathlib.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/replaceable_by_pathlib.rs @@ -8,7 +8,7 @@ use crate::rules::flake8_use_pathlib::rules::{ Glob, OsPathGetatime, OsPathGetctime, OsPathGetmtime, OsPathGetsize, }; use crate::rules::flake8_use_pathlib::violations::{ - BuiltinOpen, OsChmod, OsGetcwd, OsMakedirs, OsMkdir, OsPathAbspath, OsPathBasename, + BuiltinOpen, Joiner, OsChmod, OsGetcwd, OsMakedirs, OsMkdir, OsPathAbspath, OsPathBasename, OsPathDirname, OsPathExists, OsPathExpanduser, OsPathIsabs, OsPathIsdir, OsPathIsfile, OsPathIslink, OsPathJoin, OsPathSamefile, OsPathSplitext, OsReadlink, OsRemove, OsRename, OsReplace, OsRmdir, OsStat, OsUnlink, PyPath, @@ -60,12 +60,22 @@ pub(crate) fn replaceable_by_pathlib(checker: &mut Checker, call: &ExprCall) { ["os", "path", "join"] => Some( OsPathJoin { module: "path".to_string(), + joiner: if call.arguments.args.iter().any(Expr::is_starred_expr) { + Joiner::Joinpath + } else { + Joiner::Slash + }, } .into(), ), ["os", "sep", "join"] => Some( OsPathJoin { module: "sep".to_string(), + joiner: if call.arguments.args.iter().any(Expr::is_starred_expr) { + Joiner::Joinpath + } else { + Joiner::Slash + }, } .into(), ), 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 d42e76a7d3e6ef..92dd4a47e4b2e2 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 @@ -267,6 +267,7 @@ full_name.py:34:1: PTH123 `open()` should be replaced by `Path.open()` 34 | open(p).close() | ^^^^ PTH123 35 | os.getcwdb(p) +36 | os.path.join(p, *q) | full_name.py:35:1: PTH109 `os.getcwd()` should be replaced by `Path.cwd()` @@ -275,27 +276,46 @@ full_name.py:35:1: PTH109 `os.getcwd()` should be replaced by `Path.cwd()` 34 | open(p).close() 35 | os.getcwdb(p) | ^^^^^^^^^^ PTH109 -36 | -37 | # https://github.com/astral-sh/ruff/issues/7620 +36 | os.path.join(p, *q) +37 | os.sep.join(p, *q) | -full_name.py:44:1: PTH123 `open()` should be replaced by `Path.open()` +full_name.py:36:1: PTH118 `os.path.join()` should be replaced by `Path.joinpath()` | -42 | open(p, closefd=False) -43 | open(p, opener=opener) -44 | open(p, mode='r', buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None) +34 | open(p).close() +35 | os.getcwdb(p) +36 | os.path.join(p, *q) + | ^^^^^^^^^^^^ PTH118 +37 | os.sep.join(p, *q) + | + +full_name.py:37:1: PTH118 `os.sep.join()` should be replaced by `Path.joinpath()` + | +35 | os.getcwdb(p) +36 | os.path.join(p, *q) +37 | os.sep.join(p, *q) + | ^^^^^^^^^^^ PTH118 +38 | +39 | # https://github.com/astral-sh/ruff/issues/7620 + | + +full_name.py:46:1: PTH123 `open()` should be replaced by `Path.open()` + | +44 | open(p, closefd=False) +45 | open(p, opener=opener) +46 | open(p, mode='r', buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None) | ^^^^ PTH123 -45 | open(p, 'r', - 1, None, None, None, True, None) -46 | open(p, 'r', - 1, None, None, None, False, opener) +47 | open(p, 'r', - 1, None, None, None, True, None) +48 | open(p, 'r', - 1, None, None, None, False, opener) | -full_name.py:45:1: PTH123 `open()` should be replaced by `Path.open()` +full_name.py:47:1: PTH123 `open()` should be replaced by `Path.open()` | -43 | open(p, opener=opener) -44 | open(p, mode='r', buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None) -45 | open(p, 'r', - 1, None, None, None, True, None) +45 | open(p, opener=opener) +46 | open(p, mode='r', buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None) +47 | open(p, 'r', - 1, None, None, None, True, None) | ^^^^ PTH123 -46 | open(p, 'r', - 1, None, None, None, False, opener) +48 | open(p, 'r', - 1, None, None, None, False, opener) | diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/violations.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/violations.rs index b53250c2f757d1..7e18f8290ed4f7 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/violations.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/violations.rs @@ -799,8 +799,8 @@ impl Violation for OsPathIsabs { /// ## Why is this bad? /// `pathlib` offers a high-level API for path manipulation, as compared to /// the lower-level API offered by `os`. When possible, using `Path` object -/// methods such as `Path.joinpath()` can improve readability over the `os` -/// module's counterparts (e.g., `os.path.join()`). +/// methods such as `Path.joinpath()` or the `/` operator can improve +/// readability over the `os` module's counterparts (e.g., `os.path.join()`). /// /// Note that `os` functions may be preferable if performance is a concern, /// e.g., in hot loops. @@ -828,17 +828,31 @@ impl Violation for OsPathIsabs { /// - [No really, pathlib is great](https://treyhunner.com/2019/01/no-really-pathlib-is-great/) #[violation] pub struct OsPathJoin { - pub module: String, + pub(crate) module: String, + pub(crate) joiner: Joiner, } impl Violation for OsPathJoin { #[derive_message_formats] fn message(&self) -> String { - let OsPathJoin { module } = self; - format!("`os.{module}.join()` should be replaced by `Path` with `/` operator") + let OsPathJoin { module, joiner } = self; + match joiner { + Joiner::Slash => { + format!("`os.{module}.join()` should be replaced by `Path` with `/` operator") + } + Joiner::Joinpath => { + format!("`os.{module}.join()` should be replaced by `Path.joinpath()`") + } + } } } +#[derive(Debug, PartialEq, Eq)] +pub(crate) enum Joiner { + Slash, + Joinpath, +} + /// ## What it does /// Checks for uses of `os.path.basename`. ///