mirror of
https://github.com/astral-sh/ruff.git
synced 2025-07-24 05:25:17 +00:00
[flake8-use-pathlib] Implement os-sep-split (PTH206) (#5936)
Implements https://github.com/astral-sh/ruff/issues/5905#issuecomment-1644822548 --------- Co-authored-by: konsti <konstin@mailbox.org>
This commit is contained in:
Simon Brugman
GitHub
parent
057faabcdd
commit
f886b58c92
8 changed files with 228 additions and 0 deletions
20
crates/ruff/resources/test/fixtures/flake8_use_pathlib/PTH206.py
vendored
Normal file
20
crates/ruff/resources/test/fixtures/flake8_use_pathlib/PTH206.py
vendored
Normal file
|
|
@ -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("/")
|
||||
|
|
@ -3037,6 +3037,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);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -756,6 +756,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),
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ 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::*;
|
||||
|
||||
|
|
@ -9,5 +10,6 @@ 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;
|
||||
|
|
|
|||
|
|
@ -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()));
|
||||
}
|
||||
|
|
@ -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
|
||||
|
|
||||
|
||||
|
||||
1
ruff.schema.json
generated
1
ruff.schema.json
generated
|
|
@ -2333,6 +2333,7 @@
|
|||
"PTH203",
|
||||
"PTH204",
|
||||
"PTH205",
|
||||
"PTH206",
|
||||
"PYI",
|
||||
"PYI0",
|
||||
"PYI00",
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue