diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF071.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF071.py new file mode 100644 index 0000000000000..17d5b465797f4 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF071.py @@ -0,0 +1,23 @@ +import os +import os.path +from os.path import commonprefix +from os import path + +# Errors +os.path.commonprefix(["/usr/lib", "/usr/local/lib"]) +commonprefix(["/usr/lib", "/usr/local/lib"]) +path.commonprefix(["/usr/lib", "/usr/local/lib"]) + +# OK +os.path.commonpath(["/usr/lib", "/usr/local/lib"]) + +# Not a call — bare reference is fine +x = os.path.commonprefix + + +# User-defined function — no error +def commonprefix(paths): + return paths[0] + + +commonprefix(["/usr/lib", "/usr/local/lib"]) diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index e856ce0c6f531..4af0438baea47 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -1162,6 +1162,9 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) { checker, call, segments, ); } + if checker.is_rule_enabled(Rule::OsPathCommonprefix) { + ruff::rules::os_path_commonprefix(checker, call, segments); + } } if checker.is_rule_enabled(Rule::OsSepSplit) { diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 4b0cfba291847..d1145258b11a7 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -1066,6 +1066,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Ruff, "068") => rules::ruff::rules::DuplicateEntryInDunderAll, (Ruff, "069") => rules::ruff::rules::FloatEqualityComparison, (Ruff, "070") => rules::ruff::rules::UnnecessaryAssignBeforeYield, + (Ruff, "071") => rules::ruff::rules::OsPathCommonprefix, (Ruff, "100") => rules::ruff::rules::UnusedNOQA, (Ruff, "101") => rules::ruff::rules::RedirectedNOQA, diff --git a/crates/ruff_linter/src/rules/ruff/mod.rs b/crates/ruff_linter/src/rules/ruff/mod.rs index e62f967acd92a..f323c6960574e 100644 --- a/crates/ruff_linter/src/rules/ruff/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/mod.rs @@ -654,6 +654,7 @@ mod tests { #[test_case(Rule::ImplicitClassVarInDataclass, Path::new("RUF045.py"))] #[test_case(Rule::FloatEqualityComparison, Path::new("RUF069.py"))] #[test_case(Rule::UnnecessaryAssignBeforeYield, Path::new("RUF070.py"))] + #[test_case(Rule::OsPathCommonprefix, Path::new("RUF071.py"))] fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!( "preview__{}_{}", diff --git a/crates/ruff_linter/src/rules/ruff/rules/mod.rs b/crates/ruff_linter/src/rules/ruff/rules/mod.rs index 06cde1ce691ef..92ae2cf7c6073 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/mod.rs @@ -37,6 +37,7 @@ pub(crate) use never_union::*; pub(crate) use non_empty_init_module::*; pub(crate) use non_octal_permissions::*; pub(crate) use none_not_at_end_of_union::*; +pub(crate) use os_path_commonprefix::*; pub(crate) use parenthesize_chained_operators::*; pub(crate) use post_init_default::*; pub(crate) use property_without_return::*; @@ -108,6 +109,7 @@ mod never_union; mod non_empty_init_module; mod non_octal_permissions; mod none_not_at_end_of_union; +mod os_path_commonprefix; mod parenthesize_chained_operators; mod post_init_default; mod property_without_return; diff --git a/crates/ruff_linter/src/rules/ruff/rules/os_path_commonprefix.rs b/crates/ruff_linter/src/rules/ruff/rules/os_path_commonprefix.rs new file mode 100644 index 0000000000000..2084acce10d57 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/rules/os_path_commonprefix.rs @@ -0,0 +1,64 @@ +use ruff_macros::{ViolationMetadata, derive_message_formats}; +use ruff_python_ast as ast; +use ruff_text_size::Ranged; + +use crate::Violation; +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for uses of `os.path.commonprefix`. +/// +/// ## Why is this bad? +/// `os.path.commonprefix` performs a character-by-character string +/// comparison rather than comparing path components. This leads to +/// incorrect results when paths share a common string prefix that +/// is not a valid path component. +/// +/// `os.path.commonpath` correctly compares path components. +/// +/// `os.path.commonprefix` is deprecated as of Python 3.15. +/// +/// ## Example +/// ```python +/// import os +/// +/// # Returns "/usr/l" — not a valid directory! +/// os.path.commonprefix(["/usr/lib", "/usr/local/lib"]) +/// ``` +/// +/// Use instead: +/// ```python +/// import os +/// +/// # Returns "/usr" — correct common path +/// os.path.commonpath(["/usr/lib", "/usr/local/lib"]) +/// ``` +/// +/// ## References +/// - [Python documentation: `os.path.commonprefix`](https://docs.python.org/3/library/os.path.html#os.path.commonprefix) +/// - [Python documentation: `os.path.commonpath`](https://docs.python.org/3/library/os.path.html#os.path.commonpath) +/// - [Why `os.path.commonprefix` is deprecated](https://sethmlarson.dev/deprecate-confusing-apis-like-os-path-commonprefix) +/// - [CPython deprecation issue](https://github.com/python/cpython/issues/144347) +#[derive(ViolationMetadata)] +#[violation_metadata(preview_since = "NEXT_RUFF_VERSION")] +pub(crate) struct OsPathCommonprefix; + +impl Violation for OsPathCommonprefix { + #[derive_message_formats] + fn message(&self) -> String { + "`os.path.commonprefix()` compares strings character-by-character".to_string() + } + + fn fix_title(&self) -> Option { + Some("Use `os.path.commonpath()` to compare path components".to_string()) + } +} + +/// RUF071 +pub(crate) fn os_path_commonprefix(checker: &Checker, call: &ast::ExprCall, segments: &[&str]) { + if segments != ["os", "path", "commonprefix"] { + return; + } + let mut diagnostic = checker.report_diagnostic(OsPathCommonprefix, call.func.range()); + diagnostic.add_primary_tag(ruff_db::diagnostic::DiagnosticTag::Deprecated); +} diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF071_RUF071.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF071_RUF071.py.snap new file mode 100644 index 0000000000000..7a06011a6e43e --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF071_RUF071.py.snap @@ -0,0 +1,36 @@ +--- +source: crates/ruff_linter/src/rules/ruff/mod.rs +--- +RUF071 `os.path.commonprefix()` compares strings character-by-character + --> RUF071.py:7:1 + | +6 | # Errors +7 | os.path.commonprefix(["/usr/lib", "/usr/local/lib"]) + | ^^^^^^^^^^^^^^^^^^^^ +8 | commonprefix(["/usr/lib", "/usr/local/lib"]) +9 | path.commonprefix(["/usr/lib", "/usr/local/lib"]) + | +help: Use `os.path.commonpath()` to compare path components + +RUF071 `os.path.commonprefix()` compares strings character-by-character + --> RUF071.py:8:1 + | +6 | # Errors +7 | os.path.commonprefix(["/usr/lib", "/usr/local/lib"]) +8 | commonprefix(["/usr/lib", "/usr/local/lib"]) + | ^^^^^^^^^^^^ +9 | path.commonprefix(["/usr/lib", "/usr/local/lib"]) + | +help: Use `os.path.commonpath()` to compare path components + +RUF071 `os.path.commonprefix()` compares strings character-by-character + --> RUF071.py:9:1 + | + 7 | os.path.commonprefix(["/usr/lib", "/usr/local/lib"]) + 8 | commonprefix(["/usr/lib", "/usr/local/lib"]) + 9 | path.commonprefix(["/usr/lib", "/usr/local/lib"]) + | ^^^^^^^^^^^^^^^^^ +10 | +11 | # OK + | +help: Use `os.path.commonpath()` to compare path components diff --git a/ruff.schema.json b/ruff.schema.json index 2ed8873a2f522..218807e2b9142 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -4143,6 +4143,7 @@ "RUF069", "RUF07", "RUF070", + "RUF071", "RUF1", "RUF10", "RUF100",