Skip to content

Commit 3353d07

Browse files
LaBatata101dhruvmanilantBre
authored
[flake8-use-pathlib] Fix PTH104false positive when rename is passed a file descriptor (#17712)
## Summary Contains the same changes to the semantic type inference as #17705. Fixes #17694 <!-- What's the purpose of the change? What does it do, and why? --> ## Test Plan <!-- How was it tested? --> Snapshot tests. --------- Co-authored-by: Dhruv Manilawala <[email protected]> Co-authored-by: Brent Westbrook <[email protected]>
1 parent 41f3f21 commit 3353d07

File tree

3 files changed

+57
-23
lines changed

3 files changed

+57
-23
lines changed

crates/ruff_linter/resources/test/fixtures/flake8_use_pathlib/full_name.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,3 +82,8 @@ def func() -> int:
8282

8383
def bar(x: int):
8484
os.stat(x)
85+
86+
# https://github.com/astral-sh/ruff/issues/17694
87+
os.rename("src", "dst", src_dir_fd=3, dst_dir_fd=4)
88+
os.rename("src", "dst", src_dir_fd=3)
89+
os.rename("src", "dst", dst_dir_fd=4)

crates/ruff_linter/src/rules/flake8_use_pathlib/rules/replaceable_by_pathlib.rs

Lines changed: 22 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,27 @@ pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) {
3232
// PTH103
3333
["os", "mkdir"] => OsMkdir.into(),
3434
// PTH104
35-
["os", "rename"] => OsRename.into(),
35+
["os", "rename"] => {
36+
// `src_dir_fd` and `dst_dir_fd` are not supported by pathlib, so check if they are
37+
// are set to non-default values.
38+
// Signature as of Python 3.13 (https://docs.python.org/3/library/os.html#os.rename)
39+
// ```text
40+
// 0 1 2 3
41+
// os.rename(src, dst, *, src_dir_fd=None, dst_dir_fd=None)
42+
// ```
43+
if call
44+
.arguments
45+
.find_argument_value("src_dir_fd", 2)
46+
.is_some_and(|expr| !expr.is_none_literal_expr())
47+
|| call
48+
.arguments
49+
.find_argument_value("dst_dir_fd", 3)
50+
.is_some_and(|expr| !expr.is_none_literal_expr())
51+
{
52+
return;
53+
}
54+
OsRename.into()
55+
}
3656
// PTH105
3757
["os", "replace"] => OsReplace.into(),
3858
// PTH106
@@ -135,7 +155,7 @@ pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) {
135155
|| call
136156
.arguments
137157
.find_positional(0)
138-
.is_some_and(|expr| is_file_descriptor_or_bytes_str(expr, checker.semantic()))
158+
.is_some_and(|expr| is_file_descriptor(expr, checker.semantic()))
139159
{
140160
return;
141161
}
@@ -174,10 +194,6 @@ pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) {
174194
}
175195
}
176196

177-
fn is_file_descriptor_or_bytes_str(expr: &Expr, semantic: &SemanticModel) -> bool {
178-
is_file_descriptor(expr, semantic) || is_bytes_string(expr, semantic)
179-
}
180-
181197
/// Returns `true` if the given expression looks like a file descriptor, i.e., if it is an integer.
182198
fn is_file_descriptor(expr: &Expr, semantic: &SemanticModel) -> bool {
183199
if matches!(
@@ -201,23 +217,6 @@ fn is_file_descriptor(expr: &Expr, semantic: &SemanticModel) -> bool {
201217
typing::is_int(binding, semantic)
202218
}
203219

204-
/// Returns `true` if the given expression is a bytes string.
205-
fn is_bytes_string(expr: &Expr, semantic: &SemanticModel) -> bool {
206-
if matches!(expr, Expr::BytesLiteral(_)) {
207-
return true;
208-
}
209-
210-
let Some(name) = get_name_expr(expr) else {
211-
return false;
212-
};
213-
214-
let Some(binding) = semantic.only_binding(name).map(|id| semantic.binding(id)) else {
215-
return false;
216-
};
217-
218-
typing::is_bytes(binding, semantic)
219-
}
220-
221220
fn get_name_expr(expr: &Expr) -> Option<&ast::ExprName> {
222221
match expr {
223222
Expr::Name(name) => Some(name),

crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__full_name.py.snap

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,3 +317,33 @@ full_name.py:47:1: PTH123 `open()` should be replaced by `Path.open()`
317317
| ^^^^ PTH123
318318
48 | open(p, 'r', - 1, None, None, None, False, opener)
319319
|
320+
321+
full_name.py:65:1: PTH123 `open()` should be replaced by `Path.open()`
322+
|
323+
63 | open(f())
324+
64 |
325+
65 | open(b"foo")
326+
| ^^^^ PTH123
327+
66 | byte_str = b"bar"
328+
67 | open(byte_str)
329+
|
330+
331+
full_name.py:67:1: PTH123 `open()` should be replaced by `Path.open()`
332+
|
333+
65 | open(b"foo")
334+
66 | byte_str = b"bar"
335+
67 | open(byte_str)
336+
| ^^^^ PTH123
337+
68 |
338+
69 | def bytes_str_func() -> bytes:
339+
|
340+
341+
full_name.py:71:1: PTH123 `open()` should be replaced by `Path.open()`
342+
|
343+
69 | def bytes_str_func() -> bytes:
344+
70 | return b"foo"
345+
71 | open(bytes_str_func())
346+
| ^^^^ PTH123
347+
72 |
348+
73 | # https://github.com/astral-sh/ruff/issues/17693
349+
|

0 commit comments

Comments
 (0)