mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-27 12:29:28 +00:00
[flake8-use-pathlib
] Fix PTH104
false positive when rename
is passed a file descriptor (#17712)
## Summary Contains the same changes to the semantic type inference as https://github.com/astral-sh/ruff/pull/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 <dhruvmanila@gmail.com> Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>
This commit is contained in:
parent
41f3f21629
commit
3353d07938
3 changed files with 57 additions and 23 deletions
|
@ -82,3 +82,8 @@ os.stat(func())
|
||||||
|
|
||||||
def bar(x: int):
|
def bar(x: int):
|
||||||
os.stat(x)
|
os.stat(x)
|
||||||
|
|
||||||
|
# https://github.com/astral-sh/ruff/issues/17694
|
||||||
|
os.rename("src", "dst", src_dir_fd=3, dst_dir_fd=4)
|
||||||
|
os.rename("src", "dst", src_dir_fd=3)
|
||||||
|
os.rename("src", "dst", dst_dir_fd=4)
|
||||||
|
|
|
@ -32,7 +32,27 @@ pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) {
|
||||||
// PTH103
|
// PTH103
|
||||||
["os", "mkdir"] => OsMkdir.into(),
|
["os", "mkdir"] => OsMkdir.into(),
|
||||||
// PTH104
|
// PTH104
|
||||||
["os", "rename"] => OsRename.into(),
|
["os", "rename"] => {
|
||||||
|
// `src_dir_fd` and `dst_dir_fd` are not supported by pathlib, so check if they are
|
||||||
|
// are set to non-default values.
|
||||||
|
// Signature as of Python 3.13 (https://docs.python.org/3/library/os.html#os.rename)
|
||||||
|
// ```text
|
||||||
|
// 0 1 2 3
|
||||||
|
// os.rename(src, dst, *, src_dir_fd=None, dst_dir_fd=None)
|
||||||
|
// ```
|
||||||
|
if call
|
||||||
|
.arguments
|
||||||
|
.find_argument_value("src_dir_fd", 2)
|
||||||
|
.is_some_and(|expr| !expr.is_none_literal_expr())
|
||||||
|
|| call
|
||||||
|
.arguments
|
||||||
|
.find_argument_value("dst_dir_fd", 3)
|
||||||
|
.is_some_and(|expr| !expr.is_none_literal_expr())
|
||||||
|
{
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
OsRename.into()
|
||||||
|
}
|
||||||
// PTH105
|
// PTH105
|
||||||
["os", "replace"] => OsReplace.into(),
|
["os", "replace"] => OsReplace.into(),
|
||||||
// PTH106
|
// PTH106
|
||||||
|
@ -135,7 +155,7 @@ pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) {
|
||||||
|| call
|
|| call
|
||||||
.arguments
|
.arguments
|
||||||
.find_positional(0)
|
.find_positional(0)
|
||||||
.is_some_and(|expr| is_file_descriptor_or_bytes_str(expr, checker.semantic()))
|
.is_some_and(|expr| is_file_descriptor(expr, checker.semantic()))
|
||||||
{
|
{
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
@ -174,10 +194,6 @@ pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
fn is_file_descriptor_or_bytes_str(expr: &Expr, semantic: &SemanticModel) -> bool {
|
|
||||||
is_file_descriptor(expr, semantic) || is_bytes_string(expr, semantic)
|
|
||||||
}
|
|
||||||
|
|
||||||
/// Returns `true` if the given expression looks like a file descriptor, i.e., if it is an integer.
|
/// Returns `true` if the given expression looks like a file descriptor, i.e., if it is an integer.
|
||||||
fn is_file_descriptor(expr: &Expr, semantic: &SemanticModel) -> bool {
|
fn is_file_descriptor(expr: &Expr, semantic: &SemanticModel) -> bool {
|
||||||
if matches!(
|
if matches!(
|
||||||
|
@ -201,23 +217,6 @@ fn is_file_descriptor(expr: &Expr, semantic: &SemanticModel) -> bool {
|
||||||
typing::is_int(binding, semantic)
|
typing::is_int(binding, semantic)
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Returns `true` if the given expression is a bytes string.
|
|
||||||
fn is_bytes_string(expr: &Expr, semantic: &SemanticModel) -> bool {
|
|
||||||
if matches!(expr, Expr::BytesLiteral(_)) {
|
|
||||||
return true;
|
|
||||||
}
|
|
||||||
|
|
||||||
let Some(name) = get_name_expr(expr) else {
|
|
||||||
return false;
|
|
||||||
};
|
|
||||||
|
|
||||||
let Some(binding) = semantic.only_binding(name).map(|id| semantic.binding(id)) else {
|
|
||||||
return false;
|
|
||||||
};
|
|
||||||
|
|
||||||
typing::is_bytes(binding, semantic)
|
|
||||||
}
|
|
||||||
|
|
||||||
fn get_name_expr(expr: &Expr) -> Option<&ast::ExprName> {
|
fn get_name_expr(expr: &Expr) -> Option<&ast::ExprName> {
|
||||||
match expr {
|
match expr {
|
||||||
Expr::Name(name) => Some(name),
|
Expr::Name(name) => Some(name),
|
||||||
|
|
|
@ -317,3 +317,33 @@ full_name.py:47:1: PTH123 `open()` should be replaced by `Path.open()`
|
||||||
| ^^^^ PTH123
|
| ^^^^ PTH123
|
||||||
48 | open(p, 'r', - 1, None, None, None, False, opener)
|
48 | open(p, 'r', - 1, None, None, None, False, opener)
|
||||||
|
|
|
|
||||||
|
|
||||||
|
full_name.py:65:1: PTH123 `open()` should be replaced by `Path.open()`
|
||||||
|
|
|
||||||
|
63 | open(f())
|
||||||
|
64 |
|
||||||
|
65 | open(b"foo")
|
||||||
|
| ^^^^ PTH123
|
||||||
|
66 | byte_str = b"bar"
|
||||||
|
67 | open(byte_str)
|
||||||
|
|
|
||||||
|
|
||||||
|
full_name.py:67:1: PTH123 `open()` should be replaced by `Path.open()`
|
||||||
|
|
|
||||||
|
65 | open(b"foo")
|
||||||
|
66 | byte_str = b"bar"
|
||||||
|
67 | open(byte_str)
|
||||||
|
| ^^^^ PTH123
|
||||||
|
68 |
|
||||||
|
69 | def bytes_str_func() -> bytes:
|
||||||
|
|
|
||||||
|
|
||||||
|
full_name.py:71:1: PTH123 `open()` should be replaced by `Path.open()`
|
||||||
|
|
|
||||||
|
69 | def bytes_str_func() -> bytes:
|
||||||
|
70 | return b"foo"
|
||||||
|
71 | open(bytes_str_func())
|
||||||
|
| ^^^^ PTH123
|
||||||
|
72 |
|
||||||
|
73 | # https://github.com/astral-sh/ruff/issues/17693
|
||||||
|
|
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue