From bd82aa256e0776e788ec99e7d0f4f7e6d3281139 Mon Sep 17 00:00:00 2001 From: Dan Date: Thu, 13 Nov 2025 17:18:36 -0500 Subject: [PATCH 1/5] 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 3ba017ecb76ab..036ced3bfe6d2 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 7bb533246daf3..d0bfcc47d8a3f 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 9b58561e889ee..667da0d54a9d1 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 d3175c2035f32..9860b54e69556 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 d544acde39e16..4a6accfb93b5e 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 d1df572ed57e5..c5471223e3d71 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 c5f2293ee9261..f3f9caf041f8e 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 ef60099467acb..a7d6db6f2301e 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 7292a3b5d286f..9711df1a18c76 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 de63cbde9ddf1..e7590393ffcb6 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 236b00ff4ca10..fd705d7ca9252 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 e037400a27e6d..2cb66fce502b3 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/5] 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 036ced3bfe6d2..1f804f204dd67 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 d0bfcc47d8a3f..8da822d6f7cff 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 f3fe32a6418d6..6744ff789231b 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 d31e39eef79aa..c2c997b2d076b 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 d958a2c19c6d2..0c46156b9194e 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 c5471223e3d71..9612742aaa02e 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 f3f9caf041f8e..90261d4451fa9 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 a7d6db6f2301e..8fdefeae6ff87 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 9711df1a18c76..7292a3b5d286f 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 e7590393ffcb6..de63cbde9ddf1 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 fd705d7ca9252..236b00ff4ca10 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 2cb66fce502b3..e037400a27e6d 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 From d2581671206f517055713c9ba8f5833012fce66c Mon Sep 17 00:00:00 2001 From: Dan Date: Wed, 19 Nov 2025 19:15:55 -0500 Subject: [PATCH 3/5] Refactor applicability handling --- .../src/rules/flake8_use_pathlib/helpers.rs | 23 +++++-------------- .../rules/os_path_abspath.rs | 4 ++-- .../rules/os_path_dirname.rs | 4 ++-- .../rules/os_path_expanduser.rs | 4 ++-- .../flake8_use_pathlib/rules/os_path_isabs.rs | 3 ++- .../rules/os_path_isfile.rs | 2 +- .../rules/os_path_samefile.rs | 1 + .../flake8_use_pathlib/rules/os_readlink.rs | 16 ++++++++++--- .../flake8_use_pathlib/rules/os_rename.rs | 22 ++++++++++++++++-- .../flake8_use_pathlib/rules/os_replace.rs | 12 +++++++++- 10 files changed, 60 insertions(+), 31 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 1f804f204dd67..4a95cb20da7ba 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs @@ -96,18 +96,9 @@ pub(crate) fn check_os_pathlib_single_arg_calls( _ => { 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 + applicability.unwrap_or(Applicability::Safe) }; Fix::applicable_edits(edit, [import_edit], determined_applicability) } @@ -148,6 +139,7 @@ pub(crate) fn is_file_descriptor(expr: &Expr, semantic: &SemanticModel) -> bool typing::is_int(binding, semantic) } +#[allow(clippy::too_many_arguments)] pub(crate) fn check_os_pathlib_two_arg_calls( checker: &Checker, call: &ExprCall, @@ -156,6 +148,7 @@ pub(crate) fn check_os_pathlib_two_arg_calls( second_arg: &str, fix_enabled: bool, violation: impl Violation, + applicability: Option, ) { let range = call.range(); let mut diagnostic = checker.report_diagnostic(violation, call.func.range()); @@ -184,20 +177,16 @@ pub(crate) fn check_os_pathlib_two_arg_calls( format!("{binding}({path_code}).{attr}({second_code})") }; - let applicability = if checker.comment_ranges().intersects(range) { + let determined_applicability = if checker.comment_ranges().intersects(range) { 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 (None -> Path) - Applicability::Unsafe + applicability.unwrap_or(Applicability::Safe) }; Ok(Fix::applicable_edits( Edit::range_replacement(replacement, range), [import_edit], - applicability, + determined_applicability, )) }); } 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 667da0d54a9d1..9fa06329db5ae 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,8 +45,8 @@ 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 +/// Additionally, the fix is marked as unsafe because `os.path.abspath()` returns `str` or `bytes`, +/// while `Path.resolve()` returns a `Path` object. This change in return type can break code that uses /// the return value. /// /// ## References 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 9860b54e69556..69fd22524ebc6 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,8 +42,8 @@ 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 +/// Additionally, the fix is marked as unsafe because `os.path.dirname()` returns `str` or `bytes`, +/// while `Path.parent` returns a `Path` object. This change in return type can break code that uses /// the return value. /// /// ## Known issues 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 4a6accfb93b5e..a5971ba2613c3 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,8 +41,8 @@ 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 +/// Additionally, the fix is marked as unsafe because `os.path.expanduser()` returns `str` or `bytes`, +/// while `Path.expanduser()` returns a `Path` object. This change in return type can break code that uses /// the return value. /// /// ## References diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isabs.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isabs.rs index b1c8cb33c308d..2c8e8a11567f9 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isabs.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isabs.rs @@ -1,3 +1,4 @@ +use ruff_diagnostics::Applicability; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::ExprCall; @@ -71,6 +72,6 @@ pub(crate) fn os_path_isabs(checker: &Checker, call: &ExprCall, segments: &[&str "s", is_fix_os_path_isabs_enabled(checker.settings()), OsPathIsabs, - 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 c2c997b2d076b..b088ad278c700 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 @@ -1,3 +1,4 @@ +use ruff_diagnostics::Applicability; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::ExprCall; @@ -5,7 +6,6 @@ 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`. diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_samefile.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_samefile.rs index cbf6d7a034c62..5ad99cd6aac67 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_samefile.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_samefile.rs @@ -79,5 +79,6 @@ pub(crate) fn os_path_samefile(checker: &Checker, call: &ExprCall, segments: &[& "f2", fix_enabled, OsPathSamefile, + None, ); } 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 9612742aaa02e..16e7813c5b0c8 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 @@ -5,8 +5,10 @@ use crate::checkers::ast::Checker; use crate::preview::is_fix_os_readlink_enabled; use crate::rules::flake8_use_pathlib::helpers::{ check_os_pathlib_single_arg_calls, is_keyword_only_argument_non_default, + is_top_level_expression_call, }; use crate::{FixAvailability, Violation}; +use ruff_diagnostics::Applicability; /// ## What it does /// Checks for uses of `os.readlink`. @@ -39,7 +41,7 @@ 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 when the return value is used because the type changes -/// from `str` to a `Path` object. +/// from `str` or `bytes` to a `Path` object. /// /// ## References /// - [Python documentation: `Path.readlink`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.readline) @@ -84,13 +86,21 @@ pub(crate) fn os_readlink(checker: &Checker, call: &ExprCall, segments: &[&str]) return; } + let fix_enabled = is_fix_os_readlink_enabled(checker.settings()); + let applicability = if fix_enabled && !is_top_level_expression_call(checker, call) { + // Unsafe because the return type changes (str/bytes -> Path) + Some(Applicability::Unsafe) + } else { + None + }; + check_os_pathlib_single_arg_calls( checker, call, "readlink()", "path", - is_fix_os_readlink_enabled(checker.settings()), + fix_enabled, OsReadlink, - None, + applicability, ); } 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 90261d4451fa9..f9222434cf2db 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 @@ -2,9 +2,10 @@ use crate::checkers::ast::Checker; use crate::preview::is_fix_os_rename_enabled; use crate::rules::flake8_use_pathlib::helpers::{ check_os_pathlib_two_arg_calls, has_unknown_keywords_or_starred_expr, - is_keyword_only_argument_non_default, + is_keyword_only_argument_non_default, is_top_level_expression_call, }; use crate::{FixAvailability, Violation}; +use ruff_diagnostics::Applicability; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::ExprCall; @@ -89,5 +90,22 @@ pub(crate) fn os_rename(checker: &Checker, call: &ExprCall, segments: &[&str]) { &["src", "dst", "src_dir_fd", "dst_dir_fd"], ); - check_os_pathlib_two_arg_calls(checker, call, "rename", "src", "dst", fix_enabled, OsRename); + // Unsafe when the fix would delete comments or change a used return value + let applicability = if fix_enabled && !is_top_level_expression_call(checker, call) { + // Unsafe because the return type changes (None -> Path) + Some(Applicability::Unsafe) + } else { + None + }; + + check_os_pathlib_two_arg_calls( + checker, + call, + "rename", + "src", + "dst", + fix_enabled, + OsRename, + applicability, + ); } 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 8fdefeae6ff87..9b0a97cd8529a 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 @@ -2,9 +2,10 @@ use crate::checkers::ast::Checker; use crate::preview::is_fix_os_replace_enabled; use crate::rules::flake8_use_pathlib::helpers::{ check_os_pathlib_two_arg_calls, has_unknown_keywords_or_starred_expr, - is_keyword_only_argument_non_default, + is_keyword_only_argument_non_default, is_top_level_expression_call, }; use crate::{FixAvailability, Violation}; +use ruff_diagnostics::Applicability; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::ExprCall; @@ -92,6 +93,14 @@ pub(crate) fn os_replace(checker: &Checker, call: &ExprCall, segments: &[&str]) &["src", "dst", "src_dir_fd", "dst_dir_fd"], ); + // Unsafe when the fix would delete comments or change a used return value + let applicability = if fix_enabled && !is_top_level_expression_call(checker, call) { + // Unsafe because the return type changes (None -> Path) + Some(Applicability::Unsafe) + } else { + None + }; + check_os_pathlib_two_arg_calls( checker, call, @@ -100,5 +109,6 @@ pub(crate) fn os_replace(checker: &Checker, call: &ExprCall, segments: &[&str]) "dst", fix_enabled, OsReplace, + applicability, ); } From 28aa1098797065e3c5c7a5604288d41fa3245d6d Mon Sep 17 00:00:00 2001 From: Dan Date: Fri, 28 Nov 2025 20:08:42 -0500 Subject: [PATCH 4/5] Apply feedback --- .../src/rules/flake8_use_pathlib/helpers.rs | 21 +++++++++---------- .../flake8_use_pathlib/rules/os_getcwd.rs | 2 +- .../rules/os_path_abspath.rs | 4 ++-- .../rules/os_path_basename.rs | 2 +- .../rules/os_path_dirname.rs | 4 ++-- .../rules/os_path_exists.rs | 4 ++-- .../rules/os_path_expanduser.rs | 4 ++-- .../rules/os_path_getatime.rs | 3 ++- .../rules/os_path_getctime.rs | 3 ++- .../rules/os_path_getmtime.rs | 3 ++- .../rules/os_path_getsize.rs | 3 ++- .../flake8_use_pathlib/rules/os_path_isabs.rs | 2 +- .../flake8_use_pathlib/rules/os_path_isdir.rs | 3 ++- .../rules/os_path_isfile.rs | 2 +- .../rules/os_path_islink.rs | 2 +- .../rules/os_path_samefile.rs | 8 ++++--- .../flake8_use_pathlib/rules/os_readlink.rs | 10 ++++----- .../flake8_use_pathlib/rules/os_remove.rs | 3 ++- .../flake8_use_pathlib/rules/os_rename.rs | 6 +++--- .../flake8_use_pathlib/rules/os_replace.rs | 6 +++--- .../flake8_use_pathlib/rules/os_rmdir.rs | 3 ++- .../flake8_use_pathlib/rules/os_unlink.rs | 3 ++- 22 files changed, 55 insertions(+), 46 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 4a95cb20da7ba..e161cd5da114e 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs @@ -57,7 +57,7 @@ pub(crate) fn check_os_pathlib_single_arg_calls( fn_argument: &str, fix_enabled: bool, violation: impl Violation, - applicability: Option, + applicability: Applicability, ) { if call.arguments.len() != 1 { return; @@ -92,13 +92,12 @@ pub(crate) fn check_os_pathlib_single_arg_calls( let edit = Edit::range_replacement(replacement, range); let fix = match applicability { - Some(Applicability::Unsafe) => Fix::unsafe_edits(edit, [import_edit]), + Applicability::Unsafe => Fix::unsafe_edits(edit, [import_edit]), _ => { - let determined_applicability = if checker.comment_ranges().intersects(range) { - Applicability::Unsafe - } else { - // applicability is Some(Applicability::Safe), use it - applicability.unwrap_or(Applicability::Safe) + let determined_applicability = match applicability { + Applicability::DisplayOnly => Applicability::DisplayOnly, + _ if checker.comment_ranges().intersects(range) => Applicability::Unsafe, + _ => applicability, }; Fix::applicable_edits(edit, [import_edit], determined_applicability) } @@ -139,7 +138,7 @@ pub(crate) fn is_file_descriptor(expr: &Expr, semantic: &SemanticModel) -> bool typing::is_int(binding, semantic) } -#[allow(clippy::too_many_arguments)] +#[expect(clippy::too_many_arguments)] pub(crate) fn check_os_pathlib_two_arg_calls( checker: &Checker, call: &ExprCall, @@ -148,7 +147,7 @@ pub(crate) fn check_os_pathlib_two_arg_calls( second_arg: &str, fix_enabled: bool, violation: impl Violation, - applicability: Option, + applicability: Applicability, ) { let range = call.range(); let mut diagnostic = checker.report_diagnostic(violation, call.func.range()); @@ -180,7 +179,7 @@ pub(crate) fn check_os_pathlib_two_arg_calls( let determined_applicability = if checker.comment_ranges().intersects(range) { Applicability::Unsafe } else { - applicability.unwrap_or(Applicability::Safe) + applicability }; Ok(Fix::applicable_edits( @@ -215,6 +214,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 { +pub(crate) fn is_top_level_expression_call(checker: &Checker) -> 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 8da822d6f7cff..00b82a7ec88ca 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 @@ -88,7 +88,7 @@ pub(crate) fn os_getcwd(checker: &Checker, call: &ExprCall, segments: &[&str]) { // 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) + || !is_top_level_expression_call(checker) { Applicability::Unsafe } else { 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 9fa06329db5ae..b51ce5cc6d52c 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,7 +45,7 @@ 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 `str` or `bytes`, +/// Additionally, the fix is marked as unsafe because `os.path.abspath()` returns `str` or `bytes` (`AnyStr`), /// while `Path.resolve()` returns a `Path` object. This change in return type can break code that uses /// the return value. /// @@ -89,6 +89,6 @@ pub(crate) fn os_path_abspath(checker: &Checker, call: &ExprCall, segments: &[&s "path", is_fix_os_path_abspath_enabled(checker.settings()), OsPathAbspath, - Some(Applicability::Unsafe), + Applicability::Unsafe, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_basename.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_basename.rs index ca69d07ce347b..c11c0ac114cf3 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_basename.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_basename.rs @@ -82,6 +82,6 @@ pub(crate) fn os_path_basename(checker: &Checker, call: &ExprCall, segments: &[& "p", is_fix_os_path_basename_enabled(checker.settings()), OsPathBasename, - Some(Applicability::Unsafe), + Applicability::Unsafe, ); } 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 69fd22524ebc6..69b44738f49c3 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,7 +42,7 @@ 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 `str` or `bytes`, +/// Additionally, the fix is marked as unsafe because `os.path.dirname()` returns `str` or `bytes` (`AnyStr`), /// while `Path.parent` returns a `Path` object. This change in return type can break code that uses /// the return value. /// @@ -86,6 +86,6 @@ pub(crate) fn os_path_dirname(checker: &Checker, call: &ExprCall, segments: &[&s "p", is_fix_os_path_dirname_enabled(checker.settings()), OsPathDirname, - Some(Applicability::Unsafe), + Applicability::Unsafe, ); } 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 6744ff789231b..2b130c72d0fc9 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 @@ -1,3 +1,4 @@ +use ruff_diagnostics::Applicability; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::ExprCall; @@ -5,7 +6,6 @@ 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`. @@ -73,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, - Some(Applicability::Safe), + Applicability::Safe, ); } 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 a5971ba2613c3..2b1fdb8980657 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,7 +41,7 @@ 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 `str` or `bytes`, +/// Additionally, the fix is marked as unsafe because `os.path.expanduser()` returns `str` or `bytes` (`AnyStr`), /// while `Path.expanduser()` returns a `Path` object. This change in return type can break code that uses /// the return value. /// @@ -80,6 +80,6 @@ pub(crate) fn os_path_expanduser(checker: &Checker, call: &ExprCall, segments: & "path", is_fix_os_path_expanduser_enabled(checker.settings()), OsPathExpanduser, - Some(Applicability::Unsafe), + Applicability::Unsafe, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getatime.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getatime.rs index 0f148f403382d..eb8fd1f989eb0 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getatime.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getatime.rs @@ -1,3 +1,4 @@ +use ruff_diagnostics::Applicability; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::ExprCall; @@ -75,6 +76,6 @@ pub(crate) fn os_path_getatime(checker: &Checker, call: &ExprCall, segments: &[& "filename", is_fix_os_path_getatime_enabled(checker.settings()), OsPathGetatime, - None, + Applicability::Safe, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getctime.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getctime.rs index 86bce28aed079..3739391711963 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getctime.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getctime.rs @@ -1,3 +1,4 @@ +use ruff_diagnostics::Applicability; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::ExprCall; @@ -76,6 +77,6 @@ pub(crate) fn os_path_getctime(checker: &Checker, call: &ExprCall, segments: &[& "filename", is_fix_os_path_getctime_enabled(checker.settings()), OsPathGetctime, - None, + Applicability::Safe, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getmtime.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getmtime.rs index 42e77e3fe91cd..2853a83986b4a 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getmtime.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getmtime.rs @@ -1,3 +1,4 @@ +use ruff_diagnostics::Applicability; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::ExprCall; @@ -76,6 +77,6 @@ pub(crate) fn os_path_getmtime(checker: &Checker, call: &ExprCall, segments: &[& "filename", is_fix_os_path_getmtime_enabled(checker.settings()), OsPathGetmtime, - None, + Applicability::Safe, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getsize.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getsize.rs index a945b2224ccfd..7c17e687df598 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getsize.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getsize.rs @@ -1,3 +1,4 @@ +use ruff_diagnostics::Applicability; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::ExprCall; @@ -76,6 +77,6 @@ pub(crate) fn os_path_getsize(checker: &Checker, call: &ExprCall, segments: &[&s "filename", is_fix_os_path_getsize_enabled(checker.settings()), OsPathGetsize, - None, + Applicability::Safe, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isabs.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isabs.rs index 2c8e8a11567f9..0fcbdf3f06428 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isabs.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isabs.rs @@ -72,6 +72,6 @@ pub(crate) fn os_path_isabs(checker: &Checker, call: &ExprCall, segments: &[&str "s", is_fix_os_path_isabs_enabled(checker.settings()), OsPathIsabs, - Some(Applicability::Safe), + Applicability::Safe, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isdir.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isdir.rs index a2c1b8620fcd4..9f0de09476db6 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isdir.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isdir.rs @@ -1,3 +1,4 @@ +use ruff_diagnostics::Applicability; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::ExprCall; @@ -73,6 +74,6 @@ pub(crate) fn os_path_isdir(checker: &Checker, call: &ExprCall, segments: &[&str "s", is_fix_os_path_isdir_enabled(checker.settings()), OsPathIsdir, - None, + 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 b088ad278c700..fc723cbd2f46c 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 @@ -74,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, - Some(Applicability::Safe), + 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 0c46156b9194e..221a08cbd4ef2 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 @@ -74,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, - Some(Applicability::Safe), + Applicability::Safe, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_samefile.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_samefile.rs index 5ad99cd6aac67..af4ee0b605b09 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_samefile.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_samefile.rs @@ -1,11 +1,13 @@ +use ruff_diagnostics::Applicability; +use ruff_macros::{ViolationMetadata, derive_message_formats}; +use ruff_python_ast::ExprCall; + use crate::checkers::ast::Checker; use crate::preview::is_fix_os_path_samefile_enabled; use crate::rules::flake8_use_pathlib::helpers::{ check_os_pathlib_two_arg_calls, has_unknown_keywords_or_starred_expr, }; use crate::{FixAvailability, Violation}; -use ruff_macros::{ViolationMetadata, derive_message_formats}; -use ruff_python_ast::ExprCall; /// ## What it does /// Checks for uses of `os.path.samefile`. @@ -79,6 +81,6 @@ pub(crate) fn os_path_samefile(checker: &Checker, call: &ExprCall, segments: &[& "f2", fix_enabled, OsPathSamefile, - None, + 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 16e7813c5b0c8..0e0c6f6f4c630 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 @@ -1,3 +1,4 @@ +use ruff_diagnostics::Applicability; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::{ExprCall, PythonVersion}; @@ -8,7 +9,6 @@ use crate::rules::flake8_use_pathlib::helpers::{ is_top_level_expression_call, }; use crate::{FixAvailability, Violation}; -use ruff_diagnostics::Applicability; /// ## What it does /// Checks for uses of `os.readlink`. @@ -41,7 +41,7 @@ use ruff_diagnostics::Applicability; /// ## 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 when the return value is used because the type changes -/// from `str` or `bytes` to a `Path` object. +/// from `str` or `bytes` (`AnyStr`) to a `Path` object. /// /// ## References /// - [Python documentation: `Path.readlink`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.readline) @@ -87,11 +87,11 @@ pub(crate) fn os_readlink(checker: &Checker, call: &ExprCall, segments: &[&str]) } let fix_enabled = is_fix_os_readlink_enabled(checker.settings()); - let applicability = if fix_enabled && !is_top_level_expression_call(checker, call) { + let applicability = if fix_enabled && !is_top_level_expression_call(checker) { // Unsafe because the return type changes (str/bytes -> Path) - Some(Applicability::Unsafe) + Applicability::Unsafe } else { - None + Applicability::Safe }; check_os_pathlib_single_arg_calls( diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_remove.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_remove.rs index 43852e11e2d12..c25d52de21e45 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_remove.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_remove.rs @@ -1,3 +1,4 @@ +use ruff_diagnostics::Applicability; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::ExprCall; @@ -84,6 +85,6 @@ pub(crate) fn os_remove(checker: &Checker, call: &ExprCall, segments: &[&str]) { "path", is_fix_os_remove_enabled(checker.settings()), OsRemove, - None, + Applicability::Safe, ); } 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 f9222434cf2db..7993fcaf7a119 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 @@ -91,11 +91,11 @@ pub(crate) fn os_rename(checker: &Checker, call: &ExprCall, segments: &[&str]) { ); // Unsafe when the fix would delete comments or change a used return value - let applicability = if fix_enabled && !is_top_level_expression_call(checker, call) { + let applicability = if fix_enabled && !is_top_level_expression_call(checker) { // Unsafe because the return type changes (None -> Path) - Some(Applicability::Unsafe) + Applicability::Unsafe } else { - None + Applicability::Safe }; check_os_pathlib_two_arg_calls( 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 9b0a97cd8529a..10bc5c2596303 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 @@ -94,11 +94,11 @@ pub(crate) fn os_replace(checker: &Checker, call: &ExprCall, segments: &[&str]) ); // Unsafe when the fix would delete comments or change a used return value - let applicability = if fix_enabled && !is_top_level_expression_call(checker, call) { + let applicability = if !is_top_level_expression_call(checker) { // Unsafe because the return type changes (None -> Path) - Some(Applicability::Unsafe) + Applicability::Unsafe } else { - None + Applicability::Safe }; check_os_pathlib_two_arg_calls( diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_rmdir.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_rmdir.rs index a044e541b9658..7d7a72812dded 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_rmdir.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_rmdir.rs @@ -1,3 +1,4 @@ +use ruff_diagnostics::Applicability; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::ExprCall; @@ -84,6 +85,6 @@ pub(crate) fn os_rmdir(checker: &Checker, call: &ExprCall, segments: &[&str]) { "path", is_fix_os_rmdir_enabled(checker.settings()), OsRmdir, - None, + Applicability::Safe, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_unlink.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_unlink.rs index 9f4902546581d..28568cf479b67 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_unlink.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_unlink.rs @@ -1,3 +1,4 @@ +use ruff_diagnostics::Applicability; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::ExprCall; @@ -84,6 +85,6 @@ pub(crate) fn os_unlink(checker: &Checker, call: &ExprCall, segments: &[&str]) { "path", is_fix_os_unlink_enabled(checker.settings()), OsUnlink, - None, + Applicability::Safe, ); } From 6ca60ab830c212952f20de3d1830b67daea35de8 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 1 Dec 2025 14:47:59 -0500 Subject: [PATCH 5/5] nits --- .../src/rules/flake8_use_pathlib/helpers.rs | 26 ++++++++----------- .../flake8_use_pathlib/rules/os_getcwd.rs | 9 ++++--- .../rules/os_path_islink.rs | 2 +- .../flake8_use_pathlib/rules/os_readlink.rs | 5 ++-- .../flake8_use_pathlib/rules/os_rename.rs | 9 ++++--- .../flake8_use_pathlib/rules/os_replace.rs | 7 ++--- 6 files changed, 28 insertions(+), 30 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 e161cd5da114e..24d9daee25491 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs @@ -91,18 +91,14 @@ pub(crate) fn check_os_pathlib_single_arg_calls( let edit = Edit::range_replacement(replacement, range); - let fix = match applicability { - Applicability::Unsafe => Fix::unsafe_edits(edit, [import_edit]), - _ => { - let determined_applicability = match applicability { - Applicability::DisplayOnly => Applicability::DisplayOnly, - _ if checker.comment_ranges().intersects(range) => Applicability::Unsafe, - _ => applicability, - }; - Fix::applicable_edits(edit, [import_edit], determined_applicability) - } + let applicability = match applicability { + Applicability::DisplayOnly => Applicability::DisplayOnly, + _ if checker.comment_ranges().intersects(range) => Applicability::Unsafe, + _ => applicability, }; + let fix = Fix::applicable_edits(edit, [import_edit], applicability); + Ok(fix) }); } @@ -176,16 +172,16 @@ pub(crate) fn check_os_pathlib_two_arg_calls( format!("{binding}({path_code}).{attr}({second_code})") }; - let determined_applicability = if checker.comment_ranges().intersects(range) { - Applicability::Unsafe - } else { - applicability + let applicability = match applicability { + Applicability::DisplayOnly => Applicability::DisplayOnly, + _ if checker.comment_ranges().intersects(range) => Applicability::Unsafe, + _ => applicability, }; Ok(Fix::applicable_edits( Edit::range_replacement(replacement, range), [import_edit], - determined_applicability, + applicability, )) }); } 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 00b82a7ec88ca..4174b5825a1fd 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,12 +1,13 @@ +use ruff_diagnostics::{Applicability, Edit, Fix}; +use ruff_macros::{ViolationMetadata, derive_message_formats}; +use ruff_python_ast::ExprCall; +use ruff_text_size::Ranged; + 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}; -use ruff_python_ast::ExprCall; -use ruff_text_size::Ranged; /// ## What it does /// Checks for uses of `os.getcwd` and `os.getcwdb`. 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 221a08cbd4ef2..f64aa7713bed5 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 @@ -1,3 +1,4 @@ +use ruff_diagnostics::Applicability; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::ExprCall; @@ -5,7 +6,6 @@ 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`. 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 0e0c6f6f4c630..1505e62a77323 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 @@ -86,8 +86,7 @@ pub(crate) fn os_readlink(checker: &Checker, call: &ExprCall, segments: &[&str]) return; } - let fix_enabled = is_fix_os_readlink_enabled(checker.settings()); - let applicability = if fix_enabled && !is_top_level_expression_call(checker) { + let applicability = if !is_top_level_expression_call(checker) { // Unsafe because the return type changes (str/bytes -> Path) Applicability::Unsafe } else { @@ -99,7 +98,7 @@ pub(crate) fn os_readlink(checker: &Checker, call: &ExprCall, segments: &[&str]) call, "readlink()", "path", - fix_enabled, + is_fix_os_readlink_enabled(checker.settings()), OsReadlink, applicability, ); 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 7993fcaf7a119..523eada663f68 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 @@ -1,3 +1,7 @@ +use ruff_diagnostics::Applicability; +use ruff_macros::{ViolationMetadata, derive_message_formats}; +use ruff_python_ast::ExprCall; + use crate::checkers::ast::Checker; use crate::preview::is_fix_os_rename_enabled; use crate::rules::flake8_use_pathlib::helpers::{ @@ -5,9 +9,6 @@ use crate::rules::flake8_use_pathlib::helpers::{ is_keyword_only_argument_non_default, is_top_level_expression_call, }; use crate::{FixAvailability, Violation}; -use ruff_diagnostics::Applicability; -use ruff_macros::{ViolationMetadata, derive_message_formats}; -use ruff_python_ast::ExprCall; /// ## What it does /// Checks for uses of `os.rename`. @@ -91,7 +92,7 @@ pub(crate) fn os_rename(checker: &Checker, call: &ExprCall, segments: &[&str]) { ); // Unsafe when the fix would delete comments or change a used return value - let applicability = if fix_enabled && !is_top_level_expression_call(checker) { + let applicability = if !is_top_level_expression_call(checker) { // Unsafe because the return type changes (None -> Path) Applicability::Unsafe } else { 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 10bc5c2596303..c1211a24a56ae 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 @@ -1,3 +1,7 @@ +use ruff_diagnostics::Applicability; +use ruff_macros::{ViolationMetadata, derive_message_formats}; +use ruff_python_ast::ExprCall; + use crate::checkers::ast::Checker; use crate::preview::is_fix_os_replace_enabled; use crate::rules::flake8_use_pathlib::helpers::{ @@ -5,9 +9,6 @@ use crate::rules::flake8_use_pathlib::helpers::{ is_keyword_only_argument_non_default, is_top_level_expression_call, }; use crate::{FixAvailability, Violation}; -use ruff_diagnostics::Applicability; -use ruff_macros::{ViolationMetadata, derive_message_formats}; -use ruff_python_ast::ExprCall; /// ## What it does /// Checks for uses of `os.replace`.