From f7358ca20c37d0d5f3ccc1758fae9bfbad801135 Mon Sep 17 00:00:00 2001 From: LaBatata101 Date: Tue, 29 Apr 2025 10:50:31 -0300 Subject: [PATCH 1/3] [`flake8-use-pathlib`] Fix `PTH123` false positive when `open` is passed a file descriptor from a function call --- .../test/fixtures/flake8_use_pathlib/full_name.py | 5 +++++ .../rules/replaceable_by_pathlib.rs | 10 +++++++++- crates/ruff_python_semantic/src/analyze/typing.rs | 12 ++++++++++++ 3 files changed, 26 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 20197b9703863f..83523105e0ac43 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,8 @@ def opener(path, flags): open(x) def foo(y: int): open(y) + +# https://github.com/astral-sh/ruff/issues/17691 +def f() -> int: + return 1 +open(f()) 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 7d44916c890073..75f4432aaf8b81 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 @@ -180,7 +180,7 @@ fn is_file_descriptor(expr: &Expr, semantic: &SemanticModel) -> bool { return true; } - let Some(name) = expr.as_name_expr() else { + let Some(name) = get_name_expr(expr) else { return false; }; @@ -190,3 +190,11 @@ fn is_file_descriptor(expr: &Expr, semantic: &SemanticModel) -> bool { typing::is_int(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 9b1f1c43f9a589..c588da272aff83 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 3737d78b4b98969dbe3dedc8bee1101f92915bf2 Mon Sep 17 00:00:00 2001 From: LaBatata101 Date: Tue, 29 Apr 2025 11:18:20 -0300 Subject: [PATCH 2/3] Handle bytes string --- .../rules/replaceable_by_pathlib.rs | 28 ++++++++++++++++--- 1 file changed, 24 insertions(+), 4 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 75f4432aaf8b81..8704959a85184a 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 @@ -126,10 +126,9 @@ pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) { .arguments .find_argument_value("opener", 7) .is_some_and(|expr| !expr.is_none_literal_expr()) - || call - .arguments - .find_positional(0) - .is_some_and(|expr| is_file_descriptor(expr, checker.semantic())) + || call.arguments.find_positional(0).is_some_and(|expr| { + is_file_descriptor_or_bytes_str(expr, checker.semantic()) + }) { return None; } @@ -168,6 +167,10 @@ 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!( @@ -191,6 +194,23 @@ 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), From d795cdbc6c5811b7d260d52febda66cd59cdc730 Mon Sep 17 00:00:00 2001 From: LaBatata101 Date: Tue, 29 Apr 2025 17:42:47 -0300 Subject: [PATCH 3/3] Add bytes string test cases --- .../test/fixtures/flake8_use_pathlib/full_name.py | 8 ++++++++ 1 file changed, 8 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 83523105e0ac43..8680212b20c3aa 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 @@ -60,3 +60,11 @@ def foo(y: int): def f() -> int: return 1 open(f()) + +open(b"foo") +byte_str = b"bar" +open(byte_str) + +def bytes_str_func() -> bytes: + return b"foo" +open(bytes_str_func())