Show custom message for `Path.joinpath` with starred arguments (#7852)

Closes https://github.com/astral-sh/ruff/issues/7833.

crates/ruff_linter/resources/test/fixtures/flake8_use_pathlib/full_name.py

@@ -33,6 +33,8 @@ with open(p) as fp:
     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):

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

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 | 
+36 | os.path.join(p, *q)
-37 | # https://github.com/astral-sh/ruff/issues/7620
+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)
+34 | open(p).close()
-43 | open(p, opener=opener)
+35 | os.getcwdb(p)
-44 | open(p, mode='r', buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None)
+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)
+47 | open(p, 'r', - 1, None, None, None, True, None)
-46 | open(p, 'r', - 1, None, None, None, False, opener)
+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)
+45 | open(p, opener=opener)
-44 | open(p, mode='r', buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None)
+46 | 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)
+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)
    |
 
 

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`
+/// methods such as `Path.joinpath()` or the `/` operator can improve
-/// module's counterparts (e.g., `os.path.join()`).
+/// 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,15 +828,29 @@ 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;
+        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