From ff4f114ced8c975707d7f54b1214336ce41653ac Mon Sep 17 00:00:00 2001 From: LaBatata101 Date: Tue, 29 Apr 2025 11:49:12 -0300 Subject: [PATCH 1/4] [`flake8-use-pathlib`] Fix `PTH104`false positive when `rename` is passed a file descriptor or bytes string --- .../fixtures/flake8_use_pathlib/full_name.py | 4 ++ .../rules/replaceable_by_pathlib.rs | 55 ++++++++++++++++++- .../src/analyze/typing.rs | 12 ++++ 3 files changed, 70 insertions(+), 1 deletion(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_use_pathlib/full_name.py b/crates/ruff_linter/resources/test/fixtures/flake8_use_pathlib/full_name.py index 20197b9703863..aba9e848f0107 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_use_pathlib/full_name.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_use_pathlib/full_name.py @@ -55,3 +55,7 @@ def opener(path, flags): open(x) def foo(y: int): open(y) + +# https://github.com/astral-sh/ruff/issues/17694 +os.rename("src", "dst", src_dir_fd=3, dst_dir_fd=4) +os.rename(b"src", b"dst", src_dir_fd=3, dst_dir_fd=4) 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 7d44916c89007..95217e946c93f 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 @@ -31,7 +31,35 @@ pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) { // PTH103 ["os", "mkdir"] => Some(OsMkdir.into()), // PTH104 - ["os", "rename"] => Some(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()) + || call + .arguments + .find_positional(0) + .is_some_and(|expr| is_bytes_string(expr, checker.semantic())) + || call + .arguments + .find_positional(1) + .is_some_and(|expr| is_bytes_string(expr, checker.semantic())) + { + return None; + } + Some(OsRename.into()) + } // PTH105 ["os", "replace"] => Some(OsReplace.into()), // PTH106 @@ -190,3 +218,28 @@ fn is_file_descriptor(expr: &Expr, semantic: &SemanticModel) -> bool { 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> { + match expr { + Expr::Name(name) => Some(name), + Expr::Call(ast::ExprCall { func, .. }) => get_name_expr(func), + _ => None, + } +} diff --git a/crates/ruff_python_semantic/src/analyze/typing.rs b/crates/ruff_python_semantic/src/analyze/typing.rs index 9b1f1c43f9a58..c588da272aff8 100644 --- a/crates/ruff_python_semantic/src/analyze/typing.rs +++ b/crates/ruff_python_semantic/src/analyze/typing.rs @@ -639,6 +639,18 @@ pub fn check_type(binding: &Binding, semantic: &SemanticModel) - _ => false, }, + BindingKind::FunctionDefinition(_) => match binding.statement(semantic) { + // ```python + // def foo() -> int: + // ... + // ``` + Some(Stmt::FunctionDef(ast::StmtFunctionDef { returns, .. })) => returns + .as_ref() + .is_some_and(|return_ann| T::match_annotation(return_ann, semantic)), + + _ => false, + }, + _ => false, } } From ce10a5b893930e74e3b30c8f3498ce8cc2871f62 Mon Sep 17 00:00:00 2001 From: LaBatata101 Date: Wed, 30 Apr 2025 11:15:08 -0300 Subject: [PATCH 2/4] Add more tests --- .../resources/test/fixtures/flake8_use_pathlib/full_name.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_use_pathlib/full_name.py b/crates/ruff_linter/resources/test/fixtures/flake8_use_pathlib/full_name.py index 4601a6efc1c24..476e5fdd9f3d2 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_use_pathlib/full_name.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_use_pathlib/full_name.py @@ -73,3 +73,6 @@ def bytes_str_func() -> bytes: # https://github.com/astral-sh/ruff/issues/17694 os.rename("src", "dst", src_dir_fd=3, dst_dir_fd=4) os.rename(b"src", b"dst", src_dir_fd=3, dst_dir_fd=4) +os.rename(b"src", b"dst", src_dir_fd=3) +os.rename(b"src", b"dst", dst_dir_fd=4) +os.rename(b"src", b"dst") From 2e0edb135f68214f17c9d310dc44488c4b9e9b93 Mon Sep 17 00:00:00 2001 From: LaBatata101 Date: Thu, 1 May 2025 10:26:16 -0300 Subject: [PATCH 3/4] Remove code for handling bytes strings --- .../test/fixtures/flake8_use_pathlib/full_name.py | 6 ++---- .../flake8_use_pathlib/rules/replaceable_by_pathlib.rs | 8 -------- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_use_pathlib/full_name.py b/crates/ruff_linter/resources/test/fixtures/flake8_use_pathlib/full_name.py index 4a717ca43306d..0f99bc4b85d03 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_use_pathlib/full_name.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_use_pathlib/full_name.py @@ -85,7 +85,5 @@ def bar(x: int): # https://github.com/astral-sh/ruff/issues/17694 os.rename("src", "dst", src_dir_fd=3, dst_dir_fd=4) -os.rename(b"src", b"dst", src_dir_fd=3, dst_dir_fd=4) -os.rename(b"src", b"dst", src_dir_fd=3) -os.rename(b"src", b"dst", dst_dir_fd=4) -os.rename(b"src", b"dst") +os.rename("src", "dst", src_dir_fd=3) +os.rename("src", "dst", dst_dir_fd=4) 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 5ada3e294fc71..b825b107e5384 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 @@ -48,14 +48,6 @@ pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) { .arguments .find_argument_value("dst_dir_fd", 3) .is_some_and(|expr| !expr.is_none_literal_expr()) - || call - .arguments - .find_positional(0) - .is_some_and(|expr| is_bytes_string(expr, checker.semantic())) - || call - .arguments - .find_positional(1) - .is_some_and(|expr| is_bytes_string(expr, checker.semantic())) { return; } From 5e32964cdccb946351adc8cbed6276b6d01a1d2d Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Thu, 1 May 2025 09:38:05 -0400 Subject: [PATCH 4/4] revert all byte string checks and update pth123 snapshots --- .../rules/replaceable_by_pathlib.rs | 23 +------------- ...ake8_use_pathlib__tests__full_name.py.snap | 30 +++++++++++++++++++ 2 files changed, 31 insertions(+), 22 deletions(-) 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 b825b107e5384..14b724730ffb3 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 @@ -155,7 +155,7 @@ pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) { || call .arguments .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; } @@ -194,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. fn is_file_descriptor(expr: &Expr, semantic: &SemanticModel) -> bool { if matches!( @@ -221,23 +217,6 @@ fn is_file_descriptor(expr: &Expr, semantic: &SemanticModel) -> bool { 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> { match expr { Expr::Name(name) => Some(name), diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__full_name.py.snap b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__full_name.py.snap index e0d29b06be0f9..63d515f0b3ac3 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__full_name.py.snap +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__full_name.py.snap @@ -317,3 +317,33 @@ full_name.py:47:1: PTH123 `open()` should be replaced by `Path.open()` | ^^^^ PTH123 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 + |