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

## Summary

Marks fixes as unsafe when they change return types (`None` → `Path`,
`str`/`bytes` → `Path`, `str` → `Path`), except when the call is a
top-level expression.

Fixes #21431.

## Problem

Fixes for `os.rename`, `os.replace`, `os.getcwd`/`os.getcwdb`, and
`os.readlink` were marked safe despite changing return types, which can
break code that uses the return value.

## Approach

Added `is_top_level_expression_call` helper to detect when a call is a
top-level expression (return value unused). Updated
`check_os_pathlib_two_arg_calls` and `check_os_pathlib_single_arg_calls`
to mark fixes as unsafe unless the call is a top-level expression.
Updated PTH109 to use the helper for applicability determination.

## Test Plan

Updated snapshots for `preview_full_name.py`, `preview_import_as.py`,
`preview_import_from.py`, and `preview_import_from_as.py` to reflect
unsafe markers.

---------

Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>

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: Applicability,
 ) {
     if call.arguments.len() != 1 {
         return;
@@ -91,18 +91,14 @@ pub(crate) fn check_os_pathlib_single_arg_calls(
 
         let edit = Edit::range_replacement(replacement, range);
 
-        let fix = match applicability {
+        let applicability = match applicability {
-            Some(Applicability::Unsafe) => Fix::unsafe_edits(edit, [import_edit]),
+            Applicability::DisplayOnly => Applicability::DisplayOnly,
-            _ => {
+            _ if checker.comment_ranges().intersects(range) => Applicability::Unsafe,
-                let applicability = if checker.comment_ranges().intersects(range) {
+            _ => applicability,
-                    Applicability::Unsafe
-                } else {
-                    Applicability::Safe
-                };
-                Fix::applicable_edits(edit, [import_edit], applicability)
-            }
         };
 
+        let fix = Fix::applicable_edits(edit, [import_edit], applicability);
+
         Ok(fix)
     });
 }
@@ -138,6 +134,7 @@ pub(crate) fn is_file_descriptor(expr: &Expr, semantic: &SemanticModel) -> bool
     typing::is_int(binding, semantic)
 }
 
+#[expect(clippy::too_many_arguments)]
 pub(crate) fn check_os_pathlib_two_arg_calls(
     checker: &Checker,
     call: &ExprCall,
@@ -146,6 +143,7 @@ pub(crate) fn check_os_pathlib_two_arg_calls(
     second_arg: &str,
     fix_enabled: bool,
     violation: impl Violation,
+    applicability: Applicability,
 ) {
     let range = call.range();
     let mut diagnostic = checker.report_diagnostic(violation, call.func.range());
@@ -174,10 +172,10 @@ 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 applicability = match applicability {
-                Applicability::Unsafe
+                Applicability::DisplayOnly => Applicability::DisplayOnly,
-            } else {
+                _ if checker.comment_ranges().intersects(range) => Applicability::Unsafe,
-                Applicability::Safe
+                _ => applicability,
             };
 
             Ok(Fix::applicable_edits(
@@ -209,3 +207,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) -> bool {
+    checker.semantic().current_expression_parent().is_none()
+}

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

@@ -1,12 +1,14 @@
-use crate::checkers::ast::Checker;
-use crate::importer::ImportRequest;
-use crate::preview::is_fix_os_getcwd_enabled;
-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;
 
+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};
+
 /// ## What it does
 /// Checks for uses of `os.getcwd` and `os.getcwdb`.
 ///
@@ -37,6 +39,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 +87,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)
+            {
                 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` (`AnyStr`),
+/// 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)
@@ -85,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,
     );
 }

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

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` (`AnyStr`),
+/// 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,
@@ -82,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,
     );
 }

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;
 
@@ -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,
+        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` (`AnyStr`),
+/// 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)
@@ -76,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,
     );
 }

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

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

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

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

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,
+        Applicability::Safe,
     );
 }

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

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,
+        Applicability::Safe,
     );
 }

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;
 
@@ -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,
+        Applicability::Safe,
     );
 }

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,5 +81,6 @@ pub(crate) fn os_path_samefile(checker: &Checker, call: &ExprCall, segments: &[&
         "f2",
         fix_enabled,
         OsPathSamefile,
+        Applicability::Safe,
     );
 }

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};
 
@@ -5,6 +6,7 @@ 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};
 
@@ -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` (`AnyStr`) to a `Path` object.
 ///
 /// ## References
 /// - [Python documentation: `Path.readlink`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.readline)
@@ -82,6 +86,13 @@ pub(crate) fn os_readlink(checker: &Checker, call: &ExprCall, segments: &[&str])
         return;
     }
 
+    let applicability = if !is_top_level_expression_call(checker) {
+        // Unsafe because the return type changes (str/bytes -> Path)
+        Applicability::Unsafe
+    } else {
+        Applicability::Safe
+    };
+
     check_os_pathlib_single_arg_calls(
         checker,
         call,
@@ -89,6 +100,6 @@ pub(crate) fn os_readlink(checker: &Checker, call: &ExprCall, segments: &[&str])
         "path",
         is_fix_os_readlink_enabled(checker.settings()),
         OsReadlink,
-        None,
+        applicability,
     );
 }

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

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

@@ -1,12 +1,14 @@
+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::{
     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_macros::{ViolationMetadata, derive_message_formats};
-use ruff_python_ast::ExprCall;
 
 /// ## What it does
 /// Checks for uses of `os.rename`.
@@ -38,6 +40,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 +91,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 !is_top_level_expression_call(checker) {
+        // Unsafe because the return type changes (None -> Path)
+        Applicability::Unsafe
+    } else {
+        Applicability::Safe
+    };
+
+    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

@@ -1,12 +1,14 @@
+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::{
     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_macros::{ViolationMetadata, derive_message_formats};
-use ruff_python_ast::ExprCall;
 
 /// ## What it does
 /// Checks for uses of `os.replace`.
@@ -41,6 +43,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 +94,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 !is_top_level_expression_call(checker) {
+        // Unsafe because the return type changes (None -> Path)
+        Applicability::Unsafe
+    } else {
+        Applicability::Safe
+    };
+
     check_os_pathlib_two_arg_calls(
         checker,
         call,
@@ -98,5 +110,6 @@ pub(crate) fn os_replace(checker: &Checker, call: &ExprCall, segments: &[&str])
         "dst",
         fix_enabled,
         OsReplace,
+        applicability,
     );
 }

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

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