[flake8-use-pathlib] Mark fixes unsafe for return type changes (PTH104, PTH105, PTH109, PTH115)

crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs

@@ -94,12 +94,13 @@ 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 {
-                    Applicability::Safe
+                    // applicability is Some(Applicability::Safe), use it
+                    applicability.unwrap_or(Applicability::Safe)
                 };
-                Fix::applicable_edits(edit, [import_edit], applicability)
+                Fix::applicable_edits(edit, [import_edit], determined_applicability)
             }
         };
 
@@ -138,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,
@@ -146,6 +148,7 @@ pub(crate) fn check_os_pathlib_two_arg_calls(
     second_arg: &str,
     fix_enabled: bool,
     violation: impl Violation,
+    applicability: Option<Applicability>,
 ) {
     let range = call.range();
     let mut diagnostic = checker.report_diagnostic(violation, call.func.range());
@@ -174,16 +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 {
-                Applicability::Safe
+                applicability.unwrap_or(Applicability::Safe)
             };
 
             Ok(Fix::applicable_edits(
                 Edit::range_replacement(replacement, range),
                 [import_edit],
-                applicability,
+                determined_applicability,
             ))
         });
     }
@@ -209,3 +212,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()
+}

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

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 `str` or `bytes`,
+/// 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)

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 `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
 /// 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,

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),
     );
 }

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 `str` or `bytes`,
+/// 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)

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),
     );
 }

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;
 
@@ -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),
     );
 }

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),
     );
 }

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,
     );
 }

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`.
@@ -38,6 +40,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` or `bytes` to a `Path` object.
 ///
 /// ## References
 /// - [Python documentation: `Path.readlink`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.readline)
@@ -82,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,
     );
 }

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;
 
@@ -38,6 +39,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)
@@ -87,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,
+    );
 }

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;
 
@@ -41,6 +42,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)
@@ -90,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,
@@ -98,5 +109,6 @@ pub(crate) fn os_replace(checker: &Checker, call: &ExprCall, segments: &[&str])
         "dst",
         fix_enabled,
         OsReplace,
+        applicability,
     );
 }