[flake8-use-pathlib] Make PTH100 fix unsafe because it can change behavior

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

@@ -1,9 +1,15 @@
+use ruff_diagnostics::{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_path_abspath_enabled;
-use crate::rules::flake8_use_pathlib::helpers::check_os_pathlib_single_arg_calls;
+use crate::rules::flake8_use_pathlib::helpers::{
+    has_unknown_keywords_or_starred_expr, is_pathlib_path_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.path.abspath`.
@@ -34,13 +40,18 @@ use ruff_python_ast::ExprCall;
 /// especially on older versions of Python.
 ///
 /// ## Fix Safety
-/// This rule's fix is marked as unsafe if the replacement would remove comments attached to the original expression.
+/// This rule's fix is always marked as unsafe because `Path.resolve()` resolves symlinks, while
+/// `os.path.abspath()` does not. If resolving symlinks is important, you may need to use
+/// `Path.absolute()`. However, `Path.absolute()` also does not remove any `..` components in a
+/// path, unlike `os.path.abspath()` and `Path.resolve()`, so if that specific combination of
+/// behaviors is required, there's no existing `pathlib` alternative. See CPython issue
+/// [#69200](https://github.com/python/cpython/issues/69200).
 ///
 /// ## 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)
 /// - [PEP 428 – The pathlib module – object-oriented filesystem paths](https://peps.python.org/pep-0428/)
-/// - [Correspondence between `os` and `pathlib`](https://docs.python.org/3/library/pathlib.html#correspondence-to-tools-in-the-os-module)
+/// - [Correspondence between `os` and `pathlib`](https://docs.python.org/3/library/pathlib.html#corresponding-tools)
 /// - [Why you should be using pathlib](https://treyhunner.com/2018/12/why-you-should-be-using-pathlib/)
 /// - [No really, pathlib is great](https://treyhunner.com/2019/01/no-really-pathlib-is-great/)
 #[derive(ViolationMetadata)]
@@ -63,12 +74,44 @@ pub(crate) fn os_path_abspath(checker: &Checker, call: &ExprCall, segments: &[&s
     if segments != ["os", "path", "abspath"] {
         return;
     }
-    check_os_pathlib_single_arg_calls(
-        checker,
-        call,
-        "resolve()",
-        "path",
-        is_fix_os_path_abspath_enabled(checker.settings()),
-        OsPathAbspath,
-    );
+
+    if call.arguments.len() != 1 {
+        return;
+    }
+
+    let Some(arg) = call.arguments.find_argument_value("path", 0) else {
+        return;
+    };
+
+    let arg_code = checker.locator().slice(arg.range());
+    let range = call.range();
+
+    let mut diagnostic = checker.report_diagnostic(OsPathAbspath, call.func.range());
+
+    if has_unknown_keywords_or_starred_expr(&call.arguments, &["path"]) {
+        return;
+    }
+
+    if !is_fix_os_path_abspath_enabled(checker.settings()) {
+        return;
+    }
+
+    diagnostic.try_set_fix(|| {
+        let (import_edit, binding) = checker.importer().get_or_import_symbol(
+            &ImportRequest::import("pathlib", "Path"),
+            call.start(),
+            checker.semantic(),
+        )?;
+
+        let replacement = if is_pathlib_path_call(checker, arg) {
+            format!("{arg_code}.resolve()")
+        } else {
+            format!("{binding}({arg_code}).resolve()")
+        };
+
+        Ok(Fix::unsafe_edits(
+            Edit::range_replacement(replacement, range),
+            [import_edit],
+        ))
+    });
 }

crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_full_name.py.snap

@@ -13,7 +13,7 @@ PTH100 [*] `os.path.abspath()` should be replaced by `Path.resolve()`
   |
 help: Replace with `Path(...).resolve()`
 
-ℹ Safe fix
+ℹ Unsafe fix
 1 1 | import os
 2 2 | import os.path
   3 |+import pathlib

crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_as.py.snap

@@ -13,7 +13,7 @@ PTH100 [*] `os.path.abspath()` should be replaced by `Path.resolve()`
   |
 help: Replace with `Path(...).resolve()`
 
-ℹ Safe fix
+ℹ Unsafe fix
 1 1 | import os as foo
 2 2 | import os.path as foo_p
   3 |+import pathlib

crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_from.py.snap

@@ -13,7 +13,7 @@ PTH100 [*] `os.path.abspath()` should be replaced by `Path.resolve()`
    |
 help: Replace with `Path(...).resolve()`
 
-ℹ Safe fix
+ℹ Unsafe fix
 2  2  | from os import remove, unlink, getcwd, readlink, stat
 3  3  | from os.path import abspath, exists, expanduser, isdir, isfile, islink
 4  4  | from os.path import isabs, join, basename, dirname, samefile, splitext

crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_from_as.py.snap

@@ -13,7 +13,7 @@ PTH100 [*] `os.path.abspath()` should be replaced by `Path.resolve()`
    |
 help: Replace with `Path(...).resolve()`
 
-ℹ Safe fix
+ℹ Unsafe fix
 7  7  | from os.path import isfile as xisfile, islink as xislink, isabs as xisabs
 8  8  | from os.path import join as xjoin, basename as xbasename, dirname as xdirname
 9  9  | from os.path import samefile as xsamefile, splitext as xsplitext