diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_use_pathlib/PTH208.py b/crates/ruff_linter/resources/test/fixtures/flake8_use_pathlib/PTH208.py new file mode 100644 index 0000000000000..8a31b2b472e05 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/flake8_use_pathlib/PTH208.py @@ -0,0 +1,23 @@ +import os + +os.listdir('.') +os.listdir(b'.') + +string_path = '.' +os.listdir(string_path) + +bytes_path = b'.' +os.listdir(bytes_path) + + +from pathlib import Path + +path_path = Path('.') +os.listdir(path_path) + + +if os.listdir("dir"): + ... + +if "file" in os.listdir("dir"): + ... diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index 0c2f4e235c087..6ca710fd8f17b 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -965,6 +965,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { Rule::OsPathGetmtime, Rule::OsPathGetctime, Rule::Glob, + Rule::OsListdir, ]) { flake8_use_pathlib::rules::replaceable_by_pathlib(checker, call); } diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 1ce84c6a2b63d..d4a9ec964139b 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -908,6 +908,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Flake8UsePathlib, "205") => (RuleGroup::Stable, rules::flake8_use_pathlib::rules::OsPathGetctime), (Flake8UsePathlib, "206") => (RuleGroup::Stable, rules::flake8_use_pathlib::rules::OsSepSplit), (Flake8UsePathlib, "207") => (RuleGroup::Stable, rules::flake8_use_pathlib::rules::Glob), + (Flake8UsePathlib, "208") => (RuleGroup::Preview, rules::flake8_use_pathlib::violations::OsListdir), // flake8-logging-format (Flake8LoggingFormat, "001") => (RuleGroup::Stable, rules::flake8_logging_format::violations::LoggingStringFormat), diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/mod.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/mod.rs index 01b1ae356800e..5b678c8823e8c 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/mod.rs @@ -63,6 +63,7 @@ mod tests { #[test_case(Rule::OsPathGetctime, Path::new("PTH205.py"))] #[test_case(Rule::OsSepSplit, Path::new("PTH206.py"))] #[test_case(Rule::Glob, Path::new("PTH207.py"))] + #[test_case(Rule::OsListdir, Path::new("PTH208.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( 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 aa1db51279f61..fd97c1fe3b8ac 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 @@ -10,10 +10,10 @@ use crate::rules::flake8_use_pathlib::rules::{ Glob, OsPathGetatime, OsPathGetctime, OsPathGetmtime, OsPathGetsize, }; use crate::rules::flake8_use_pathlib::violations::{ - 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, + BuiltinOpen, Joiner, OsChmod, OsGetcwd, OsListdir, OsMakedirs, OsMkdir, OsPathAbspath, + OsPathBasename, OsPathDirname, OsPathExists, OsPathExpanduser, OsPathIsabs, OsPathIsdir, + OsPathIsfile, OsPathIslink, OsPathJoin, OsPathSamefile, OsPathSplitext, OsReadlink, OsRemove, + OsRename, OsReplace, OsRmdir, OsStat, OsUnlink, PyPath, }; use crate::settings::types::PythonVersion; @@ -155,6 +155,8 @@ pub(crate) fn replaceable_by_pathlib(checker: &mut Checker, call: &ExprCall) { ["os", "readlink"] if checker.settings.target_version >= PythonVersion::Py39 => { Some(OsReadlink.into()) } + // PTH208, + ["os", "listdir"] => Some(OsListdir.into()), _ => None, }) { diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__PTH208_PTH208.py.snap b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__PTH208_PTH208.py.snap new file mode 100644 index 0000000000000..27d5009df016d --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__PTH208_PTH208.py.snap @@ -0,0 +1,60 @@ +--- +source: crates/ruff_linter/src/rules/flake8_use_pathlib/mod.rs +snapshot_kind: text +--- +PTH208.py:3:1: PTH208 Use `pathlib.Path.iterdir()` instead. + | +1 | import os +2 | +3 | os.listdir('.') + | ^^^^^^^^^^ PTH208 +4 | os.listdir(b'.') + | + +PTH208.py:4:1: PTH208 Use `pathlib.Path.iterdir()` instead. + | +3 | os.listdir('.') +4 | os.listdir(b'.') + | ^^^^^^^^^^ PTH208 +5 | +6 | string_path = '.' + | + +PTH208.py:7:1: PTH208 Use `pathlib.Path.iterdir()` instead. + | +6 | string_path = '.' +7 | os.listdir(string_path) + | ^^^^^^^^^^ PTH208 +8 | +9 | bytes_path = b'.' + | + +PTH208.py:10:1: PTH208 Use `pathlib.Path.iterdir()` instead. + | + 9 | bytes_path = b'.' +10 | os.listdir(bytes_path) + | ^^^^^^^^^^ PTH208 + | + +PTH208.py:16:1: PTH208 Use `pathlib.Path.iterdir()` instead. + | +15 | path_path = Path('.') +16 | os.listdir(path_path) + | ^^^^^^^^^^ PTH208 + | + +PTH208.py:19:4: PTH208 Use `pathlib.Path.iterdir()` instead. + | +19 | if os.listdir("dir"): + | ^^^^^^^^^^ PTH208 +20 | ... + | + +PTH208.py:22:14: PTH208 Use `pathlib.Path.iterdir()` instead. + | +20 | ... +21 | +22 | if "file" in os.listdir("dir"): + | ^^^^^^^^^^ PTH208 +23 | ... + | 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 01e88ea89b85c..12dd0044765b6 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/violations.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/violations.rs @@ -1103,3 +1103,48 @@ impl Violation for PyPath { "`py.path` is in maintenance mode, use `pathlib` instead".to_string() } } + +/// ## What it does +/// Checks for uses of `os.listdir`. +/// +/// ## 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 `pathlib`'s +/// `Path.iterdir()` can improve readability over `os.listdir()`. +/// +/// ## Example +/// +/// ```python +/// p = "." +/// for d in os.listdir(p): +/// ... +/// +/// if os.listdir(p): +/// ... +/// +/// if "file" in os.listdir(p): +/// ... +/// ``` +/// +/// Use instead: +/// +/// ```python +/// p = Path(".") +/// for d in p.iterdir(): +/// ... +/// +/// if any(p.iterdir()): +/// ... +/// +/// if (p / "file").exists(): +/// ... +/// ``` +#[derive(ViolationMetadata)] +pub(crate) struct OsListdir; + +impl Violation for OsListdir { + #[derive_message_formats] + fn message(&self) -> String { + "Use `pathlib.Path.iterdir()` instead.".to_string() + } +} diff --git a/ruff.schema.json b/ruff.schema.json index fee16aaec8c57..2ba04db4e103a 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3711,6 +3711,7 @@ "PTH205", "PTH206", "PTH207", + "PTH208", "PYI", "PYI0", "PYI00",