Skip to content

Commit e4dfddc

Browse files
[ruff] Add os-path-commonprefix (RUF071) (#23814)
<!-- Thank you for contributing to Ruff/ty! To help us out with reviewing, please consider the following: - Does this pull request include a summary of the change? (See below.) - Does this pull request include a descriptive title? (Please prefix with `[ty]` for ty pull requests.) - Does this pull request include references to any relevant issues? --> ## Summary Adds a new rule os-path-commonprefix (RUF071) that detects calls to os.path.commonprefix(), which performs character-by-character string comparison instead of path-component comparison — a well-known footgun. os.path.commonpath() is the correct alternative. ## Test Plan cargo nextest run -p ruff_linter -- rule_ospathcommonprefix Closes #22981 --------- Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>
1 parent 074d32e commit e4dfddc

8 files changed

Lines changed: 131 additions & 0 deletions

File tree

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
import os
2+
import os.path
3+
from os.path import commonprefix
4+
from os import path
5+
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"])
10+
11+
# OK
12+
os.path.commonpath(["/usr/lib", "/usr/local/lib"])
13+
14+
# Not a call — bare reference is fine
15+
x = os.path.commonprefix
16+
17+
18+
# User-defined function — no error
19+
def commonprefix(paths):
20+
return paths[0]
21+
22+
23+
commonprefix(["/usr/lib", "/usr/local/lib"])

crates/ruff_linter/src/checkers/ast/analyze/expression.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1162,6 +1162,9 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
11621162
checker, call, segments,
11631163
);
11641164
}
1165+
if checker.is_rule_enabled(Rule::OsPathCommonprefix) {
1166+
ruff::rules::os_path_commonprefix(checker, call, segments);
1167+
}
11651168
}
11661169

11671170
if checker.is_rule_enabled(Rule::OsSepSplit) {

crates/ruff_linter/src/codes.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1066,6 +1066,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
10661066
(Ruff, "068") => rules::ruff::rules::DuplicateEntryInDunderAll,
10671067
(Ruff, "069") => rules::ruff::rules::FloatEqualityComparison,
10681068
(Ruff, "070") => rules::ruff::rules::UnnecessaryAssignBeforeYield,
1069+
(Ruff, "071") => rules::ruff::rules::OsPathCommonprefix,
10691070

10701071
(Ruff, "100") => rules::ruff::rules::UnusedNOQA,
10711072
(Ruff, "101") => rules::ruff::rules::RedirectedNOQA,

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -654,6 +654,7 @@ mod tests {
654654
#[test_case(Rule::ImplicitClassVarInDataclass, Path::new("RUF045.py"))]
655655
#[test_case(Rule::FloatEqualityComparison, Path::new("RUF069.py"))]
656656
#[test_case(Rule::UnnecessaryAssignBeforeYield, Path::new("RUF070.py"))]
657+
#[test_case(Rule::OsPathCommonprefix, Path::new("RUF071.py"))]
657658
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
658659
let snapshot = format!(
659660
"preview__{}_{}",

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ pub(crate) use never_union::*;
3737
pub(crate) use non_empty_init_module::*;
3838
pub(crate) use non_octal_permissions::*;
3939
pub(crate) use none_not_at_end_of_union::*;
40+
pub(crate) use os_path_commonprefix::*;
4041
pub(crate) use parenthesize_chained_operators::*;
4142
pub(crate) use post_init_default::*;
4243
pub(crate) use property_without_return::*;
@@ -108,6 +109,7 @@ mod never_union;
108109
mod non_empty_init_module;
109110
mod non_octal_permissions;
110111
mod none_not_at_end_of_union;
112+
mod os_path_commonprefix;
111113
mod parenthesize_chained_operators;
112114
mod post_init_default;
113115
mod property_without_return;
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
use ruff_macros::{ViolationMetadata, derive_message_formats};
2+
use ruff_python_ast as ast;
3+
use ruff_text_size::Ranged;
4+
5+
use crate::Violation;
6+
use crate::checkers::ast::Checker;
7+
8+
/// ## What it does
9+
/// Checks for uses of `os.path.commonprefix`.
10+
///
11+
/// ## Why is this bad?
12+
/// `os.path.commonprefix` performs a character-by-character string
13+
/// comparison rather than comparing path components. This leads to
14+
/// incorrect results when paths share a common string prefix that
15+
/// is not a valid path component.
16+
///
17+
/// `os.path.commonpath` correctly compares path components.
18+
///
19+
/// `os.path.commonprefix` is deprecated as of Python 3.15.
20+
///
21+
/// ## Example
22+
/// ```python
23+
/// import os
24+
///
25+
/// # Returns "/usr/l" — not a valid directory!
26+
/// os.path.commonprefix(["/usr/lib", "/usr/local/lib"])
27+
/// ```
28+
///
29+
/// Use instead:
30+
/// ```python
31+
/// import os
32+
///
33+
/// # Returns "/usr" — correct common path
34+
/// os.path.commonpath(["/usr/lib", "/usr/local/lib"])
35+
/// ```
36+
///
37+
/// ## References
38+
/// - [Python documentation: `os.path.commonprefix`](https://docs.python.org/3/library/os.path.html#os.path.commonprefix)
39+
/// - [Python documentation: `os.path.commonpath`](https://docs.python.org/3/library/os.path.html#os.path.commonpath)
40+
/// - [Why `os.path.commonprefix` is deprecated](https://sethmlarson.dev/deprecate-confusing-apis-like-os-path-commonprefix)
41+
/// - [CPython deprecation issue](https://github.com/python/cpython/issues/144347)
42+
#[derive(ViolationMetadata)]
43+
#[violation_metadata(preview_since = "NEXT_RUFF_VERSION")]
44+
pub(crate) struct OsPathCommonprefix;
45+
46+
impl Violation for OsPathCommonprefix {
47+
#[derive_message_formats]
48+
fn message(&self) -> String {
49+
"`os.path.commonprefix()` compares strings character-by-character".to_string()
50+
}
51+
52+
fn fix_title(&self) -> Option<String> {
53+
Some("Use `os.path.commonpath()` to compare path components".to_string())
54+
}
55+
}
56+
57+
/// RUF071
58+
pub(crate) fn os_path_commonprefix(checker: &Checker, call: &ast::ExprCall, segments: &[&str]) {
59+
if segments != ["os", "path", "commonprefix"] {
60+
return;
61+
}
62+
let mut diagnostic = checker.report_diagnostic(OsPathCommonprefix, call.func.range());
63+
diagnostic.add_primary_tag(ruff_db::diagnostic::DiagnosticTag::Deprecated);
64+
}
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
---
2+
source: crates/ruff_linter/src/rules/ruff/mod.rs
3+
---
4+
RUF071 `os.path.commonprefix()` compares strings character-by-character
5+
--> RUF071.py:7:1
6+
|
7+
6 | # Errors
8+
7 | os.path.commonprefix(["/usr/lib", "/usr/local/lib"])
9+
| ^^^^^^^^^^^^^^^^^^^^
10+
8 | commonprefix(["/usr/lib", "/usr/local/lib"])
11+
9 | path.commonprefix(["/usr/lib", "/usr/local/lib"])
12+
|
13+
help: Use `os.path.commonpath()` to compare path components
14+
15+
RUF071 `os.path.commonprefix()` compares strings character-by-character
16+
--> RUF071.py:8:1
17+
|
18+
6 | # Errors
19+
7 | os.path.commonprefix(["/usr/lib", "/usr/local/lib"])
20+
8 | commonprefix(["/usr/lib", "/usr/local/lib"])
21+
| ^^^^^^^^^^^^
22+
9 | path.commonprefix(["/usr/lib", "/usr/local/lib"])
23+
|
24+
help: Use `os.path.commonpath()` to compare path components
25+
26+
RUF071 `os.path.commonprefix()` compares strings character-by-character
27+
--> RUF071.py:9:1
28+
|
29+
7 | os.path.commonprefix(["/usr/lib", "/usr/local/lib"])
30+
8 | commonprefix(["/usr/lib", "/usr/local/lib"])
31+
9 | path.commonprefix(["/usr/lib", "/usr/local/lib"])
32+
| ^^^^^^^^^^^^^^^^^
33+
10 |
34+
11 | # OK
35+
|
36+
help: Use `os.path.commonpath()` to compare path components

ruff.schema.json

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)