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..1f804f204d 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs @@ -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,9 @@ 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 { + 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 7bb533246d..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 @@ -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,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 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) @@ -83,7 +86,10 @@ 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 { Applicability::Safe 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_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_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_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 d1df572ed5..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,6 +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 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 c5f2293ee9..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,6 +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 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 ef60099467..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,6 +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 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)