[flake8-use-pathlib] Implement os-sep-split (PTH206)

crates/ruff/resources/test/fixtures/flake8_use_pathlib/PTH206.py

@@ -0,0 +1,20 @@
+import os
+from os import sep
+
+
+file_name = "foo/bar"
+
+# PTH206
+"foo/bar/".split(os.sep)
+"foo/bar/".split(sep=os.sep)
+"foo/bar/".split(os.sep)[-1]
+"foo/bar/".split(os.sep)[-2]
+"foo/bar/".split(os.sep)[-2:]
+"fizz/buzz".split(sep)
+"fizz/buzz".split(sep)[-1]
+os.path.splitext("path/to/hello_world.py")[0].split(os.sep)[-1]
+file_name.split(os.sep)
+(os.path.abspath(file_name)).split(os.sep)
+
+# OK
+"foo/bar/".split("/")

crates/ruff/src/checkers/ast/mod.rs

@@ -3010,6 +3010,9 @@ where
                 if self.enabled(Rule::PathConstructorCurrentDirectory) {
                     flake8_use_pathlib::rules::path_constructor_current_directory(self, expr, func);
                 }
+                if self.enabled(Rule::OsSepSplit) {
+                    flake8_use_pathlib::rules::os_sep_split(self, func, args, keywords);
+                }
                 if self.enabled(Rule::NumpyLegacyRandom) {
                     numpy::rules::legacy_random(self, func);
                 }

crates/ruff/src/codes.rs

@@ -755,6 +755,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Flake8UsePathlib, "203") => (RuleGroup::Unspecified, rules::flake8_use_pathlib::rules::OsPathGetatime),
         (Flake8UsePathlib, "204") => (RuleGroup::Unspecified, rules::flake8_use_pathlib::rules::OsPathGetmtime),
         (Flake8UsePathlib, "205") => (RuleGroup::Unspecified, rules::flake8_use_pathlib::rules::OsPathGetctime),
+        (Flake8UsePathlib, "206") => (RuleGroup::Unspecified, rules::flake8_use_pathlib::rules::OsSepSplit),
 
         // flake8-logging-format
         (Flake8LoggingFormat, "001") => (RuleGroup::Unspecified, rules::flake8_logging_format::violations::LoggingStringFormat),

crates/ruff/src/rules/flake8_use_pathlib/mod.rs

@@ -61,6 +61,7 @@ mod tests {
     #[test_case(Rule::OsPathGetatime, Path::new("PTH203.py"))]
     #[test_case(Rule::OsPathGetmtime, Path::new("PTH204.py"))]
     #[test_case(Rule::OsPathGetctime, Path::new("PTH205.py"))]
+    #[test_case(Rule::OsSepSplit, Path::new("PTH206.py"))]
     fn rules_pypath(rule_code: Rule, path: &Path) -> Result<()> {
         let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
         let diagnostics = test_path(

crates/ruff/src/rules/flake8_use_pathlib/rules/mod.rs

@@ -2,12 +2,14 @@ pub(crate) use os_path_getatime::*;
 pub(crate) use os_path_getctime::*;
 pub(crate) use os_path_getmtime::*;
 pub(crate) use os_path_getsize::*;
+pub(crate) use os_sep_split::*;
 pub(crate) use path_constructor_current_directory::*;
 pub(crate) use replaceable_by_pathlib::*;
 
 mod os_path_getatime;
 mod os_path_getctime;
 mod os_path_getmtime;
 mod os_path_getsize;
+mod os_sep_split;
 mod path_constructor_current_directory;
 mod replaceable_by_pathlib;

crates/ruff/src/rules/flake8_use_pathlib/rules/os_sep_split.rs

@@ -0,0 +1,98 @@
+use ruff_diagnostics::{Diagnostic, Violation};
+use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::helpers::find_keyword;
+use rustpython_parser::ast::{Expr, ExprAttribute, Keyword, Ranged};
+
+use crate::checkers::ast::Checker;
+
+/// ## What it does
+/// Checks for uses of `.split(os.sep)`
+///
+/// ## Why is this bad?
+/// The `pathlib` module in the standard library should be used for path
+/// manipulation. It provides a high-level API with the functionality
+/// needed for common operations on `Path` objects.
+///
+/// ## Example
+/// If not all parts of the path are needed, then the `name` and `parent`
+/// attributes of the `Path` object should be used. Otherwise, the `parts`
+/// attribute can be used as shown in the last example.
+/// ```python
+/// import os
+///
+/// "path/to/file_name.txt".split(os.sep)[-1]
+///
+/// "path/to/file_name.txt".split(os.sep)[-2]
+///
+/// # Iterating over the path parts
+/// if any(part in blocklist for part in "my/file/path".split(os.sep)):
+///     ...
+/// ```
+///
+/// Use instead:
+/// ```python
+/// from pathlib import Path
+///
+/// Path("path/to/file_name.txt").name
+///
+/// Path("path/to/file_name.txt").parent.name
+///
+/// # Iterating over the path parts
+/// if any(part in blocklist for part in Path("my/file/path").parts):
+///     ...
+/// ```
+#[violation]
+pub struct OsSepSplit;
+
+impl Violation for OsSepSplit {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        format!("Replace `.split(os.sep)` with `Path.parts`")
+    }
+}
+
+/// PTH206
+pub(crate) fn os_sep_split(
+    checker: &mut Checker,
+    func: &Expr,
+    args: &[Expr],
+    keywords: &[Keyword],
+) {
+    let Expr::Attribute(ExprAttribute { attr, .. }) = func else {
+        return;
+    };
+
+    if attr.as_str() != "split" {
+        return;
+    };
+
+    let sep = if !args.is_empty() {
+        // `.split(os.sep)`
+        let [arg] = args else {
+            return;
+        };
+        arg
+    } else if !keywords.is_empty() {
+        // `.split(sep=os.sep)`
+        let Some(keyword) = find_keyword(keywords, "sep") else {
+            return;
+        };
+        &keyword.value
+    } else {
+        return;
+    };
+
+    if !checker
+        .semantic()
+        .resolve_call_path(sep)
+        .map_or(false, |call_path| {
+            matches!(call_path.as_slice(), ["os", "sep"])
+        })
+    {
+        return;
+    }
+
+    checker
+        .diagnostics
+        .push(Diagnostic::new(OsSepSplit, attr.range()));
+}

crates/ruff/src/rules/flake8_use_pathlib/snapshots/ruff__rules__flake8_use_pathlib__tests__PTH206_PTH206.py.snap

@@ -0,0 +1,102 @@
+---
+source: crates/ruff/src/rules/flake8_use_pathlib/mod.rs
+---
+PTH206.py:8:12: PTH206 Replace `.split(os.sep)` with `Path.parts`
+   |
+ 7 | # PTH206
+ 8 | "foo/bar/".split(os.sep)
+   |            ^^^^^ PTH206
+ 9 | "foo/bar/".split(sep=os.sep)
+10 | "foo/bar/".split(os.sep)[-1]
+   |
+
+PTH206.py:9:12: PTH206 Replace `.split(os.sep)` with `Path.parts`
+   |
+ 7 | # PTH206
+ 8 | "foo/bar/".split(os.sep)
+ 9 | "foo/bar/".split(sep=os.sep)
+   |            ^^^^^ PTH206
+10 | "foo/bar/".split(os.sep)[-1]
+11 | "foo/bar/".split(os.sep)[-2]
+   |
+
+PTH206.py:10:12: PTH206 Replace `.split(os.sep)` with `Path.parts`
+   |
+ 8 | "foo/bar/".split(os.sep)
+ 9 | "foo/bar/".split(sep=os.sep)
+10 | "foo/bar/".split(os.sep)[-1]
+   |            ^^^^^ PTH206
+11 | "foo/bar/".split(os.sep)[-2]
+12 | "foo/bar/".split(os.sep)[-2:]
+   |
+
+PTH206.py:11:12: PTH206 Replace `.split(os.sep)` with `Path.parts`
+   |
+ 9 | "foo/bar/".split(sep=os.sep)
+10 | "foo/bar/".split(os.sep)[-1]
+11 | "foo/bar/".split(os.sep)[-2]
+   |            ^^^^^ PTH206
+12 | "foo/bar/".split(os.sep)[-2:]
+13 | "fizz/buzz".split(sep)
+   |
+
+PTH206.py:12:12: PTH206 Replace `.split(os.sep)` with `Path.parts`
+   |
+10 | "foo/bar/".split(os.sep)[-1]
+11 | "foo/bar/".split(os.sep)[-2]
+12 | "foo/bar/".split(os.sep)[-2:]
+   |            ^^^^^ PTH206
+13 | "fizz/buzz".split(sep)
+14 | "fizz/buzz".split(sep)[-1]
+   |
+
+PTH206.py:13:13: PTH206 Replace `.split(os.sep)` with `Path.parts`
+   |
+11 | "foo/bar/".split(os.sep)[-2]
+12 | "foo/bar/".split(os.sep)[-2:]
+13 | "fizz/buzz".split(sep)
+   |             ^^^^^ PTH206
+14 | "fizz/buzz".split(sep)[-1]
+15 | os.path.splitext("path/to/hello_world.py")[0].split(os.sep)[-1]
+   |
+
+PTH206.py:14:13: PTH206 Replace `.split(os.sep)` with `Path.parts`
+   |
+12 | "foo/bar/".split(os.sep)[-2:]
+13 | "fizz/buzz".split(sep)
+14 | "fizz/buzz".split(sep)[-1]
+   |             ^^^^^ PTH206
+15 | os.path.splitext("path/to/hello_world.py")[0].split(os.sep)[-1]
+16 | file_name.split(os.sep)
+   |
+
+PTH206.py:15:47: PTH206 Replace `.split(os.sep)` with `Path.parts`
+   |
+13 | "fizz/buzz".split(sep)
+14 | "fizz/buzz".split(sep)[-1]
+15 | os.path.splitext("path/to/hello_world.py")[0].split(os.sep)[-1]
+   |                                               ^^^^^ PTH206
+16 | file_name.split(os.sep)
+17 | (os.path.abspath(file_name)).split(os.sep)
+   |
+
+PTH206.py:16:11: PTH206 Replace `.split(os.sep)` with `Path.parts`
+   |
+14 | "fizz/buzz".split(sep)[-1]
+15 | os.path.splitext("path/to/hello_world.py")[0].split(os.sep)[-1]
+16 | file_name.split(os.sep)
+   |           ^^^^^ PTH206
+17 | (os.path.abspath(file_name)).split(os.sep)
+   |
+
+PTH206.py:17:30: PTH206 Replace `.split(os.sep)` with `Path.parts`
+   |
+15 | os.path.splitext("path/to/hello_world.py")[0].split(os.sep)[-1]
+16 | file_name.split(os.sep)
+17 | (os.path.abspath(file_name)).split(os.sep)
+   |                              ^^^^^ PTH206
+18 | 
+19 | # OK
+   |
+
+