From bd82aa256e0776e788ec99e7d0f4f7e6d3281139 Mon Sep 17 00:00:00 2001 From: Dan Date: Thu, 13 Nov 2025 17:18:36 -0500 Subject: [PATCH 1/2] fix-21431 --- .../src/rules/flake8_use_pathlib/helpers.rs | 41 +++++++++++++++++-- .../flake8_use_pathlib/rules/os_getcwd.rs | 10 ++++- .../rules/os_path_abspath.rs | 4 ++ .../rules/os_path_dirname.rs | 4 ++ .../rules/os_path_expanduser.rs | 4 ++ .../flake8_use_pathlib/rules/os_readlink.rs | 3 ++ .../flake8_use_pathlib/rules/os_rename.rs | 3 ++ .../flake8_use_pathlib/rules/os_replace.rs | 3 ++ ..._pathlib__tests__preview_full_name.py.snap | 4 ++ ..._pathlib__tests__preview_import_as.py.snap | 4 ++ ...athlib__tests__preview_import_from.py.snap | 4 ++ ...lib__tests__preview_import_from_as.py.snap | 4 ++ 12 files changed, 83 insertions(+), 5 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs index 3ba017ecb7..036ced3bfe 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs @@ -1,4 +1,4 @@ -use ruff_python_ast::{self as ast, Arguments, Expr, ExprCall}; +use ruff_python_ast::{self as ast, Arguments, Expr, ExprCall, Stmt}; use ruff_python_semantic::{SemanticModel, analyze::typing}; use ruff_text_size::Ranged; @@ -94,12 +94,22 @@ pub(crate) fn check_os_pathlib_single_arg_calls( let fix = match applicability { Some(Applicability::Unsafe) => Fix::unsafe_edits(edit, [import_edit]), _ => { - let applicability = if checker.comment_ranges().intersects(range) { + let determined_applicability = if checker.comment_ranges().intersects(range) { Applicability::Unsafe + } else if applicability.is_none() { + // When applicability is None, we need to determine it based on return type changes + if is_top_level_expression_call(checker, call) { + // Safe when the call is a top-level expression (return value not used) + Applicability::Safe + } else { + // Unsafe because the return type changes (str/bytes -> Path or None -> Path) + Applicability::Unsafe + } } else { + // applicability is Some(Applicability::Safe), use it Applicability::Safe }; - Fix::applicable_edits(edit, [import_edit], applicability) + Fix::applicable_edits(edit, [import_edit], determined_applicability) } }; @@ -176,8 +186,12 @@ pub(crate) fn check_os_pathlib_two_arg_calls( let applicability = if checker.comment_ranges().intersects(range) { Applicability::Unsafe - } else { + } else if is_top_level_expression_call(checker, call) { + // Safe when the call is a top-level expression (return value not used) Applicability::Safe + } else { + // Unsafe because the return type changes (None -> Path) + Applicability::Unsafe }; Ok(Fix::applicable_edits( @@ -209,3 +223,22 @@ pub(crate) fn is_argument_non_default(arguments: &Arguments, name: &str, positio .find_argument_value(name, position) .is_some_and(|expr| !expr.is_none_literal_expr()) } + +/// Returns `true` if the given call is a top-level expression in its statement. +/// This means the call's return value is not used, so return type changes don't matter. +pub(crate) fn is_top_level_expression_call(checker: &Checker, call: &ExprCall) -> bool { + if let Stmt::Expr(ast::StmtExpr { + value: child, + range: _, + node_index: _, + }) = checker.semantic().current_statement() + { + // Check if the call is the same expression as the statement's value + // We compare by checking if the call's range is contained within the child's range + // and if they're the same expression node + if let Expr::Call(call_expr) = child.as_ref() { + return call_expr.range() == call.range(); + } + } + false +} diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_getcwd.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_getcwd.rs index 7bb533246d..d0bfcc47d8 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_getcwd.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_getcwd.rs @@ -1,6 +1,7 @@ use crate::checkers::ast::Checker; use crate::importer::ImportRequest; use crate::preview::is_fix_os_getcwd_enabled; +use crate::rules::flake8_use_pathlib::helpers::is_top_level_expression_call; use crate::{FixAvailability, Violation}; use ruff_diagnostics::{Applicability, Edit, Fix}; use ruff_macros::{ViolationMetadata, derive_message_formats}; @@ -37,6 +38,9 @@ use ruff_text_size::Ranged; /// /// ## Fix Safety /// This rule's fix is marked as unsafe if the replacement would remove comments attached to the original expression. +/// Additionally, the fix is marked as unsafe because `os.getcwd()` and `os.getcwdb()` return `str` or `bytes`, +/// while `Path.cwd()` returns a `Path` object. This change in return type can break code that uses the return value. +/// The fix is safe when the function call is a top-level expression in its statement (i.e., the return value is not used). /// /// ## References /// - [Python documentation: `Path.cwd`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.cwd) @@ -85,8 +89,12 @@ pub(crate) fn os_getcwd(checker: &Checker, call: &ExprCall, segments: &[&str]) { let applicability = if checker.comment_ranges().intersects(range) { Applicability::Unsafe - } else { + } else if is_top_level_expression_call(checker, call) { + // Safe when the call is a top-level expression (return value not used) Applicability::Safe + } else { + // Unsafe because the return type changes (str/bytes -> Path) + Applicability::Unsafe }; let replacement = format!("{binding}.cwd()"); diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_abspath.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_abspath.rs index 9b58561e88..667da0d54a 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_abspath.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_abspath.rs @@ -45,6 +45,10 @@ use crate::{FixAvailability, Violation}; /// behaviors is required, there's no existing `pathlib` alternative. See CPython issue /// [#69200](https://github.com/python/cpython/issues/69200). /// +/// Additionally, the fix is marked as unsafe because `os.path.abspath()` returns a `str`, while +/// `Path.resolve()` returns a `Path` object. This change in return type can break code that uses +/// the return value. +/// /// ## References /// - [Python documentation: `Path.resolve`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.resolve) /// - [Python documentation: `os.path.abspath`](https://docs.python.org/3/library/os.path.html#os.path.abspath) diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_dirname.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_dirname.rs index d3175c2035..9860b54e69 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_dirname.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_dirname.rs @@ -42,6 +42,10 @@ use crate::{FixAvailability, Violation}; /// As a result, code relying on the exact string returned by `os.path.dirname` /// may behave differently after the fix. /// +/// Additionally, the fix is marked as unsafe because `os.path.dirname()` returns a `str`, while +/// `Path.parent` returns a `Path` object. This change in return type can break code that uses +/// the return value. +/// /// ## Known issues /// While using `pathlib` can improve the readability and type safety of your code, /// it can be less performant than the lower-level alternatives that work directly with strings, diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_expanduser.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_expanduser.rs index d544acde39..4a6accfb93 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_expanduser.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_expanduser.rs @@ -41,6 +41,10 @@ use crate::{FixAvailability, Violation}; /// directory can't be resolved: `os.path.expanduser` returns the /// input unchanged, while `Path.expanduser` raises `RuntimeError`. /// +/// Additionally, the fix is marked as unsafe because `os.path.expanduser()` returns a `str`, while +/// `Path.expanduser()` returns a `Path` object. This change in return type can break code that uses +/// the return value. +/// /// ## References /// - [Python documentation: `Path.expanduser`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.expanduser) /// - [Python documentation: `os.path.expanduser`](https://docs.python.org/3/library/os.path.html#os.path.expanduser) diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_readlink.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_readlink.rs index d1df572ed5..c5471223e3 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_readlink.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_readlink.rs @@ -38,6 +38,9 @@ use crate::{FixAvailability, Violation}; /// /// ## Fix Safety /// This rule's fix is marked as unsafe if the replacement would remove comments attached to the original expression. +/// Additionally, the fix is marked as unsafe because `os.readlink()` returns a `str`, while `Path.readlink()` returns a `Path` object. +/// This change in return type can break code that uses the return value. +/// The fix is safe when the function call is a top-level expression in its statement (i.e., the return value is not used). /// /// ## References /// - [Python documentation: `Path.readlink`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.readline) diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_rename.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_rename.rs index c5f2293ee9..f3f9caf041 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_rename.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_rename.rs @@ -38,6 +38,9 @@ use ruff_python_ast::ExprCall; /// /// ## Fix Safety /// This rule's fix is marked as unsafe if the replacement would remove comments attached to the original expression. +/// Additionally, the fix is marked as unsafe because `os.rename()` returns `None`, while `Path.rename()` returns a `Path` object. +/// This change in return type can break code that uses the return value. +/// The fix is safe when the function call is a top-level expression in its statement (i.e., the return value is not used). /// /// ## References /// - [Python documentation: `Path.rename`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.rename) diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_replace.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_replace.rs index ef60099467..a7d6db6f23 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_replace.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_replace.rs @@ -41,6 +41,9 @@ use ruff_python_ast::ExprCall; /// /// ## Fix Safety /// This rule's fix is marked as unsafe if the replacement would remove comments attached to the original expression. +/// Additionally, the fix is marked as unsafe because `os.replace()` returns `None`, while `Path.replace()` returns a `Path` object. +/// This change in return type can break code that uses the return value. +/// The fix is safe when the function call is a top-level expression in its statement (i.e., the return value is not used). /// /// ## References /// - [Python documentation: `Path.replace`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.replace) diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_full_name.py.snap b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_full_name.py.snap index 7292a3b5d2..9711df1a18 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_full_name.py.snap +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_full_name.py.snap @@ -233,6 +233,7 @@ help: Replace with `Path(...).exists()` 19 | bb = os.path.expanduser(p) 20 | bbb = os.path.isdir(p) 21 | bbbb = os.path.isfile(p) +note: This is an unsafe fix and may change runtime behavior PTH111 [*] `os.path.expanduser()` should be replaced by `Path.expanduser()` --> full_name.py:18:6 @@ -288,6 +289,7 @@ help: Replace with `Path(...).is_dir()` 21 | bbbb = os.path.isfile(p) 22 | bbbbb = os.path.islink(p) 23 | os.readlink(p) +note: This is an unsafe fix and may change runtime behavior PTH113 [*] `os.path.isfile()` should be replaced by `Path.is_file()` --> full_name.py:20:8 @@ -315,6 +317,7 @@ help: Replace with `Path(...).is_file()` 22 | bbbbb = os.path.islink(p) 23 | os.readlink(p) 24 | os.stat(p) +note: This is an unsafe fix and may change runtime behavior PTH114 [*] `os.path.islink()` should be replaced by `Path.is_symlink()` --> full_name.py:21:9 @@ -342,6 +345,7 @@ help: Replace with `Path(...).is_symlink()` 23 | os.readlink(p) 24 | os.stat(p) 25 | os.path.isabs(p) +note: This is an unsafe fix and may change runtime behavior PTH115 [*] `os.readlink()` should be replaced by `Path.readlink()` --> full_name.py:22:1 diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_as.py.snap b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_as.py.snap index de63cbde9d..e7590393ff 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_as.py.snap +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_as.py.snap @@ -233,6 +233,7 @@ help: Replace with `Path(...).exists()` 19 | bb = foo_p.expanduser(p) 20 | bbb = foo_p.isdir(p) 21 | bbbb = foo_p.isfile(p) +note: This is an unsafe fix and may change runtime behavior PTH111 [*] `os.path.expanduser()` should be replaced by `Path.expanduser()` --> import_as.py:18:6 @@ -288,6 +289,7 @@ help: Replace with `Path(...).is_dir()` 21 | bbbb = foo_p.isfile(p) 22 | bbbbb = foo_p.islink(p) 23 | foo.readlink(p) +note: This is an unsafe fix and may change runtime behavior PTH113 [*] `os.path.isfile()` should be replaced by `Path.is_file()` --> import_as.py:20:8 @@ -315,6 +317,7 @@ help: Replace with `Path(...).is_file()` 22 | bbbbb = foo_p.islink(p) 23 | foo.readlink(p) 24 | foo.stat(p) +note: This is an unsafe fix and may change runtime behavior PTH114 [*] `os.path.islink()` should be replaced by `Path.is_symlink()` --> import_as.py:21:9 @@ -342,6 +345,7 @@ help: Replace with `Path(...).is_symlink()` 23 | foo.readlink(p) 24 | foo.stat(p) 25 | foo_p.isabs(p) +note: This is an unsafe fix and may change runtime behavior PTH115 [*] `os.readlink()` should be replaced by `Path.readlink()` --> import_as.py:22:1 diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_from.py.snap b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_from.py.snap index 236b00ff4c..fd705d7ca9 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_from.py.snap +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_from.py.snap @@ -240,6 +240,7 @@ help: Replace with `Path(...).exists()` 21 | bb = expanduser(p) 22 | bbb = isdir(p) 23 | bbbb = isfile(p) +note: This is an unsafe fix and may change runtime behavior PTH111 [*] `os.path.expanduser()` should be replaced by `Path.expanduser()` --> import_from.py:20:6 @@ -297,6 +298,7 @@ help: Replace with `Path(...).is_dir()` 23 | bbbb = isfile(p) 24 | bbbbb = islink(p) 25 | readlink(p) +note: This is an unsafe fix and may change runtime behavior PTH113 [*] `os.path.isfile()` should be replaced by `Path.is_file()` --> import_from.py:22:8 @@ -325,6 +327,7 @@ help: Replace with `Path(...).is_file()` 24 | bbbbb = islink(p) 25 | readlink(p) 26 | stat(p) +note: This is an unsafe fix and may change runtime behavior PTH114 [*] `os.path.islink()` should be replaced by `Path.is_symlink()` --> import_from.py:23:9 @@ -353,6 +356,7 @@ help: Replace with `Path(...).is_symlink()` 25 | readlink(p) 26 | stat(p) 27 | isabs(p) +note: This is an unsafe fix and may change runtime behavior PTH115 [*] `os.readlink()` should be replaced by `Path.readlink()` --> import_from.py:24:1 diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_from_as.py.snap b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_from_as.py.snap index e037400a27..2cb66fce50 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_from_as.py.snap +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_from_as.py.snap @@ -240,6 +240,7 @@ help: Replace with `Path(...).exists()` 26 | bb = xexpanduser(p) 27 | bbb = xisdir(p) 28 | bbbb = xisfile(p) +note: This is an unsafe fix and may change runtime behavior PTH111 [*] `os.path.expanduser()` should be replaced by `Path.expanduser()` --> import_from_as.py:25:6 @@ -297,6 +298,7 @@ help: Replace with `Path(...).is_dir()` 28 | bbbb = xisfile(p) 29 | bbbbb = xislink(p) 30 | xreadlink(p) +note: This is an unsafe fix and may change runtime behavior PTH113 [*] `os.path.isfile()` should be replaced by `Path.is_file()` --> import_from_as.py:27:8 @@ -325,6 +327,7 @@ help: Replace with `Path(...).is_file()` 29 | bbbbb = xislink(p) 30 | xreadlink(p) 31 | xstat(p) +note: This is an unsafe fix and may change runtime behavior PTH114 [*] `os.path.islink()` should be replaced by `Path.is_symlink()` --> import_from_as.py:28:9 @@ -353,6 +356,7 @@ help: Replace with `Path(...).is_symlink()` 30 | xreadlink(p) 31 | xstat(p) 32 | xisabs(p) +note: This is an unsafe fix and may change runtime behavior PTH115 [*] `os.readlink()` should be replaced by `Path.readlink()` --> import_from_as.py:29:1 From 4e31a60e287eea35a9b08571f213166bfc5665a9 Mon Sep 17 00:00:00 2001 From: Dan Date: Fri, 14 Nov 2025 15:53:39 -0500 Subject: [PATCH 2/2] Refactor, clarify documentation --- .../src/rules/flake8_use_pathlib/helpers.rs | 19 +++---------------- .../flake8_use_pathlib/rules/os_getcwd.rs | 16 +++++++--------- .../rules/os_path_exists.rs | 3 ++- .../rules/os_path_isfile.rs | 3 ++- .../rules/os_path_islink.rs | 3 ++- .../flake8_use_pathlib/rules/os_readlink.rs | 5 ++--- .../flake8_use_pathlib/rules/os_rename.rs | 5 ++--- .../flake8_use_pathlib/rules/os_replace.rs | 5 ++--- ..._pathlib__tests__preview_full_name.py.snap | 4 ---- ..._pathlib__tests__preview_import_as.py.snap | 4 ---- ...athlib__tests__preview_import_from.py.snap | 4 ---- ...lib__tests__preview_import_from_as.py.snap | 4 ---- 12 files changed, 22 insertions(+), 53 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs index 036ced3bfe..1f804f204d 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs @@ -1,4 +1,4 @@ -use ruff_python_ast::{self as ast, Arguments, Expr, ExprCall, Stmt}; +use ruff_python_ast::{self as ast, Arguments, Expr, ExprCall}; use ruff_python_semantic::{SemanticModel, analyze::typing}; use ruff_text_size::Ranged; @@ -226,19 +226,6 @@ pub(crate) fn is_argument_non_default(arguments: &Arguments, name: &str, positio /// Returns `true` if the given call is a top-level expression in its statement. /// This means the call's return value is not used, so return type changes don't matter. -pub(crate) fn is_top_level_expression_call(checker: &Checker, call: &ExprCall) -> bool { - if let Stmt::Expr(ast::StmtExpr { - value: child, - range: _, - node_index: _, - }) = checker.semantic().current_statement() - { - // Check if the call is the same expression as the statement's value - // We compare by checking if the call's range is contained within the child's range - // and if they're the same expression node - if let Expr::Call(call_expr) = child.as_ref() { - return call_expr.range() == call.range(); - } - } - false +pub(crate) fn is_top_level_expression_call(checker: &Checker, _call: &ExprCall) -> bool { + checker.semantic().current_expression_parent().is_none() } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_getcwd.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_getcwd.rs index d0bfcc47d8..8da822d6f7 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_getcwd.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_getcwd.rs @@ -38,9 +38,8 @@ use ruff_text_size::Ranged; /// /// ## Fix Safety /// This rule's fix is marked as unsafe if the replacement would remove comments attached to the original expression. -/// Additionally, the fix is marked as unsafe because `os.getcwd()` and `os.getcwdb()` return `str` or `bytes`, -/// while `Path.cwd()` returns a `Path` object. This change in return type can break code that uses the return value. -/// The fix is safe when the function call is a top-level expression in its statement (i.e., the return value is not used). +/// Additionally, the fix is marked as unsafe when the return value is used because the type changes +/// from `str` or `bytes` to a `Path` object. /// /// ## References /// - [Python documentation: `Path.cwd`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.cwd) @@ -87,14 +86,13 @@ pub(crate) fn os_getcwd(checker: &Checker, call: &ExprCall, segments: &[&str]) { checker.semantic(), )?; - let applicability = if checker.comment_ranges().intersects(range) { + // Unsafe when the fix would delete comments or change a used return value + let applicability = if checker.comment_ranges().intersects(range) + || !is_top_level_expression_call(checker, call) + { Applicability::Unsafe - } else if is_top_level_expression_call(checker, call) { - // Safe when the call is a top-level expression (return value not used) - Applicability::Safe } else { - // Unsafe because the return type changes (str/bytes -> Path) - Applicability::Unsafe + Applicability::Safe }; let replacement = format!("{binding}.cwd()"); diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_exists.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_exists.rs index f3fe32a641..6744ff7892 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_exists.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_exists.rs @@ -5,6 +5,7 @@ use crate::checkers::ast::Checker; use crate::preview::is_fix_os_path_exists_enabled; use crate::rules::flake8_use_pathlib::helpers::check_os_pathlib_single_arg_calls; use crate::{FixAvailability, Violation}; +use ruff_diagnostics::Applicability; /// ## What it does /// Checks for uses of `os.path.exists`. @@ -72,6 +73,6 @@ pub(crate) fn os_path_exists(checker: &Checker, call: &ExprCall, segments: &[&st "path", is_fix_os_path_exists_enabled(checker.settings()), OsPathExists, - None, + Some(Applicability::Safe), ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isfile.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isfile.rs index d31e39eef7..c2c997b2d0 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isfile.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isfile.rs @@ -5,6 +5,7 @@ use crate::checkers::ast::Checker; use crate::preview::is_fix_os_path_isfile_enabled; use crate::rules::flake8_use_pathlib::helpers::check_os_pathlib_single_arg_calls; use crate::{FixAvailability, Violation}; +use ruff_diagnostics::Applicability; /// ## What it does /// Checks for uses of `os.path.isfile`. @@ -73,6 +74,6 @@ pub(crate) fn os_path_isfile(checker: &Checker, call: &ExprCall, segments: &[&st "path", is_fix_os_path_isfile_enabled(checker.settings()), OsPathIsfile, - None, + Some(Applicability::Safe), ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_islink.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_islink.rs index d958a2c19c..0c46156b91 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_islink.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_islink.rs @@ -5,6 +5,7 @@ use crate::checkers::ast::Checker; use crate::preview::is_fix_os_path_islink_enabled; use crate::rules::flake8_use_pathlib::helpers::check_os_pathlib_single_arg_calls; use crate::{FixAvailability, Violation}; +use ruff_diagnostics::Applicability; /// ## What it does /// Checks for uses of `os.path.islink`. @@ -73,6 +74,6 @@ pub(crate) fn os_path_islink(checker: &Checker, call: &ExprCall, segments: &[&st "path", is_fix_os_path_islink_enabled(checker.settings()), OsPathIslink, - None, + Some(Applicability::Safe), ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_readlink.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_readlink.rs index c5471223e3..9612742aaa 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_readlink.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_readlink.rs @@ -38,9 +38,8 @@ use crate::{FixAvailability, Violation}; /// /// ## Fix Safety /// This rule's fix is marked as unsafe if the replacement would remove comments attached to the original expression. -/// Additionally, the fix is marked as unsafe because `os.readlink()` returns a `str`, while `Path.readlink()` returns a `Path` object. -/// This change in return type can break code that uses the return value. -/// The fix is safe when the function call is a top-level expression in its statement (i.e., the return value is not used). +/// Additionally, the fix is marked as unsafe when the return value is used because the type changes +/// from `str` to a `Path` object. /// /// ## References /// - [Python documentation: `Path.readlink`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.readline) diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_rename.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_rename.rs index f3f9caf041..90261d4451 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_rename.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_rename.rs @@ -38,9 +38,8 @@ use ruff_python_ast::ExprCall; /// /// ## Fix Safety /// This rule's fix is marked as unsafe if the replacement would remove comments attached to the original expression. -/// Additionally, the fix is marked as unsafe because `os.rename()` returns `None`, while `Path.rename()` returns a `Path` object. -/// This change in return type can break code that uses the return value. -/// The fix is safe when the function call is a top-level expression in its statement (i.e., the return value is not used). +/// Additionally, the fix is marked as unsafe when the return value is used because the type changes +/// from `None` to a `Path` object. /// /// ## References /// - [Python documentation: `Path.rename`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.rename) diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_replace.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_replace.rs index a7d6db6f23..8fdefeae6f 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_replace.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_replace.rs @@ -41,9 +41,8 @@ use ruff_python_ast::ExprCall; /// /// ## Fix Safety /// This rule's fix is marked as unsafe if the replacement would remove comments attached to the original expression. -/// Additionally, the fix is marked as unsafe because `os.replace()` returns `None`, while `Path.replace()` returns a `Path` object. -/// This change in return type can break code that uses the return value. -/// The fix is safe when the function call is a top-level expression in its statement (i.e., the return value is not used). +/// Additionally, the fix is marked as unsafe when the return value is used because the type changes +/// from `None` to a `Path` object. /// /// ## References /// - [Python documentation: `Path.replace`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.replace) diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_full_name.py.snap b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_full_name.py.snap index 9711df1a18..7292a3b5d2 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_full_name.py.snap +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_full_name.py.snap @@ -233,7 +233,6 @@ help: Replace with `Path(...).exists()` 19 | bb = os.path.expanduser(p) 20 | bbb = os.path.isdir(p) 21 | bbbb = os.path.isfile(p) -note: This is an unsafe fix and may change runtime behavior PTH111 [*] `os.path.expanduser()` should be replaced by `Path.expanduser()` --> full_name.py:18:6 @@ -289,7 +288,6 @@ help: Replace with `Path(...).is_dir()` 21 | bbbb = os.path.isfile(p) 22 | bbbbb = os.path.islink(p) 23 | os.readlink(p) -note: This is an unsafe fix and may change runtime behavior PTH113 [*] `os.path.isfile()` should be replaced by `Path.is_file()` --> full_name.py:20:8 @@ -317,7 +315,6 @@ help: Replace with `Path(...).is_file()` 22 | bbbbb = os.path.islink(p) 23 | os.readlink(p) 24 | os.stat(p) -note: This is an unsafe fix and may change runtime behavior PTH114 [*] `os.path.islink()` should be replaced by `Path.is_symlink()` --> full_name.py:21:9 @@ -345,7 +342,6 @@ help: Replace with `Path(...).is_symlink()` 23 | os.readlink(p) 24 | os.stat(p) 25 | os.path.isabs(p) -note: This is an unsafe fix and may change runtime behavior PTH115 [*] `os.readlink()` should be replaced by `Path.readlink()` --> full_name.py:22:1 diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_as.py.snap b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_as.py.snap index e7590393ff..de63cbde9d 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_as.py.snap +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_as.py.snap @@ -233,7 +233,6 @@ help: Replace with `Path(...).exists()` 19 | bb = foo_p.expanduser(p) 20 | bbb = foo_p.isdir(p) 21 | bbbb = foo_p.isfile(p) -note: This is an unsafe fix and may change runtime behavior PTH111 [*] `os.path.expanduser()` should be replaced by `Path.expanduser()` --> import_as.py:18:6 @@ -289,7 +288,6 @@ help: Replace with `Path(...).is_dir()` 21 | bbbb = foo_p.isfile(p) 22 | bbbbb = foo_p.islink(p) 23 | foo.readlink(p) -note: This is an unsafe fix and may change runtime behavior PTH113 [*] `os.path.isfile()` should be replaced by `Path.is_file()` --> import_as.py:20:8 @@ -317,7 +315,6 @@ help: Replace with `Path(...).is_file()` 22 | bbbbb = foo_p.islink(p) 23 | foo.readlink(p) 24 | foo.stat(p) -note: This is an unsafe fix and may change runtime behavior PTH114 [*] `os.path.islink()` should be replaced by `Path.is_symlink()` --> import_as.py:21:9 @@ -345,7 +342,6 @@ help: Replace with `Path(...).is_symlink()` 23 | foo.readlink(p) 24 | foo.stat(p) 25 | foo_p.isabs(p) -note: This is an unsafe fix and may change runtime behavior PTH115 [*] `os.readlink()` should be replaced by `Path.readlink()` --> import_as.py:22:1 diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_from.py.snap b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_from.py.snap index fd705d7ca9..236b00ff4c 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_from.py.snap +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_from.py.snap @@ -240,7 +240,6 @@ help: Replace with `Path(...).exists()` 21 | bb = expanduser(p) 22 | bbb = isdir(p) 23 | bbbb = isfile(p) -note: This is an unsafe fix and may change runtime behavior PTH111 [*] `os.path.expanduser()` should be replaced by `Path.expanduser()` --> import_from.py:20:6 @@ -298,7 +297,6 @@ help: Replace with `Path(...).is_dir()` 23 | bbbb = isfile(p) 24 | bbbbb = islink(p) 25 | readlink(p) -note: This is an unsafe fix and may change runtime behavior PTH113 [*] `os.path.isfile()` should be replaced by `Path.is_file()` --> import_from.py:22:8 @@ -327,7 +325,6 @@ help: Replace with `Path(...).is_file()` 24 | bbbbb = islink(p) 25 | readlink(p) 26 | stat(p) -note: This is an unsafe fix and may change runtime behavior PTH114 [*] `os.path.islink()` should be replaced by `Path.is_symlink()` --> import_from.py:23:9 @@ -356,7 +353,6 @@ help: Replace with `Path(...).is_symlink()` 25 | readlink(p) 26 | stat(p) 27 | isabs(p) -note: This is an unsafe fix and may change runtime behavior PTH115 [*] `os.readlink()` should be replaced by `Path.readlink()` --> import_from.py:24:1 diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_from_as.py.snap b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_from_as.py.snap index 2cb66fce50..e037400a27 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_from_as.py.snap +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_from_as.py.snap @@ -240,7 +240,6 @@ help: Replace with `Path(...).exists()` 26 | bb = xexpanduser(p) 27 | bbb = xisdir(p) 28 | bbbb = xisfile(p) -note: This is an unsafe fix and may change runtime behavior PTH111 [*] `os.path.expanduser()` should be replaced by `Path.expanduser()` --> import_from_as.py:25:6 @@ -298,7 +297,6 @@ help: Replace with `Path(...).is_dir()` 28 | bbbb = xisfile(p) 29 | bbbbb = xislink(p) 30 | xreadlink(p) -note: This is an unsafe fix and may change runtime behavior PTH113 [*] `os.path.isfile()` should be replaced by `Path.is_file()` --> import_from_as.py:27:8 @@ -327,7 +325,6 @@ help: Replace with `Path(...).is_file()` 29 | bbbbb = xislink(p) 30 | xreadlink(p) 31 | xstat(p) -note: This is an unsafe fix and may change runtime behavior PTH114 [*] `os.path.islink()` should be replaced by `Path.is_symlink()` --> import_from_as.py:28:9 @@ -356,7 +353,6 @@ help: Replace with `Path(...).is_symlink()` 30 | xreadlink(p) 31 | xstat(p) 32 | xisabs(p) -note: This is an unsafe fix and may change runtime behavior PTH115 [*] `os.readlink()` should be replaced by `Path.readlink()` --> import_from_as.py:29:1